fix(docker): fail fast when prod compose secrets are unset - #5836
fix(docker): fail fast when prod compose secrets are unset#5836Souptik96 wants to merge 2 commits into
Conversation
Running the documented prod quickstart on a fresh checkout
(`docker compose -f docker-compose.prod.yml up`) starts the app with empty
BETTER_AUTH_SECRET, ENCRYPTION_KEY, and INTERNAL_API_SECRET. Those are passed
straight from the environment with no default (unlike POSTGRES_USER:-postgres),
and apps/sim/lib/core/config/env.ts declares all three as z.string().min(32).
Because createEnv runs with skipValidation: true, the empty values are not
rejected at boot; they surface later - ENCRYPTION_KEY throws in
lib/core/security/encryption.ts and Better Auth cannot sign sessions with an
empty secret - so creating the first admin account fails with a cleared screen
and no visible error.
Guard the three required secrets with Compose's ${VAR:?message} syntax in both
the simstudio and realtime services, so `docker compose up` aborts before
starting any container with a message naming the missing variable and how to
generate it (openssl rand -hex 32). No insecure defaults are introduced - the
failure is made clear and actionable instead of silent.
Also document the mandatory secrets in the Docker Compose quickstart in
README.md and .github/CONTRIBUTING.md (Option 2), matching the openssl
convention already used by the manual-setup instructions.
fixes simstudioai#5625
|
@Souptik96 is attempting to deploy a commit to the Sim Team on Vercel. A member of the Team first needs to authorize it. |
PR SummaryLow Risk Overview
Reviewed by Cursor Bugbot for commit 30467d3. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR makes production Compose fail early when mandatory secrets are missing. The main changes are:
Confidence Score: 4/5The quickstart can erase deployment settings and rotate persistent secrets when rerun in an existing checkout.
README.md and .github/CONTRIBUTING.md Important Files Changed
Reviews (1): Last reviewed commit: "fix(docker): fail fast when prod compose..." | Re-trigger Greptile |
| git clone https://github.com/simstudioai/sim.git && cd sim | ||
|
|
||
| # Generate the required secrets. Docker Compose reads them from this .env file. | ||
| cat > .env <<EOF |
There was a problem hiding this comment.
Existing Environment Is Truncated
Running this step in an existing checkout replaces the entire root .env, deleting settings such as POSTGRES_PASSWORD, TRUSTED_ORIGINS, REDIS_URL, and API keys. Rerunning it also rotates persistent auth and encryption secrets, which invalidates sessions and can make previously encrypted data unreadable; generate only missing entries or stop when .env already exists. The mirrored command in .github/CONTRIBUTING.md has the same behavior.
…prod-missing-secrets # Conflicts: # README.md # docker-compose.prod.yml
|
Updated — and @greptile-apps, the P1 was correct, thank you. The quickstart snippet I added used I removed both documentation hunks instead of making the snippet idempotent, because Also merged current The PR is now a single file, +5/-5, and the guard messages point at One caveat I want to be upfront about: since |
|
The reasoning is clear and the P1 acknowledgment is accurate. The current diff is exactly what it should be: the doc hunks are gone, the compose conflict is resolved cleanly, and all five guard sites are present. The change looks good to me. The approach is correct:
On the scope question: I'd merge this rather than close as "already fixed by setup script." The CONTRIBUTING.md at lines 169 and 199 still documents direct compose invocation, so there's a documented path that currently fails silently. This PR closes that gap with a one-liner per variable. The two fixes are complementary, not redundant. The one remaining caveat you called out — no end-to-end Docker verification — is the only open item. If a maintainer can run Tip: You can customize Greptile's behavior for this repo with |
Fixes #5625 —
docker compose -f docker-compose.prod.yml up -dstarts with empty auth secrets, so creating the first account fails with no usable error.Re-scoped 2026-07-28: this is now a one-file change. It was 3 files; the two documentation hunks have been removed. Details under "What changed since first review".
The problem
docker-compose.prod.ymlpassedBETTER_AUTH_SECRET,ENCRYPTION_KEYandINTERNAL_API_SECRETthrough with no default.env.tsdeclares themz.string().min(32), butcreateEnv({ skipValidation: true })lets empty values through, so the stack boots anyway:encryption.tsthen throws on an empty key and Better Auth cannot sign a session. The user sees a blank screen or a 500 on signup with nothing naming the cause.The change
${VAR:?message}guards on those three variables, in both thesimstudioandrealtimeservices. Compose now refuses to start and names the missing variable. No insecure defaults are introduced, and the error text points atbun run setup, which is the current documented way to get a populated.env.ENCRYPTION_KEY's message asks for exactly 64 hex characters, matching what the app actually enforces —scripts/setup/env-files.tsvalidates it against/^[0-9a-f]{64}$/because it is read as raw AES-256 material, so a merely-long passphrase passes a length check and then fails every encryption path at runtime.What changed since first review
Two things, both worth being explicit about.
1. The documentation hunks are gone, and @greptile-apps was right to flag them. The review noted a P1 on the quickstart step I had added: it wrote the root
.envwithcat > .env <<EOF, so re-running it in an existing checkout would truncate the file — droppingPOSTGRES_PASSWORD,TRUSTED_ORIGINS,REDIS_URLand API keys — and rotateBETTER_AUTH_SECRETandENCRYPTION_KEY, invalidating sessions and leaving previously encrypted data unreadable. That is a real defect and it was mine.I removed the hunks rather than patching them, because
bun run setupnow does this properly:scripts/setup/env-files.tsupserts individual keys into an existing file, seeds a missing one from.env.example, archives before destructive operations, and detects placeholder values. Hand-rolling the same thing in a README snippet would be a strictly worse duplicate, so the right move was to delete mine and leave the setup script as the single path.2. Rebased onto current
stagingby merge. The branch was 105 commits behind and had conflicts in bothREADME.mdanddocker-compose.prod.yml. The compose conflict is resolved keeping all of upstream's newer lines —AUTH_TRUSTED_PROXIES,REDIS_URLdefaulting toredis://redis:6379, and the:-defaults onCOPILOT_API_KEY/SIM_AGENT_API_URL— with the guards re-applied on top of them.README.mdand.github/CONTRIBUTING.mdare byte-identical tostaging.Scope, honestly
Because
bun run setupseeds the secrets for the documented path, the happy path for #5625 is arguably already covered upstream. What remains is the failure mode the issue actually reported: anyone who runs the compose file directly — which.github/CONTRIBUTING.mdstill documents at lines 169 and 199 — gets empty secrets and a silent failure. This turns that into one line naming the variable. If you would rather close #5625 as fixed by the setup script, that is a reasonable call and I have no objection.Verification
docker-compose.prod.ymlparses as YAML; all 33${...}interpolations are well-formed; all five guard sites confirmed present (simstudio: all three;realtime:BETTER_AUTH_SECRET+INTERNAL_API_SECRET).{,}or": ", so they cannot break Compose interpolation or YAML parsing.stagingis one file, +5/-5.docker compose configor boot the stack. The${VAR:?err}semantics are standard Compose behaviour, but a reviewer with Docker should confirm the abort path once. This limitation was in the original description too.