Skip to content

fix(docker): fail fast when prod compose secrets are unset - #5836

Open
Souptik96 wants to merge 2 commits into
simstudioai:stagingfrom
Souptik96:fix/5625-docker-prod-missing-secrets
Open

fix(docker): fail fast when prod compose secrets are unset#5836
Souptik96 wants to merge 2 commits into
simstudioai:stagingfrom
Souptik96:fix/5625-docker-prod-missing-secrets

Conversation

@Souptik96

@Souptik96 Souptik96 commented Jul 22, 2026

Copy link
Copy Markdown

Fixes #5625docker compose -f docker-compose.prod.yml up -d starts 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.yml passed BETTER_AUTH_SECRET, ENCRYPTION_KEY and INTERNAL_API_SECRET through with no default. env.ts declares them z.string().min(32), but createEnv({ skipValidation: true }) lets empty values through, so the stack boots anyway: encryption.ts then 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 the simstudio and realtime services. Compose now refuses to start and names the missing variable. No insecure defaults are introduced, and the error text points at bun run setup, which is the current documented way to get a populated .env.

$ docker compose -f docker-compose.prod.yml up -d
error: BETTER_AUTH_SECRET is required. Run bun run setup, or set it in .env (32+ characters)

ENCRYPTION_KEY's message asks for exactly 64 hex characters, matching what the app actually enforces — scripts/setup/env-files.ts validates 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 .env with cat > .env <<EOF, so re-running it in an existing checkout would truncate the file — dropping POSTGRES_PASSWORD, TRUSTED_ORIGINS, REDIS_URL and API keys — and rotate BETTER_AUTH_SECRET and ENCRYPTION_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 setup now does this properly: scripts/setup/env-files.ts upserts 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 staging by merge. The branch was 105 commits behind and had conflicts in both README.md and docker-compose.prod.yml. The compose conflict is resolved keeping all of upstream's newer lines — AUTH_TRUSTED_PROXIES, REDIS_URL defaulting to redis://redis:6379, and the :- defaults on COPILOT_API_KEY / SIM_AGENT_API_URL — with the guards re-applied on top of them. README.md and .github/CONTRIBUTING.md are byte-identical to staging.

Scope, honestly

Because bun run setup seeds 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.md still 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.yml parses as YAML; all 33 ${...} interpolations are well-formed; all five guard sites confirmed present (simstudio: all three; realtime: BETTER_AUTH_SECRET + INTERNAL_API_SECRET).
  • Guard messages contain no {, } or ": ", so they cannot break Compose interpolation or YAML parsing.
  • Net diff against staging is one file, +5/-5.
  • Not verified end to end: I have no Docker on this machine (non-admin Windows), so I could not run docker compose config or 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.
  • The failing Vercel check is the fork-PR authorization gate ("A member of the Team first needs to authorize it"), not a defect in this change.

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
@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

@Souptik96 is attempting to deploy a commit to the Sim Team on Vercel.

A member of the Team first needs to authorize it.

@cursor

cursor Bot commented Jul 22, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Compose-only deployment guardrails; no application code, auth logic, or data paths change.

Overview
Production Compose no longer starts simstudio or realtime with empty auth/crypto secrets. BETTER_AUTH_SECRET, ENCRYPTION_KEY, and INTERNAL_API_SECRET now use ${VAR:?…} interpolation so docker compose aborts before containers start when those variables are missing, with a message pointing to bun run setup or .env instead of failing later at first admin signup.

realtime gets the same guard for BETTER_AUTH_SECRET and INTERNAL_API_SECRET; ENCRYPTION_KEY is required only on simstudio. No insecure defaults were added.

Reviewed by Cursor Bugbot for commit 30467d3. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes production Compose fail early when mandatory secrets are missing. The main changes are:

  • Adds required-variable guards to the Sim and realtime services.
  • Generates the three required secrets in the README quickstart.
  • Mirrors the secret setup in the contributor guide.

Confidence Score: 4/5

The quickstart can erase deployment settings and rotate persistent secrets when rerun in an existing checkout.

  • The Compose guards consistently cover the secrets used by each service.
  • Both documented setup commands replace the complete root .env.
  • Secret rotation can invalidate sessions and break access to encrypted data.

README.md and .github/CONTRIBUTING.md

Important Files Changed

Filename Overview
docker-compose.prod.yml Adds non-empty secret guards to both production services.
README.md Documents secret generation but replaces any existing root environment file.
.github/CONTRIBUTING.md Mirrors the quickstart and its destructive environment-file replacement.

Reviews (1): Last reviewed commit: "fix(docker): fail fast when prod compose..." | Re-trigger Greptile

Comment thread README.md Outdated
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

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 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
@Souptik96

Copy link
Copy Markdown
Author

Updated — and @greptile-apps, the P1 was correct, thank you.

The quickstart snippet I added used cat > .env <<EOF, so re-running it in an existing checkout would have truncated the file (losing POSTGRES_PASSWORD, TRUSTED_ORIGINS, REDIS_URL, API keys) and rotated BETTER_AUTH_SECRET and ENCRYPTION_KEY — the second of which is the serious one, since anything already encrypted under the old key becomes unreadable. That was a real defect in my own change, not a false positive.

I removed both documentation hunks instead of making the snippet idempotent, because bun run setup already does this correctly: scripts/setup/env-files.ts upserts individual keys into an existing file, seeds a missing one from .env.example, archives before destructive operations, and rejects placeholder values. A README snippet reimplementing that in bash would be a worse duplicate, so README.md and .github/CONTRIBUTING.md are now byte-identical to staging.

Also merged current staging in — the branch was 105 commits behind, with conflicts in README.md and docker-compose.prod.yml. The compose resolution keeps all of upstream's newer lines (AUTH_TRUSTED_PROXIES, REDIS_URL defaulting to redis://redis:6379, the :- defaults on COPILOT_API_KEY and SIM_AGENT_API_URL) with the guards re-applied on top.

The PR is now a single file, +5/-5, and the guard messages point at bun run setup rather than a manual openssl command. ENCRYPTION_KEY's message asks for exactly 64 hex characters to match what env-files.ts actually validates, since it is used as raw AES-256 material.

One caveat I want to be upfront about: since bun run setup seeds these secrets, the documented happy path for #5625 may already be covered upstream, and what's left here is the fail-fast for anyone invoking the compose file directly (which .github/CONTRIBUTING.md still documents at lines 169 and 199). If you'd rather close #5625 as already fixed, that's a fair call.

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

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:

  • ${VAR:?message} is standard POSIX/Compose interpolation that aborts at startup rather than passing an empty value through
  • ENCRYPTION_KEY's message correctly specifies "exactly 64 hex characters" — matching what env-files.ts actually validates, not just a generic length hint
  • The guard messages point to bun run setup rather than a manual command, which is the right single source of truth
  • The realtime service correctly guards BETTER_AUTH_SECRET and INTERNAL_API_SECRET but not ENCRYPTION_KEY (since realtime doesn't do AES operations — that tracks)

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 docker compose -f docker-compose.prod.yml config against a .env missing one of the three secrets, that confirms the abort path and this is ready to merge.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant