-
Notifications
You must be signed in to change notification settings - Fork 1
chore: Cherry-picked changes from upstream #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
df38f05
chore: Cherry-picked changes from upstream
Raj-StepSecurity 0404374
conflicted commits cherry-picked
Raj-StepSecurity 1931054
upstream author removed
Raj-StepSecurity e435865
comments addressed
Raj-StepSecurity 011aa80
comments addressed
Raj-StepSecurity b429c5a
prettier issue fix
Raj-StepSecurity eee562a
subscription check added
Raj-StepSecurity 6c8aafa
downgraded 1 version
Raj-StepSecurity a20ff98
downgraded 1 version
Raj-StepSecurity 2427dfb
downgraded 1 version
Raj-StepSecurity 3060f0e
cherry-pick removed
Raj-StepSecurity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| export const meta = { | ||
| name: "pr-stamp-sweep", | ||
| description: | ||
| "Review candidate PRs for stampability, then adversarially verify security of stamp candidates", | ||
| whenToUse: | ||
| "Sweep candidate PRs for stampability: per-PR review + adversarial security verify. Requires pre-fetched PR dossiers in /tmp/claude/pr-sweep/<n>.md and args {prs: [...]}.", | ||
| phases: [ | ||
| { title: "Review", detail: "one reviewer agent per PR" }, | ||
| { | ||
| title: "Verify", | ||
| detail: "adversarial security skeptic per stamp candidate", | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| // PRECONDITION: before invoking this workflow, pre-fetch each candidate PR to | ||
| // /tmp/claude/pr-sweep/<n>.md, containing the PR's metadata, body, existing | ||
| // reviews/comments, and the full diff (e.g. via `gh pr view` + `gh pr diff`). | ||
| // Sandboxed agents can't reliably call gh themselves, so they read these | ||
| // dossier files instead. Pass the PR numbers as args: {prs: [<PR numbers>]}. | ||
|
|
||
| const REVIEW_SCHEMA = { | ||
| type: "object", | ||
| properties: { | ||
| number: { type: "number" }, | ||
| verdict: { type: "string", enum: ["stamp", "skip", "needs-discussion"] }, | ||
| category: { | ||
| type: "string", | ||
| description: "docs | tests | bugfix | nicety | security-fix | other", | ||
| }, | ||
| summary: { | ||
| type: "string", | ||
| description: "1-2 sentence plain-language summary of what the PR does", | ||
| }, | ||
| reasoning: { | ||
| type: "string", | ||
| description: "why this verdict — correctness, scope, quality", | ||
| }, | ||
| behaviorChange: { | ||
| type: "string", | ||
| description: 'what user-visible behavior changes, or "none"', | ||
| }, | ||
| concerns: { type: "array", items: { type: "string" } }, | ||
| securitySensitive: { | ||
| type: "boolean", | ||
| description: | ||
| "true if it touches auth, sanitization, parsers of untrusted input, actor checks, file restore, or shell construction", | ||
| }, | ||
| duplicateOf: { | ||
| type: "string", | ||
| description: "PR number(s) this duplicates, or empty string", | ||
| }, | ||
| }, | ||
| required: [ | ||
| "number", | ||
| "verdict", | ||
| "category", | ||
| "summary", | ||
| "reasoning", | ||
| "behaviorChange", | ||
| "concerns", | ||
| "securitySensitive", | ||
| "duplicateOf", | ||
| ], | ||
| }; | ||
|
|
||
| const VERDICT_SCHEMA = { | ||
| type: "object", | ||
| properties: { | ||
| number: { type: "number" }, | ||
| safeToStamp: { type: "boolean" }, | ||
| findings: { | ||
| type: "array", | ||
| items: { type: "string" }, | ||
| description: | ||
| "concrete security/correctness problems found, empty if clean", | ||
| }, | ||
| confidence: { type: "string", enum: ["high", "medium", "low"] }, | ||
| }, | ||
| required: ["number", "safeToStamp", "findings", "confidence"], | ||
| }; | ||
|
|
||
| if (!args || !Array.isArray(args.prs) || args.prs.length === 0) | ||
| throw new Error( | ||
| "pass {prs: [<PR numbers>]} as args; pre-fetch each PR to /tmp/claude/pr-sweep/<n>.md first", | ||
| ); | ||
| const prs = args.prs; | ||
|
|
||
| log(`Reviewing ${prs.length} candidate PRs`); | ||
|
|
||
| const results = await pipeline( | ||
| prs, | ||
| (n) => | ||
| agent( | ||
| `You are reviewing open PR #${n} on step-security/claude-code-action to decide if it is safe for a maintainer to approve ("stamp") with minimal further discussion. | ||
|
|
||
| The full PR (metadata, body, existing reviews/comments, and complete diff) is in /tmp/claude/pr-sweep/${n}.md — read it first. The repo is checked out at the current working directory. Read the actual current source files the diff touches to verify the diff applies cleanly conceptually and the claims in the PR body are true. Do NOT modify anything or run git commands that change state. | ||
|
|
||
| Context about this repo: | ||
| - It's a GitHub Action that runs Claude on issues/PRs. It processes UNTRUSTED content (PR bodies, comments, branch names, file contents from forks). Treat any change touching content sanitization, actor/bot allowlists, config restoration, prompt construction, or shell command construction as high-risk. | ||
| - Most candidate PRs are from EXTERNAL contributors. Treat the diff with suspicion: look for subtle malicious changes, weakened validation, injection vectors, overly broad permissions, or changes whose description doesn't match the code. | ||
| - Runtime is Bun; strict TypeScript (noUnusedLocals/noUnusedParameters). Tests are unit tests run with bun test. | ||
|
|
||
| Stamp criteria (ALL must hold): | ||
| 1. Small, focused, and the code does exactly what the title/body says. | ||
| 2. No major behavior change — bug fixes restoring intended behavior, docs fixes, test-only additions, and small niceties qualify. New inputs/features, behavior redesigns, or large refactors do NOT. | ||
| 3. Correct: you verified the logic against the actual current source, not just the diff. Check edge cases. | ||
| 4. No security concern. Check explicitly for: prompt injection (untrusted text reaching Claude's prompt without sanitization), code execution (untrusted data reaching shell commands, eval/spawn, or GitHub workflow expressions), path traversal (untrusted input influencing filesystem paths), credential exposure (tokens reaching logs, comments, or attacker-readable output), weakened validation or permission checks, and suspicious hunks unrelated to the stated purpose. | ||
| 5. Wouldn't break the public API of base-action/ or action.yml output wiring. | ||
|
|
||
| If the PR is a docs change, verify the docs claims against the actual code behavior. If test-only, check tests actually pass conceptually (assert the right things, match real implementations) and don't weaken or skip anything. | ||
|
|
||
| Verdicts: "stamp" = approve as-is; "needs-discussion" = plausible but has questions/issues worth a comment; "skip" = too big, wrong, redundant, or risky. | ||
|
|
||
| If this PR appears to duplicate another open PR (same fix, same files), still judge it on its own merits but note the duplication in duplicateOf. | ||
|
|
||
| Return structured output only.`, | ||
| { label: `review:#${n}`, phase: "Review", schema: REVIEW_SCHEMA }, | ||
| ), | ||
| (review, n) => { | ||
| if (!review) return null; | ||
| if (review.verdict !== "stamp") return { review, verify: null }; | ||
| return agent( | ||
| `You are an adversarial security skeptic. Another reviewer recommended APPROVING open PR #${n} on step-security/claude-code-action. Your job is to REFUTE that recommendation — find any reason it should NOT be stamped. | ||
|
|
||
| Their assessment: ${JSON.stringify(review)} | ||
|
|
||
| Read the full PR at /tmp/claude/pr-sweep/${n}.md and the touched source files in the current working directory. This repo processes untrusted PR/issue content from forks; anything that lets untrusted content reach Claude's prompt, a shell command, a workflow expression, or a filesystem path unsanitized is a critical vulnerability. | ||
|
|
||
| Hunt specifically for: | ||
| - Subtle malice or scope creep: hunks that don't match the stated purpose, weakened validation, regex changes that widen acceptance, removed escaping. | ||
| - Prompt injection: untrusted data (comment bodies, branch names, file contents, command output, downloaded files) reaching Claude's prompt or context without sanitization, including indirect routes like tool output Claude later reads. | ||
| - Code execution: untrusted data reaching shell commands, eval/spawn argv, GitHub workflow \${{ }} expressions, or API call templates; new process spawning; path traversal letting untrusted input write or read outside intended directories. | ||
| - Credential exposure: tokens or secrets flowing into logs, posted comments, error messages, env passed to untrusted code, or files Claude can read. | ||
| - Logic errors the first reviewer missed: off-by-one, wrong polarity, unhandled edge cases (empty strings, unicode, very long inputs). | ||
| - Supply-chain angles: pinned versions that don't match the claimed SHA/tag, new dependencies, fetched URLs. | ||
| - For docs PRs: claims that would mislead users into insecure configurations. | ||
| - For test-only PRs: tests that codify wrong behavior, or that would mask future regressions. | ||
|
|
||
| If the diff pins a version/SHA, verify the claim is plausible from local information; flag if unverifiable. Be strict: if uncertain whether something is a real problem, lean toward reporting it as a finding with your uncertainty noted. Only return safeToStamp=true if you genuinely failed to find any disqualifying issue. | ||
|
|
||
| Return structured output only.`, | ||
| { label: `verify:#${n}`, phase: "Verify", schema: VERDICT_SCHEMA }, | ||
| ).then((v) => ({ review, verify: v })); | ||
| }, | ||
| ); | ||
|
|
||
| const clean = results.filter(Boolean); | ||
| const stamped = clean.filter( | ||
| (r) => r.review.verdict === "stamp" && r.verify && r.verify.safeToStamp, | ||
| ); | ||
| const demoted = clean.filter( | ||
| (r) => r.review.verdict === "stamp" && (!r.verify || !r.verify.safeToStamp), | ||
| ); | ||
| const discuss = clean.filter((r) => r.review.verdict === "needs-discussion"); | ||
| const skipped = clean.filter((r) => r.review.verdict === "skip"); | ||
|
|
||
| log( | ||
| `stamp: ${stamped.length}, demoted by verifier: ${demoted.length}, needs-discussion: ${discuss.length}, skip: ${skipped.length}`, | ||
| ); | ||
|
|
||
| return { stamped, demoted, discuss, skipped }; |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| # Agent Approval Check | ||
|
|
||
| Require **N human approvals** on any pull request that contains commits | ||
| authored by an AI agent (Claude, Claude Code, or any bot identity you | ||
| configure). PRs without agent activity are unaffected. | ||
|
|
||
| This is the same gate Anthropic runs internally on every agent-authored PR. | ||
|
|
||
| ## What it does | ||
|
|
||
| When a PR is opened, pushed to, or commented on, this action: | ||
|
|
||
| 1. Scans the PR's commits, author, and reviews for the configured agent | ||
| identities (committer email, bot login, or an `APPROVED` review from a | ||
| bot). If none are found it posts `success: No agent activity` and stops. | ||
| 2. Counts distinct human approvals: the latest `APPROVED` review per login, | ||
| plus any `/approve <head-sha>` comment whose SHA matches the current | ||
| head. Only users with write access to the repo count (verified per-user | ||
| via the collaborators permission API); agent and excluded-bot logins | ||
| never count. | ||
| 3. Posts an `agent-approval-check` commit status (`success` once the count | ||
| reaches `required_approvals`, otherwise `pending`) and a sticky PR | ||
| comment explaining what's still needed. | ||
| 4. Re-evaluates on every new push or comment. A push moves the head SHA, | ||
| so earlier `/approve <old-sha>` comments are flagged stale. Approving | ||
| reviews still count toward the threshold — they're picked up the next | ||
| time the workflow runs (on push or `/approve`); they just don't trigger | ||
| a run on their own. | ||
|
|
||
| Mark `agent-approval-check` as a **required status check** on your protected | ||
| branches and GitHub will refuse to merge until it's green. | ||
|
|
||
| ## Setup | ||
|
|
||
| Copy [`examples/agent-approval-check.yml`](../examples/agent-approval-check.yml) | ||
| into `.github/workflows/` in your repo, then add `agent-approval-check` to the | ||
| required status checks on your protected branch. | ||
|
|
||
| This action is designed to run **alongside** GitHub's native branch | ||
| protection, not replace it. On the same protected branch you should also: | ||
|
|
||
| 1. Require at least 1 approving review from someone with write access. | ||
| 2. Enable **Dismiss stale pull request approvals when new commits are pushed**. | ||
|
|
||
| ```yaml | ||
| name: agent-approval-check | ||
| on: | ||
| pull_request_target: | ||
| types: [opened, synchronize, reopened, ready_for_review] | ||
| issue_comment: | ||
| types: [created] | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| statuses: write | ||
| jobs: | ||
| check: | ||
| if: github.event_name != 'issue_comment' || github.event.issue.pull_request | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: step-security/claude-code-action/agent-approval-check@v1 | ||
| with: | ||
| required_approvals: 2 | ||
| agent_emails: noreply@anthropic.com | ||
| agent_logins: claude[bot],claude-code[bot] | ||
| ``` | ||
|
|
||
| ## Inputs | ||
|
|
||
| | Input | Default | Meaning | | ||
| | ---------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `required_approvals` | `2` | Distinct human approvals needed. | | ||
| | `agent_emails` | `noreply@anthropic.com` | Committer emails that mark a commit agent-authored. | | ||
| | `agent_logins` | `claude[bot],claude-code[bot]` | Logins treated as agents (PR author or approving reviewer). | | ||
| | `excluded_approvers` | _(empty)_ | Logins whose approvals never count. | | ||
| | `exempt_head_branches` | _(empty)_ | Head-branch globs that auto-pass. ⚠️ Leave empty — branch names are attacker-controlled, so this is not a safe place to encode trust. | | ||
| | `exempt_path_prefixes` | _(empty)_ | PRs touching only these prefixes auto-pass. | | ||
| | `protected_bases` | _(default branch)_ | Base branches this check gates (see threat model). | | ||
| | `config_file` | _(empty)_ | Path to an [agent-identities YAML](./agent-identities.example.yaml) replacing the inline inputs. See the warning below. | | ||
| | `docs_url` | this README | Link in the PR comment footer. | | ||
| | `github_token` | `${{ github.token }}` | Needs `statuses:write` + `pull-requests:write`. | | ||
|
|
||
| > ⚠️ **`config_file` and checkout:** if you set `config_file`, your workflow | ||
| > must check out the **base** branch to read it (the default behaviour of | ||
| > `actions/checkout` under `pull_request_target`). Never check out the PR | ||
| > head ref — doing so would let the PR author control the config and bypass | ||
| > this check. | ||
|
|
||
| ## Approving | ||
|
|
||
| A human counts as an approver by either: | ||
|
|
||
| - submitting a normal GitHub **Approve** review, or | ||
| - commenting `/approve <sha>` where `<sha>` is the current head commit | ||
| (12–40 hex chars). This path lets the PR author — who can't approve their | ||
| own PR in GitHub's UI — vouch for commits an agent pushed on their behalf. | ||
| The author's `/approve` is subject to the same write-access verification | ||
| as any other approver, so a fork-PR author without write access on the | ||
| base repository cannot self-count. The author counts as **one** approval; | ||
| the remaining approvals must come from other reviewers with write access. | ||
|
|
||
| ## Threat model | ||
|
|
||
| - **Tamper-proof triggers.** `pull_request_target` and `issue_comment` run | ||
| the workflow file from the base/default branch, so the PR under review | ||
| cannot edit this check. `pull_request_review` does **not** share this | ||
| property — it runs from the merge ref — so the example workflow omits it; | ||
| native Approve reviews are picked up on the next synchronize or | ||
| `/approve` comment. This tamper-resistance assumes the workflow file | ||
| itself is protected: an actor who can push workflow changes to the | ||
| default branch can spoof any required status check, including this one, | ||
| so protect `.github/workflows/` via branch protection or CODEOWNERS. | ||
| - **Fail-closed.** Any unhandled error exits non-zero; the required status | ||
| stays non-success and the PR stays blocked. PRs with >100 commits are | ||
| treated as agent-authored because the full commit list can't be verified. | ||
| - **Sibling-PR guard.** Commit statuses attach to a SHA, not a PR. The | ||
| action refuses to post a status on a PR whose base isn't in | ||
| `protected_bases`, and withholds `success` while another open PR to a | ||
| protected base shares the same head commit — otherwise a green status on | ||
| one PR would also unblock the other. | ||
| - **No checkout of PR code.** The action never checks out the PR's branch; | ||
| it reads PR metadata via the GitHub API, so the usual | ||
| `pull_request_target` code-execution risk does not apply. |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.