Stop the chain verifier from reporting tampering on its own output - #109
Merged
b-macker merged 1 commit intoJul 31, 2026
Merged
Conversation
L24-06 — the level added in #106 precisely because the example had been building a tamper-evident chain nothing ever verified — failed on real run files: "RunEnd declares 717 chained events but 770 observed". That is a hard BREAK, exit 1, on files nobody had touched. emitEndOfRunHealthWarnings() is the only chained-telemetry writer in the engine that did not go through chainPrevLocked(). It seeded prev_hash from the in-memory last_telemetry_hash_ and never incremented chained_events_this_run_. Three symptoms from those two lines: - RunEnd under-declared its count, so the verifier reported tampering. - When the warnings were a process's first chained events the in-memory hash was empty, so prev_hash fell back to genesis mid-file — a spurious LEGACY RESTART on any shared telemetry file. - The lazy RunStart anchor landed behind the events it anchors, because only chainPrevLocked() emits it. Reproduced on a print("hello") script with governance_health, CDD and BSD enabled and no agent activity: 4 chained events, RunEnd declaring 2. The unit is 2 per affected run — the two inert-instrumentation warnings — which matches the diff observed across four live run groups. The originally reported 53 was an artifact of aggregating run groups, not one large gap. A verifier that cries tamper on its own output is worse than no verifier: it trains the reader to ignore the one signal meant to be unignorable. The fix is what every other chained writer already does — chainPrevLocked(fp) plus the increment, inside the lock the lambda already held. Group E covers it, and E-01 is the control: without health warnings actually firing, E-02.. E-04 would be vacuous. Verified by reverting the fix and confirming all three fail while E-01 still passes; Group A passes either way, which is why this survived. Also here: docs/governance-campaign-findings.md gains a transition-admissibility phase section covering the five defects from #104-#108 plus this one, each with mechanism, pinning test, and a separately-scoped live status. A fourth method note records the defect class this phase kept producing — an assertion that looks specific but is satisfiable without the property holding (C-05 matching an empty value, L24-02's unsound equality, L25-03's wrong-scope total). Running the degraded case was the only defence that ever caught it. L25-03's baseline comment records the second keyed observation: 7-8 against a ceiling of 18, from a more complete run than the one that produced 18. The count is not monotone in run length, so two non-monotone points do not support tightening — a baseline that fails on a healthy run teaches everyone to raise it. Left at 18 with the reasoning written down; tightening needs the variance attributed to a site first. Full suite: 441 tests, 0 unexpected failures. Security leak check: 874/0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
NAAb Governance Report
All governance checks passed! Generated by NAAb Governance Engine v4.0 |
b-macker
marked this pull request as ready for review
July 31, 2026 23:12
This was referenced Jul 31, 2026
b-macker
added a commit
that referenced
this pull request
Aug 1, 2026
* Reconcile the chained-event count when a run reports twice A keyed run showed L24-06 still failing after #109: BREAK in run ...: RunEnd declares 737 chained events but 785 observed #109 was confirmed by that run — every small run group came back diff 0, exactly where the end-of-run health warnings fire. But I called that writer the complete explanation, and it was not. Every large run in the earlier sample balanced exactly, which is consistent with one writer at unit 2 — only because none of those runs tripped the quality gate. The second cause was invisible in that sample, not absent. writeReports() is called from ~17 sites and a clean execute() is not the last of them: main.cpp's contract-error, quality-gate and baseline-regression exits all call it again after the VM has already written and sealed the run. The five file report formats are idempotent because they truncate and rewrite, but telemetry appends and check_results_ is never cleared, so the second call re-emitted the entire set after RunEnd had declared the count, and run_end_emitted_ stopped the declaration ever catching up. The residual is one full second dump: GovernanceCheck 38 + RuleViolation 8 + summary 1 + snapshot 1. The double-call was already known — the comment "writeReports() already called inside execute() on success" sits three lines above two more sites that do it. Fixed at the invariant rather than the instance. RunEnd re-emits whenever chained events followed the previous anchor; the verifier reads the last RunEnd per run_id, so the newest declaration reconciles, and any future writer that appends after an anchor is self-healing. Separately the dump resumes from telemetry_results_dumped_ instead of restarting, the dedup key set persists across calls so the guarantee still holds, and ScoringSnapshot re-emits only when the score moved. The duplicate records mattered beyond the verifier: every count derived from the telemetry file was inflated, including L25-03's taint total, which had been reading roughly double. Attributed by source location the same run has 8 distinct sink sites and 7-8 violations per segment, not the 4 sites that comment claimed. The baseline is left at 18 as a loose ceiling and marked for re-measurement on a keyed run built after this fix, since only a run can say what the corrected count is. Reproduced in miniature: pre-fix 10 records / 13 chained / declares 8 / BREAK, post-fix 5 / 8 / 8 / clean. Group F asserts it, and F-01 is the control — unless the quality gate actually fires, writeReports ran once and F-02..F-04 prove nothing. Reverting the fix fails F-02, F-03 and F-04 with the control still passing. Full suite: 441 tests, 0 unexpected failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC * Tighten the taint baseline now that the count is trustworthy The post-fix keyed run settled what two data points could not. L25-03 measured 7, 8, 18, then 8 — and the 18 is the single run where the writeReports double-dump fired, counting most violations twice. The apparent run-to-run variance was that artifact, not behaviour: a violation fires about once per distinct sink site, so the total tracks the ~8 sites rather than the length of the run. The earlier comment reasoned the count "is NOT monotone in run length" and declined to tighten on that basis. Declining on two points was right; the premise was reading the artifact as signal. Inflation is now impossible, so 18 -> 12: ~50% headroom over every non-inflated measurement, while still catching a leak. The build path runs 16 extraction sites across every feature iteration, so losing its sanitizer adds far more than the 4 violations of slack. Also records the run that confirms the reconciliation fix live — all four run groups at diff 0, including a 708-event group carrying two RunEnd anchors, the second declaring 708 of 708. That group reached its second writeReports() through an exit path other than the quality gate, so the invariant held somewhere the constructed repro never went. Replayed the assertion: 8 passes, 13 fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC --------- Co-authored-by: Claude <noreply@anthropic.com>
4 tasks
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.
Summary
L24-06— the level added in #106 precisely because the example had been building a tamper-evident chain nothing ever verified — failed on real run files:That is a hard BREAK, exit 1, on files nobody had touched.
emitEndOfRunHealthWarnings()is the only chained-telemetry writer in the engine that did not go throughchainPrevLocked(). It seededprev_hashfrom the in-memorylast_telemetry_hash_and never incrementedchained_events_this_run_. Three symptoms from those two lines:RunEndunder-declared its count, so the verifier reported tampering.prev_hashfell back to genesis mid-file — a spuriousLEGACY RESTARTon any shared telemetry file.RunStartanchor landed behind the events it anchors, because onlychainPrevLocked()emits it.Reproduced on a
print("hello")script withgovernance_health, CDD and BSD enabled and no agent activity: 4 chained events,RunEnddeclaring 2. The unit is 2 per affected run — the two inert-instrumentation warnings — which matches the diff observed across four live run groups. The originally reported 53 was an artifact of aggregating run groups, not one large gap.A verifier that cries tamper on its own output is worse than no verifier: it trains the reader to ignore the one signal meant to be unignorable.
Changes
src/runtime/governance_engine.cpp—emitEndOfRunHealthWarnings()now does what every other chained writer already does:chainPrevLocked(fp)plus the counter increment, inside the lock the lambda already held. No new lock, no lock-ordering change;chained_events_this_run_was already guarded bytelemetry_hash_mutex_at every other site.tests/governance_v4/test_evidence_chain.sh— Group E (4 assertions). E-01 is the control: without health warnings actually firing, E-02..E-04 would be vacuous. E-02 verifies a shared file across three runs, E-03 checks declared-vs-observed counts perrun_id, E-04 checks anchor ordering.docs/governance-campaign-findings.md— new Transition-admissibility phase section covering the five defects from Test that a blocked action leaves no trace of having happened #104–Govern the example's LLM-output-to-execution flow #108 plus this one, each with mechanism, pinning test, and a separately-scoped live status. Adds a fourth method note recording the defect class this phase kept producing: an assertion that looks specific but is satisfiable without the property holding (C-05 matching an empty value, L24-02's unsound equality, L25-03's wrong-scope total). Running the degraded case was the only defence that ever caught it.CLAUDE.md— states the invariant as an invariant: every writer emitting aprev_hash/hashpair must usechainPrevLocked(fp)and increment the counter, both undertelemetry_hash_mutex_.examples/living-script_extended/run.sh— records the second keyed observation against the L25-03 taint baseline (see below). Comment only; the threshold is unchanged.Test Plan
bash run-all-tests.shwith no new failures — 441 tests, 0 unexpected failures (376 passed, 52 error-behavior, 1 missing-executor, 12 needs-tree-walk)test_evidence_chain.shGroup E, 24 assertions total in the file, all passingbash tests/security/test_error_msg_leaks.sh— 874 checks, 0 failuresBefore and after, three runs sharing one telemetry file:
Not in this change
Tightening the L25-03 taint baseline. The second keyed run put the count at 7–8 against a ceiling of 18, which is loose — a leak would need to add ~10 violations to trip it. But the 18 came from a less complete run (feature 2 never finished) and the 7–8 from a more complete one, so the count is not monotone in run length. Two non-monotone points do not support picking a lower number, and a baseline that fails on a healthy run teaches everyone to raise it. Left at 18 with both observations and the reasoning recorded next to it; tightening properly means attributing the spread to one of the four contributing sites, which is its own change.
Related Issues
Follow-up to #104, #105, #106, #107, #108. The defect was surfaced by the verifier added in #106.
Generated by Claude Code