Skip to content

fix(ci): restore fork guard on pr-triage.yml's pull_request_target trigger - #7052

Open
holistis wants to merge 1 commit into
google:mainfrom
holistis:fix/pr-triage-fork-guard
Open

fix(ci): restore fork guard on pr-triage.yml's pull_request_target trigger#7052
holistis wants to merge 1 commit into
google:mainfrom
holistis:fix/pr-triage-fork-guard

Conversation

@holistis

@holistis holistis commented Sep 8, 2026

Copy link
Copy Markdown

Summary

pr-triage.yml runs on pull_request_target for every PR opened against this repo, with pull-requests: write permission and the ADK_TRIAGE_AGENT / GOOGLE_API_KEY secrets available to it, and calls a Gemini-backed agent that can assign a component owner to the PR.

PR #6053 (merged as 0d20b7c) scoped the automatic run to same-repository PRs precisely because pull_request_target runs with base-repo secrets even for a PR opened from a fork by a first-time, unauthenticated contributor:

-    if: github.event_name == 'workflow_dispatch' || !contains(github.event.pull_request.labels.*.name, 'google-contributor')
+    if: >-
+      github.event_name == 'workflow_dispatch' || (
+        github.event.pull_request.head.repo.full_name == github.repository &&
+        !contains(github.event.pull_request.labels.*.name, 'google-contributor')
+      )

That guard was silently lost a few weeks later. Tracing the file's history:

  1. 0d20b7c (2026-06-11) adds the fork guard (PR security: gate pr-triage secrets on same-repository pull_request_target #6053).
  2. dbd4bb07d0 (2026-06-23) removes the pull_request_target trigger entirely (switches to schedule + workflow_dispatch only) as part of adding batch mode, and the guard goes with it — harmless at the time, since there was no pull_request_target trigger left to guard.
  3. f41bc79922 (2026-07-09, "ADK changes") reintroduces the pull_request_target trigger, but the job condition comes back as a bare if: github.repository == 'google/adk-python' — true for every PR opened against this public repo, fork or not. The guard was not restored.
  4. 4148f852aa (2026-08-13) narrows the trigger's types: further but leaves the same unguarded if:.

As of main today, opening a PR from a fork — the normal way an external contributor sends a PR to this repo — is enough to make this job run automatically: it spends GOOGLE_API_KEY-backed Gemini calls on the PR's content and can write an assignee to it, with no check on who opened the PR and no per-account rate limit. The workflow sets INTERACTIVE to the (by default unset) vars.PR_TRIAGE_INTERACTIVE, which settings.py treats as falsy, so the agent's "do not ask for approval before assigning" instruction path is the one that actually runs — this is fully automatic, not gated on a human confirming first.

To be clear about the blast radius: the agent's only tools are list_unassigned_pull_requests, get_pull_request_details, and assign_owner_to_pr, and assign_owner_to_pr can only pick from the fixed LABEL_TO_OWNER map and only assigns users GitHub reports as assignable. So this isn't a secrets-exfiltration or repo-takeover path — it's unauthenticated, uncapped triggering of a paid Gemini call plus the ability to force one of a fixed set of Google accounts to be assigned to an arbitrary (including spam/low-effort) fork PR. That's exactly the class #6053 already fixed once.

The fix

Restores the same-repository condition from #6053 (the google-contributor label check is left out on purpose — it was separately removed in 4148f852aa ("chore: stop auto-labeling pull requests"), and reintroducing it isn't needed to close this gap). workflow_dispatch (the maintainer batch/manual path) is untouched.

Testing plan

  • Parsed the edited file with PyYAML to confirm it's still valid YAML.
  • The restored if: expression is the same github.event.pull_request.head.repo.full_name == github.repository check already reviewed and merged for this exact job in 0d20b7c, just combined with the github.repository == check that replaced it — no new expression syntax.
  • Did not open a live PR from an external fork against google/adk-python to demonstrate the trigger, to avoid actually spending the project's Gemini/API budget — this mirrors how security: gate pr-triage secrets on same-repository pull_request_target #6053 itself was verified ("No live exploit was performed. Local trust-boundary simulation only.").
  • Not able to test on a private fork run of the actual pull_request_target event end-to-end (that requires the real secrets), so a maintainer re-running this job on a fork-originated PR after merge is the remaining verification step.

🤖 Generated with Claude Code

@google-cla

google-cla Bot commented Sep 8, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@holistis

holistis commented Sep 8, 2026

Copy link
Copy Markdown
Author

For context: #5976 also touches pr-triage.yml and proposes an author_association check, but it predates this regression (opened 2026-06-05, before #6053 added the fork guard this PR restores) and is now stale/conflicting with main. This PR is narrower and independent of it: it just restores the exact same-repository condition #6053 already got merged, which was lost when the trigger was reintroduced in f41bc79. Not a duplicate of #5976, though a maintainer picking either up should probably close the other once one lands.

…igger

pr-triage.yml runs on pull_request_target for every PR opened against
this repo and holds secrets (ADK_TRIAGE_AGENT, GOOGLE_API_KEY) that let
it call Gemini and write PR assignees. PR google#6053 (merged in 0d20b7c)
scoped this to same-repository PRs so an untrusted, first-time fork PR
could not auto-trigger the privileged agent. That guard was silently
dropped when the pull_request_target trigger was reintroduced in
f41bc79 ("ADK changes", 2026-07-09) with a bare
`if: github.repository == 'google/adk-python'`, which is true for every
PR opened against this public repo regardless of where it comes from.

As of main today, opening a PR from a fork (the normal flow for an
external contributor) is enough to force this job to run: it spends
GOOGLE_API_KEY-backed Gemini calls and can assign one of the
component-owner accounts to the PR, fully automatically (the workflow
sets INTERACTIVE to the unset vars.PR_TRIAGE_INTERACTIVE, which
settings.py treats as falsy, so the agent's "do not ask for approval"
instruction path is the one that runs) and with no cap on how many
times a single external account can trigger it.

This restores the same-repository condition google#6053 added, combined with
the existing repository check, and leaves workflow_dispatch (the
maintainer batch/manual path) untouched.

Testing plan:
- YAML parses (validated with PyYAML) and the `if:` mirrors the exact
  expression already merged in 0d20b7c for the same job, so no new
  syntax is introduced.
- Traced the regression through GitHub's commit history for this file
  (0d20b7c adds the guard -> dbd4bb0 removes the whole
  pull_request_target trigger along with it while switching to
  schedule-only -> f41bc79 reintroduces pull_request_target without
  the guard -> 4148f85 keeps it absent through the most recent
  change), confirming this is a regression rather than a deliberate
  change.
- No live PR was opened from an external fork against google/adk-python
  to avoid actually spending the project's API budget; this mirrors how
  google#6053 itself was verified ("No live exploit was performed. Local
  trust-boundary simulation only.").
@holistis

holistis commented Sep 8, 2026

Copy link
Copy Markdown
Author

I signed it!

@holistis
holistis force-pushed the fix/pr-triage-fork-guard branch from 83c54a6 to dea0a52 Compare September 8, 2026 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants