diff --git a/backend/src/config/secret-bundle.ts b/backend/src/config/secret-bundle.ts index da3ffbae..db2ace5b 100644 --- a/backend/src/config/secret-bundle.ts +++ b/backend/src/config/secret-bundle.ts @@ -34,6 +34,23 @@ export interface SecretBundleConfig { THT_SECRETS_FILE?: string; } +/** Injectable filesystem boundary used by the race-condition tests. */ +export interface SecretBundleFsOps { + lstat(path: string): Stats; + open(path: string, flags: number): number; + fstat(fd: number): Stats; + read(fd: number): string; + close(fd: number): void; +} + +const realFs: SecretBundleFsOps = { + lstat: lstatSync, + open: openSync, + fstat: fstatSync, + read: (fd) => readFileSync(fd, "utf8"), + close: closeSync, +}; + function unavailable(): Error { return new Error("secret bundle is unavailable"); } function secureStat(info: Stats, docker: boolean): boolean { @@ -43,26 +60,26 @@ function secureStat(info: Stats, docker: boolean): boolean { return info.uid === (process.getuid?.() ?? info.uid) && (mode === 0o400 || mode === 0o600); } -function readSecure(file: string): string { +function readSecure(file: string, fs: SecretBundleFsOps): string { let fd: number | undefined; try { if (!file || file.trim() !== file || file.includes("\0")) throw unavailable(); const docker = file.startsWith("/run/secrets/") && !file.slice("/run/secrets/".length).includes("/"); if (file.startsWith("/run/secrets/") && !docker) throw unavailable(); if (docker) { - const parent = lstatSync("/run/secrets"); + const parent = fs.lstat("/run/secrets"); if (!parent.isDirectory() || parent.uid !== 0 || (parent.mode & 0o022) !== 0) throw unavailable(); } - const before = lstatSync(file); + const before = fs.lstat(file); if (!secureStat(before, docker)) throw unavailable(); - fd = openSync(file, constants.O_RDONLY | constants.O_NOFOLLOW); - const opened = fstatSync(fd); + fd = fs.open(file, constants.O_RDONLY | constants.O_NOFOLLOW); + const opened = fs.fstat(fd); if (!secureStat(opened, docker) || before.dev !== opened.dev || before.ino !== opened.ino) throw unavailable(); - return readFileSync(fd, "utf8"); + return fs.read(fd); } catch { throw unavailable(); } finally { - if (fd !== undefined) try { closeSync(fd); } catch { /* sanitized by design */ } + if (fd !== undefined) try { fs.close(fd); } catch { /* sanitized by design */ } } } @@ -86,7 +103,12 @@ function parseBundle(text: string): ReadonlyMap { } export function loadSecretBundle(file: string): ReadonlyMap { - try { return parseBundle(readSecure(file)); } catch { throw unavailable(); } + return loadSecretBundleWithFs(file, realFs); +} + +/** Same loader with an injectable filesystem boundary; useful for TOCTOU tests. */ +export function loadSecretBundleWithFs(file: string, fs: SecretBundleFsOps): ReadonlyMap { + try { return parseBundle(readSecure(file, fs)); } catch { throw unavailable(); } } /** Resolve a value from the bundle, with the pre-bundle *_SECRET_FILE fallback. */ @@ -104,7 +126,7 @@ export function secretValue(config: SecretBundleConfig, key: string): string | u })() : undefined; if (!legacyPath) return undefined; - const value = readSecure(legacyPath); + const value = readSecure(legacyPath, realFs); if (!value || /\s/.test(value)) throw unavailable(); return value; } diff --git a/backend/test/secret-bundle.test.ts b/backend/test/secret-bundle.test.ts index da7aea4f..a3f2b550 100644 --- a/backend/test/secret-bundle.test.ts +++ b/backend/test/secret-bundle.test.ts @@ -2,7 +2,7 @@ import { afterEach, expect, test } from "vitest"; import { chmodSync, mkdtempSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { loadSecretBundle, secretValue } from "../src/config/secret-bundle.js"; +import { loadSecretBundle, loadSecretBundleWithFs, secretValue } from "../src/config/secret-bundle.js"; const dirs: string[] = []; afterEach(() => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); @@ -48,6 +48,22 @@ test("checks inode identity before parsing", () => { expect(loadSecretBundle(file).get("THT_MODEL_API_KEY")).toBe("replaced"); }); +test("rejects inode replacement between lstat and open without reading", () => { + let reads = 0; + const stat = (ino: number) => ({ + dev: 7, ino, uid: process.getuid?.() ?? 0, mode: 0o100600, nlink: 1, size: 24, + isFile: () => true, isDirectory: () => false, isSymbolicLink: () => false, + }); + expect(() => loadSecretBundleWithFs("/safe/bundle", { + lstat: () => stat(1) as any, + open: () => 9, + fstat: () => stat(2) as any, + read: () => { reads += 1; return "THT_MODEL_API_KEY=secret\n"; }, + close: () => undefined, + })).toThrow("secret bundle is unavailable"); + expect(reads).toBe(0); +}); + test("secretValue prefers bundle and supports the legacy file fallback", () => { const file = bundle("THT_MODEL_API_KEY=from-bundle\n"); const legacy = bundle("from-legacy");