fix: keep legacy auth-secret key as a decrypt fallback so migrating BETTER_AUTH_SECRET does not orphan encrypted env vars#4834
Open
tanaymishra wants to merge 2 commits into
Conversation
Environment variables are encrypted at rest with a key derived from BETTER_AUTH_SECRET. The decryption keyring had no slot for a previous secret, so replacing the deprecated hardcoded default with a real secret made every existing env, buildArgs and buildSecrets value undecryptable. The startup banner asks every affected install to make exactly that change, and because decryption fails open the ciphertext then reaches dotenv, parses to an empty object, and the service redeploys with no environment variables while the deployment still reports success. HARDCODED_LEGACY_SECRET is a published constant, so keeping its derived key as a decrypt-only fallback discloses nothing that was not already derivable from the source, while letting values written under the legacy secret stay readable and lazily re-encrypt under the new key on the next write. This mirrors the fallback already in place for the ENCRYPTION_KEY adoption path. Encryption still always uses the primary key, so values written after the migration are protected by the real secret. Fixes Dokploy#4833
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR about?
Environment variables are encrypted at rest with a key derived from
BETTER_AUTH_SECRET. The decryption keyring had no slot for a previous secret, so replacing the deprecated hardcoded default with a real one made every existingenv,buildArgsandbuildSecretsvalue permanently undecryptable. The startup banner asks every affected install to make exactly that change ("CRITICAL SECURITY RISK... Please migrate to Docker Secrets"), so following Dokploy's own security advice was enough to trigger it.Because decryption fails open, the damage was silent: the raw
enc:v1:...ciphertext is returned, passed toprepareEnvironmentVariables, anddotenv.parseturns it into{}. The service is then redeployed with no environment variables at all and the deployment still reports success. Same end symptom as #4817, different trigger.This PR keeps
deriveKey(HARDCODED_LEGACY_SECRET)in the decryption keyring as a decrypt-only fallback. That constant is published in this repository, so it discloses nothing that was not already derivable by anyone with the source and a database dump, and it only ever applies to values that were already encrypted under it. Encryption still always usesprimaryKey, so values written after the migration are protected by the real secret, and existing values lazily re-encrypt under the new key on the next write. This mirrors the fallback already in place for theENCRYPTION_KEYadoption path, and the keyring is deduplicated so installs still on the legacy default are unaffected.Scope note: this fixes the migration path the product actively tells users to take. It does not add a general previous-secret rotation mechanism, and it does not change the fail-open polarity. Both are discussed in #4833 and I am happy to follow up on either if you want them addressed separately.
Checklist
Before submitting this PR, please make sure that:
canarybranch.__test__/envsuite passes (80 tests),@dokploy/servertypecheck is clean, and Biome reports no lint findings on the touched files.Issues related (if applicable)
closes #4833
Screenshots (if applicable)
Not applicable (backend encryption keyring). Behavior is covered by the added regression tests.