[kyle-sexton]

Refs: melodic-software/claude-code-plugins#8295

Summary

The claude-review lane uploads each leg's review handoff and the status job's shadow report as artifacts, which are public on a public repository. This change scans each one with claude-code-plugins' scanLeak and withholds the upload on a hit, so a token cannot leak through them. The claude-code-plugins pull request does the same for that repository's lane and eval uploads.

Fix

• New action .github/actions/scan-upload/ (scan.cjs):
  • Fetches lib/publish-gate.mjs from claude-code-plugins' default-branch tip with an anonymous --depth=1 clone, using /usr/bin/git, an empty config and a minimal environment.
  • Refuses a gate that imports anything other than node: builtins.
  • Scans each file and its name. A hit, link, compressed file, glob or unfetchable gate withholds the upload without printing the value.
• pr-review-cli.yml:
  • A scan step runs before each leg's "Upload the review handoff" and before "Upload the shadow report", over the same path.
  • Each upload's if: now also requires the scan's clean == 'true'.
  • Each scan step adds 1 minute to its job's timeout.

Verification

• pr-review-cli.test.cjs passes 45 of 45. It checks that each upload follows a scan over the same path under the same condition, and that a planted credential-shaped string is withheld without being printed. It also checks that an unloadable gate withholds the upload, a non-builtin import is refused, and the git environment holds no inherited settings.
• pr-review-cli-api-calls.test.cjs passes 10 of 10. biome ci (2.5.11) is clean.
• Two independent security-review rounds found no HIGH or CRITICAL issue. Round 1's P4 (the git environment) is fixed here.

Related

• fix(lanes): secret-scan lane artifacts before upload and shorten their retention claude-code-plugins#8295. The claude-code-plugins half has its own pull request. This workflow is reusable and called @main, so no pin moves.
• timeout-minutes lines here also change in fix(pr-review-cli): review a diff over the read budget in chunks #944. Whichever merges second needs a trivial merge.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-01T01:57:08.954005Z	f891780	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-review reviewed f891780783d8e8e8a87ce774d3f6542bb9e982e4: 0 finding(s), 0 as inline comments.

Code review: PR #945 (scan review handoff and shadow report uploads)

No findings. I posted no inline comments.

What I read: the whole diff (.claude-lane/pr.diff), the PR context, the surrounding upload steps in pr-review-cli.yml and the .claude/rules/pr-body-contract.md rule. That rule governs the PR body and title, not the changed files, so nothing in it applies. There is no settled-threads file, and the PR context names no closing issue (Refs: only), so there are no acceptance criteria to check.

What I checked:

• Scan path matches upload path. The four scan steps (three handoff legs and the shadow report) use the same path as the upload that follows them. The scan's if: equals the upload's if: without the clean clause.
• Failure withholds the upload. clean is set only on a fully clean scan. If the scan step fails, times out or never runs, the composite output is empty, so steps.<id>.outputs.clean == 'true' is false and the upload is skipped. That holds under always() and continue-on-error, so the gate fails closed.
• Walk logic. scan.cjs rejects symlinks, non-regular files, compressed files, globs and excludes. A missing path is left to if-no-files-found, which is error on the handoff and ignore on the shadow report. Directories are walked in sorted order with relative names, and each file's name is scanned as well as its bytes.
• Output and env hardening. Nothing printed carries a matched value; the test asserts this. The git environment is explicit and does not inherit from process.env.
• Timeouts. Each job's timeout goes up by 1 minute to cover its 1-minute scan step.
• $/ prefix. The $/.github/actions/scan-upload prefix is the existing convention in this workflow; check-key-credit and report-api-budget use it too.
• Tests. The tests added or changed for this PR cover each of these.

Not verified:

• I could not run pr-review-cli.test.cjs, because the sandbox denied the command. The 45/45 and 10/10 results in the PR body are the author's claims and I have not confirmed them.
• I did not run the live clone of claude-code-plugins' lib/publish-gate.mjs, so I have not confirmed the --filter=blob:none --no-checkout clone followed by git show works against the real remote.

Residual note, not a finding: checkImports accepts every node: specifier, including node:child_process. It is a guard against stray dependencies, not a sandbox. The gate is code fetched from a sibling repo in the same org and imported into a job that can hold the review credential, so the trust rests on that repo's default branch. That belongs to the security lane.

[github-actions]

claude-security-review reviewed f891780783d8e8e8a87ce774d3f6542bb9e982e4: 0 finding(s), 0 as inline comments.

Security review: PR #945 (head f891780)

No security findings. critical-findings: 0

Change. The PR adds a scan-upload composite action (action.yml, scan.cjs). The review lane runs it before each actions/upload-artifact step in the three review legs and the status job's shadow report. It withholds the upload unless scanLeak from claude-code-plugins' lib/publish-gate.mjs judges every file clean. The PR also adds tests and bumps four job timeouts by 1 minute.

What I checked

• Fail-closed behaviour. clean=true is written to GITHUB_OUTPUT only on the all-clean path (scan.cjs main). Every other outcome returns exit code 1 with no output:

  • a fetch failure, a gate that fails to load, or a gate that exports no scanLeak;
  • a symlink, a non-regular file, a compressed file, an unreadable path, a glob or ! entry;
  • a scanLeak block.

  Each upload's if: requires steps.<scan>.outputs.clean == 'true', and the scan step has continue-on-error: true. A failed scan therefore leaves the output unset and withholds the upload, even under always(). The new workflow test evaluates this for "" and null.

• Whether a model step can force clean=true.

  • The model leg runs without Bash, Edit or Write (disallowed-tools in pr-review-cli.yml).
  • run-claude-cli also cuts off GITHUB_ENV and GITHUB_PATH (lane.cjs) and rejects NODE_OPTIONS, LD_PRELOAD and similar in the step environment.
  • fetchGate runs /usr/bin/git with a minimal environment, no system or global config and core.hooksPath=/dev/null.
  • So I found no route for model-influenced PATH, environment or git config to reach the scan.

• Remote gate code.

  • The action imports lib/publish-gate.mjs from the claude-code-plugins default-branch tip, rejecting any non-node: import by regex.
  • The status job already does the same in its "Fetch the publish gate" step, and the review plugin is already taken from that tip.
  • The scan step's environment holds only SCAN_PATHS.
  • This adds no new trusted source, and I did not report it. The unpinned-ref angle belongs to zizmor.

• Untrusted input into the scan.

  • SCAN_PATHS comes from workflow-authored path: values (runner.temp plus a fixed name), not from the pull request.
  • It reaches the script through env:, not through run: interpolation, so there is no injection.
  • Entries that look like globs fail closed.

• Trust controls. The diff does not touch the trusted-trigger check, token scopes, the kill switch or merge holds. It adds no permissions: and no pull_request_target or workflow_run trigger, and it widens no token, so no trust control is widened. The step uses uses: $/.github/actions/..., the same form as the existing local actions in this file.

• Manifests, instruction surfaces. The diff has no plugin manifests and deletes no standing instructions.

The scan cannot detect an encoded or split credential, and it stops short of a symlinked directory. Both are limits of a content scanner, not defects introduced here.