From 2f06bcdca7573b2b52c03b55e148ed2dfab0b7e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:31:28 +0000 Subject: [PATCH 01/14] chore(tests): rerun a root vitest run as another user from the config Root ignores file permissions, so the tests that deny access with chmod fail as root and pass in CI. vitest.config.ts now calls a guard while vitest loads it. As root, the guard reruns the same command as nobody, or as the checkout's owner when root does not own it, after preparing that user's scratch folders and a Node binary it can run. When no rerun is possible it prints one warning and the run continues as root. Co-Authored-By: Claude Opus 5.5 --- knip.json | 2 +- scripts/__tests__/root-test-guard.test.ts | 431 +++++++++++++++++++ scripts/root-test-guard.ts | 494 ++++++++++++++++++++++ vitest.config.ts | 7 + 4 files changed, 933 insertions(+), 1 deletion(-) create mode 100644 scripts/__tests__/root-test-guard.test.ts create mode 100644 scripts/root-test-guard.ts diff --git a/knip.json b/knip.json index 2fe2405de..ce7768f6f 100644 --- a/knip.json +++ b/knip.json @@ -5,5 +5,5 @@ "src/__tests__/docker/fixtures/sync-engine-oracle.ts" ], "project": ["src/**/*.ts", "cli/src/**/*.ts", "scripts/**/*.ts"], - "ignoreBinaries": ["optipng", "perl"] + "ignoreBinaries": ["optipng", "perl", "setpriv"] } diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts new file mode 100644 index 000000000..5bce07340 --- /dev/null +++ b/scripts/__tests__/root-test-guard.test.ts @@ -0,0 +1,431 @@ +import { + chmodSync, + existsSync, + lstatSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { describe, expect, it, onTestFinished, vi } from "vitest" +import { + addCoverageFlags, + isCoverageRun, + planRootRun, + prepareRerun, + readNvmrcMajor, +} from "../root-test-guard.js" + +const CHECKOUT = "/repo" +const ROOT_NODE = "/root/.nvm/versions/node/v24.21.0/bin/node" +const VITEST_ENTRY = "/repo/node_modules/.bin/vitest" +const NOBODY = { kind: "nobody", name: "nobody", uid: 65534, gid: 65534 } as const +const OWNER = { kind: "owner", name: "the checkout's owner", uid: 1000, gid: 1000 } as const +const ALL_PROBES_PASS = { + setprivWorks: true, + canReadConfig: true, + canRunNode: true, + canReadExtraCaCerts: true, +} + +/** planRootRun's inputs for a plain `vitest run` as nobody, with every probe + * passing; each test overrides what it varies. */ +const rootRun = (overrides: Partial[0]> = {}) => { + return planRootRun({ + checkoutRoot: CHECKOUT, + target: NOBODY, + probes: ALL_PROBES_PASS, + execPath: ROOT_NODE, + nodeVersion: "v24.21.0", + nvmrcText: "24\n", + execArguments: [], + vitestEntry: VITEST_ENTRY, + vitestArguments: ["run"], + coverageRun: false, + extraCaCertsSet: false, + pathEnvironment: "/usr/bin:/bin", + ...overrides, + }) +} + +const SCRATCH_FOLDERS = [ + "/repo/node_modules/.vite-temp", + "/repo/node_modules/.vitest", + "/repo/node_modules/.test-home", + "/repo/coverage", + "/repo/src/vault-mcp/mcp-core/__tests__/__snapshots__", +] + +const createCheckout = (): string => { + const checkout = mkdtempSync(join(tmpdir(), "vault-cortex-root-test-guard-")) + + onTestFinished(() => rmSync(checkout, { recursive: true, force: true })) + return checkout +} + +describe("readNvmrcMajor", () => { + it.each([ + { label: "a bare major", nvmrcText: "24\n", major: 24 }, + { label: "a full version with v", nvmrcText: "v24.1.0\n", major: 24 }, + { label: "a leading space", nvmrcText: " 24\n", major: 24 }, + { label: "a trailing comment", nvmrcText: "24 # CI runs this\n", major: 24 }, + { label: "a CRLF line end", nvmrcText: "24\r\n", major: 24 }, + { label: "an alias", nvmrcText: "lts/*\n", major: null }, + { label: "an empty file", nvmrcText: "", major: null }, + ])("reads $label", ({ nvmrcText, major }) => { + expect(readNvmrcMajor(nvmrcText)).toBe(major) + }) +}) + +describe("addCoverageFlags", () => { + it("appends the flags when the arguments hold no lone --", () => { + expect(addCoverageFlags(["run", "--coverage"])).toEqual([ + "run", + "--coverage", + "--coverage.clean=false", + "--coverage.cleanOnRerun=false", + ]) + }) + + it("puts the flags before a lone --, after which vitest ignores flags", () => { + expect(addCoverageFlags(["run", "--coverage", "--", "--other"])).toEqual([ + "run", + "--coverage", + "--coverage.clean=false", + "--coverage.cleanOnRerun=false", + "--", + "--other", + ]) + }) +}) + +describe("isCoverageRun", () => { + it.each([ + { label: "--coverage", vitestArguments: ["run", "--coverage"], coverageRun: true }, + { + label: "--coverage.enabled", + vitestArguments: ["run", "--coverage.enabled"], + coverageRun: true, + }, + { label: "--coverage=true", vitestArguments: ["run", "--coverage=true"], coverageRun: true }, + { label: "no coverage flag", vitestArguments: ["run"], coverageRun: false }, + { + label: "--coverage.enabled=false", + vitestArguments: ["run", "--coverage.enabled=false"], + coverageRun: false, + }, + ])("reads $label the way vitest does", ({ vitestArguments, coverageRun }) => { + expect(isCoverageRun(vitestArguments)).toBe(coverageRun) + }) +}) + +describe("planRootRun", () => { + it("reruns as nobody with a copy of a Node binary nobody cannot run", () => { + const plan = rootRun({ probes: { ...ALL_PROBES_PASS, canRunNode: false } }) + + expect(plan).toEqual({ + kind: "rerun", + target: NOBODY, + scratchFolders: SCRATCH_FOLDERS, + coverageFolderToEmpty: null, + node: { + kind: "copy", + sourcePath: ROOT_NODE, + nodePath: "/repo/node_modules/.nobody-node/node-v24.21.0/node", + }, + setprivArguments: [ + "--reuid=65534", + "--regid=65534", + "--clear-groups", + "/usr/bin/env", + "HOME=/repo/node_modules/.test-home", + "PATH=/repo/node_modules/.nobody-node/node-v24.21.0:/usr/bin:/bin", + "/repo/node_modules/.nobody-node/node-v24.21.0/node", + VITEST_ENTRY, + "run", + ], + announcement: + "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v24.21.0.\n" + + ' setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env HOME=/repo/node_modules/.test-home PATH=/repo/node_modules/.nobody-node/node-v24.21.0:"$PATH" /repo/node_modules/.nobody-node/node-v24.21.0/node /repo/node_modules/.bin/vitest run', + }) + }) + + it("reruns as the checkout's owner on the Node binary as it is, when the owner can run it", () => { + const plan = rootRun({ target: OWNER, execPath: "/usr/local/bin/node" }) + + expect(plan).toEqual({ + kind: "rerun", + target: OWNER, + scratchFolders: SCRATCH_FOLDERS, + coverageFolderToEmpty: null, + node: { kind: "as-is", nodePath: "/usr/local/bin/node" }, + setprivArguments: [ + "--reuid=1000", + "--regid=1000", + "--clear-groups", + "/usr/bin/env", + "HOME=/repo/node_modules/.test-home", + "PATH=/usr/local/bin:/usr/bin:/bin", + "/usr/local/bin/node", + VITEST_ENTRY, + "run", + ], + announcement: + "Root ignores the file permissions some tests check, so the tests rerun as the checkout's owner (uid 1000) on Node v24.21.0.\n" + + ' setpriv --reuid=1000 --regid=1000 --clear-groups /usr/bin/env HOME=/repo/node_modules/.test-home PATH=/usr/local/bin:"$PATH" /usr/local/bin/node /repo/node_modules/.bin/vitest run', + }) + }) + + it("warns instead of copying a Node binary into an owner's checkout", () => { + const plan = rootRun({ target: OWNER, probes: { ...ALL_PROBES_PASS, canRunNode: false } }) + + expect(plan).toEqual({ + kind: "warn", + message: + "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: the checkout's owner cannot run this Node binary.", + }) + }) + + it("warns when setpriv cannot switch to the target", () => { + const plan = rootRun({ probes: { ...ALL_PROBES_PASS, setprivWorks: false } }) + + expect(plan).toEqual({ + kind: "warn", + message: + "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: setpriv cannot switch to nobody.", + }) + }) + + it("warns when the target cannot read the checkout", () => { + const plan = rootRun({ probes: { ...ALL_PROBES_PASS, canReadConfig: false } }) + + expect(plan).toEqual({ + kind: "warn", + message: + "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: nobody cannot read the checkout.", + }) + }) + + it("unsets NODE_EXTRA_CA_CERTS when the target cannot read its file", () => { + const plan = rootRun({ + extraCaCertsSet: true, + probes: { ...ALL_PROBES_PASS, canReadExtraCaCerts: false }, + }) + + expect(plan.kind === "rerun" ? plan.setprivArguments.slice(3, 7) : plan).toEqual([ + "/usr/bin/env", + "-u", + "NODE_EXTRA_CA_CERTS", + "HOME=/repo/node_modules/.test-home", + ]) + }) + + it("keeps NODE_EXTRA_CA_CERTS when the target can read its file", () => { + const plan = rootRun({ extraCaCertsSet: true }) + + expect(plan.kind === "rerun" ? plan.setprivArguments.slice(3, 5) : plan).toEqual([ + "/usr/bin/env", + "HOME=/repo/node_modules/.test-home", + ]) + }) + + it("empties coverage/ and tells vitest not to clean it on a coverage run", () => { + const plan = rootRun({ vitestArguments: ["run", "--coverage"], coverageRun: true }) + + expect( + plan.kind === "rerun" + ? { + coverageFolderToEmpty: plan.coverageFolderToEmpty, + rerunArguments: plan.setprivArguments.slice(-4), + } + : plan, + ).toEqual({ + coverageFolderToEmpty: "/repo/coverage", + rerunArguments: [ + "run", + "--coverage", + "--coverage.clean=false", + "--coverage.cleanOnRerun=false", + ], + }) + }) + + it("passes Node's own flags through ahead of vitest's entry", () => { + const plan = rootRun({ execArguments: ["--inspect-brk"] }) + + expect(plan.kind === "rerun" ? plan.setprivArguments.slice(-4) : plan).toEqual([ + ROOT_NODE, + "--inspect-brk", + VITEST_ENTRY, + "run", + ]) + }) + + it("notes a Node major that differs from the one .nvmrc names", () => { + const plan = rootRun({ nodeVersion: "v22.22.0" }) + + expect(plan.kind === "rerun" ? plan.announcement.split("\n")[0] : plan).toBe( + "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v22.22.0. .nvmrc names Node 24, which CI runs, so a result can differ from CI's.", + ) + }) + + it("adds no version note when .nvmrc names an alias", () => { + const plan = rootRun({ nodeVersion: "v22.22.0", nvmrcText: "lts/*\n" }) + + expect(plan.kind === "rerun" ? plan.announcement.split("\n")[0] : plan).toBe( + "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v22.22.0.", + ) + }) + + it("quotes a path with a space in the printed command", () => { + const plan = rootRun({ + checkoutRoot: "/my repo", + vitestEntry: "/my repo/node_modules/.bin/vitest", + }) + + expect(plan.kind === "rerun" ? plan.announcement.split("\n")[1] : plan).toBe( + ` setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env 'HOME=/my repo/node_modules/.test-home' PATH=/root/.nvm/versions/node/v24.21.0/bin:"$PATH" /root/.nvm/versions/node/v24.21.0/bin/node '/my repo/node_modules/.bin/vitest' run`, + ) + }) +}) + +describe("prepareRerun", () => { + const nobodyPlanFor = ( + checkout: string, + overrides: Partial[0]> = {}, + ) => { + const plan = rootRun({ checkoutRoot: checkout, ...overrides }) + + if (plan.kind !== "rerun") throw new Error(`expected a rerun plan, got ${plan.message}`) + return plan + } + + it("creates every scratch folder and hands it, and everything in it, to the target", () => { + const checkout = createCheckout() + const cacheFile = join(checkout, "node_modules", ".vitest", "results.json") + mkdirSync(join(checkout, "node_modules", ".vitest"), { recursive: true }) + writeFileSync(cacheFile, "{}\n") + // chmod, because the umask cuts the mode writeFileSync is given. + chmodSync(cacheFile, 0o666) + const changeOwner = vi.fn() + + prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner }) + + const handedOver = changeOwner.mock.calls.map(([{ path }]) => path) + expect({ + foldersExist: [".vite-temp", ".vitest", ".test-home"].map((folder) => + existsSync(join(checkout, "node_modules", folder)), + ), + handedOver: handedOver.toSorted(), + cacheFileMode: lstatSync(cacheFile).mode & 0o777, + }).toEqual({ + foldersExist: [true, true, true], + handedOver: [ + join(checkout, "coverage"), + join(checkout, "node_modules", ".test-home"), + join(checkout, "node_modules", ".vite-temp"), + join(checkout, "node_modules", ".vitest"), + cacheFile, + join(checkout, "src/vault-mcp/mcp-core/__tests__/__snapshots__"), + ].toSorted(), + cacheFileMode: 0o644, + }) + expect(changeOwner).toHaveBeenCalledWith({ path: cacheFile, uid: 65534, gid: 65534 }) + }) + + it("hands over a symlink without changing the mode of what it points to", () => { + const checkout = createCheckout() + const outsideFile = join(checkout, "outside.txt") + writeFileSync(outsideFile, "keep\n") + chmodSync(outsideFile, 0o666) + mkdirSync(join(checkout, "node_modules", ".vitest"), { recursive: true }) + const link = join(checkout, "node_modules", ".vitest", "link") + symlinkSync(outsideFile, link) + const changeOwner = vi.fn() + + prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner }) + + expect(lstatSync(outsideFile).mode & 0o777).toBe(0o666) + expect(changeOwner).toHaveBeenCalledWith({ path: link, uid: 65534, gid: 65534 }) + }) + + it("empties coverage/ on a coverage run but keeps the folder", () => { + const checkout = createCheckout() + mkdirSync(join(checkout, "coverage", "html"), { recursive: true }) + writeFileSync(join(checkout, "coverage", "coverage-final.json"), "{}\n") + + prepareRerun({ + plan: nobodyPlanFor(checkout, { vitestArguments: ["run", "--coverage"], coverageRun: true }), + changeOwner: vi.fn(), + }) + + expect(readdirSync(join(checkout, "coverage"))).toEqual([]) + }) + + it("leaves coverage/ alone on a run without coverage", () => { + const checkout = createCheckout() + mkdirSync(join(checkout, "coverage"), { recursive: true }) + writeFileSync(join(checkout, "coverage", "coverage-final.json"), "{}\n") + + prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner: vi.fn() }) + + expect(readdirSync(join(checkout, "coverage"))).toEqual(["coverage-final.json"]) + }) + + it("copies the Node binary for nobody, mode 755, when the plan calls for a copy", () => { + const checkout = createCheckout() + const sourceNode = join(checkout, "source-node") + writeFileSync(sourceNode, "#!/bin/sh\n# node v24.21.0\n", { mode: 0o700 }) + const plan = nobodyPlanFor(checkout, { + execPath: sourceNode, + probes: { ...ALL_PROBES_PASS, canRunNode: false }, + }) + + prepareRerun({ plan, changeOwner: vi.fn() }) + + const nodeCopy = join(checkout, "node_modules", ".nobody-node", "node-v24.21.0", "node") + expect({ + content: readFileSync(nodeCopy, "utf8"), + mode: lstatSync(nodeCopy).mode & 0o777, + folderEntries: readdirSync(join(checkout, "node_modules", ".nobody-node", "node-v24.21.0")), + }).toEqual({ content: "#!/bin/sh\n# node v24.21.0\n", mode: 0o755, folderEntries: ["node"] }) + }) + + it("reuses an existing copy instead of copying again", () => { + const checkout = createCheckout() + const sourceNode = join(checkout, "source-node") + writeFileSync(sourceNode, "new\n", { mode: 0o755 }) + const nodeCopy = join(checkout, "node_modules", ".nobody-node", "node-v24.21.0", "node") + mkdirSync(join(checkout, "node_modules", ".nobody-node", "node-v24.21.0"), { recursive: true }) + writeFileSync(nodeCopy, "existing\n", { mode: 0o755 }) + const plan = nobodyPlanFor(checkout, { + execPath: sourceNode, + probes: { ...ALL_PROBES_PASS, canRunNode: false }, + }) + + prepareRerun({ plan, changeOwner: vi.fn() }) + + expect(readFileSync(nodeCopy, "utf8")).toBe("existing\n") + }) + + it("never hands the Node copy's folder to the target", () => { + const checkout = createCheckout() + const sourceNode = join(checkout, "source-node") + writeFileSync(sourceNode, "node\n", { mode: 0o755 }) + const changeOwner = vi.fn() + const plan = nobodyPlanFor(checkout, { + execPath: sourceNode, + probes: { ...ALL_PROBES_PASS, canRunNode: false }, + }) + + prepareRerun({ plan, changeOwner }) + + const handedOver = changeOwner.mock.calls.map(([{ path }]) => String(path)) + expect(handedOver.filter((path) => path.includes(".nobody-node"))).toEqual([]) + }) +}) diff --git a/scripts/root-test-guard.ts b/scripts/root-test-guard.ts new file mode 100644 index 000000000..d846291af --- /dev/null +++ b/scripts/root-test-guard.ts @@ -0,0 +1,494 @@ +/** + * Reruns a root vitest run as a user the permission tests can rely on. + * Root ignores file permissions, so tests that chmod a file or folder to deny + * access, then expect the access to fail, fail as root and pass in CI. + * - vitest.config.ts calls `stopRootTestRun` while vitest loads it, before any + * test file, the coverage provider or the watcher starts. + * - The rerun runs as nobody when root owns the checkout (a cloud or agent + * container's clone), and as the checkout's owner otherwise (a contributor's + * checkout mounted into a root container). + * - When no rerun is possible, it prints one warning and the run continues as + * root, so a root run is never worse than it was without this guard. + */ + +import { spawnSync } from "node:child_process" +import { + chmodSync, + copyFileSync, + existsSync, + lchownSync, + lstatSync, + mkdirSync, + readdirSync, + readFileSync, + renameSync, + rmSync, + statSync, +} from "node:fs" +import { dirname, join } from "node:path" +import { parseCLI } from "vitest/node" + +/** Folder names under node_modules the rerun writes in: vite's config + * bundle, vitest's cache, and the rerun's HOME (npm's cache and logs). */ +const NODE_MODULES_SCRATCH_FOLDERS = [".vite-temp", ".vitest", ".test-home"] + +/** The one snapshot folder in the repo; `snapshot:update` rewrites it. */ +const SNAPSHOT_FOLDER = "src/vault-mcp/mcp-core/__tests__/__snapshots__" + +/** A plain shell word that needs no quoting when the command is printed. */ +const SHELL_SAFE_WORD = /^[\w@%+=:,./-]+$/ + +type RerunTarget = { + /** nobody for a root-owned checkout; the checkout's owner otherwise. */ + kind: "nobody" | "owner" + name: string + uid: number + gid: number +} + +/** Why a rerun can't happen, named in the warning. */ +type WarningCause = + | { kind: "no-setpriv" } + | { kind: "no-nobody-user" } + | { kind: "setpriv-fails"; targetName: string } + | { kind: "config-unreadable"; targetName: string } + | { kind: "node-unrunnable"; targetName: string } + | { kind: "prepare-failed"; reason: string } + | { kind: "spawn-failed"; reason: string } + +/** What the target can do, each probed by running a command as the target. */ +type TargetProbes = { + setprivWorks: boolean + canReadConfig: boolean + canRunNode: boolean + /** Only probed when NODE_EXTRA_CA_CERTS is set. */ + canReadExtraCaCerts: boolean +} + +type NodeChoice = + { kind: "as-is"; nodePath: string } | { kind: "copy"; sourcePath: string; nodePath: string } + +type RerunPlan = { + kind: "rerun" + target: RerunTarget + scratchFolders: readonly string[] + coverageFolderToEmpty: string | null + node: NodeChoice + setprivArguments: readonly string[] + announcement: string +} + +type GuardPlan = { kind: "warn"; message: string } | RerunPlan + +const describeWarningCause = (cause: WarningCause): string => { + switch (cause.kind) { + case "no-setpriv": + return "setpriv is not installed" + case "no-nobody-user": + return "this system has no nobody user" + case "setpriv-fails": + return `setpriv cannot switch to ${cause.targetName}` + case "config-unreadable": + return `${cause.targetName} cannot read the checkout` + case "node-unrunnable": + return `${cause.targetName} cannot run this Node binary` + case "prepare-failed": + return `preparing the rerun failed (${cause.reason})` + case "spawn-failed": + return `starting the rerun failed (${cause.reason})` + } +} + +const formatWarning = (cause: WarningCause): string => { + return `vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: ${describeWarningCause(cause)}.` +} + +/** The major version .nvmrc names, or null for an alias such as lts/*. */ +export const readNvmrcMajor = (nvmrcText: string): number | null => { + const firstWord = nvmrcText.trimStart().split(/\s/)[0] ?? "" + const majorText = firstWord.replace(/^v/, "").split(".")[0] ?? "" + + if (!/^\d+$/.test(majorText)) return null + return Number(majorText) +} + +/** Quotes a word for a POSIX shell only when it needs it. */ +const quoteForShell = (word: string): string => { + if (SHELL_SAFE_WORD.test(word)) return word + return `'${word.replaceAll("'", `'\\''`)}'` +} + +/** + * Appends the flags a non-root coverage run needs: it cannot delete + * coverage/ from a root-owned checkout, so the guard empties the folder as + * root and vitest must not clean it. vitest ignores flags after a lone `--`, + * so they go before one. + */ +export const addCoverageFlags = (vitestArguments: readonly string[]): string[] => { + const coverageFlags = ["--coverage.clean=false", "--coverage.cleanOnRerun=false"] + const doubleDashIndex = vitestArguments.indexOf("--") + + if (doubleDashIndex === -1) return [...vitestArguments, ...coverageFlags] + return [ + ...vitestArguments.slice(0, doubleDashIndex), + ...coverageFlags, + ...vitestArguments.slice(doubleDashIndex), + ] +} + +/** Whether the vitest command line turns coverage on, read the way vitest + * reads it. parseCLI wants "vitest" first and rewrites the array it gets. */ +export const isCoverageRun = (vitestArguments: readonly string[]): boolean => { + const { options } = parseCLI(["vitest", ...vitestArguments]) + return options.coverage?.enabled === true +} + +const chooseNode = ({ + canRunNode, + target, + execPath, + nodeCopyPath, +}: { + canRunNode: boolean + target: RerunTarget + execPath: string + nodeCopyPath: string +}): NodeChoice | null => { + if (canRunNode) return { kind: "as-is", nodePath: execPath } + // An owner's checkout never gets a root-owned binary copied into it. + if (target.kind === "owner") return null + return { kind: "copy", sourcePath: execPath, nodePath: nodeCopyPath } +} + +/** Plans the guard's response for a root run whose target is known. */ +export const planRootRun = ({ + checkoutRoot, + target, + probes, + execPath, + nodeVersion, + nvmrcText, + execArguments, + vitestEntry, + vitestArguments, + coverageRun, + extraCaCertsSet, + pathEnvironment, +}: { + checkoutRoot: string + target: RerunTarget + probes: TargetProbes + execPath: string + nodeVersion: string + nvmrcText: string | null + execArguments: readonly string[] + vitestEntry: string + vitestArguments: readonly string[] + coverageRun: boolean + extraCaCertsSet: boolean + pathEnvironment: string +}): GuardPlan => { + if (!probes.setprivWorks) { + return { + kind: "warn", + message: formatWarning({ kind: "setpriv-fails", targetName: target.name }), + } + } + if (!probes.canReadConfig) { + return { + kind: "warn", + message: formatWarning({ kind: "config-unreadable", targetName: target.name }), + } + } + + const nodeCopyPath = join( + checkoutRoot, + "node_modules", + ".nobody-node", + `node-${nodeVersion}`, + "node", + ) + const node = chooseNode({ canRunNode: probes.canRunNode, target, execPath, nodeCopyPath }) + + if (!node) { + return { + kind: "warn", + message: formatWarning({ kind: "node-unrunnable", targetName: target.name }), + } + } + + const nodeModules = join(checkoutRoot, "node_modules") + const home = join(nodeModules, ".test-home") + const coverageFolder = join(checkoutRoot, "coverage") + const scratchFolders = [ + ...NODE_MODULES_SCRATCH_FOLDERS.map((folderName) => join(nodeModules, folderName)), + coverageFolder, + join(checkoutRoot, SNAPSHOT_FOLDER), + ] + const unsetsExtraCaCerts = extraCaCertsSet && !probes.canReadExtraCaCerts + const rerunArguments = coverageRun ? addCoverageFlags(vitestArguments) : [...vitestArguments] + const environmentArguments = [ + ...(unsetsExtraCaCerts ? ["-u", "NODE_EXTRA_CA_CERTS"] : []), + `HOME=${home}`, + ] + const commandAfterPath = [node.nodePath, ...execArguments, vitestEntry, ...rerunArguments] + const switchArguments = [`--reuid=${target.uid}`, `--regid=${target.gid}`, "--clear-groups"] + const nodeFolder = dirname(node.nodePath) + const setprivArguments = [ + ...switchArguments, + "/usr/bin/env", + ...environmentArguments, + `PATH=${nodeFolder}:${pathEnvironment}`, + ...commandAfterPath, + ] + // The printed command keeps $PATH literal, so it stays short and still + // runs as written in a shell. + const printedCommand = [ + "setpriv", + ...switchArguments, + "/usr/bin/env", + ...environmentArguments.map(quoteForShell), + `PATH=${quoteForShell(nodeFolder)}:"$PATH"`, + ...commandAfterPath.map(quoteForShell), + ].join(" ") + const nvmrcMajor = nvmrcText === null ? null : readNvmrcMajor(nvmrcText) + const nodeMajor = Number(nodeVersion.replace(/^v/, "").split(".")[0]) + const versionNote = + nvmrcMajor !== null && nvmrcMajor !== nodeMajor + ? ` .nvmrc names Node ${nvmrcMajor}, which CI runs, so a result can differ from CI's.` + : "" + + return { + kind: "rerun", + target, + scratchFolders, + coverageFolderToEmpty: coverageRun ? coverageFolder : null, + node, + setprivArguments, + announcement: `Root ignores the file permissions some tests check, so the tests rerun as ${target.name} (uid ${target.uid}) on Node ${nodeVersion}.${versionNote}\n ${printedCommand}`, + } +} + +/** Changes the owner of a path without following a symlink. */ +type ChangeOwner = (params: { path: string; uid: number; gid: number }) => void + +/** Hands a folder and everything inside it to the target, and removes group + * and other write access, because vitest runs code from its caches. */ +const handOverFolder = ({ + folder, + target, + changeOwner, +}: { + folder: string + target: RerunTarget + changeOwner: ChangeOwner +}): void => { + const entries = readdirSync(folder, { recursive: true, encoding: "utf8" }) + const paths = [folder, ...entries.map((entry) => join(folder, entry))] + + for (const path of paths) { + changeOwner({ path, uid: target.uid, gid: target.gid }) + const stats = lstatSync(path) + + // chmod would follow a symlink to its target. + if (stats.isSymbolicLink()) continue + chmodSync(path, stats.mode & ~0o022) + } +} + +/** Copies the Node binary for nobody, root-owned and readable by all, under a + * temp name first so a half-written copy never sits at the path a later run + * reuses. */ +const copyNodeBinary = ({ + sourcePath, + nodePath, +}: { + sourcePath: string + nodePath: string +}): void => { + if (existsSync(nodePath)) return + + const temporaryPath = `${nodePath}.${process.pid}` + + mkdirSync(dirname(nodePath), { recursive: true, mode: 0o755 }) + try { + copyFileSync(sourcePath, temporaryPath) + chmodSync(temporaryPath, 0o755) + renameSync(temporaryPath, nodePath) + } finally { + rmSync(temporaryPath, { force: true }) + } +} + +/** Prepares the folders and Node binary a rerun plan needs. */ +export const prepareRerun = ({ + plan, + changeOwner, +}: { + plan: RerunPlan + changeOwner: ChangeOwner +}): void => { + if (plan.coverageFolderToEmpty && existsSync(plan.coverageFolderToEmpty)) { + for (const entry of readdirSync(plan.coverageFolderToEmpty)) { + rmSync(join(plan.coverageFolderToEmpty, entry), { recursive: true, force: true }) + } + } + for (const folder of plan.scratchFolders) { + mkdirSync(folder, { recursive: true }) + handOverFolder({ folder, target: plan.target, changeOwner }) + } + if (plan.node.kind === "copy") copyNodeBinary(plan.node) +} + +/** Runs a command as the target and reports whether it exited 0. */ +const runsAsTarget = ({ + target, + command, +}: { + target: RerunTarget + command: readonly string[] +}): boolean => { + const result = spawnSync( + "setpriv", + [`--reuid=${target.uid}`, `--regid=${target.gid}`, "--clear-groups", ...command], + { stdio: "ignore" }, + ) + return result.status === 0 +} + +/** The id `id` prints for a user, or null when there is no such user. */ +const readUserId = ({ flag, user }: { flag: "-u" | "-g"; user: string }): number | null => { + const result = spawnSync("id", [flag, user], { encoding: "utf8" }) + + if (result.status !== 0) return null + return Number(result.stdout.trim()) +} + +/** The user the rerun runs as, or the reason there is none. */ +const findRerunTarget = (checkoutRoot: string): RerunTarget | WarningCause => { + const setprivCheck = spawnSync("setpriv", ["--version"], { stdio: "ignore" }) + + if (setprivCheck.error) return { kind: "no-setpriv" } + + const checkoutStats = statSync(checkoutRoot) + + if (checkoutStats.uid !== 0) { + return { + kind: "owner", + name: "the checkout's owner", + uid: checkoutStats.uid, + gid: checkoutStats.gid, + } + } + + const nobodyUid = readUserId({ flag: "-u", user: "nobody" }) + const nobodyGid = readUserId({ flag: "-g", user: "nobody" }) + + if (nobodyUid === null || nobodyGid === null) return { kind: "no-nobody-user" } + return { kind: "nobody", name: "nobody", uid: nobodyUid, gid: nobodyGid } +} + +const isRerunTarget = (value: RerunTarget | WarningCause): value is RerunTarget => { + return value.kind === "nobody" || value.kind === "owner" +} + +const probeTarget = ({ + target, + configPath, + execPath, + extraCaCerts, +}: { + target: RerunTarget + configPath: string + execPath: string + extraCaCerts: string | undefined +}): TargetProbes => { + const testsFile = (testFlag: "-r" | "-x", path: string): boolean => { + return runsAsTarget({ target, command: ["/bin/sh", "-c", `test ${testFlag} "$1"`, "sh", path] }) + } + + return { + setprivWorks: runsAsTarget({ target, command: ["true"] }), + canReadConfig: testsFile("-r", configPath), + canRunNode: testsFile("-x", execPath), + canReadExtraCaCerts: extraCaCerts ? testsFile("-r", extraCaCerts) : true, + } +} + +/** + * Called by vitest.config.ts while vitest loads it. Does nothing unless the + * process is root. Otherwise reruns the same vitest command as another user + * and exits with its status, or prints a warning and returns so the run + * continues as root. + */ +export const stopRootTestRun = ({ + checkoutRoot, + configPath, +}: { + checkoutRoot: string + configPath: string +}): void => { + if (process.getuid?.() !== 0) return + + const warn = (cause: WarningCause): void => { + process.stderr.write(`${formatWarning(cause)}\n`) + } + const targetOrCause = findRerunTarget(checkoutRoot) + + if (!isRerunTarget(targetOrCause)) { + warn(targetOrCause) + return + } + + const extraCaCerts = process.env.NODE_EXTRA_CA_CERTS + const nvmrcPath = join(checkoutRoot, ".nvmrc") + const [, vitestEntry, ...vitestArguments] = process.argv + + // Node always puts the script it runs (vitest's entry) in argv[1]. + if (!vitestEntry) throw new Error("vitest's entry file is missing from process.argv") + + const plan = planRootRun({ + checkoutRoot, + target: targetOrCause, + probes: probeTarget({ + target: targetOrCause, + configPath, + execPath: process.execPath, + extraCaCerts, + }), + execPath: process.execPath, + nodeVersion: process.version, + nvmrcText: existsSync(nvmrcPath) ? readFileSync(nvmrcPath, "utf8") : null, + execArguments: process.execArgv, + vitestEntry, + vitestArguments, + coverageRun: isCoverageRun(vitestArguments), + extraCaCertsSet: Boolean(extraCaCerts), + pathEnvironment: process.env.PATH ?? "", + }) + + if (plan.kind === "warn") { + process.stderr.write(`${plan.message}\n`) + return + } + + try { + prepareRerun({ + plan, + changeOwner: ({ path, uid, gid }) => lchownSync(path, uid, gid), + }) + } catch (error) { + warn({ kind: "prepare-failed", reason: error instanceof Error ? error.message : String(error) }) + return + } + + process.stderr.write(`${plan.announcement}\n`) + + const rerun = spawnSync("setpriv", [...plan.setprivArguments], { stdio: "inherit" }) + + if (rerun.error) { + warn({ kind: "spawn-failed", reason: rerun.error.message }) + return + } + process.exit(rerun.status ?? 1) +} diff --git a/vitest.config.ts b/vitest.config.ts index 219a9e6cd..79c2eddfd 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,4 +1,11 @@ import { defineConfig } from "vitest/config" +import { stopRootTestRun } from "./scripts/root-test-guard.js" + +// vitest sets VITEST before it loads this file. Other tools that load it, +// such as knip, don't, so the guard runs only for a real vitest run. +if (process.env.VITEST === "true") { + stopRootTestRun({ checkoutRoot: import.meta.dirname, configPath: import.meta.filename }) +} export default defineConfig({ test: { From 9238225c1fdcbf442efe1565c116760c057a274a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:33:32 +0000 Subject: [PATCH 02/14] chore(hooks): remove the hook that guessed root test runs from Bash commands The PreToolUse hook read every Bash command's text to guess whether vitest would start. It blocked commands that only mentioned a test run and missed runs it could not parse. vitest.config.ts now reruns a root test run itself, so the hook, its test, its settings.json entry and its references go. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 349 ------ .claude/settings.json | 12 - .gitignore | 1 - scripts/__tests__/lint-shell.test.ts | 1 - src/__tests__/require-nonroot-tests.test.ts | 1078 ------------------- 5 files changed, 1441 deletions(-) delete mode 100755 .claude/hooks/require-nonroot-tests.sh delete mode 100644 src/__tests__/require-nonroot-tests.test.ts diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh deleted file mode 100755 index 3d2f925b8..000000000 --- a/.claude/hooks/require-nonroot-tests.sh +++ /dev/null @@ -1,349 +0,0 @@ -#!/usr/bin/env bash -# Stops a test run in a root session and replies with the setpriv prefix that -# runs the same command as the nobody user. Root ignores file permissions, so a -# test that chmods a file or folder to deny access, then expects the access to -# fail, breaks as root, while CI, which runs as a normal user, passes it. -# - Claude Code's cloud sessions run as root. In a non-root session the hook -# exits at its first check, so it costs nothing on a developer machine. -# - Before replying, it makes the folders a test run writes in the checkout -# writable for nobody. -# - .claude/settings.json registers the hook on PreToolUse for Bash with a -# 10-second timeout. Claude Code cancels a slower hook and runs the command -# anyway, so the hook must stay fast. -# - Exit 0 lets the command run. Exit 2 blocks it, and Claude Code passes the -# hook's stderr text to the model as the reply. -set -euo pipefail - -if [[ "$(id -u)" != "0" ]]; then - exit 0 -fi - -# Without setpriv or a nobody user there is no command to offer, so the hook -# lets the run through rather than blocking it with no way forward. -if ! command -v setpriv >/dev/null 2>&1 || ! id nobody >/dev/null 2>&1; then - exit 0 -fi - -# Claude Code sends the PreToolUse payload as JSON on stdin -# (`{ "tool_input": { "command": ... }, "cwd": ..., ... }`). node prints the -# payload's command and cwd (the session's current folder), each ended by a -# NUL, the one character a shell command cannot hold. -# - Without node, or with a payload node cannot parse, node prints nothing. -# Each read then fails and leaves its variable empty (`|| true` keeps set -e -# from ending the hook), and an empty command lets the call through. -# - The script is single-quoted on purpose: its ${...} is JavaScript, not shell. -# shellcheck disable=SC2016 -{ - IFS= read -r -d '' tool_command || true - IFS= read -r -d '' payload_cwd || true -} < <(node -e ' - const chunks = [] - // Buffer.concat runs before toString, because a multi-byte character can span two chunks. - process.stdin.on("data", (chunk) => chunks.push(chunk)).on("end", () => { - const parsed = JSON.parse(Buffer.concat(chunks).toString("utf8")) - process.stdout.write(`${parsed.tool_input?.command ?? ""}\0${parsed.cwd ?? ""}\0`) - }) -' 2>/dev/null) - -# Two patterns match the flags between a command and what it runs. -# - flags_without_argument matches flags that take no argument (`env -i`, -# `time -p`). -# - flags_with_optional_argument matches flags that may each take one argument -# (`timeout -k 5`, `npx -p vitest`). A word after such a flag can be read as -# its argument or as the next part of the command (`npx -y vitest`, -# `timeout -k 5 600`), and the match takes whichever reading lets the rest -# of the pattern match. -flags_without_argument='([[:space:]]+-[^[:space:]]*)*' -flags_with_optional_argument='([[:space:]]+-[^[:space:]]*([[:space:]]+[^-[:space:]][^[:space:]]*)?)*' - -# Three patterns match a test runner. -# - vitest_runner matches vitest itself, bare or by path -# (node_modules/.bin/vitest), with or without a version (`vitest@latest`), -# after npx and its flags (`npx -y vitest`), or as -# `node node_modules/vitest/vitest.mjs`. -# - npm_exec_runner matches vitest after `npm exec` or `npm x`, each with -# npm's flags (`npm x -y vitest`). -# - npm_runner matches `npm test`, `npm t`, and `npm run` or `npm run-script` -# of the scripts test, test:coverage, test:watch and snapshot:update, each -# after any npm flags (`npm -s test`, `npm run --silent test`). It leaves -# out test:remote-boot and test:cli-pty, the two suites exempt from this -# hook because they cannot run as nobody; the exemptions below give each -# reason. -vitest_binary='([^[:space:]]*/)?vitest(@[^[:space:]]*)?' -vitest_runner="(npx${flags_with_optional_argument}[[:space:]]+)?${vitest_binary}|node[[:space:]]+[^[:space:]]*vitest\.mjs" -npm_exec_runner="npm${flags_with_optional_argument}[[:space:]]+(exec|x)${flags_with_optional_argument}[[:space:]]+${vitest_binary}" -npm_runner="npm${flags_with_optional_argument}[[:space:]]+(test|t|run(-script)?${flags_with_optional_argument}[[:space:]]+(test|test:coverage|test:watch|snapshot:update))" -test_runner="${vitest_runner}|${npm_exec_runner}|${npm_runner}" - -# variable_setting matches a VAR=value setting before the command -# (`CI=1 npm test`), with the value in one of three forms. -# - double_quoted matches text in double quotes, with no " inside. -# - single_quoted matches text in single quotes, with no ' inside. -# - unquoted matches text up to the first space, ' or ". -double_quoted='"[^"]*"' -single_quoted="'[^']*'" -unquoted="[^[:space:]'\"]*" -variable_setting="[A-Za-z_][A-Za-z0-9_]*=(${double_quoted}|${single_quoted}|${unquoted})[[:space:]]+" - -# wrapper matches these commands, which run the command after them: -# - env and time, with flags that take no argument, because a flag's argument -# would look the same as the command it runs. -# - exec and command, without flags, so `command -v vitest` is not a run. -# - timeout, whose flags may each take one argument, then the duration -# (`timeout -s KILL 600 npm test`, `timeout -k 5 600 npm test`). -# - bash, sh and zsh with their flags, where a group of one-letter flags that -# holds o or O takes an argument (`bash -euo pipefail -c`, -# `bash -O extglob -c`), then the opening quote of the command they run and -# any spaces after it (`bash -lc 'npm test'`). The split below ignores -# quotes, so a run or a cd after ; or && inside the quoted text counts too -# (`bash -c 'npm ci && cd /repo && npm test'`), and a mention there -# (`bash -c 'echo npm test'`) does not. -# setpriv is not a wrapper here: a run through setpriv is the nobody run the -# reply asks for, so the runner after it never counts. -timeout_wrapper="timeout${flags_with_optional_argument}[[:space:]]+[0-9][^[:space:]]*" -shell_flags='([[:space:]]+([-+][[:alpha:]]*[oO][[:alpha:]]*[[:space:]]+[^[:space:]]+|[-+][^[:space:]]*))*' -shell_wrapper="(bash|sh|zsh)${shell_flags}[[:space:]]+([\"'][[:space:]]*)?" -wrapper="((env|time)${flags_without_argument}|exec|command|${timeout_wrapper})[[:space:]]+|${shell_wrapper}" - -# shell_keyword matches a shell keyword a command can follow: if -# (`if npm test`), then, do, else, elif, while, until, ! (`! npm test`) and { -# (`{ npm test; }`). -shell_keyword='(if|then|do|else|elif|while|until|!|\{)[[:space:]]+' - -command_prefix="(${wrapper}|${shell_keyword}|${variable_setting})*" - -# The runner ends at a space, a ), the end of its command, or a closing quote -# followed only by spaces and then a redirection (`bash -c 'npm test' 2>`), a -# ) (`$(bash -c 'npm test')`), a comment or the end. So `npm run testx` and -# `npm run test:cli-pty` are not runs, and neither is the `vitest' src/` that -# `grep -E 'npm test|vitest' src/` splits into. -runner_end="([[:space:]]|\\)|[\"'][[:space:]]*([0-9]*[<>]|\\)|$)|[\"'][[:space:]]+#|$)" - -# A command that runs tests starts with the runner, after any prefix. A -# mention elsewhere, as in `grep vitest package.json`, is not a run. -test_run="^[[:space:]]*${command_prefix}(${test_runner})${runner_end}" - -# exempt_suite_run matches a run of the remote-boot or cli-pty suite, the two -# exempt suites, by the config flag that names the suite's config file, with -# or without quotes around the name. The file name must end where a runner -# ends, so `vitest.cli-pty.config.ts.bak` does not count. -# - arguments_before_double_dash matches words other than a lone `--`. vitest -# reads no flags after a lone `--`, so -# `vitest run -- --config vitest.cli-pty.config.ts` runs the main suite. -# - vitest_exempt_suite_run matches the flag anywhere among vitest's arguments -# before a lone `--` (`vitest run --config vitest.cli-pty.config.ts`). -# - npm_exempt_suite_run matches the flag only after npm's first lone `--`, -# which hands the flag to vitest, and before a second one. npm itself -# consumes the flag in `npm test --config x`, and the script runs the main -# suite. -# - Neither matches a run through `npm exec` or `npm x`. npm reads a flag -# after the command as its own unless a lone `--` comes first, so vitest -# never sees the flag in `npm exec vitest run --config -# vitest.cli-pty.config.ts`. The hook stops every such run rather than -# track where npm's `--` falls. -exempt_suite_config="(-c|--config)(=|[[:space:]]+)[\"']?([^[:space:]\"']*/)?vitest\.(remote-boot|cli-pty)\.config\.ts[\"']?${runner_end}" -arguments_before_double_dash='([[:space:]]+(-|-?[^-[:space:]][^[:space:]]*|--[^[:space:]]+))*' -vitest_exempt_suite_run="^[[:space:]]*${command_prefix}(${vitest_runner})${arguments_before_double_dash}[[:space:]]+${exempt_suite_config}" -npm_exempt_suite_run="^[[:space:]]*${command_prefix}(${npm_runner})${arguments_before_double_dash}[[:space:]]+--${arguments_before_double_dash}[[:space:]]+${exempt_suite_config}" -exempt_suite_run="${vitest_exempt_suite_run}|${npm_exempt_suite_run}" - -# The walk below tells four kinds of directory change apart. The same prefix -# a test run may carry (a wrapper, a shell keyword, a VAR=value setting: -# `{ cd /repo; npm test; }`, `then cd /repo`, `env bash -c 'cd /repo && npm -# test'`) is cut off first, with leading_command_prefix, so the first three -# patterns start at cd or pushd and the path is the second group of their -# match. any_cd accepts that prefix itself for the grep below. -# - cd_to_absolute_path matches an unquoted path that starts with /. -# - cd_to_quoted_absolute_path matches a quoted path that starts with / and -# holds no $, backtick or ~, so the shell would use it as written. -# - cd_to_relative_path matches an unquoted relative path. Its first -# character cannot be - (`cd -` or a flag), # (a comment, which leaves a -# bare `cd`), or ~, $ or a quote, which only the shell can expand. -# - any_cd matches any cd, pushd or popd. The walk tries it last, so it -# catches the rest: a bare `cd`, with or without a comment after it, -# `cd -`, popd, and a ~, $ or quoted path the others do not take. -directory_change="^[[:space:]]*(cd|pushd)[[:space:]]+" -cd_to_absolute_path="${directory_change}(/[^[:space:]]*)" -cd_to_quoted_absolute_path="${directory_change}[\"'](/[^\"'\$\`~]*)[\"']" -cd_to_relative_path="${directory_change}([^-~#\"'\$[:space:]][^[:space:]]*)" -leading_command_prefix="^[[:space:]]*${command_prefix}" -any_cd="^[[:space:]]*${command_prefix}(cd|pushd|popd)([[:space:]]|$)" - -# The command is split into the commands it runs, one per line, and only the -# cds and test runs the walk below acts on are kept. -# - A command starts at the start of a line, or after ;, &, |, ( or a -# backtick. The ( also covers $( ). -# - tr maps each of the five characters to a line break, which SC2020 takes -# for a word replacement. tr, not bash's ${//}, because bash's substitution -# slows to seconds on a large multi-byte command. -# - grep drops the other commands in one pass. The walk's =~ tests take -# seconds when they run on each line of a command thousands of lines long. -# - grep exits 1 when it keeps no line, so `|| true` stops set -e and -# pipefail from ending the hook. -# shellcheck disable=SC2020 -cd_or_test_commands="$(printf '%s\n' "${tool_command}" | - tr ';&|(`' '\n\n\n\n\n' | - grep -E "${any_cd}|${test_run}")" || true - -# The walk checks each command on its own, so an exemption covers only its -# own command, and it tracks the folder the first stopped run runs in. -# - Two kinds of run are exempt. -# - A run through setpriv never matches test_run, because setpriv is not -# one of the wrappers above. -# - Two suites have no permission tests and cannot run as nobody. Their -# npm scripts never match test_run, and exempt_suite_run drops a run with -# either suite's config. -# - remote-boot needs the Docker socket, which nobody cannot open. -# - cli-pty starts the CLI with npx from a temp folder, so npx downloads -# tsx into nobody's empty npm cache. Cloud sessions reach the npm -# registry through a proxy whose CA bundle sits under /root, where -# nobody cannot read it, so that download fails. -# - The run's folder starts as the session's folder, the first of these that -# is set: the payload's cwd, CLAUDE_PROJECT_DIR (which Claude Code sets to -# the project root), and `.`. Claude Code runs the hook in the session's -# current folder, so `.` and any path built on it resolve against that -# folder. -# - Each `cd` or `pushd` before the run moves it: to an absolute path, -# unquoted or quoted as written (`git pull && cd /other/checkout && npm -# test`), or relative to the folder so far for an unquoted relative path -# (`cd /other/checkout && cd src`). For any other cd, and for popd, the -# session's folder stands in for the target, which the hook does not -# resolve. A cd after the run does not count. -# - A cd after env, exec or timeout moves it too, though the shell's folder -# stays put: those commands run a program, and cd is a shell builtin. -# - npm's --prefix folder is not followed: `npm --prefix /other/checkout -# test` counts as `npm test`. -session_dir="${payload_cwd:-${CLAUDE_PROJECT_DIR:-.}}" -run_dir="${session_dir}" -current_dir="${session_dir}" - -# root_test_runs collects the stopped runs' commands, one per line, so the -# coverage hint at the end can look for a coverage run among them. -root_test_runs="" -while IFS= read -r split_command; do - cd_candidate="${split_command}" - if [[ "${split_command}" =~ ${leading_command_prefix} ]]; then - cd_candidate="${split_command:${#BASH_REMATCH[0]}}" - fi - if [[ "${cd_candidate}" =~ ${cd_to_absolute_path} ]]; then - current_dir="${BASH_REMATCH[2]}" - continue - fi - if [[ "${cd_candidate}" =~ ${cd_to_quoted_absolute_path} ]]; then - current_dir="${BASH_REMATCH[2]}" - continue - fi - if [[ "${cd_candidate}" =~ ${cd_to_relative_path} ]]; then - current_dir="${current_dir}/${BASH_REMATCH[2]}" - continue - fi - if [[ "${cd_candidate}" =~ ${any_cd} ]]; then - current_dir="${session_dir}" - continue - fi - - # Every line left is a test run, except the one empty line the here-string - # passes when grep kept no line. - [[ "${split_command}" =~ ${test_run} ]] || continue - - if [[ "${split_command}" =~ ${exempt_suite_run} ]]; then - continue - fi - - if [[ -z "${root_test_runs}" ]]; then - run_dir="${current_dir}" - fi - root_test_runs+="${split_command}"$'\n' -done <<<"${cd_or_test_commands}" - -if [[ -z "${root_test_runs}" ]]; then - exit 0 -fi - -# The hook prepares the checkout that holds the run's folder. -# - When the run's folder is in no checkout, the session's folder's checkout -# stands in, and the run is still stopped rather than let through as root. -# - When neither folder is in a checkout, there are no folders to prepare, -# and the run is let through. -checkout="$(git -C "${run_dir}" rev-parse --show-toplevel 2>/dev/null || - git -C "${session_dir}" rev-parse --show-toplevel 2>/dev/null)" || exit 0 - -# Without node_modules there is nothing to run yet. A folder made under -# node_modules here would also stop the install: install-deps.sh skips npm ci -# in a checkout that has a node_modules folder it did not install. -if [[ ! -d "${checkout}/node_modules" ]]; then - exit 0 -fi - -nobody_uid="$(id -u nobody)" -nobody_gid="$(id -g nobody)" - -# The prefix runs whichever node nobody finds on PATH. nobody cannot read -# root's home, where nvm installs Node, so that node can be another version -# than the .nvmrc one CI runs, or missing. Without one there is no command to -# offer, so the run goes through, as it does without setpriv or a nobody user. -nobody_node_version="$(setpriv --reuid="${nobody_uid}" --regid="${nobody_gid}" --clear-groups node --version 2>/dev/null)" || nobody_node_version="" - -if [[ -z "${nobody_node_version}" ]]; then - exit 0 -fi - -# A test run writes in these folders: vitest's caches under node_modules, a -# fake HOME that keeps npm's cache and logs out of root's home, and coverage/ -# for test:coverage's report (gitignored). -nobody_writable=( - "${checkout}/node_modules/.vite-temp" - "${checkout}/node_modules/.vitest" - "${checkout}/node_modules/.nobody-home" - "${checkout}/coverage" -) -mkdir -p "${nobody_writable[@]}" - -# snapshot:update rewrites the tool-surface baseline in this folder, the only -# snapshot folder in the repo. git tracks neither owner nor write bits, so the -# changes below add nothing to a diff. -snapshots_dir="${checkout}/src/vault-mcp/mcp-core/__tests__/__snapshots__" - -if [[ -d "${snapshots_dir}" ]]; then - nobody_writable+=("${snapshots_dir}") -fi - -# The folders go to nobody and are not writable by group or other users, -# because vitest runs code from its caches and checks test output against the -# snapshots. Both commands recurse, so the files a root run left inside also -# go to nobody and lose group and other write access. -chown -R "${nobody_uid}:${nobody_gid}" "${nobody_writable[@]}" -chmod -R go-w "${nobody_writable[@]}" - -# %q quotes the HOME path for the shell, so the prefix still works when the -# checkout's path holds a space. A path of plain characters prints unchanged. -printf -v nobody_home_for_shell '%q' "${checkout}/node_modules/.nobody-home" - -cat >&2 </dev/null)" || nvmrc_version="" -nvmrc_version="${nvmrc_version%%[[:space:]]*}" -nvmrc_major="${nvmrc_version#v}" -nvmrc_major="${nvmrc_major%%.*}" -nobody_node_major="${nobody_node_version#v}" -nobody_node_major="${nobody_node_major%%.*}" - -if [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then - echo "nobody's node is ${nobody_node_version}, not the ${nvmrc_version} that .nvmrc names and CI runs, so a result can differ from CI's." >&2 -fi - -# vitest deletes coverage/ before a coverage run, and deleting it needs write -# access to the checkout root, so the run must keep the folder instead. -if [[ "${root_test_runs}" == *test:coverage* || "${root_test_runs}" == *--coverage* ]]; then - echo "For a coverage run, also pass --coverage.clean=false (after -- for npm run): vitest otherwise deletes coverage/ first, which nobody cannot do in a root-owned checkout." >&2 -fi - -exit 2 diff --git a/.claude/settings.json b/.claude/settings.json index e1a26bbef..853d77bc3 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -12,18 +12,6 @@ ] } ], - "PreToolUse": [ - { - "matcher": "Bash", - "hooks": [ - { - "type": "command", - "command": "\"${CLAUDE_PROJECT_DIR:-.}\"/.claude/hooks/require-nonroot-tests.sh", - "timeout": 10 - } - ] - } - ], "PostToolUse": [ { "matcher": "EnterWorktree", diff --git a/.gitignore b/.gitignore index 63a26cb49..3e9e36875 100644 --- a/.gitignore +++ b/.gitignore @@ -14,7 +14,6 @@ CLAUDE.local.md !.claude/hooks/ .claude/hooks/* !.claude/hooks/install-deps.sh -!.claude/hooks/require-nonroot-tests.sh TASKS.md .DS_Store diff --git a/scripts/__tests__/lint-shell.test.ts b/scripts/__tests__/lint-shell.test.ts index b1edc7dfd..65f524662 100644 --- a/scripts/__tests__/lint-shell.test.ts +++ b/scripts/__tests__/lint-shell.test.ts @@ -18,7 +18,6 @@ import { describe, expect, it, onTestFinished } from "vitest" /** Every shell script in the repo, sorted: what a run with no paths checks. */ const ALL_SHELL_SCRIPTS = [ ".claude/hooks/install-deps.sh", - ".claude/hooks/require-nonroot-tests.sh", ".github/scripts/generate-notes.sh", ".github/scripts/update-changelog.sh", "cli/src/__tests__/integration/fixtures/docker", diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts deleted file mode 100644 index 98f8ff195..000000000 --- a/src/__tests__/require-nonroot-tests.test.ts +++ /dev/null @@ -1,1078 +0,0 @@ -import { execFileSync, spawnSync } from "node:child_process" -import { - chmodSync, - existsSync, - mkdirSync, - mkdtempSync, - readFileSync, - realpathSync, - rmSync, - statSync, - symlinkSync, - writeFileSync, -} from "node:fs" -import { tmpdir } from "node:os" -import { basename, dirname, join, resolve } from "node:path" - -import { describe, expect, it, onTestFinished } from "vitest" - -/** - * Behavioral spec for the Claude Code hook that stops test runs in a root - * session (.claude/hooks/require-nonroot-tests.sh): which commands it stops, - * which checkout's folders it prepares, and the reply it gives. The real hook - * runs under `bash` with real node and git, and with stub `id`, `setpriv` and - * `chown` executables first on PATH. The stub `id` poses as root and reports - * the test runner's own ids for nobody, so the hook's chown needs no - * privileges. - */ - -const HOOK_PATH = resolve(import.meta.dirname, "../../.claude/hooks/require-nonroot-tests.sh") - -/** Stub `id`: `-u` prints STUB_SESSION_UID (0 poses as root), `nobody` - * succeeds only when STUB_NOBODY_EXISTS is 1, and `-u nobody` / `-g nobody` - * print STUB_NOBODY_UID / STUB_NOBODY_GID. Any other call fails loudly. */ -const ID_STUB = `#!/bin/sh -case "$*" in - "-u") echo "$STUB_SESSION_UID" ;; - "nobody") - if [ "$STUB_NOBODY_EXISTS" = "1" ]; then - echo "uid=$STUB_NOBODY_UID(nobody) gid=$STUB_NOBODY_GID(nogroup)" - else - echo "id: 'nobody': no such user" >&2 - exit 1 - fi - ;; - "-u nobody") echo "$STUB_NOBODY_UID" ;; - "-g nobody") echo "$STUB_NOBODY_GID" ;; - *) - echo "stub id: unexpected arguments: $*" >&2 - exit 64 - ;; -esac -` - -/** Stub `setpriv`: drops its own options and runs the rest of the command - * with STUB_NOBODY_PATH as PATH, standing in for the PATH entries nobody - * can read. */ -const SETPRIV_STUB = `#!/bin/sh -while [ "$#" -gt 0 ]; do - case "$1" in - --*) shift ;; - *) break ;; - esac -done -PATH="$STUB_NOBODY_PATH" -export PATH -exec "$@" -` - -/** Stub `chown`: appends its arguments to STUB_CHOWN_LOG, one call per line, - * then runs the real chown at STUB_REAL_CHOWN. The stub cannot look the - * real one up itself: macOS's /bin/sh is bash 3.2, whose `command -p -v` - * searches the current PATH, where the stub comes first. */ -const CHOWN_STUB = `#!/bin/sh -printf '%s\\n' "$*" >> "$STUB_CHOWN_LOG" -exec "$STUB_REAL_CHOWN" "$@" -` - -/** The node nobody finds, which prints STUB_NOBODY_NODE_VERSION as its version. */ -const NOBODY_NODE_STUB = `#!/bin/sh -echo "$STUB_NOBODY_NODE_VERSION" -` - -/** The fixture checkout's .nvmrc, and a nobody node version that matches it. */ -const NVMRC_VERSION = "24" -const MATCHING_NODE_VERSION = "v24.11.0" - -const SNAPSHOTS_FOLDER = "src/vault-mcp/mcp-core/__tests__/__snapshots__" - -type HookFixture = { - /** A fresh git checkout with a node_modules folder and an .nvmrc, symlinks - * resolved the way `git rev-parse --show-toplevel` reports it. */ - checkout: string - /** A folder in no checkout: the hook's working directory and HOME. */ - outsideDir: string - stubBinDir: string - /** nobody's PATH when it has a node: holds only the node stub. */ - nobodyBinDir: string - /** nobody's PATH when it has no node. */ - emptyBinDir: string - chownLog: string -} - -type HookRun = { - status: number | null - stdout: string - stderr: string -} - -type HookRunOptions = { - fixture: HookFixture - /** The hook's stdin, usually built by payloadFor. */ - stdin: string - /** What the stub `id -u` prints. Defaults to 0, posing as root. */ - sessionUid?: number - /** Whether the stub `id nobody` succeeds. Defaults to true. */ - nobodyExists?: boolean - /** Whether nobody finds a node on PATH. Defaults to true. */ - nobodyHasNode?: boolean - /** What nobody's node prints for --version. Defaults to a version that - * matches the fixture's .nvmrc. */ - nobodyNodeVersion?: string - claudeProjectDir?: string - /** The hook's PATH. Defaults to the stubs ahead of the runner's PATH. */ - path?: string -} - -/** The test runner's own ids, which the stub reports for nobody: chown to - * your own uid and primary group needs no privileges. */ -const runnerIds = (): { uid: number; gid: number } => { - const uid = process.geteuid?.() - const gid = process.getegid?.() - - // Compared to undefined, not checked for truth: 0 is root's valid id. - if (uid === undefined || gid === undefined) throw new Error("the hook tests need POSIX ids") - return { uid, gid } -} - -/** The runner's own PATH, which the hook's real tools are found on. */ -const runnerPath = (): string => { - const path = process.env.PATH - - if (!path) throw new Error("the hook tests need a PATH") - return path -} - -/** The real chown on the runner's PATH, which has no stub on it. */ -const realChownPath = (): string => { - return execFileSync("bash", ["-c", "command -v chown"], { encoding: "utf8" }).trimEnd() -} - -/** PATH lookup skips a file without the executable bit, and would run the - * real binary instead. */ -const writeExecutable = (path: string, script: string): void => { - writeFileSync(path, script) - chmodSync(path, 0o755) -} - -const createHookFixture = (): HookFixture => { - // realpath: git reports the checkout with symlinks resolved, and macOS's - // tmpdir is a symlink. - const tempDir = realpathSync(mkdtempSync(join(tmpdir(), "require-nonroot-tests-"))) - onTestFinished(() => rmSync(tempDir, { recursive: true, force: true })) - const stubBinDir = join(tempDir, "bin") - const nobodyBinDir = join(tempDir, "nobody-bin") - const emptyBinDir = join(tempDir, "empty-bin") - const checkout = join(tempDir, "checkout") - const outsideDir = join(tempDir, "outside") - for (const folder of [stubBinDir, nobodyBinDir, emptyBinDir, outsideDir]) { - mkdirSync(folder) - } - - writeExecutable(join(stubBinDir, "id"), ID_STUB) - writeExecutable(join(stubBinDir, "setpriv"), SETPRIV_STUB) - writeExecutable(join(stubBinDir, "chown"), CHOWN_STUB) - writeExecutable(join(nobodyBinDir, "node"), NOBODY_NODE_STUB) - - execFileSync("git", ["init", "--quiet", checkout], { - env: { PATH: runnerPath(), HOME: outsideDir }, - stdio: "pipe", - }) - mkdirSync(join(checkout, "node_modules")) - writeFileSync(join(checkout, ".nvmrc"), `${NVMRC_VERSION}\n`) - return { - checkout, - outsideDir, - stubBinDir, - nobodyBinDir, - emptyBinDir, - chownLog: join(tempDir, "chown.log"), - } -} - -/** A second git checkout with a node_modules folder, beside the fixture's - * checkout, so a test can tell which of the two the hook prepares. */ -const createOtherCheckout = (fixture: HookFixture, folderName = "other-checkout"): string => { - const otherCheckout = join(dirname(fixture.checkout), folderName) - - execFileSync("git", ["init", "--quiet", otherCheckout], { - env: { PATH: runnerPath(), HOME: fixture.outsideDir }, - stdio: "pipe", - }) - mkdirSync(join(otherCheckout, "node_modules")) - return otherCheckout -} - -/** Every tool the hook runs on a stopped run except setpriv. */ -const HOOK_TOOLS_BESIDES_SETPRIV = [ - "bash", - "node", - "git", - "grep", - "tr", - "cat", - "mkdir", - "chown", - "chmod", -] - -/** A PATH folder linking the stub `id` and the real tools, without setpriv. - * With the tools present, only the missing setpriv can let the run through: - * the hook's setpriv check, or its nobody node check, which runs setpriv. */ -const createBinDirWithoutSetpriv = (fixture: HookFixture): string => { - const binDir = join(fixture.outsideDir, "bin-without-setpriv") - mkdirSync(binDir) - const toolPaths = execFileSync( - "bash", - ["-c", 'command -v "$@"', "bash", ...HOOK_TOOLS_BESIDES_SETPRIV], - { encoding: "utf8" }, - ) - .trimEnd() - .split("\n") - for (const toolPath of toolPaths) { - symlinkSync(toolPath, join(binDir, basename(toolPath))) - } - symlinkSync(join(fixture.stubBinDir, "id"), join(binDir, "id")) - return binDir -} - -const hookEnv = (options: Omit): Record => { - const { uid, gid } = runnerIds() - const { fixture } = options - - // Built from scratch, so a CLAUDE_PROJECT_DIR or GIT_DIR in the runner's - // own environment never reaches the hook. - return { - PATH: options.path ?? `${fixture.stubBinDir}:${runnerPath()}`, - HOME: fixture.outsideDir, - STUB_SESSION_UID: String(options.sessionUid ?? 0), - STUB_NOBODY_EXISTS: options.nobodyExists === false ? "0" : "1", - STUB_NOBODY_UID: String(uid), - STUB_NOBODY_GID: String(gid), - STUB_NOBODY_PATH: options.nobodyHasNode === false ? fixture.emptyBinDir : fixture.nobodyBinDir, - STUB_NOBODY_NODE_VERSION: options.nobodyNodeVersion ?? MATCHING_NODE_VERSION, - STUB_CHOWN_LOG: fixture.chownLog, - STUB_REAL_CHOWN: realChownPath(), - ...(options.claudeProjectDir ? { CLAUDE_PROJECT_DIR: options.claudeProjectDir } : {}), - } -} - -const runHook = (options: HookRunOptions): HookRun => { - const result = spawnSync("bash", [HOOK_PATH], { - // Outside any checkout, so the hook's "." fallback can never reach the - // repo running these tests. - cwd: options.fixture.outsideDir, - input: options.stdin, - encoding: "utf8", - env: hookEnv(options), - }) - - return { status: result.status, stdout: result.stdout, stderr: result.stderr } -} - -const payloadFor = ({ command, cwd }: { command: string; cwd?: string }): string => { - return JSON.stringify({ tool_input: { command }, ...(cwd ? { cwd } : {}) }) -} - -/** Runs the hook on a command issued from the fixture's checkout. */ -const runCommandThroughHook = (fixture: HookFixture, command: string): HookRun => { - return runHook({ fixture, stdin: payloadFor({ command, cwd: fixture.checkout }) }) -} - -/** The hook's reply to a stopped run, test-owned so a change to the prefix - * or the wording shows up here. nobodyHomeInPrefix is the HOME path as the - * prefix spells it: the plain path, unless the path needs quoting. */ -const expectedRefusal = ( - checkout: string, - { - nobodyHomeInPrefix = `${checkout}/node_modules/.nobody-home`, - }: { nobodyHomeInPrefix?: string } = {}, -): string => { - const { uid, gid } = runnerIds() - - return [ - "Tests do not run as root in this repo: root reads the files the permission tests make unreadable, so those tests fail here but pass in CI.", - "Run the same test command as the nobody user, with this prefix in front of the test command itself (after any cd):", - ` setpriv --reuid=${uid} --regid=${gid} --clear-groups env HOME=${nobodyHomeInPrefix}`, - `The folders nobody needs to write in ${checkout} are ready.`, - "", - ].join("\n") -} - -const scratchFolders = (checkout: string): string[] => [ - join(checkout, "node_modules", ".vite-temp"), - join(checkout, "node_modules", ".vitest"), - join(checkout, "node_modules", ".nobody-home"), - join(checkout, "coverage"), -] - -const permissionBits = (path: string): number => statSync(path).mode & 0o777 - -/** The chown calls the hook made, each as its space-joined arguments. */ -const recordedChownCalls = (fixture: HookFixture): string[] => { - if (!existsSync(fixture.chownLog)) return [] - return readFileSync(fixture.chownLog, "utf8").trimEnd().split("\n") -} - -const LET_THROUGH: HookRun = { status: 0, stdout: "", stderr: "" } - -describe("require-nonroot-tests hook", () => { - it("resolves id, setpriv and chown to the stubs ahead of the real binaries", () => { - const fixture = createHookFixture() - - const lookup = spawnSync( - "bash", - ["-c", "command -v id; command -v setpriv; command -v chown"], - { cwd: fixture.outsideDir, encoding: "utf8", env: hookEnv({ fixture }) }, - ) - - expect(lookup.stdout).toBe( - `${fixture.stubBinDir}/id\n${fixture.stubBinDir}/setpriv\n${fixture.stubBinDir}/chown\n`, - ) - }) - - describe("in a root session", () => { - it.each([ - { label: "npm test", command: "npm test" }, - { label: "npm t", command: "npm t" }, - { label: "npm run test:watch", command: "npm run test:watch" }, - { label: "a test command on a second line", command: "git status\nnpm test" }, - { label: "a test command after cd and &&", command: "cd / && npm test" }, - { label: "npx vitest", command: "npx vitest run x" }, - { label: "npx with a flag", command: "npx -y vitest run x" }, - { label: "npx with a versioned vitest", command: "npx vitest@latest run" }, - { label: "npm exec vitest", command: "npm exec vitest -- run" }, - { label: "npm x with a flag before vitest", command: "npm x -y vitest run" }, - { label: "npm run-script test", command: "npm run-script test" }, - { label: "bash -c with a quoted test run", command: "bash -lc 'npm test'" }, - { label: "sh -c with a double-quoted vitest run", command: 'sh -c "npx vitest run"' }, - { label: "bash -c with a redirection after the quote", command: "bash -c 'npm test' 2>&1" }, - { label: "bash -c inside a command substitution", command: "out=$(bash -c 'npm test')" }, - { - label: "bash -c with a comment after the quote", - command: "bash -c 'npm test' # main suite", - }, - { label: "bash -c with a space after the opening quote", command: "bash -c ' npm test'" }, - { - label: "bash with -o and its argument before -c", - command: "bash -o pipefail -c 'npm test'", - }, - { - label: "bash with a flag group ending in o and its argument before -c", - command: "bash -euo pipefail -c 'npm test'", - }, - { - label: "sh with o before c in one flag group and o's argument after it", - command: "sh -oc pipefail 'npx vitest run'", - }, - { - label: "vitest with the main config in quotes", - command: 'npx vitest run --config "vitest.config.ts"', - }, - // npm reads the config flag as its own, so vitest never sees it. - { - label: "npm exec vitest with the cli-pty config", - command: "npm exec vitest run --config vitest.cli-pty.config.ts", - }, - // @vitest/ui is not a runner on its own, so only reading it as -p's - // argument reaches the vitest after it. - { - label: "npx with a package flag and its argument", - command: "npx -p @vitest/ui vitest run", - }, - { - label: "npm test with a config flag npm consumes", - command: "npm test --config vitest.cli-pty.config.ts", - }, - { label: "npm with a flag before the script", command: "npm -s test" }, - { - label: "npm with a flag and its argument before the script", - command: "npm --prefix . test", - }, - { - label: "npm run with a flag before the script", - command: "npm run --silent snapshot:update", - }, - { - label: "npm run with a flag and its argument before the script", - command: "npm run --loglevel silent test", - }, - { - label: "vitest by its path under node_modules/.bin", - command: "node_modules/.bin/vitest run", - }, - { label: "node running vitest.mjs", command: "node node_modules/vitest/vitest.mjs run" }, - { label: "npm run snapshot:update", command: "npm run snapshot:update" }, - { label: "a subshell", command: "(cd sub; npm test)" }, - { label: "a subshell inside command substitution", command: "out=$( (npm test) )" }, - { label: "a backtick substitution", command: "out=`npm test 2>&1`" }, - { label: "a variable setting before npx", command: "CI=1 npx vitest run" }, - { label: "env with a variable setting", command: "env CI=1 npm test" }, - { label: "env with a flag", command: "env -i npm test" }, - { label: "a double-quoted value with a space", command: 'FOO="a b" npm test' }, - { label: "time", command: "time npm test" }, - { label: "time with a flag", command: "time -p npm test" }, - { - label: "timeout with a flag argument and a duration", - command: "timeout -k 5 600 npm test", - }, - { label: "timeout with a named signal", command: "timeout -s KILL 600 npm test" }, - { label: "an if condition", command: "if npm test; then :; fi" }, - { label: "a while condition", command: "while npm test; do :; done" }, - { label: "an until condition", command: "until npm test; do sleep 1; done" }, - { label: "a brace group", command: "{ npm test; }" }, - { - label: "a test run after an exempt suite's npm script", - command: "npm run test:cli-pty && npm test", - }, - { - label: "a vitest run whose test filter names an exempt suite", - command: "npm run test:remote-boot; npx vitest run -t cli-pty", - }, - { - label: "vitest with an exempt suite's config placed after --", - command: "npx vitest run -- --config vitest.cli-pty.config.ts", - }, - { - label: "npm handing an exempt suite's config to vitest after a second --", - command: "npm test -- -- --config vitest.cli-pty.config.ts", - }, - { - label: "vitest with a config whose name only starts with an exempt suite's", - command: "npx vitest run --config vitest.cli-pty.config.ts.bak", - }, - { - label: "a test run after a command that mentions setpriv", - command: "grep setpriv notes; npm test", - }, - { - label: "a test run after a run through setpriv", - command: - "setpriv --reuid=65534 --regid=65534 --clear-groups env HOME=/repo/node_modules/.nobody-home npm test; npm test", - }, - ])("stops $label and replies with the nobody prefix", ({ command }) => { - const fixture = createHookFixture() - - const run = runCommandThroughHook(fixture, command) - - expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) - }) - - // Each case also stops `npm test` in the same fixture, so the pass-through - // comes from the command itself, not from a fixture the hook bails on. - it.each([ - { label: "git status", command: "git status" }, - { label: "a mention of vitest as an argument", command: "grep vitest package.json" }, - { label: "a lookup of vitest with command -v", command: "command -v vitest" }, - { label: "a quoted mention of npm test", command: 'echo "npm test"' }, - { label: "a script whose name only starts with test", command: "npm run testx" }, - { label: "npm run test:remote-boot", command: "npm run test:remote-boot" }, - { label: "npm run test:cli-pty", command: "npm run test:cli-pty" }, - { label: "npm run-script of another script", command: "npm run-script lint" }, - { label: "npm exec of another binary", command: "npm exec prettier -- --version" }, - { label: "bash -c with a quoted non-test command", command: "bash -c 'npm run lint'" }, - { - label: "a quoted grep pattern that a | splits after a runner name", - command: "grep -E 'npm test|vitest' src/", - }, - { - label: "vitest with the cli-pty config in quotes", - command: 'npx vitest run --config "vitest.cli-pty.config.ts"', - }, - { - label: "vitest with the remote-boot config in quotes after =", - command: "npx vitest run --config='vitest.remote-boot.config.ts'", - }, - { - label: "a versioned vitest with the cli-pty config", - command: "npx vitest@4 run --config vitest.cli-pty.config.ts", - }, - // vitest refuses a second config flag and runs nothing. - { - label: "vitest with a second config flag", - command: "npx vitest run --config vitest.cli-pty.config.ts --config vitest.config.ts", - }, - // vitest ignores a flag after a lone --, so the cli-pty suite runs. - { - label: "vitest with a second config after --", - command: "npx vitest run --config vitest.cli-pty.config.ts -- --config vitest.config.ts", - }, - { - label: "vitest with the cli-pty config", - command: "npx vitest run --config vitest.cli-pty.config.ts", - }, - { - label: "vitest with the remote-boot config, given with = and a path", - command: "npx vitest run --config=./vitest.remote-boot.config.ts", - }, - { - label: "npx with a flag running vitest with the cli-pty config", - command: "npx -y vitest run --config vitest.cli-pty.config.ts", - }, - { - label: "npm test handing the cli-pty config to vitest after --", - command: "npm test -- --config vitest.cli-pty.config.ts", - }, - { - label: "npm with a flag handing the cli-pty config to vitest after --", - command: "npm -s test -- --config vitest.cli-pty.config.ts", - }, - { - label: "npm test handing the cli-pty config to vitest among other arguments", - command: "npm test --silent -- --reporter=dot --config vitest.cli-pty.config.ts", - }, - { label: "a variable setting as an argument", command: "echo CI=1 npm test" }, - { label: "a variable setting before another command", command: "FOO=1 echo npm test" }, - { label: "env with a flag running another command", command: "env -i echo npm test" }, - { label: "timeout running another command", command: "timeout 600 echo npm test" }, - { - label: "a command already run through setpriv", - command: - "setpriv --reuid=65534 --regid=65534 --clear-groups env HOME=/repo/node_modules/.nobody-home npm test", - }, - ])("lets $label through without preparing folders", ({ command }) => { - const fixture = createHookFixture() - - const run = runCommandThroughHook(fixture, command) - const foldersMade = scratchFolders(fixture.checkout).filter((folder) => existsSync(folder)) - const controlRun = runCommandThroughHook(fixture, "npm test") - - expect({ run, foldersMade, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - foldersMade: [], - controlStatus: 2, - }) - }) - - it("lets a payload that is not JSON through", () => { - const fixture = createHookFixture() - - const run = runHook({ fixture, stdin: "{not json" }) - const controlRun = runCommandThroughHook(fixture, "npm test") - - expect({ run, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - controlStatus: 2, - }) - }) - - it.each([ - { label: "npm run test:coverage", command: "npm run test:coverage" }, - { label: "vitest --coverage", command: "npx vitest run --coverage" }, - ])("adds the coverage hint to the reply for $label", ({ command }) => { - const coverageHint = - "For a coverage run, also pass --coverage.clean=false (after -- for npm run): vitest otherwise deletes coverage/ first, which nobody cannot do in a root-owned checkout.\n" - const fixture = createHookFixture() - - const run = runCommandThroughHook(fixture, command) - - expect(run).toEqual({ - status: 2, - stdout: "", - stderr: `${expectedRefusal(fixture.checkout)}${coverageHint}`, - }) - }) - - it("leaves the coverage hint out when only an exempt suite's run asks for coverage", () => { - const fixture = createHookFixture() - - const run = runCommandThroughHook( - fixture, - "npx vitest run --config vitest.cli-pty.config.ts --coverage; npm test", - ) - - expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) - }) - - it("adds a line to the reply when nobody's node is another major version than .nvmrc", () => { - const fixture = createHookFixture() - - const run = runHook({ - fixture, - stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), - nobodyNodeVersion: "v22.22.0", - }) - - expect(run).toEqual({ - status: 2, - stdout: "", - stderr: `${expectedRefusal(fixture.checkout)}nobody's node is v22.22.0, not the 24 that .nvmrc names and CI runs, so a result can differ from CI's.\n`, - }) - }) - - it("lets a test run through, preparing nothing, when nobody finds no node", () => { - const fixture = createHookFixture() - const stdin = payloadFor({ command: "npm test", cwd: fixture.checkout }) - - const run = runHook({ fixture, stdin, nobodyHasNode: false }) - const foldersMade = scratchFolders(fixture.checkout).filter((folder) => existsSync(folder)) - const controlRun = runHook({ fixture, stdin }) - - expect({ run, foldersMade, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - foldersMade: [], - controlStatus: 2, - }) - }) - - it("quotes the HOME path in the prefix when the checkout's path holds a space", () => { - const fixture = createHookFixture() - const spacedCheckout = createOtherCheckout(fixture, "spaced checkout") - - const run = runHook({ - fixture, - stdin: payloadFor({ command: "npm test", cwd: spacedCheckout }), - }) - - expect(run).toEqual({ - status: 2, - stdout: "", - stderr: expectedRefusal(spacedCheckout, { - nobodyHomeInPrefix: `${spacedCheckout.replaceAll(" ", "\\ ")}/node_modules/.nobody-home`, - }), - }) - }) - - it("compares the major versions when .nvmrc ends its line with CRLF", () => { - const fixture = createHookFixture() - writeFileSync(join(fixture.checkout, ".nvmrc"), `${NVMRC_VERSION}\r\n`) - - const run = runHook({ - fixture, - stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), - nobodyNodeVersion: "v22.22.0", - }) - - expect(run).toEqual({ - status: 2, - stdout: "", - stderr: `${expectedRefusal(fixture.checkout)}nobody's node is v22.22.0, not the 24 that .nvmrc names and CI runs, so a result can differ from CI's.\n`, - }) - }) - - it("leaves the version line out when .nvmrc names an alias rather than a version", () => { - const fixture = createHookFixture() - writeFileSync(join(fixture.checkout, ".nvmrc"), "lts/*\n") - - const run = runHook({ - fixture, - stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), - nobodyNodeVersion: "v22.22.0", - }) - - expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) - }) - - it("creates the four scratch folders a test run writes", () => { - const fixture = createHookFixture() - - const run = runCommandThroughHook(fixture, "npm test") - const missingFolders = scratchFolders(fixture.checkout).filter( - (folder) => !existsSync(folder), - ) - - expect({ status: run.status, missingFolders }).toEqual({ status: 2, missingFolders: [] }) - }) - - it("gives the scratch folders to nobody", () => { - const fixture = createHookFixture() - const { uid, gid } = runnerIds() - - const run = runCommandThroughHook(fixture, "npm test") - - expect({ status: run.status, chownCalls: recordedChownCalls(fixture) }).toEqual({ - status: 2, - chownCalls: [`-R ${uid}:${gid} ${scratchFolders(fixture.checkout).join(" ")}`], - }) - }) - - it("gives the tool-surface snapshot folder to nobody along with the scratch folders", () => { - const fixture = createHookFixture() - const { uid, gid } = runnerIds() - const snapshotsFolder = join(fixture.checkout, SNAPSHOTS_FOLDER) - mkdirSync(join(snapshotsFolder, "tool-surface"), { recursive: true }) - - const run = runCommandThroughHook(fixture, "npm test") - const nobodyOwnedFolders = [...scratchFolders(fixture.checkout), snapshotsFolder] - - expect({ status: run.status, chownCalls: recordedChownCalls(fixture) }).toEqual({ - status: 2, - chownCalls: [`-R ${uid}:${gid} ${nobodyOwnedFolders.join(" ")}`], - }) - }) - - it("closes the scratch folders, and files left inside them, to group and other writes", () => { - const fixture = createHookFixture() - const folders = scratchFolders(fixture.checkout) - for (const folder of folders) { - mkdirSync(folder, { recursive: true }) - chmodSync(folder, 0o777) - } - // A file an earlier root run left behind. - const leftoverFile = join(fixture.checkout, "node_modules", ".vitest", "results.json") - writeFileSync(leftoverFile, "{}") - chmodSync(leftoverFile, 0o666) - - const run = runCommandThroughHook(fixture, "npm test") - - expect({ - status: run.status, - folderModes: folders.map(permissionBits), - leftoverFileMode: permissionBits(leftoverFile), - }).toEqual({ - status: 2, - folderModes: [0o755, 0o755, 0o755, 0o755], - leftoverFileMode: 0o644, - }) - }) - - it("closes the tool-surface snapshot folder, and the files in it, to group and other writes", () => { - const fixture = createHookFixture() - const snapshotFolder = join(fixture.checkout, SNAPSHOTS_FOLDER, "tool-surface") - const snapshotFile = join(snapshotFolder, "default.json") - mkdirSync(snapshotFolder, { recursive: true }) - chmodSync(snapshotFolder, 0o777) - writeFileSync(snapshotFile, "{}") - chmodSync(snapshotFile, 0o666) - - const run = runCommandThroughHook(fixture, "npm test") - - expect({ - status: run.status, - folderMode: permissionBits(snapshotFolder), - fileMode: permissionBits(snapshotFile), - }).toEqual({ status: 2, folderMode: 0o755, fileMode: 0o644 }) - }) - - it("prepares the checkout a leading cd names, not the session's folder", () => { - const fixture = createHookFixture() - - const run = runHook({ - fixture, - stdin: payloadFor({ - command: `cd ${fixture.checkout} && npm test`, - cwd: fixture.outsideDir, - }), - }) - // Without the cd, the session's folder is in no checkout. - const controlRun = runHook({ - fixture, - stdin: payloadFor({ command: "npm test", cwd: fixture.outsideDir }), - }) - - expect({ run, controlStatus: controlRun.status }).toEqual({ - run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, - controlStatus: 0, - }) - }) - - it.each([ - { - label: "after another command", - command: (checkout: string) => `git pull && cd ${checkout} && npm test`, - }, - { - label: "on an earlier line", - command: (checkout: string) => `set -e\ncd ${checkout}\nnpm test`, - }, - { - label: "after a cd to a folder in no checkout", - command: (checkout: string, outsideDir: string) => { - return `cd ${outsideDir} && cd ${checkout} && npm test` - }, - }, - { - label: "before the first of two runs", - command: (checkout: string, outsideDir: string) => { - return `cd ${checkout} && npm test; cd ${outsideDir} && npm test` - }, - }, - { - label: "in double quotes", - command: (checkout: string) => `cd "${checkout}" && npm test`, - }, - { - label: "in single quotes after an unresolvable cd", - command: (checkout: string) => `cd ~ && cd '${checkout}' && npm test`, - }, - { - label: "with pushd", - command: (checkout: string) => `pushd ${checkout} && npm test`, - }, - { - label: "inside a brace group", - command: (checkout: string) => `{ cd ${checkout}; npm test; }`, - }, - { - label: "inside an if block", - command: (checkout: string) => `if true; then cd ${checkout}; fi; npm test`, - }, - { - label: "inside a bash -c body run through env", - command: (checkout: string) => `env bash -c 'cd ${checkout} && npm test'`, - }, - { - label: "after a variable setting", - command: (checkout: string) => `CI=1 cd ${checkout} && npm test`, - }, - { - label: "inside a bash -c body", - command: (checkout: string) => `bash -c 'cd ${checkout} && npm test'`, - }, - { - label: "inside a bash -c body that opens with a space", - command: (checkout: string) => `bash -c ' cd ${checkout} && npm test'`, - }, - { - label: "inside a bash -c body after a flag group that takes an argument", - command: (checkout: string) => `bash -euo pipefail -c 'cd ${checkout} && npm test'`, - }, - ])("prepares the checkout an absolute cd names $label", ({ command }) => { - const fixture = createHookFixture() - - const run = runHook({ - fixture, - stdin: payloadFor({ - command: command(fixture.checkout, fixture.outsideDir), - cwd: fixture.outsideDir, - }), - }) - - expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) - }) - - it("prepares the checkout a relative cd reaches from the folder an earlier cd names", () => { - const fixture = createHookFixture() - mkdirSync(join(fixture.checkout, "src")) - // The parent folder is in no checkout, so the run reaches the checkout - // only when the hook joins the relative path onto the parent. - const checkoutParent = dirname(fixture.checkout) - const relativeSubfolder = join(basename(fixture.checkout), "src") - - const run = runHook({ - fixture, - stdin: payloadFor({ - command: `cd ${checkoutParent} && cd ${relativeSubfolder} && npm test`, - cwd: fixture.outsideDir, - }), - }) - - expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) - }) - - it.each([ - { label: "a ~ path", target: "~" }, - { label: "a variable", target: "$HOME" }, - { label: "a double-quoted path", target: '"src"' }, - { label: "a single-quoted path", target: "'src'" }, - { label: "the previous folder", target: "-" }, - ])( - "prepares the session's checkout for a cd to $label, which only the shell can resolve", - ({ target }) => { - const fixture = createHookFixture() - const otherCheckout = createOtherCheckout(fixture) - // A folder named with the target's literal text: a hook that read the - // target as a relative path would land in the other checkout. - mkdirSync(join(otherCheckout, target)) - - const run = runCommandThroughHook( - fixture, - `cd ${otherCheckout} && cd ${target} && npm test`, - ) - // The first cd alone moves the run to the other checkout, so the run - // above reaches the session's checkout through the second cd, not - // through a hook that ignores every cd. - const controlRun = runCommandThroughHook(fixture, `cd ${otherCheckout} && npm test`) - - expect({ run, controlRun }).toEqual({ - run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, - controlRun: { status: 2, stdout: "", stderr: expectedRefusal(otherCheckout) }, - }) - }, - ) - - it("prepares the session's checkout for a cd followed only by a comment, which bash runs as a bare cd", () => { - const fixture = createHookFixture() - const otherCheckout = createOtherCheckout(fixture) - // A folder named #: a hook that read the comment as a relative path - // would land in the other checkout. - mkdirSync(join(otherCheckout, "#")) - - const run = runCommandThroughHook(fixture, `cd ${otherCheckout}\ncd # back home\nnpm test`) - // The first cd alone moves the run to the other checkout, so the run - // above reaches the session's checkout through the commented cd. - const controlRun = runCommandThroughHook(fixture, `cd ${otherCheckout}\nnpm test`) - - expect({ run, controlRun }).toEqual({ - run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, - controlRun: { status: 2, stdout: "", stderr: expectedRefusal(otherCheckout) }, - }) - }) - - it("prepares the checkout of the first stopped run, not of an exempt run before it", () => { - const fixture = createHookFixture() - const otherCheckout = createOtherCheckout(fixture) - - const run = runCommandThroughHook( - fixture, - `npx vitest run --config vitest.cli-pty.config.ts && cd ${otherCheckout} && npm test`, - ) - - expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(otherCheckout) }) - }) - - it("prepares the session's checkout, not npm's --prefix folder", () => { - const fixture = createHookFixture() - const otherCheckout = createOtherCheckout(fixture) - - const run = runCommandThroughHook(fixture, `npm --prefix ${otherCheckout} test`) - // A cd to the other checkout moves the run there, so the fixture can - // tell the two checkouts apart. - const controlRun = runCommandThroughHook(fixture, `cd ${otherCheckout} && npm test`) - - expect({ run, controlRun }).toEqual({ - run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, - controlRun: { status: 2, stdout: "", stderr: expectedRefusal(otherCheckout) }, - }) - }) - - it("stops a test run thousands of commands after the cd that names its checkout", () => { - const fixture = createHookFixture() - const otherCommands = Array.from({ length: 5000 }, (_, lineIndex) => `echo line ${lineIndex}`) - const command = [`cd ${fixture.checkout}`, ...otherCommands, "npm test"].join("\n") - - const run = runHook({ fixture, stdin: payloadFor({ command, cwd: fixture.outsideDir }) }) - - expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) - }) - - it.each([ - { - label: "a cd after the test run", - command: (checkout: string) => `npm test; cd ${checkout}`, - }, - { - label: "a relative cd out of the checkout", - command: (checkout: string) => `cd ${checkout} && cd .. && npm test`, - }, - { - label: "a quoted cd holding an expansion after an absolute one", - command: (checkout: string) => `cd ${checkout} && cd "$PWD" && npm test`, - }, - { - label: "a popd after an absolute cd", - command: (checkout: string) => `cd ${checkout} && popd && npm test`, - }, - ])("lets a test run through from a folder in no checkout despite $label", ({ command }) => { - const fixture = createHookFixture() - const stdin = payloadFor({ command: command(fixture.checkout), cwd: fixture.outsideDir }) - - const run = runHook({ fixture, stdin }) - const foldersMade = scratchFolders(fixture.checkout).filter((folder) => existsSync(folder)) - // The same cd at the start stops the run, so the pass-through comes from - // the cd's position or form. - const controlRun = runHook({ - fixture, - stdin: payloadFor({ - command: `cd ${fixture.checkout} && npm test`, - cwd: fixture.outsideDir, - }), - }) - - expect({ run, foldersMade, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - foldersMade: [], - controlStatus: 2, - }) - }) - - it("uses CLAUDE_PROJECT_DIR when the payload has no cwd", () => { - const fixture = createHookFixture() - const stdin = payloadFor({ command: "npm test" }) - - const run = runHook({ fixture, stdin, claudeProjectDir: fixture.checkout }) - // Without it, the hook falls back to its own working directory, which is - // in no checkout. - const controlRun = runHook({ fixture, stdin }) - - expect({ run, controlStatus: controlRun.status }).toEqual({ - run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, - controlStatus: 0, - }) - }) - - it("lets a test run through, creating nothing, in a checkout without node_modules", () => { - const fixture = createHookFixture() - const nodeModules = join(fixture.checkout, "node_modules") - rmSync(nodeModules, { recursive: true }) - - const run = runCommandThroughHook(fixture, "npm test") - const nodeModulesCreated = existsSync(nodeModules) - mkdirSync(nodeModules) - const controlRun = runCommandThroughHook(fixture, "npm test") - - expect({ run, nodeModulesCreated, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - nodeModulesCreated: false, - controlStatus: 2, - }) - }) - - it("prepares the session's checkout when a leading cd names a folder in no checkout", () => { - const fixture = createHookFixture() - - const run = runCommandThroughHook(fixture, `cd ${fixture.outsideDir} && npm test`) - const missingFolders = scratchFolders(fixture.checkout).filter( - (folder) => !existsSync(folder), - ) - - expect({ run, missingFolders }).toEqual({ - run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, - missingFolders: [], - }) - }) - - it("lets a test run through, preparing nothing, when setpriv is missing", () => { - const fixture = createHookFixture() - const stdin = payloadFor({ command: "npm test", cwd: fixture.checkout }) - - const run = runHook({ fixture, stdin, path: createBinDirWithoutSetpriv(fixture) }) - const foldersMade = scratchFolders(fixture.checkout).filter((folder) => existsSync(folder)) - const controlRun = runHook({ fixture, stdin }) - - expect({ run, foldersMade, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - foldersMade: [], - controlStatus: 2, - }) - }) - - it("lets a test run through when there is no nobody user", () => { - const fixture = createHookFixture() - const stdin = payloadFor({ command: "npm test", cwd: fixture.checkout }) - - const run = runHook({ fixture, stdin, nobodyExists: false }) - const controlRun = runHook({ fixture, stdin }) - - expect({ run, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - controlStatus: 2, - }) - }) - }) - - it("lets a test run through in a non-root session without preparing folders", () => { - const fixture = createHookFixture() - const stdin = payloadFor({ command: "npm test", cwd: fixture.checkout }) - - const run = runHook({ fixture, stdin, sessionUid: 1000 }) - const foldersMade = scratchFolders(fixture.checkout).filter((folder) => existsSync(folder)) - const controlRun = runHook({ fixture, stdin, sessionUid: 0 }) - - expect({ run, foldersMade, controlStatus: controlRun.status }).toEqual({ - run: LET_THROUGH, - foldersMade: [], - controlStatus: 2, - }) - }) -}) From 363d90151a187bda465891e5d77e76a7fd0b4f70 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:34:16 +0000 Subject: [PATCH 03/14] docs: describe how a root test run reruns as another user AGENTS.md drops the removed hook and names scripts/root-test-guard.ts, and its test conventions say a root run reruns itself. CONTRIBUTING.md tells contributors who run the tests as root what to expect. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 18 ++++++++++++------ CONTRIBUTING.md | 6 ++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fbc9969fd..b19564890 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,9 +61,8 @@ on a folder for the full list. server.json # MCP server registry manifest render.yaml # Render Blueprint; stays at the repo root, the only place Render reads it Dockerfile # Two-target build: local (default) + remote -.claude/ # Committed Claude Code hook setup only (the rest is gitignored): settings.json registers the two hooks below +.claude/ # Committed Claude Code hook setup only (the rest is gitignored): settings.json registers the hook below hooks/install-deps.sh # Runs at session start and worktree entry: loads nvm, installs dependencies when needed (npm ci, sst install), and at session start puts the checkout's Node (the .nvmrc version when nvm has it, else nvm's default alias) first on PATH for the session's later Bash commands - hooks/require-nonroot-tests.sh # Runs before each Bash command: blocks a root session's test runs, makes the folders they write writable for the nobody user, and replies to Claude with the setpriv prefix that reruns the command as nobody obsidian-headless/ # Lockfile-pinned obsidian-headless Sync CLI for the :remote image rootfs/ # Container filesystem overlay for the :remote image: s6 init chain and services in etc/s6-overlay/, and usr/local/bin/get-sync-token, an in-container terminal sign-in to Obsidian that prints the Sync token for .env templates/memory/ # About Me/ memory file templates for new vaults @@ -71,6 +70,7 @@ deploy/ # End-user quickstarts (no clone needed): local/ and remote/ (guide, com assets/ # Static assets (not shipped in Docker) scripts/ # Dev and ops helpers in TypeScript (not shipped in Docker); most open with a header comment saying what they do deployment-env.ts # Loads ~/.config/vault-cortex/.env; shell variables override its values + root-test-guard.ts # Loaded by vitest.config.ts while vitest starts: reruns a root test run as nobody, or as the checkout's owner when root doesn't own it, and warns when it can't instance-env.ts # Derives PUBLIC_URL for manual deploys (npm run lightsail:up) the same way sst.config.ts and deploy.yml do, or the Lambda authorizer rejects tokens tool-surface-capture.ts # Boots one server per settings combination for the tool-surface snapshot test, size report, and LobeHub manifest; an on/off setting that gates tools goes in its SURFACE_AXES cli/src/ # npx vault-cortex CLI, run on the host: bin (entry) → main (wiring) → program (Commander), one module per command (init, configure, upgrade, get-sync-token; lifecycle holds start, restart, logs, and down), and the shared modules they use (prompts, docker, env, scaffold, and others) @@ -858,6 +858,12 @@ Two naming layers — MCP (JSON wire format) and TypeScript (internal): ### Test conventions +- Tests never run as root: root ignores file permissions, so the tests + that deny access with `chmod` fail as root and pass in CI. + `vitest.config.ts` reruns a root run as `nobody`, or as the checkout's + owner, through `scripts/root-test-guard.ts`. When it can't, it prints a + warning and the run continues as root. Run tests as usual; don't add a + `setpriv` prefix yourself. - Tests read as a behavioral spec. One focused `it()` per behavior — a failing test name should identify which behavior regressed without reading the body. @@ -939,17 +945,17 @@ Two naming layers — MCP (JSON wire format) and TypeScript (internal): **Exception — shell scripts outside vitest's include paths.** The remote image's s6 init scripts under `rootfs/etc/s6-overlay/scripts/` are the `vault-mcp` server's boot chain for the `:remote` target, and - the Claude Code hooks sit in `.claude/hooks/`. Neither is a TypeScript + the Claude Code hook sits in `.claude/hooks/`. Neither is a TypeScript module, and vitest's include paths (`src/`, `cli/src/`, `scripts/`) reach neither folder. The init-script tests live in `src/vault-mcp/__tests__/` (`init-check-auth.test.ts`, `init-obsidian-login.test.ts`, `init-first-sync.test.ts`, `init-setup-user.test.ts`, `init-setup-vault.test.ts`, `print-derived-env.test.ts`, which covers the derivation that - `init-derive-env` publishes), and the hook tests in `src/__tests__/` - (`install-deps.test.ts`, `require-nonroot-tests.test.ts`). These script + `init-derive-env` publishes), and the hook's test in `src/__tests__/` + (`install-deps.test.ts`). These script tests run the real script under its own shell (`sh` for the init - scripts, `bash` for the hooks) with stub binaries on `PATH`, and name + scripts, `bash` for the hook) with stub binaries on `PATH`, and name the script they cover — don't move them next to the scripts or widen vitest's include for them. Whole-image behaviour (the init chain's ordering, diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2258d4419..fdfe6171e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,6 +51,12 @@ each config combination serves the correct tool surface, that auth rejects invalid tokens, that write operations actually mutate the vault (read-back verified), and that misconfiguration fails fast at boot. +Some tests deny access to a file or folder and expect the access to +fail, which never happens as root. So when you run the tests as root, for +example inside a container, they rerun as the checkout's owner, or as the +`nobody` user when root owns the checkout, and print a line saying so. +When that isn't possible, they print a warning and those tests fail. + `npm run test:coverage` runs the same suite with a statement-coverage report (V8 provider); the `coverage/` output directory is gitignored. From 95e663a3d9d44f55bf658e0ebe4e0d5ca291d75e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:36:07 +0000 Subject: [PATCH 04/14] docs: say local tooling commits are never feat or fix Hooks, dev scripts and other local tooling are not the server this repo ships, so their commits use chore, ci or build and stay out of the changelog's Features and Bug Fixes. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index b19564890..bdda1284c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1327,6 +1327,12 @@ Contributor and release conventions live in `BREAKING CHANGE:` footer in the PR description, so the generated release notes list it (other accepted flags are in `CONTRIBUTING.md`). +Pick a commit's conventional-commit type by what it changes in the server +this repo ships. Claude Code hooks, dev scripts and other local tooling +are never `feat` or `fix`: use `chore`, or `ci` or `build` where they fit, +so they stay out of the changelog's Features and Bug Fixes. A squash merge +lands the PR title, so the title takes the type of the PR's main purpose. + ### Files that track feature surface **No hardcoded tool or prompt counts in user-facing surfaces.** Never From 84cc147cec89e600da78de50ff7f387ff3a04aca Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:53:44 +0000 Subject: [PATCH 05/14] fix(review): hand vitest's results cache to the rerun user vitest 5 writes its results cache to node_modules/.vite/vitest, not node_modules/.vitest, so a rerun as nobody could not update it and lost the failed-first and duration ordering of test files. Ship-Check: pr-review --- scripts/__tests__/root-test-guard.test.ts | 14 +++++++------- scripts/root-test-guard.ts | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts index 5bce07340..8becb5f33 100644 --- a/scripts/__tests__/root-test-guard.test.ts +++ b/scripts/__tests__/root-test-guard.test.ts @@ -55,7 +55,7 @@ const rootRun = (overrides: Partial[0]> = {}) => const SCRATCH_FOLDERS = [ "/repo/node_modules/.vite-temp", - "/repo/node_modules/.vitest", + "/repo/node_modules/.vite/vitest", "/repo/node_modules/.test-home", "/repo/coverage", "/repo/src/vault-mcp/mcp-core/__tests__/__snapshots__", @@ -307,8 +307,8 @@ describe("prepareRerun", () => { it("creates every scratch folder and hands it, and everything in it, to the target", () => { const checkout = createCheckout() - const cacheFile = join(checkout, "node_modules", ".vitest", "results.json") - mkdirSync(join(checkout, "node_modules", ".vitest"), { recursive: true }) + const cacheFile = join(checkout, "node_modules", ".vite", "vitest", "results.json") + mkdirSync(join(checkout, "node_modules", ".vite", "vitest"), { recursive: true }) writeFileSync(cacheFile, "{}\n") // chmod, because the umask cuts the mode writeFileSync is given. chmodSync(cacheFile, 0o666) @@ -318,7 +318,7 @@ describe("prepareRerun", () => { const handedOver = changeOwner.mock.calls.map(([{ path }]) => path) expect({ - foldersExist: [".vite-temp", ".vitest", ".test-home"].map((folder) => + foldersExist: [".vite-temp", ".vite/vitest", ".test-home"].map((folder) => existsSync(join(checkout, "node_modules", folder)), ), handedOver: handedOver.toSorted(), @@ -329,7 +329,7 @@ describe("prepareRerun", () => { join(checkout, "coverage"), join(checkout, "node_modules", ".test-home"), join(checkout, "node_modules", ".vite-temp"), - join(checkout, "node_modules", ".vitest"), + join(checkout, "node_modules", ".vite", "vitest"), cacheFile, join(checkout, "src/vault-mcp/mcp-core/__tests__/__snapshots__"), ].toSorted(), @@ -343,8 +343,8 @@ describe("prepareRerun", () => { const outsideFile = join(checkout, "outside.txt") writeFileSync(outsideFile, "keep\n") chmodSync(outsideFile, 0o666) - mkdirSync(join(checkout, "node_modules", ".vitest"), { recursive: true }) - const link = join(checkout, "node_modules", ".vitest", "link") + mkdirSync(join(checkout, "node_modules", ".vite", "vitest"), { recursive: true }) + const link = join(checkout, "node_modules", ".vite", "vitest", "link") symlinkSync(outsideFile, link) const changeOwner = vi.fn() diff --git a/scripts/root-test-guard.ts b/scripts/root-test-guard.ts index d846291af..c23753f14 100644 --- a/scripts/root-test-guard.ts +++ b/scripts/root-test-guard.ts @@ -28,9 +28,9 @@ import { import { dirname, join } from "node:path" import { parseCLI } from "vitest/node" -/** Folder names under node_modules the rerun writes in: vite's config - * bundle, vitest's cache, and the rerun's HOME (npm's cache and logs). */ -const NODE_MODULES_SCRATCH_FOLDERS = [".vite-temp", ".vitest", ".test-home"] +/** Folders under node_modules the rerun writes in: vite's config bundle, + * vitest's results cache, and the rerun's HOME (npm's cache and logs). */ +const NODE_MODULES_SCRATCH_FOLDERS = [".vite-temp", ".vite/vitest", ".test-home"] /** The one snapshot folder in the repo; `snapshot:update` rewrites it. */ const SNAPSHOT_FOLDER = "src/vault-mcp/mcp-core/__tests__/__snapshots__" From 999d72ed5e164e69b64f4961bf3dfa0cc50d5aeb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 02:02:34 +0000 Subject: [PATCH 06/14] chore(tests): make the root test guard read without tracing - Rename stopRootTestRun to rerunRootTestRun, since warning paths let the run continue as root, and readNvmrcMajor to readNodeMajor, which now also reads process.version. - Give the target lookup its own result type instead of sharing the kind field with the warning causes. - Name the target's display label, the folders handed over (which stay with the target), the test HOME folder and the file modes. - Build setpriv's switch flags once, derive the coverage run inside the planner, and replace the version parsing with a documented regex. - Say why the target cannot run root's Node, why an owner gets no copy, why the CA setting is dropped, why the Node folder leads PATH, and why the rerun does not rerun itself. - AGENTS.md: the test-conventions bullet opens with what happens, the tree stays alphabetical, and the commit-type rule names what ships. - CONTRIBUTING.md: say what to do when no rerun is possible. Ship-Check: code-quality --- AGENTS.md | 21 ++- CONTRIBUTING.md | 4 +- scripts/__tests__/root-test-guard.test.ts | 121 +++++++------ scripts/root-test-guard.ts | 198 ++++++++++++++-------- vitest.config.ts | 7 +- 5 files changed, 218 insertions(+), 133 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bdda1284c..1012b60d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,8 +70,8 @@ deploy/ # End-user quickstarts (no clone needed): local/ and remote/ (guide, com assets/ # Static assets (not shipped in Docker) scripts/ # Dev and ops helpers in TypeScript (not shipped in Docker); most open with a header comment saying what they do deployment-env.ts # Loads ~/.config/vault-cortex/.env; shell variables override its values - root-test-guard.ts # Loaded by vitest.config.ts while vitest starts: reruns a root test run as nobody, or as the checkout's owner when root doesn't own it, and warns when it can't instance-env.ts # Derives PUBLIC_URL for manual deploys (npm run lightsail:up) the same way sst.config.ts and deploy.yml do, or the Lambda authorizer rejects tokens + root-test-guard.ts # Loaded by vitest.config.ts while vitest starts: reruns a root test run as nobody, or as the checkout's owner when root doesn't own it, and warns when it can't tool-surface-capture.ts # Boots one server per settings combination for the tool-surface snapshot test, size report, and LobeHub manifest; an on/off setting that gates tools goes in its SURFACE_AXES cli/src/ # npx vault-cortex CLI, run on the host: bin (entry) → main (wiring) → program (Commander), one module per command (init, configure, upgrade, get-sync-token; lifecycle holds start, restart, logs, and down), and the shared modules they use (prompts, docker, env, scaffold, and others) __tests__/integration/ # Interactive flows through node-pty in a real PTY: pty-harness, the cli-pty tests, and a fake docker binary in fixtures/ @@ -858,12 +858,14 @@ Two naming layers — MCP (JSON wire format) and TypeScript (internal): ### Test conventions -- Tests never run as root: root ignores file permissions, so the tests - that deny access with `chmod` fail as root and pass in CI. - `vitest.config.ts` reruns a root run as `nobody`, or as the checkout's - owner, through `scripts/root-test-guard.ts`. When it can't, it prints a - warning and the run continues as root. Run tests as usual; don't add a - `setpriv` prefix yourself. +- A root test run reruns itself as another user, because root ignores + file permissions: the tests that deny access with `chmod` fail as root + and pass in CI. `vitest.config.ts` calls `scripts/root-test-guard.ts`, + which reruns the same command as `nobody`, or as the checkout's owner + when root doesn't own the checkout. Run tests as usual; NEVER add a + `setpriv` prefix yourself. When no rerun is possible, the guard prints + a warning naming the cause and the run continues as root, so the + permission tests fail; fix the cause or run as a non-root user. - Tests read as a behavioral spec. One focused `it()` per behavior — a failing test name should identify which behavior regressed without reading the body. @@ -1327,8 +1329,9 @@ Contributor and release conventions live in `BREAKING CHANGE:` footer in the PR description, so the generated release notes list it (other accepted flags are in `CONTRIBUTING.md`). -Pick a commit's conventional-commit type by what it changes in the server -this repo ships. Claude Code hooks, dev scripts and other local tooling +Pick a commit's conventional-commit type by what it changes in what this +repo ships: the server's Docker images and the `vault-cortex` npm CLI. +Claude Code hooks, dev scripts and other local tooling are never `feat` or `fix`: use `chore`, or `ci` or `build` where they fit, so they stay out of the changelog's Features and Bug Fixes. A squash merge lands the PR title, so the title takes the type of the PR's main purpose. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fdfe6171e..416bf29b4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,7 +55,9 @@ Some tests deny access to a file or folder and expect the access to fail, which never happens as root. So when you run the tests as root, for example inside a container, they rerun as the checkout's owner, or as the `nobody` user when root owns the checkout, and print a line saying so. -When that isn't possible, they print a warning and those tests fail. +When that isn't possible, they print a warning naming the cause and those +tests fail. To fix it, install `setpriv` (part of util-linux) if the +warning says it is missing, or run the tests as a non-root user. `npm run test:coverage` runs the same suite with a statement-coverage report (V8 provider); the `coverage/` output directory is gitignored. diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts index 8becb5f33..8b04a4597 100644 --- a/scripts/__tests__/root-test-guard.test.ts +++ b/scripts/__tests__/root-test-guard.test.ts @@ -18,14 +18,14 @@ import { isCoverageRun, planRootRun, prepareRerun, - readNvmrcMajor, + readNodeMajor, } from "../root-test-guard.js" const CHECKOUT = "/repo" const ROOT_NODE = "/root/.nvm/versions/node/v24.21.0/bin/node" const VITEST_ENTRY = "/repo/node_modules/.bin/vitest" -const NOBODY = { kind: "nobody", name: "nobody", uid: 65534, gid: 65534 } as const -const OWNER = { kind: "owner", name: "the checkout's owner", uid: 1000, gid: 1000 } as const +const NOBODY = { kind: "nobody", label: "nobody", uid: 65534, gid: 65534 } as const +const OWNER = { kind: "owner", label: "the checkout's owner", uid: 1000, gid: 1000 } as const const ALL_PROBES_PASS = { setprivWorks: true, canReadConfig: true, @@ -46,14 +46,24 @@ const rootRun = (overrides: Partial[0]> = {}) => execArguments: [], vitestEntry: VITEST_ENTRY, vitestArguments: ["run"], - coverageRun: false, extraCaCertsSet: false, pathEnvironment: "/usr/bin:/bin", ...overrides, }) } -const SCRATCH_FOLDERS = [ +/** rootRun's plan, narrowed to a rerun; any other plan fails the test. */ +const rootRerun = (overrides: Partial[0]> = {}) => { + const plan = rootRun(overrides) + + if (plan.kind !== "rerun") throw new Error(`expected a rerun plan, got ${plan.message}`) + return plan +} + +/** A changeOwner stub typed as prepareRerun's, so its calls read back typed. */ +const createChangeOwner = () => vi.fn[0]["changeOwner"]>() + +const FOLDERS_TO_HAND_OVER = [ "/repo/node_modules/.vite-temp", "/repo/node_modules/.vite/vitest", "/repo/node_modules/.test-home", @@ -68,7 +78,7 @@ const createCheckout = (): string => { return checkout } -describe("readNvmrcMajor", () => { +describe("readNodeMajor", () => { it.each([ { label: "a bare major", nvmrcText: "24\n", major: 24 }, { label: "a full version with v", nvmrcText: "v24.1.0\n", major: 24 }, @@ -78,7 +88,7 @@ describe("readNvmrcMajor", () => { { label: "an alias", nvmrcText: "lts/*\n", major: null }, { label: "an empty file", nvmrcText: "", major: null }, ])("reads $label", ({ nvmrcText, major }) => { - expect(readNvmrcMajor(nvmrcText)).toBe(major) + expect(readNodeMajor(nvmrcText)).toBe(major) }) }) @@ -131,7 +141,7 @@ describe("planRootRun", () => { expect(plan).toEqual({ kind: "rerun", target: NOBODY, - scratchFolders: SCRATCH_FOLDERS, + foldersToHandOver: FOLDERS_TO_HAND_OVER, coverageFolderToEmpty: null, node: { kind: "copy", @@ -161,7 +171,7 @@ describe("planRootRun", () => { expect(plan).toEqual({ kind: "rerun", target: OWNER, - scratchFolders: SCRATCH_FOLDERS, + foldersToHandOver: FOLDERS_TO_HAND_OVER, coverageFolderToEmpty: null, node: { kind: "as-is", nodePath: "/usr/local/bin/node" }, setprivArguments: [ @@ -212,39 +222,49 @@ describe("planRootRun", () => { }) it("unsets NODE_EXTRA_CA_CERTS when the target cannot read its file", () => { - const plan = rootRun({ + const plan = rootRerun({ extraCaCertsSet: true, probes: { ...ALL_PROBES_PASS, canReadExtraCaCerts: false }, }) - expect(plan.kind === "rerun" ? plan.setprivArguments.slice(3, 7) : plan).toEqual([ + expect(plan.setprivArguments).toEqual([ + "--reuid=65534", + "--regid=65534", + "--clear-groups", "/usr/bin/env", "-u", "NODE_EXTRA_CA_CERTS", "HOME=/repo/node_modules/.test-home", + "PATH=/root/.nvm/versions/node/v24.21.0/bin:/usr/bin:/bin", + ROOT_NODE, + VITEST_ENTRY, + "run", ]) }) it("keeps NODE_EXTRA_CA_CERTS when the target can read its file", () => { - const plan = rootRun({ extraCaCertsSet: true }) + const plan = rootRerun({ extraCaCertsSet: true }) - expect(plan.kind === "rerun" ? plan.setprivArguments.slice(3, 5) : plan).toEqual([ + expect(plan.setprivArguments).toEqual([ + "--reuid=65534", + "--regid=65534", + "--clear-groups", "/usr/bin/env", "HOME=/repo/node_modules/.test-home", + "PATH=/root/.nvm/versions/node/v24.21.0/bin:/usr/bin:/bin", + ROOT_NODE, + VITEST_ENTRY, + "run", ]) }) it("empties coverage/ and tells vitest not to clean it on a coverage run", () => { - const plan = rootRun({ vitestArguments: ["run", "--coverage"], coverageRun: true }) - - expect( - plan.kind === "rerun" - ? { - coverageFolderToEmpty: plan.coverageFolderToEmpty, - rerunArguments: plan.setprivArguments.slice(-4), - } - : plan, - ).toEqual({ + const plan = rootRerun({ vitestArguments: ["run", "--coverage"] }) + + expect({ + coverageFolderToEmpty: plan.coverageFolderToEmpty, + rerunArguments: plan.setprivArguments.slice(-4), + }).toEqual({ coverageFolderToEmpty: "/repo/coverage", rerunArguments: [ "run", @@ -256,9 +276,9 @@ describe("planRootRun", () => { }) it("passes Node's own flags through ahead of vitest's entry", () => { - const plan = rootRun({ execArguments: ["--inspect-brk"] }) + const plan = rootRerun({ execArguments: ["--inspect-brk"] }) - expect(plan.kind === "rerun" ? plan.setprivArguments.slice(-4) : plan).toEqual([ + expect(plan.setprivArguments.slice(-4)).toEqual([ ROOT_NODE, "--inspect-brk", VITEST_ENTRY, @@ -267,28 +287,28 @@ describe("planRootRun", () => { }) it("notes a Node major that differs from the one .nvmrc names", () => { - const plan = rootRun({ nodeVersion: "v22.22.0" }) + const plan = rootRerun({ nodeVersion: "v22.22.0" }) - expect(plan.kind === "rerun" ? plan.announcement.split("\n")[0] : plan).toBe( + expect(plan.announcement.split("\n")[0]).toBe( "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v22.22.0. .nvmrc names Node 24, which CI runs, so a result can differ from CI's.", ) }) it("adds no version note when .nvmrc names an alias", () => { - const plan = rootRun({ nodeVersion: "v22.22.0", nvmrcText: "lts/*\n" }) + const plan = rootRerun({ nodeVersion: "v22.22.0", nvmrcText: "lts/*\n" }) - expect(plan.kind === "rerun" ? plan.announcement.split("\n")[0] : plan).toBe( + expect(plan.announcement.split("\n")[0]).toBe( "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v22.22.0.", ) }) it("quotes a path with a space in the printed command", () => { - const plan = rootRun({ + const plan = rootRerun({ checkoutRoot: "/my repo", vitestEntry: "/my repo/node_modules/.bin/vitest", }) - expect(plan.kind === "rerun" ? plan.announcement.split("\n")[1] : plan).toBe( + expect(plan.announcement.split("\n")[1]).toBe( ` setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env 'HOME=/my repo/node_modules/.test-home' PATH=/root/.nvm/versions/node/v24.21.0/bin:"$PATH" /root/.nvm/versions/node/v24.21.0/bin/node '/my repo/node_modules/.bin/vitest' run`, ) }) @@ -299,32 +319,33 @@ describe("prepareRerun", () => { checkout: string, overrides: Partial[0]> = {}, ) => { - const plan = rootRun({ checkoutRoot: checkout, ...overrides }) - - if (plan.kind !== "rerun") throw new Error(`expected a rerun plan, got ${plan.message}`) - return plan + return rootRerun({ checkoutRoot: checkout, ...overrides }) } - it("creates every scratch folder and hands it, and everything in it, to the target", () => { + it("creates every folder to hand over and hands it, and everything in it, to the target", () => { const checkout = createCheckout() const cacheFile = join(checkout, "node_modules", ".vite", "vitest", "results.json") mkdirSync(join(checkout, "node_modules", ".vite", "vitest"), { recursive: true }) writeFileSync(cacheFile, "{}\n") // chmod, because the umask cuts the mode writeFileSync is given. chmodSync(cacheFile, 0o666) - const changeOwner = vi.fn() + const changeOwner = createChangeOwner() prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner }) const handedOver = changeOwner.mock.calls.map(([{ path }]) => path) expect({ - foldersExist: [".vite-temp", ".vite/vitest", ".test-home"].map((folder) => - existsSync(join(checkout, "node_modules", folder)), - ), + foldersExist: [ + "node_modules/.vite-temp", + "node_modules/.vite/vitest", + "node_modules/.test-home", + "coverage", + "src/vault-mcp/mcp-core/__tests__/__snapshots__", + ].map((folder) => existsSync(join(checkout, folder))), handedOver: handedOver.toSorted(), cacheFileMode: lstatSync(cacheFile).mode & 0o777, }).toEqual({ - foldersExist: [true, true, true], + foldersExist: [true, true, true, true, true], handedOver: [ join(checkout, "coverage"), join(checkout, "node_modules", ".test-home"), @@ -340,17 +361,17 @@ describe("prepareRerun", () => { it("hands over a symlink without changing the mode of what it points to", () => { const checkout = createCheckout() - const outsideFile = join(checkout, "outside.txt") - writeFileSync(outsideFile, "keep\n") - chmodSync(outsideFile, 0o666) + const symlinkTarget = join(checkout, "symlink-target.txt") + writeFileSync(symlinkTarget, "keep\n") + chmodSync(symlinkTarget, 0o666) mkdirSync(join(checkout, "node_modules", ".vite", "vitest"), { recursive: true }) const link = join(checkout, "node_modules", ".vite", "vitest", "link") - symlinkSync(outsideFile, link) - const changeOwner = vi.fn() + symlinkSync(symlinkTarget, link) + const changeOwner = createChangeOwner() prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner }) - expect(lstatSync(outsideFile).mode & 0o777).toBe(0o666) + expect(lstatSync(symlinkTarget).mode & 0o777).toBe(0o666) expect(changeOwner).toHaveBeenCalledWith({ path: link, uid: 65534, gid: 65534 }) }) @@ -360,7 +381,7 @@ describe("prepareRerun", () => { writeFileSync(join(checkout, "coverage", "coverage-final.json"), "{}\n") prepareRerun({ - plan: nobodyPlanFor(checkout, { vitestArguments: ["run", "--coverage"], coverageRun: true }), + plan: nobodyPlanFor(checkout, { vitestArguments: ["run", "--coverage"] }), changeOwner: vi.fn(), }) @@ -417,7 +438,7 @@ describe("prepareRerun", () => { const checkout = createCheckout() const sourceNode = join(checkout, "source-node") writeFileSync(sourceNode, "node\n", { mode: 0o755 }) - const changeOwner = vi.fn() + const changeOwner = createChangeOwner() const plan = nobodyPlanFor(checkout, { execPath: sourceNode, probes: { ...ALL_PROBES_PASS, canRunNode: false }, @@ -425,7 +446,7 @@ describe("prepareRerun", () => { prepareRerun({ plan, changeOwner }) - const handedOver = changeOwner.mock.calls.map(([{ path }]) => String(path)) + const handedOver = changeOwner.mock.calls.map(([{ path }]) => path) expect(handedOver.filter((path) => path.includes(".nobody-node"))).toEqual([]) }) }) diff --git a/scripts/root-test-guard.ts b/scripts/root-test-guard.ts index c23753f14..9300eeade 100644 --- a/scripts/root-test-guard.ts +++ b/scripts/root-test-guard.ts @@ -2,8 +2,8 @@ * Reruns a root vitest run as a user the permission tests can rely on. * Root ignores file permissions, so tests that chmod a file or folder to deny * access, then expect the access to fail, fail as root and pass in CI. - * - vitest.config.ts calls `stopRootTestRun` while vitest loads it, before any - * test file, the coverage provider or the watcher starts. + * - vitest.config.ts calls `rerunRootTestRun` while vitest loads it, before + * any test file, the coverage provider or the watcher starts. * - The rerun runs as nobody when root owns the checkout (a cloud or agent * container's clone), and as the checkout's owner otherwise (a contributor's * checkout mounted into a root container). @@ -28,20 +28,37 @@ import { import { dirname, join } from "node:path" import { parseCLI } from "vitest/node" +/** The rerun's HOME under node_modules, where npm keeps its cache and logs. */ +const TEST_HOME_FOLDER = ".test-home" + /** Folders under node_modules the rerun writes in: vite's config bundle, - * vitest's results cache, and the rerun's HOME (npm's cache and logs). */ -const NODE_MODULES_SCRATCH_FOLDERS = [".vite-temp", ".vite/vitest", ".test-home"] + * vitest's results cache, and the rerun's HOME. */ +const NODE_MODULES_SCRATCH_FOLDERS = [".vite-temp", ".vite/vitest", TEST_HOME_FOLDER] -/** The one snapshot folder in the repo; `snapshot:update` rewrites it. */ +/** The only folder inside the checkout that tests write (`snapshot:update` + * rewrites it). A test that writes another folder needs it handed over too, + * or the rerun cannot write there. */ const SNAPSHOT_FOLDER = "src/vault-mcp/mcp-core/__tests__/__snapshots__" /** A plain shell word that needs no quoting when the command is printed. */ const SHELL_SAFE_WORD = /^[\w@%+=:,./-]+$/ +/** The leading major number of a Node version (`24`, `v24.1.0`), ending at a + * dot, whitespace or the end of the text. */ +const NODE_MAJOR_VERSION = /^v?(\d+)(?:[.\s]|$)/ + +/** Write access for group and other, removed from every handed-over path. */ +const GROUP_AND_OTHER_WRITE = 0o022 + +/** rwxr-xr-x: the Node copy and its folders stay root-owned and unwritable by + * the target, but the target can run the copy. */ +const EXECUTABLE_BY_ALL_MODE = 0o755 + type RerunTarget = { /** nobody for a root-owned checkout; the checkout's owner otherwise. */ kind: "nobody" | "owner" - name: string + /** How messages name the target: a user name, or a phrase for an owner. */ + label: string uid: number gid: number } @@ -50,18 +67,22 @@ type RerunTarget = { type WarningCause = | { kind: "no-setpriv" } | { kind: "no-nobody-user" } - | { kind: "setpriv-fails"; targetName: string } - | { kind: "config-unreadable"; targetName: string } - | { kind: "node-unrunnable"; targetName: string } + | { kind: "setpriv-fails"; targetLabel: string } + | { kind: "config-unreadable"; targetLabel: string } + | { kind: "node-unrunnable"; targetLabel: string } | { kind: "prepare-failed"; reason: string } | { kind: "spawn-failed"; reason: string } +/** The user the rerun runs as, or why there is none. */ +type TargetLookup = { kind: "found"; target: RerunTarget } | { kind: "none"; cause: WarningCause } + /** What the target can do, each probed by running a command as the target. */ type TargetProbes = { setprivWorks: boolean + /** The config file stands in for the checkout: vitest must read it first. */ canReadConfig: boolean canRunNode: boolean - /** Only probed when NODE_EXTRA_CA_CERTS is set. */ + /** True when NODE_EXTRA_CA_CERTS is unset; probed only when it is set. */ canReadExtraCaCerts: boolean } @@ -71,7 +92,9 @@ type NodeChoice = type RerunPlan = { kind: "rerun" target: RerunTarget - scratchFolders: readonly string[] + /** Handed to the target and left with it after the run: the scratch + * folders under node_modules, coverage/ and the snapshot folder. */ + foldersToHandOver: readonly string[] coverageFolderToEmpty: string | null node: NodeChoice setprivArguments: readonly string[] @@ -87,11 +110,11 @@ const describeWarningCause = (cause: WarningCause): string => { case "no-nobody-user": return "this system has no nobody user" case "setpriv-fails": - return `setpriv cannot switch to ${cause.targetName}` + return `setpriv cannot switch to ${cause.targetLabel}` case "config-unreadable": - return `${cause.targetName} cannot read the checkout` + return `${cause.targetLabel} cannot read the checkout` case "node-unrunnable": - return `${cause.targetName} cannot run this Node binary` + return `${cause.targetLabel} cannot run this Node binary` case "prepare-failed": return `preparing the rerun failed (${cause.reason})` case "spawn-failed": @@ -103,15 +126,21 @@ const formatWarning = (cause: WarningCause): string => { return `vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: ${describeWarningCause(cause)}.` } -/** The major version .nvmrc names, or null for an alias such as lts/*. */ -export const readNvmrcMajor = (nvmrcText: string): number | null => { - const firstWord = nvmrcText.trimStart().split(/\s/)[0] ?? "" - const majorText = firstWord.replace(/^v/, "").split(".")[0] ?? "" +/** The major version a Node version or .nvmrc names, or null for an alias + * such as lts/*. */ +export const readNodeMajor = (versionText: string): number | null => { + const majorText = NODE_MAJOR_VERSION.exec(versionText.trimStart())?.[1] - if (!/^\d+$/.test(majorText)) return null + if (!majorText) return null return Number(majorText) } +/** setpriv's flags that switch to the target's uid and gid and drop root's + * supplementary groups. */ +const buildSwitchArguments = (target: RerunTarget): string[] => { + return [`--reuid=${target.uid}`, `--regid=${target.gid}`, "--clear-groups"] +} + /** Quotes a word for a POSIX shell only when it needs it. */ const quoteForShell = (word: string): string => { if (SHELL_SAFE_WORD.test(word)) return word @@ -119,8 +148,9 @@ const quoteForShell = (word: string): string => { } /** - * Appends the flags a non-root coverage run needs: it cannot delete - * coverage/ from a root-owned checkout, so the guard empties the folder as + * Appends the flags a non-root coverage run needs. vitest cleans by deleting + * coverage/ itself, which needs write access to the checkout folder, and root + * owns that folder in a container's clone. So the guard empties coverage/ as * root and vitest must not clean it. vitest ignores flags after a lone `--`, * so they go before one. */ @@ -143,6 +173,8 @@ export const isCoverageRun = (vitestArguments: readonly string[]): boolean => { return options.coverage?.enabled === true } +/** The Node binary the rerun starts, or null when the target has none it can + * run. */ const chooseNode = ({ canRunNode, target, @@ -155,8 +187,13 @@ const chooseNode = ({ nodeCopyPath: string }): NodeChoice | null => { if (canRunNode) return { kind: "as-is", nodePath: execPath } - // An owner's checkout never gets a root-owned binary copied into it. + // The target cannot run root's Node when it sits under /root, as nvm's + // does: /root is mode 700, so no other user can enter it. A copy in the + // checkout fixes that, but in an owner's checkout it would leave a large + // root-owned file the owner cannot delete, so an owner gets the warning. if (target.kind === "owner") return null + // nobody can already read the checkout (the config probe passed), and the + // copy's folders are created mode 755, so nobody can reach and run it. return { kind: "copy", sourcePath: execPath, nodePath: nodeCopyPath } } @@ -171,7 +208,6 @@ export const planRootRun = ({ execArguments, vitestEntry, vitestArguments, - coverageRun, extraCaCertsSet, pathEnvironment, }: { @@ -184,20 +220,19 @@ export const planRootRun = ({ execArguments: readonly string[] vitestEntry: string vitestArguments: readonly string[] - coverageRun: boolean extraCaCertsSet: boolean pathEnvironment: string }): GuardPlan => { if (!probes.setprivWorks) { return { kind: "warn", - message: formatWarning({ kind: "setpriv-fails", targetName: target.name }), + message: formatWarning({ kind: "setpriv-fails", targetLabel: target.label }), } } if (!probes.canReadConfig) { return { kind: "warn", - message: formatWarning({ kind: "config-unreadable", targetName: target.name }), + message: formatWarning({ kind: "config-unreadable", targetLabel: target.label }), } } @@ -213,26 +248,31 @@ export const planRootRun = ({ if (!node) { return { kind: "warn", - message: formatWarning({ kind: "node-unrunnable", targetName: target.name }), + message: formatWarning({ kind: "node-unrunnable", targetLabel: target.label }), } } const nodeModules = join(checkoutRoot, "node_modules") - const home = join(nodeModules, ".test-home") const coverageFolder = join(checkoutRoot, "coverage") - const scratchFolders = [ + const foldersToHandOver = [ ...NODE_MODULES_SCRATCH_FOLDERS.map((folderName) => join(nodeModules, folderName)), coverageFolder, join(checkoutRoot, SNAPSHOT_FOLDER), ] - const unsetsExtraCaCerts = extraCaCertsSet && !probes.canReadExtraCaCerts + + const coverageRun = isCoverageRun(vitestArguments) const rerunArguments = coverageRun ? addCoverageFlags(vitestArguments) : [...vitestArguments] + // Node warns on stderr at every start when it cannot read this file, and + // tests that assert a child's exact stderr then fail. + const unsetsExtraCaCerts = extraCaCertsSet && !probes.canReadExtraCaCerts const environmentArguments = [ ...(unsetsExtraCaCerts ? ["-u", "NODE_EXTRA_CA_CERTS"] : []), - `HOME=${home}`, + `HOME=${join(nodeModules, TEST_HOME_FOLDER)}`, ] const commandAfterPath = [node.nodePath, ...execArguments, vitestEntry, ...rerunArguments] - const switchArguments = [`--reuid=${target.uid}`, `--regid=${target.gid}`, "--clear-groups"] + const switchArguments = buildSwitchArguments(target) + // The Node folder goes first on PATH so that a test starting a bare `node` + // child process gets the same binary as the rerun. const nodeFolder = dirname(node.nodePath) const setprivArguments = [ ...switchArguments, @@ -241,8 +281,11 @@ export const planRootRun = ({ `PATH=${nodeFolder}:${pathEnvironment}`, ...commandAfterPath, ] - // The printed command keeps $PATH literal, so it stays short and still - // runs as written in a shell. + + // The guard runs the command itself; it is printed so the reader sees what + // runs as whom. Rerunning it by hand works once the guard has prepared the + // folders, as it has by the time the line prints. $PATH stays literal, so + // the line stays short and still runs as written in a shell. const printedCommand = [ "setpriv", ...switchArguments, @@ -251,8 +294,11 @@ export const planRootRun = ({ `PATH=${quoteForShell(nodeFolder)}:"$PATH"`, ...commandAfterPath.map(quoteForShell), ].join(" ") - const nvmrcMajor = nvmrcText === null ? null : readNvmrcMajor(nvmrcText) - const nodeMajor = Number(nodeVersion.replace(/^v/, "").split(".")[0]) + + // The rerun uses the Node that started vitest, whatever its major. CI runs + // the .nvmrc major, and a different one can pass or fail tests CI doesn't. + const nvmrcMajor = nvmrcText === null ? null : readNodeMajor(nvmrcText) + const nodeMajor = readNodeMajor(nodeVersion) const versionNote = nvmrcMajor !== null && nvmrcMajor !== nodeMajor ? ` .nvmrc names Node ${nvmrcMajor}, which CI runs, so a result can differ from CI's.` @@ -261,11 +307,11 @@ export const planRootRun = ({ return { kind: "rerun", target, - scratchFolders, + foldersToHandOver, coverageFolderToEmpty: coverageRun ? coverageFolder : null, node, setprivArguments, - announcement: `Root ignores the file permissions some tests check, so the tests rerun as ${target.name} (uid ${target.uid}) on Node ${nodeVersion}.${versionNote}\n ${printedCommand}`, + announcement: `Root ignores the file permissions some tests check, so the tests rerun as ${target.label} (uid ${target.uid}) on Node ${nodeVersion}.${versionNote}\n ${printedCommand}`, } } @@ -292,7 +338,7 @@ const handOverFolder = ({ // chmod would follow a symlink to its target. if (stats.isSymbolicLink()) continue - chmodSync(path, stats.mode & ~0o022) + chmodSync(path, stats.mode & ~GROUP_AND_OTHER_WRITE) } } @@ -310,12 +356,14 @@ const copyNodeBinary = ({ const temporaryPath = `${nodePath}.${process.pid}` - mkdirSync(dirname(nodePath), { recursive: true, mode: 0o755 }) + mkdirSync(dirname(nodePath), { recursive: true, mode: EXECUTABLE_BY_ALL_MODE }) try { copyFileSync(sourcePath, temporaryPath) - chmodSync(temporaryPath, 0o755) + chmodSync(temporaryPath, EXECUTABLE_BY_ALL_MODE) renameSync(temporaryPath, nodePath) } finally { + // Removes a half-written copy when the copy or chmod threw; after a + // successful rename the temp name no longer exists. rmSync(temporaryPath, { force: true }) } } @@ -333,10 +381,12 @@ export const prepareRerun = ({ rmSync(join(plan.coverageFolderToEmpty, entry), { recursive: true, force: true }) } } - for (const folder of plan.scratchFolders) { + + for (const folder of plan.foldersToHandOver) { mkdirSync(folder, { recursive: true }) handOverFolder({ folder, target: plan.target, changeOwner }) } + if (plan.node.kind === "copy") copyNodeBinary(plan.node) } @@ -348,11 +398,9 @@ const runsAsTarget = ({ target: RerunTarget command: readonly string[] }): boolean => { - const result = spawnSync( - "setpriv", - [`--reuid=${target.uid}`, `--regid=${target.gid}`, "--clear-groups", ...command], - { stdio: "ignore" }, - ) + const result = spawnSync("setpriv", [...buildSwitchArguments(target), ...command], { + stdio: "ignore", + }) return result.status === 0 } @@ -364,32 +412,34 @@ const readUserId = ({ flag, user }: { flag: "-u" | "-g"; user: string }): number return Number(result.stdout.trim()) } -/** The user the rerun runs as, or the reason there is none. */ -const findRerunTarget = (checkoutRoot: string): RerunTarget | WarningCause => { +const findRerunTarget = (checkoutRoot: string): TargetLookup => { const setprivCheck = spawnSync("setpriv", ["--version"], { stdio: "ignore" }) - if (setprivCheck.error) return { kind: "no-setpriv" } + if (setprivCheck.error) return { kind: "none", cause: { kind: "no-setpriv" } } const checkoutStats = statSync(checkoutRoot) if (checkoutStats.uid !== 0) { - return { + const owner: RerunTarget = { kind: "owner", - name: "the checkout's owner", + label: "the checkout's owner", uid: checkoutStats.uid, gid: checkoutStats.gid, } + + return { kind: "found", target: owner } } const nobodyUid = readUserId({ flag: "-u", user: "nobody" }) const nobodyGid = readUserId({ flag: "-g", user: "nobody" }) - if (nobodyUid === null || nobodyGid === null) return { kind: "no-nobody-user" } - return { kind: "nobody", name: "nobody", uid: nobodyUid, gid: nobodyGid } -} - -const isRerunTarget = (value: RerunTarget | WarningCause): value is RerunTarget => { - return value.kind === "nobody" || value.kind === "owner" + if (nobodyUid === null || nobodyGid === null) { + return { kind: "none", cause: { kind: "no-nobody-user" } } + } + return { + kind: "found", + target: { kind: "nobody", label: "nobody", uid: nobodyUid, gid: nobodyGid }, + } } const probeTarget = ({ @@ -403,15 +453,19 @@ const probeTarget = ({ execPath: string extraCaCerts: string | undefined }): TargetProbes => { - const testsFile = (testFlag: "-r" | "-x", path: string): boolean => { - return runsAsTarget({ target, command: ["/bin/sh", "-c", `test ${testFlag} "$1"`, "sh", path] }) + const passesFileTest = (testFlag: "-r" | "-x", path: string): boolean => { + // The path goes in as "$1", never inside the script, so the shell does + // not parse it; "sh" fills $0, the slot before $1. + const command = ["/bin/sh", "-c", `test ${testFlag} "$1"`, "sh", path] + + return runsAsTarget({ target, command }) } return { setprivWorks: runsAsTarget({ target, command: ["true"] }), - canReadConfig: testsFile("-r", configPath), - canRunNode: testsFile("-x", execPath), - canReadExtraCaCerts: extraCaCerts ? testsFile("-r", extraCaCerts) : true, + canReadConfig: passesFileTest("-r", configPath), + canRunNode: passesFileTest("-x", execPath), + canReadExtraCaCerts: extraCaCerts ? passesFileTest("-r", extraCaCerts) : true, } } @@ -421,25 +475,29 @@ const probeTarget = ({ * and exits with its status, or prints a warning and returns so the run * continues as root. */ -export const stopRootTestRun = ({ +export const rerunRootTestRun = ({ checkoutRoot, configPath, }: { checkoutRoot: string configPath: string }): void => { + // The rerun loads this config again as its own user, so this check is what + // keeps the rerun from rerunning itself. if (process.getuid?.() !== 0) return const warn = (cause: WarningCause): void => { process.stderr.write(`${formatWarning(cause)}\n`) } - const targetOrCause = findRerunTarget(checkoutRoot) + const targetLookup = findRerunTarget(checkoutRoot) - if (!isRerunTarget(targetOrCause)) { - warn(targetOrCause) + if (targetLookup.kind === "none") { + warn(targetLookup.cause) return } + const { target } = targetLookup + const extraCaCerts = process.env.NODE_EXTRA_CA_CERTS const nvmrcPath = join(checkoutRoot, ".nvmrc") const [, vitestEntry, ...vitestArguments] = process.argv @@ -449,9 +507,9 @@ export const stopRootTestRun = ({ const plan = planRootRun({ checkoutRoot, - target: targetOrCause, + target, probes: probeTarget({ - target: targetOrCause, + target, configPath, execPath: process.execPath, extraCaCerts, @@ -462,7 +520,6 @@ export const stopRootTestRun = ({ execArguments: process.execArgv, vitestEntry, vitestArguments, - coverageRun: isCoverageRun(vitestArguments), extraCaCertsSet: Boolean(extraCaCerts), pathEnvironment: process.env.PATH ?? "", }) @@ -490,5 +547,6 @@ export const stopRootTestRun = ({ warn({ kind: "spawn-failed", reason: rerun.error.message }) return } + // status is null when a signal ended the rerun; that run failed too. process.exit(rerun.status ?? 1) } diff --git a/vitest.config.ts b/vitest.config.ts index 79c2eddfd..3492e6667 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,10 +1,11 @@ import { defineConfig } from "vitest/config" -import { stopRootTestRun } from "./scripts/root-test-guard.js" +import { rerunRootTestRun } from "./scripts/root-test-guard.js" // vitest sets VITEST before it loads this file. Other tools that load it, -// such as knip, don't, so the guard runs only for a real vitest run. +// such as knip, don't, so the guard runs only for a real vitest run. The +// rerun loads this file again, but not as root, so the guard returns at once. if (process.env.VITEST === "true") { - stopRootTestRun({ checkoutRoot: import.meta.dirname, configPath: import.meta.filename }) + rerunRootTestRun({ checkoutRoot: import.meta.dirname, configPath: import.meta.filename }) } export default defineConfig({ From 28184ce273df354613633753a2f357c4e2cfc7bd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 02:03:56 +0000 Subject: [PATCH 07/14] chore(tests): hand coverage/ to the rerun user only on a coverage run A run without coverage never writes there, so handing the folder over on every root run only left an empty coverage/ owned by another user. Co-Authored-By: Claude Opus 5.5 --- scripts/__tests__/root-test-guard.test.ts | 10 ++++++---- scripts/root-test-guard.ts | 8 +++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts index 8b04a4597..ac353e9c9 100644 --- a/scripts/__tests__/root-test-guard.test.ts +++ b/scripts/__tests__/root-test-guard.test.ts @@ -67,7 +67,6 @@ const FOLDERS_TO_HAND_OVER = [ "/repo/node_modules/.vite-temp", "/repo/node_modules/.vite/vitest", "/repo/node_modules/.test-home", - "/repo/coverage", "/repo/src/vault-mcp/mcp-core/__tests__/__snapshots__", ] @@ -262,9 +261,11 @@ describe("planRootRun", () => { const plan = rootRerun({ vitestArguments: ["run", "--coverage"] }) expect({ + foldersToHandOver: plan.foldersToHandOver, coverageFolderToEmpty: plan.coverageFolderToEmpty, rerunArguments: plan.setprivArguments.slice(-4), }).toEqual({ + foldersToHandOver: [...FOLDERS_TO_HAND_OVER, "/repo/coverage"], coverageFolderToEmpty: "/repo/coverage", rerunArguments: [ "run", @@ -339,15 +340,16 @@ describe("prepareRerun", () => { "node_modules/.vite-temp", "node_modules/.vite/vitest", "node_modules/.test-home", - "coverage", "src/vault-mcp/mcp-core/__tests__/__snapshots__", ].map((folder) => existsSync(join(checkout, folder))), + // A run without coverage leaves coverage/ alone. + coverageFolderExists: existsSync(join(checkout, "coverage")), handedOver: handedOver.toSorted(), cacheFileMode: lstatSync(cacheFile).mode & 0o777, }).toEqual({ - foldersExist: [true, true, true, true, true], + foldersExist: [true, true, true, true], + coverageFolderExists: false, handedOver: [ - join(checkout, "coverage"), join(checkout, "node_modules", ".test-home"), join(checkout, "node_modules", ".vite-temp"), join(checkout, "node_modules", ".vite", "vitest"), diff --git a/scripts/root-test-guard.ts b/scripts/root-test-guard.ts index 9300eeade..be119f2fa 100644 --- a/scripts/root-test-guard.ts +++ b/scripts/root-test-guard.ts @@ -93,7 +93,8 @@ type RerunPlan = { kind: "rerun" target: RerunTarget /** Handed to the target and left with it after the run: the scratch - * folders under node_modules, coverage/ and the snapshot folder. */ + * folders under node_modules, the snapshot folder, and coverage/ on a + * coverage run. */ foldersToHandOver: readonly string[] coverageFolderToEmpty: string | null node: NodeChoice @@ -254,13 +255,14 @@ export const planRootRun = ({ const nodeModules = join(checkoutRoot, "node_modules") const coverageFolder = join(checkoutRoot, "coverage") + const coverageRun = isCoverageRun(vitestArguments) const foldersToHandOver = [ ...NODE_MODULES_SCRATCH_FOLDERS.map((folderName) => join(nodeModules, folderName)), - coverageFolder, join(checkoutRoot, SNAPSHOT_FOLDER), + // Only a coverage run writes in coverage/. + ...(coverageRun ? [coverageFolder] : []), ] - const coverageRun = isCoverageRun(vitestArguments) const rerunArguments = coverageRun ? addCoverageFlags(vitestArguments) : [...vitestArguments] // Node warns on stderr at every start when it cannot read this file, and // tests that assert a child's exact stderr then fail. From fe685528a91829ec1c0176afb9d890cf63ae78c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 02:08:45 +0000 Subject: [PATCH 08/14] chore(tests): prove prepareRerun ran in the guard's keep-as-is tests Three prepareRerun tests asserted that something stayed as it was: coverage/ untouched, an existing Node copy kept, the Node copy's folder not handed over. Each passed if prepareRerun did nothing at all. They now also assert a side effect only a real run produces, and the hand-over test asserts the exact list of handed-over paths instead of filtering it down to an empty one. Ship-Check: test-audit Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- scripts/__tests__/root-test-guard.test.ts | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts index ac353e9c9..9c0acef85 100644 --- a/scripts/__tests__/root-test-guard.test.ts +++ b/scripts/__tests__/root-test-guard.test.ts @@ -397,7 +397,11 @@ describe("prepareRerun", () => { prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner: vi.fn() }) - expect(readdirSync(join(checkout, "coverage"))).toEqual(["coverage-final.json"]) + expect({ + coverageEntries: readdirSync(join(checkout, "coverage")), + // Proves prepareRerun ran: it creates the rerun's HOME. + testHomeExists: existsSync(join(checkout, "node_modules", ".test-home")), + }).toEqual({ coverageEntries: ["coverage-final.json"], testHomeExists: true }) }) it("copies the Node binary for nobody, mode 755, when the plan calls for a copy", () => { @@ -433,7 +437,11 @@ describe("prepareRerun", () => { prepareRerun({ plan, changeOwner: vi.fn() }) - expect(readFileSync(nodeCopy, "utf8")).toBe("existing\n") + expect({ + nodeCopyContent: readFileSync(nodeCopy, "utf8"), + // Proves prepareRerun ran: it creates the rerun's HOME. + testHomeExists: existsSync(join(checkout, "node_modules", ".test-home")), + }).toEqual({ nodeCopyContent: "existing\n", testHomeExists: true }) }) it("never hands the Node copy's folder to the target", () => { @@ -449,6 +457,15 @@ describe("prepareRerun", () => { prepareRerun({ plan, changeOwner }) const handedOver = changeOwner.mock.calls.map(([{ path }]) => path) - expect(handedOver.filter((path) => path.includes(".nobody-node"))).toEqual([]) + const nodeCopy = join(checkout, "node_modules", ".nobody-node", "node-v24.21.0", "node") + expect({ handedOver: handedOver.toSorted(), nodeCopyExists: existsSync(nodeCopy) }).toEqual({ + handedOver: [ + join(checkout, "node_modules", ".test-home"), + join(checkout, "node_modules", ".vite-temp"), + join(checkout, "node_modules", ".vite", "vitest"), + join(checkout, "src/vault-mcp/mcp-core/__tests__/__snapshots__"), + ].toSorted(), + nodeCopyExists: true, + }) }) }) From db067639498442aa8f47b19cd0175c2d1b219613 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 02:13:58 +0000 Subject: [PATCH 09/14] chore(tests): cover the guard's root-run wiring and its failure paths The guard's tests covered the planner and the folder preparation, but not rerunRootTestRun, the function vitest.config.ts calls. New tests run it with process.getuid, spawnSync, stderr and process.exit stubbed, and cover: - a non-root run returning at once, so the rerun never reruns itself - the warnings for a missing setpriv and a missing nobody user - the choice of target: nobody's uid and gid for a root-owned checkout, the owner's for any other - a failed preparation warning and continuing as root - the exit status: the rerun's own, and 1 when a signal ended it They also cover a first coverage run in a checkout with no coverage/ folder, a single quote in a quoted path in the printed command, and a Node copy that fails part way leaving nothing at the path a later run reuses. Ship-Check: test-audit Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- scripts/__tests__/root-test-guard.test.ts | 265 ++++++++++++++++++++++ 1 file changed, 265 insertions(+) diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts index 9c0acef85..874f70076 100644 --- a/scripts/__tests__/root-test-guard.test.ts +++ b/scripts/__tests__/root-test-guard.test.ts @@ -1,3 +1,4 @@ +import { type SpawnSyncReturns, spawnSync } from "node:child_process" import { chmodSync, existsSync, @@ -7,6 +8,7 @@ import { readdirSync, readFileSync, rmSync, + statSync, symlinkSync, writeFileSync, } from "node:fs" @@ -19,8 +21,14 @@ import { planRootRun, prepareRerun, readNodeMajor, + rerunRootTestRun, } from "../root-test-guard.js" +vi.mock("node:child_process", { spy: true }) +vi.mock("node:fs", { spy: true }) + +const realFs = await vi.importActual("node:fs") + const CHECKOUT = "/repo" const ROOT_NODE = "/root/.nvm/versions/node/v24.21.0/bin/node" const VITEST_ENTRY = "/repo/node_modules/.bin/vitest" @@ -313,6 +321,17 @@ describe("planRootRun", () => { ` setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env 'HOME=/my repo/node_modules/.test-home' PATH=/root/.nvm/versions/node/v24.21.0/bin:"$PATH" /root/.nvm/versions/node/v24.21.0/bin/node '/my repo/node_modules/.bin/vitest' run`, ) }) + + it("escapes a single quote in a quoted path in the printed command", () => { + const plan = rootRerun({ + checkoutRoot: "/dev's repo", + vitestEntry: "/dev's repo/node_modules/.bin/vitest", + }) + + expect(plan.announcement.split("\n")[1]).toBe( + ` setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env 'HOME=/dev'\\''s repo/node_modules/.test-home' PATH=/root/.nvm/versions/node/v24.21.0/bin:"$PATH" /root/.nvm/versions/node/v24.21.0/bin/node '/dev'\\''s repo/node_modules/.bin/vitest' run`, + ) + }) }) describe("prepareRerun", () => { @@ -390,6 +409,23 @@ describe("prepareRerun", () => { expect(readdirSync(join(checkout, "coverage"))).toEqual([]) }) + it("creates coverage/ and hands it over on a coverage run in a checkout without one", () => { + const checkout = createCheckout() + const changeOwner = createChangeOwner() + + prepareRerun({ + plan: nobodyPlanFor(checkout, { vitestArguments: ["run", "--coverage"] }), + changeOwner, + }) + + expect(readdirSync(join(checkout, "coverage"))).toEqual([]) + expect(changeOwner).toHaveBeenCalledWith({ + path: join(checkout, "coverage"), + uid: 65534, + gid: 65534, + }) + }) + it("leaves coverage/ alone on a run without coverage", () => { const checkout = createCheckout() mkdirSync(join(checkout, "coverage"), { recursive: true }) @@ -468,4 +504,233 @@ describe("prepareRerun", () => { nodeCopyExists: true, }) }) + + it("leaves no Node copy behind when the copy fails part way", () => { + const checkout = createCheckout() + const sourceNode = join(checkout, "source-node") + writeFileSync(sourceNode, "node\n", { mode: 0o755 }) + const copyFailure = new Error("ENOSPC: no space left on device") + const chmodSpy = vi.mocked(chmodSync).mockImplementation((path, mode) => { + // Fails the chmod on the temp copy, after its bytes are written. + if (String(path).endsWith(`node.${process.pid}`)) throw copyFailure + realFs.chmodSync(path, mode) + }) + onTestFinished(() => chmodSpy.mockRestore()) + const plan = nobodyPlanFor(checkout, { + execPath: sourceNode, + probes: { ...ALL_PROBES_PASS, canRunNode: false }, + }) + + expect(() => prepareRerun({ plan, changeOwner: vi.fn() })).toThrow(copyFailure) + expect(readdirSync(join(checkout, "node_modules", ".nobody-node", "node-v24.21.0"))).toEqual([]) + }) +}) + +describe("rerunRootTestRun", () => { + /** A spawnSync result: an exit status, what the command printed, and the + * error spawnSync reports when the command could not start. */ + const spawnResult = ({ + status, + stdout = "", + error, + }: { + status: number | null + stdout?: string + error?: Error + }): SpawnSyncReturns => { + return { + pid: 0, + output: [], + stdout, + stderr: "", + status, + signal: null, + ...(error ? { error } : {}), + } + } + + /** Runs the guard as uid `uid` with spawnSync answering each command the way + * `answerCommand` says. process.exit throws instead of ending the worker. */ + const runGuard = ({ + uid, + checkoutRoot, + answerCommand, + }: { + uid: number + checkoutRoot: string + answerCommand: ( + command: string, + commandArguments: readonly string[], + ) => SpawnSyncReturns + }) => { + const getuidSpy = vi.spyOn(process, "getuid").mockReturnValue(uid) + const stderrSpy = vi.spyOn(process.stderr, "write").mockReturnValue(true) + const exitSpy = vi.spyOn(process, "exit").mockImplementation((code) => { + throw new Error(`process.exit(${String(code)})`) + }) + const spawnSpy = vi + .mocked(spawnSync) + .mockImplementation((command: string, commandArguments?: readonly string[]) => { + return answerCommand(command, commandArguments ?? []) + }) + onTestFinished(() => { + getuidSpy.mockRestore() + stderrSpy.mockRestore() + exitSpy.mockRestore() + spawnSpy.mockRestore() + }) + + const runOnce = (): void => { + rerunRootTestRun({ checkoutRoot, configPath: join(checkoutRoot, "vitest.config.ts") }) + } + return { runOnce, stderrSpy, spawnSpy } + } + + /** Answers setpriv --version and id for nobody; every other setpriv call + * (the probes) fails, so the guard stops at the setpriv warning. */ + const answerWithFailingProbes = (command: string, commandArguments: readonly string[]) => { + if (command === "id") { + return spawnResult({ + status: 0, + stdout: commandArguments[0] === "-u" ? "65534\n" : "65533\n", + }) + } + if (commandArguments[0] === "--version") return spawnResult({ status: 0 }) + return spawnResult({ status: 1 }) + } + + const WARNING_PREFIX = + "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: " + + it("does nothing when the process is not root", () => { + const { runOnce, stderrSpy, spawnSpy } = runGuard({ + uid: 1000, + checkoutRoot: "/", + answerCommand: answerWithFailingProbes, + }) + + runOnce() + + expect(spawnSpy).not.toHaveBeenCalled() + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it("warns and continues as root when setpriv is not installed", () => { + const { runOnce, stderrSpy } = runGuard({ + uid: 0, + checkoutRoot: "/", + answerCommand: () => + spawnResult({ status: null, error: new Error("spawnSync setpriv ENOENT") }), + }) + + runOnce() + + expect(stderrSpy).toHaveBeenCalledTimes(1) + expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}setpriv is not installed.\n`) + }) + + it("warns and continues as root when the system has no nobody user", () => { + const { runOnce, stderrSpy } = runGuard({ + uid: 0, + checkoutRoot: "/", + answerCommand: (command, commandArguments) => { + if (command === "id") return spawnResult({ status: 1 }) + return answerWithFailingProbes(command, commandArguments) + }, + }) + + runOnce() + + expect(stderrSpy).toHaveBeenCalledTimes(1) + expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}this system has no nobody user.\n`) + }) + + it("targets nobody's uid and gid when root owns the checkout", () => { + // Root owns / on every Linux system. + const { runOnce, stderrSpy, spawnSpy } = runGuard({ + uid: 0, + checkoutRoot: "/", + answerCommand: answerWithFailingProbes, + }) + + runOnce() + + expect(stderrSpy).toHaveBeenCalledTimes(1) + expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}setpriv cannot switch to nobody.\n`) + expect(spawnSpy).toHaveBeenCalledWith( + "setpriv", + ["--reuid=65534", "--regid=65533", "--clear-groups", "true"], + { stdio: "ignore" }, + ) + }) + + it("targets the checkout owner's uid and gid when root does not own the checkout", () => { + // The test runs as a non-root user, so it owns the checkout it creates. + const checkout = createCheckout() + const checkoutStats = statSync(checkout) + const { runOnce, stderrSpy, spawnSpy } = runGuard({ + uid: 0, + checkoutRoot: checkout, + answerCommand: answerWithFailingProbes, + }) + + runOnce() + + expect(stderrSpy).toHaveBeenCalledTimes(1) + expect(stderrSpy).toHaveBeenCalledWith( + `${WARNING_PREFIX}setpriv cannot switch to the checkout's owner.\n`, + ) + expect(spawnSpy).toHaveBeenCalledWith( + "setpriv", + [`--reuid=${checkoutStats.uid}`, `--regid=${checkoutStats.gid}`, "--clear-groups", "true"], + { stdio: "ignore" }, + ) + }) + + it("warns and continues as root when preparing the rerun fails", () => { + const checkout = createCheckout() + // A file where a scratch folder goes makes creating that folder fail. + mkdirSync(join(checkout, "node_modules")) + writeFileSync(join(checkout, "node_modules", ".vite-temp"), "") + const { runOnce, stderrSpy } = runGuard({ + uid: 0, + checkoutRoot: checkout, + answerCommand: () => spawnResult({ status: 0 }), + }) + + runOnce() + + expect(stderrSpy).toHaveBeenCalledTimes(1) + expect(stderrSpy).toHaveBeenCalledWith( + `${WARNING_PREFIX}preparing the rerun failed (EEXIST: file already exists, mkdir '${join(checkout, "node_modules", ".vite-temp")}').\n`, + ) + }) + + it("exits with the rerun's exit status", () => { + const checkout = createCheckout() + const { runOnce } = runGuard({ + uid: 0, + checkoutRoot: checkout, + answerCommand: (_command, commandArguments) => { + const isRerun = commandArguments.includes("/usr/bin/env") + return spawnResult({ status: isRerun ? 3 : 0 }) + }, + }) + + expect(runOnce).toThrow(new Error("process.exit(3)")) + }) + + it("exits with status 1 when a signal ended the rerun", () => { + const checkout = createCheckout() + const { runOnce } = runGuard({ + uid: 0, + checkoutRoot: checkout, + answerCommand: (_command, commandArguments) => { + const isRerun = commandArguments.includes("/usr/bin/env") + return spawnResult({ status: isRerun ? null : 0 }) + }, + }) + + expect(runOnce).toThrow(new Error("process.exit(1)")) + }) }) From 2e0956cb14f0d2e482ff2fab560973dd2d1260fd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 02:23:56 +0000 Subject: [PATCH 10/14] chore(tests): remove the root run's config bundle and rerun only vitest's own command - vite deletes its bundled copy of vitest.config.ts once the import finishes, but the guard exits during that import, so every root run left a bundle in node_modules/.vite-temp. After the rerun, the guard now removes this config's bundles written before it started. - The guard reran whatever script sat in argv[1]. A program that starts vitest through its API with VITEST=true would have had its own script rerun as another user. The guard now reruns only when argv[1] resolves to vitest.mjs, and otherwise warns and continues as root. Ship-Check: bug-check Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- scripts/__tests__/root-test-guard.test.ts | 129 ++++++++++++++++++++++ scripts/root-test-guard.ts | 79 +++++++++++-- 2 files changed, 201 insertions(+), 7 deletions(-) diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts index 874f70076..9e5faa2b4 100644 --- a/scripts/__tests__/root-test-guard.test.ts +++ b/scripts/__tests__/root-test-guard.test.ts @@ -10,6 +10,7 @@ import { rmSync, statSync, symlinkSync, + utimesSync, writeFileSync, } from "node:fs" import { tmpdir } from "node:os" @@ -21,6 +22,7 @@ import { planRootRun, prepareRerun, readNodeMajor, + removeLeftoverConfigBundles, rerunRootTestRun, } from "../root-test-guard.js" @@ -526,6 +528,58 @@ describe("prepareRerun", () => { }) }) +describe("removeLeftoverConfigBundles", () => { + /** Writes a file in `folder` with its modification time set to `mtimeMs`. */ + const writeFileModifiedAt = ({ + folder, + name, + mtimeMs, + }: { + folder: string + name: string + mtimeMs: number + }): void => { + const path = join(folder, name) + + writeFileSync(path, "") + utimesSync(path, mtimeMs / 1000, mtimeMs / 1000) + } + + const CUTOFF_MS = 1_800_000_000_000 + + it("removes this config's bundles written before the cutoff and keeps every other file", () => { + const bundleFolder = createCheckout() + writeFileModifiedAt({ + folder: bundleFolder, + name: "vitest.config.ts.timestamp-1-a.mjs", + mtimeMs: CUTOFF_MS - 60_000, + }) + writeFileModifiedAt({ + folder: bundleFolder, + name: "vitest.config.ts.timestamp-2-b.mjs", + mtimeMs: CUTOFF_MS + 1_000, + }) + writeFileModifiedAt({ + folder: bundleFolder, + name: "vitest.cli-pty.config.ts.timestamp-3-c.mjs", + mtimeMs: CUTOFF_MS - 60_000, + }) + writeFileModifiedAt({ folder: bundleFolder, name: "other.mjs", mtimeMs: CUTOFF_MS - 60_000 }) + + removeLeftoverConfigBundles({ + bundleFolder, + configName: "vitest.config.ts", + writtenBeforeMs: CUTOFF_MS, + }) + + expect(readdirSync(bundleFolder).sort()).toEqual([ + "other.mjs", + "vitest.cli-pty.config.ts.timestamp-3-c.mjs", + "vitest.config.ts.timestamp-2-b.mjs", + ]) + }) +}) + describe("rerunRootTestRun", () => { /** A spawnSync result: an exit status, what the command printed, and the * error spawnSync reports when the command could not start. */ @@ -549,12 +603,21 @@ describe("rerunRootTestRun", () => { } } + /** A file named like vitest's command-line entry, in a temp folder. */ + const createVitestEntry = (): string => { + const entryPath = join(createCheckout(), "vitest.mjs") + + writeFileSync(entryPath, "") + return entryPath + } + /** Runs the guard as uid `uid` with spawnSync answering each command the way * `answerCommand` says. process.exit throws instead of ending the worker. */ const runGuard = ({ uid, checkoutRoot, answerCommand, + entryPath = createVitestEntry(), }: { uid: number checkoutRoot: string @@ -562,7 +625,14 @@ describe("rerunRootTestRun", () => { command: string, commandArguments: readonly string[], ) => SpawnSyncReturns + /** The script in process.argv[1]; vitest's entry unless a test varies it. */ + entryPath?: string }) => { + const originalArgv = process.argv + process.argv = [process.execPath, entryPath, "run"] + onTestFinished(() => { + process.argv = originalArgv + }) const getuidSpy = vi.spyOn(process, "getuid").mockReturnValue(uid) const stderrSpy = vi.spyOn(process.stderr, "write").mockReturnValue(true) const exitSpy = vi.spyOn(process, "exit").mockImplementation((code) => { @@ -615,6 +685,43 @@ describe("rerunRootTestRun", () => { expect(stderrSpy).not.toHaveBeenCalled() }) + it("warns and continues as root when another program started vitest", () => { + const programScript = join(createCheckout(), "worker.js") + writeFileSync(programScript, "") + const { runOnce, stderrSpy, spawnSpy } = runGuard({ + uid: 0, + checkoutRoot: "/", + answerCommand: answerWithFailingProbes, + entryPath: programScript, + }) + + runOnce() + + expect(spawnSpy).not.toHaveBeenCalled() + expect(stderrSpy).toHaveBeenCalledTimes(1) + expect(stderrSpy).toHaveBeenCalledWith( + `${WARNING_PREFIX}vitest was started by another program, not by its own command.\n`, + ) + }) + + it("accepts vitest's entry through node_modules/.bin's symlink", () => { + const binFolder = createCheckout() + const binLink = join(binFolder, "vitest") + symlinkSync(createVitestEntry(), binLink) + const { runOnce, stderrSpy } = runGuard({ + uid: 0, + checkoutRoot: "/", + answerCommand: answerWithFailingProbes, + entryPath: binLink, + }) + + runOnce() + + // Reaching the probes shows the entry check passed. + expect(stderrSpy).toHaveBeenCalledTimes(1) + expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}setpriv cannot switch to nobody.\n`) + }) + it("warns and continues as root when setpriv is not installed", () => { const { runOnce, stderrSpy } = runGuard({ uid: 0, @@ -720,6 +827,28 @@ describe("rerunRootTestRun", () => { expect(runOnce).toThrow(new Error("process.exit(3)")) }) + it("removes the config bundle vite left behind for the root run once the rerun ends", () => { + const checkout = createCheckout() + const bundleFolder = join(checkout, "node_modules", ".vite-temp") + const bundlePath = join(bundleFolder, "vitest.config.ts.timestamp-1-a.mjs") + mkdirSync(bundleFolder, { recursive: true }) + writeFileSync(bundlePath, "") + // vite wrote the root run's bundle before the config, and so the guard, ran. + const writtenSeconds = Date.now() / 1000 - 60 + utimesSync(bundlePath, writtenSeconds, writtenSeconds) + const { runOnce, spawnSpy } = runGuard({ + uid: 0, + checkoutRoot: checkout, + answerCommand: () => spawnResult({ status: 0 }), + }) + + expect(runOnce).toThrow(new Error("process.exit(0)")) + expect(spawnSpy).toHaveBeenCalledWith("setpriv", expect.arrayContaining(["/usr/bin/env"]), { + stdio: "inherit", + }) + expect(existsSync(bundlePath)).toBe(false) + }) + it("exits with status 1 when a signal ended the rerun", () => { const checkout = createCheckout() const { runOnce } = runGuard({ diff --git a/scripts/root-test-guard.ts b/scripts/root-test-guard.ts index be119f2fa..d8d846ebe 100644 --- a/scripts/root-test-guard.ts +++ b/scripts/root-test-guard.ts @@ -21,19 +21,24 @@ import { mkdirSync, readdirSync, readFileSync, + realpathSync, renameSync, rmSync, statSync, } from "node:fs" -import { dirname, join } from "node:path" +import { basename, dirname, join } from "node:path" import { parseCLI } from "vitest/node" /** The rerun's HOME under node_modules, where npm keeps its cache and logs. */ const TEST_HOME_FOLDER = ".test-home" +/** The folder under node_modules where vite writes the bundled config it + * imports. */ +const CONFIG_BUNDLE_FOLDER = ".vite-temp" + /** Folders under node_modules the rerun writes in: vite's config bundle, * vitest's results cache, and the rerun's HOME. */ -const NODE_MODULES_SCRATCH_FOLDERS = [".vite-temp", ".vite/vitest", TEST_HOME_FOLDER] +const NODE_MODULES_SCRATCH_FOLDERS = [CONFIG_BUNDLE_FOLDER, ".vite/vitest", TEST_HOME_FOLDER] /** The only folder inside the checkout that tests write (`snapshot:update` * rewrites it). A test that writes another folder needs it handed over too, @@ -65,6 +70,7 @@ type RerunTarget = { /** Why a rerun can't happen, named in the warning. */ type WarningCause = + | { kind: "not-vitest-command" } | { kind: "no-setpriv" } | { kind: "no-nobody-user" } | { kind: "setpriv-fails"; targetLabel: string } @@ -106,6 +112,8 @@ type GuardPlan = { kind: "warn"; message: string } | RerunPlan const describeWarningCause = (cause: WarningCause): string => { switch (cause.kind) { + case "not-vitest-command": + return "vitest was started by another program, not by its own command" case "no-setpriv": return "setpriv is not installed" case "no-nobody-user": @@ -123,6 +131,10 @@ const describeWarningCause = (cause: WarningCause): string => { } } +const readErrorMessage = (error: unknown): string => { + return error instanceof Error ? error.message : String(error) +} + const formatWarning = (cause: WarningCause): string => { return `vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: ${describeWarningCause(cause)}.` } @@ -392,6 +404,37 @@ export const prepareRerun = ({ if (plan.node.kind === "copy") copyNodeBinary(plan.node) } +/** + * Removes the bundles vite wrote for this config before `writtenBeforeMs`. + * - vite deletes a bundle once importing it finishes, but the guard exits + * while vite is still importing the root run's bundle, so it stays behind. + * - vite imports a bundle milliseconds after writing it, and the rerun runs + * for seconds, so a bundle written before the guard started has been + * imported, by this run or a concurrent one, and is safe to remove. + */ +export const removeLeftoverConfigBundles = ({ + bundleFolder, + configName, + writtenBeforeMs, +}: { + bundleFolder: string + configName: string + writtenBeforeMs: number +}): void => { + const bundlePrefix = `${configName}.timestamp-` + + for (const entry of readdirSync(bundleFolder)) { + if (!entry.startsWith(bundlePrefix)) continue + + const bundlePath = join(bundleFolder, entry) + // vite may delete another run's bundle between the listing and this stat. + const bundleStats = statSync(bundlePath, { throwIfNoEntry: false }) + + if (!bundleStats || bundleStats.mtimeMs >= writtenBeforeMs) continue + rmSync(bundlePath, { force: true }) + } +} + /** Runs a command as the target and reports whether it exited 0. */ const runsAsTarget = ({ target, @@ -488,9 +531,22 @@ export const rerunRootTestRun = ({ // keeps the rerun from rerunning itself. if (process.getuid?.() !== 0) return + const guardStartMs = Date.now() const warn = (cause: WarningCause): void => { process.stderr.write(`${formatWarning(cause)}\n`) } + const [, vitestEntry, ...vitestArguments] = process.argv + // Node puts the script it runs in argv[1]. That is vitest's entry for a + // vitest command, but another program's script when that program starts + // vitest through its API, and rerunning that script is not a test run. + const startedByVitestCommand = + vitestEntry !== undefined && basename(realpathSync(vitestEntry)) === "vitest.mjs" + + if (!startedByVitestCommand) { + warn({ kind: "not-vitest-command" }) + return + } + const targetLookup = findRerunTarget(checkoutRoot) if (targetLookup.kind === "none") { @@ -502,10 +558,6 @@ export const rerunRootTestRun = ({ const extraCaCerts = process.env.NODE_EXTRA_CA_CERTS const nvmrcPath = join(checkoutRoot, ".nvmrc") - const [, vitestEntry, ...vitestArguments] = process.argv - - // Node always puts the script it runs (vitest's entry) in argv[1]. - if (!vitestEntry) throw new Error("vitest's entry file is missing from process.argv") const plan = planRootRun({ checkoutRoot, @@ -537,7 +589,7 @@ export const rerunRootTestRun = ({ changeOwner: ({ path, uid, gid }) => lchownSync(path, uid, gid), }) } catch (error) { - warn({ kind: "prepare-failed", reason: error instanceof Error ? error.message : String(error) }) + warn({ kind: "prepare-failed", reason: readErrorMessage(error) }) return } @@ -549,6 +601,19 @@ export const rerunRootTestRun = ({ warn({ kind: "spawn-failed", reason: rerun.error.message }) return } + + try { + removeLeftoverConfigBundles({ + bundleFolder: join(checkoutRoot, "node_modules", CONFIG_BUNDLE_FOLDER), + configName: basename(configPath), + writtenBeforeMs: guardStartMs, + }) + } catch (error) { + process.stderr.write( + `Removing vite's leftover config bundles failed: ${readErrorMessage(error)}\n`, + ) + } + // status is null when a signal ended the rerun; that run failed too. process.exit(rerun.status ?? 1) } From ad4cccb0128b5058ee3d2673fc8b75d389f36cbf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 03:02:58 +0000 Subject: [PATCH 11/14] docs: put CONTRIBUTING's root test run in a table Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 416bf29b4..0c40d0529 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,13 +51,17 @@ each config combination serves the correct tool surface, that auth rejects invalid tokens, that write operations actually mutate the vault (read-back verified), and that misconfiguration fails fast at boot. -Some tests deny access to a file or folder and expect the access to -fail, which never happens as root. So when you run the tests as root, for -example inside a container, they rerun as the checkout's owner, or as the -`nobody` user when root owns the checkout, and print a line saying so. -When that isn't possible, they print a warning naming the cause and those -tests fail. To fix it, install `setpriv` (part of util-linux) if the -warning says it is missing, or run the tests as a non-root user. +Run as root (inside a container, for example), the tests rerun as another +user, because root ignores the file permissions some tests check: + +| Checkout owned by | Tests rerun as | +| ----------------- | -------------- | +| root | `nobody` | +| another user | that user | + +When no rerun is possible, a warning names the cause and the permission +tests fail. Install `setpriv` (part of util-linux) if the warning says it +is missing; otherwise run the tests as a non-root user. `npm run test:coverage` runs the same suite with a statement-coverage report (V8 provider); the `coverage/` output directory is gitignored. From 73d2bc05a5fcbd37a1d1c1bc6f98d411b236d9de Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 03:18:47 +0000 Subject: [PATCH 12/14] chore(tests): say which tools skip the root test guard Co-Authored-By: Claude Opus 5.5 --- vitest.config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vitest.config.ts b/vitest.config.ts index 3492e6667..288af1510 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,8 +1,8 @@ import { defineConfig } from "vitest/config" import { rerunRootTestRun } from "./scripts/root-test-guard.js" -// vitest sets VITEST before it loads this file. Other tools that load it, -// such as knip, don't, so the guard runs only for a real vitest run. The +// vitest sets VITEST before it loads this file. Tools that load it without +// running tests, such as knip, don't, so they never reach the guard. The // rerun loads this file again, but not as root, so the guard returns at once. if (process.env.VITEST === "true") { rerunRootTestRun({ checkoutRoot: import.meta.dirname, configPath: import.meta.filename }) From f99891de73326cd108594d5079ce1677eaa4c880 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:05:21 +0000 Subject: [PATCH 13/14] chore(tests): skip the permission tests as root instead of rerunning the suite The root guard reran every root test run as another user, which took about 600 lines and depended on vitest's and vite's internals. Root only breaks the 25 tests that deny file access and expect it to fail, so those tests now skip as root through one shared runsAsRoot flag, and CI still runs them as a normal user. To run them from a root shell, npm run test:as-nobody runs vitest as nobody: it hands nobody the three folders vitest writes in, copies root's Node where nobody can run it, and drops the CA setting nobody can't read. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 15 +- CONTRIBUTING.md | 15 +- package.json | 1 + scripts/__tests__/root-test-guard.test.ts | 865 ------------------ scripts/root-test-guard.ts | 619 ------------- scripts/test-as-nobody.ts | 50 + .../server-error-contracts.test.ts | 108 ++- src/__tests__/runs-as-root.ts | 3 + src/utils/__tests__/fs.test.ts | 33 +- .../__tests__/memory-store.test.ts | 156 ++-- .../__tests__/trash-sweeper.test.ts | 207 +++-- vitest.config.ts | 8 - 12 files changed, 343 insertions(+), 1737 deletions(-) delete mode 100644 scripts/__tests__/root-test-guard.test.ts delete mode 100644 scripts/root-test-guard.ts create mode 100644 scripts/test-as-nobody.ts create mode 100644 src/__tests__/runs-as-root.ts diff --git a/AGENTS.md b/AGENTS.md index 1012b60d7..418df0793 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,7 +71,6 @@ assets/ # Static assets (not shipped in Docker) scripts/ # Dev and ops helpers in TypeScript (not shipped in Docker); most open with a header comment saying what they do deployment-env.ts # Loads ~/.config/vault-cortex/.env; shell variables override its values instance-env.ts # Derives PUBLIC_URL for manual deploys (npm run lightsail:up) the same way sst.config.ts and deploy.yml do, or the Lambda authorizer rejects tokens - root-test-guard.ts # Loaded by vitest.config.ts while vitest starts: reruns a root test run as nobody, or as the checkout's owner when root doesn't own it, and warns when it can't tool-surface-capture.ts # Boots one server per settings combination for the tool-surface snapshot test, size report, and LobeHub manifest; an on/off setting that gates tools goes in its SURFACE_AXES cli/src/ # npx vault-cortex CLI, run on the host: bin (entry) → main (wiring) → program (Commander), one module per command (init, configure, upgrade, get-sync-token; lifecycle holds start, restart, logs, and down), and the shared modules they use (prompts, docker, env, scaffold, and others) __tests__/integration/ # Interactive flows through node-pty in a real PTY: pty-harness, the cli-pty tests, and a fake docker binary in fixtures/ @@ -858,14 +857,12 @@ Two naming layers — MCP (JSON wire format) and TypeScript (internal): ### Test conventions -- A root test run reruns itself as another user, because root ignores - file permissions: the tests that deny access with `chmod` fail as root - and pass in CI. `vitest.config.ts` calls `scripts/root-test-guard.ts`, - which reruns the same command as `nobody`, or as the checkout's owner - when root doesn't own the checkout. Run tests as usual; NEVER add a - `setpriv` prefix yourself. When no rerun is possible, the guard prints - a warning naming the cause and the run continues as root, so the - permission tests fail; fix the cause or run as a non-root user. +- A test that denies file access with `chmod` and expects the access to + fail can't pass as root, because root ignores file permissions. Mark it + `it.skipIf(runsAsRoot)`, importing `runsAsRoot` from + `src/__tests__/runs-as-root.ts`. CI runs it as a normal user. To run + these tests from a root shell, such as a cloud session, use + `npm run test:as-nobody`; it takes the same arguments as `vitest run`. - Tests read as a behavioral spec. One focused `it()` per behavior — a failing test name should identify which behavior regressed without reading the body. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0c40d0529..2ec2b8ed0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,17 +51,10 @@ each config combination serves the correct tool surface, that auth rejects invalid tokens, that write operations actually mutate the vault (read-back verified), and that misconfiguration fails fast at boot. -Run as root (inside a container, for example), the tests rerun as another -user, because root ignores the file permissions some tests check: - -| Checkout owned by | Tests rerun as | -| ----------------- | -------------- | -| root | `nobody` | -| another user | that user | - -When no rerun is possible, a warning names the cause and the permission -tests fail. Install `setpriv` (part of util-linux) if the warning says it -is missing; otherwise run the tests as a non-root user. +Some tests deny access to a file and expect the access to fail. Root +ignores file permissions, so those tests skip when you run the suite as +root (inside a container, for example). To run them from a root shell, +use `npm run test:as-nobody`, which runs the suite as the `nobody` user. `npm run test:coverage` runs the same suite with a statement-coverage report (V8 provider); the `coverage/` output directory is gitignored. diff --git a/package.json b/package.json index 129eec77f..6a162b823 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,7 @@ "test": "vitest run", "snapshot:update": "vitest run src/vault-mcp/mcp-core/__tests__/tool-surface-snapshot.test.ts --update", "test:coverage": "vitest run --coverage", + "test:as-nobody": "tsx scripts/test-as-nobody.ts", "test:watch": "vitest", "test:cli-pty": "vitest run --config vitest.cli-pty.config.ts", "build:remote-image": "docker build --target remote -t vault-cortex:remote-ci .", diff --git a/scripts/__tests__/root-test-guard.test.ts b/scripts/__tests__/root-test-guard.test.ts deleted file mode 100644 index 9e5faa2b4..000000000 --- a/scripts/__tests__/root-test-guard.test.ts +++ /dev/null @@ -1,865 +0,0 @@ -import { type SpawnSyncReturns, spawnSync } from "node:child_process" -import { - chmodSync, - existsSync, - lstatSync, - mkdirSync, - mkdtempSync, - readdirSync, - readFileSync, - rmSync, - statSync, - symlinkSync, - utimesSync, - writeFileSync, -} from "node:fs" -import { tmpdir } from "node:os" -import { join } from "node:path" -import { describe, expect, it, onTestFinished, vi } from "vitest" -import { - addCoverageFlags, - isCoverageRun, - planRootRun, - prepareRerun, - readNodeMajor, - removeLeftoverConfigBundles, - rerunRootTestRun, -} from "../root-test-guard.js" - -vi.mock("node:child_process", { spy: true }) -vi.mock("node:fs", { spy: true }) - -const realFs = await vi.importActual("node:fs") - -const CHECKOUT = "/repo" -const ROOT_NODE = "/root/.nvm/versions/node/v24.21.0/bin/node" -const VITEST_ENTRY = "/repo/node_modules/.bin/vitest" -const NOBODY = { kind: "nobody", label: "nobody", uid: 65534, gid: 65534 } as const -const OWNER = { kind: "owner", label: "the checkout's owner", uid: 1000, gid: 1000 } as const -const ALL_PROBES_PASS = { - setprivWorks: true, - canReadConfig: true, - canRunNode: true, - canReadExtraCaCerts: true, -} - -/** planRootRun's inputs for a plain `vitest run` as nobody, with every probe - * passing; each test overrides what it varies. */ -const rootRun = (overrides: Partial[0]> = {}) => { - return planRootRun({ - checkoutRoot: CHECKOUT, - target: NOBODY, - probes: ALL_PROBES_PASS, - execPath: ROOT_NODE, - nodeVersion: "v24.21.0", - nvmrcText: "24\n", - execArguments: [], - vitestEntry: VITEST_ENTRY, - vitestArguments: ["run"], - extraCaCertsSet: false, - pathEnvironment: "/usr/bin:/bin", - ...overrides, - }) -} - -/** rootRun's plan, narrowed to a rerun; any other plan fails the test. */ -const rootRerun = (overrides: Partial[0]> = {}) => { - const plan = rootRun(overrides) - - if (plan.kind !== "rerun") throw new Error(`expected a rerun plan, got ${plan.message}`) - return plan -} - -/** A changeOwner stub typed as prepareRerun's, so its calls read back typed. */ -const createChangeOwner = () => vi.fn[0]["changeOwner"]>() - -const FOLDERS_TO_HAND_OVER = [ - "/repo/node_modules/.vite-temp", - "/repo/node_modules/.vite/vitest", - "/repo/node_modules/.test-home", - "/repo/src/vault-mcp/mcp-core/__tests__/__snapshots__", -] - -const createCheckout = (): string => { - const checkout = mkdtempSync(join(tmpdir(), "vault-cortex-root-test-guard-")) - - onTestFinished(() => rmSync(checkout, { recursive: true, force: true })) - return checkout -} - -describe("readNodeMajor", () => { - it.each([ - { label: "a bare major", nvmrcText: "24\n", major: 24 }, - { label: "a full version with v", nvmrcText: "v24.1.0\n", major: 24 }, - { label: "a leading space", nvmrcText: " 24\n", major: 24 }, - { label: "a trailing comment", nvmrcText: "24 # CI runs this\n", major: 24 }, - { label: "a CRLF line end", nvmrcText: "24\r\n", major: 24 }, - { label: "an alias", nvmrcText: "lts/*\n", major: null }, - { label: "an empty file", nvmrcText: "", major: null }, - ])("reads $label", ({ nvmrcText, major }) => { - expect(readNodeMajor(nvmrcText)).toBe(major) - }) -}) - -describe("addCoverageFlags", () => { - it("appends the flags when the arguments hold no lone --", () => { - expect(addCoverageFlags(["run", "--coverage"])).toEqual([ - "run", - "--coverage", - "--coverage.clean=false", - "--coverage.cleanOnRerun=false", - ]) - }) - - it("puts the flags before a lone --, after which vitest ignores flags", () => { - expect(addCoverageFlags(["run", "--coverage", "--", "--other"])).toEqual([ - "run", - "--coverage", - "--coverage.clean=false", - "--coverage.cleanOnRerun=false", - "--", - "--other", - ]) - }) -}) - -describe("isCoverageRun", () => { - it.each([ - { label: "--coverage", vitestArguments: ["run", "--coverage"], coverageRun: true }, - { - label: "--coverage.enabled", - vitestArguments: ["run", "--coverage.enabled"], - coverageRun: true, - }, - { label: "--coverage=true", vitestArguments: ["run", "--coverage=true"], coverageRun: true }, - { label: "no coverage flag", vitestArguments: ["run"], coverageRun: false }, - { - label: "--coverage.enabled=false", - vitestArguments: ["run", "--coverage.enabled=false"], - coverageRun: false, - }, - ])("reads $label the way vitest does", ({ vitestArguments, coverageRun }) => { - expect(isCoverageRun(vitestArguments)).toBe(coverageRun) - }) -}) - -describe("planRootRun", () => { - it("reruns as nobody with a copy of a Node binary nobody cannot run", () => { - const plan = rootRun({ probes: { ...ALL_PROBES_PASS, canRunNode: false } }) - - expect(plan).toEqual({ - kind: "rerun", - target: NOBODY, - foldersToHandOver: FOLDERS_TO_HAND_OVER, - coverageFolderToEmpty: null, - node: { - kind: "copy", - sourcePath: ROOT_NODE, - nodePath: "/repo/node_modules/.nobody-node/node-v24.21.0/node", - }, - setprivArguments: [ - "--reuid=65534", - "--regid=65534", - "--clear-groups", - "/usr/bin/env", - "HOME=/repo/node_modules/.test-home", - "PATH=/repo/node_modules/.nobody-node/node-v24.21.0:/usr/bin:/bin", - "/repo/node_modules/.nobody-node/node-v24.21.0/node", - VITEST_ENTRY, - "run", - ], - announcement: - "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v24.21.0.\n" + - ' setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env HOME=/repo/node_modules/.test-home PATH=/repo/node_modules/.nobody-node/node-v24.21.0:"$PATH" /repo/node_modules/.nobody-node/node-v24.21.0/node /repo/node_modules/.bin/vitest run', - }) - }) - - it("reruns as the checkout's owner on the Node binary as it is, when the owner can run it", () => { - const plan = rootRun({ target: OWNER, execPath: "/usr/local/bin/node" }) - - expect(plan).toEqual({ - kind: "rerun", - target: OWNER, - foldersToHandOver: FOLDERS_TO_HAND_OVER, - coverageFolderToEmpty: null, - node: { kind: "as-is", nodePath: "/usr/local/bin/node" }, - setprivArguments: [ - "--reuid=1000", - "--regid=1000", - "--clear-groups", - "/usr/bin/env", - "HOME=/repo/node_modules/.test-home", - "PATH=/usr/local/bin:/usr/bin:/bin", - "/usr/local/bin/node", - VITEST_ENTRY, - "run", - ], - announcement: - "Root ignores the file permissions some tests check, so the tests rerun as the checkout's owner (uid 1000) on Node v24.21.0.\n" + - ' setpriv --reuid=1000 --regid=1000 --clear-groups /usr/bin/env HOME=/repo/node_modules/.test-home PATH=/usr/local/bin:"$PATH" /usr/local/bin/node /repo/node_modules/.bin/vitest run', - }) - }) - - it("warns instead of copying a Node binary into an owner's checkout", () => { - const plan = rootRun({ target: OWNER, probes: { ...ALL_PROBES_PASS, canRunNode: false } }) - - expect(plan).toEqual({ - kind: "warn", - message: - "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: the checkout's owner cannot run this Node binary.", - }) - }) - - it("warns when setpriv cannot switch to the target", () => { - const plan = rootRun({ probes: { ...ALL_PROBES_PASS, setprivWorks: false } }) - - expect(plan).toEqual({ - kind: "warn", - message: - "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: setpriv cannot switch to nobody.", - }) - }) - - it("warns when the target cannot read the checkout", () => { - const plan = rootRun({ probes: { ...ALL_PROBES_PASS, canReadConfig: false } }) - - expect(plan).toEqual({ - kind: "warn", - message: - "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: nobody cannot read the checkout.", - }) - }) - - it("unsets NODE_EXTRA_CA_CERTS when the target cannot read its file", () => { - const plan = rootRerun({ - extraCaCertsSet: true, - probes: { ...ALL_PROBES_PASS, canReadExtraCaCerts: false }, - }) - - expect(plan.setprivArguments).toEqual([ - "--reuid=65534", - "--regid=65534", - "--clear-groups", - "/usr/bin/env", - "-u", - "NODE_EXTRA_CA_CERTS", - "HOME=/repo/node_modules/.test-home", - "PATH=/root/.nvm/versions/node/v24.21.0/bin:/usr/bin:/bin", - ROOT_NODE, - VITEST_ENTRY, - "run", - ]) - }) - - it("keeps NODE_EXTRA_CA_CERTS when the target can read its file", () => { - const plan = rootRerun({ extraCaCertsSet: true }) - - expect(plan.setprivArguments).toEqual([ - "--reuid=65534", - "--regid=65534", - "--clear-groups", - "/usr/bin/env", - "HOME=/repo/node_modules/.test-home", - "PATH=/root/.nvm/versions/node/v24.21.0/bin:/usr/bin:/bin", - ROOT_NODE, - VITEST_ENTRY, - "run", - ]) - }) - - it("empties coverage/ and tells vitest not to clean it on a coverage run", () => { - const plan = rootRerun({ vitestArguments: ["run", "--coverage"] }) - - expect({ - foldersToHandOver: plan.foldersToHandOver, - coverageFolderToEmpty: plan.coverageFolderToEmpty, - rerunArguments: plan.setprivArguments.slice(-4), - }).toEqual({ - foldersToHandOver: [...FOLDERS_TO_HAND_OVER, "/repo/coverage"], - coverageFolderToEmpty: "/repo/coverage", - rerunArguments: [ - "run", - "--coverage", - "--coverage.clean=false", - "--coverage.cleanOnRerun=false", - ], - }) - }) - - it("passes Node's own flags through ahead of vitest's entry", () => { - const plan = rootRerun({ execArguments: ["--inspect-brk"] }) - - expect(plan.setprivArguments.slice(-4)).toEqual([ - ROOT_NODE, - "--inspect-brk", - VITEST_ENTRY, - "run", - ]) - }) - - it("notes a Node major that differs from the one .nvmrc names", () => { - const plan = rootRerun({ nodeVersion: "v22.22.0" }) - - expect(plan.announcement.split("\n")[0]).toBe( - "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v22.22.0. .nvmrc names Node 24, which CI runs, so a result can differ from CI's.", - ) - }) - - it("adds no version note when .nvmrc names an alias", () => { - const plan = rootRerun({ nodeVersion: "v22.22.0", nvmrcText: "lts/*\n" }) - - expect(plan.announcement.split("\n")[0]).toBe( - "Root ignores the file permissions some tests check, so the tests rerun as nobody (uid 65534) on Node v22.22.0.", - ) - }) - - it("quotes a path with a space in the printed command", () => { - const plan = rootRerun({ - checkoutRoot: "/my repo", - vitestEntry: "/my repo/node_modules/.bin/vitest", - }) - - expect(plan.announcement.split("\n")[1]).toBe( - ` setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env 'HOME=/my repo/node_modules/.test-home' PATH=/root/.nvm/versions/node/v24.21.0/bin:"$PATH" /root/.nvm/versions/node/v24.21.0/bin/node '/my repo/node_modules/.bin/vitest' run`, - ) - }) - - it("escapes a single quote in a quoted path in the printed command", () => { - const plan = rootRerun({ - checkoutRoot: "/dev's repo", - vitestEntry: "/dev's repo/node_modules/.bin/vitest", - }) - - expect(plan.announcement.split("\n")[1]).toBe( - ` setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env 'HOME=/dev'\\''s repo/node_modules/.test-home' PATH=/root/.nvm/versions/node/v24.21.0/bin:"$PATH" /root/.nvm/versions/node/v24.21.0/bin/node '/dev'\\''s repo/node_modules/.bin/vitest' run`, - ) - }) -}) - -describe("prepareRerun", () => { - const nobodyPlanFor = ( - checkout: string, - overrides: Partial[0]> = {}, - ) => { - return rootRerun({ checkoutRoot: checkout, ...overrides }) - } - - it("creates every folder to hand over and hands it, and everything in it, to the target", () => { - const checkout = createCheckout() - const cacheFile = join(checkout, "node_modules", ".vite", "vitest", "results.json") - mkdirSync(join(checkout, "node_modules", ".vite", "vitest"), { recursive: true }) - writeFileSync(cacheFile, "{}\n") - // chmod, because the umask cuts the mode writeFileSync is given. - chmodSync(cacheFile, 0o666) - const changeOwner = createChangeOwner() - - prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner }) - - const handedOver = changeOwner.mock.calls.map(([{ path }]) => path) - expect({ - foldersExist: [ - "node_modules/.vite-temp", - "node_modules/.vite/vitest", - "node_modules/.test-home", - "src/vault-mcp/mcp-core/__tests__/__snapshots__", - ].map((folder) => existsSync(join(checkout, folder))), - // A run without coverage leaves coverage/ alone. - coverageFolderExists: existsSync(join(checkout, "coverage")), - handedOver: handedOver.toSorted(), - cacheFileMode: lstatSync(cacheFile).mode & 0o777, - }).toEqual({ - foldersExist: [true, true, true, true], - coverageFolderExists: false, - handedOver: [ - join(checkout, "node_modules", ".test-home"), - join(checkout, "node_modules", ".vite-temp"), - join(checkout, "node_modules", ".vite", "vitest"), - cacheFile, - join(checkout, "src/vault-mcp/mcp-core/__tests__/__snapshots__"), - ].toSorted(), - cacheFileMode: 0o644, - }) - expect(changeOwner).toHaveBeenCalledWith({ path: cacheFile, uid: 65534, gid: 65534 }) - }) - - it("hands over a symlink without changing the mode of what it points to", () => { - const checkout = createCheckout() - const symlinkTarget = join(checkout, "symlink-target.txt") - writeFileSync(symlinkTarget, "keep\n") - chmodSync(symlinkTarget, 0o666) - mkdirSync(join(checkout, "node_modules", ".vite", "vitest"), { recursive: true }) - const link = join(checkout, "node_modules", ".vite", "vitest", "link") - symlinkSync(symlinkTarget, link) - const changeOwner = createChangeOwner() - - prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner }) - - expect(lstatSync(symlinkTarget).mode & 0o777).toBe(0o666) - expect(changeOwner).toHaveBeenCalledWith({ path: link, uid: 65534, gid: 65534 }) - }) - - it("empties coverage/ on a coverage run but keeps the folder", () => { - const checkout = createCheckout() - mkdirSync(join(checkout, "coverage", "html"), { recursive: true }) - writeFileSync(join(checkout, "coverage", "coverage-final.json"), "{}\n") - - prepareRerun({ - plan: nobodyPlanFor(checkout, { vitestArguments: ["run", "--coverage"] }), - changeOwner: vi.fn(), - }) - - expect(readdirSync(join(checkout, "coverage"))).toEqual([]) - }) - - it("creates coverage/ and hands it over on a coverage run in a checkout without one", () => { - const checkout = createCheckout() - const changeOwner = createChangeOwner() - - prepareRerun({ - plan: nobodyPlanFor(checkout, { vitestArguments: ["run", "--coverage"] }), - changeOwner, - }) - - expect(readdirSync(join(checkout, "coverage"))).toEqual([]) - expect(changeOwner).toHaveBeenCalledWith({ - path: join(checkout, "coverage"), - uid: 65534, - gid: 65534, - }) - }) - - it("leaves coverage/ alone on a run without coverage", () => { - const checkout = createCheckout() - mkdirSync(join(checkout, "coverage"), { recursive: true }) - writeFileSync(join(checkout, "coverage", "coverage-final.json"), "{}\n") - - prepareRerun({ plan: nobodyPlanFor(checkout), changeOwner: vi.fn() }) - - expect({ - coverageEntries: readdirSync(join(checkout, "coverage")), - // Proves prepareRerun ran: it creates the rerun's HOME. - testHomeExists: existsSync(join(checkout, "node_modules", ".test-home")), - }).toEqual({ coverageEntries: ["coverage-final.json"], testHomeExists: true }) - }) - - it("copies the Node binary for nobody, mode 755, when the plan calls for a copy", () => { - const checkout = createCheckout() - const sourceNode = join(checkout, "source-node") - writeFileSync(sourceNode, "#!/bin/sh\n# node v24.21.0\n", { mode: 0o700 }) - const plan = nobodyPlanFor(checkout, { - execPath: sourceNode, - probes: { ...ALL_PROBES_PASS, canRunNode: false }, - }) - - prepareRerun({ plan, changeOwner: vi.fn() }) - - const nodeCopy = join(checkout, "node_modules", ".nobody-node", "node-v24.21.0", "node") - expect({ - content: readFileSync(nodeCopy, "utf8"), - mode: lstatSync(nodeCopy).mode & 0o777, - folderEntries: readdirSync(join(checkout, "node_modules", ".nobody-node", "node-v24.21.0")), - }).toEqual({ content: "#!/bin/sh\n# node v24.21.0\n", mode: 0o755, folderEntries: ["node"] }) - }) - - it("reuses an existing copy instead of copying again", () => { - const checkout = createCheckout() - const sourceNode = join(checkout, "source-node") - writeFileSync(sourceNode, "new\n", { mode: 0o755 }) - const nodeCopy = join(checkout, "node_modules", ".nobody-node", "node-v24.21.0", "node") - mkdirSync(join(checkout, "node_modules", ".nobody-node", "node-v24.21.0"), { recursive: true }) - writeFileSync(nodeCopy, "existing\n", { mode: 0o755 }) - const plan = nobodyPlanFor(checkout, { - execPath: sourceNode, - probes: { ...ALL_PROBES_PASS, canRunNode: false }, - }) - - prepareRerun({ plan, changeOwner: vi.fn() }) - - expect({ - nodeCopyContent: readFileSync(nodeCopy, "utf8"), - // Proves prepareRerun ran: it creates the rerun's HOME. - testHomeExists: existsSync(join(checkout, "node_modules", ".test-home")), - }).toEqual({ nodeCopyContent: "existing\n", testHomeExists: true }) - }) - - it("never hands the Node copy's folder to the target", () => { - const checkout = createCheckout() - const sourceNode = join(checkout, "source-node") - writeFileSync(sourceNode, "node\n", { mode: 0o755 }) - const changeOwner = createChangeOwner() - const plan = nobodyPlanFor(checkout, { - execPath: sourceNode, - probes: { ...ALL_PROBES_PASS, canRunNode: false }, - }) - - prepareRerun({ plan, changeOwner }) - - const handedOver = changeOwner.mock.calls.map(([{ path }]) => path) - const nodeCopy = join(checkout, "node_modules", ".nobody-node", "node-v24.21.0", "node") - expect({ handedOver: handedOver.toSorted(), nodeCopyExists: existsSync(nodeCopy) }).toEqual({ - handedOver: [ - join(checkout, "node_modules", ".test-home"), - join(checkout, "node_modules", ".vite-temp"), - join(checkout, "node_modules", ".vite", "vitest"), - join(checkout, "src/vault-mcp/mcp-core/__tests__/__snapshots__"), - ].toSorted(), - nodeCopyExists: true, - }) - }) - - it("leaves no Node copy behind when the copy fails part way", () => { - const checkout = createCheckout() - const sourceNode = join(checkout, "source-node") - writeFileSync(sourceNode, "node\n", { mode: 0o755 }) - const copyFailure = new Error("ENOSPC: no space left on device") - const chmodSpy = vi.mocked(chmodSync).mockImplementation((path, mode) => { - // Fails the chmod on the temp copy, after its bytes are written. - if (String(path).endsWith(`node.${process.pid}`)) throw copyFailure - realFs.chmodSync(path, mode) - }) - onTestFinished(() => chmodSpy.mockRestore()) - const plan = nobodyPlanFor(checkout, { - execPath: sourceNode, - probes: { ...ALL_PROBES_PASS, canRunNode: false }, - }) - - expect(() => prepareRerun({ plan, changeOwner: vi.fn() })).toThrow(copyFailure) - expect(readdirSync(join(checkout, "node_modules", ".nobody-node", "node-v24.21.0"))).toEqual([]) - }) -}) - -describe("removeLeftoverConfigBundles", () => { - /** Writes a file in `folder` with its modification time set to `mtimeMs`. */ - const writeFileModifiedAt = ({ - folder, - name, - mtimeMs, - }: { - folder: string - name: string - mtimeMs: number - }): void => { - const path = join(folder, name) - - writeFileSync(path, "") - utimesSync(path, mtimeMs / 1000, mtimeMs / 1000) - } - - const CUTOFF_MS = 1_800_000_000_000 - - it("removes this config's bundles written before the cutoff and keeps every other file", () => { - const bundleFolder = createCheckout() - writeFileModifiedAt({ - folder: bundleFolder, - name: "vitest.config.ts.timestamp-1-a.mjs", - mtimeMs: CUTOFF_MS - 60_000, - }) - writeFileModifiedAt({ - folder: bundleFolder, - name: "vitest.config.ts.timestamp-2-b.mjs", - mtimeMs: CUTOFF_MS + 1_000, - }) - writeFileModifiedAt({ - folder: bundleFolder, - name: "vitest.cli-pty.config.ts.timestamp-3-c.mjs", - mtimeMs: CUTOFF_MS - 60_000, - }) - writeFileModifiedAt({ folder: bundleFolder, name: "other.mjs", mtimeMs: CUTOFF_MS - 60_000 }) - - removeLeftoverConfigBundles({ - bundleFolder, - configName: "vitest.config.ts", - writtenBeforeMs: CUTOFF_MS, - }) - - expect(readdirSync(bundleFolder).sort()).toEqual([ - "other.mjs", - "vitest.cli-pty.config.ts.timestamp-3-c.mjs", - "vitest.config.ts.timestamp-2-b.mjs", - ]) - }) -}) - -describe("rerunRootTestRun", () => { - /** A spawnSync result: an exit status, what the command printed, and the - * error spawnSync reports when the command could not start. */ - const spawnResult = ({ - status, - stdout = "", - error, - }: { - status: number | null - stdout?: string - error?: Error - }): SpawnSyncReturns => { - return { - pid: 0, - output: [], - stdout, - stderr: "", - status, - signal: null, - ...(error ? { error } : {}), - } - } - - /** A file named like vitest's command-line entry, in a temp folder. */ - const createVitestEntry = (): string => { - const entryPath = join(createCheckout(), "vitest.mjs") - - writeFileSync(entryPath, "") - return entryPath - } - - /** Runs the guard as uid `uid` with spawnSync answering each command the way - * `answerCommand` says. process.exit throws instead of ending the worker. */ - const runGuard = ({ - uid, - checkoutRoot, - answerCommand, - entryPath = createVitestEntry(), - }: { - uid: number - checkoutRoot: string - answerCommand: ( - command: string, - commandArguments: readonly string[], - ) => SpawnSyncReturns - /** The script in process.argv[1]; vitest's entry unless a test varies it. */ - entryPath?: string - }) => { - const originalArgv = process.argv - process.argv = [process.execPath, entryPath, "run"] - onTestFinished(() => { - process.argv = originalArgv - }) - const getuidSpy = vi.spyOn(process, "getuid").mockReturnValue(uid) - const stderrSpy = vi.spyOn(process.stderr, "write").mockReturnValue(true) - const exitSpy = vi.spyOn(process, "exit").mockImplementation((code) => { - throw new Error(`process.exit(${String(code)})`) - }) - const spawnSpy = vi - .mocked(spawnSync) - .mockImplementation((command: string, commandArguments?: readonly string[]) => { - return answerCommand(command, commandArguments ?? []) - }) - onTestFinished(() => { - getuidSpy.mockRestore() - stderrSpy.mockRestore() - exitSpy.mockRestore() - spawnSpy.mockRestore() - }) - - const runOnce = (): void => { - rerunRootTestRun({ checkoutRoot, configPath: join(checkoutRoot, "vitest.config.ts") }) - } - return { runOnce, stderrSpy, spawnSpy } - } - - /** Answers setpriv --version and id for nobody; every other setpriv call - * (the probes) fails, so the guard stops at the setpriv warning. */ - const answerWithFailingProbes = (command: string, commandArguments: readonly string[]) => { - if (command === "id") { - return spawnResult({ - status: 0, - stdout: commandArguments[0] === "-u" ? "65534\n" : "65533\n", - }) - } - if (commandArguments[0] === "--version") return spawnResult({ status: 0 }) - return spawnResult({ status: 1 }) - } - - const WARNING_PREFIX = - "vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: " - - it("does nothing when the process is not root", () => { - const { runOnce, stderrSpy, spawnSpy } = runGuard({ - uid: 1000, - checkoutRoot: "/", - answerCommand: answerWithFailingProbes, - }) - - runOnce() - - expect(spawnSpy).not.toHaveBeenCalled() - expect(stderrSpy).not.toHaveBeenCalled() - }) - - it("warns and continues as root when another program started vitest", () => { - const programScript = join(createCheckout(), "worker.js") - writeFileSync(programScript, "") - const { runOnce, stderrSpy, spawnSpy } = runGuard({ - uid: 0, - checkoutRoot: "/", - answerCommand: answerWithFailingProbes, - entryPath: programScript, - }) - - runOnce() - - expect(spawnSpy).not.toHaveBeenCalled() - expect(stderrSpy).toHaveBeenCalledTimes(1) - expect(stderrSpy).toHaveBeenCalledWith( - `${WARNING_PREFIX}vitest was started by another program, not by its own command.\n`, - ) - }) - - it("accepts vitest's entry through node_modules/.bin's symlink", () => { - const binFolder = createCheckout() - const binLink = join(binFolder, "vitest") - symlinkSync(createVitestEntry(), binLink) - const { runOnce, stderrSpy } = runGuard({ - uid: 0, - checkoutRoot: "/", - answerCommand: answerWithFailingProbes, - entryPath: binLink, - }) - - runOnce() - - // Reaching the probes shows the entry check passed. - expect(stderrSpy).toHaveBeenCalledTimes(1) - expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}setpriv cannot switch to nobody.\n`) - }) - - it("warns and continues as root when setpriv is not installed", () => { - const { runOnce, stderrSpy } = runGuard({ - uid: 0, - checkoutRoot: "/", - answerCommand: () => - spawnResult({ status: null, error: new Error("spawnSync setpriv ENOENT") }), - }) - - runOnce() - - expect(stderrSpy).toHaveBeenCalledTimes(1) - expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}setpriv is not installed.\n`) - }) - - it("warns and continues as root when the system has no nobody user", () => { - const { runOnce, stderrSpy } = runGuard({ - uid: 0, - checkoutRoot: "/", - answerCommand: (command, commandArguments) => { - if (command === "id") return spawnResult({ status: 1 }) - return answerWithFailingProbes(command, commandArguments) - }, - }) - - runOnce() - - expect(stderrSpy).toHaveBeenCalledTimes(1) - expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}this system has no nobody user.\n`) - }) - - it("targets nobody's uid and gid when root owns the checkout", () => { - // Root owns / on every Linux system. - const { runOnce, stderrSpy, spawnSpy } = runGuard({ - uid: 0, - checkoutRoot: "/", - answerCommand: answerWithFailingProbes, - }) - - runOnce() - - expect(stderrSpy).toHaveBeenCalledTimes(1) - expect(stderrSpy).toHaveBeenCalledWith(`${WARNING_PREFIX}setpriv cannot switch to nobody.\n`) - expect(spawnSpy).toHaveBeenCalledWith( - "setpriv", - ["--reuid=65534", "--regid=65533", "--clear-groups", "true"], - { stdio: "ignore" }, - ) - }) - - it("targets the checkout owner's uid and gid when root does not own the checkout", () => { - // The test runs as a non-root user, so it owns the checkout it creates. - const checkout = createCheckout() - const checkoutStats = statSync(checkout) - const { runOnce, stderrSpy, spawnSpy } = runGuard({ - uid: 0, - checkoutRoot: checkout, - answerCommand: answerWithFailingProbes, - }) - - runOnce() - - expect(stderrSpy).toHaveBeenCalledTimes(1) - expect(stderrSpy).toHaveBeenCalledWith( - `${WARNING_PREFIX}setpriv cannot switch to the checkout's owner.\n`, - ) - expect(spawnSpy).toHaveBeenCalledWith( - "setpriv", - [`--reuid=${checkoutStats.uid}`, `--regid=${checkoutStats.gid}`, "--clear-groups", "true"], - { stdio: "ignore" }, - ) - }) - - it("warns and continues as root when preparing the rerun fails", () => { - const checkout = createCheckout() - // A file where a scratch folder goes makes creating that folder fail. - mkdirSync(join(checkout, "node_modules")) - writeFileSync(join(checkout, "node_modules", ".vite-temp"), "") - const { runOnce, stderrSpy } = runGuard({ - uid: 0, - checkoutRoot: checkout, - answerCommand: () => spawnResult({ status: 0 }), - }) - - runOnce() - - expect(stderrSpy).toHaveBeenCalledTimes(1) - expect(stderrSpy).toHaveBeenCalledWith( - `${WARNING_PREFIX}preparing the rerun failed (EEXIST: file already exists, mkdir '${join(checkout, "node_modules", ".vite-temp")}').\n`, - ) - }) - - it("exits with the rerun's exit status", () => { - const checkout = createCheckout() - const { runOnce } = runGuard({ - uid: 0, - checkoutRoot: checkout, - answerCommand: (_command, commandArguments) => { - const isRerun = commandArguments.includes("/usr/bin/env") - return spawnResult({ status: isRerun ? 3 : 0 }) - }, - }) - - expect(runOnce).toThrow(new Error("process.exit(3)")) - }) - - it("removes the config bundle vite left behind for the root run once the rerun ends", () => { - const checkout = createCheckout() - const bundleFolder = join(checkout, "node_modules", ".vite-temp") - const bundlePath = join(bundleFolder, "vitest.config.ts.timestamp-1-a.mjs") - mkdirSync(bundleFolder, { recursive: true }) - writeFileSync(bundlePath, "") - // vite wrote the root run's bundle before the config, and so the guard, ran. - const writtenSeconds = Date.now() / 1000 - 60 - utimesSync(bundlePath, writtenSeconds, writtenSeconds) - const { runOnce, spawnSpy } = runGuard({ - uid: 0, - checkoutRoot: checkout, - answerCommand: () => spawnResult({ status: 0 }), - }) - - expect(runOnce).toThrow(new Error("process.exit(0)")) - expect(spawnSpy).toHaveBeenCalledWith("setpriv", expect.arrayContaining(["/usr/bin/env"]), { - stdio: "inherit", - }) - expect(existsSync(bundlePath)).toBe(false) - }) - - it("exits with status 1 when a signal ended the rerun", () => { - const checkout = createCheckout() - const { runOnce } = runGuard({ - uid: 0, - checkoutRoot: checkout, - answerCommand: (_command, commandArguments) => { - const isRerun = commandArguments.includes("/usr/bin/env") - return spawnResult({ status: isRerun ? null : 0 }) - }, - }) - - expect(runOnce).toThrow(new Error("process.exit(1)")) - }) -}) diff --git a/scripts/root-test-guard.ts b/scripts/root-test-guard.ts deleted file mode 100644 index d8d846ebe..000000000 --- a/scripts/root-test-guard.ts +++ /dev/null @@ -1,619 +0,0 @@ -/** - * Reruns a root vitest run as a user the permission tests can rely on. - * Root ignores file permissions, so tests that chmod a file or folder to deny - * access, then expect the access to fail, fail as root and pass in CI. - * - vitest.config.ts calls `rerunRootTestRun` while vitest loads it, before - * any test file, the coverage provider or the watcher starts. - * - The rerun runs as nobody when root owns the checkout (a cloud or agent - * container's clone), and as the checkout's owner otherwise (a contributor's - * checkout mounted into a root container). - * - When no rerun is possible, it prints one warning and the run continues as - * root, so a root run is never worse than it was without this guard. - */ - -import { spawnSync } from "node:child_process" -import { - chmodSync, - copyFileSync, - existsSync, - lchownSync, - lstatSync, - mkdirSync, - readdirSync, - readFileSync, - realpathSync, - renameSync, - rmSync, - statSync, -} from "node:fs" -import { basename, dirname, join } from "node:path" -import { parseCLI } from "vitest/node" - -/** The rerun's HOME under node_modules, where npm keeps its cache and logs. */ -const TEST_HOME_FOLDER = ".test-home" - -/** The folder under node_modules where vite writes the bundled config it - * imports. */ -const CONFIG_BUNDLE_FOLDER = ".vite-temp" - -/** Folders under node_modules the rerun writes in: vite's config bundle, - * vitest's results cache, and the rerun's HOME. */ -const NODE_MODULES_SCRATCH_FOLDERS = [CONFIG_BUNDLE_FOLDER, ".vite/vitest", TEST_HOME_FOLDER] - -/** The only folder inside the checkout that tests write (`snapshot:update` - * rewrites it). A test that writes another folder needs it handed over too, - * or the rerun cannot write there. */ -const SNAPSHOT_FOLDER = "src/vault-mcp/mcp-core/__tests__/__snapshots__" - -/** A plain shell word that needs no quoting when the command is printed. */ -const SHELL_SAFE_WORD = /^[\w@%+=:,./-]+$/ - -/** The leading major number of a Node version (`24`, `v24.1.0`), ending at a - * dot, whitespace or the end of the text. */ -const NODE_MAJOR_VERSION = /^v?(\d+)(?:[.\s]|$)/ - -/** Write access for group and other, removed from every handed-over path. */ -const GROUP_AND_OTHER_WRITE = 0o022 - -/** rwxr-xr-x: the Node copy and its folders stay root-owned and unwritable by - * the target, but the target can run the copy. */ -const EXECUTABLE_BY_ALL_MODE = 0o755 - -type RerunTarget = { - /** nobody for a root-owned checkout; the checkout's owner otherwise. */ - kind: "nobody" | "owner" - /** How messages name the target: a user name, or a phrase for an owner. */ - label: string - uid: number - gid: number -} - -/** Why a rerun can't happen, named in the warning. */ -type WarningCause = - | { kind: "not-vitest-command" } - | { kind: "no-setpriv" } - | { kind: "no-nobody-user" } - | { kind: "setpriv-fails"; targetLabel: string } - | { kind: "config-unreadable"; targetLabel: string } - | { kind: "node-unrunnable"; targetLabel: string } - | { kind: "prepare-failed"; reason: string } - | { kind: "spawn-failed"; reason: string } - -/** The user the rerun runs as, or why there is none. */ -type TargetLookup = { kind: "found"; target: RerunTarget } | { kind: "none"; cause: WarningCause } - -/** What the target can do, each probed by running a command as the target. */ -type TargetProbes = { - setprivWorks: boolean - /** The config file stands in for the checkout: vitest must read it first. */ - canReadConfig: boolean - canRunNode: boolean - /** True when NODE_EXTRA_CA_CERTS is unset; probed only when it is set. */ - canReadExtraCaCerts: boolean -} - -type NodeChoice = - { kind: "as-is"; nodePath: string } | { kind: "copy"; sourcePath: string; nodePath: string } - -type RerunPlan = { - kind: "rerun" - target: RerunTarget - /** Handed to the target and left with it after the run: the scratch - * folders under node_modules, the snapshot folder, and coverage/ on a - * coverage run. */ - foldersToHandOver: readonly string[] - coverageFolderToEmpty: string | null - node: NodeChoice - setprivArguments: readonly string[] - announcement: string -} - -type GuardPlan = { kind: "warn"; message: string } | RerunPlan - -const describeWarningCause = (cause: WarningCause): string => { - switch (cause.kind) { - case "not-vitest-command": - return "vitest was started by another program, not by its own command" - case "no-setpriv": - return "setpriv is not installed" - case "no-nobody-user": - return "this system has no nobody user" - case "setpriv-fails": - return `setpriv cannot switch to ${cause.targetLabel}` - case "config-unreadable": - return `${cause.targetLabel} cannot read the checkout` - case "node-unrunnable": - return `${cause.targetLabel} cannot run this Node binary` - case "prepare-failed": - return `preparing the rerun failed (${cause.reason})` - case "spawn-failed": - return `starting the rerun failed (${cause.reason})` - } -} - -const readErrorMessage = (error: unknown): string => { - return error instanceof Error ? error.message : String(error) -} - -const formatWarning = (cause: WarningCause): string => { - return `vitest is running as root, which ignores the file permissions some tests check, so those tests will fail. It cannot rerun them as another user: ${describeWarningCause(cause)}.` -} - -/** The major version a Node version or .nvmrc names, or null for an alias - * such as lts/*. */ -export const readNodeMajor = (versionText: string): number | null => { - const majorText = NODE_MAJOR_VERSION.exec(versionText.trimStart())?.[1] - - if (!majorText) return null - return Number(majorText) -} - -/** setpriv's flags that switch to the target's uid and gid and drop root's - * supplementary groups. */ -const buildSwitchArguments = (target: RerunTarget): string[] => { - return [`--reuid=${target.uid}`, `--regid=${target.gid}`, "--clear-groups"] -} - -/** Quotes a word for a POSIX shell only when it needs it. */ -const quoteForShell = (word: string): string => { - if (SHELL_SAFE_WORD.test(word)) return word - return `'${word.replaceAll("'", `'\\''`)}'` -} - -/** - * Appends the flags a non-root coverage run needs. vitest cleans by deleting - * coverage/ itself, which needs write access to the checkout folder, and root - * owns that folder in a container's clone. So the guard empties coverage/ as - * root and vitest must not clean it. vitest ignores flags after a lone `--`, - * so they go before one. - */ -export const addCoverageFlags = (vitestArguments: readonly string[]): string[] => { - const coverageFlags = ["--coverage.clean=false", "--coverage.cleanOnRerun=false"] - const doubleDashIndex = vitestArguments.indexOf("--") - - if (doubleDashIndex === -1) return [...vitestArguments, ...coverageFlags] - return [ - ...vitestArguments.slice(0, doubleDashIndex), - ...coverageFlags, - ...vitestArguments.slice(doubleDashIndex), - ] -} - -/** Whether the vitest command line turns coverage on, read the way vitest - * reads it. parseCLI wants "vitest" first and rewrites the array it gets. */ -export const isCoverageRun = (vitestArguments: readonly string[]): boolean => { - const { options } = parseCLI(["vitest", ...vitestArguments]) - return options.coverage?.enabled === true -} - -/** The Node binary the rerun starts, or null when the target has none it can - * run. */ -const chooseNode = ({ - canRunNode, - target, - execPath, - nodeCopyPath, -}: { - canRunNode: boolean - target: RerunTarget - execPath: string - nodeCopyPath: string -}): NodeChoice | null => { - if (canRunNode) return { kind: "as-is", nodePath: execPath } - // The target cannot run root's Node when it sits under /root, as nvm's - // does: /root is mode 700, so no other user can enter it. A copy in the - // checkout fixes that, but in an owner's checkout it would leave a large - // root-owned file the owner cannot delete, so an owner gets the warning. - if (target.kind === "owner") return null - // nobody can already read the checkout (the config probe passed), and the - // copy's folders are created mode 755, so nobody can reach and run it. - return { kind: "copy", sourcePath: execPath, nodePath: nodeCopyPath } -} - -/** Plans the guard's response for a root run whose target is known. */ -export const planRootRun = ({ - checkoutRoot, - target, - probes, - execPath, - nodeVersion, - nvmrcText, - execArguments, - vitestEntry, - vitestArguments, - extraCaCertsSet, - pathEnvironment, -}: { - checkoutRoot: string - target: RerunTarget - probes: TargetProbes - execPath: string - nodeVersion: string - nvmrcText: string | null - execArguments: readonly string[] - vitestEntry: string - vitestArguments: readonly string[] - extraCaCertsSet: boolean - pathEnvironment: string -}): GuardPlan => { - if (!probes.setprivWorks) { - return { - kind: "warn", - message: formatWarning({ kind: "setpriv-fails", targetLabel: target.label }), - } - } - if (!probes.canReadConfig) { - return { - kind: "warn", - message: formatWarning({ kind: "config-unreadable", targetLabel: target.label }), - } - } - - const nodeCopyPath = join( - checkoutRoot, - "node_modules", - ".nobody-node", - `node-${nodeVersion}`, - "node", - ) - const node = chooseNode({ canRunNode: probes.canRunNode, target, execPath, nodeCopyPath }) - - if (!node) { - return { - kind: "warn", - message: formatWarning({ kind: "node-unrunnable", targetLabel: target.label }), - } - } - - const nodeModules = join(checkoutRoot, "node_modules") - const coverageFolder = join(checkoutRoot, "coverage") - const coverageRun = isCoverageRun(vitestArguments) - const foldersToHandOver = [ - ...NODE_MODULES_SCRATCH_FOLDERS.map((folderName) => join(nodeModules, folderName)), - join(checkoutRoot, SNAPSHOT_FOLDER), - // Only a coverage run writes in coverage/. - ...(coverageRun ? [coverageFolder] : []), - ] - - const rerunArguments = coverageRun ? addCoverageFlags(vitestArguments) : [...vitestArguments] - // Node warns on stderr at every start when it cannot read this file, and - // tests that assert a child's exact stderr then fail. - const unsetsExtraCaCerts = extraCaCertsSet && !probes.canReadExtraCaCerts - const environmentArguments = [ - ...(unsetsExtraCaCerts ? ["-u", "NODE_EXTRA_CA_CERTS"] : []), - `HOME=${join(nodeModules, TEST_HOME_FOLDER)}`, - ] - const commandAfterPath = [node.nodePath, ...execArguments, vitestEntry, ...rerunArguments] - const switchArguments = buildSwitchArguments(target) - // The Node folder goes first on PATH so that a test starting a bare `node` - // child process gets the same binary as the rerun. - const nodeFolder = dirname(node.nodePath) - const setprivArguments = [ - ...switchArguments, - "/usr/bin/env", - ...environmentArguments, - `PATH=${nodeFolder}:${pathEnvironment}`, - ...commandAfterPath, - ] - - // The guard runs the command itself; it is printed so the reader sees what - // runs as whom. Rerunning it by hand works once the guard has prepared the - // folders, as it has by the time the line prints. $PATH stays literal, so - // the line stays short and still runs as written in a shell. - const printedCommand = [ - "setpriv", - ...switchArguments, - "/usr/bin/env", - ...environmentArguments.map(quoteForShell), - `PATH=${quoteForShell(nodeFolder)}:"$PATH"`, - ...commandAfterPath.map(quoteForShell), - ].join(" ") - - // The rerun uses the Node that started vitest, whatever its major. CI runs - // the .nvmrc major, and a different one can pass or fail tests CI doesn't. - const nvmrcMajor = nvmrcText === null ? null : readNodeMajor(nvmrcText) - const nodeMajor = readNodeMajor(nodeVersion) - const versionNote = - nvmrcMajor !== null && nvmrcMajor !== nodeMajor - ? ` .nvmrc names Node ${nvmrcMajor}, which CI runs, so a result can differ from CI's.` - : "" - - return { - kind: "rerun", - target, - foldersToHandOver, - coverageFolderToEmpty: coverageRun ? coverageFolder : null, - node, - setprivArguments, - announcement: `Root ignores the file permissions some tests check, so the tests rerun as ${target.label} (uid ${target.uid}) on Node ${nodeVersion}.${versionNote}\n ${printedCommand}`, - } -} - -/** Changes the owner of a path without following a symlink. */ -type ChangeOwner = (params: { path: string; uid: number; gid: number }) => void - -/** Hands a folder and everything inside it to the target, and removes group - * and other write access, because vitest runs code from its caches. */ -const handOverFolder = ({ - folder, - target, - changeOwner, -}: { - folder: string - target: RerunTarget - changeOwner: ChangeOwner -}): void => { - const entries = readdirSync(folder, { recursive: true, encoding: "utf8" }) - const paths = [folder, ...entries.map((entry) => join(folder, entry))] - - for (const path of paths) { - changeOwner({ path, uid: target.uid, gid: target.gid }) - const stats = lstatSync(path) - - // chmod would follow a symlink to its target. - if (stats.isSymbolicLink()) continue - chmodSync(path, stats.mode & ~GROUP_AND_OTHER_WRITE) - } -} - -/** Copies the Node binary for nobody, root-owned and readable by all, under a - * temp name first so a half-written copy never sits at the path a later run - * reuses. */ -const copyNodeBinary = ({ - sourcePath, - nodePath, -}: { - sourcePath: string - nodePath: string -}): void => { - if (existsSync(nodePath)) return - - const temporaryPath = `${nodePath}.${process.pid}` - - mkdirSync(dirname(nodePath), { recursive: true, mode: EXECUTABLE_BY_ALL_MODE }) - try { - copyFileSync(sourcePath, temporaryPath) - chmodSync(temporaryPath, EXECUTABLE_BY_ALL_MODE) - renameSync(temporaryPath, nodePath) - } finally { - // Removes a half-written copy when the copy or chmod threw; after a - // successful rename the temp name no longer exists. - rmSync(temporaryPath, { force: true }) - } -} - -/** Prepares the folders and Node binary a rerun plan needs. */ -export const prepareRerun = ({ - plan, - changeOwner, -}: { - plan: RerunPlan - changeOwner: ChangeOwner -}): void => { - if (plan.coverageFolderToEmpty && existsSync(plan.coverageFolderToEmpty)) { - for (const entry of readdirSync(plan.coverageFolderToEmpty)) { - rmSync(join(plan.coverageFolderToEmpty, entry), { recursive: true, force: true }) - } - } - - for (const folder of plan.foldersToHandOver) { - mkdirSync(folder, { recursive: true }) - handOverFolder({ folder, target: plan.target, changeOwner }) - } - - if (plan.node.kind === "copy") copyNodeBinary(plan.node) -} - -/** - * Removes the bundles vite wrote for this config before `writtenBeforeMs`. - * - vite deletes a bundle once importing it finishes, but the guard exits - * while vite is still importing the root run's bundle, so it stays behind. - * - vite imports a bundle milliseconds after writing it, and the rerun runs - * for seconds, so a bundle written before the guard started has been - * imported, by this run or a concurrent one, and is safe to remove. - */ -export const removeLeftoverConfigBundles = ({ - bundleFolder, - configName, - writtenBeforeMs, -}: { - bundleFolder: string - configName: string - writtenBeforeMs: number -}): void => { - const bundlePrefix = `${configName}.timestamp-` - - for (const entry of readdirSync(bundleFolder)) { - if (!entry.startsWith(bundlePrefix)) continue - - const bundlePath = join(bundleFolder, entry) - // vite may delete another run's bundle between the listing and this stat. - const bundleStats = statSync(bundlePath, { throwIfNoEntry: false }) - - if (!bundleStats || bundleStats.mtimeMs >= writtenBeforeMs) continue - rmSync(bundlePath, { force: true }) - } -} - -/** Runs a command as the target and reports whether it exited 0. */ -const runsAsTarget = ({ - target, - command, -}: { - target: RerunTarget - command: readonly string[] -}): boolean => { - const result = spawnSync("setpriv", [...buildSwitchArguments(target), ...command], { - stdio: "ignore", - }) - return result.status === 0 -} - -/** The id `id` prints for a user, or null when there is no such user. */ -const readUserId = ({ flag, user }: { flag: "-u" | "-g"; user: string }): number | null => { - const result = spawnSync("id", [flag, user], { encoding: "utf8" }) - - if (result.status !== 0) return null - return Number(result.stdout.trim()) -} - -const findRerunTarget = (checkoutRoot: string): TargetLookup => { - const setprivCheck = spawnSync("setpriv", ["--version"], { stdio: "ignore" }) - - if (setprivCheck.error) return { kind: "none", cause: { kind: "no-setpriv" } } - - const checkoutStats = statSync(checkoutRoot) - - if (checkoutStats.uid !== 0) { - const owner: RerunTarget = { - kind: "owner", - label: "the checkout's owner", - uid: checkoutStats.uid, - gid: checkoutStats.gid, - } - - return { kind: "found", target: owner } - } - - const nobodyUid = readUserId({ flag: "-u", user: "nobody" }) - const nobodyGid = readUserId({ flag: "-g", user: "nobody" }) - - if (nobodyUid === null || nobodyGid === null) { - return { kind: "none", cause: { kind: "no-nobody-user" } } - } - return { - kind: "found", - target: { kind: "nobody", label: "nobody", uid: nobodyUid, gid: nobodyGid }, - } -} - -const probeTarget = ({ - target, - configPath, - execPath, - extraCaCerts, -}: { - target: RerunTarget - configPath: string - execPath: string - extraCaCerts: string | undefined -}): TargetProbes => { - const passesFileTest = (testFlag: "-r" | "-x", path: string): boolean => { - // The path goes in as "$1", never inside the script, so the shell does - // not parse it; "sh" fills $0, the slot before $1. - const command = ["/bin/sh", "-c", `test ${testFlag} "$1"`, "sh", path] - - return runsAsTarget({ target, command }) - } - - return { - setprivWorks: runsAsTarget({ target, command: ["true"] }), - canReadConfig: passesFileTest("-r", configPath), - canRunNode: passesFileTest("-x", execPath), - canReadExtraCaCerts: extraCaCerts ? passesFileTest("-r", extraCaCerts) : true, - } -} - -/** - * Called by vitest.config.ts while vitest loads it. Does nothing unless the - * process is root. Otherwise reruns the same vitest command as another user - * and exits with its status, or prints a warning and returns so the run - * continues as root. - */ -export const rerunRootTestRun = ({ - checkoutRoot, - configPath, -}: { - checkoutRoot: string - configPath: string -}): void => { - // The rerun loads this config again as its own user, so this check is what - // keeps the rerun from rerunning itself. - if (process.getuid?.() !== 0) return - - const guardStartMs = Date.now() - const warn = (cause: WarningCause): void => { - process.stderr.write(`${formatWarning(cause)}\n`) - } - const [, vitestEntry, ...vitestArguments] = process.argv - // Node puts the script it runs in argv[1]. That is vitest's entry for a - // vitest command, but another program's script when that program starts - // vitest through its API, and rerunning that script is not a test run. - const startedByVitestCommand = - vitestEntry !== undefined && basename(realpathSync(vitestEntry)) === "vitest.mjs" - - if (!startedByVitestCommand) { - warn({ kind: "not-vitest-command" }) - return - } - - const targetLookup = findRerunTarget(checkoutRoot) - - if (targetLookup.kind === "none") { - warn(targetLookup.cause) - return - } - - const { target } = targetLookup - - const extraCaCerts = process.env.NODE_EXTRA_CA_CERTS - const nvmrcPath = join(checkoutRoot, ".nvmrc") - - const plan = planRootRun({ - checkoutRoot, - target, - probes: probeTarget({ - target, - configPath, - execPath: process.execPath, - extraCaCerts, - }), - execPath: process.execPath, - nodeVersion: process.version, - nvmrcText: existsSync(nvmrcPath) ? readFileSync(nvmrcPath, "utf8") : null, - execArguments: process.execArgv, - vitestEntry, - vitestArguments, - extraCaCertsSet: Boolean(extraCaCerts), - pathEnvironment: process.env.PATH ?? "", - }) - - if (plan.kind === "warn") { - process.stderr.write(`${plan.message}\n`) - return - } - - try { - prepareRerun({ - plan, - changeOwner: ({ path, uid, gid }) => lchownSync(path, uid, gid), - }) - } catch (error) { - warn({ kind: "prepare-failed", reason: readErrorMessage(error) }) - return - } - - process.stderr.write(`${plan.announcement}\n`) - - const rerun = spawnSync("setpriv", [...plan.setprivArguments], { stdio: "inherit" }) - - if (rerun.error) { - warn({ kind: "spawn-failed", reason: rerun.error.message }) - return - } - - try { - removeLeftoverConfigBundles({ - bundleFolder: join(checkoutRoot, "node_modules", CONFIG_BUNDLE_FOLDER), - configName: basename(configPath), - writtenBeforeMs: guardStartMs, - }) - } catch (error) { - process.stderr.write( - `Removing vite's leftover config bundles failed: ${readErrorMessage(error)}\n`, - ) - } - - // status is null when a signal ended the rerun; that run failed too. - process.exit(rerun.status ?? 1) -} diff --git a/scripts/test-as-nobody.ts b/scripts/test-as-nobody.ts new file mode 100644 index 000000000..6151ee233 --- /dev/null +++ b/scripts/test-as-nobody.ts @@ -0,0 +1,50 @@ +/** + * Runs vitest as `nobody` from a root shell, so the tests that skip as root + * (they deny file access, which root ignores) run too. Extra arguments go to + * `vitest run`: `npm run test:as-nobody -- src/utils`. + */ + +import { execFileSync, spawnSync } from "node:child_process" +import { chmodSync, copyFileSync, existsSync, mkdirSync } from "node:fs" +import { join } from "node:path" + +const readNobodyId = (flag: "-u" | "-g"): string => + execFileSync("id", [flag, "nobody"], { encoding: "utf8" }).trim() +const uid = readNobodyId("-u") +const gid = readNobodyId("-g") +const nodeModules = join(process.cwd(), "node_modules") +const testHome = join(nodeModules, ".test-home") + +// nobody writes only vite's config bundle, vitest's results cache, and its HOME. +const writableFolders = [".vite-temp", ".vite/vitest", ".test-home"].map((folder) => + join(nodeModules, folder), +) +for (const folder of writableFolders) mkdirSync(folder, { recursive: true }) +execFileSync("chown", ["-R", `${uid}:${gid}`, ...writableFolders]) + +// Root's Node usually sits under a folder nobody can't enter, such as /root/.nvm. +const nodeFolder = join(nodeModules, ".nobody-node", `node-${process.version}`) +const nodePath = join(nodeFolder, "node") + +if (!existsSync(nodePath)) { + mkdirSync(nodeFolder, { recursive: true }) + copyFileSync(process.execPath, nodePath) + chmodSync(nodePath, 0o755) +} + +const switchToNobody = [`--reuid=${uid}`, `--regid=${gid}`, "--clear-groups", "/usr/bin/env"] +// nobody may be unable to read root's CA bundle, and Node then warns on every start. +const environment = ["-u", "NODE_EXTRA_CA_CERTS", `HOME=${testHome}`] +const path = `PATH=${nodeFolder}:${process.env.PATH ?? ""}` +const vitest = [ + nodePath, + join(nodeModules, "vitest", "vitest.mjs"), + "run", + ...process.argv.slice(2), +] +const run = spawnSync("setpriv", [...switchToNobody, ...environment, path, ...vitest], { + stdio: "inherit", +}) + +if (run.error) throw run.error +process.exit(run.status ?? 1) diff --git a/src/__tests__/integration/server-error-contracts.test.ts b/src/__tests__/integration/server-error-contracts.test.ts index 6e7876af8..a4d29a56f 100644 --- a/src/__tests__/integration/server-error-contracts.test.ts +++ b/src/__tests__/integration/server-error-contracts.test.ts @@ -18,6 +18,7 @@ import { buildPasswordProtectedPdf, buildZeroPagePdf, } from "../../vault-mcp/obsidian-markdown/__tests__/pdf-fixture.js" +import { runsAsRoot } from "../runs-as-root.js" vi.setConfig({ testTimeout: 15_000 }) @@ -722,7 +723,7 @@ describe("filesystem errors name the path vault-relative", () => { }) } - it.each([ + it.skipIf(runsAsRoot).each([ { name: "vault_read_note", args: {} }, { name: "vault_update_properties", args: { properties: { status: "draft" } } }, { name: "vault_patch_note", args: { operation: "append", content: "added" } }, @@ -740,34 +741,44 @@ describe("filesystem errors name the path vault-relative", () => { }) }) - it("vault_read_file on a file it cannot open names the file vault-relative", async () => { - await plantFileWithMode({ filePath: "Locked.png", mode: 0o000 }) + it.skipIf(runsAsRoot)( + "vault_read_file on a file it cannot open names the file vault-relative", + async () => { + await plantFileWithMode({ filePath: "Locked.png", mode: 0o000 }) - const result = await callTool({ client, name: "vault_read_file", args: { path: "Locked.png" } }) - expect(result).toEqual({ - content: [{ type: "text", text: "[Error]: EACCES: permission denied, open 'Locked.png'" }], - isError: true, - }) - }) + const result = await callTool({ + client, + name: "vault_read_file", + args: { path: "Locked.png" }, + }) + expect(result).toEqual({ + content: [{ type: "text", text: "[Error]: EACCES: permission denied, open 'Locked.png'" }], + isError: true, + }) + }, + ) - it("vault_get_daily_note on a daily note it cannot open names the note vault-relative", async () => { - await plantFileWithMode({ filePath: "Daily Notes/2026-03-01.md", mode: 0o000 }) + it.skipIf(runsAsRoot)( + "vault_get_daily_note on a daily note it cannot open names the note vault-relative", + async () => { + await plantFileWithMode({ filePath: "Daily Notes/2026-03-01.md", mode: 0o000 }) - const result = await callTool({ - client, - name: "vault_get_daily_note", - args: { date: "2026-03-01" }, - }) - expect(result).toEqual({ - content: [ - { - type: "text", - text: "[Error]: EACCES: permission denied, open 'Daily Notes/2026-03-01.md'", - }, - ], - isError: true, - }) - }) + const result = await callTool({ + client, + name: "vault_get_daily_note", + args: { date: "2026-03-01" }, + }) + expect(result).toEqual({ + content: [ + { + type: "text", + text: "[Error]: EACCES: permission denied, open 'Daily Notes/2026-03-01.md'", + }, + ], + isError: true, + }) + }, + ) // A name of 233 bytes reads, but the temp file a save writes beside it // adds 41 bytes and goes over the filesystem's 255-byte limit @@ -801,27 +812,30 @@ describe("filesystem errors name the path vault-relative", () => { ) }) - it("vault_write_note into a read-only folder names the temp file vault-relative", async () => { - await plantFolderWithMode({ folderPath: "Sealed", mode: 0o555 }) + it.skipIf(runsAsRoot)( + "vault_write_note into a read-only folder names the temp file vault-relative", + async () => { + await plantFolderWithMode({ folderPath: "Sealed", mode: 0o555 }) - const result = await callTool({ - client, - name: "vault_write_note", - args: { path: "Sealed/new.md", body: "refused" }, - }) - expect(result).toEqual({ - content: [ - { - type: "text", - // The temp file's name ends in a random UUID - text: expect.stringMatching( - /^\[Error\]: EACCES: permission denied, open 'Sealed\/new\.md\.[0-9a-f-]{36}\.tmp'$/, - ), - }, - ], - isError: true, - }) - }) + const result = await callTool({ + client, + name: "vault_write_note", + args: { path: "Sealed/new.md", body: "refused" }, + }) + expect(result).toEqual({ + content: [ + { + type: "text", + // The temp file's name ends in a random UUID + text: expect.stringMatching( + /^\[Error\]: EACCES: permission denied, open 'Sealed\/new\.md\.[0-9a-f-]{36}\.tmp'$/, + ), + }, + ], + isError: true, + }) + }, + ) it("vault_write_note through a note names the note vault-relative", async () => { const result = await callTool({ @@ -942,7 +956,7 @@ describe("filesystem errors name the path vault-relative", () => { }) }) - it("vault_list_notes on a vault holding a folder it cannot open", async () => { + it.skipIf(runsAsRoot)("vault_list_notes on a vault holding a folder it cannot open", async () => { await plantFolderWithMode({ folderPath: "Sealed Folder", mode: 0o000 }) const result = await callTool({ client, name: "vault_list_notes", args: {} }) diff --git a/src/__tests__/runs-as-root.ts b/src/__tests__/runs-as-root.ts new file mode 100644 index 000000000..f118da8fa --- /dev/null +++ b/src/__tests__/runs-as-root.ts @@ -0,0 +1,3 @@ +/** Root ignores file permissions, so a test that denies access and expects the + * access to fail can't pass as root. Skip it there; CI runs as a normal user. */ +export const runsAsRoot = process.getuid?.() === 0 diff --git a/src/utils/__tests__/fs.test.ts b/src/utils/__tests__/fs.test.ts index 40cbb56a7..09c9a95a5 100644 --- a/src/utils/__tests__/fs.test.ts +++ b/src/utils/__tests__/fs.test.ts @@ -14,6 +14,7 @@ import { lstatOrNull, statOrNull, } from "../fs.js" +import { runsAsRoot } from "../../__tests__/runs-as-root.js" const makeTempDir = async (): Promise => { const dir = await mkdtemp(join(tmpdir(), "utils-fs-test-")) @@ -62,11 +63,14 @@ describe("readFileOrNull", () => { expect(await readFileOrNull(dir)).toBeNull() }) - it("rethrows a permission error rather than swallowing it as missing", async () => { - const dir = await makeTempDir() - const lockedDir = await makeUnreadableDir(dir) - await expect(readFileOrNull(join(lockedDir, "inner.md"))).rejects.toThrow(/EACCES/) - }) + it.skipIf(runsAsRoot)( + "rethrows a permission error rather than swallowing it as missing", + async () => { + const dir = await makeTempDir() + const lockedDir = await makeUnreadableDir(dir) + await expect(readFileOrNull(join(lockedDir, "inner.md"))).rejects.toThrow(/EACCES/) + }, + ) }) describe("readdirOrNull", () => { @@ -132,7 +136,7 @@ describe("statOrNull", () => { expect(await statOrNull(join(filePath, "child"))).toBeNull() }) - it("rethrows a permission error rather than swallowing it", async () => { + it.skipIf(runsAsRoot)("rethrows a permission error rather than swallowing it", async () => { const dir = await makeTempDir() const lockedDir = await makeUnreadableDir(dir) await expect(statOrNull(join(lockedDir, "inner.md"))).rejects.toThrow(/EACCES/) @@ -169,7 +173,7 @@ describe("lstatOrNull", () => { expect(await lstatOrNull(join(filePath, "child"))).toBeNull() }) - it("rethrows a permission error rather than swallowing it", async () => { + it.skipIf(runsAsRoot)("rethrows a permission error rather than swallowing it", async () => { const dir = await makeTempDir() const lockedDir = await makeUnreadableDir(dir) await expect(lstatOrNull(join(lockedDir, "inner.md"))).rejects.toThrow(/EACCES/) @@ -199,7 +203,7 @@ describe("realpathOrNull", () => { expect(await realpathOrNull(join(filePath, "child"))).toBeNull() }) - it("rethrows a permission error rather than swallowing it", async () => { + it.skipIf(runsAsRoot)("rethrows a permission error rather than swallowing it", async () => { const dir = await makeTempDir() const lockedDir = await makeUnreadableDir(dir) await expect(realpathOrNull(join(lockedDir, "inner.md"))).rejects.toThrow(/EACCES/) @@ -226,9 +230,12 @@ describe("fileExists", () => { expect(await fileExists(join(filePath, "child.md"))).toBe(false) }) - it("rethrows a permission error rather than reporting the path missing", async () => { - const dir = await makeTempDir() - const lockedDir = await makeUnreadableDir(dir) - await expect(fileExists(join(lockedDir, "inner.md"))).rejects.toThrow(/EACCES/) - }) + it.skipIf(runsAsRoot)( + "rethrows a permission error rather than reporting the path missing", + async () => { + const dir = await makeTempDir() + const lockedDir = await makeUnreadableDir(dir) + await expect(fileExists(join(lockedDir, "inner.md"))).rejects.toThrow(/EACCES/) + }, + ) }) diff --git a/src/vault-mcp/vault-operations/__tests__/memory-store.test.ts b/src/vault-mcp/vault-operations/__tests__/memory-store.test.ts index 9b429ea35..b275738b4 100644 --- a/src/vault-mcp/vault-operations/__tests__/memory-store.test.ts +++ b/src/vault-mcp/vault-operations/__tests__/memory-store.test.ts @@ -19,6 +19,7 @@ import { } from "../../obsidian-markdown/frontmatter.js" import { createMemoryStore } from "../memory-store.js" import { logger } from "../../../logger.js" +import { runsAsRoot } from "../../../__tests__/runs-as-root.js" vi.mock("node:fs/promises", { spy: true }) const createBlockedMemoryReads = async (vaultPath: string) => { @@ -176,14 +177,17 @@ describe("getMemory", () => { expect(await getMemory({ vaultPath: vault }, logger)).toBe(withoutSymlink) }) - it("rejects an unreadable memory file with a vault-relative message in the all-files read", async () => { - // afterEach deletes the vault, which removes a mode-000 file inside it, so - // the permissions need no restore. - await chmod(join(vault, "About Me", "Opinions.md"), 0o000) - await expect(getMemory({ vaultPath: vault }, logger)).rejects.toThrow( - new Error('cannot read memory file "About Me/Opinions.md"'), - ) - }) + it.skipIf(runsAsRoot)( + "rejects an unreadable memory file with a vault-relative message in the all-files read", + async () => { + // afterEach deletes the vault, which removes a mode-000 file inside it, so + // the permissions need no restore. + await chmod(join(vault, "About Me", "Opinions.md"), 0o000) + await expect(getMemory({ vaultPath: vault }, logger)).rejects.toThrow( + new Error('cannot read memory file "About Me/Opinions.md"'), + ) + }, + ) it("concatenates all memory files when no file specified", async () => { const result = await getMemory({ vaultPath: vault }, logger) @@ -271,23 +275,26 @@ describe("getMemory", () => { ) }) - it("rejects an unreadable memory file with a vault-relative message in the named read and logs the raw error", async () => { - const lockedFile = join(vault, "About Me", "Opinions.md") - // afterEach deletes the vault, which removes a mode-000 file inside it, so - // the permissions need no restore. - await chmod(lockedFile, 0o000) - const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}) - onTestFinished(() => warnSpy.mockRestore()) + it.skipIf(runsAsRoot)( + "rejects an unreadable memory file with a vault-relative message in the named read and logs the raw error", + async () => { + const lockedFile = join(vault, "About Me", "Opinions.md") + // afterEach deletes the vault, which removes a mode-000 file inside it, so + // the permissions need no restore. + await chmod(lockedFile, 0o000) + const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}) + onTestFinished(() => warnSpy.mockRestore()) - await expect(getMemory({ vaultPath: vault, file: "Opinions" }, logger)).rejects.toThrow( - new Error('cannot read memory file "About Me/Opinions.md"'), - ) - expect(warnSpy).toHaveBeenCalledTimes(1) - expect(warnSpy).toHaveBeenCalledWith("cannot read memory file", { - file: "Opinions.md", - error: `[Error]: EACCES: permission denied, open '${lockedFile}'`, - }) - }) + await expect(getMemory({ vaultPath: vault, file: "Opinions" }, logger)).rejects.toThrow( + new Error('cannot read memory file "About Me/Opinions.md"'), + ) + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith("cannot read memory file", { + file: "Opinions.md", + error: `[Error]: EACCES: permission denied, open '${lockedFile}'`, + }) + }, + ) it("throws on non-existent section", async () => { await expect( @@ -565,23 +572,26 @@ created: 2026-01-01T00:00:00-05:00 expect(result).toBe("- **2026-05-15**: first entry") }) - it("rejects an unreadable memory file with a vault-relative message", async () => { - // afterEach deletes the vault, which removes a mode-000 file inside it, so - // the permissions need no restore. - await chmod(join(vault, "About Me", "Opinions.md"), 0o000) - await expect( - updateMemory( - { - vaultPath: vault, - file: "Opinions", - section: "Code patterns", - entry: "never written", - date: "2026-05-15", - }, - logger, - ), - ).rejects.toThrow(new Error('cannot read memory file "About Me/Opinions.md"')) - }) + it.skipIf(runsAsRoot)( + "rejects an unreadable memory file with a vault-relative message", + async () => { + // afterEach deletes the vault, which removes a mode-000 file inside it, so + // the permissions need no restore. + await chmod(join(vault, "About Me", "Opinions.md"), 0o000) + await expect( + updateMemory( + { + vaultPath: vault, + file: "Opinions", + section: "Code patterns", + entry: "never written", + date: "2026-05-15", + }, + logger, + ), + ).rejects.toThrow(new Error('cannot read memory file "About Me/Opinions.md"')) + }, + ) it("leaves a symlink to a folder in place when it has the memory file's name", async () => { await mkdir(join(vault, "linked-folder")) @@ -1874,15 +1884,18 @@ describe("listMemoryFiles", () => { expect(outlines.map((outline) => outline.file)).toEqual(["Opinions", "Principles"]) }) - it("rejects an unreadable memory file with a vault-relative message", async () => { - const lockedFile = join(vault, "About Me", "Opinions.md") - // afterEach deletes the vault, which removes a mode-000 file inside it, so - // the permissions need no restore. - await chmod(lockedFile, 0o000) - await expect(listMemoryFiles({ vaultPath: vault }, logger)).rejects.toThrow( - new Error('cannot read memory file "About Me/Opinions.md"'), - ) - }) + it.skipIf(runsAsRoot)( + "rejects an unreadable memory file with a vault-relative message", + async () => { + const lockedFile = join(vault, "About Me", "Opinions.md") + // afterEach deletes the vault, which removes a mode-000 file inside it, so + // the permissions need no restore. + await chmod(lockedFile, 0o000) + await expect(listMemoryFiles({ vaultPath: vault }, logger)).rejects.toThrow( + new Error('cannot read memory file "About Me/Opinions.md"'), + ) + }, + ) it("returns outlines sorted by filename", async () => { const outlines = await listMemoryFiles({ vaultPath: vault }, logger) @@ -2123,27 +2136,30 @@ describe("listMemoryFileNames", () => { ).rejects.toThrow(new Error('cannot list memory folder "About Me"')) }) - it("rejects an unreadable memory folder with a vault-relative message and logs the raw error", async () => { - const vaultWithLockedMemoryFolder = await mkdtemp(join(tmpdir(), "locked-memory-folder-")) - onTestFinished(() => rm(vaultWithLockedMemoryFolder, { recursive: true })) - const memoryFolder = join(vaultWithLockedMemoryFolder, "About Me") - await mkdir(memoryFolder) - await chmod(memoryFolder, 0o000) - // Registered after the removal so that it runs first, which makes the - // folder readable again before the vault is removed. - onTestFinished(() => chmod(memoryFolder, 0o700)) - const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}) - onTestFinished(() => warnSpy.mockRestore()) + it.skipIf(runsAsRoot)( + "rejects an unreadable memory folder with a vault-relative message and logs the raw error", + async () => { + const vaultWithLockedMemoryFolder = await mkdtemp(join(tmpdir(), "locked-memory-folder-")) + onTestFinished(() => rm(vaultWithLockedMemoryFolder, { recursive: true })) + const memoryFolder = join(vaultWithLockedMemoryFolder, "About Me") + await mkdir(memoryFolder) + await chmod(memoryFolder, 0o000) + // Registered after the removal so that it runs first, which makes the + // folder readable again before the vault is removed. + onTestFinished(() => chmod(memoryFolder, 0o700)) + const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}) + onTestFinished(() => warnSpy.mockRestore()) - await expect( - listMemoryFileNames({ vaultPath: vaultWithLockedMemoryFolder }, logger), - ).rejects.toThrow(new Error('cannot list memory folder "About Me"')) - expect(warnSpy).toHaveBeenCalledTimes(1) - expect(warnSpy).toHaveBeenCalledWith("cannot list memory folder", { - folder: "About Me", - error: `[Error]: EACCES: permission denied, scandir '${memoryFolder}'`, - }) - }) + await expect( + listMemoryFileNames({ vaultPath: vaultWithLockedMemoryFolder }, logger), + ).rejects.toThrow(new Error('cannot list memory folder "About Me"')) + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith("cannot list memory folder", { + folder: "About Me", + error: `[Error]: EACCES: permission denied, scandir '${memoryFolder}'`, + }) + }, + ) }) describe("custom memoryDir", () => { diff --git a/src/vault-mcp/vault-operations/__tests__/trash-sweeper.test.ts b/src/vault-mcp/vault-operations/__tests__/trash-sweeper.test.ts index 32cca1beb..84086379f 100644 --- a/src/vault-mcp/vault-operations/__tests__/trash-sweeper.test.ts +++ b/src/vault-mcp/vault-operations/__tests__/trash-sweeper.test.ts @@ -24,6 +24,7 @@ import { readTrashFileState, vaultFs } from "../vault-filesystem.js" import { createSearchIndex } from "../../search/search-index.js" import type { TrashEntry, TrashEntryStore } from "../../search/search-index.js" import { logger } from "../../../logger.js" +import { runsAsRoot } from "../../../__tests__/runs-as-root.js" /** A temp vault with a .trash/ folder, removed when the test finishes. */ const createTestVault = async (): Promise => { @@ -437,57 +438,65 @@ describe("sweepExpiredTrashEntries", () => { }) }) - it("keeps the row and warns when realpath fails with a non-ENOENT error", async () => { - const vault = await createTestVault() - const lockedDir = join(vault, ".trash", "noaccess") - await mkdir(lockedDir, { recursive: true }) - await writeFile(join(lockedDir, "stuck.md"), "perm error", "utf8") - const index = createSearchIndex(":memory:") - index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/noaccess/stuck.md")) - // Remove all permissions from .trash/ so realpath on the parent fails. - await chmod(join(vault, ".trash"), 0o000) - onTestFinished(() => chmod(join(vault, ".trash"), 0o755)) - const warnSpy = vi.spyOn(logger, "warn") - onTestFinished(() => warnSpy.mockRestore()) + it.skipIf(runsAsRoot)( + "keeps the row and warns when realpath fails with a non-ENOENT error", + async () => { + const vault = await createTestVault() + const lockedDir = join(vault, ".trash", "noaccess") + await mkdir(lockedDir, { recursive: true }) + await writeFile(join(lockedDir, "stuck.md"), "perm error", "utf8") + const index = createSearchIndex(":memory:") + index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/noaccess/stuck.md")) + // Remove all permissions from .trash/ so realpath on the parent fails. + await chmod(join(vault, ".trash"), 0o000) + onTestFinished(() => chmod(join(vault, ".trash"), 0o755)) + const warnSpy = vi.spyOn(logger, "warn") + onTestFinished(() => warnSpy.mockRestore()) - await sweepAfterRetention(vault, index) + await sweepAfterRetention(vault, index) - expect(index.getTrashEntry(".trash/noaccess/stuck.md")?.trashPath).toBe( - ".trash/noaccess/stuck.md", - ) - expect(warnSpy).toHaveBeenCalledTimes(1) - expect(warnSpy).toHaveBeenCalledWith("failed to resolve trash entry path", { - trashPath: ".trash/noaccess/stuck.md", - error: `[Error]: EACCES: permission denied, realpath '${lockedDir}'`, - }) - }) + expect(index.getTrashEntry(".trash/noaccess/stuck.md")?.trashPath).toBe( + ".trash/noaccess/stuck.md", + ) + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith("failed to resolve trash entry path", { + trashPath: ".trash/noaccess/stuck.md", + error: `[Error]: EACCES: permission denied, realpath '${lockedDir}'`, + }) + }, + ) - it("keeps the row and warns when unlink fails with a non-ENOENT error", async () => { - // A permission error during unlink keeps the row so the next sweep - // retries; one bad row never aborts the sweep. - const vault = await createTestVault() - const lockedDir = join(vault, ".trash", "locked") - await mkdir(lockedDir, { recursive: true }) - await writeFile(join(lockedDir, "stuck.md"), "perm error", "utf8") - const index = createSearchIndex(":memory:") - index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/locked/stuck.md")) - // Remove write permission from the parent so unlink fails with EACCES. - await chmod(lockedDir, 0o555) - onTestFinished(() => chmod(lockedDir, 0o755)) - const warnSpy = vi.spyOn(logger, "warn") - onTestFinished(() => warnSpy.mockRestore()) + it.skipIf(runsAsRoot)( + "keeps the row and warns when unlink fails with a non-ENOENT error", + async () => { + // A permission error during unlink keeps the row so the next sweep + // retries; one bad row never aborts the sweep. + const vault = await createTestVault() + const lockedDir = join(vault, ".trash", "locked") + await mkdir(lockedDir, { recursive: true }) + await writeFile(join(lockedDir, "stuck.md"), "perm error", "utf8") + const index = createSearchIndex(":memory:") + index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/locked/stuck.md")) + // Remove write permission from the parent so unlink fails with EACCES. + await chmod(lockedDir, 0o555) + onTestFinished(() => chmod(lockedDir, 0o755)) + const warnSpy = vi.spyOn(logger, "warn") + onTestFinished(() => warnSpy.mockRestore()) - await sweepAfterRetention(vault, index) + await sweepAfterRetention(vault, index) - expect(index.getTrashEntry(".trash/locked/stuck.md")?.trashPath).toBe(".trash/locked/stuck.md") - const content = await readFile(join(lockedDir, "stuck.md"), "utf8") - expect(content).toBe("perm error") - expect(warnSpy).toHaveBeenCalledTimes(1) - expect(warnSpy).toHaveBeenCalledWith("failed to remove expired trash entry", { - trashPath: ".trash/locked/stuck.md", - error: `[Error]: EACCES: permission denied, unlink '${join(lockedDir, "stuck.md")}'`, - }) - }) + expect(index.getTrashEntry(".trash/locked/stuck.md")?.trashPath).toBe( + ".trash/locked/stuck.md", + ) + const content = await readFile(join(lockedDir, "stuck.md"), "utf8") + expect(content).toBe("perm error") + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith("failed to remove expired trash entry", { + trashPath: ".trash/locked/stuck.md", + error: `[Error]: EACCES: permission denied, unlink '${join(lockedDir, "stuck.md")}'`, + }) + }, + ) it("an unrecorded delete clears a stale row at its landed path before any sweep runs", async () => { // A row can outlive its file in .trash/. A keep-forever "local" delete @@ -736,34 +745,37 @@ describe("sweepExpiredTrashEntries", () => { }) }) - it("keeps the row and warns when the identity read fails with a non-ENOENT error", async () => { - const vault = await createTestVault() - const index = createSearchIndex(":memory:") - const lockedDir = join(vault, ".trash", "nosearch") - await mkdir(lockedDir) - await writeFile(join(lockedDir, "stuck.md"), "stays", "utf8") - index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/nosearch/stuck.md")) - // Without search permission the folder itself still resolves, so the - // containment gate passes, but no entry inside it can be looked up. - await chmod(lockedDir, 0o600) - onTestFinished(() => chmod(lockedDir, 0o755)) - const warnSpy = vi.spyOn(logger, "warn") - onTestFinished(() => warnSpy.mockRestore()) + it.skipIf(runsAsRoot)( + "keeps the row and warns when the identity read fails with a non-ENOENT error", + async () => { + const vault = await createTestVault() + const index = createSearchIndex(":memory:") + const lockedDir = join(vault, ".trash", "nosearch") + await mkdir(lockedDir) + await writeFile(join(lockedDir, "stuck.md"), "stays", "utf8") + index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/nosearch/stuck.md")) + // Without search permission the folder itself still resolves, so the + // containment gate passes, but no entry inside it can be looked up. + await chmod(lockedDir, 0o600) + onTestFinished(() => chmod(lockedDir, 0o755)) + const warnSpy = vi.spyOn(logger, "warn") + onTestFinished(() => warnSpy.mockRestore()) - await sweepAfterRetention(vault, index) + await sweepAfterRetention(vault, index) - expect(index.getTrashEntry(".trash/nosearch/stuck.md")?.trashPath).toBe( - ".trash/nosearch/stuck.md", - ) - expect(warnSpy).toHaveBeenCalledTimes(1) - expect(warnSpy).toHaveBeenCalledWith("failed to read trash entry identity", { - trashPath: ".trash/nosearch/stuck.md", - error: `[Error]: EACCES: permission denied, lstat '${join(lockedDir, "stuck.md")}'`, - }) - await chmod(lockedDir, 0o755) - const stuckContent = await readFile(join(lockedDir, "stuck.md"), "utf8") - expect(stuckContent).toBe("stays") - }) + expect(index.getTrashEntry(".trash/nosearch/stuck.md")?.trashPath).toBe( + ".trash/nosearch/stuck.md", + ) + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith("failed to read trash entry identity", { + trashPath: ".trash/nosearch/stuck.md", + error: `[Error]: EACCES: permission denied, lstat '${join(lockedDir, "stuck.md")}'`, + }) + await chmod(lockedDir, 0o755) + const stuckContent = await readFile(join(lockedDir, "stuck.md"), "utf8") + expect(stuckContent).toBe("stays") + }, + ) it("prunes the folders an unlink empties, keeping .trash/ itself", async () => { const vault = await createTestVault() @@ -1349,31 +1361,36 @@ describe("purgeOrphanedTrashEntries", () => { }) }) - it("keeps the row and warns when stat fails with a non-ENOENT error", async () => { - const vault = await createTestVault() - const lockedDir = join(vault, ".trash", "locked") - await mkdir(lockedDir, { recursive: true }) - await writeFile(join(lockedDir, "stuck.md"), "perm error", "utf8") - const index = createSearchIndex(":memory:") - index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/locked/stuck.md")) - // Remove traverse permission from the parent so stat fails with EACCES. - await chmod(lockedDir, 0o000) - onTestFinished(() => chmod(lockedDir, 0o755)) - const warnSpy = vi.spyOn(logger, "warn") - onTestFinished(() => warnSpy.mockRestore()) + it.skipIf(runsAsRoot)( + "keeps the row and warns when stat fails with a non-ENOENT error", + async () => { + const vault = await createTestVault() + const lockedDir = join(vault, ".trash", "locked") + await mkdir(lockedDir, { recursive: true }) + await writeFile(join(lockedDir, "stuck.md"), "perm error", "utf8") + const index = createSearchIndex(":memory:") + index.recordTrashEntry(await recordedTrashFileAt(vault, ".trash/locked/stuck.md")) + // Remove traverse permission from the parent so stat fails with EACCES. + await chmod(lockedDir, 0o000) + onTestFinished(() => chmod(lockedDir, 0o755)) + const warnSpy = vi.spyOn(logger, "warn") + onTestFinished(() => warnSpy.mockRestore()) - await trashSweeper.purgeOrphanedTrashEntries( - { vaultPath: vault, trashEntryStore: index }, - logger, - ) + await trashSweeper.purgeOrphanedTrashEntries( + { vaultPath: vault, trashEntryStore: index }, + logger, + ) - expect(index.getTrashEntry(".trash/locked/stuck.md")?.trashPath).toBe(".trash/locked/stuck.md") - expect(warnSpy).toHaveBeenCalledTimes(1) - expect(warnSpy).toHaveBeenCalledWith("failed to stat trash entry", { - trashPath: ".trash/locked/stuck.md", - error: `[Error]: EACCES: permission denied, lstat '${join(lockedDir, "stuck.md")}'`, - }) - }) + expect(index.getTrashEntry(".trash/locked/stuck.md")?.trashPath).toBe( + ".trash/locked/stuck.md", + ) + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith("failed to stat trash entry", { + trashPath: ".trash/locked/stuck.md", + error: `[Error]: EACCES: permission denied, lstat '${join(lockedDir, "stuck.md")}'`, + }) + }, + ) it("retains a traversal-path row when the target exists outside .trash/", async () => { const base = await mkdtemp(join(tmpdir(), "trash-traversal-")) diff --git a/vitest.config.ts b/vitest.config.ts index 288af1510..219a9e6cd 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,12 +1,4 @@ import { defineConfig } from "vitest/config" -import { rerunRootTestRun } from "./scripts/root-test-guard.js" - -// vitest sets VITEST before it loads this file. Tools that load it without -// running tests, such as knip, don't, so they never reach the guard. The -// rerun loads this file again, but not as root, so the guard returns at once. -if (process.env.VITEST === "true") { - rerunRootTestRun({ checkoutRoot: import.meta.dirname, configPath: import.meta.filename }) -} export default defineConfig({ test: { From 081605e06037ee2fa6529756f045145289b18112 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:16:23 +0000 Subject: [PATCH 14/14] chore(tests): copy nobody's Node under a temporary name first A copy cut short by a full disk would otherwise sit at the final path and be reused on every later run. Co-Authored-By: Claude Opus 5.5 --- scripts/test-as-nobody.ts | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/scripts/test-as-nobody.ts b/scripts/test-as-nobody.ts index 6151ee233..a7e5667b5 100644 --- a/scripts/test-as-nobody.ts +++ b/scripts/test-as-nobody.ts @@ -5,13 +5,12 @@ */ import { execFileSync, spawnSync } from "node:child_process" -import { chmodSync, copyFileSync, existsSync, mkdirSync } from "node:fs" +import { chmodSync, copyFileSync, existsSync, mkdirSync, renameSync } from "node:fs" import { join } from "node:path" const readNobodyId = (flag: "-u" | "-g"): string => execFileSync("id", [flag, "nobody"], { encoding: "utf8" }).trim() -const uid = readNobodyId("-u") -const gid = readNobodyId("-g") +const [uid, gid] = (["-u", "-g"] as const).map(readNobodyId) const nodeModules = join(process.cwd(), "node_modules") const testHome = join(nodeModules, ".test-home") @@ -22,27 +21,25 @@ const writableFolders = [".vite-temp", ".vite/vitest", ".test-home"].map((folder for (const folder of writableFolders) mkdirSync(folder, { recursive: true }) execFileSync("chown", ["-R", `${uid}:${gid}`, ...writableFolders]) -// Root's Node usually sits under a folder nobody can't enter, such as /root/.nvm. +// Root's Node usually sits where nobody can't enter, such as /root/.nvm. const nodeFolder = join(nodeModules, ".nobody-node", `node-${process.version}`) const nodePath = join(nodeFolder, "node") if (!existsSync(nodePath)) { mkdirSync(nodeFolder, { recursive: true }) - copyFileSync(process.execPath, nodePath) - chmodSync(nodePath, 0o755) + // A temporary name keeps a copy cut short (a full disk) from being reused. + copyFileSync(process.execPath, `${nodePath}.partial`) + chmodSync(`${nodePath}.partial`, 0o755) + renameSync(`${nodePath}.partial`, nodePath) } -const switchToNobody = [`--reuid=${uid}`, `--regid=${gid}`, "--clear-groups", "/usr/bin/env"] +const asNobody = [`--reuid=${uid}`, `--regid=${gid}`, "--clear-groups", "/usr/bin/env"] // nobody may be unable to read root's CA bundle, and Node then warns on every start. -const environment = ["-u", "NODE_EXTRA_CA_CERTS", `HOME=${testHome}`] +const nobodyEnv = ["-u", "NODE_EXTRA_CA_CERTS", `HOME=${testHome}`] const path = `PATH=${nodeFolder}:${process.env.PATH ?? ""}` -const vitest = [ - nodePath, - join(nodeModules, "vitest", "vitest.mjs"), - "run", - ...process.argv.slice(2), -] -const run = spawnSync("setpriv", [...switchToNobody, ...environment, path, ...vitest], { +const vitestEntry = join(nodeModules, "vitest", "vitest.mjs") +const vitest = [nodePath, vitestEntry, "run", ...process.argv.slice(2)] +const run = spawnSync("setpriv", [...asNobody, ...nobodyEnv, path, ...vitest], { stdio: "inherit", })