diff --git a/.changes/pr-pipeline/fix-9922-capture-probe-never-tried-e6f07c02.md b/.changes/pr-pipeline/fix-9922-capture-probe-never-tried-e6f07c02.md new file mode 100644 index 0000000000..2649a671ae --- /dev/null +++ b/.changes/pr-pipeline/fix-9922-capture-probe-never-tried-e6f07c02.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- "Check the probe" in the PR capture, and the subprocess scrub probe in `upstream-apply-changelog.yml`, now say the fence was not tested when the probe session made no Read call on a fenced path, instead of reporting the file tools as unfenced. `capture-probe.mjs reads` exits 3 for that case. Both still fail the job, and any Read that came back or got no answer is still reported as an open fence. diff --git a/.github/scripts/pr-verify/capture-probe.mjs b/.github/scripts/pr-verify/capture-probe.mjs index 84c1b01df9..4d4bdea987 100644 --- a/.github/scripts/pr-verify/capture-probe.mjs +++ b/.github/scripts/pr-verify/capture-probe.mjs @@ -153,7 +153,10 @@ export function fileReach(messages, targets) { } // `reads ...`: exits 0 only when every path's Read was -// refused, and prints each path's status, never a file's content. +// refused; 1 when any Read came back or went unanswered, whatever else went +// untried; 3 when none did but a path was never tried, so the session left +// the fence untested rather than showing it open. Prints each path's status, +// never a file's content. function reads(argv) { let messages; try { @@ -164,7 +167,13 @@ function reads(argv) { } const { clean, reach } = fileReach(messages, argv.slice(4)); console.log(`capture-probe: driver file tools: ${reach.map(({ target, status }) => `${target} ${status}`).join(", ")}`); - return clean ? 0 : 1; + if (clean) return 0; + if (reach.some(({ status }) => status === "read" || status === "unknown")) return 1; + if (reach.some(({ status }) => status === "not tried")) { + console.log("capture-probe: the session made no Read call on a path above, so the fence was not tested there"); + return 3; + } + return 1; } export async function main({ env = process.env, cwd = process.cwd(), procRoot = "/proc", argv = process.argv } = {}) { diff --git a/.github/scripts/pr-verify/capture-probe.test.mjs b/.github/scripts/pr-verify/capture-probe.test.mjs index 86e418db93..9014d9bde4 100644 --- a/.github/scripts/pr-verify/capture-probe.test.mjs +++ b/.github/scripts/pr-verify/capture-probe.test.mjs @@ -156,6 +156,33 @@ test("the reads subcommand exits 0 only on a refused read of every path, and pri assert.equal(run().status, 1); }); +test("the reads subcommand exits 3, not 1, when the session never tried a path, and 1 whenever any Read came back", () => { + const dir = tempDir("capture-probe-untried-"); + const file = path.join(dir, "execution.json"); + const run = (messages) => { + writeFileSync(file, JSON.stringify(messages)); + return spawnSync(process.execPath, [SCRIPT, "reads", file, ...TARGETS], { encoding: "utf8" }); + }; + const bash = { type: "assistant", message: { content: [{ type: "tool_use", id: "z", name: "Bash", input: { command: "true" } }] } }; + for (const [messages, why] of [ + [[], "no message at all"], + [[bash, resultMessage()], "a Bash call and no Read"], + ]) { + const none = run(messages); + assert.equal(none.status, 3, `${why}: ${none.stdout}`); + assert.match(none.stdout, /\/proc\/self\/environ not tried, \/runner\/_temp\/probe\.md not tried/, why); + assert.match(none.stdout, /the fence was not tested/, why); + } + const partial = run([readUse("a", TARGETS[0]), readResult("a", true)]); + assert.equal(partial.status, 3, partial.stdout); + assert.match(partial.stdout, /\/runner\/_temp\/probe\.md not tried/); + // A Read that came back, or one with no result, is a breach whatever else went untried. + const read = run([readUse("b", TARGETS[1]), readResult("b", false)]); + assert.equal(read.status, 1, read.stdout); + assert.doesNotMatch(read.stdout, /not tested/); + assert.equal(run([readUse("b", TARGETS[1])]).status, 1); +}); + test("run where a key is in reach, the probe fails, names it and writes no value", { skip: !existsSync("/proc/self/environ") && "needs /proc" }, () => { const dir = tempDir("capture-probe-run-"); const result = path.join(dir, "result.json"); diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index 3e093a8369..85ab53508e 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -1651,8 +1651,36 @@ test("the driver's file tools are fenced to the head and the run directory, and assert.match(probePrompt, /^ {12}echo " {4}\$FENCE_LINK"$/m); assert.match( capStep(PROBE_CHECK), - /capture-probe\.mjs reads "\$EXECUTION_FILE" \/proc\/self\/environ "\$PROMPT_FILE" "\$HOME\/\.claude\/pr-verify-probe-fence" "\$GITHUB_WORKSPACE\/capture-run\/probe-environ-link" "\$GITHUB_WORKSPACE\/capture-run\/probe-fence-link"; then\n {12}echo "::error::.*"\n {12}exit 1$/m, + /^ {10}node \.base\/\.github\/scripts\/pr-verify\/capture-probe\.mjs reads "\$EXECUTION_FILE" \/proc\/self\/environ "\$PROMPT_FILE" "\$HOME\/\.claude\/pr-verify-probe-fence" "\$GITHUB_WORKSPACE\/capture-run\/probe-environ-link" "\$GITHUB_WORKSPACE\/capture-run\/probe-fence-link" \|\| reads=\$\?\n {10}case \$reads in\n {12}0\) ;;\n {12}3\)\n {14}echo "::error::.*"\n {14}exit 1\n {14};;\n {12}\*\)\n {14}echo "::error::.*"\n {14}exit 1\n {14};;\n {10}esac$/m, ); + // Run against hand-built transcripts: a session with no Read call fails as an + // untested fence, a Read that came back fails as an open one. + const checkDir = tempDir("capture-probe-check-reads-"); + const scripts = path.join(checkDir, ".base/.github/scripts/pr-verify"); + mkdirSync(scripts, { recursive: true }); + writeFileSync(path.join(scripts, "capture-probe.mjs"), readFileSync(new URL("./capture-probe.mjs", import.meta.url))); + const probeResult = path.join(checkDir, "capture-probe.json"); + writeFileSync(probeResult, JSON.stringify({ clean: true, canary: true, hits: [] })); + const transcript = path.join(checkDir, "execution.json"); + const checkWith = (messages) => { + writeFileSync(transcript, JSON.stringify(messages)); + return spawnSync("bash", ["--noprofile", "--norc", "-eo", "pipefail", "-c", scriptOf(capStep(PROBE_CHECK))], { + cwd: checkDir, + encoding: "utf8", + env: { PATH: process.env.PATH, HOME: checkDir, GITHUB_WORKSPACE: checkDir, PROBE_OUTCOME: "success", RESULT: probeResult, EXECUTION_FILE: transcript, PROMPT_FILE: "/runner/_temp/probe.md" }, + }); + }; + const untried = checkWith([]); + assert.equal(untried.status, 1, untried.stdout + untried.stderr); + assert.match(untried.stdout, /^::error::the probe's session made no Read call on a fenced path, so the driver's file-tool fence was not tested; no pull request code runs$/m); + assert.doesNotMatch(untried.stdout, /not fenced off/); + const leaked = checkWith([ + { type: "assistant", message: { content: [{ type: "tool_use", id: "r", name: "Read", input: { file_path: "/proc/self/environ" } }] } }, + { type: "user", message: { content: [{ type: "tool_result", tool_use_id: "r", content: "x" }] } }, + ]); + assert.equal(leaked.status, 1, leaked.stdout + leaked.stderr); + assert.match(leaked.stdout, /^::error::the driver's file tools are not fenced off \/proc, the runner's temp directory and ~\/\.claude, directly or through a link; no pull request code runs$/m); + assert.doesNotMatch(leaked.stdout, /not tested/); // Every launcher step carries the same fence, the probe's and the driver's alike. const launchers = [...CAP_STEPS, ...SUB_STEPS].filter((s) => /uses: melodic-software\/ci-workflows\/\.github\/actions\/run-claude-cli@/.test(s.block)); assert.equal(launchers.length, 2); diff --git a/.github/scripts/upstream-apply-changelog/upstream-apply-changelog.test.mjs b/.github/scripts/upstream-apply-changelog/upstream-apply-changelog.test.mjs index 3ffc1bb8ab..68291a23bd 100644 --- a/.github/scripts/upstream-apply-changelog/upstream-apply-changelog.test.mjs +++ b/.github/scripts/upstream-apply-changelog/upstream-apply-changelog.test.mjs @@ -354,7 +354,13 @@ test("the check names the release and hit names on every failure, never a value, fails(check({ result: clean, env: { LEG_OUTCOME: "failure", LEG_CLASS: "install-failed" } }), "a failed leg, even with a clean result on disk"); fails(check({ result: clean, env: { CLI_VERSION: "2.1.319" } }), "another CLI"); leg("read"); - fails(check({ result: clean }), "a Read that came back"); + const read = check({ result: clean }); + fails(read, "a Read that came back"); + assert.match(read.stdout, /file tools were not refused/); + writeFileSync(EXECUTION_FILE, "[]"); + const untried = check({ result: clean }); + fails(untried, "a leg that made no Read call"); + assert.match(untried.stdout, /^::error::Claude Code 2\.1\.320: the subprocess scrub probe failed: the leg made no Read call on \/proc or the runner's temp directory, so its file-tool fence was not tested$/m); }); const SELECT = /^ {8}uses: melodic-software\/ci-workflows\/\.github\/actions\/select-model-credential@[0-9a-f]{40} # \S/m; diff --git a/.github/workflows/pr-verify-capture.yml b/.github/workflows/pr-verify-capture.yml index e7a0d243c2..45527a5dd2 100644 --- a/.github/workflows/pr-verify-capture.yml +++ b/.github/workflows/pr-verify-capture.yml @@ -977,7 +977,8 @@ jobs: # needs a Read call that was refused (capture-probe.mjs reads), and the # Write and Grep through links a refused call too, with nothing left # where the Write would have landed, so a fence that let one through, - # or a leg that never tried, fails the job. + # or a leg that never tried, fails the job. A leg that never tried a + # Read (reads exits 3) is reported as an untested fence, not an open one. - name: Check the probe id: probe-check if: steps.credential.outputs.present == 'true' && steps.probe.outputs.handoff != 'true' @@ -1000,10 +1001,19 @@ jobs: echo "::error::head code could reach a credential or token, or the probe could not show its scan works (${reach:-no readable result}); no pull request code runs" exit 1 fi - if ! node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE" "$HOME/.claude/pr-verify-probe-fence" "$GITHUB_WORKSPACE/capture-run/probe-environ-link" "$GITHUB_WORKSPACE/capture-run/probe-fence-link"; then - echo "::error::the driver's file tools are not fenced off /proc, the runner's temp directory and ~/.claude, directly or through a link; no pull request code runs" - exit 1 - fi + reads=0 + node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE" "$HOME/.claude/pr-verify-probe-fence" "$GITHUB_WORKSPACE/capture-run/probe-environ-link" "$GITHUB_WORKSPACE/capture-run/probe-fence-link" || reads=$? + case $reads in + 0) ;; + 3) + echo "::error::the probe's session made no Read call on a fenced path, so the driver's file-tool fence was not tested; no pull request code runs" + exit 1 + ;; + *) + echo "::error::the driver's file tools are not fenced off /proc, the runner's temp directory and ~/.claude, directly or through a link; no pull request code runs" + exit 1 + ;; + esac # The Write and the Grep, read from the transcript as capture-probe.mjs # reads a Read: at least one call on the path, and every one refused # (listed in permission_denials, or answered with an error). diff --git a/.github/workflows/upstream-apply-changelog.yml b/.github/workflows/upstream-apply-changelog.yml index 57fc299f31..66a4efffe2 100644 --- a/.github/workflows/upstream-apply-changelog.yml +++ b/.github/workflows/upstream-apply-changelog.yml @@ -1452,9 +1452,13 @@ jobs: reach=$(jq -r '[.hits[]?.name] | unique | join(", ")' "$RESULT" 2>/dev/null || true) fail "a Bash child could reach a credential or token, or the scan could not show it works (${reach:-no hit named})" fi - if ! node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE"; then - fail "the leg's file tools were not refused /proc and the runner's temp directory" - fi + reads=0 + node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE" || reads=$? + case $reads in + 0) ;; + 3) fail "the leg made no Read call on /proc or the runner's temp directory, so its file-tool fence was not tested" ;; + *) fail "the leg's file tools were not refused /proc and the runner's temp directory" ;; + esac echo "::notice::Claude Code $RELEASE: the subprocess scrub probe passed" echo "Claude Code $RELEASE: the subprocess scrub probe passed" >>"$GITHUB_STEP_SUMMARY" @@ -1676,9 +1680,13 @@ jobs: reach=$(jq -r '[.hits[]?.name] | unique | join(", ")' "$RESULT" 2>/dev/null || true) fail "a Bash child could reach a credential or token, or the scan could not show it works (${reach:-no hit named})" fi - if ! node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE"; then - fail "the leg's file tools were not refused /proc and the runner's temp directory" - fi + reads=0 + node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE" || reads=$? + case $reads in + 0) ;; + 3) fail "the leg made no Read call on /proc or the runner's temp directory, so its file-tool fence was not tested" ;; + *) fail "the leg's file tools were not refused /proc and the runner's temp directory" ;; + esac echo "::notice::Claude Code $RELEASE: the subprocess scrub probe passed" echo "Claude Code $RELEASE: the subprocess scrub probe passed" >>"$GITHUB_STEP_SUMMARY" @@ -1895,9 +1903,13 @@ jobs: reach=$(jq -r '[.hits[]?.name] | unique | join(", ")' "$RESULT" 2>/dev/null || true) fail "a Bash child could reach a credential or token, or the scan could not show it works (${reach:-no hit named})" fi - if ! node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE"; then - fail "the leg's file tools were not refused /proc and the runner's temp directory" - fi + reads=0 + node .base/.github/scripts/pr-verify/capture-probe.mjs reads "$EXECUTION_FILE" /proc/self/environ "$PROMPT_FILE" || reads=$? + case $reads in + 0) ;; + 3) fail "the leg made no Read call on /proc or the runner's temp directory, so its file-tool fence was not tested" ;; + *) fail "the leg's file tools were not refused /proc and the runner's temp directory" ;; + esac echo "::notice::Claude Code $RELEASE: the subprocess scrub probe passed" echo "Claude Code $RELEASE: the subprocess scrub probe passed" >>"$GITHUB_STEP_SUMMARY"