What happened
On PR #6028, the review agent identified that classic PATs cannot be repo-scoped ([low] secrets-scope) but missed the critical security concern: the behaviour job runs under pull_request_target, checks out untrusted PR-head code via allow-unsafe-pr-checkout: true, then executes make behaviour-test with the new PAT secrets in the step's env — meaning PR-controlled Go code can read and exfiltrate those bearer tokens. Human reviewer @waynesun09 traced this full 5-component attack chain and then verified blast radius via GitHub API. The repo's docs/contributing/ci-workflows.md covers concurrency, timeouts, permissions, and checkout pinning but has no section on the security implications of adding secrets to jobs that execute untrusted code under pull_request_target.
What could go better
The agent's security sub-agent analyzed the new secret references in isolation (noting PAT scope limitations) rather than tracing the execution path through the workflow to determine what code can access those secrets at runtime. This is a well-documented GitHub Actions security pattern (the 'pwn request' class). The repo has ADR-0009 documenting why pull_request_target is safe for the shim workflow (which never checks out PR code), but this safety reasoning does not transfer to the e2e workflow, which explicitly does check out PR code. Without documented guidance on this interaction, both agents and new human reviewers must independently derive the risk. Confidence is high that this documentation gap contributed to the shallow finding — the agent reads ci-workflows.md when reviewing workflow changes (per the AGENTS.md index) but found no security-relevant content there.
Proposed change
Add a new section to docs/contributing/ci-workflows.md titled 'Secrets in pull_request_target jobs' covering: (1) the known-dangerous pattern — pull_request_target + checkout of PR head (allow-unsafe-pr-checkout: true or ref: pointing to PR head SHA) + secrets in the same job's env means untrusted code can read and exfiltrate those secrets; (2) the distinction between short-lived narrowly-scoped credentials (GitHub App installation tokens, WIF tokens) and long-lived broadly-scoped credentials (classic PATs, PEM keys) — the latter have higher blast radius on exfiltration; (3) the existing gate job (check-e2e-authorization) as primary mitigation and its limitations; (4) a review checklist for PRs adding secrets to the behaviour or e2e jobs: trace the execution path to confirm what code can access the secret, verify blast radius if exfiltrated, prefer short-lived narrowly-scoped credentials, and do not wire secrets into env before consumption code exists. Also update the AGENTS.md table entry for ci-workflows.md to mention the security content so agents know to look for it.
Validation criteria
On the next 2 PRs that add or modify secret references in a pull_request_target job (in this repo or repos using the same e2e pattern), the review agent should flag the exfiltration risk at medium or higher, referencing the execution path through the untrusted checkout. The agent's finding should trace at least 3 of the 5 components in the attack chain (event type, checkout mode, code execution, env access, credential type) rather than stopping at generic PAT scope concerns.
Generated by retro agent from #6028
What happened
On PR #6028, the review agent identified that classic PATs cannot be repo-scoped ([low] secrets-scope) but missed the critical security concern: the behaviour job runs under
pull_request_target, checks out untrusted PR-head code viaallow-unsafe-pr-checkout: true, then executesmake behaviour-testwith the new PAT secrets in the step's env — meaning PR-controlled Go code can read and exfiltrate those bearer tokens. Human reviewer @waynesun09 traced this full 5-component attack chain and then verified blast radius via GitHub API. The repo'sdocs/contributing/ci-workflows.mdcovers concurrency, timeouts, permissions, and checkout pinning but has no section on the security implications of adding secrets to jobs that execute untrusted code underpull_request_target.What could go better
The agent's security sub-agent analyzed the new secret references in isolation (noting PAT scope limitations) rather than tracing the execution path through the workflow to determine what code can access those secrets at runtime. This is a well-documented GitHub Actions security pattern (the 'pwn request' class). The repo has ADR-0009 documenting why
pull_request_targetis safe for the shim workflow (which never checks out PR code), but this safety reasoning does not transfer to the e2e workflow, which explicitly does check out PR code. Without documented guidance on this interaction, both agents and new human reviewers must independently derive the risk. Confidence is high that this documentation gap contributed to the shallow finding — the agent readsci-workflows.mdwhen reviewing workflow changes (per the AGENTS.md index) but found no security-relevant content there.Proposed change
Add a new section to
docs/contributing/ci-workflows.mdtitled 'Secrets in pull_request_target jobs' covering: (1) the known-dangerous pattern —pull_request_target+ checkout of PR head (allow-unsafe-pr-checkout: trueorref:pointing to PR head SHA) + secrets in the same job's env means untrusted code can read and exfiltrate those secrets; (2) the distinction between short-lived narrowly-scoped credentials (GitHub App installation tokens, WIF tokens) and long-lived broadly-scoped credentials (classic PATs, PEM keys) — the latter have higher blast radius on exfiltration; (3) the existing gate job (check-e2e-authorization) as primary mitigation and its limitations; (4) a review checklist for PRs adding secrets to the behaviour or e2e jobs: trace the execution path to confirm what code can access the secret, verify blast radius if exfiltrated, prefer short-lived narrowly-scoped credentials, and do not wire secrets into env before consumption code exists. Also update the AGENTS.md table entry for ci-workflows.md to mention the security content so agents know to look for it.Validation criteria
On the next 2 PRs that add or modify secret references in a
pull_request_targetjob (in this repo or repos using the same e2e pattern), the review agent should flag the exfiltration risk at medium or higher, referencing the execution path through the untrusted checkout. The agent's finding should trace at least 3 of the 5 components in the attack chain (event type, checkout mode, code execution, env access, credential type) rather than stopping at generic PAT scope concerns.Generated by retro agent from #6028