diff --git a/scripts/verify-package-setup-plan.d.mts b/scripts/verify-package-setup-plan.d.mts new file mode 100644 index 0000000000..08fe24aed8 --- /dev/null +++ b/scripts/verify-package-setup-plan.d.mts @@ -0,0 +1,8 @@ +/** Preserve an unexpected JSON result while describing the failed plan check. */ +export function setupPlanFailureMessage(options: { + readonly planned: unknown; + readonly plannedExecution: { readonly status: number | null }; + readonly plannedClaudeConfig: string; + readonly plannedCodexConfig: string; + readonly plannedCursorConfig: string; +}): string; diff --git a/scripts/verify-package-setup-plan.mjs b/scripts/verify-package-setup-plan.mjs new file mode 100644 index 0000000000..4734607367 --- /dev/null +++ b/scripts/verify-package-setup-plan.mjs @@ -0,0 +1,23 @@ +/** Describe a failed package setup-plan check without masking its result shape. */ +export const setupPlanFailureMessage = ({ + planned, + plannedExecution, + plannedClaudeConfig, + plannedCodexConfig, + plannedCursorConfig, +}) => { + const actionKinds = (actions) => + Array.isArray(actions) + ? actions.map((action) => action?.kind ?? action) + : actions; + return `packaged setup plan was not read-only: ${JSON.stringify({ + status: planned?.status, + exitCode: plannedExecution.status, + plannedKinds: actionKinds(planned?.plannedActions), + appliedKinds: actionKinds(planned?.appliedActions), + result: planned, + claudeConfig: plannedClaudeConfig, + codexConfig: plannedCodexConfig, + cursorConfig: plannedCursorConfig, + })}`; +}; diff --git a/scripts/verify-package-setup.mjs b/scripts/verify-package-setup.mjs index ad23548cb3..e44c4f5c88 100644 --- a/scripts/verify-package-setup.mjs +++ b/scripts/verify-package-setup.mjs @@ -21,6 +21,7 @@ import { } from "./lib/verify-package-core.mjs"; import { parse as parseJsonc } from "jsonc-parser"; import { skillReferenceIssues } from "./lib/docs-facts.mjs"; +import { setupPlanFailureMessage } from "./verify-package-setup-plan.mjs"; export const terminalModesRestored = (termios) => { if (typeof termios !== "string") return false; @@ -250,23 +251,37 @@ const verifySetupPlan = async ({ ); const planned = json(plannedExecution.stdout); if (supportedSetupHost) { + const plannedActions = Array.isArray(planned?.plannedActions) + ? planned.plannedActions + : undefined; + const appliedActions = Array.isArray(planned?.appliedActions) + ? planned.appliedActions + : undefined; const plannedClaudeConfig = await readFile(claudeConfig, "utf8"); const plannedCodexConfig = await readFile(codexTarget, "utf8"); const plannedCursorConfig = await readFile(cursorConfig, "utf8"); const plannedOpenCodeConfig = await readFile(opencodeConfig, "utf8"); if ( - planned.status !== "planned" || + planned?.status !== "planned" || plannedExecution.status !== 0 || - planned.appliedActions.length !== 0 || - !planned.plannedActions.some(({ kind }) => kind === "configure_client") || - !planned.plannedActions.some(({ kind }) => kind === "install_skill") || + plannedActions === undefined || + appliedActions === undefined || + appliedActions.length !== 0 || + !plannedActions.some((action) => action?.kind === "configure_client") || + !plannedActions.some((action) => action?.kind === "install_skill") || plannedClaudeConfig !== '{"existing":true}\n' || plannedCodexConfig !== CODEX_ORIGINAL || plannedCursorConfig !== '{"existing":true}\n' || plannedOpenCodeConfig !== opencodeOriginal ) throw new Error( - `packaged setup plan was not read-only: ${JSON.stringify({ status: planned.status, exitCode: plannedExecution.status, plannedKinds: planned.plannedActions.map(({ kind }) => kind), appliedKinds: planned.appliedActions.map(({ kind }) => kind), claudeConfig: plannedClaudeConfig, codexConfig: plannedCodexConfig, cursorConfig: plannedCursorConfig })}`, + setupPlanFailureMessage({ + planned, + plannedExecution, + plannedClaudeConfig, + plannedCodexConfig, + plannedCursorConfig, + }), ); } return { planned }; diff --git a/scripts/verify/recorded-crash/trace-ownership.d.mts b/scripts/verify/recorded-crash/trace-ownership.d.mts new file mode 100644 index 0000000000..0daeba3737 --- /dev/null +++ b/scripts/verify/recorded-crash/trace-ownership.d.mts @@ -0,0 +1,6 @@ +/** Assert the documented procfs allowance over strace -ff process/thread files. */ +export function assertHistoricalPidReadsAreOwnershipOnly(options: { + readonly traces: readonly { readonly pid: number; readonly text: string }[]; + readonly sentinelPid: number; + readonly psExecutables: ReadonlySet; +}): string[]; diff --git a/scripts/verify/recorded-crash/trace-ownership.mjs b/scripts/verify/recorded-crash/trace-ownership.mjs new file mode 100644 index 0000000000..0267dcdaa8 --- /dev/null +++ b/scripts/verify/recorded-crash/trace-ownership.mjs @@ -0,0 +1,127 @@ +import assert from "node:assert/strict"; + +const ownershipPsArguments = + '["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="]'; + +/** + * Allow only the procfs paths used by the process-ownership scan: ps may + * inspect its process-table fields, and its caller's thread group may read + * native identity and UID records. With strace -ff, libuv worker threads get + * separate files; follow CLONE_THREAD edges, never ordinary child-process + * edges. strace cannot distinguish the purpose of a stat/status read inside + * the same Node thread group, so this is deliberately limited to those two + * files; provider Python/GDB children remain outside the permit. + */ +export const assertHistoricalPidReadsAreOwnershipOnly = ({ + traces, + sentinelPid, + psExecutables, +}) => { + const processGroups = new Map(); + const findGroup = (pid) => { + const parent = processGroups.get(pid); + if (parent === undefined) { + processGroups.set(pid, pid); + return pid; + } + if (parent === pid) return pid; + const root = findGroup(parent); + processGroups.set(pid, root); + return root; + }; + const joinThreadGroup = (left, right) => { + const leftRoot = findGroup(left); + const rightRoot = findGroup(right); + if (leftRoot !== rightRoot) processGroups.set(rightRoot, leftRoot); + }; + + const cloneEvents = []; + for (const trace of traces) { + let unfinished; + for (const rawLine of trace.text.split("\n")) { + const line = rawLine.trimStart(); + const start = /^(clone3?|fork|vfork)\((.*)$/u.exec( + line, + ); + if (start !== null) { + unfinished = { name: start[1], text: start[2] }; + continue; + } + const resumed = /^<\.\.\. (clone3?|fork|vfork) resumed>(.*)$/u.exec(line); + if (resumed !== null && resumed[1] === unfinished?.name) { + cloneEvents.push({ + parentPid: trace.pid, + text: `${unfinished.text}${resumed[2]}`, + }); + unfinished = undefined; + continue; + } + const complete = + /^(clone3?|fork|vfork)\((.*\)) = (\d+)(?:<[^>]*>)?$/u.exec(line); + if (complete !== null) + cloneEvents.push({ + parentPid: trace.pid, + childPid: Number(complete[3]), + text: complete[2], + }); + } + } + + for (const event of cloneEvents) { + const childPid = + event.childPid ?? /\) = (\d+)(?:<[^>]*>)?$/u.exec(event.text)?.[1]; + if (childPid === undefined) continue; + event.childPid = Number(childPid); + if (/\bCLONE_THREAD\b/u.test(event.text)) + joinThreadGroup(event.parentPid, event.childPid); + } + + const psPids = new Set(); + for (const trace of traces) { + const successfulPsExec = trace.text.split("\n").some((line) => { + const executable = /^execve\("([^"]+)"/u.exec(line)?.[1]; + return ( + executable !== undefined && + psExecutables.has(executable) && + line.includes(ownershipPsArguments) && + line.endsWith(" = 0") + ); + }); + if (successfulPsExec) psPids.add(trace.pid); + } + + const psParentGroups = new Set( + cloneEvents + .filter(({ childPid }) => childPid !== undefined && psPids.has(childPid)) + .map(({ parentPid }) => findGroup(parentPid)), + ); + + const ownershipReads = []; + const pidPrefix = `/proc/${sentinelPid}/`; + for (const trace of traces) { + const isPsProcess = psPids.has(trace.pid); + const isOwnershipProcessThread = psParentGroups.has(findGroup(trace.pid)); + const pidReads = trace.text + .split("\n") + .filter((line) => line.includes(pidPrefix)); + for (const line of pidReads) { + const psAllowedPath = new RegExp( + `/proc/${sentinelPid}/(?:stat|status|cmdline|wchan|environ)"`, + "u", + ).test(line); + const ownershipThreadPath = new RegExp( + `/proc/${sentinelPid}/(?:stat|status)"`, + "u", + ).test(line); + const psRead = isPsProcess && psAllowedPath; + const ownershipThreadRead = + isOwnershipProcessThread && ownershipThreadPath; + assert.ok( + psRead || ownershipThreadRead, + `Historical PID caused a provider lookup in trace.${trace.pid}: ${line}`, + ); + ownershipReads.push(line); + } + } + return ownershipReads; +}; diff --git a/scripts/verify/recorded-crash/trace.mjs b/scripts/verify/recorded-crash/trace.mjs index 7cf5835087..44ef4f9632 100644 --- a/scripts/verify/recorded-crash/trace.mjs +++ b/scripts/verify/recorded-crash/trace.mjs @@ -3,6 +3,7 @@ import { readdir, readFile, realpath } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { isAbsolute, join, resolve } from "node:path"; import { runOwnedCommand } from "../../../dist/process/OwnedCommand.js"; +import { assertHistoricalPidReadsAreOwnershipOnly } from "./trace-ownership.mjs"; /** Check attempted syscalls, including failed attempts, on a recorded PID that collides with an owned live sentinel. */ export async function verifyCoreInspectionTrace({ @@ -68,11 +69,22 @@ export async function verifyCoreInspectionTrace({ { timeoutMs: 90_000, diagnosticBytes: 1024 * 1024 }, ); const executions = []; - const ownershipReads = []; let gdbObserved = false; - for (const file of await readdir(root)) { - if (!file.startsWith(`inspection-${mode}.trace.`)) continue; - const text = await readFile(join(root, file), "utf8"); + const traces = await Promise.all( + (await readdir(root)) + .filter((file) => file.startsWith(`inspection-${mode}.trace.`)) + .map(async (file) => ({ + file, + pid: Number(file.slice(`inspection-${mode}.trace.`.length)), + text: await readFile(join(root, file), "utf8"), + })), + ); + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + traces, + sentinelPid: sentinel.pid, + psExecutables: psPaths, + }); + for (const { file, text } of traces) { // Keep the source executable present: GDB 16 otherwise silently hides a // core-only violation when its automatic executable lookup finds nothing. const recordedExecutable = JSON.stringify(join(root, "fixture-program")); @@ -80,36 +92,6 @@ export async function verifyCoreInspectionTrace({ !text.split("\n").some((line) => line.includes(recordedExecutable)), `Inspection attempted access to the recorded executable: ${file}`, ); - const ownershipTrace = text - .split("\n") - .some( - (line) => - /^execve\("\/(?:usr\/)?bin\/ps",/u.test(line) && - line.includes( - '["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="]', - ) && - line.endsWith(" = 0"), - ); - const pidReads = text - .split("\n") - .filter((line) => line.includes(`/proc/${sentinel.pid}/`)); - if (ownershipTrace) { - for (const line of pidReads) { - assert.ok( - new RegExp( - `/proc/${sentinel.pid}/(?:stat|status|cmdline|wchan|environ)"`, - "u", - ).test(line), - `Unexpected ownership observation of the sentinel: ${line}`, - ); - ownershipReads.push(line); - } - } else - assert.deepEqual( - pidReads, - [], - `Historical PID caused a provider lookup in ${file}: ${pidReads.join("\n")}`, - ); assert.ok( !/\bptrace\(/u.test(text), `Inspection attempted ptrace: ${file}`, diff --git a/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts b/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts new file mode 100644 index 0000000000..7de7c1d420 --- /dev/null +++ b/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts @@ -0,0 +1,39 @@ +import { expect, it } from "vitest"; + +import { setupPlanFailureMessage } from "../../../scripts/verify-package-setup-plan.mjs"; + +it("preserves setup failure details when an error result has no action arrays", () => { + const result = { + status: "needs_human", + remediation: "Review the host setup requirements.", + }; + const message = setupPlanFailureMessage({ + planned: result, + plannedExecution: { status: 1 }, + plannedClaudeConfig: "unchanged", + plannedCodexConfig: "unchanged", + plannedCursorConfig: "unchanged", + }); + + expect(message).toContain("packaged setup plan was not read-only"); + expect(message).toContain('"status":"needs_human"'); + expect(message).toContain("Review the host setup requirements."); + expect(message).not.toContain("TypeError"); +}); + +it("still formats action kinds for a structured setup result", () => { + const message = setupPlanFailureMessage({ + planned: { + status: "planned", + plannedActions: [{ kind: "configure_client" }], + appliedActions: [], + }, + plannedExecution: { status: 0 }, + plannedClaudeConfig: "unchanged", + plannedCodexConfig: "unchanged", + plannedCursorConfig: "unchanged", + }); + + expect(message).toContain('"plannedKinds":["configure_client"]'); + expect(message).toContain('"appliedKinds":[]'); +}); diff --git a/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts b/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts new file mode 100644 index 0000000000..f03fbd5ba1 --- /dev/null +++ b/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts @@ -0,0 +1,144 @@ +import { expect, it } from "vitest"; + +import { assertHistoricalPidReadsAreOwnershipOnly } from "../../../scripts/verify/recorded-crash/trace-ownership.mjs"; + +const psExec = + 'execve("/usr/bin/ps", ["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="], 0x7fff) = 0'; + +it("correlates the ps child trace with its parent identity read", () => { + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: [ + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD}, 88) = 1202", + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY|O_CLOEXEC) = 21', + ].join("\n"), + }, + { + pid: 1202, + text: [ + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY|O_CLOEXEC) = 21', + 'openat(AT_FDCWD, "/proc/4008/status", O_RDONLY|O_CLOEXEC) = 22', + ].join("\n"), + }, + { + pid: 1201, + text: [ + psExec, + 'openat(AT_FDCWD, "/proc/4008/status", O_RDONLY) = 3', + 'openat(AT_FDCWD, "/proc/4008/cmdline", O_RDONLY) = 4', + ].join("\n"), + }, + ], + }); + + expect(ownershipReads).toHaveLength(5); +}); + +it("joins unfinished and resumed CLONE_THREAD records", () => { + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: [ + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD, child_tid=0x3 ", + "<... clone3 resumed>) = 1202", + ].join("\n"), + }, + { + pid: 1202, + text: 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3', + }, + { pid: 1201, text: psExec }, + ], + }); + + expect(ownershipReads).toHaveLength(1); +}); + +it("rejects provider-style reads outside the ps child and stat-only caller", () => { + const traces = [ + { + pid: 1200, + text: [ + "vfork() = 1201", + 'openat(AT_FDCWD, "/proc/4008/cmdline", O_RDONLY) = 21', + ].join("\n"), + }, + { pid: 1201, text: psExec }, + ]; + + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + traces, + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("does not trust a ps command with different arguments or failed exec", () => { + for (const text of [ + 'execve("/usr/bin/ps", ["ps", "-ef"], 0x7fff) = 0', + psExec.replace(" = 0", " = -1 EACCES"), + ]) + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: `vfork() = 1201\nopenat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3`, + }, + { pid: 1201, text }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("allows only known procfs fields inside the exact ps child", () => { + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { pid: 1200, text: "vfork() = 1201" }, + { + pid: 1201, + text: `${psExec}\nopenat(AT_FDCWD, "/proc/4008/exe", O_RDONLY) = 3`, + }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("does not grant the ownership process thread permit to a forked child", () => { + for (const field of ["stat", "status"]) + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: ["vfork() = 1201", "clone3({flags=SIGCHLD}, 88) = 1203"].join( + "\n", + ), + }, + { pid: 1201, text: psExec }, + { + pid: 1203, + text: `openat(AT_FDCWD, "/proc/4008/${field}", O_RDONLY) = 3`, + }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +});