Skip to content

fix(encryption): keep env vars readable after moving off the legacy auth secret - #5718

Open
Gourav224 wants to merge 2 commits into
Dokploy:canaryfrom
Gourav224:fix/legacy-auth-secret-decrypt-fallback
Open

Gourav224 wants to merge 2 commits into
Dokploy:canaryfrom
Gourav224:fix/legacy-auth-secret-decrypt-fallback

Conversation

@Gourav224

@Gourav224 Gourav224 commented Oct 20, 2026 •

Copy link
Copy Markdown

What is this PR about?

Installs without BETTER_AUTH_SECRET encrypt env vars with a key derived from the hardcoded legacy secret. When you follow the startup warning and set a real secret, the key changes, and every existing env, buildArgs and buildSecrets value becomes undecryptable (#4833). It then fails silently: encryptedText returns the ciphertext, dotenv parses enc:v1:… to {}, and the service deploys with an empty environment while the deployment shows done.

Two small changes:

  1. Decrypt-only fallback for the legacy key (packages/server/src/lib/encryption.ts). decryptValue also tries deriveKey(HARDCODED_LEGACY_SECRET) when it isn't already the primary key. This is the same pattern the code already uses when adopting ENCRYPTION_KEY. Encryption still always uses the primary key, so values re-encrypt with the real secret on their next write. The legacy secret is a published constant, so this exposes nothing new. The key is not added to exportEncryptionKeys() (backups).
  2. Fail the deployment instead of deploying an empty environment (prepareEnvironmentVariables). If a service, project or environment env still starts with enc:v1:, it throws a clear error. The error lands in the deployment log, like the existing unresolved-vault-reference check. This covers any other key mismatch too, such as a restore under a different secret.

Rotating between two real secrets is still out of scope. That needs a re-encryption step, as migrate-auth-secret.ts does for 2FA, and is a bigger change.

Checklist

Before submitting this PR, please make sure that:

Testing (real AES-GCM, no crypto mocks):

  • __test__/env/encryption.test.ts: a value encrypted on the legacy secret decrypts after BETTER_AUTH_SECRET is set; new writes use the new secret; the legacy key isn't exported; values from an unrelated earlier secret still fail.
  • __test__/env/shared.test.ts: prepareEnvironmentVariables throws for encrypted service, project and environment env.
  • Full vitest suite: 1294 passed. application.real.test.ts needs nixpacks and was excluded; it fails the same way on canary. tsc --noEmit for packages/server passes.

Issues related (if applicable)

closes #4833

This follows the approach proposed in the issue thread (#4834, closed by its author without being merged).

🤖 Generated with Claude Code

RetriggerView in GreptileConfidence Score: 4/5

Fix the Docker Compose path before merging so unreadable service variables stop the deployment.

Summary

This PR keeps values encrypted with the legacy auth secret readable after a secret change.

  • Values saved with the old default secret stay readable after the secret changes.
  • Environment preparation stops when a saved value is still encrypted.

Reviews (1) · Last reviewed commit: "fix(encryption): keep env vars readable ..." · Reviewed by Greptile

…uth secret

Setting BETTER_AUTH_SECRET, as the startup warning asks, changed the
encryption key, so every value encrypted under the hardcoded legacy
secret became undecryptable. The column then returned ciphertext, dotenv
parsed it to nothing, and services deployed with an empty environment.

Keep the legacy-secret key as a decrypt-only fallback (it is public, and
values re-encrypt with the real key on the next write), and fail the
deployment when environment variables still arrive encrypted.

Closes Dokploy#4833

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment on lines +473 to +477
if (source && isEncrypted(source)) {
throw new Error(
"Environment variables could not be decrypted. BETTER_AUTH_SECRET or ENCRYPTION_KEY changed after they were saved; restore the previous secret or re-enter the variables.",
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Compose still drops saved variables

For Docker Compose deployments, getCreateEnvFileCommand adds APP_NAME and COMPOSE_PROJECT_NAME before compose.env. If the saved service variables cannot be decrypted, the resulting string no longer starts with enc:v1:, so this check misses it. With readable project and environment settings, dotenv drops the encrypted line and the deployment can report success without the saved service variables.

Check the original compose.env before adding generated variables, and add a test through getCreateEnvFileCommand.

Knowledge Base Used:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, fixed in 90ba1e2. The guard in prepareEnvironmentVariables now checks every line for enc:v1:, not just the start of the string. Ciphertext is base64 and always on its own line, so this covers compose's generated APP_NAME/COMPOSE_PROJECT_NAME prefix and any other caller that combines env text. I added tests for both the prefixed string and getCreateEnvFileCommand with an undecryptable compose.env.

…pose

getCreateEnvFileCommand prepends APP_NAME and COMPOSE_PROJECT_NAME, so a
check on the start of the string missed the ciphertext. Check every line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant