Skip to content

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
Dokploy:canaryfrom
tanaymishra:fix/decrypt-legacy-auth-secret
Open

fix: keep legacy auth-secret key as a decrypt fallback so migrating BETTER_AUTH_SECRET does not orphan encrypted env vars#4834
tanaymishra wants to merge 2 commits into
Dokploy:canaryfrom
tanaymishra:fix/decrypt-legacy-auth-secret

Conversation

@tanaymishra

Copy link
Copy Markdown
Contributor

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 existing env, buildArgs and buildSecrets value 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 to prepareEnvironmentVariables, and dotenv.parse turns 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 uses primaryKey, 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 the ENCRYPTION_KEY adoption 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:

  • You created a dedicated branch based on the canary branch.
  • You have read the suggestions in the CONTRIBUTING.md file https://github.com/Dokploy/dokploy/blob/canary/CONTRIBUTING.md#pull-request
  • You have tested this PR in your local instance. Added a regression test that reproduces the bug: it fails on canary (the migrated instance cannot read values written under the legacy secret) and passes with this change. Full __test__/env suite passes (80 tests), @dokploy/server typecheck 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.

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
@tanaymishra
tanaymishra requested a review from Siumauricio as a code owner July 16, 2026 06:31
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

1 participant