fix: harden secret-file readers

This commit is contained in:
Peter Steinberger
2026-03-10 23:40:10 +00:00
parent 208fb1aa35
commit 201420a7ee
26 changed files with 433 additions and 188 deletions

View File

@@ -0,0 +1,70 @@
import { mkdir, symlink, writeFile } from "node:fs/promises";
import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js";
import {
DEFAULT_SECRET_FILE_MAX_BYTES,
readSecretFileSync,
tryReadSecretFileSync,
} from "./secret-file.js";
const tempDirs = createTrackedTempDirs();
const createTempDir = () => tempDirs.make("openclaw-secret-file-test-");
afterEach(async () => {
await tempDirs.cleanup();
});
describe("readSecretFileSync", () => {
it("reads and trims a regular secret file", async () => {
const dir = await createTempDir();
const file = path.join(dir, "secret.txt");
await writeFile(file, " top-secret \n", "utf8");
expect(readSecretFileSync(file, "Gateway password")).toBe("top-secret");
});
it("rejects files larger than the secret-file limit", async () => {
const dir = await createTempDir();
const file = path.join(dir, "secret.txt");
await writeFile(file, "x".repeat(DEFAULT_SECRET_FILE_MAX_BYTES + 1), "utf8");
expect(() => readSecretFileSync(file, "Gateway password")).toThrow(
`Gateway password file at ${file} exceeds ${DEFAULT_SECRET_FILE_MAX_BYTES} bytes.`,
);
});
it("rejects non-regular files", async () => {
const dir = await createTempDir();
const nestedDir = path.join(dir, "secret-dir");
await mkdir(nestedDir);
expect(() => readSecretFileSync(nestedDir, "Gateway password")).toThrow(
`Gateway password file at ${nestedDir} must be a regular file.`,
);
});
it("rejects symlinks when configured", async () => {
const dir = await createTempDir();
const target = path.join(dir, "target.txt");
const link = path.join(dir, "secret-link.txt");
await writeFile(target, "top-secret\n", "utf8");
await symlink(target, link);
expect(() => readSecretFileSync(link, "Gateway password", { rejectSymlink: true })).toThrow(
`Gateway password file at ${link} must not be a symlink.`,
);
});
it("returns undefined from the non-throwing helper for rejected files", async () => {
const dir = await createTempDir();
const target = path.join(dir, "target.txt");
const link = path.join(dir, "secret-link.txt");
await writeFile(target, "top-secret\n", "utf8");
await symlink(target, link);
expect(tryReadSecretFileSync(link, "Telegram bot token", { rejectSymlink: true })).toBe(
undefined,
);
});
});

133
src/infra/secret-file.ts Normal file
View File

@@ -0,0 +1,133 @@
import fs from "node:fs";
import { resolveUserPath } from "../utils.js";
import { openVerifiedFileSync } from "./safe-open-sync.js";
export const DEFAULT_SECRET_FILE_MAX_BYTES = 16 * 1024;
export type SecretFileReadOptions = {
maxBytes?: number;
rejectSymlink?: boolean;
};
export type SecretFileReadResult =
| {
ok: true;
secret: string;
resolvedPath: string;
}
| {
ok: false;
message: string;
resolvedPath?: string;
error?: unknown;
};
export function loadSecretFileSync(
filePath: string,
label: string,
options: SecretFileReadOptions = {},
): SecretFileReadResult {
const trimmedPath = filePath.trim();
const resolvedPath = resolveUserPath(trimmedPath);
if (!resolvedPath) {
return { ok: false, message: `${label} file path is empty.` };
}
const maxBytes = options.maxBytes ?? DEFAULT_SECRET_FILE_MAX_BYTES;
let previewStat: fs.Stats;
try {
previewStat = fs.lstatSync(resolvedPath);
} catch (error) {
return {
ok: false,
resolvedPath,
error,
message: `Failed to inspect ${label} file at ${resolvedPath}: ${String(error)}`,
};
}
if (options.rejectSymlink && previewStat.isSymbolicLink()) {
return {
ok: false,
resolvedPath,
message: `${label} file at ${resolvedPath} must not be a symlink.`,
};
}
if (!previewStat.isFile()) {
return {
ok: false,
resolvedPath,
message: `${label} file at ${resolvedPath} must be a regular file.`,
};
}
if (previewStat.size > maxBytes) {
return {
ok: false,
resolvedPath,
message: `${label} file at ${resolvedPath} exceeds ${maxBytes} bytes.`,
};
}
const opened = openVerifiedFileSync({
filePath: resolvedPath,
rejectPathSymlink: options.rejectSymlink,
maxBytes,
});
if (!opened.ok) {
const error =
opened.reason === "validation" ? new Error("security validation failed") : opened.error;
return {
ok: false,
resolvedPath,
error,
message: `Failed to read ${label} file at ${resolvedPath}: ${String(error)}`,
};
}
try {
const raw = fs.readFileSync(opened.fd, "utf8");
const secret = raw.trim();
if (!secret) {
return {
ok: false,
resolvedPath,
message: `${label} file at ${resolvedPath} is empty.`,
};
}
return { ok: true, secret, resolvedPath };
} catch (error) {
return {
ok: false,
resolvedPath,
error,
message: `Failed to read ${label} file at ${resolvedPath}: ${String(error)}`,
};
} finally {
fs.closeSync(opened.fd);
}
}
export function readSecretFileSync(
filePath: string,
label: string,
options: SecretFileReadOptions = {},
): string {
const result = loadSecretFileSync(filePath, label, options);
if (result.ok) {
return result.secret;
}
throw new Error(result.message, result.error ? { cause: result.error } : undefined);
}
export function tryReadSecretFileSync(
filePath: string | undefined,
label: string,
options: SecretFileReadOptions = {},
): string | undefined {
if (!filePath?.trim()) {
return undefined;
}
const result = loadSecretFileSync(filePath, label, options);
return result.ok ? result.secret : undefined;
}