test: cover degraded paths and shell commands in the write guard (#36)
Review of PR #39: the spies only watched authenticated requests on complete fixtures, so a write on an error path passed the whole suite (reviewer mutations R1, R2, R3 and R11: quarantining a corrupt agents-map.json, recreating a missing status.json, logging a 401/403 or a refused payload to disk). A new case walks rejections (401/403/404), invalid input (400/413), a corrupt agents-map.json, missing Defenseurs files and a corrupt snapshot, requires the degraded content (no_data, neverSeen), and requires no write outside HOSTS_DIR. child_process is now spied the same way as node:fs and the set of shell commands is pinned to the two read-only ones metrics.js runs, with GET /health as positive control. R1, R2, R3 and R11 went from 0 to 1 red each; R4 (touch through execFileSync) is now caught by this file. A write deferred by a timer longer than 20 ms (R8b) stays out of sight, as the header says. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
6a2f7fec04
commit
7523b397f0
1 changed files with 141 additions and 15 deletions
|
|
@ -2,6 +2,7 @@ const http = require("node:http");
|
||||||
const fs = require("node:fs");
|
const fs = require("node:fs");
|
||||||
const os = require("node:os");
|
const os = require("node:os");
|
||||||
const path = require("node:path");
|
const path = require("node:path");
|
||||||
|
const child_process = require("node:child_process");
|
||||||
const { fileURLToPath } = require("node:url");
|
const { fileURLToPath } = require("node:url");
|
||||||
|
|
||||||
// The API's only write surface is HOSTS_DIR (issue #36).
|
// The API's only write surface is HOSTS_DIR (issue #36).
|
||||||
|
|
@ -11,13 +12,20 @@ const { fileURLToPath } = require("node:url");
|
||||||
// or /data/defenseurs would not fail in the other test files: their fixtures
|
// or /data/defenseurs would not fail in the other test files: their fixtures
|
||||||
// live in a writable temp dir, and CI runs as root, so a chmod-555 fixture
|
// live in a writable temp dir, and CI runs as root, so a chmod-555 fixture
|
||||||
// would prove nothing either. It would fail in production, as a 500 (EROFS)
|
// would prove nothing either. It would fail in production, as a 500 (EROFS)
|
||||||
// on the route that tried it. This file moves that failure to CI.
|
// on the route that tried it. This file moves that failure to CI, for the
|
||||||
|
// paths it exercises: boot, the nominal reads and ingest, and the degraded
|
||||||
|
// ones (rejections, invalid input, a corrupt file, missing files).
|
||||||
//
|
//
|
||||||
// Every node:fs function that can write is spied BEFORE a fresh require,
|
// Every node:fs function that can write is spied BEFORE a fresh require,
|
||||||
// because the runtime modules destructure their fs functions at load time
|
// because the runtime modules destructure their fs functions at load time
|
||||||
// (same technique as the timingSafeEqual spy in auth.test.js). A call is
|
// (same technique as the timingSafeEqual spy in auth.test.js). A call is
|
||||||
// recorded when its stack goes through a runtime file of the repo, then passed
|
// recorded when its stack goes through a runtime file of the repo, then passed
|
||||||
// through to the real function.
|
// through to the real function. A shell command can write too, out of the fs
|
||||||
|
// spies' sight, so child_process is spied the same way and the set of commands
|
||||||
|
// is pinned to the two read-only ones metrics.js runs.
|
||||||
|
//
|
||||||
|
// Limits: a path no case takes stays out of sight, and so does a write
|
||||||
|
// deferred past settle() (a timer longer than 20 ms).
|
||||||
//
|
//
|
||||||
// Boot is exercised by calling the exported sweepStaleTempFiles(), which is
|
// Boot is exercised by calling the exported sweepStaleTempFiles(), which is
|
||||||
// what the `require.main === module` block runs before listen. That block
|
// what the `require.main === module` block runs before listen. That block
|
||||||
|
|
@ -29,7 +37,7 @@ const INGEST_TOKEN = "test-ingest-token";
|
||||||
const ORPHAN = ".thinkpad.4242.1700000000000.tmp";
|
const ORPHAN = ".thinkpad.4242.1700000000000.tmp";
|
||||||
|
|
||||||
// Runtime file = under the repo root, outside node_modules and __tests__.
|
// Runtime file = under the repo root, outside node_modules and __tests__.
|
||||||
// Used both to attribute a write to the API and to purge the module cache.
|
// Used both to attribute a call to the API and to purge the module cache.
|
||||||
function isRuntimeFile(file) {
|
function isRuntimeFile(file) {
|
||||||
const rel = path.relative(REPO_ROOT, file);
|
const rel = path.relative(REPO_ROOT, file);
|
||||||
if (rel === "" || rel === ".." || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) return false;
|
if (rel === "" || rel === ".." || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) return false;
|
||||||
|
|
@ -47,6 +55,14 @@ function fromRuntimeFile(stack) {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function captureStack() {
|
||||||
|
const limit = Error.stackTraceLimit;
|
||||||
|
Error.stackTraceLimit = 50;
|
||||||
|
const { stack } = new Error();
|
||||||
|
Error.stackTraceLimit = limit;
|
||||||
|
return stack;
|
||||||
|
}
|
||||||
|
|
||||||
// Write APIs of node:fs, by the argument(s) naming what gets written. Each is
|
// Write APIs of node:fs, by the argument(s) naming what gets written. Each is
|
||||||
// spied in its callback, Sync and fs.promises forms, when that form exists.
|
// spied in its callback, Sync and fs.promises forms, when that form exists.
|
||||||
const O_WRITE =
|
const O_WRITE =
|
||||||
|
|
@ -86,7 +102,17 @@ const WRITE_APIS = [
|
||||||
["open", (args) => (opensForWriting(args[1]) ? [args[0]] : null)],
|
["open", (args) => (opensForWriting(args[1]) ? [args[0]] : null)],
|
||||||
];
|
];
|
||||||
|
|
||||||
|
// The two commands metrics.js runs, to read /proc/stat and the root disk.
|
||||||
|
const ALLOWED_COMMANDS = ["df -k /", "head -1 /proc/stat"];
|
||||||
|
const CHILD_PROCESS_APIS = ["exec", "execSync", "execFile", "execFileSync", "spawn", "spawnSync", "fork"];
|
||||||
|
|
||||||
|
// `exec("cmd")` and `execFile("file", ["arg"])` both come out as one string.
|
||||||
|
function commandOf(args) {
|
||||||
|
return (Array.isArray(args[1]) ? [args[0], ...args[1]] : [args[0]]).map(String).join(" ");
|
||||||
|
}
|
||||||
|
|
||||||
let log = [];
|
let log = [];
|
||||||
|
let commandLog = [];
|
||||||
|
|
||||||
function drain() {
|
function drain() {
|
||||||
const out = log;
|
const out = log;
|
||||||
|
|
@ -94,6 +120,10 @@ function drain() {
|
||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function foreignCommands() {
|
||||||
|
return commandLog.filter((command) => !ALLOWED_COMMANDS.includes(command));
|
||||||
|
}
|
||||||
|
|
||||||
function installWriteSpies() {
|
function installWriteSpies() {
|
||||||
for (const [name, targetsOf] of WRITE_APIS) {
|
for (const [name, targetsOf] of WRITE_APIS) {
|
||||||
for (const [owner, key] of [
|
for (const [owner, key] of [
|
||||||
|
|
@ -105,11 +135,7 @@ function installWriteSpies() {
|
||||||
if (typeof owner[key] !== "function") continue;
|
if (typeof owner[key] !== "function") continue;
|
||||||
const original = owner[key];
|
const original = owner[key];
|
||||||
vi.spyOn(owner, key).mockImplementation(function (...args) {
|
vi.spyOn(owner, key).mockImplementation(function (...args) {
|
||||||
const limit = Error.stackTraceLimit;
|
if (fromRuntimeFile(captureStack())) {
|
||||||
Error.stackTraceLimit = 50;
|
|
||||||
const { stack } = new Error();
|
|
||||||
Error.stackTraceLimit = limit;
|
|
||||||
if (fromRuntimeFile(stack)) {
|
|
||||||
const targets = targetsOf(args);
|
const targets = targetsOf(args);
|
||||||
if (targets) log.push({ fn: key, targets });
|
if (targets) log.push({ fn: key, targets });
|
||||||
}
|
}
|
||||||
|
|
@ -119,8 +145,19 @@ function installWriteSpies() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function installCommandSpies() {
|
||||||
|
for (const key of CHILD_PROCESS_APIS) {
|
||||||
|
if (typeof child_process[key] !== "function") continue;
|
||||||
|
const original = child_process[key];
|
||||||
|
vi.spyOn(child_process, key).mockImplementation(function (...args) {
|
||||||
|
if (fromRuntimeFile(captureStack())) commandLog.push(commandOf(args));
|
||||||
|
return original.apply(this, args);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Every runtime module, not only index.js: a module left in the cache would
|
// Every runtime module, not only index.js: a module left in the cache would
|
||||||
// keep the fs functions it destructured under a previous test's spies, which
|
// keep the functions it destructured under a previous test's spies, which
|
||||||
// vi.restoreAllMocks() has turned back into silent pass-throughs.
|
// vi.restoreAllMocks() has turned back into silent pass-throughs.
|
||||||
function purgeRuntimeModules() {
|
function purgeRuntimeModules() {
|
||||||
for (const file of Object.keys(require.cache)) {
|
for (const file of Object.keys(require.cache)) {
|
||||||
|
|
@ -177,6 +214,26 @@ let server;
|
||||||
let baseUrl;
|
let baseUrl;
|
||||||
let bootWrites;
|
let bootWrites;
|
||||||
|
|
||||||
|
// A transport failure (a connection cut before a 413 is read) comes back as
|
||||||
|
// "transport-error", so it fails an assertion instead of passing for a code.
|
||||||
|
async function send(route, { method = "GET", auth = `Bearer ${TOKEN}`, body } = {}) {
|
||||||
|
const headers = auth ? { Authorization: auth } : {};
|
||||||
|
if (body !== undefined) headers["Content-Type"] = "application/json";
|
||||||
|
try {
|
||||||
|
const res = await fetch(`${baseUrl}${route}`, { method, headers, body });
|
||||||
|
const text = await res.text();
|
||||||
|
let json = null;
|
||||||
|
try {
|
||||||
|
json = JSON.parse(text);
|
||||||
|
} catch {
|
||||||
|
// Not JSON: the status alone is asserted.
|
||||||
|
}
|
||||||
|
return { status: res.status, body: json };
|
||||||
|
} catch {
|
||||||
|
return { status: "transport-error", body: null };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Created before the spies: nothing here is attributed to the API.
|
// Created before the spies: nothing here is attributed to the API.
|
||||||
function writeFixtures() {
|
function writeFixtures() {
|
||||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "vps-health-writes-test-"));
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "vps-health-writes-test-"));
|
||||||
|
|
@ -226,7 +283,9 @@ function writeFixtures() {
|
||||||
beforeEach(async () => {
|
beforeEach(async () => {
|
||||||
writeFixtures();
|
writeFixtures();
|
||||||
log = [];
|
log = [];
|
||||||
|
commandLog = [];
|
||||||
installWriteSpies();
|
installWriteSpies();
|
||||||
|
installCommandSpies();
|
||||||
purgeRuntimeModules();
|
purgeRuntimeModules();
|
||||||
const { handler, sweepStaleTempFiles } = require("../index.js");
|
const { handler, sweepStaleTempFiles } = require("../index.js");
|
||||||
sweepStaleTempFiles();
|
sweepStaleTempFiles();
|
||||||
|
|
@ -247,6 +306,7 @@ describe("fs writes — the API writes only under HOSTS_DIR (#36)", () => {
|
||||||
expect(bootWrites.map(({ fn }) => fn)).toContain("unlinkSync");
|
expect(bootWrites.map(({ fn }) => fn)).toContain("unlinkSync");
|
||||||
expect(fs.existsSync(path.join(process.env.HOSTS_DIR, ORPHAN))).toBe(false);
|
expect(fs.existsSync(path.join(process.env.HOSTS_DIR, ORPHAN))).toBe(false);
|
||||||
expect(violations(bootWrites)).toEqual([]);
|
expect(violations(bootWrites)).toEqual([]);
|
||||||
|
expect(foreignCommands()).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
// Each route with the content that proves its fixture was really read: a
|
// Each route with the content that proves its fixture was really read: a
|
||||||
|
|
@ -277,28 +337,94 @@ describe("fs writes — the API writes only under HOSTS_DIR (#36)", () => {
|
||||||
// This test's own spies are live: they saw the boot sweep.
|
// This test's own spies are live: they saw the boot sweep.
|
||||||
expect(bootWrites).not.toHaveLength(0);
|
expect(bootWrites).not.toHaveLength(0);
|
||||||
|
|
||||||
const res = await fetch(`${baseUrl}${route}`, { headers: { Authorization: `Bearer ${TOKEN}` } });
|
const { status, body } = await send(route);
|
||||||
expect(res.status).toBe(200);
|
expect(status).toBe(200);
|
||||||
const body = await res.json();
|
|
||||||
expect(body.status).not.toBe("no_data");
|
expect(body.status).not.toBe("no_data");
|
||||||
expectContent(body);
|
expectContent(body);
|
||||||
|
|
||||||
await settle();
|
await settle();
|
||||||
expect(drain()).toEqual([]);
|
expect(drain()).toEqual([]);
|
||||||
|
expect(foreignCommands()).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("POST /hosts/<id> writes, and only under HOSTS_DIR", async () => {
|
test("POST /hosts/<id> writes, and only under HOSTS_DIR", async () => {
|
||||||
const res = await fetch(`${baseUrl}/hosts/thinkpad`, {
|
const { status } = await send("/hosts/thinkpad", {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { Authorization: `Bearer ${INGEST_TOKEN}`, "Content-Type": "application/json" },
|
auth: `Bearer ${INGEST_TOKEN}`,
|
||||||
body: JSON.stringify(validSnapshot()),
|
body: JSON.stringify(validSnapshot()),
|
||||||
});
|
});
|
||||||
expect(res.status).toBe(204);
|
expect(status).toBe(204);
|
||||||
|
|
||||||
await settle();
|
await settle();
|
||||||
const writes = drain();
|
const writes = drain();
|
||||||
expect(writes.map(({ fn }) => fn)).toEqual(expect.arrayContaining(["writeFileSync", "renameSync"]));
|
expect(writes.map(({ fn }) => fn)).toEqual(expect.arrayContaining(["writeFileSync", "renameSync"]));
|
||||||
expect(violations(writes)).toEqual([]);
|
expect(violations(writes)).toEqual([]);
|
||||||
|
expect(foreignCommands()).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The branches the nominal cases never take, and where the most plausible
|
||||||
|
// future writes would land: quarantining a corrupt file, recreating a
|
||||||
|
// missing one, logging a rejection or a refused payload to disk.
|
||||||
|
test("rejections, invalid input, corrupt and missing files write nothing outside HOSTS_DIR", async () => {
|
||||||
|
expect(bootWrites).not.toHaveLength(0);
|
||||||
|
const ingest = `Bearer ${INGEST_TOKEN}`;
|
||||||
|
const rejected = [
|
||||||
|
[401, "/defenseurs", { auth: null }],
|
||||||
|
[401, "/defenseurs", { auth: "Bearer wrong" }],
|
||||||
|
[401, "/hosts", { auth: ingest }],
|
||||||
|
[404, "/nope", {}],
|
||||||
|
[400, "/defenseurs/findings", {}],
|
||||||
|
[400, "/defenseurs/findings?project=proj&severity=BOGUS", {}],
|
||||||
|
[404, "/defenseurs/findings?project=unknown", {}],
|
||||||
|
[400, "/reports/scans?date=../../etc", {}],
|
||||||
|
[401, "/hosts/thinkpad", { method: "POST", auth: "Bearer wrong", body: "{}" }],
|
||||||
|
[403, "/hosts/laptop", { method: "POST", auth: ingest, body: JSON.stringify(validSnapshot()) }],
|
||||||
|
[400, "/hosts/thinkpad", { method: "POST", auth: ingest, body: "{not json" }],
|
||||||
|
[400, "/hosts/thinkpad", { method: "POST", auth: ingest, body: JSON.stringify({ hostname: "x" }) }],
|
||||||
|
[413, "/hosts/thinkpad", { method: "POST", auth: ingest, body: "x".repeat(5000) }],
|
||||||
|
];
|
||||||
|
for (const [expected, route, options] of rejected) {
|
||||||
|
const { status } = await send(route, options);
|
||||||
|
expect([options.method || "GET", route, status]).toEqual([options.method || "GET", route, expected]);
|
||||||
|
}
|
||||||
|
|
||||||
|
// A corrupt agents-map.json: where a quarantine rename would go.
|
||||||
|
fs.writeFileSync(process.env.DEFENSEURS_AGENTS_MAP_PATH, "{corrupt");
|
||||||
|
expect((await send("/defenseurs/findings?project=proj")).status).toBe(500);
|
||||||
|
|
||||||
|
// Missing Defenseurs files and a corrupt snapshot: where a "recreate it"
|
||||||
|
// write would go. Each route must degrade, with the content that says so.
|
||||||
|
fs.writeFileSync(process.env.DEFENSEURS_AGENTS_MAP_PATH, JSON.stringify({ proj: "defenseur-proj" }));
|
||||||
|
fs.rmSync(process.env.DEFENSEURS_STATUS_PATH);
|
||||||
|
fs.rmSync(process.env.DEFENSEURS_FINDINGS_STATE_PATH);
|
||||||
|
fs.rmSync(process.env.REPORTS_DIR, { recursive: true });
|
||||||
|
fs.writeFileSync(path.join(process.env.HOSTS_DIR, "thinkpad.json"), "{corrupt");
|
||||||
|
const degraded = [
|
||||||
|
["/defenseurs", (body) => expect(body.status).toBe("no_data")],
|
||||||
|
["/defenseurs/state", (body) => expect(body.status).toBe("no_data")],
|
||||||
|
["/defenseurs/findings?project=proj", (body) => expect(body).toEqual({ findings: [], status: "no_data" })],
|
||||||
|
["/reports/scans?date=2026-09-30", (body) => expect(body.count).toBe(0)],
|
||||||
|
["/hosts", (body) => expect(body.hosts.find(({ id }) => id === "thinkpad").neverSeen).toBe(true)],
|
||||||
|
];
|
||||||
|
for (const [route, expectContent] of degraded) {
|
||||||
|
const { status, body } = await send(route);
|
||||||
|
expect([route, status]).toEqual([route, 200]);
|
||||||
|
expectContent(body);
|
||||||
|
}
|
||||||
|
|
||||||
|
await settle();
|
||||||
|
expect(violations(drain())).toEqual([]);
|
||||||
|
expect(foreignCommands()).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("shell commands — only the two read-only metrics ones (#36)", () => {
|
||||||
|
// Positive control for the child_process spies: /health must show exactly
|
||||||
|
// the two commands. The other cases only check that nothing else ran.
|
||||||
|
test("GET /health runs exactly `head -1 /proc/stat` and `df -k /`", async () => {
|
||||||
|
const { status } = await send("/health");
|
||||||
|
expect(status).toBe(200);
|
||||||
|
expect([...new Set(commandLog)].sort()).toEqual([...ALLOWED_COMMANDS].sort());
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue