Addresses the confirmed findings from /sh-security-review + the GPT-4.1
cross-review of the Plane-2 scaffold. Full suite: 589 passed; ruff clean.
FIXED (proven-exploitable):
- CI-guard denylist bypass (HIGH): Python fnmatch '**/' is non-recursive, so
root-level template.yaml/*.tf/cdk.json/*.pem/*.key/*-stack.* evaded the
trust-control surface. Replaced fnmatch with a recursive, case-insensitive
glob->regex matcher. (verified: fnmatch('template.yaml','**/template.yaml')==False)
- CI-guard scope bypass (HIGH): a '**' declared_scope made every path in-scope.
Scope is now concrete-prefix confinement (reduces a glob to its leading
metacharacter-free segments; '**' -> empty -> dropped -> unscoped reject).
- Box-side vs CI denylist divergence (MED): builders.py _DENY_PATTERNS now covers
Terraform, *.pem/*.key, CDK stack files, .github/actions, *iam*, bare policy*.json
(case-insensitive), matching the CI surface.
- force-resume was backwards (MED): it superseded the answered row recovery
resumes from, making a stuck task permanently un-resumable while printing
success. Now re-opens an EXPIRED (parked) question via a new reopen_question
CAS helper; never supersedes an answered row; honest exit codes.
- operator attribution (MED): run-team.py --operator defaulted to "" -> now the
OS login, so destructive actions are always attributable.
- audit-log append race (MED): replaced read-modify-rewrite (lost records under
concurrent operators) with an O_APPEND single-line write, mode 600 enforced.
- lstrip("ab/") path-mangling in the symlink error path -> regex prefix strip.
Regression tests added across test_ci_gate_workflow / test_builders / test_run_team
/ test_schema. Design-level findings (resume-worker durability, egress breadth,
answered_at ordering, DB-swap TOCTOU, diff-hash threat-model) are pre-deployment
/ P1-build-proper and recorded with written justification in
agent-team/.security-review/suppressions.json; CI README diff-hash wording made
honest.
|
||
|---|---|---|
| .. | ||
| agent-team-apply-verify.yml | ||
| README.md | ||
agent-team/ci — split-job CI apply/verify workflow (Plane-2 leaf)
Pre-deployment scaffolding for the R720 agent-team SDLC pipeline. This directory
holds the split-job CI apply/verify workflow that turns a builder agent's
untrusted candidate diff into a verified draft PR — the §3.3.2 trust
boundary, Phase P3 (§7.1) of ../../docs/r720-agent-team-design.md.
STATUS: DEPLOY-GATED. NOT ENABLED, NOT PROVISIONED. This is authored as files only. Per the design (§3.3.2, §7.1 P3) the workflow + its OIDC role must clear BOTH
/sh-security-reviewAND the mandatory GPT-4.1 cross-review before deployment, because it is IaC/IAM + untrusted-input handling. The privileged draft-PR step is hard-disabled (if: ${{ false }}) and the OIDCid-token/pull-requests: writegrants are left commented until those gates pass. Nothing here is wired to a live org repo.
Files
| File | What it is |
|---|---|
agent-team-apply-verify.yml |
The split-job workflow. Self-contained: the load-bearing gate logic (diff integrity, trust-control denylist, declared-scope check, pure-code pass/fail) is embedded inline as stdlib-only, type-hinted Python heredocs, so the workflow has no external script dependency. |
README.md |
This file. |
The filename is kebab-case per the handbook. Deployment target (later, after the
gates): promote into Sea-Haven-Industries/.github as a reusable workflow
(engineering-handbook/cicd.md); the Option-B OIDC apply path invokes it.
The trust boundary (design §3.3.2)
The builder agents are semi-trusted: an LLM that read repo content can be wrong or prompt-injected, so the candidate diff is treated as untrusted code. The threat is that executing it in CI with org credentials lets a bad diff exfiltrate secrets, assume the deploy role, or tamper with other repos. The workflow implements all five boundaries:
- Split CI — untrusted execution is credential-less. The job that checks
out and runs the diff (
build-test) runs withpermissions: contents: read, no secrets, no OIDC, no write token, and egress blocked (harden-runner). The patch executes only there, where there is nothing to steal and nothing to assume. Every privileged action (the eventual OIDC role, the draft-PR open) runs in a separategate-and-prjob that never checks out or executes patch-controlled code — it consumes the build/test report as data only. There is nopull_request_target+ head-ref checkout (the "pwn request" anti-pattern). - Trust-control-surface denylist (CI-side hard fail). The
guardjob rejects any diff that touches.github/workflows/**, IAM/policy IaC (CDK/SAM/Terraform), branch-protection /CODEOWNERS/ Dependabot config, or files outside the task's declared scope. The match is not naive: it canonicalizes paths, rejects parent-directory traversal, and inspectsrename from/toheaders, so a rename into a denied path — or path indirection — cannot bypass it. Such a diff is escalated to mandatory human + GPT cross-review, never auto-built. - Diff integrity, box → CI. The builder records the candidate diff's sha256
in the task ledger (foundation
agent_team.state_storecontent-hash idiom). CI re-hashes the diff and verifies it equals the recorded hash before applying (in bothguardand again pre-apply inbuild-test). Precisely: this is an unkeyed hash that binds the bytes CI applies to the hash the dispatcher recorded — it detects accidental corruption or substitution of the artifact in transit between the trusted dispatcher and CI. It does not prove the diff matches the approved plan, and it cannot defend a compromised box that generates both the diff and its hash (see "Threat-model honesty" below). A keyed/attested binding to the approval record is a later hardening. - Pure-code pass/fail gate over authenticated results. Mirroring secrev's
"one pure-code script owns the block decision," the
gate-and-prgate reads only the authenticatedneeds.*.resultjob conclusions (GitHub-controlled, patch-independent) keyed to the run id + diff hash. It never trusts a success/failure file or artifact the patch could have written. The verifier agent only reads failures to propose the next fix; it cannot declare success. - Branch protection is the final enforcement. The draft PR targets a protected branch; the locked-down checks are required; merge needs them green plus the security-review, the Claude Code App review, and human approval. The agent never auto-merges (D2).
Threat-model honesty (§3.3.2). The diff hash protects box → CI transit (tamper/substitution), not a box that is itself compromised — the box is in the trusted computing base for what it proposes. The real backstops against a bad box are the credential-less build job, the trust-control-surface denylist, and above all the human review of the draft PR plus required checks (nothing merges without Adam).
Job graph
workflow_dispatch (task_id, diff_artifact_name, expected_diff_hash, declared_scope)
│
▼
guard contents:read, no secrets — validates the diff as DATA:
(boundaries 2,3) re-hash + denylist + scope. Never applies it.
│ (needs)
▼
build-test contents:read, no secrets, no OIDC, egress blocked —
(boundary 1) the ONLY job that applies + runs the UNTRUSTED patch.
│ (needs) Emits a NON-authoritative report artifact.
▼
gate-and-pr privileged home, but NEVER checks out patch code. Pure-code
(boundaries 1,4,5) gate over authenticated needs.*.result → DRAFT PR
(hard-disabled until the review gates pass).
permissions: {} at the workflow level (least privilege); each job re-declares
its own grant explicitly. The trigger is workflow_dispatch only — the patch
never runs in a context carrying write or secret scope.
SHA-pinned actions (handbook Pinning Principle, §3.3.2)
Every third-party action is pinned to a full commit SHA with the human-readable tag in a trailing comment:
| Action | SHA | Tag |
|---|---|---|
actions/checkout |
11bd71901bbe5b1630ceea73d27597364c9af683 |
v4.2.2 |
actions/download-artifact |
fa0a91b85d4f404e444e00e005971372dc801d16 |
v4.1.8 |
actions/upload-artifact |
b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 |
v4.4.3 |
actions/setup-python |
0b93645e9fea7318ecaed2b359559ac225c90a2b |
v5.3.0 |
step-security/harden-runner |
0080882f6c36860b6ba35c610c98ce87d4e2f26f |
v2.10.2 |
Relationship to the foundation
This leaf imports the committed Plane-2 foundation contracts verbatim (it does not redefine them):
- The diff-hash recorded in the ledger and re-checked in CI is the same
content-hash idiom as
agent_team.state_store.compute_content_hash(§6.7). - The task this workflow verifies is an
agent_team.task_model.TaskRecord; itscandidate_diff+diff_hashfields (§3.3) are exactly theexpected_diff_hashthis workflow consumes, andci_resultsis what the verifier writes back from the authenticated gate (boundary 4). - The ledger that records provenance (diff hash, run id, gate decision) is the
agent_team.dbschema (pending_questions/budget_ledgerlive there; per-task CI provenance is recorded against the task thread).
Tests
The workflow's embedded gate logic (diff integrity, the trust-control denylist
with path-canonicalization + rename/copy/delete detection, the symlink-escape
reject, and declared-scope enforcement) is stdlib-only, type-hinted, and
ruff-clean, and is covered by a committed, runnable suite:
../tests/test_ci_gate_workflow.py extracts the inline guard script from this
YAML and executes it against good and adversarial diffs — clean in-scope,
hash mismatch, workflow delete, copy-into-denied, symlink addition, non-UTF-8,
out-of-scope, unscoped, and escaping-scope. Run it with the rest of the suite:
python3 -m pytest agent-team/tests/ -q from the repo root. (The claim that the
gate is "verified" is therefore backed by that test, not by authoring alone.)
Deploy gating (do NOT skip)
Before this ships (§3.3.2, §7.1 P3):
/sh-security-reviewover this workflow (IaC + untrusted-input handling).- Mandatory GPT-4.1 cross-review of the workflow and the Option-B OIDC role it will assume (IAM change).
- A documented, exercised rollback (remove the role, revert the workflow).
- Only then: uncomment the
id-token/pull-requests: writegrants, enable the draft-PR step, and promote toSea-Haven-Industries/.github. Draft PRs only; never auto-merge.