[bench] runner+hook: fail closed on governance errors#12
Conversation
A stage PIPELINE_ERROR (or a constitution load failure) previously short-circuited to a fail-open PASS, and the hook fell back to allow on pipeline-import failure or its own internal error. That let a broken or exploited judge wave changes through without adjudication (both Codex P1s on PR #11 rode this fail-open). Flip these to fail closed: - runner.py: constitution-load failure and Challenger/Defender/Oracle PIPELINE_ERROR now return VETO (pipeline_error=True) with remediation, so an unadjudicated change is blocked, not allowed. - hooks/pre-tool-use.py: pipeline-import failure and the outer exception handler now deny instead of allow. The BENCH_SUBPROCESS reentrancy guard stays fail-open (a Bench-spawned claude -p child must not recurse), and a ledger-write failure still returns the rendered verdict (recording is not adjudication). - Tests updated to assert fail-closed VETO/deny; added a hook pipeline-exception test. README fail-closed note corrected. Tradeoff (intended): a genuinely broken pipeline now blocks all governed Write/Edit until fixed. Recovery is an out-of-band human action (edit files directly, outside the governed tools), never an automatic pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2a2563940
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 on PR #12. Fail-closed error VETOs are enforced correctly (the hook denies), but they were illegible in the ledger's own accounting: append_entry only persisted the stage dicts, and consumers derived the verdict from oracle.verdict. A constitution-load failure (no stage at all) vanished from stats entirely, and stage-error VETOs were mislabeled FAIL-OPEN. For a system whose thesis is "the ledger is the proof," that is a real gap. - ledger/chain.py: record the authoritative top-level `verdict` and `pipeline_error` on every entry (safe: additive fields, per-entry hash, existing entries and the chain untouched). - utils/stats.py: add entry_verdict() (top-level verdict, then oracle fallback, then None) and teach entry_has_pipeline_error() the top-level flag; compute_ledger_stats uses them. - cli/commands.py, utils/viewer.py: read entry_verdict; the FAIL-OPEN label now only applies to genuinely old fail-open entries (verdict-less + pipeline error), so history stays accurate. - tests: entry_verdict precedence, top-level pipeline_error, and fail-closed VETOs counted as both vetoed and pipeline_error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/Nuralyn/Bench/blob/fb95f24b4b62270a6bb3f251416de07696d0819b/ledger/bench-ledger.json#L26022-L26025
Remove external project receipts from the committed ledger
This appended receipt is for a different local project under C:\Users\mstar\.claude\projects\D--Business-Builder and the following added entries include detailed Huzzah product notes/source snippets. If the Bench repo is pushed or shared, these machine-local governance receipts disclose private project paths and proprietary strategy/code through ledger/bench-ledger.json and the viewer/CLI; please scrub/regenerate the ledger so this commit only contains receipts intended for this repository.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… P2) Codex re-review P2 on PR #12. The README "Fail-Open by Design" section still described pipeline errors as allowed through with a warning, directly contradicting the new fail-closed note added earlier in this PR. Rewrote it to "Fail-Closed by Design": on an unadjudicable change the pipeline denies with a stderr warning and a pipeline_error VETO in the ledger, with the reentrancy guard as the lone exception. (CLAUDE.md carries no fail-open error statement, so nothing to reconcile there.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Governance receipts appended after the prior commits were staged. Recorded here so the ledger stays continuous with the fail-closed change set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A governance receipt landed after the branch's final commit was staged. Recording it on main keeps the chain continuous. No source changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
pipeline/runner.py: constitution-load failure and Challenger/Defender/OraclePIPELINE_ERRORnow return a bindingVETO(withpipeline_error=Trueand remediation) instead of a fail-openPASS.hooks/pre-tool-use.py: pipeline-import failure and the outer exception handler nowdenyinstead ofallow. TheBENCH_SUBPROCESSreentrancy guard stays fail-open (a Bench-spawnedclaude -pchild must not recurse), and a ledger-write failure still returns the already-rendered verdict (recording is not adjudication).VETO/deny; added a hook pipeline-exception test. README note corrected.This strengthens enforcement (pro-
C-007);bench.jsondoes not encode the policy, so no constitution version bump.Tradeoff (intended)
A genuinely broken pipeline (API outage, bad model config, unimportable pipeline) now blocks all governed Write/Edit until fixed. Recovery is an out-of-band human action (edit files directly, outside the governed tools), never an automatic pass. This is the correct security model for a governance system: a human can always intervene; a model cannot.
Test plan
python -m unittest discover -s tests-> 331 pass, 1 skipped.BENCH_SUBPROCESSbypass in user-facing remediation, which was removed.🤖 Generated with Claude Code