fix(gate): close operator-state leaks in check.sh and selftests - #88
Merged
Conversation
One commit now gets one ./scripts/check.sh verdict on CI, in a worker worktree and in the control-plane checkout. - check.sh queue no longer runs `queue.sh check` over the checkout's live records; `fleet-status.sh --records` validates them (and the registry map) instead, through the same validators. - check.sh automerge judges a fresh-clone root holding only the tracked example, rather than answering skip beside an operator auto-merge.conf. - check.sh yaml no longer validates the gitignored registry map; the onboarding selftest holds a generated map to that shape instead. - check.sh voice renders off the tracked glyph and agent defaults (install-extension.sh now honours FLEET_GLYPH_ROOT). - scripts/lib/selftest-env.sh pins git config, HOME, forge credentials, THURBOX_SESSION and every FLEET_* settings root for all seven selftests. - scripts/isolation-selftest.sh (check.sh isolation, CI job) re-runs the checks that could leak in a poisoned copy of the tree under a hostile host. Claude-Session: https://claude.ai/code/session_01TVf5c2rWYcoVREhFKgquNJ
…, not by source-text grep
…escription of registry validation
LeTuR
added a commit
that referenced
this pull request
Sep 13, 2026
…rator checkouts (#89) ## Intent Fleet task test-isolation/01-gate-reads-no-operator-state ("Fix isolation in fleet tests"; the Rust port is postponed). Goal: one commit gives the same ./scripts/check.sh result wherever it runs - CI, a worker worktree, or the operator's control-plane checkout with its live queue and gitignored configs. Leaks to confirm and close: (1) check_queue ran `queue.sh check` over the checkout's live orchestration/queue - moved out of the gate into an operator health check reachable from scripts/fleet-status.sh (implemented as `fleet-status.sh --records`, a flag rather than a screen section because validation opens archived topics' task files, which the default screen promises never to open; it validates both queue records via queue.py record_problems() and the registry map via check_yaml.py registry_problems(), and degrades/exits 0 like the rest of fleet-status). (2) check_automerge answered skip beside an operator auto-merge.conf - now judges a temp root holding only the tracked example. (3) selftests building git repos without pinning host git config - one shared helper scripts/lib/selftest-env.sh (selftest_isolate <dir>) pins GIT_CONFIG_*, GIT_DIR family, HOME/XDG, identity, forge credentials, THURBOX_SESSION and every FLEET_* settings root to tracked-example copies, keeping PATH and PYTHONUSERBASE; all seven selftests call it. (4) audit: check_yaml validated the gitignored registry map (removed from gate; onboarding-selftest now holds a generated map to the shape instead), check_voice rendered through the checkout's session-glyphs.conf/agent.conf (install-extension.sh now honours FLEET_GLYPH_ROOT), fleet-status-selftest's queue setup reached a real thurbox-cli (now runs under its bare PATH). Test first (operator rule): scripts/isolation-selftest.sh, wired as `check.sh isolation` and a CI job in All Checks, poisons a throwaway copy of the tree and a hostile host and re-runs only the checks that can leak plus the cheap selftests; §3 sources each selftest up to its real selftest_isolate call under the hostile host (so the two slow selftests are held to the helper without being re-run). It fails on main naming each leak and passes on the branch. Hard constraints: keep it simple, no test runner/sharding/new dependency; do not weaken what a check proves - move the live-data half, do not delete it; tracked files name no operator tool/vendor/agent; fixtures are made up; do not touch docs/design/testing.md (PR #85 owns it). Squash-merge only; the worker does not merge. This run is a follow-up on PR #88: the latest commit fixes §3's probe, which printed `committed=$?` after an echo and so always reported 0. ## What Changed - Move live operator-state validation (queue records, registry map) out of `check.sh` and into a new `fleet-status.sh --records` health check backed by `queue.py record_problems()` and `check_yaml.py registry_problems()`; `check_automerge` now judges a temp root containing only the tracked `auto-merge.conf` example instead of the operator's live config. - Add `scripts/lib/selftest-env.sh` (`selftest_isolate`) to pin git config, `GIT_DIR`, `HOME`/XDG, identity, forge credentials, `THURBOX_SESSION`, and every `FLEET_*` root to tracked-example copies, and adopt it across all seven selftests (queue, install, fleet-status, onboarding, pane, sync, reconcile); `install-extension.sh` also gains `FLEET_GLYPH_ROOT` so glyph rendering can be isolated the same way. - Add `scripts/isolation-selftest.sh`, wired as `check.sh isolation` and a new CI job, which poisons a throwaway checkout and a hostile host and re-runs the leak-prone checks and selftests to prove the gate reads no operator state. ## Risk Assessment ✅ Low: The target commit is a narrow, correct one-line fix (capturing the probe's exit status before it is clobbered by an intervening echo), and the surrounding branch content — already vetted in prior review rounds — consistently implements the stated goal (gate reads no operator state; live records moved to fleet-status.sh --records; automerge judged against a fresh-clone-only root; all seven selftests route through the shared selftest_isolate helper) with no source-content-only test assertions and no touch to docs/design/testing.md. ## Testing Ran `./scripts/isolation-selftest.sh` directly (the smallest test that exercises this exact fix) and it passed cleanly end-to-end (all §1–§5 assertions green, exit 0). To prove the fix has teeth rather than just re-passing, I reproduced the pre-fix bug in isolation with two standalone scripts mirroring the buggy vs. fixed code under a hostile gpgsign-forcing git config: the buggy version always printed `committed=0` regardless of the real commit outcome (echo resetting `$?`), while the fixed version correctly reported `committed=128` when the commit genuinely failed. I then verified the fix in situ by temporarily reintroducing a real leak in scripts/lib/selftest-env.sh (letting `GIT_CONFIG_GLOBAL` survive `selftest_isolate`), re-ran isolation-selftest.sh, and confirmed the fixed §3 probe now correctly fails with `committed=128` for every selftest instead of silently passing — then reverted the temporary change, confirmed `git status` is clean, and re-ran isolation-selftest.sh to confirm it is green again. ## Pipeline Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes) <!-- no-mistakes-pipeline-attestation:v1 {"head_sha":"a656e44c7472fb5652b46f4931a3b37dad60848e","steps":[{"step":"intent","status":"completed"},{"step":"rebase","status":"completed"},{"step":"review","status":"completed"},{"step":"test","status":"completed"},{"step":"document","status":"completed"},{"step":"lint","status":"completed"},{"step":"push","status":"completed"},{"step":"pr","status":"running"},{"step":"ci","status":"pending"}]} --> <details> <summary>✅ **intent** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Rebase** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Review** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Test** - passed</summary> ✅ No issues found. - `./scripts/check.sh` - `./scripts/isolation-selftest.sh (full run, post-fix, all green)` - `manual repro: buggy vs fixed committed=$? capture pattern under a gpgsign-forcing hostile git config` - `manual regression check: temporarily broke scripts/lib/selftest-env.sh's GIT_CONFIG_GLOBAL unset, re-ran ./scripts/isolation-selftest.sh, confirmed the fixed §3 probe now fails with committed=128 (detecting the reintroduced leak), then reverted and confirmed green + clean git status` </details> <details> <summary>✅ **Document** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Lint** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Push** - passed</summary> ✅ No issues found. </details>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fleet task test-isolation/01-gate-reads-no-operator-state ("Fix isolation in fleet tests"; the Rust port is postponed). Goal: one commit gives the same ./scripts/check.sh result wherever it runs - CI, a worker worktree, or the operator's control-plane checkout with its live queue and gitignored configs. Leaks to confirm and close: (1) check_queue ran
) pins GIT_CONFIG_, GIT_DIR family, HOME/XDG, identity, forge credentials, THURBOX_SESSION and every FLEET_ settings root to tracked-example copies, keeping PATH and PYTHONUSERBASE; all seven selftests call it. (4) audit: check_yaml validated the gitignored registry map (removed from gate; onboarding-selftest now holds a generated map to the shape instead), check_voice rendered through the checkout's session-glyphs.conf/agent.conf (install-extension.sh now honours FLEET_GLYPH_ROOT), fleet-status-selftest's queue setup reached a real thurbox-cli (now runs under its bare PATH). Test first (operator rule): scripts/isolation-selftest.sh, wired asqueue.sh checkover the checkout's live orchestration/queue - moved out of the gate into an operator health check reachable from scripts/fleet-status.sh (implemented asfleet-status.sh --records, a flag rather than a screen section because validation opens archived topics' task files, which the default screen promises never to open; it validates both queue records via queue.py record_problems() and the registry map via check_yaml.py registry_problems(), and degrades/exits 0 like the rest of fleet-status). (2) check_automerge answered skip beside an operator auto-merge.conf - now judges a temp root holding only the tracked example. (3) selftests building git repos without pinning host git config - one shared helper scripts/lib/selftest-env.sh (selftest_isolatecheck.sh isolationand a CI job in All Checks, poisons a throwaway copy of the tree (malformed queue record, auto-merge/publish/agent/glyph/voice confs, registry map, reconcile dir) and a hostile host (signing gpg that fails, failing hooks, trunk default branch, tokens, tripwire gh/glab/thurbox-cli/quota-axi/ssh) and re-runs only the checks that can leak plus the cheap selftests; the two slow selftests (queue ~190s, reconcile ~90s) are held to the helper by §3 instead of being re-run, to keep runtime ~11s. It fails on main naming each leak and passes on the branch. Hard constraints: keep it simple, no test runner/sharding/new dependency; do not weaken what a check proves - move the live-data half, do not delete it; tracked files name no operator tool/vendor/agent; fixtures are made up, never copied from a live control plane; do not touch docs/design/testing.md (PR #85 owns it). Squash-merge only; the worker does not merge. The previous pipeline run already reviewed, tested and pushed this branch (its review fix made §3 behavioural) and failed only at PR creation on a GitHub HTTP 502.What Changed
check.shno longer runsqueue.sh checkagainst the checkout's liveorchestration/queueor validates the gitignored registry map; that validation moves intofleet-status.sh --records(backed by newqueue.py record_problems()andcheck_yaml.py registry_problems()helpers), andcheck_automergenow judges a temp root holding only the trackedauto-merge.example.confinstead of an operator's realauto-merge.conf.scripts/lib/selftest-env.shwith aselftest_isolate <dir>helper that pinsGIT_CONFIG_*/GIT_DIRfamily,HOME/XDG, git identity, forge credentials,THURBOX_SESSION, and everyFLEET_*settings root to tracked-example copies (keepingPATH/PYTHONUSERBASE); wires it into all seven selftests (fleet-status,install,onboarding,pane,queue,reconcile,sync).scripts/isolation-selftest.sh, wired ascheck.sh isolationand a new CI job, which poisons a throwaway copy of the tree (malformed queue record, auto-merge/publish/agent/glyph/voice confs, registry map, reconcile dir) and a hostile host (failing gpg/hooks, trunk default branch, tripwire CLIs) to prove the gate reads no operator state;install-extension.shnow honoursFLEET_GLYPH_ROOTandonboarding-selftest.shvalidates a generated registry map instead of the gitignored one.Risk Assessment
✅ Low: The diff (isolation gate, check.sh restructuring, fleet-status --records, check_yaml/queue.py refactors) is internally consistent, matches every constraint in the stated user intent, is exercised by new behavioral (non-source-grep) tests, and passes syntax/AST checks; the one previously-identified defect (isolation-selftest.sh's
committed=$?being clobbered by the interveningechoat lines 304-305) is unchanged from the state the user already reviewed and explicitly declined to fix, so it is not a new risk introduced by this diff.Testing
Baseline
./scripts/check.shhad already passed; targeted testing focused on the branch's actual deliverable, scripts/isolation-selftest.sh, run both standalone and via./scripts/check.sh isolation. Both runs demonstrate the end-user behavior directly: a poisoned checkout plus hostile host produce the identical clean verdict as a real runner across queue, automerge, yaml, and voice checks, the shared selftest_isolate helper defeats the hostile host, all seven selftests genuinely invoke that helper (proven behaviorally via a DEBUG-trap probe, not source grep), the four cheap selftests pass poisoned, and no tripwire tool was invoked — all ~90 assertions green in ~11s. No leftover artifacts in the worktree. No findings.Evidence: isolation-selftest.sh full run output
Evidence: check.sh isolation entry point
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
./scripts/check.sh./scripts/isolation-selftest.sh (direct run, all sections §1-§5 passed, ~11.6s)./scripts/check.sh isolation (CLI entry point, passed, ~11.1s)git status --porcelain=v1 -uall (confirmed no transient artifacts left in worktree after test run)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.