fix(docker): require auth secrets in prod and Ollama Compose files#5806
fix(docker): require auth secrets in prod and Ollama Compose files#5806waleedlatif1 wants to merge 3 commits into
Conversation
The Ollama Compose file fell back to broken defaults: docker compose does
not perform $(...) command substitution, so BETTER_AUTH_SECRET and
ENCRYPTION_KEY resolved to literal, shared, weak values (and the
ENCRYPTION_KEY literal was under the 32-char minimum the app enforces).
INTERNAL_API_SECRET — required (min 32) by both the app and realtime — was
not passed at all, so first-user signup failed the same way prod did.
Mirror the prod hardening: require BETTER_AUTH_SECRET, ENCRYPTION_KEY, and
INTERNAL_API_SECRET via mandatory ${VAR:?...} interpolation, add
INTERNAL_API_SECRET to both services, extend the regression test to cover
the Ollama Compose file, and note the .env prerequisite in the docs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Self-hosting docs ( A Vitest regression ( Reviewed by Cursor Bugbot for commit 41f1fef. Configure here. |
Greptile SummaryThis PR hardens secret handling for production and Ollama Docker deployments. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "docs(docker): add upgrade path for insta..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The generation block refuses to overwrite an existing .env, so an installation that predates the newly required secrets had no documented way to add them before the hardened Compose files fail-fast on startup. Add an idempotent snippet that appends only the missing secrets while preserving existing values, across the README, contributor guide, and all localized Docker guides.
|
@cursor review |
Greptile SummaryThis PR makes production and Ollama deployments require persistent secrets. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "docs(docker): add upgrade path for insta..." | Re-trigger Greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 41f1fef. Configure here.
2 participants
Summary
Builds on #5801 (included as the first commit) and closes the remaining gap in that fix: the same first-user-signup failure still exists in
docker-compose.ollama.yml, which many self-hosters use because it's the "try Sim with local models" path.Root cause (the part #5801 didn't cover)
docker-compose.ollama.ymlfell back to broken secret defaults:BETTER_AUTH_SECRET=${BETTER_AUTH_SECRET:-sim_auth_secret_$(openssl rand -hex 16)}andENCRYPTION_KEY=${ENCRYPTION_KEY:-$(openssl rand -hex 32)}— Docker Compose does not perform$(...)command substitution, so these resolve to the literal strings, i.e. a fixed, shared, weak secret across every install. Worse, theENCRYPTION_KEYliteral ($(openssl rand -hex 32), 22 chars) is under the 32-char minimum the app enforces (z.string().min(32)), so it fails env validation.INTERNAL_API_SECRET— required (z.string().min(32)) by both the app (apps/sim/lib/core/config/env.ts) and realtime (apps/realtime/src/env.ts) — was not passed at all, so the Ollama quick start couldn't sign up the first admin, exactly like the prod bug in [BUG] signup first admin account fails in docker compose #5625.Changes
docker-compose.ollama.yml: requireBETTER_AUTH_SECRET,ENCRYPTION_KEY, andINTERNAL_API_SECRETvia mandatory${VAR:?...}interpolation, and addINTERNAL_API_SECRETto bothsimstudioandrealtime.docker-compose.prod.ymlanddocker-compose.ollama.yml..envprerequisite in the Ollama section of the docs (en/de/es/fr/ja/zh) and CONTRIBUTING.Validation
vitest run lib/core/config/docker-compose-secrets.test.ts— 10 passed (5 prod + 5 ollama; the test parses the real Compose files).biome checkon the test file — passed.js-yaml).Credit
First commit is @brainx's original #5801. This PR supersedes it — merging this lands both fixes together.
Fixes #5625