readSecretInput: never print the path, refuse relative paths and blank secrets

Soul F1: the unreadable-file error carried the NAME_FILE value, and Idunn
admits any string for a declared name, so a URL bound into
GC_ACCESS_DATABASE_URL_FILE by mistake was printed in full at startup. Errors
now name only the variable and an error code, and a NAME_FILE that is not an
absolute path is refused naming the variable alone.

Soul F2 and F6: a blank database URL file selected memory storage silently,
and a whitespace one selected postgres with a URL of " ". The reader now
refuses an empty or whitespace-only value from either form, so custody's own
"set but empty" check is redundant and is deleted; a short key still fails its
length check. The legacy service.env holds no blank secret-class value
(checked as name/blank booleans), so the legacy unit is unaffected.

Soul F3: tests pin that an empty NAME_FILE is refused rather than unset (S1),
and that each call site (token key, provider secret, app shared secret,
database URL, patron secret) refuses both forms, so no site can read the
plaintext name ahead of the reader (S6, S7, S8).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/config.ts

@@ -131,7 +131,12 @@ function readIdunnCandidateBind(
  * Setting both is refused, so one input has one owner. Under Idunn the recipe
  * declares only the `_FILE` names, so the plaintext form cannot be bound there.
  *
- * Errors name the variable and the path, never a value or a file's contents.
+ * `NAME_FILE` must be an absolute path, and a value that is empty or only
+ * whitespace is refused: a blank secret is a misconfiguration, never "unset".
+ *
+ * Errors name only the variable and an error code. They never carry the path,
+ * a value or a file's contents: Idunn admits any string for a declared name, so
+ * a secret bound into a `_FILE` slot by mistake would otherwise be printed.
  */
 export function readSecretInput(env: NodeJS.ProcessEnv, name: string): string | undefined {
   const fileVariable = `${name}_FILE`;
@@ -142,19 +147,31 @@ export function readSecretInput(env: NodeJS.ProcessEnv, name: string): string |
     throw new Error(`${name} and ${fileVariable} are both set; set only ${fileVariable}.`);
   }
 
+  let value: string;
+  let source: string;
   if (filePath === undefined) {
-    return plaintext;
+    if (plaintext === undefined) {
+      return undefined;
+    }
+    value = plaintext;
+    source = name;
+  } else {
+    if (!path.isAbsolute(filePath)) {
+      throw new Error(`${fileVariable} is not an absolute path.`);
+    }
+    try {
+      value = readFileSync(filePath, "utf8").replace(/\r?\n$/, "");
+    } catch (error) {
+      const code = (error as NodeJS.ErrnoException).code ?? "read error";
+      throw new Error(`${fileVariable} could not be read (${code}).`);
+    }
+    source = fileVariable;
   }
 
-  let contents: string;
-  try {
-    contents = readFileSync(filePath, "utf8");
-  } catch (error) {
-    const code = (error as NodeJS.ErrnoException).code ?? "read error";
-    throw new Error(`${fileVariable} names ${filePath}, which could not be read (${code}).`);
+  if (value.trim() === "") {
+    throw new Error(`${source} is empty.`);
   }
-
-  return contents.replace(/\r?\n$/, "");
+  return value;
 }
 
 function readProviderConfig(env: NodeJS.ProcessEnv, provider: Provider): ProviderClientConfig {

src/custody.ts

@@ -12,12 +12,8 @@ export interface TokenCustody {
 }
 
 function decodeConfiguredKey(base64Value: string): Buffer {
-  const normalized = base64Value.trim();
-  if (!normalized) {
-    throw new Error("GC_ACCESS_TOKEN_ENCRYPTION_KEY_BASE64_FILE is set but empty.");
-  }
-
-  const key = Buffer.from(normalized, "base64");
+  // readSecretInput refuses a blank key; a short one fails the length check.
+  const key = Buffer.from(base64Value.trim(), "base64");
   if (key.byteLength !== TOKEN_KEY_BYTES) {
     throw new Error(
       `GC_ACCESS_TOKEN_ENCRYPTION_KEY_BASE64_FILE must decode to exactly ${TOKEN_KEY_BYTES} bytes for AES-256-GCM.`

tests/config-secrets.test.ts

@@ -85,15 +85,27 @@ describe("readSecretInput", () => {
     expect(() => readSecretInput(env, "EXAMPLE_SECRET")).toThrow(/both set/);
   });
 
-  it("names the variable and the path of a missing file", () => {
-    const missing = join(tempDir(), "absent-credential");
-    const message = thrownMessage(() => readSecretInput({ EXAMPLE_SECRET_FILE: missing }, "EXAMPLE_SECRET"));
+  it("refuses an unreadable path naming only the variable and the code, not the path", () => {
+    // A secret bound into a _FILE slot by mistake must not be printed back.
+    const canaryPath = join(tempDir(), "canary-path-3b7e1d");
+    const message = thrownMessage(() => readSecretInput({ EXAMPLE_SECRET_FILE: canaryPath }, "EXAMPLE_SECRET"));
     expect(message).toContain("EXAMPLE_SECRET_FILE");
-    expect(message).toContain(missing);
     expect(message).toContain("ENOENT");
+    expect(message).not.toContain("canary");
   });
 
-  it("names the variable and the path of an unreadable file, and carries no byte of what sits beside it", () => {
+  it("refuses a _FILE value that is not an absolute path, without echoing it", () => {
+    for (const value of ["postgres://heimdall:canary-pw-6f02aa@db/heimdall", "relative/canary-rel-0c9d", ""]) {
+      const message = thrownMessage(() => readSecretInput({ EXAMPLE_SECRET_FILE: value }, "EXAMPLE_SECRET"));
+      expect(message).toBe("EXAMPLE_SECRET_FILE is not an absolute path.");
+    }
+  });
+
+  it("does not treat an empty _FILE as unset, even with no plaintext form", () => {
+    expect(() => readSecretInput({ EXAMPLE_SECRET_FILE: "" }, "EXAMPLE_SECRET")).toThrow(/EXAMPLE_SECRET_FILE/);
+  });
+
+  it("refuses an unreadable file, and carries no byte of what sits in or beside it", () => {
     const dir = tempDir();
     const canary = "canary-sibling-5d2f88";
     writeFileSync(join(dir, "sibling"), canary);
@@ -102,9 +114,27 @@ describe("readSecretInput", () => {
     writeFileSync(join(notAFile, "inside"), canary);
 
     const message = thrownMessage(() => readSecretInput({ EXAMPLE_SECRET_FILE: notAFile }, "EXAMPLE_SECRET"));
-    expect(message).toContain("EXAMPLE_SECRET_FILE");
-    expect(message).toContain(notAFile);
-    expect(message).not.toContain("canary");
+    expect(message).toBe("EXAMPLE_SECRET_FILE could not be read (EISDIR).");
+  });
+
+  it("refuses a blank secret file rather than treating it as unset", () => {
+    for (const contents of ["", "\n", " \n", "\t\r\n", "   "]) {
+      const env = { EXAMPLE_SECRET_FILE: secretFile(contents) };
+      expect(thrownMessage(() => readSecretInput(env, "EXAMPLE_SECRET"))).toBe("EXAMPLE_SECRET_FILE is empty.");
+    }
+  });
+
+  it("refuses a blank plaintext secret too", () => {
+    for (const value of ["", "  ", "\n"]) {
+      expect(thrownMessage(() => readSecretInput({ EXAMPLE_SECRET: value }, "EXAMPLE_SECRET"))).toBe(
+        "EXAMPLE_SECRET is empty."
+      );
+    }
+  });
+
+  it("keeps a value whose only whitespace is inside or around real content", () => {
+    const env = { EXAMPLE_SECRET_FILE: secretFile(" padded value \n") };
+    expect(readSecretInput(env, "EXAMPLE_SECRET")).toBe(" padded value ");
   });
 });
 
@@ -157,18 +187,32 @@ describe("loadConfig secret inputs", () => {
   it("refuses to start when the database URL file is absent", () => {
     const missing = join(tempDir(), "database-url");
     const message = thrownMessage(() => loadConfig({ GC_ACCESS_DATABASE_URL_FILE: missing }, []));
-    expect(message).toContain("GC_ACCESS_DATABASE_URL_FILE");
-    expect(message).toContain(missing);
+    expect(message).toBe("GC_ACCESS_DATABASE_URL_FILE could not be read (ENOENT).");
   });
 
-  it("refuses a secret bound both ways at startup", () => {
-    const env = {
-      GC_ACCESS_TOKEN_ENCRYPTION_KEY_BASE64: "plain",
-      GC_ACCESS_TOKEN_ENCRYPTION_KEY_BASE64_FILE: secretFile("file"),
-    };
-    expect(() => loadConfig(env, [])).toThrow(/GC_ACCESS_TOKEN_ENCRYPTION_KEY_BASE64_FILE/);
+  it("refuses to start on a blank database URL file instead of falling back to memory storage", () => {
+    for (const contents of ["", "\n", " \n"]) {
+      const env = { GC_ACCESS_DATABASE_URL_FILE: secretFile(contents) };
+      expect(() => loadConfig(env, [])).toThrow("GC_ACCESS_DATABASE_URL_FILE is empty.");
+    }
   });
 
+  // One per call site: a site that reads the plaintext name itself before the
+  // reader would let the plaintext win silently instead of refusing.
+  const bothSetSites: Array<[string, string]> = [
+    ["token encryption key", "GC_ACCESS_TOKEN_ENCRYPTION_KEY_BASE64"],
+    ["provider client secret", "GC_ACCESS_PROVIDER_DISCORD_CLIENT_SECRET"],
+    ["app shared secret", "GC_ACCESS_APP_GHOSTLIGHT_SHARED_SECRET"],
+    ["database URL", "GC_ACCESS_DATABASE_URL"],
+    ["patron support secret", "GC_ACCESS_BIFROST_PATRON_SUPPORT_SECRET"],
+  ];
+  for (const [site, name] of bothSetSites) {
+    it(`refuses the ${site} bound both ways at startup`, () => {
+      const env = { [name]: "plain", [`${name}_FILE`]: secretFile("file") };
+      expect(() => loadConfig(env, [])).toThrow(`${name} and ${name}_FILE are both set`);
+    });
+  }
+
   it("publishes provider secret variable names that actually configure the secret", () => {
     for (const provider of providers) {
       const secretVariable = providerCatalog[provider].clientSecretEnv;