[HDX] Harden deep-review workflow for fork PRs

[teeohhem]

Summary

Ports the hardened deep-review workflow from hyperdx. Fork PRs are checked out without a persisted token, and the reviewer is locked to a short allowlist of read-only git and gh subcommands. That list drops helm and gh api, because helm template --post-renderer would let a fork PR run its own code. The job also confirms the local diff matches the PR's file list before reviewing, then skips the whole fan-out on later pushes if the merge-base diff hash hasn't changed.

I kept this repo's Helm prompt and severity rubric. id-token: write stays out (it was removed in #282), and so do the references to hyperdx-only files.

Testing

Not run yet. PR events use main's copy of the workflow, so the only real test before merge is a manual workflow_dispatch run from this branch.

🤖 Generated with Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: 2937bf6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

Deep Review

🔴 P0/P1 -- must fix

• .github/workflows/deep-review.yml:694 -- The allowlisted git log, git show and git diff prefixes still accept --output=<path> and --format=, so a prompt-injected reviewer can write arbitrary bytes into .git/config inside the writable checkout, for example [diff] external = sh -c ... or core.fsmonitor, and the next allowlisted git diff or git status then runs attacker-chosen commands, which contradicts the comment at line 682 saying this is no longer an exec path.
  • Fix: Make the repo config immutable before the review step, for example chattr +i .git/config with GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_NOSYSTEM=1 set, or route git through a trusted wrapper that rejects output-path options, then correct the comment.
  • _{security, correctness}

🟡 P2 -- recommended

• .github/workflows/deep-review.yml:192 -- git diff --name-only runs with the default core.quotePath=true, which C-quotes non-ASCII or special-character paths while the files API returns raw UTF-8, so any PR touching such a filename fails the equality check and gets no review.
  • Fix: Build LOCAL with git -c core.quotePath=false diff --name-only --no-renames "$MB" and emit .filename plus .previous_filename // empty on the API side so both lists match.

🔵 P3 nitpicks (6)

• .github/workflows/deep-review.yml:256 -- The gate takes the first state marker found anywhere in the prior comment, so after an unhealthy run, where the real marker is omitted, a marker-shaped line in the model-written review body can make the gate skip the next push.

  • Fix: Parse only the first line of PRIOR_BODY and anchor MARKER_RE with ^.

• .github/workflows/deep-review.yml:192 -- Rename handling differs between the local list, which uses git's rename detection subject to diff.renameLimit, and the GitHub files API, and the API caps results at 3000 files, so large or rename-heavy PRs can fail the check even when the base is correct.

  • Fix: Use --no-renames locally, include previous_filename from the API, and downgrade the check to a warning when changed_files exceeds 3000.

• .github/workflows/deep-review.yml:242 -- The gate hashes a default -U3 diff but the skill's Stage 1 reviews a -U10 diff, so a main merge that only changes context 4-10 lines from a PR hunk is skipped even though reviewers would see different content.

  • Fix: Hash git diff -U10 --no-color --no-renames "$MB" HEAD so the gate matches what reviewers receive.

• .github/workflows/deep-review.yml:884 -- The <!-- deep-review --> anchor that find-comment and the gate depend on comes only from model output, so a review that drops that line produces a new comment on every push and the gate never finds its prior state.

  • Fix: Strip any leading anchor line from REVIEW and have the extract step always write <!-- deep-review --> itself next to the state marker.

• .github/workflows/deep-review.yml:889 -- always() also fires on cancellation, so a run cancelled by cancel-in-progress after its review step succeeded still posts a stale review and hash, and it can race the newer run into creating a duplicate sticky comment.

  • Fix: Replace always() && with !cancelled() && in the extract, post, and deferred-failure conditions.

• .github/workflows/deep-review.yml:694 -- Removing helm template/helm lint from the allowlist leaves the prompt's P0 criterion (a template that fails to render) with no way to be checked, so reviewers can only guess at it from reading the templates.

  • Fix: Tell the prompt that helm is unavailable, or render the chart in a trusted pre-step and write its output or error into .deep-review/ for the reviewers to read.

---

Reviewers (4): correctness, security, adversarial, reliability

Testing gaps:

• No canary run confirms that the narrowed allowlist blocks file-writing flags such as git log --output=.git/config.
• The round trip between the extract step's marker and the gate's MARKER_RE, including the omitted-marker fail-open path, has not been exercised.
• The file-list check has not been run against a PR with non-ASCII filenames, renames, or more than 3000 files.
• No run has confirmed that the skill's Stage 1 shell commands pass under the new allowlist without permission denials.

[teeohhem]

Addressed the deep review findings in ff69c0b:

A fork PR can commit .deep-review/prior-comments.md (or .deep-review itself) as a symlink, and this privileged pull_request_target step will then overwrite any file the runner user can write.

Fixed differently. The step now refuses to write if .deep-review already exists in the checkout (checked with lstat, so dangling symlinks count), then creates the dir with a non-recursive mkdir and writes the file with flag: 'wx'. I kept the file in the workspace rather than moving it to $RUNNER_TEMP, since that would also need --add-dir for the reviewer and can't be tested before merge. Tested locally against a dir symlink, a dangling symlink, and a file symlink inside a tracked dir. All three are refused and the targets are untouched.

gh pr view --json files returns at most 100 files.

Fixed. Confirmed on kubernetes/kubernetes#142173 (167 files): gh pr view returned 100 and paginated REST returned 167. The check now uses gh api --paginate .../pulls/N/files.

The model ID claude-opus-4-8[1m] is hardcoded both here and in the job-level ANTHROPIC_DEFAULT_SONNET_MODEL

Fixed. --model now reads ${{ env.ANTHROPIC_DEFAULT_SONNET_MODEL }}.