fix(policy): refuse the inert reload config, and stop re-reading per call - #492
Merged
Merged
Conversation
…call Two fixes from docs/spec/policy-hot-reload.md that did not depend on which reload direction was chosen, plus the direction now being recorded. POLICY-003: policy_reload_interval_seconds > 0 alongside a pinned CMCP_POLICY_HASH now aborts startup. Every reload was re-validated against that startup hash, so a bundle that actually changed was always rejected while the old policy kept being enforced, at WARNING, with an error message about the gateway not starting on a gateway that was already running. It could never do what an operator setting it intends, so it is refused up front rather than discovered in production. Dev mode pins no hash and is unaffected, which is the one place reload works. The load amplifier: _last_reload_at is now stamped before the attempt rather than after a success. Advancing it only on success left the staleness check true whenever a reload failed, and because the reload always failed in production, every subsequent tool call re-read every policy file and recomputed the bundle hash on the enforcement path. Measured before the fix: 50 evaluations produced 50 full bundle reads. Now one attempt per interval. Verified by reverting the stamp and watching the new test fail. Tests now cover pinned and unpinned side by side, which is the gap that hid all of this: every previous reload test constructed PolicyStore without expected_hash, the dev-mode shape production never uses. One asserts a pinned hash cannot install a changed bundle, one asserts it can without the pin, and one counts the reads. Direction decided: option A, pin a signing key rather than an artifact hash, with a monotonic bundle version so a validly signed older bundle is not a downgrade attack. Not built. The doc records what the decision commits to and the sub-questions the implementation issue must answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
The two fixes from policy-hot-reload.md (#490) that did not depend on which direction was chosen, plus the direction now recorded.
1. A configuration that could never work now refuses to start
policy_reload_interval_seconds > 0alongside a pinnedCMCP_POLICY_HASHaborts startup withPOLICY_RELOAD_PINNED_HASH.Every reload was re-validated against the hash pinned at startup, so the check asked a question that answers itself: has the bundle changed, and does it still hash to what it was before it changed? A changed bundle was always rejected, the old policy kept being enforced, and the only signal was a
WARNINGreadinggateway will not start— emitted on a gateway that was already running and not going to stop.Dev mode pins no hash and is unaffected. That is the one configuration where reload actually works, and there is now a test asserting the refusal does not swallow it.
This refusal is durable rather than a stopgap: under the chosen direction a hash pin and a mutable policy stay alternatives rather than layers.
2. The load amplifier
_last_reload_atis now stamped before the attempt instead of after a success.Advancing it only on success left the staleness check true whenever a reload failed — and in production the reload always failed.
reload_if_stale()runs once per policy evaluation, so every subsequent tool call re-read every policy file and recomputed the bundle hash, on the enforcement path.Measured before the fix: 50 evaluations → 50 full bundle reads. After: one attempt per interval. I verified the new test by reverting the stamp and watching it fail.
3. The test gap that hid both
Every previous reload test constructed
PolicyStorewithoutexpected_hash— the dev-mode shape production never uses.test_policy_store_bundle_swap_on_hash_changepassed and correctly proved the swap logic, in the one configuration that never runs.Now covered side by side:
test_pinned_hash_reload_cannot_install_a_changed_bundle— the defect, documented rather than endorsed, with a note to replace it rather than delete it quietly if a signing-key model makes runtime change worktest_unpinned_reload_does_install_a_changed_bundle— the same edit without the pin, so the difference the pin makes is visible in the suitetest_a_failing_reload_costs_one_attempt_per_interval_not_one_per_call— counts the reads4. Direction recorded: pin a signing key
Option A. Authority for a runtime policy change comes from a pinned public key, not a pinned artifact hash. Not built; the doc records what the decision commits to:
author_identity/commit_shaare currently unsigned assertions)versionmust increase monotonically, or a validly signed older bundle is a downgrade attack — every signature checks out while yesterday's more permissive policy is replayedDocs
STATUS.md,docs/configuration.mdand the README now describe the refusal rather than the trap, andPOLICY_RELOAD_PINNED_HASHis registered inerror-codes.md.Tests
1076 passed, 24 skipped.
ruffandmypyclean.Note: this repo's venv had
agent-manifest0.7.0 against a>=0.10requirement, so 9 test modules failed to import locally until I upgraded it. CI installs fresh and was unaffected, but it is the second repo today with that skew.🤖 Generated with Claude Code