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).
209 lines
14 KiB
Markdown
209 lines
14 KiB
Markdown
# 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 memory `project_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:write` token**, **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-posture` resident 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.py` rather than re-authoring.
|
|
|
|
1. **Split CI.** An **untrusted build/test job**: `contents: read` only, **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** (no
|
|
`pull_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.**
|
|
2. **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.
|
|
3. **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.
|
|
4. **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**.
|
|
5. **Gate-weakening detection.** A diff that **lowers a gate** — adds `# noqa`, `# type: ignore`,
|
|
test `skip`/`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).
|
|
6. **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.
|
|
7. **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).
|
|
8. **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-review` on 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.yml` with §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`,
|
|
`.gitattributes` filters, 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-verify` or 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-apply` environment, 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-apply` GitHub 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: write` and
|
|
`environment: agent-apply`; flip the two `if: ${{ 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-comment `environment:`, set
|
|
`build_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-run` only 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.
|