From ef0fd4ddb60bc5552f3a981c6a033e7715710b57 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 27 Apr 2026 14:52:53 +0200 Subject: [PATCH 1/5] Trigger Claude PR reviewer via repository_dispatch on C+ approval Replaces the pull_request_review trigger added in #88584. That trigger cannot work for fork PRs because GitHub does not expose secrets to a workflow when a fork PR fires pull_request_review, so OS_BOTIFY_TOKEN, ANTHROPIC_API_KEY, and pull-requests: write are all unavailable on exactly the PRs we most want C+ re-reviews for. Instead, the GitHub webhook handled by Web-Expensify checks the C+ team membership and (for an approving review on Expensify/App) fires a repository_dispatch event of type claude-review-request against this repo. repository_dispatch always runs in the base-repo context with full secrets, including for fork PRs, and is gated by Contents: write so externals/forks cannot trigger it. Hardening: - github.actor must be OSBotify or a *[bot] account - client_payload values are passed via env vars and validated with regex before reaching shell - PR metadata (head SHA, base ref, draft state, title) is re-resolved via gh pr view rather than trusted from the payload Removes the now-unused isContributorPlus composite action; the membership check lives in Web-Expensify alongside other PHP-side team lookups. Paired with Expensify/Web-Expensify#52390. Made-with: Cursor --- .../composite/isContributorPlus/action.yml | 33 ------ .github/workflows/claude-review.yml | 110 ++++++++++++++---- 2 files changed, 86 insertions(+), 57 deletions(-) delete mode 100644 .github/actions/composite/isContributorPlus/action.yml diff --git a/.github/actions/composite/isContributorPlus/action.yml b/.github/actions/composite/isContributorPlus/action.yml deleted file mode 100644 index 42c3d2f044dc..000000000000 --- a/.github/actions/composite/isContributorPlus/action.yml +++ /dev/null @@ -1,33 +0,0 @@ -name: Is Contributor+ -description: Check whether a GitHub user is a member of the Expensify/contributor-plus team. Sets IS_CPLUS=true when the user is a member, IS_CPLUS=false otherwise. - -inputs: - USERNAME: - description: The GitHub login of the user to check. - required: true - OS_BOTIFY_TOKEN: - description: OSBotify token. Needed to read team memberships (the default GITHUB_TOKEN lacks the read:org scope). - required: true - -outputs: - IS_CPLUS: - description: "'true' if the user is a member of Expensify/contributor-plus, 'false' otherwise." - value: ${{ steps.check.outputs.IS_CPLUS }} - -runs: - using: composite - steps: - - name: Check Contributor+ membership - id: check - shell: bash - env: - GH_TOKEN: ${{ inputs.OS_BOTIFY_TOKEN }} - USERNAME: ${{ inputs.USERNAME }} - run: | - if gh api "/orgs/Expensify/teams/contributor-plus/memberships/$USERNAME" --silent; then - echo "::notice::✅ $USERNAME is a Contributor+ member" - echo "IS_CPLUS=true" >> "$GITHUB_OUTPUT" - else - echo "::notice::$USERNAME is not a Contributor+ member" - echo "IS_CPLUS=false" >> "$GITHUB_OUTPUT" - fi diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 34c1fb9702a6..48c25b626ae3 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -7,11 +7,18 @@ permissions: on: pull_request_target: types: [opened, ready_for_review] - pull_request_review: - types: [submitted] + # Re-run via repository_dispatch when a Contributor+ approves a PR. + # Web-Expensify's GitHub webhook fires this event after verifying C+ membership. + # We use repository_dispatch (not pull_request_review) because pull_request_review + # on fork PRs is treated like pull_request: no secrets, read-only token. Anyone + # who can hit /repos/Expensify/App/dispatches already needs Contents: write on + # this repo, so externals/forks cannot fire it. + repository_dispatch: + types: [claude-review-request] concurrency: - group: claude-review-${{ github.event.pull_request.html_url }} + # Scope to PR so rapid C+ re-approvals on the same PR cancel earlier in-progress runs. + group: claude-review-${{ github.event.pull_request.html_url || format('pr-{0}', github.event.client_payload.pr_number) }} cancel-in-progress: true jobs: @@ -23,42 +30,95 @@ jobs: PR_AUTHOR: ${{ github.event.pull_request.user.login }} AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }} - checkCPlusApproval: - if: github.event_name == 'pull_request_review' && github.event.review.state == 'approved' + resolvePR: + if: github.event_name == 'repository_dispatch' runs-on: blacksmith-2vcpu-ubuntu-2404 outputs: - IS_CPLUS: ${{ steps.check.outputs.IS_CPLUS }} + PR_NUMBER: ${{ steps.pr.outputs.PR_NUMBER }} + HEAD_SHA: ${{ steps.pr.outputs.HEAD_SHA }} + BASE_REF: ${{ steps.pr.outputs.BASE_REF }} + IS_DRAFT: ${{ steps.pr.outputs.IS_DRAFT }} + TITLE: ${{ steps.pr.outputs.TITLE }} steps: - - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - with: - fetch-depth: 1 + # Defense in depth: GitHub already restricts /dispatches to tokens with Contents: + # write on this repo, which excludes externals and forks. On top of that we require + # the dispatcher to be either OSBotify or a GitHub App (a "*[bot]" actor); a real + # human committer triggering this event by hand would fail this check. + - name: Verify dispatcher + env: + ACTOR: ${{ github.actor }} + run: | + if [[ "$ACTOR" == "OSBotify" || "$ACTOR" == *"[bot]" ]]; then + echo "::notice::claude-review-request dispatched by $ACTOR" + else + echo "::error::Unexpected dispatcher for claude-review-request: $ACTOR" + exit 1 + fi - - name: Check Contributor+ membership - id: check - uses: ./.github/actions/composite/isContributorPlus - with: - USERNAME: ${{ github.event.review.user.login }} - OS_BOTIFY_TOKEN: ${{ secrets.OS_BOTIFY_TOKEN }} + # Treat the client_payload as untrusted: pass through env vars and validate format + # before letting any value reach a shell command. Avoids ${{ ... }} interpolation + # injection from a forged-by-an-insider payload. + - name: Validate payload + env: + PR_NUMBER_RAW: ${{ github.event.client_payload.pr_number }} + REVIEWER_RAW: ${{ github.event.client_payload.reviewer }} + run: | + if ! [[ "$PR_NUMBER_RAW" =~ ^[0-9]+$ ]]; then + echo "::error::Invalid pr_number in client_payload: $PR_NUMBER_RAW" + exit 1 + fi + if ! [[ "$REVIEWER_RAW" =~ ^[A-Za-z0-9-]{1,39}$ ]]; then + echo "::error::Invalid reviewer in client_payload" + exit 1 + fi + + # Re-resolve PR data from GitHub rather than trusting the dispatch payload for + # anything beyond pr_number. This way a bogus payload can only point Claude at a + # different real PR; it cannot fabricate code or metadata. + - name: Resolve PR + id: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.client_payload.pr_number }} + run: | + JSON=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \ + --json number,title,isDraft,headRefOid,baseRefName) + { + echo "PR_NUMBER=$(echo "$JSON" | jq -r '.number')" + echo "HEAD_SHA=$(echo "$JSON" | jq -r '.headRefOid')" + echo "BASE_REF=$(echo "$JSON" | jq -r '.baseRefName')" + echo "IS_DRAFT=$(echo "$JSON" | jq -r '.isDraft')" + echo "TITLE<> "$GITHUB_OUTPUT" review: - needs: [validate, checkCPlusApproval] + needs: [validate, resolvePR] if: | !cancelled() - && github.event.pull_request.draft != true - && !contains(github.event.pull_request.title, 'Revert') && ( - (github.event_name == 'pull_request_target' && needs.validate.outputs.IS_AUTHORIZED == 'true') - || (github.event_name == 'pull_request_review' && needs.checkCPlusApproval.outputs.IS_CPLUS == 'true') + (github.event_name == 'pull_request_target' + && needs.validate.outputs.IS_AUTHORIZED == 'true' + && github.event.pull_request.draft != true + && !contains(github.event.pull_request.title, 'Revert')) + || (github.event_name == 'repository_dispatch' + && needs.resolvePR.result == 'success' + && needs.resolvePR.outputs.IS_DRAFT == 'false' + && !contains(needs.resolvePR.outputs.TITLE, 'Revert')) ) runs-on: blacksmith-2vcpu-ubuntu-2404 env: - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ github.event.pull_request.number || needs.resolvePR.outputs.PR_NUMBER }} steps: - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: fetch-depth: 1 + # For pull_request_target the default ref is fine; for repository_dispatch we + # need to explicitly point at the PR's head SHA so paths-filter and Claude see + # the code being reviewed (not the default branch). + ref: ${{ github.event.pull_request.head.sha || needs.resolvePR.outputs.HEAD_SHA }} - name: Setup Node uses: ./.github/actions/composite/setupNode @@ -67,6 +127,8 @@ jobs: uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 id: filter with: + base: ${{ github.event.pull_request.base.ref || needs.resolvePR.outputs.BASE_REF }} + ref: ${{ github.event.pull_request.head.sha || needs.resolvePR.outputs.HEAD_SHA }} filters: | code: - 'src/**' @@ -97,7 +159,7 @@ jobs: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} github_token: ${{ secrets.GITHUB_TOKEN }} allowed_non_write_users: "*" - prompt: "/review-code-pr REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }}" + prompt: "/review-code-pr REPO: ${{ github.repository }} PR_NUMBER: ${{ env.PR_NUMBER }}" claude_args: | --model claude-opus-4-6 --allowedTools "Task,Glob,Grep,Read,Bash(gh pr diff:*),Bash(gh pr view:*),Bash(check-compiler.sh:*)" --json-schema '${{ steps.schema.outputs.json }}' @@ -133,7 +195,7 @@ jobs: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} github_token: ${{ secrets.GITHUB_TOKEN }} allowed_non_write_users: "*" - prompt: "/review-helpdot-pr REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }}" + prompt: "/review-helpdot-pr REPO: ${{ github.repository }} PR_NUMBER: ${{ env.PR_NUMBER }}" claude_args: | --model claude-opus-4-6 --allowedTools "Task,Glob,Grep,Read,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),mcp__github_inline_comment__create_inline_comment" From c2166aff4194f4de57caeb56a449d0981b2bce6e Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 27 Apr 2026 15:09:59 +0200 Subject: [PATCH 2/5] Pin claude-review-request dispatcher to expensify-bot[bot] GitHub's /dispatches permission check (Contents: write) already excludes externals and forks, but several Expensify-controlled identities also have that permission (org admins, write-collaborator PATs, other bots). The PHP webhook in Web-Expensify is the only intended caller, and it authenticates as App ID 179547 (https://github.com/apps/expensify-bot, github.actor = "expensify-bot[bot]") via EXPENSIFY_GITHUB_APP_PRIVATE_KEY. Pin the gate to that exact slug so a leaked PAT from any other code path cannot route an arbitrary client_payload through this workflow, which has access to ANTHROPIC_API_KEY and pull-requests: write. Made-with: Cursor --- .github/workflows/claude-review.yml | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 48c25b626ae3..aa39f2222a4e 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -41,19 +41,17 @@ jobs: TITLE: ${{ steps.pr.outputs.TITLE }} steps: # Defense in depth: GitHub already restricts /dispatches to tokens with Contents: - # write on this repo, which excludes externals and forks. On top of that we require - # the dispatcher to be either OSBotify or a GitHub App (a "*[bot]" actor); a real - # human committer triggering this event by hand would fail this check. + # write on this repo, which excludes externals and forks. On top of that we pin the + # dispatcher to the exact GitHub App that Web-Expensify authenticates as when it + # calls createDispatchEvent() via EXPENSIFY_GITHUB_APP_PRIVATE_KEY (App ID 179547, + # https://github.com/apps/expensify-bot). Any other actor — even another Expensify + # bot or an org admin with a PAT — will fail this check, so a leaked token from a + # different code path cannot reach the Anthropic-key-holding job below. - name: Verify dispatcher - env: - ACTOR: ${{ github.actor }} + if: github.actor != 'expensify-bot[bot]' run: | - if [[ "$ACTOR" == "OSBotify" || "$ACTOR" == *"[bot]" ]]; then - echo "::notice::claude-review-request dispatched by $ACTOR" - else - echo "::error::Unexpected dispatcher for claude-review-request: $ACTOR" - exit 1 - fi + echo "::error::claude-review-request can only be dispatched by expensify-bot[bot] (got: ${{ github.actor }})" + exit 1 # Treat the client_payload as untrusted: pass through env vars and validate format # before letting any value reach a shell command. Avoids ${{ ... }} interpolation From 21271759a66032f05ff4a3b3fa30fbfce0dbda13 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 27 Apr 2026 15:14:50 +0200 Subject: [PATCH 3/5] Drop verbose comment above dispatcher check Made-with: Cursor --- .github/workflows/claude-review.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index aa39f2222a4e..ecb6627483ee 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -40,13 +40,6 @@ jobs: IS_DRAFT: ${{ steps.pr.outputs.IS_DRAFT }} TITLE: ${{ steps.pr.outputs.TITLE }} steps: - # Defense in depth: GitHub already restricts /dispatches to tokens with Contents: - # write on this repo, which excludes externals and forks. On top of that we pin the - # dispatcher to the exact GitHub App that Web-Expensify authenticates as when it - # calls createDispatchEvent() via EXPENSIFY_GITHUB_APP_PRIVATE_KEY (App ID 179547, - # https://github.com/apps/expensify-bot). Any other actor — even another Expensify - # bot or an org admin with a PAT — will fail this check, so a leaked token from a - # different code path cannot reach the Anthropic-key-holding job below. - name: Verify dispatcher if: github.actor != 'expensify-bot[bot]' run: | From 12eb78e94fb9cd53fd8a755b56c73e06af4d170e Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 27 Apr 2026 16:09:25 +0200 Subject: [PATCH 4/5] Drop backend-specific reference from workflow comment Made-with: Cursor --- .github/workflows/claude-review.yml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index ecb6627483ee..4a3eaba14306 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -7,12 +7,10 @@ permissions: on: pull_request_target: types: [opened, ready_for_review] - # Re-run via repository_dispatch when a Contributor+ approves a PR. - # Web-Expensify's GitHub webhook fires this event after verifying C+ membership. - # We use repository_dispatch (not pull_request_review) because pull_request_review - # on fork PRs is treated like pull_request: no secrets, read-only token. Anyone - # who can hit /repos/Expensify/App/dispatches already needs Contents: write on - # this repo, so externals/forks cannot fire it. + # Re-run via repository_dispatch when a Contributor+ approves a PR. We use + # repository_dispatch instead of pull_request_review because pull_request_review on + # fork PRs is treated like pull_request: no secrets, read-only token. /dispatches + # requires Contents: write on this repo, so externals/forks cannot fire it. repository_dispatch: types: [claude-review-request] From 497587472bcf49d30c188c04698d20c92717a88c Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 27 Apr 2026 17:45:18 +0200 Subject: [PATCH 5/5] Stop loading fork code in review job Removes ref: from actions/checkout so the review job always runs against the trusted base ref, addressing the pwn-request pattern where running repo scripts against fork-controlled PR-head code would expose base-repo secrets. Replaces dorny/paths-filter with gh pr view --json files so path detection works the same way for both pull_request_target and repository_dispatch without needing PR-head code on disk. Made-with: Cursor --- .github/workflows/claude-review.yml | 36 ++++++++++++++++++----------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 4a3eaba14306..03d826667fd4 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -100,30 +100,38 @@ jobs: env: PR_NUMBER: ${{ github.event.pull_request.number || needs.resolvePR.outputs.PR_NUMBER }} steps: + # Always check out the trusted base ref. Claude reads PR contents via the + # gh pr diff/view tools, and the path filter below uses gh pr view --json files, + # so we never need fork code on disk. This avoids the pwn-request pattern under + # pull_request_target where running repo scripts against PR-head code would + # expose secrets to fork-controlled changes. - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: fetch-depth: 1 - # For pull_request_target the default ref is fine; for repository_dispatch we - # need to explicitly point at the PR's head SHA so paths-filter and Claude see - # the code being reviewed (not the default branch). - ref: ${{ github.event.pull_request.head.sha || needs.resolvePR.outputs.HEAD_SHA }} - name: Setup Node uses: ./.github/actions/composite/setupNode + # Determine which review prompts to run from the PR's file list via the API, + # so the same step works for pull_request_target and repository_dispatch + # without needing PR-head code or extra git history on disk. - name: Filter paths - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 id: filter - with: - base: ${{ github.event.pull_request.base.ref || needs.resolvePR.outputs.BASE_REF }} - ref: ${{ github.event.pull_request.head.sha || needs.resolvePR.outputs.HEAD_SHA }} - filters: | - code: - - 'src/**' - docs: - - 'docs/**/*.md' - - 'docs/**/*.csv' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + FILES=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json files --jq '.files[].path') + if echo "$FILES" | grep -qE '^src/'; then + echo "code=true" >> "$GITHUB_OUTPUT" + else + echo "code=false" >> "$GITHUB_OUTPUT" + fi + if echo "$FILES" | grep -qE '^docs/.+\.(md|csv)$'; then + echo "docs=true" >> "$GITHUB_OUTPUT" + else + echo "docs=false" >> "$GITHUB_OUTPUT" + fi - name: Add claude utility scripts to PATH run: |