fix(config): clear every blank compose forward so data/.env.generated is not shadowed - #1039
Merged
Merged
Conversation
… is not shadowed
docker-compose.yml forwards a variable as `- KEY=${KEY:-}` so a real host value reaches the
container. With nothing set, that renders an empty value, which still occupies process.env and
blocks the project .env and data/.env.generated layers — both loaded with dotenv override:false,
which will not replace a key that is already present, blank or not.
clearBlankEnv deletes those blanks before the file layers load, but its key list had drifted
behind the compose file. Eight forwarded variables had no entry, so a value set for any of them
in data/.env.generated was discarded with no error: AUTO_START_SESSIONS, BODY_SIZE_LIMIT,
API_MASTER_KEY, TRUSTED_PROXIES, CSP_UPGRADE_INSECURE_REQUESTS, WWEBJS_WEB_VERSION,
WWEBJS_WEB_VERSION_REMOTE_PATH and WWEBJS_AUTH_TIMEOUT_MS.
AUTO_START_SESSIONS is the one operators noticed. In 0.11.1 and earlier it had no route into the
container at all; 0.12.0 added the forward without the clear entry, which relocated the failure
rather than ending it. The flag still resolved to off, SessionService's bootstrap hook returned
before querying a session, and authenticated sessions stayed at disconnected with no engine and a
null lastError.
Every read site already treats a blank value as unset, so clearing the blank changes nothing for a
deployment that sets nothing — it only stops the blank from blocking the file layers.
The clear list is now covered by a test that derives the expected set from docker-compose.yml, so a
forward added without its clear entry fails in CI instead of shipping inert.
Closes #981
2 tasks
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
docker-compose.ymlforwards a variable as- KEY=${KEY:-}so that a real host value reaches thecontainer. When the operator sets nothing, that line does not omit the variable — it injects it
present and empty. An empty value still occupies
process.env, and both lower-priority layers,the project
.envanddata/.env.generated, are loaded with dotenvoverride: false, which refusesto replace a key that is already present, blank or not.
clearBlankEnvexists to delete those blanks before the file layers load. Its key list had driftedbehind the compose file: eight forwarded variables had no entry, so a value set for any of them in
data/.env.generatedwas discarded with no error and no warning.AUTO_START_SESSIONSBODY_SIZE_LIMITAPI_MASTER_KEYTRUSTED_PROXIESCSP_UPGRADE_INSECURE_REQUESTSWWEBJS_WEB_VERSIONWWEBJS_WEB_VERSION_REMOTE_PATHWWEBJS_AUTH_TIMEOUT_MSdata/.env.generatedis not an internal artefact — its first-run header tells operators to "modifythis file directly", and
saveConfigpreserves keys it does not own, so hand-set values persistacross dashboard saves and are expected to apply.
Why it surfaced now
AUTO_START_SESSIONSis the key operators noticed, reported in #981.In 0.11.1 and earlier the variable had no route into the container at all: there is no
env_file:entry and
.envis excluded from the build context. That is why the flag appeared inert across the0.7–0.11 range regardless of how it was set.
0.12.0 added the compose forward but not the matching clear entry, which relocated the failure rather
than ending it. The blank forward now actively shadows
data/.env.generated, so the flag stillresolved to off,
SessionService.onApplicationBootstrapreturned before querying a single session,and previously authenticated sessions stayed at
disconnectedwith no engine ever created and a nulllastError.Changes
src/config/env-precedence.ts— the eight missing keys are added toBLANK_SHADOWED_ENV_KEYS. Thedoc comment's rule is corrected: being dashboard-managed was never a second condition, because the
file is hand-editable. Every
${KEY:-}forward belongs in the list.src/config/env-precedence.spec.ts— two tests:forward and
AUTO_START_SESSIONS=trueindata/.env.generated,computeFeatureFlagsreportsautoStartSessions: true;docker-compose.ymlanddocker-compose.dev.yml, collects every- KEY=${KEY:-}forward and asserts each has a clear-list entry. It is paired with a guard testasserting the parser finds a known forward, so a pattern that matched nothing could not make it
vacuously pass.
Read sites were checked before adding each key: all eight already treat a blank value as unset
(
?.trim(),|| '', a falsy check, or an exact=== 'true'comparison), so clearing the blankchanges nothing for a deployment that sets nothing. It only stops the blank from blocking the file
layers.
docker-compose.dev.ymlwas already fully covered; it is added to the guard so it cannotdrift either.
Verification
Behaviour, on the shipped
v0.12.1image. A green unit test proves the code, not the deployment,so the mechanism was confirmed end to end against the published container image with a mounted
data/.env.generatedcontainingAUTO_START_SESSIONS=true:-e AUTO_START_SESSIONS=raw="" flag=false— file shadowedraw="true" flag=true— file applies-e AUTO_START_SESSIONS=raw="true" flag=true— file applies[Bootstrap] Loading saved configuration from: /app/data/.env.generatedis printed in all threecases, so A does not fail because the file went unread. Case B is the control that separates
"shadowed" from "never loaded".
The premise itself was also confirmed rather than assumed: on a live Compose deployment,
docker inspectshows blank-forwarded keys present with an empty value(
WWEBJS_WEB_VERSION=,TRUSTED_PROXIES=,S3_ENDPOINT=,BODY_SIZE_LIMIT=), anddocker compose configrenders them asBODY_SIZE_LIMIT: "". Compose injects the key empty; it doesnot omit it.
Tests.
drifted. The new test derives the expectation from the compose files instead.
- MUTANT_KEY=${MUTANT_KEY:-}line todocker-compose.yml,and a
- DEV_MUTANT=${DEV_MUTANT:-}line todocker-compose.dev.yml, each fail the invariant testnaming that key; removing the lines restores green.
(
Expected: true, Received: false) and the invariant test listing all eight keys.npm run lint,npm run build,npx tsc --noEmit,npm --prefix dashboard run buildclean;full Jest suite 251 suites / 4053 tests green.
Not covered here
Two adjacent problems were found while verifying this one and are deliberately left for separate
changes, since neither is fixed by clearing blanks:
.env.exampleshipsAUTO_START_SESSIONS=falseuncommented, and the setup docs saycp .env.example .env. An operator who followed that has a realfalseat top precedence, which iscorrect precedence but an unhelpful default — and it is one of 19 uncommented blank-forwarded keys in
that file, each of which pins a dashboard-managed setting on copy.
scripts/backup.shandscripts/restore.shdocument app-parity config resolution but readprocess.envonly, without the blank-clear or the two file layers.Closes #981