REQUEST CHANGES from the cross-family plan-review (2026-06-22), dispositioned: - expanded denylist (§4.2): submodules/.gitmodules, git hooks, .gitattributes filters, lockfile postinstall, generated artifacts - runner-trust assertion (privileged jobs GitHub-hosted only) - concrete diff-transport spec + threat model (signed artifact / branch-only token, nonce anti-replay) - gate-weakening detection (noqa/skip/excludes/--no-verify) - PR-metadata secret sanitization; ledger diff-hash anti-tamper - Phase 1b: recovery for an accidentally-merged/applied privileged change + draft-PR rate monitoring + stale-PR cleanup - required-check-name discovery; deploy-before-merge enforcement; no-write-token audit Notes which BLOCK items are already implemented in PR #17's CI (Phase 1 verifies, not rebuilds).
14 KiB
P3-LIVE-FLIP PLAN — agent-team build → verify → draft-PR
Formal phased plan to take the agent-team Plane-2 pipeline from clarify+plan only
to producing reviewable draft PRs, while keeping the always-on R720 box
read-only and the apply path zero-AWS. Status as of 2026-06-22: NOT STARTED
(P3 is built but inert). This plan is the input to /sh-plan-review before any build.
Prerequisite reading:
docs/r720-agent-team-design.md§3.3.2 (CI-as-verifier trust boundary, "B4"),PROVISIONING-RUNBOOK.md(the P3-live-flip section), and memoryproject_r720_agent_team(locked decisions).
1. Objective & current state
Today (inert): the pipeline runs INTAKE → CLARIFIER → PLANNER → REVIEW, but
serve passes build_verify_wiring=None, the Tier-3 fixer is --dry-run only, and
agent-team/ci/agent-team-apply-verify.yml has its privileged steps disabled with
if: ${{ false }} and pull-requests:write / environment: commented out. So it
clarifies + plans but writes no code and opens no PR.
After P3: the pipeline can emit a diff, have org CI build/test/security-review it in an untrusted sandbox, a pure-code gate confirm green from authenticated Checks-API results, and a scoped GitHub App open a draft PR for human review. The box never gains a write token.
2. Locked decisions (carried in — do not relitigate here)
- D-OIDC: apply path uses a GitHub App
pull-requests:writetoken, ZERO AWS, no OIDC. - D2: box stays read-only / no standing write token; org CI does the applying.
- B4: the LLM-proposed diff is untrusted code; the CI trust boundary (below) is mandatory.
- Output is draft PRs only — nothing auto-merges; human approval is the merge gate.
- (Separate, not part of this plan:
aws-postureresident access via step-ca + IAM Roles Anywhere — that IAM is already cross-review-approved and is its own sub-task.)
3. Hard gates (must clear before the flip — these block everything)
| Gate | Why | Owner |
|---|---|---|
/sh-plan-review on THIS plan |
adversarial plan audit before build | me → GPT-4.1 |
/sh-security-review on the apply/verify CI surface |
auth + untrusted-input + CI trust boundary | me |
| GPT-4.1 cross-family review on the apply/verify CI + any permission change | mandatory for the trust-boundary / permissions surface | orchestrator |
GH_TOKEN→GITHUB_TOKEN resolved |
the GitHub transport reads GITHUB_TOKEN; box has GH_TOKEN |
me (folded in here) |
The flip does NOT proceed until /sh-security-review AND the GPT-4.1 cross-review on
the CI surface both pass.
Plan-review disposition (GPT-4.1 cross-family, 2026-06-22 — REQUEST CHANGES). Findings folded into §4 and Phases 1/1b: expanded denylist vectors, runner-trust, concrete diff-transport + threat model, gate-weakening detection, PR-metadata sanitization, ledger anti-tamper, recovery for a merged-privileged change, required-check-name discovery, deploy-before-merge enforcement, no-write-token audit, draft-PR rate monitoring + stale-PR cleanup. Several items the reviewer marked BLOCK are already implemented in PR #17's CI (canonicalization, egress, SHA-pin, empty-hash fail-closed, Checks-API) — Phase 1 verifies them rather than rebuilding. Open QUESTIONs to answer when building: how human reviewers are notified of new draft PRs, and how "every deployable repo has CI" is enforced for targets.
4. The CI trust boundary (design B4 — what the workflow must enforce)
Already implemented in the merged P3-live CI hardening (PR #17) — Phase 1 VERIFIES, does not rebuild: denylist path canonicalization + symlink/rename/traversal resistance, egress restriction on the untrusted job, SHA-pinned actions, empty/missing-hash fail-closed, and authenticated Checks-API result consumption. The GPT-4.1 plan-review (2026-06-22) flagged these as "missing" because the plan under-restated them; confirm each against the actual
agent-team/ci/agent-team-apply-verify.yml+ci_fetcher.py/ci_gate.pyrather than re-authoring.
- Split CI. An untrusted build/test job:
contents: readonly, no secrets / no OIDC / no write token, egress-restricted (verify/audit the restriction, don't just assert it). A separate privileged job that never checks out the patch code (nopull_request_target+ head checkout) opens the draft PR. Privileged jobs MUST run only on GitHub-hosted runners — assert no self-hosted/user-provided runner can pick them up. - Denylist (reject/escalate, never auto-build). Beyond
.github/workflows/**, IAM/permission IaC, branch-protection /CODEOWNERS/ Dependabot config, and out-of-scope files, the denylist MUST also cover these RCE/priv-esc vectors:.gitmodules/ submodule changes, git hooks (.git/hooks,core.hooksPath,.husky/**),.gitattributes(filter/clean-smudge process), lockfiles + package-manager postinstall/preinstall hooks, and generated/build-artifact files (codegen output is not reviewable as source). Path matching is canonicalized (PR #17) so symlinks/renames/traversal can't slip a denied path past. - Diff-transport integrity (concretely specified, threat-modeled). The box records the
diff content hash in its ledger; CI verifies the hash before apply. The diff
reaches CI as a content-addressed signed artifact (HMAC/keyed digest the box and the
privileged job share via an Actions secret) OR a short-lived, branch-only token
scoped to a single ref — the box never holds a write token. Threat model the path:
tamper-in-transit (defeated by hash+signature verify), replay of an old diff (defeated by
per-task nonce + the
status='open'/one-shot ledger state), and a hostile artifact name. - Pure-code green gate. Pass/fail is owned by a pure-code gate reading authenticated Checks-API results (run id + diff hash). The LLM verifier may propose fixes but can never declare a build green.
- Gate-weakening detection. A diff that lowers a gate — adds
# noqa,# type: ignore, testskip/xfail, coverage/lint excludes,--no-verify, or edits the gate config itself — is flagged and escalated (a build can't make itself pass by disabling the checks). - PR-metadata sanitization. The draft-PR title / body / comments are sanitized so a hostile diff or LLM output can't exfiltrate secrets/env or inject content into the PR text.
- Ledger anti-tamper. The diff-hash ledger entry is integrity-protected (the existing atomic-write + integrity-check substrate; verify the hash row can't be silently rewritten between record and apply).
- Merge gate. Draft PR + required checks +
/sh-security-review+ Claude Code App review + human approval.
5. Phases
Phase 0 — Plan review & pre-reqs 🤖/🧑
- Run
/sh-plan-reviewon this doc; fold BLOCK/FIX items in. - Confirm a clean revert point (git tag main; Hyper-V snapshot of sh-secrev).
- Resolve
GH_TOKEN→GITHUB_TOKEN(transport accepts both / box env updated). - Rollback: none (no state changed).
Phase 1 — Author/verify the split-CI apply/verify workflow 🤖 (review-gated)
- Reconcile
agent-team/ci/agent-team-apply-verify.ymlwith §4. First confirm the PR-#17 controls are present (canonicalized denylist, egress restriction, SHA-pins, empty-hash fail-closed, Checks-API consumption); only then add the new §4 items. - Add denylist vectors (§4.2): submodules/
.gitmodules, git hooks/core.hooksPath/.husky,.gitattributesfilters, lockfile postinstall/preinstall, generated/build artifacts. Add a test suite proving canonicalization resists symlink/rename/traversal. - Runner-trust assertion (§4.1): test that privileged jobs cannot run on a self-hosted/user-provided runner.
- Concretize + threat-model the diff transport (§4.3): pick content-addressed signed artifact (shared HMAC secret) or short-lived branch-only token; add per-task nonce anti-replay; document and test it.
- Gate-weakening detector (§4.5): CI step that fails on a diff adding
noqa/type: ignore/skip/xfail/excludes/--no-verifyor editing the gate config. - PR-metadata sanitization (§4.6) and ledger anti-tamper (§4.7) implemented + tested.
agent_team/ci_fetcher.py(read-only Checks-API fetcher; fails closed) +ci_gate.py(pure-code green). Add a mechanism for the gate to discover the correct required check names per repo/branch (avoid hardcoded check-name drift across repos).- Deploy-before-merge enforcement: a documented/CI gate ensuring the privileged flip is exercised on the box before the workflow change is merged (Sea Haven deploy-then-merge).
- Concrete "no write token on the box" audit (a test/script, not just a claim).
/sh-security-review+ GPT-4.1 cross-review on this surface. Hard stop until both pass.- Rollback: workflow file stays inert (
if: ${{ false }}not yet flipped); delete the file.
Phase 1b — Recovery for an accidentally-merged/applied privileged change 🤖/🧑
- Document + exercise once a rollback for the case where a privileged change (the
apply/verify workflow, the
agent-applyenvironment, the GitHub App perms, or branch-protection) is merged or applied in error: revert the SHA, rotate the GitHub App token, restore branch-protection/environment from a recorded baseline, and confirm no draft-PR apply ran in the window. (The plan previously only covered reverting inert files.) - Add light monitoring on draft-PR creation rate (runaway-volume alarm) and an orphaned/stale draft-PR cleanup step.
Phase 2 — Provision the GitHub App + environment 🧑 OPERATOR (browser/admin)
- Create a dedicated GitHub App with
pull-requests:write(+ minimal contents to open a branch/PR); install on the org. Token lives in CI, never on the box. - Create the
agent-applyGitHub Actions Environment with a required reviewer (Adam) + branch-protection so the privileged job cannot run unreviewed. - Store the App credentials as repo/org Actions secrets (not on the box).
- Rollback: uninstall the App; delete the environment + secrets.
Phase 3 — Bind the live wiring (still gated by the environment) 🤖
- In the workflow: uncomment
permissions: pull-requests: writeandenvironment: agent-apply; flip the twoif: ${{ false }}→ enabled. - Bind
agent_team.coordinator.gated_build_verify_wiring(...)(real diff builder + read-only CI-result fetcher) so a leaf calls it only after the gate clears. - Set the box-side apply env vars the live path reads (read-only CI-result token + dispatch target). Confirm no write token lands on the box.
- Rollback: re-set
if: ${{ false }}, re-commentenvironment:, setbuild_verify_wiring=None; restart the coordinator. (Exercise this rollback once.)
Phase 4 — Smoke test to a first draft PR 🧑/🤖
- Drive one trivial, in-scope task end-to-end → confirm: untrusted job builds/tests with no secrets, denylist rejects an out-of-scope diff, pure-code gate gates on real Checks results, privileged job opens a draft PR with required checks attached, nothing merged.
- Flip the Tier-3 fixer off
--dry-runonly after the smoke test passes; verify a dependency-CVE bump produces a draft PR. - Rollback: close the draft PR; Phase-3 rollback.
Phase 5 — Enable the cross-plane loop 🤖
- Allow confirmed Plane-1 checker findings (
intake-checker) to flow into pipeline tasks that end in draft-PR fixes (start conservative: highest-severity, one at a time). - Rollback: revert intake-checker wiring to report-only.
Phase 6 — Docs & memory 🤖
- Update
OPERATOR-RUNBOOK.md, the Confluence host page, and memory: P3 is LIVE, what the apply path can/can't do, the denylist, the merge gate, the rollback.
6. What changes (and what does NOT)
Changes:
- The pipeline produces draft PRs for well-scoped work (dep bumps, small fixes, docs, checker-surfaced issues). Your role shifts from author → reviewer/approver for that class.
- Tier-3 fixer goes live (dep-CVE bumps → draft PRs).
- Plane-1 findings can become draft-PR fixes (cross-plane loop).
Does NOT change:
- Box stays read-only; CI applies, not the box. No AWS / no OIDC in the apply path.
- Denylisted, high-blast-radius changes (workflows, IAM, branch protection, CODEOWNERS, Dependabot) are never auto-built — always escalated.
- Human gates stay: the clarifier question-set AND the PR approval are both yours.
- Drafts, not merges — you remain the merge authority.
7. Risks & mitigations
| Risk | Mitigation |
|---|---|
| Untrusted diff exfiltrates secrets / escalates in CI | split CI: build job has no secrets/OIDC/write, egress-restricted; privileged job never checks out patch code |
| Malicious diff edits CI/IAM/branch-protection to self-escalate | denylist → reject/escalate, never auto-build |
| LLM "declares" a broken build green | pure-code gate reads authenticated Checks-API only; LLM can't set status |
| Diff tampered between box and CI | diff-hash recorded in ledger, verified before apply |
| Standing write capability on the always-on box | there is none — App token lives in CI; box holds only a read-only CI-result token |
| Runaway PR volume | start with Tier-3 only + one finding at a time; required-reviewer environment gates each |
8. Definition of done
/sh-plan-review,/sh-security-review, and GPT-4.1 cross-review on the CI surface all passed.- Phase-4 smoke test produced a draft PR; nothing auto-merged; rollback exercised once.
- No write token on the box (verified); apply path is zero-AWS.
- Docs + Confluence + memory updated.
- Snapshot retained until P3 runs clean for one cycle, then pruned.