Skip to content

Review-seat isolation policy (STANDING, 2026-08-14, FLT-235)

Fleet-wide. Every machine, every model, every harness. Governs where a code-review seat runs and how its output is trusted. Sibling to worktree-isolation-policy.md: that one keeps sessions from clobbering each other, this one keeps a reviewer from clobbering the thing it reviews.

The rule — four clauses, and clause 1 alone is not the control

  1. The seat's cwd is a disposable detached worktree at the sha under review (or a scratchpad, for seats fed the diff inline). Never the tree under review.
  2. The seat runs with push disabled and credentials stripped from its environment. cwd is not a push boundary.
  3. After the seat, both the seat tree and the live tree must match their pre-seat porcelain and HEAD.
  4. Any miss is FAILED-CONTAMINATED — the whole seat, not just the affected findings.

A seat with write access to the artifact under review is not a reviewer. It is a second author whose changes nobody checked, and whose findings may be about its own edits.

Clause 3 is the one that catches WING-501. An implementer who reads only clause 1 and concludes the assert is optional has reproduced the incident.

Why this is a policy and not a preference — two measured incidents

2026-07-16 (FLT-56). A Codex seat asked only to review instead fixed, committed, and pushed 3f48415 to origin. The reviewer became the author, which destroys the only property the gate has. The code happened to be correct; that is luck, not a control.

2026-08-14 (WING-501). codex review --base master, run in worktree WorkWingman-wt-WT-ecb3-WING-501 after commit 40c4afb39 landed at 11:21:52, modified seven files between 11:32 and 11:36:

  • electron/package.json
  • electron/main.js
  • electron/build-installer.ps1
  • src/ChatGPTMcpServer/Program.cs
  • src/WorkWingman.Infrastructure/Automation/BrowserSession.cs
  • tests/WorkWingman.Tests/BrowserSessionTests.cs
  • tools/verify-powershell-syntax.ps1

It reverted build.win.artifactName, added a top-level zip block to package.json, and then reported that zip block back as its P1 finding — "unsupported configuration blocks all installer builds". Its Contains("msedge") BrowserSession finding was likewise about code it had just written.

Cost: a full investigation cycle to prove the P1 was not in the commit, plus a rebuild to prove the committed config was valid. Unnoticed, those edits would have been committed as reviewed work — the exact failure the gate exists to prevent, produced by the gate.

Root cause

-c sandbox_mode="danger-full-access" is mandatory for the Codex seat on Windows. Without it every command Codex runs dies with windows sandbox: CreateProcessWithLogonW failed: 2, so it cannot inspect the diff and still exits 0 reporting "no findings" — a blind pass wearing a clean review's clothes. The override is also exactly what grants the write.

There is no read-only Codex sandbox on this platform. No flag makes the seat safe. Only cwd constrains it.

Do not "fix" this by relaxing the flag. That restores the blind-pass failure while leaving the write path open the moment anyone re-enables it.

What isolation does and does not buy (read this before Rule 1)

The first version of this policy claimed "only cwd constrains the seat." That was wrong, and the FLT-235 council gate said so on every security seat. Stated honestly:

It does NOT buy: a boundary against a hostile process. A seat runs with the user's full rights and can walk to any path that user can reach, by absolute path, at any time. Nothing short of an OS boundary — a restricted token, an ACL denying the main tree, a Job Object — changes that, and none of those are implemented.

It does buy: the tool never hands a seat the tree under review; ordinary in-cwd writes land somewhere disposable; the common accidental push paths are closed; and the assert detects the writes that happen anyway. That is the difference between a reviewer that edits your code by default and one that has to go out of its way.

Do not restore the stronger claim. Two incidents happened because the guarantee was believed to be bigger than it was.

Rule 1 — Disposable review CLONE for any seat with filesystem reach

A worktree is NOT sufficient, and that was the gate's Critical finding. git worktree add gives a second working tree but the SAME common git directory: objects, refs, hooks and config all belong to the main repo. From a worktree a seat can git update-ref a live branch, rewrite shared config, or plant .git/hooks/* that execute the next time a human commits in the real tree — none of which touches its own cwd, so the assert reports it clean. I had already hit one instance of this (a push-url edit in the checkout changes the real repo's config, because config is shared) and fixed that one symptom without asking what else was shared. Everything was.

Use git clone --local --no-hardlinks: separate object store, refs, config and hooks, so ordinary git operations inside the checkout cannot reach the tree under review. --no-hardlinks because a hardlinked object store is shared mutable state with the repo we are protecting. Then remove the clone's origin — a clone points it at the source repo — and point core.hooksPath at an empty directory.

$sha  = git rev-parse --verify HEAD  # pin the artifact — a detached SHA, never a branch name,
$rc   = Join-Path $env:TEMP "council-rc-$($sha.Substring(0,8))"   # so a peer push cannot move it
git clone --local --no-hardlinks --no-checkout -- $PWD $rc
git -C $rc remote remove origin
git -C $rc config core.hooksPath (New-Item -ItemType Directory "$rc\.empty-hooks").FullName
git -C $rc checkout --detach --quiet $sha

$before = (git -C $rc status --porcelain) -join "`n"
$beforeHead = git -C $rc rev-parse HEAD

Push-Location $rc
try { $null | codex review --base master -c sandbox_mode="danger-full-access" } finally { Pop-Location }

# assert, THEN tear down
$after = (git -C $rc status --porcelain) -join "`n"
$afterHead = git -C $rc rev-parse HEAD
git -C $rc status --porcelain --untracked-files=all    # capture the file list before removal
git worktree remove $rc --force

Two seats needing a checkout get two checkouts. They must not share a tree they can both write.

Rule 1b — Push prevention: cwd is not a push boundary

FLT-56's real damage was a push to origin, not an edit of a session tree. A seat that commits and pushes from the disposable checkout leaves the live tree's porcelain and HEAD looking pristine, and the post-run assert is an audit that cannot un-push. So prevention goes first and the assert is the second line:

  • GIT_TERMINAL_PROMPT=0 and GCM_INTERACTIVE=never — a headless seat fails instead of hanging on a credential prompt.
  • credential.helper= (empty) and credential.interactive=false. Stripping GH_TOKEN is not enough: GCM_INTERACTIVE=never only suppresses the prompt, and a credential already in Windows Credential Manager still authenticates a push. The gate flagged this on two seats.
  • Every network URL form rewritten via url.<blocked>.insteadOf for https://, http://, ssh://, git:// and git@. Overriding remote.<name>.pushurl blocks nothing, because git push https://host/org/repo HEAD:master never consults remote config.
  • The repo under review passed as a blocked path prefix. A local-path push carries no URL scheme at all, so the rewrites above miss it; the one destination that reproduces the original incident is the repo itself.
  • core.sshCommand=false and a dead http.proxy — no transport of last resort.
  • Credential and transport variables removed: GH_TOKEN, GITHUB_TOKEN, GH_ENTERPRISE_TOKEN, GH_CONFIG_DIR, GIT_ASKPASS, SSH_ASKPASS, SSH_AUTH_SOCK, GIT_SSH, GIT_SSH_COMMAND, plus every GIT_CONFIG_* and GIT_DIR/GIT_WORK_TREE so an inherited value cannot reintroduce what this removes — or redirect the tool's own git at a different repository.
  • GIT_CONFIG_COUNT always set, even at zero: leaving it unset lets a parent's value stay authoritative over a stale set of key/value pairs.

Never do this by writing repo config. A linked worktree shares .git/config with the main checkout, so git remote set-url --push inside the review checkout disables push in the user's real repo. Env-scoped overrides die with the process and are inherited by any git the seat shells out to.

Residual gap, stated plainly: this stops the accidental and agentic push — the same class of control as Grok's --disallowed-tools. It is not proof against a seat that deliberately re-adds a remote or clears the variables. Verifying the remote tip needs a network round trip; after any contaminated run, check git log origin/<branch>.. by hand.

Two rounds of review kept rediscovering the same thing in new clothes: every in-process check can be defeated by a process running with the user's rights. Round 1 said the worktree shared a gitdir. Round 2 said the teardown followed junctions, the guard resolved only the deepest ancestor, and insteadOf missed file://. Each is real and each is fixed, but the pattern is that hardening in-process controls converges slowly, if at all.

The durable fix is an OS boundary — a restricted token, an ACL denying the main tree, a Job Object for the process tree, or a container. None of that is implemented, and it is deliberately recorded here as not-implemented rather than half-built. Until it exists, treat this as raising the cost of an accident, not as containing an adversary.

Rule 2 — The contamination assert, on every seat, every run

git status --porcelain and git rev-parse HEAD must be byte-identical before and after — in the seat's cwd and in the live worktree. The seat runs with full user rights; nothing stops it walking to a sibling path.

  • Both match → seat is clean.
  • Either differs → FAILED-CONTAMINATED.
  • Head moved → the seat committed. Say so explicitly and check git log origin/<branch>.. for a push.

Run it even on seats you believe have zero filesystem reach. Believed reach is a belief; the assert is a receipt. It costs two git status calls.

Rule 3 — FAILED-CONTAMINATED is its own status

Distinct from FAILED (seat produced nothing) and skipped (never attempted):

  • FAILED — no usable output. Something is broken.
  • FAILED-CONTAMINATED — produced output and wrote to the tree it reviewed. The output exists but cannot be trusted to be about the artifact under review.

A contaminated seat does not count toward the ≥2-surviving-seats quorum, however good its other findings look. The roster names the modified paths.

Rule 4 — A contaminated seat is dropped whole, not filtered

None of a contaminated seat's findings reach the chairman. Not the ones citing modified paths, and not the rest either. The seat can edit A and file a finding on B; it can change a comment, a lockfile, or surrounding context that changes how B reads. A mixed trusted/untrusted set is worse than no set, because the chairman cannot tell which half is which — and it has no way to distinguish "defect in the commit" from "defect the reviewer just introduced", which is precisely how WING-501's self-authored zip block was ranked P1, blocks all installer builds.

Still compute the path intersection and report it as "quarantined — seat modified this file". It is the diagnostic that shows what happened; it is not the filter that makes the rest safe.

Re-run the seat in a fresh checkout if quorum drops below 2.

Rule 5 — Live-tree contamination aborts the run

Excluding a contaminated seat from quorum is fail-closed for voting. It does not restore the artifact — and every later seat, test, and review in that worktree is now reading a tree the reviewer wrote. That is the same "second author nobody checked" property, one level down.

  • Seat tree only: contained. git worktree remove --force is the restore.
  • Live tree touched: abort the run. Restore the live tree to the sha under review before any other seat starts. git status + git diff it, revert the seat's writes deliberately, re-run the build. Never carry unreviewed seat edits into a commit, and never run the remaining seats against the dirty tree.

If the seat committed or pushed, do not silently revert correct code to punish process (FLT-56 precedent — that seat closed a real bearer-leak-on-redirect). Get the independent review it should have had, then decide.

Seat reach matrix

Seat Reach Where it runs
Cedric (Codex) Full write — danger-full-access is the only working Windows mode Disposable checkout. The seat that has actually written, twice.
Chairman (codex exec) Full write if it shells out Scratchpad, reviews + diff inline, --skip-git-repo-check, and no sandbox override — it inspects nothing, so it needs none.
Jenny (agy/Gemini) None while the diff is inline and the prompt forbids tools Scratchpad. The prompt must end with "the diff is inline, do NOT use any tool" — that line is the control, not a workaround.
Gronktayvius (grok) None — --disallowed-tools "write,edit,bash,shell" Scratchpad. The flag is load-bearing; drop it and this seat inherits Codex's requirement.
Clahadore (claude -p / subagent) Edit/Write available by default Scratchpad, diff inline. Give it repo cwd or write tools and it needs the disposable checkout.

Jenny and Gronktayvius did not write on WING-501. Both are read-only only because of a flag or a prompt line that can be dropped silently in a future edit, which is why Rule 2 applies to them anyway.

Enforcement — the tool, not the doc

council-seat (src/CouncilSeat.Cli, this repo) is the only supported way to run a seat. It builds each seat's command line and working directory itself: there is no --exec, no --cwd, and no way to pass a raw command. An escape hatch is how isolation gets skipped on the one busy afternoon it matters, so there isn't one.

council-seat run --seat Cedric --repo . --sha HEAD --base master --out <dir>
council-seat run --seat Jenny  --repo . --prompt-file <file> --out <dir> --timeout 420
council-seat seats

Exit codes: 0 seated · 1 failed · 2 FAILED-CONTAMINATED · 3 skipped (CLI absent) · 4 usage · 5 git error. Each run writes seat-<name>.json (the roster row, machine-readable) plus .out and .err, never merged.

One call does: checkout → snapshot (seat tree and live tree) → launch with push disabled and credentials stripped → snapshot again → validate the output → scan for self-citations → tear down. The no-tools invariant is appended by the tool for every prompt-fed seat, so it cannot be forgotten by whoever wrote the prompt.

Docs are a control only while someone reads them; this is the part that holds when nobody does.

Also enforced in: - ~/.claude/skills/council-code-review/SKILL.md — §"Seat isolation", precondition 7, seat-validation item 6, FAILED-CONTAMINATED consequences, roster cwd / Clean after columns. - ~/.claude/skills/codex-review/SKILL.md — same procedure for the single-reviewer fallback. - ~/.claude/skills/gemini-review/SKILL.md — the no-tools invariant, as an invariant. - Memory: review-seat-must-not-write.

Open follow-up: remote-tip verification (the residual gap in Rule 1b) is still manual, and a leaked checkout is reported rather than retried.