fix(ci): restore fork guard on pr-triage.yml's pull_request_target trigger - #7052
fix(ci): restore fork guard on pr-triage.yml's pull_request_target trigger#7052holistis wants to merge 1 commit into
Conversation
|
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. |
|
For context: #5976 also touches |
…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.").
|
I signed it! |
83c54a6 to
dea0a52
Compare
Summary
pr-triage.ymlruns onpull_request_targetfor every PR opened against this repo, withpull-requests: writepermission and theADK_TRIAGE_AGENT/GOOGLE_API_KEYsecrets 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 becausepull_request_targetruns with base-repo secrets even for a PR opened from a fork by a first-time, unauthenticated contributor:That guard was silently lost a few weeks later. Tracing the file's history:
0d20b7c(2026-06-11) adds the fork guard (PR security: gate pr-triage secrets on same-repository pull_request_target #6053).dbd4bb07d0(2026-06-23) removes thepull_request_targettrigger entirely (switches toschedule+workflow_dispatchonly) as part of adding batch mode, and the guard goes with it — harmless at the time, since there was nopull_request_targettrigger left to guard.f41bc79922(2026-07-09, "ADK changes") reintroduces thepull_request_targettrigger, but the job condition comes back as a bareif: github.repository == 'google/adk-python'— true for every PR opened against this public repo, fork or not. The guard was not restored.4148f852aa(2026-08-13) narrows the trigger'stypes:further but leaves the same unguardedif:.As of
maintoday, 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 spendsGOOGLE_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 setsINTERACTIVEto the (by default unset)vars.PR_TRIAGE_INTERACTIVE, whichsettings.pytreats 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, andassign_owner_to_pr, andassign_owner_to_prcan only pick from the fixedLABEL_TO_OWNERmap 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-contributorlabel check is left out on purpose — it was separately removed in4148f852aa("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
if:expression is the samegithub.event.pull_request.head.repo.full_name == github.repositorycheck already reviewed and merged for this exact job in0d20b7c, just combined with thegithub.repository ==check that replaced it — no new expression syntax.google/adk-pythonto 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.").pull_request_targetevent 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