ci: carry the gh.sh digest in a step output so Write cannot forge it

[dwin-gharibi]

Closes #1444.

Motivation

code-review.yml guards scripts/gh.sh with a digest snapshot so the review model cannot swap out the
script that later runs with GH_TOKEN. The snapshot lives at ${RUNNER_TEMP}/gh.sh.sha256 — outside the
workspace, but not outside Write's reach. Write is in --allowedTools, takes absolute paths, and
RUNNER_TEMP is right there in the environment, so writing two files instead of one defeats the check.

What this changes

The digest moves from a file on disk to a step output.

Prefetch step gains id: prefetch and records the digest with
echo "gh_sh_sha256=$(sha256sum scripts/gh.sh | cut -d" " -f1)" >> "${GITHUB_OUTPUT}".

Publish step receives it via env: EXPECTED_GH_SH_SHA256: ${{ steps.prefetch.outputs.gh_sh_sha256 }},
recomputes the digest, and compares.

This works because Actions captures a step's outputs when that step ends and stores them in the workflow
context. By the time the model runs, the value is no longer read from disk — appending to GITHUB_OUTPUT
during a later step affects only that step's own outputs, which nothing consumes. There is no longer any
file whose contents the check depends on.

Two smaller changes in the same block:

• An empty EXPECTED_GH_SH_SHA256 is now an explicit failure rather than a comparison against "", so a
  future refactor that drops the id: or renames the output fails closed instead of silently.
• The mismatch branch prints expected and actual, which sha256sum -c did not.

The value is passed through env: rather than interpolated into the script body. It is a hex digest from a
trusted checkout so injection was never the concern, but env: is the right habit in a
pull_request_target workflow.

The comment on the snapshot line was rewritten because the old wording ("outside the workspace") describes a
mechanism this PR removes. It is the same two lines; no new commentary.

Testing

The workflow's own two run: blocks were extracted from the YAML with yaml.safe_load and executed
verbatim against a simulated runner: real RUNNER_TEMP and GITHUB_OUTPUT, a stub gh on PATH, a real
scripts/gh.sh that prints PUBLISHED, and a tamper step between them standing in for the model's Write
tool. Step-output capture is modelled faithfully — outputs are read when the prefetch step ends, and the
tamper attempts to append to GITHUB_OUTPUT afterwards, exactly as the attacker would.

=== MASTER ===
  no tampering                                    exit=0  PUBLISHED by the real scripts/gh.sh
  Write overwrites scripts/gh.sh only             exit=1  sha256sum: WARNING: 1 computed checksum did NOT match
  Write overwrites gh.sh AND the digest snapshot  exit=0  *** PWNED: attacker-controlled script ran with GH_TOKEN ***

=== THIS BRANCH ===
  no tampering                                    exit=0  PUBLISHED by the real scripts/gh.sh
  Write overwrites scripts/gh.sh only             exit=1    actual   cbe5346a4d50a152c...
  Write overwrites gh.sh AND the digest snapshot  exit=1    actual   cbe5346a4d50a152c...

Row 1 is the regression guard — the normal path still publishes. Row 2 shows the existing control still
works. Row 3 is the fix.

yaml.safe_load on the workflow parses clean, and no RUNNER_TEMP reference remains in the file.

CI gates: .github/workflows/ is not covered by fmt-check or unit-test-check, and the repo has no
actionlint or yamllint step, so there is no gate to satisfy beyond the file being valid YAML. This
workflow only runs on pull_request_target, so it cannot be exercised until the PR exists.

Not fixed here

The stronger fix the issue mentions — dropping Write entirely and having the model return the review body
as a step output — depends on what anthropics/claude-code-action@v1 supports for structured output. That
is worth pursuing, and it would let both the digest check and the review-input/summary.md marker checks go
away, but it is a redesign of the publish path rather than a fix to a broken control.

Also unchanged: Write is still unconfined, so it can still reach absolute paths under RUNNER_TEMP and
elsewhere on the runner. This PR removes the one place where that mattered. If the action ever grows a
workspace-confinement option for Write, that is the real boundary and should be turned on.

Risk / rollout

Low. Same check, same failure message, one fewer file. The only behavioural difference on the happy path is
that the publish step prints two extra lines when the digest does not match.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cubesandboxbot]

The fix is correct for the hole it targets: step outputs are captured by the runner when the prefetch step ends and are not filesystem-reachable from a later step, so Write can no longer forge the expected digest.

One residual gap worth being explicit about (pre-existing, and acknowledged in the PR description): this check only vouches for scripts/gh.sh's own bytes. scripts/gh.sh invokes bare gh, gh api, and python3 resolved via PATH (lines 89/95/112/175/206/218). Because Write is still unconfined, a compromised model could plant a malicious gh/python3 into a writable, PATH-fronted directory — on hosted runners /home/runner/.local/bin is first on PATH and writable by the runner user — and the untampered scripts/gh.sh would then execute it with GH_TOKEN in the environment while this check passes. That doesn't mean the fix is wrong; it means the integrity check narrows but does not eliminate the "attacker-controlled code runs with GH_TOKEN" outcome. The durable boundary remains confining Write to the workspace, or hashing the dependency chain gh.sh actually execs.

[cubesandboxbot]

AI-generated review — produced by an automated review agent; no human approval is implied.

Overview

PR #1445 hardens the pull_request_target code-review workflow against the review model's Write tool forging the scripts/gh.sh integrity snapshot. Previously the expected SHA-256 was stored at ${RUNNER_TEMP}/gh.sh.sha256; since Write accepts absolute paths and RUNNER_TEMP is in the environment, the model could overwrite both scripts/gh.sh and the snapshot, making the check a no-op. This PR moves the digest into a step output (steps.prefetch.outputs.gh_sh_sha256), captured by the runner when the prefetch step ends and not reachable via the filesystem from a later step.

Verdict

The fix is correct for the hole it targets. GitHub Actions step outputs are stored in the runner's internal job context at step completion; the model step runs after prefetch, and its Write cannot retroactively alter the prefetch step's captured output. The publish step recomputes the digest of the on-disk scripts/gh.sh and compares against that captured value, so "overwrite both the script and the snapshot" no longer defeats the check. The added empty-value guard fails closed, and printing expected/actual on mismatch is strictly more diagnostic than sha256sum -c. The command used (sha256sum … | cut -d" " -f1) is consistent between the two steps and correct for sha256sum's two-space text-mode output on the ubuntu-latest runner.

Key residual risk (not introduced by this PR, but directly relevant to its goal)

The check only vouches for scripts/gh.sh's own bytes. scripts/gh.sh executes bare gh, gh api, and python3 resolved via PATH (scripts/gh.sh:89, 95, 112, 175, 206, 218). Because Write remains unconfined, a compromised review model could plant a malicious gh/python3 in a writable, PATH-fronted directory (on GitHub-hosted runners /home/runner/.local/bin is first on PATH and writable by the runner user), and the untampered scripts/gh.sh would then run that code with GH_TOKEN in its environment while this check passes. The durable fix remains confining Write to the workspace — as the PR description itself notes — or hashing the dependency chain gh.sh actually execs. This PR correctly closes the snapshot-forgery hole but does not yet establish the stronger guarantee that whatever runs with GH_TOKEN is trusted.

Minor nits

• Under set -u, if the env: block supplying EXPECTED_GH_SH_SHA256 were ever removed entirely (rather than the value being empty), the shell would exit with an "unbound variable" error before reaching the friendly [ -z … ] message. Fail-closed either way, so cosmetic only.
• The trusted digest is now echoed to the Actions log on mismatch; harmless for a public-repo file hash.

Verification notes

The PR's test matrix (master "PWNED" row vs. branch failure rows) matches the expected behavior of the two mechanisms, and the YAML parses cleanly. No RUNNER_TEMP / gh.sh.sha256 references remain in the workflow.