ci(#6026): route actor PAT secrets into behaviour job

[ifireball]

Summary

• Wire TEST_ACTOR_WRITE_PAT, TEST_ACTOR_TRIAGE_PAT, and TEST_ACTOR_OUTSIDER_PAT into the behaviour job env in .github/workflows/e2e.yml (same names as repo secrets).
• Document the secrets in the e2e inventory and behaviour runner-env snippet.
• e2e-ok-to-test.yml already uses secrets: inherit — no change needed.

Closes #6026.

Note: the blocked label on #6026 can be cleared — #6024 landed and the secrets exist. Driver/step consumption remains #6027.

Test plan

• Confirm behaviour job env in e2e.yml maps all three ${{ secrets.TEST_ACTOR_*_PAT }} entries
• Confirm admin e2e job is unchanged
• After merge, a behaviour-relevant CI run has non-empty env for the three vars (without printing values)
• Clear blocked on test(behaviour): route write/triage/outsider actor PAT secrets into the behaviour job #6026

Made with Cursor

[qodo-code-review]

PR Summary by Qodo

Route actor PAT repo secrets into behaviour job environment

⚙️ Configuration changes 📝 Documentation 🕐 10-20 Minutes

AI Description

• Expose three human-like actor PAT secrets to the behaviour GitHub Actions job env.
• Keep secret names consistent between repo secrets and runner environment variables.
• Document the required secrets in behaviour/e2e testing guides.

Diagram

graph TD
  A[".github/workflows/e2e.yml"] -->|"defines"| B["Behaviour job"] -->|"runs"| C["Behaviour runner"] -->|"SCM ops"| D["GitHub API"]
  E["Repo secrets"] -->|"mapped to env"| B
  F["Docs (testing guides)"] -->|"document secrets"| E

Loading

High-Level Assessment

The chosen approach—explicitly mapping ${{ secrets.* }} into the behaviour job env using the same variable names—is the clearest and safest way to scope secret exposure to only the job that needs them. Alternatives like workflow-global env would broaden exposure, and relying on secrets: inherit isn’t applicable here unless refactoring into a reusable workflow.

Files changed (3) +11 / -0

Documentation (2) +6 / -0

behaviour-testing.mdDocument actor PATs in behaviour runner env snippet +3/-0

Document actor PATs in behaviour runner env snippet

• Extends the behaviour runner environment example to include the three actor PAT variables. Clarifies intended privilege levels for write/triage/outsider actors.

docs/guides/dev/behaviour-testing.md

e2e-testing.mdAdd actor PATs to required e2e repository secrets list +3/-0

Add actor PATs to required e2e repository secrets list

• Documents three new required repository secrets for behaviour testing and notes they are exposed to the behaviour job under the same env names. Provides brief descriptions and associated test actor identities.

docs/guides/dev/e2e-testing.md

Other (1) +5 / -0

e2e.ymlExpose actor PAT secrets in behaviour job env +5/-0

Expose actor PAT secrets in behaviour job env

• Adds 'TEST_ACTOR_WRITE_PAT', 'TEST_ACTOR_TRIAGE_PAT', and 'TEST_ACTOR_OUTSIDER_PAT' to the behaviour job environment by mapping them directly from same-named repository secrets. Includes inline comments clarifying intent and deferred consumption by drivers/steps.

.github/workflows/e2e.yml

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:19 AM UTC · Completed 9:31 AM UTC

Commit: 84ee1ca · View workflow run →

[github-actions]

Site preview

Preview: https://439c6fc3-site.fullsend-ai.workers.dev

Commit: 84ee1ca594260ac39890c7eb2b6a1d49d52810c8

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (1)

1. e2e-testing.md disallowed directory 📜 Skill insight ⌂ Architecture

Description

This PR modifies guide documentation under docs/guides/dev/, but guides must live under
docs/guides/admin/ or docs/guides/user/ to meet the documented organization and single-audience
targeting requirements. Leaving guides in the nonstandard dev subtree breaks compliance and makes
the intended audience unclear.

Code

docs/guides/dev/e2e-testing.md[R65-67]

+| `TEST_ACTOR_WRITE_PAT` | Classic PAT for the write-level human-like test actor (`fstest-write`); exposed to the behaviour job under the same env name |
+| `TEST_ACTOR_TRIAGE_PAT` | Classic PAT for the triage-level human-like test actor (`fstest-triage`); exposed to the behaviour job under the same env name |
+| `TEST_ACTOR_OUTSIDER_PAT` | Classic PAT for the outsider (no org write) human-like test actor (`fstest-outsider`); exposed to the behaviour job under the same env name |

Relevance

● Weak

Past reviews flagged docs/guides/dev placement as noncompliant, but relocation suggestions were
rejected and dev guides kept.

PR-#4901
PR-#5502
PR-#5457

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The stated compliance rule constrains guide locations to docs/guides/admin/ or
docs/guides/user/, yet the PR’s modified guide files are still located under docs/guides/dev/
(e.g., the referenced changed lines within docs/guides/dev/e2e-testing.md and
docs/guides/dev/behaviour-testing.md). The additional referenced lines in docs/guides/README.md
and cross-linked guide files further indicate the current guide structure and references assume
these documents exist in the dev subtree, reinforcing that the changes remain out of the permitted
directories.

docs/guides/dev/e2e-testing.md[65-67]
docs/guides/dev/behaviour-testing.md[231-233]
Skill: writing-user-docs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR updates documentation guides that currently live under `docs/guides/dev/` (e.g., `e2e-testing.md` and `behaviour-testing.md`), but compliance requires all guides to be located under either `docs/guides/admin/` or `docs/guides/user/` and to target a single audience. Relocate these guides into the appropriate allowed subtree and adjust any references so the guide organization and audience targeting requirements are met.

## Issue Context
This PR modified the guides (including adding secrets-related documentation), so the touched guide content should be brought into compliance by moving it out of `docs/guides/dev/` into the correct `admin` or `user` guides subtree and then updating any internal/external links or index references that still point to the old locations.

## Fix Focus Areas
- docs/guides/dev/e2e-testing.md[65-67]
- docs/guides/dev/e2e-testing.md[41-41]
- docs/guides/dev/behaviour-testing.md[209-210]
- docs/guides/dev/behaviour-testing.md[231-233]
- docs/guides/README.md[54-54]
- docs/guides/dev/behaviour-drivers.md[35-35]
- docs/guides/infrastructure/mint-administration.md[78-78]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. PATs exposed to PR code 🐞 Bug ⛨ Security

Description

The behaviour job checks out and runs untrusted PR head code under pull_request_target, and this
PR adds three classic PAT secrets into that job’s make behaviour-test environment. Once the gate
authorizes a run, PR-controlled code can read/exfiltrate these PATs and use them to perform
write/triage SCM operations, bypassing the job’s intentionally-limited GITHUB_TOKEN permissions.

Code

.github/workflows/e2e.yml[R276-278]

+          TEST_ACTOR_WRITE_PAT: ${{ secrets.TEST_ACTOR_WRITE_PAT }}
+          TEST_ACTOR_TRIAGE_PAT: ${{ secrets.TEST_ACTOR_TRIAGE_PAT }}
+          TEST_ACTOR_OUTSIDER_PAT: ${{ secrets.TEST_ACTOR_OUTSIDER_PAT }}

Relevance

● Weak

Similar request to restructure pull_request_target to avoid exposing secrets to PR code was rejected
previously.

PR-#2534
PR-#5181

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The workflow explicitly states that the behaviour job checks out untrusted PR head code (including
enabling unsafe checkout) and then runs make behaviour-test; this PR adds three PAT secrets to
that step environment, making them directly accessible to PR-controlled code once authorized.

.github/workflows/e2e.yml[183-194]
.github/workflows/e2e.yml[234-240]
.github/workflows/e2e.yml[253-278]
.github/actions/check-e2e-authorization/action.yml[94-97]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The workflow currently enforces a security boundary by limiting `GITHUB_TOKEN` permissions in jobs that check out PR head code. This PR introduces three classic PAT secrets into the `behaviour` job environment, which is explicitly designed to run untrusted PR head code under `pull_request_target` (using `allow-unsafe-pr-checkout`). That makes the PATs accessible to attacker-controlled code once the gate is satisfied.

## Issue Context
The risk is not theoretical: any code executed by `make behaviour-test` (from the PR head checkout) can read process environment variables and exfiltrate them. Because PATs are bearer tokens with potentially broad scopes, they bypass the job-level `permissions:` restrictions.

## Fix Focus Areas
- .github/workflows/e2e.yml[183-278]

## Suggested remediation
Choose one of these approaches:
1. **Do not pass PATs on `pull_request_target` runs**: gate the PAT env values so they are empty for `github.event_name == 'pull_request_target'` (or only set them in push/merge_group contexts).
2. **Split privileged SCM ops into a trusted job/workflow**: keep PR-head checkout jobs secret-minimal, and move PAT-consuming operations into a separate job that does *not* check out PR code (or uses base-branch code only) and/or is protected by a GitHub Environment with required reviewers.
3. **Replace PAT usage with short-lived, least-privilege credentials** where feasible (e.g., minted tokens / GitHub App tokens), so even if exposed the blast radius is constrained.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] .github/workflows/e2e.yml — This PR modifies .github/workflows/e2e.yml, which is under the .github/ protected path. The change is authorized by issue test(behaviour): route write/triage/outsider actor PAT secrets into the behaviour job #6026 and the PR description explains the rationale. Human approval is always required for changes to governance and infrastructure files regardless of context.

Low

• [secrets-scope] .github/workflows/e2e.yml:274 — The PR documents these as Classic PATs for human-like test actors (fstest-write, fstest-triage, fstest-outsider). Classic PATs cannot be scoped to specific repositories — they grant the stated permission level across all repos the user has access to. If the test actor accounts have access to repos beyond what the test suite requires, those would also be accessible to code executing in the behaviour job. This is mitigated by the gate job authorization requirement.
  Remediation: Consider using fine-grained PATs scoped to specific test repositories, or verify that the test actor accounts have access only to required repos.

---

Labels: PR modifies e2e CI workflow and e2e testing documentation

[fullsend-ai-review]

See the review comment for full details.

[waynesun09]

Approving. Verified the fstest-* account scoping directly via the GitHub API before signing off: none of fstest-write/fstest-triage/fstest-outsider are fullsend-ai org members, and all three show only read permission on both fullsend-ai/fullsend and fullsend-ai/agents (including fstest-write, despite its "write-level" label — its actual write capability is scoped to the disposable pool-org test-repo-NN infrastructure per docs/guides/dev/behaviour-testing.md, not production repos). So even if a PAT leaked via the untrusted-code-exfiltration path I flagged, the blast radius is contained — it cannot write to the real codebase. CI green, no other blockers.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:39 PM UTC · Completed 7:51 PM UTC

Commit: 84ee1ca · View workflow run →

[fullsend-ai-retro]

Retro: PR #6028 — ci(#6026): route actor PAT secrets into behaviour job

Timeline

PR #6028 was opened 2026-08-10 09:18 UTC by Barak Korren (via Cursor), adding 11 lines across 3 files to wire TEST_ACTOR_{WRITE,TRIAGE,OUTSIDER}_PAT into the e2e behaviour job environment, plus documentation. The review agent completed in ~13 minutes, posting two findings: [medium] protected-path (valid — .github/ requires human approval) and [low] secrets-scope (classic PATs cannot be repo-scoped). The agent's verdict was comment (not approve or request-changes). Human reviewer @waynesun09 then identified a deeper [medium] exfiltration risk: the behaviour job runs under pull_request_target, checks out untrusted PR-head code via allow-unsafe-pr-checkout: true, and executes make behaviour-test — meaning PR-controlled Go code can read the new PAT env vars and exfiltrate them. waynesun09 verified via GitHub API that fstest-* accounts have only read on production repos, concluding blast radius was contained. PR merged at 19:32 UTC.

Review Quality Assessment

The agent correctly identified the protected-path concern (novel — the human did not flag it) and noted PAT scope limitations. However, it missed the primary security concern: tracing the execution path from pull_request_target through untrusted checkout to code execution with secrets in the environment. The human operated at a significantly deeper level — tracing a 5-component attack chain and then performing external API verification of account permissions.

Dimension	Agent	Human
Protected path flagging	✅ Medium	Not flagged (implicit)
PAT scope limitations	✅ Low	✅ Part of larger finding
Exfiltration chain tracing	❌ Missed	✅ Full 5-step chain
Blast radius verification	❌ Not attempted	✅ GitHub API verification
Sequencing risk (secrets before consumption)	❌ Missed	✅ Deferred to #6027

Rework Rate & Efficiency

No rework — single commit, single review cycle. The agent ran once (13 min, ~6 min agent thinking + ~3 min challenger). The human review took ~4 hours wall-clock (13:37-19:20 UTC) including the approval and follow-up deferral comment. Total time to merge: ~10 hours. This is reasonable for a security-sensitive CI change requiring human judgment.

Evidence for Existing Issues

• #2933 (review agent should flag CI workflow security anti-patterns): PR ci(#6026): route actor PAT secrets into behaviour job #6028 provides additional evidence. The agent's security sub-agent identified PAT scope as a generic concern but did not recognize the pull_request_target + untrusted checkout + secrets pattern as a CI security anti-pattern.
• fullsend-ai/agents#535 (review agent should escalate security sub-agent depth based on PR labels/file-path signals): The PR carried component/ci and requires-manual-review labels and modified .github/workflows/e2e.yml. Despite these signals, the security sub-agent ran at standard depth, finding only a Low-severity concern while the human found a Medium-severity exfiltration chain.
• #5221 (behaviour debug artifacts can exfiltrate secrets): PR ci(#6026): route actor PAT secrets into behaviour job #6028 adds three more PAT secrets to the behaviour job environment, expanding the blast radius of the artifact exfiltration vector documented in e2e: behaviour debug artifacts can exfiltrate secrets without redaction #5221.

Autonomy Assessment

This PR is counter-evidence for autonomous approval of CI workflow changes that modify secrets. The agent's review was materially insufficient for the security dimension — it missed the primary concern and could not perform blast-radius verification. The requires-manual-review label was correctly applied. No autonomy increase is warranted for this class of change.

Proposals filed

• Document pull_request_target + secrets security review checklist in ci-workflows.md (in fullsend-ai/fullsend)
• Document fstest-* test actor account permission scope for blast-radius assessment (in fullsend-ai/fullsend)