From c98505589757d60b6e34516131d4946939e4dab6 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 10 Jul 2026 14:13:57 -0700 Subject: [PATCH] fix(selfhost): exclude Docker Compose's reserved _FILE vars from the secret-dereference scan (#4403) loadFileSecrets() treats every *_FILE env var as a gittensory Docker-secret pointer, including Compose's own reserved COMPOSE_FILE (a colon-delimited list of compose file paths, never a single readable file) and COMPOSE_ENV_FILE. Both threw on every readFileSync attempt, logging a guaranteed-false level:error on every container boot -- undermining the convention that error-level logs are real operator-alertable signals. Also extracts loadFileSecrets into its own module: server.ts boots the whole app on import and is Codecov-ignored, so it had no way to carry a real runtime regression test for this fix. --- src/selfhost/load-file-secrets.ts | 37 ++++++++++ src/server.ts | 25 +------ test/unit/selfhost-load-file-secrets.test.ts | 78 ++++++++++++++++++++ 3 files changed, 117 insertions(+), 23 deletions(-) create mode 100644 src/selfhost/load-file-secrets.ts create mode 100644 test/unit/selfhost-load-file-secrets.test.ts diff --git a/src/selfhost/load-file-secrets.ts b/src/selfhost/load-file-secrets.ts new file mode 100644 index 0000000000..93093acd23 --- /dev/null +++ b/src/selfhost/load-file-secrets.ts @@ -0,0 +1,37 @@ +// Resolve `_FILE` env vars (Docker secrets / multi-line keys) into `` at self-host startup. +// Extracted from server.ts (#4403) so this has a real test harness -- server.ts itself boots the whole +// app on import and is Codecov-ignored, so it has no runtime test coverage of its own. +import { readFileSync } from "node:fs"; + +// Docker Compose's OWN reserved `_FILE`-suffixed environment variables -- never gittensory's secret-file +// convention, so they must never be dereferenced below. `COMPOSE_FILE` is a colon-delimited list of +// compose file paths (never a single readable file itself, so readFileSync always throws), and +// `COMPOSE_ENV_FILE` (less commonly set, but equally reserved by Compose) points at an operator's custom +// .env file, not a secret. Excluding both by name is the fix (#4403) -- a real operator secret is never +// named exactly one of these. +const COMPOSE_RESERVED_FILE_VARS = new Set(["COMPOSE_FILE", "COMPOSE_ENV_FILE"]); + +/** `env` and `readFile` are injectable purely for testability -- every real caller uses the defaults + * (`process.env`, `node:fs`'s `readFileSync`), so this is byte-identical to a hardcoded version at + * runtime while letting tests pass a plain object and a mock reader instead of mutating global state. */ +export function loadFileSecrets( + env: Record = process.env, + readFile: (path: string) => string = (path) => readFileSync(path, "utf8"), +): void { + for (const key of Object.keys(env)) { + if (!key.endsWith("_FILE") || !env[key] || COMPOSE_RESERVED_FILE_VARS.has(key)) continue; + const target = key.slice(0, -"_FILE".length); + if (env[target]) continue; // an explicit value wins + try { + env[target] = readFile(env[key] as string).trim(); + } catch { + console.error( + JSON.stringify({ + level: "error", + event: "selfhost_secret_file_unreadable", + var: key, + }), + ); + } + } +} diff --git a/src/server.ts b/src/server.ts index 859f780d65..42c1bff218 100644 --- a/src/server.ts +++ b/src/server.ts @@ -6,7 +6,7 @@ // Serves the Hono app via @hono/node-server, drives the queue with the same processJob, ticks the same // scheduled handler on a timer, exposes /health /ready /metrics, and shuts down gracefully. The Cloudflare // Worker (src/index.ts) is untouched — this is a parallel entry the self-host esbuild build bundles. -import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, writeFileSync } from "node:fs"; import { delimiter, join } from "node:path"; import { randomUUID } from "node:crypto"; import { DatabaseSync } from "node:sqlite"; @@ -42,6 +42,7 @@ import { import { createOrbRelayRegistrationState, isOrbBrokerMode, registerOrbRelayTargetWithRetry } from "./orb/broker-client"; import { exportOrbBatch } from "./selfhost/orb-collector"; import { createD1Adapter, nodeSqliteDriver } from "./selfhost/d1-adapter"; +import { loadFileSecrets } from "./selfhost/load-file-secrets"; import { backupAcknowledgedGaugeValue, buildHealthBody, @@ -106,28 +107,6 @@ import { probeReesSecretAtStartup } from "./review/enrichment-wire"; import { sampleRecentDeadLetters } from "./selfhost/dlq-recent"; import type { JobMessage } from "./types"; -/** Resolve `_FILE` env vars (Docker secrets / multi-line keys) into `` at startup. */ -function loadFileSecrets(): void { - for (const key of Object.keys(process.env)) { - if (!key.endsWith("_FILE") || !process.env[key]) continue; - const target = key.slice(0, -"_FILE".length); - if (process.env[target]) continue; // an explicit value wins - try { - process.env[target] = readFileSync( - process.env[key] as string, - "utf8", - ).trim(); - } catch { - console.error( - JSON.stringify({ - level: "error", - event: "selfhost_secret_file_unreadable", - var: key, - }), - ); - } - } -} interface Backend { db: D1Database; diff --git a/test/unit/selfhost-load-file-secrets.test.ts b/test/unit/selfhost-load-file-secrets.test.ts new file mode 100644 index 0000000000..42fb79d43a --- /dev/null +++ b/test/unit/selfhost-load-file-secrets.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it, vi } from "vitest"; +import { loadFileSecrets } from "../../src/selfhost/load-file-secrets"; + +describe("loadFileSecrets (#4403)", () => { + it("REGRESSION: never dereferences COMPOSE_FILE, and logs no false error for it", () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const readFile = vi.fn(() => "should never be called"); + const env: Record = { + COMPOSE_FILE: "docker-compose.yml:docker-compose.override.yml:docker-compose.local-gpu.yml", + }; + loadFileSecrets(env, readFile); + expect(env.COMPOSE).toBeUndefined(); + expect(readFile).not.toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + + it("also excludes COMPOSE_ENV_FILE, Compose's other reserved _FILE var", () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const readFile = vi.fn(() => "should never be called"); + const env: Record = { COMPOSE_ENV_FILE: ".env.prod" }; + loadFileSecrets(env, readFile); + expect(env.COMPOSE_ENV).toBeUndefined(); + expect(readFile).not.toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + + it("dereferences a real gittensory secret _FILE var into its target name", () => { + const readFile = vi.fn(() => "s3cr3t-value\n"); + const env: Record = { SENTRY_DSN_FILE: "/run/secrets/sentry_dsn" }; + loadFileSecrets(env, readFile); + expect(readFile).toHaveBeenCalledWith("/run/secrets/sentry_dsn"); + expect(env.SENTRY_DSN).toBe("s3cr3t-value"); // trimmed + }); + + it("does not overwrite an already-set explicit value", () => { + const readFile = vi.fn(() => "from-file"); + const env: Record = { SENTRY_DSN_FILE: "/run/secrets/sentry_dsn", SENTRY_DSN: "already-set" }; + loadFileSecrets(env, readFile); + expect(readFile).not.toHaveBeenCalled(); + expect(env.SENTRY_DSN).toBe("already-set"); + }); + + it("ignores a key that doesn't end in _FILE, and a _FILE key with no value", () => { + const readFile = vi.fn(); + const env: Record = { NOT_A_SECRET: "x", EMPTY_FILE: "" }; + loadFileSecrets(env, readFile); + expect(readFile).not.toHaveBeenCalled(); + }); + + it("logs a structured error and leaves the target unset when the file read fails, for a genuine secret var", () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const readFile = vi.fn(() => { + throw new Error("ENOENT"); + }); + const env: Record = { SENTRY_DSN_FILE: "/run/secrets/missing" }; + loadFileSecrets(env, readFile); + expect(env.SENTRY_DSN).toBeUndefined(); + expect(errorSpy).toHaveBeenCalledWith( + JSON.stringify({ level: "error", event: "selfhost_secret_file_unreadable", var: "SENTRY_DSN_FILE" }), + ); + errorSpy.mockRestore(); + }); + + it("defaults to process.env and the real node:fs reader when called with no arguments", () => { + const original = process.env.NOT_A_REAL_SECRET_FILE; + process.env.NOT_A_REAL_SECRET_FILE = "/definitely/does/not/exist"; + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + loadFileSecrets(); + expect(errorSpy).toHaveBeenCalledWith( + JSON.stringify({ level: "error", event: "selfhost_secret_file_unreadable", var: "NOT_A_REAL_SECRET_FILE" }), + ); + errorSpy.mockRestore(); + if (original === undefined) delete process.env.NOT_A_REAL_SECRET_FILE; + else process.env.NOT_A_REAL_SECRET_FILE = original; + }); +});