From 2f7b55151d680a5fbfcde59380c3865747b142d0 Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Fri, 4 Sep 2026 07:08:58 +0900 Subject: [PATCH] Trim NODE_ENV before the key gate reads it, so a stray space cannot ship the public key A deployment that never changed KEY_ENCRYPTION_KEY encrypts its credential vault with the key printed in .env.example. It is a valid key - right length, right encoding - so nothing else about it fails a check, and the refusal under NODE_ENV=production is the only thing standing between "copied the example file" and that outcome. That refusal compared environment.NODE_ENV raw. The other production refusal in the same file, on private-host browsing, reads it through `optional` and therefore trims, and its comment says exactly why: Through `optional`, so the comparison trims. Read raw, `NODE_ENV="production "` out of an env file would slip past a gate that the switch beside it, which does trim, would still trip. Both sides of the comparison come out of one env file, and a trailing space there is invisible: Docker's env_file preserves it verbatim, and so does every hosting dashboard with a text box. So `NODE_ENV=production ` tripped one gate, slipped past the other, and started the deployment on a key published in this repository with a console warning nobody reads at boot. The question moves into `isProduction`, used by both, so the next gate that needs it cannot pick the wrong way to ask. `environment.NODE_ENV` was the only raw environment read left in the file; everything else already goes through required, optional or commaSeparated. The example key had no test of its own at all, which is part of why this drifted, so the production refusal and the local warning are now pinned alongside the whitespace case. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 10 ++++++ server/src/config.ts | 29 +++++++++++++++--- server/tests/config.test.ts | 61 +++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11fb9c67f..3656a4ca3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A stray space in NODE_ENV no longer lets the public example key through + +A deployment that never changed `KEY_ENCRYPTION_KEY` encrypts its credential vault with the key +printed in `.env.example`, so the server refuses to start with it under `NODE_ENV=production`. That +refusal compared the variable exactly as written, while the other production refusal beside it — +private-host browsing — trimmed first. Both read the same env file, and a trailing space there is +invisible: Docker's `env_file` preserves it and so does every hosting dashboard with a text box. So +`NODE_ENV=production ` tripped one refusal, slipped past the other, and started the deployment on the +public key with only a warning at boot. Both gates now ask the same question the same way. + ### Duplicating a coworker keeps the endpoint it was copied from Duplicate used to point every copy at this deployment's own managed Bot, whatever the coworker being diff --git a/server/src/config.ts b/server/src/config.ts index 2779cf891..0921b05a1 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -322,6 +322,27 @@ function optional(environment: Environment, name: string): string | undefined { return environment[name]?.trim() || undefined; } +/** + * Whether this deployment says it is in production, which is what the two hard refusals turn on. + * + * ONE PLACE, BECAUSE THE TWO GATES DID NOT AGREE. Both refuse a local-only setting on a deployed + * server — the example encryption key, and private-host browsing — and both compare `NODE_ENV` + * against `"production"`. The private-hosts gate read it through `optional`, so the comparison + * trimmed; the key gate compared `environment.NODE_ENV` raw. + * + * Both sides of that comparison come out of the same file. `NODE_ENV=production ` with a trailing + * space — invisible in an env file, and preserved verbatim by Docker's `env_file` and by every + * hosting dashboard with a text box — therefore tripped one refusal and slipped past the other. The + * one it slipped past is the one that decides whether the credential vault may be encrypted with a + * key printed in this repository. + * + * A helper rather than a second `optional` call, so the next gate that needs this question cannot + * pick the wrong way to ask it. + */ +function isProduction(environment: Environment): boolean { + return optional(environment, "NODE_ENV") === "production"; +} + /** * The key in `.env.example`, which every clone of this repository starts with. * @@ -344,7 +365,7 @@ function keyEncryptionKey(environment: Environment): string { * in any deployment. */ if (value === PLACEHOLDER_KEY) { - if (environment.NODE_ENV === "production") { + if (isProduction(environment)) { throw new Error( "KEY_ENCRYPTION_KEY is still the example key from .env.example, which is public. Generate one with: openssl rand -base64 32", ); @@ -660,9 +681,9 @@ function privateHostsAllowed(environment: Environment): boolean { return false; } - // Through `optional`, so the comparison trims. Read raw, `NODE_ENV="production "` out of an env - // file would slip past a gate that the switch beside it, which does trim, would still trip. - if (optional(environment, "NODE_ENV") === "production") { + // Through `isProduction`, so the comparison trims. Read raw, `NODE_ENV="production "` out of an + // env file would slip past a gate that the switch beside it, which does trim, would still trip. + if (isProduction(environment)) { throw new Error( "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true is for local development only: it lets a Bot reach this deployment's own network. Remove it from this deployment's environment.", ); diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 50b54ce5b..865676600 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -179,6 +179,67 @@ describe("deployment configuration", () => { ).toThrow("KEY_ENCRYPTION_KEY must be a base64-encoded 32-byte key"); }); + /* + * The key in `.env.example`, refused on a deployed server. + * + * It is a valid key — right length, right encoding — so nothing else about it fails a check. A + * deployment that never changed it encrypts its credential vault with a value printed in a public + * repository and looks exactly like one that did, which is why this refusal is the only thing + * standing between "copied the example file" and that outcome. + */ + test("refuses the example encryption key on a production deployment", () => { + expect(() => + loadConfig({ + ...baseEnvironment, + NODE_ENV: "production", + }), + ).toThrow("KEY_ENCRYPTION_KEY is still the example key"); + }); + + /* + * The same trim the private-hosts gate below already gets, on the gate that matters more. + * + * Both sides of the comparison come out of one env file, and a trailing space there is invisible: + * Docker's `env_file` preserves it verbatim and so does every hosting dashboard with a text box. + * Compared raw, `NODE_ENV="production "` downgraded this refusal to a warning nobody reads at boot + * and started the deployment on the public key. + */ + test("refuses the example key when NODE_ENV carries whitespace", () => { + expect(() => + loadConfig({ + ...baseEnvironment, + NODE_ENV: "production ", + }), + ).toThrow("KEY_ENCRYPTION_KEY is still the example key"); + }); + + // The local workflow is the reason the example key is usable at all, so off production it still + // does exactly what it did: warns, and starts. + test.each(["development", undefined])( + "warns about the example key and still starts under NODE_ENV=%p", + (nodeEnv) => { + const consoleWarn = spyOn(console, "warn").mockImplementation(() => {}); + + try { + expect(() => + loadConfig({ + ...baseEnvironment, + ...(nodeEnv ? { NODE_ENV: nodeEnv } : {}), + }), + ).not.toThrow(); + + const warning = consoleWarn.mock.calls + .map(([first]) => String(first)) + .find((line) => line.includes("KEY_ENCRYPTION_KEY")); + + expect(warning).toBeDefined(); + expect(warning).toContain("which is public"); + } finally { + consoleWarn.mockRestore(); + } + }, + ); + test("enables Google authentication when its complete deployment contract is present", () => { const config = loadConfig({ ...baseEnvironment,