Repository navigation
fix: refuse Claude reviews of fork pushes made after the comment [COMP-2904] - #741
Open
cristianrcv wants to merge 2 commits into
Open
cristianrcv wants to merge 2 commits into
cristianrcv wants to merge 2 commits into
Conversation
…(COMP-2904) The author_association gate vouches for the commenter, not for the PR head. On a fork PR the author could push a new commit between a maintainer's @claude comment and the job start; the run would then check out and hand Claude code nobody with write access had seen, in a job that holds the Anthropic API key and a pull-requests: write token. Resolving the SHA once only closed the shorter gap between resolving and checkout. The resolve step now stops the run when the fork's pushed_at is at or after the triggering comment or review. pushed_at is set by GitHub on push, unlike commit dates, and is read after the SHA so a push in between also stops the run. Same-repo PRs skip the check because their pushers already have write access, and the automatic pull_request review has no comment to compare against. This is the comment-vs-head date check CodeQL requires for issue_comment checkouts (actions/untrusted-checkout/high, code-scanning alert 15). Values reach the script through env instead of inline expressions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…P-2904) Makes the link between the date-checked resolve step and the checkout explicit, so nobody swaps the ref back to the mutable PR branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cristianrcv
changed the title
cristianrcv
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes code-scanning alert #15 (CodeQL
actions/untrusted-checkout/high, "Checkout of untrusted code in a privileged context", triggerissue_comment) in.github/workflows/claude.yml.The gap. The job's
author_associationgate vouches for the commenter, not for the PR head. On a fork PR, the author can push a new commit between a maintainer's@claudecomment and the job start. The run then checks out code nobody with write access has seen, in a job that holdsENG_ANTHROPIC_API_KEYand apull-requests: writetoken. The existing "resolve the SHA once" step (#681) only closed the shorter window between resolving and checkout.The fix. The resolve step now stops the run on a fork PR when the fork's
pushed_atis at or after the triggering comment or review:pushed_atis set by GitHub on push. Commit dates are set by the author and can be backdated.pull_requestreview has no comment and is already limited to same-repo PRs.env:rather than inline${{ }}expressions.This is the comment-vs-head date check that CodeQL's query requires for
issue_commentcheckouts, in addition to the actor/association check the job already has (UntrustedCheckoutHigh.ql,BashCommentVsHeadDateCheckinControlChecks.qll).Why this and not something bigger. The checkout stays, so the review keeps reading the PR's own files, which was the reason for adding it. Alternatives I considered:
environment:would also satisfy CodeQL, but every@claudewould then wait for a manual approval.Already mitigated, unchanged here. At the pinned version,
claude-code-actionrestores.claude/,.mcp.json,CLAUDE.mdand similar files from the base branch before the CLI starts (src/github/operations/restore-config.ts), so a PR cannot plant hooks or MCP servers. Claude's tools are limited to Read/Grep/Glob/Write and scopedgh prverbs.Behaviour change. If a fork is pushed after a maintainer comments, the run fails with an annotation asking them to review the latest push and mention
@claudeagain. Any push to the fork counts, including other branches and "Sync fork", which errs on the safe side.Verification
actionlint(with shellcheck) passes.date -d <iso> +%sshim (no GNU date or Docker locally):actionsresults, against 1 (alert Manage teams #15) on master. PR analyses here are diff-scoped, so the second commit adds a one-line comment inside the checkout step to bring the alert's own location into the diff (5 diff ranges, including line 104 in the checkout step). Whether that scoping matches on overlap or on start line only is my inference, not documented, so the merge run below is the real proof.The Claude review cannot review this PR itself, because the workflow file differs from the default branch. That is expected.
Other squad repos run the same template (DEVOPS-1775) and probably need the same change.
JIRA
COMP-2904: Fix Checkout of untrusted code in a privileged context
Security Advisory
https://github.com/seqeralabs/tower-cli/security/code-scanning/15
🤖 Generated with Claude Code