Trim NODE_ENV before the key gate reads it, so a stray space cannot ship the public key

CHANGELOG.md

@@ -8,6 +8,15 @@ 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.
 ## 0.0.6
 
 ### Setting up needs one Intelligence credential, not two

server/src/config.ts

@@ -328,6 +328,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.
  *
@@ -350,7 +371,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",
       );
@@ -671,9 +692,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.",
     );

server/tests/config.test.ts

@@ -208,6 +208,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,