Skip to content

Make propose and commit agree on time-varying authority - #107

Merged
b-macker merged 1 commit into
masterfrom
claude/naab-inadmissible-action-prevention-4cmn1m
Jul 30, 2026
Merged

Make propose and commit agree on time-varying authority#107
b-macker merged 1 commit into
masterfrom
claude/naab-inadmissible-action-prevention-4cmn1m

Conversation

@b-macker

Copy link
Copy Markdown
Owner

Summary

#105 established the invariant: every check whose truth can change with the passage of time belongs on the commit half, because authority is time-varying and a proposal is not. Two holes in that invariant remained, both on the propose/commit path.

A1 — proposals now carry the config generation they were evaluated under

agent.commit() deliberately does not call reloadIfChanged() — traced and rejected in #105, because an accepted reload runs onAgentConfigChanged(), which re-baselines mandate_keywords and would score an already-generated candidate against a mandate it never received, besides dangling config through the atomic rules_ptr_ swap. That decision was right, but its consequence was left open: a candidate produced under one configuration could still be committed under another.

Proposals are now stamped with reload_count_ at propose time and commit refuses when it has moved — declining to commit under conditions the candidate never saw, rather than re-scoring it under them.

reload_count_ and not governance_epoch_: the epoch also increments on pulse verdict transitions (governance_engine.cpp:6906), and the pulse sawtooths, so proposals would be voided during normal degraded operation.

This was not additive. reload_count_ was a plain int (governance.h:3478) while governance_epoch_ beside it is std::atomic<int>. It is written under reload_mutex_ but getReloadCount() read it unsynchronised — reading it from agent worker threads during batch/fan_out would have been a data race. It is now atomic. The accessor had no callers, so the change is contained to four sites.

A2 — propose consults the lease even with step-up disabled

agentPropose's lease test sat inside if (cb.step_up_enabled), and step_up_enabled defaults to false (governance.h:1600). So by default propose did not consult the lease at all, while agent.send() hard-throws (agent_impl.cpp:1461) and agent.commit() checks it unconditionally (#105). Propose was the only entry point where expired authority passed silently.

The lease test is now unconditional. The level test stays inside the guard — required_level derives from step_up_at_level, so it is intrinsically about step-up and means nothing when step-up is off. Refusal wording matches the two existing shapes so callers keep matching on them.

Regression surface is empty, verified. Only four configs in the repo enable propose:

config step_up lease
test_split_commit.sh absent → false none
test_absorption_degenerate.sh absent → false none
test_propose_commit.sh true configured
living-script_extended/src/govern.json true configured

Nothing combines propose + a lease + step-up off. The two with step-up off have no lease, so lease_configured is false and the hoisted check is a no-op for them. The change reaches only the configuration nobody was using — which is the hole.

Two mechanics that cost real time to find

Both are recorded in the test, since anyone writing a similar case will hit them:

  • reloadIfChanged() compares mtime at second granularity (governance_config.cpp:3753). A rewrite landing in the same second as the initial load is invisible — the first attempt at H-01 silently did nothing.
  • Subprocess env is scrubbed, so a re-sign must be passed --signing-key explicitly; NAAB_SIGNING_KEY does not survive into the child. This is the same workaround living-script.naab already uses.

Test Plan

  • test_propose_commit.sh23/23, Groups A–G unchanged, H and I new
  • Vacuity-checked, the signature that matters being which assertion fails:
    • stamp comparison disabled → H-01 fails, H-02 still passes (the no-change control survives)
    • old step_up_enabled nesting restored → I-01 fails, I-02 and I-03 still pass (the no-op and anti-regression controls survive)
  • tests/security/test_error_msg_leaks.sh — 874/0 (new refusal text)
  • bash run-all-tests.sh441 tests, 0 unexpected failures
  • asan/ubsan — not runnable locally without a separate sanitizer build; CI covers both, and they are the jobs that would catch the reload_count_ atomic change being wrong. Worth watching on this PR specifically.

Honest scoping

The reload stamp will rarely fire in living-script_extended — nothing between propose, select_admissible and commit triggers a reload there, and the one path that does send (the #106 re-auth retry) already invalidates the proposal. This is an invariant, not a detector. It earns its place in multi-agent scripts where a sibling agent's send, a polyglot block (polyglot.cpp:158) or the VM (vm.cpp:2815) reloads mid-gap.

PR B (example taint/integrity coverage) is deliberately separate and does not depend on this.


Generated by Claude Code

#105 established that every check whose truth can change with the passage of
time belongs on the commit half, because authority is time-varying and a
proposal is not. Two holes in that invariant remained, both on this path.

Config changes were not re-checked across the propose→commit gap. commit()
deliberately does not call reloadIfChanged() — an accepted reload runs
onAgentConfigChanged(), which re-baselines mandate_keywords and would score an
already-generated candidate against a mandate it never received, besides
dangling `config` through the atomic rules_ptr_ swap. That decision was right,
but its consequence was left open: a candidate produced under one configuration
could still be committed under another. Proposals now carry the accepted-config
generation they were evaluated under, and commit refuses when it has moved —
declining to commit under conditions the candidate never saw, rather than
re-scoring it under them.

reload_count_ and not governance_epoch_: the epoch also moves on pulse verdict
transitions, and the pulse sawtooths, so proposals would be voided during
normal degraded operation.

That stamp is not additive. reload_count_ was a plain int while
governance_epoch_ beside it is atomic; it is written under reload_mutex_ but
getReloadCount() reads it unsynchronised, so reading it from agent worker
threads during batch/fan_out would have been a data race. It is now
std::atomic<int>. The accessor had no callers, so the change is contained to
four sites.

Second hole: agentPropose's lease test sat inside `if (cb.step_up_enabled)`,
and step_up_enabled defaults to false — so by default propose did not consult
the lease at all, while send() hard-throws on an expired lease and commit()
checks it unconditionally. Propose was the only entry point where expired
authority passed silently. The lease test is now unconditional; the level test
stays inside the guard because required_level derives from step_up_at_level and
means nothing when step-up is off. Refusal wording matches the two existing
shapes so callers keep matching on them.

No config in the repo combines propose with a lease and step-up off — the two
propose configs with step-up off have no lease at all, so the hoisted check is
a no-op for them. The change reaches only the configuration nobody was using,
which is the hole.

Groups H and I cover both. Two mechanics cost real time to find and are
recorded in the test: reloadIfChanged() compares mtime at second granularity,
so a rewrite in the same second is invisible; and subprocess env is scrubbed,
so the re-sign must be passed --signing-key explicitly.

Vacuity-checked: disabling the stamp fails H-01 while H-02 still passes,
restoring the old nesting fails I-01 while I-02 and I-03 still pass.

Full suite: 441 tests, 0 unexpected failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
@github-actions

Copy link
Copy Markdown

NAAb Governance Report

Metric Count
Files checked 16
Passed 16
Failed 0

All governance checks passed!

Generated by NAAb Governance Engine v4.0

@b-macker
b-macker marked this pull request as ready for review July 30, 2026 23:43
@b-macker
b-macker merged commit f357275 into master Jul 30, 2026
23 checks passed
@b-macker
b-macker deleted the claude/naab-inadmissible-action-prevention-4cmn1m branch July 30, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants