test(config): cover secret bundle inode races
This commit is contained in:
@@ -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<string, string> {
|
||||
}
|
||||
|
||||
export function loadSecretBundle(file: string): ReadonlyMap<string, string> {
|
||||
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<string, string> {
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user