feat(security): enforce the store DEK's calendar expiry, with a named opt-out (BACKLOG #1004) - #815
feat(security): enforce the store DEK's calendar expiry, with a named opt-out (BACKLOG #1004)#815wshallwshall wants to merge 2 commits into
Conversation
The store DEK has two expiry axes and only one of them stopped anything. The usage axis refuses unconditionally at 2**32 encrypts. The calendar axis computed the same overdue condition and emitted a single alert, so the annual cadence the shipped docs document was unenforced. Adds enforce_store_key_expiry() to pipeline/secret_rotation.py, raising StoreKeyRotationOverdueError, and calls it from Engine.start OUTSIDE the blanket `except Exception` that guards the rotation-meta reconcile. The siting is the control: beneath that handler, whose entire body is a log call, the refusal would be logged and stepped over and the engine would start on an expired key. An UNDETERMINED age refuses too. When the reconcile fails, the handler swallows it and the stamps stay empty, so a gate written to read them would silently not fire. An undetermined age is not a young one, and it alerts as well as refusing. [secret_rotation].enforce_store_key_expiry (default true) is the opt-out. It suppresses the raise and nothing else, so the reminder still fires, and security_loosenings() names it on every boot and in GET /security/posture. That adds a required parameter to security_loosenings, which is deliberate: every parameter there is required so a deviation the registry cannot see is a type error rather than a second posture. Default true on merit, not on habit: the usage axis on the same key refuses unconditionally, and a default-off build would buy the setting without the posture, leaving ASVS 13.3.4 exactly where it was. Docs corrected to match shipped behaviour, which is a record correction that FOLLOWS the code rather than an edit substituting for it: the "does not force-rotate or hard-expire" sentence and the ENFORCE-escalation bullet in ASVS-L2-PHASE0-CHANGES.md, "still an alert, never a refusal" in CONFIGURATION.md, and a new SECURITY-LOOSENING.md entry. Mutation control, run and recorded: moving the call inside the reconcile handler reds three tests, and the swallowed run logs "secret-rotation stamp reconcile failed; continuing with config dates" while the engine starts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… silence (BACKLOG #1004) The overdue branch relied on `_maybe_escalate_dek` having already alerted on the same condition, which is true only when the reconcile completed. One combination broke that: the reconcile fails AND the operator has set `store_key_last_rotated`. There is then an effective date to judge, so the gate takes the overdue branch rather than the undetermined one, but no escalation alert ever went out. With the opt-out on, that started a knowably expired key with no signal at all. An empty stamp map is the flag, and it is exact: the engine assigns the stamps only on the reconcile's success path, and `_maybe_escalate_dek` runs at the end of that same path. So `not stamps` means nothing has alerted, and the gate sends the alert itself. On the normal path it stays silent rather than double-sending, because a duplicate enforced alert trains operators to ignore it. Both directions are pinned, so neither the alert nor its absence can drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Label disclosure, and a warning that matters more than the labelI did NOT apply the This pull request must not land yet, and the label does not change that. BACKLOG #1004 carries its own dispatch ruling: the engine half may be built freely and must not be landed until the vault half is lined up to land with it. That is why this is a draft and why I left it unlabelled. The Builder marked it draft for the same reason and said so at the top of the body. The hazard is specific and it is not the obvious one. The ASVS absence claim this change falsifies is keyed on the exception name, not on behaviour. The Builder chose Whoever holds the vault half must check the token set rather than inherit that reasoning. I am recording the label's provenance because I commissioned this diff and the playbook requires the commissioning party to say so when it labels its own workers' work. Here the accurate statement is that the label is not mine. |
DRAFT AND HELD — this is the ENGINE HALF of BACKLOG #1004 and it must not land alone
The item carries a dispatch ruling: the engine half may be built freely and must not be landed
until the vault half is lined up to land with it. The vault half — removing or rewriting the ASVS
13.3.4 absence claim, re-deriving the verdict, re-pointing anchors, re-reading the residual
down-triggers — belongs to the ASVS Tracker and is not this PR's. No vault file was read or
written here. The item's ordering amendment goes further and says re-key the absence check on
BEHAVIOUR strictly before this merges, because the two repos have no atomic cross-repo merge and
a coin flip lands wrong half the time.
The exception name is
StoreKeyRotationOverdueError. Read the next section before merging.Two commits, and the second fixes a defect in the first. See "A silent path found and closed"
below — it is called out rather than folded in because a fix written a pass later is the text a
reviewer should look at hardest.
The exception name, which this item rules is a coordination-visible decision
The 13.3.4 absence claim is keyed on the exception name, not on behaviour. It trips on a raise
whose name contains a rotation-or-overdue token and does not trip on names outside that set — so the
failure mode is not a red gate. It is that this lands clean, the gate stays green, and the recorded
absence quietly goes false. A red gate here is the safe outcome, because it is the record
noticing.
I chose
StoreKeyRotationOverdueErrorover the more obviousStoreKeyExpiredErrorfor that reason.It carries both a rotation token and an overdue token, it is accurate (the condition is computed
as
days_overduein a module namedsecret_rotation.py, beside an alert namedsecret_rotation_due), and if the claim is keyed on either token it fires.That is an intent, not a measurement, and the difference matters.
docs/security/is gitignoredin this repository, so from an engine checkout
git ls-files docs/securityreturns zero. I cannotread the token set and cannot confirm the claim actually trips. Whoever holds the vault half must
check it rather than inherit this paragraph. If it does not trip, the recorded absence has gone
false silently and someone has to be told. The durable fix the item names — re-key the claim on
behaviour so any refusal on this axis falsifies it whatever the exception is called — is not mine to
take.
The defect, re-verified at head before building
Confirmed with a control that must fire:
enforce_store_key_expiryappeared only indocs/BACKLOG.mdprose and nowhere in code, while the control needleenforce_grace_daysreturnedreal engine hits. The calendar axis at
pipeline/secret_rotation.py_maybe_escalate_dekstill onlyalerted;
store/crypto.pystill refused unconditionally on the usage axis. The defect stood.What ships
enforce_store_key_expiry()inmessagefoundry/pipeline/secret_rotation.py, raisingStoreKeyRotationOverdueError. It reuses the arithmetic_maybe_escalate_dekalready computes offthe three shipped knobs (
store_key_max_age_days,enforce_grace_days,warn_days), so the alertand the refusal can never disagree about what "overdue" means. A test pins that they report the
same
days_overdue.Engine.start, sited OUTSIDE the blanketexcept Exceptionthat guards therotation-meta reconcile. That handler's entire body is a log call, so a refusal beneath it is a
traceback and not a control. From here it propagates out of
Engine.start()and aborts the ASGIlifespan. The handler itself is untouched — its stated purpose is correct and narrowing it is a
different decision.
names: when the reconcile fails the handler eats it, the stamps stay empty, and a gate written to
read them silently does not fire. An undetermined age is not a young one. The alert is emitted in
addition, never instead, and a broken sink cannot swallow the stop (its own containment is scoped
to the sink call, and the raise sits outside it).
[secret_rotation].enforce_store_key_expiry, defaulttrue. It suppresses the raise andnothing else — the reminder still fires, because an operator who accepted the risk still needs to
be told the key is stale.
security_loosenings(), so an off-by-choice posture is named onevery boot and in
GET /security/posture. Reported as what the site gives up, not as "a setting isoff".
Default
trueon merit. The usage axis on the same key refuses unconditionally with no opt-outat all, so a calendar axis shipping OFF would be strictly weaker than its own sibling. And a
default-off build does not move the cell: 13.3.4 grades the engine against its own documented annual
cadence, so an unenforced default leaves the requirement exactly where it was. Per CLAUDE.md §0 there
are zero deployments, so "it would break existing installs" is not available as a reason — the
migration cost is zero and the correct end state is preferable to a staged one. That cuts one way
only: it removes a vacuous cost, it does not lower the bar.
security_loosenings()gained a required parameter, deliberatelysecret_rotation: SecretRotationSettingsis required, matching every other parameter there. Thefunction's own docstring gives the reason: an optional parameter is a detector that silently fails to
fire, while a required one makes omission a type error at every call site. Three source call sites
and six test call sites moved with it;
create_managed_appstashes the settings onapp.statetheway it already does for
cert_monitor_settings.Owed, not done, and named rather than left implied: the completeness floor in
tests/test_security_posture_defaults.pyiterates[security],[store]and[auth]model fields.It does not cover
[secret_rotation], so a future bool added there could join silently. This PRpins both directions for this switch (reported when off, silent at the default) but does not extend
the shared floor — that is the same gap
[alerts]carried before #323, and widening a shared helperin that file conflicts with other open work.
The measuring-document edit, and which kind it is
docs/ASVS-L2-PHASE0-CHANGES.mdsaid the engine "does not force-rotate or hard-expire a secret" andthat a DEK past max-age + grace "escalates" at restart. Both became false for the DEK's calendar
axis the moment this landed, and leaving them standing is the "compensating control must not rest on
a false premise" defect.
This is a record correction that FOLLOWS a shipped code change, not the forbidden lever. The
13.3.4 cell forbids editing that document as a substitute for a posture change, because an edit
alone moves the cell with zero posture change. Here the posture moved first and the document is being
brought back into agreement with it. Same for
docs/CONFIGURATION.md's "still an alert, never arefusal" and the new
docs/SECURITY-LOOSENING.mdentry. Note that editing eitherASVS-L2-PHASE0-CHANGES.mdsentence is itself one of the cell's re-score triggers, which is a thirdreason the vault change cannot lag.
Proof — five arms plus a run mutation control
tests/test_store_key_calendar_expiry.py, 24 tests, all green.test_overdue_calendar_age_refuses,test_an_overdue_key_aborts_Engine_start; plus both first-start trip cases the item requires (test_the_operator_override_date_drives_the_refusal,test_a_short_max_age_trips_at_first_start)test_undetermined_age_refuses_and_alerts,test_an_undetermined_age_aborts_Engine_start(reconcile monkeypatched to raise),test_a_broken_sink_cannot_swallow_the_undetermined_refusaltest_opt_out_suppresses_the_refusal_but_keeps_the_alert,test_opt_out_suppresses_the_overdue_refusal,test_the_opt_out_is_a_NAMED_security_loosening, and both engine-level opt-out startstest_the_overdue_branch_does_not_double_alert_on_the_normal_path,test_the_overdue_branch_DOES_alert_when_nothing_else_hastest_a_young_key_is_untouched,test_within_the_grace_window_is_untouched,test_no_refusal_under_warn_enforcement,test_a_keyless_store_has_no_dek_to_expire,test_a_fresh_keyed_engine_starts_on_the_shipped_defaults,test_the_shipped_default_is_not_reported_as_a_looseningtest_the_refusal_is_sited_OUTSIDE_the_blanket_reconcile_handler(AST: locates thetryby content, not line number, and asserts noenforce_store_key_expirycall inside it), withtest_the_reconcile_handler_is_still_the_blanket_one_this_test_guards_againstas its positive control — that one reds if the handler ever narrows or re-raises, which would make the siting test pin nothingThe mutation control was RUN, not asserted. Moving the call inside the guarded
tryandre-running:
and the swallowed run's captured log is exactly the defect being replaced:
— logged, stepped over, engine started. The mutation was then reverted and all pass. Both
engine-level tests are siting proofs in their own right: with the gate beneath the handler,
start()returns normally andpytest.raisesfails, so their green is evidence only because of thered above.
A silent path found and closed after the first commit (second commit)
The overdue branch leaned on
_maybe_escalate_dekhaving already alerted on the same condition. Thatholds only when the reconcile completed. One combination broke it: the reconcile fails and the
operator has set
store_key_last_rotated. There is then an effective date to judge, so the gate takesthe overdue branch rather than the undetermined one — but no escalation alert ever went out. With
the opt-out on, that started a knowably expired key with no signal at all.
An empty stamp map is the flag, and it is exact: the engine assigns the stamps only on the reconcile's
success path, and
_maybe_escalate_dekruns at the end of that same path. Sonot stampsmeansnothing has alerted, and the gate sends the alert itself. On the normal path it stays silent rather
than double-sending, because a duplicate enforced alert trains operators to ignore it. Both directions
are pinned.
Worth saying plainly: that defect was in text I had just written and was confident about, not in
the code I was sent to fix. The fix is the riskiest part of any change, which is why it is called out
here rather than folded in quietly.
Checks run in this worktree
The worktree had no
.venv; I created one with the same extrasscripts/worktree/new.ps1installs(
dev,harness,fhir,dicom,x12,xml,webauthnplus the web console) underconstraints.lock.ruff format --check .ruff check .mypy messagefoundry(strict)pytest tests/test_store_key_calendar_expiry.py-k "loosen or posture or security_config or documentation or docs or configuration")tests/test_security_static.py,test_settings.py,test_secret_rotation_inventory.py,test_api_security_posture.pyscripts/security/crypto_inventory_check.pytests/is outsideWALK_ROOTS)scripts/docs/backlog_status_check.pyscripts/docs/backlog_citation_check.pymessagefoundry security showsmoke, both directionsA second focused sweep over the suites most exposed to an
Engine.startchange — staged pipeline,leader tasks, API posture, API auth, CLI, wiring/serve, the GCM invocation bound, the server-backend
secret-rotation meta suite, startup attestation, lifespan unwind and passthrough — 290 passed, 2
skipped, run against the tree as it stands after both commits.
Not run here, and named rather than claimed. The full 15k-test suite did not finish inside this
turn. A broad
-ksweep was started before the second commit and I killed it rather than reportit: its collection predated the change, so its result would have described a tree that no longer
existed. The focused sweep above replaced it.
scripts/docs/citation_line_check.pyexits 1 on thistree, but every one of its 159 drifted anchors is pre-existing — zero name a file this PR touches —
and it is in neither
.pre-commit-config.yamlnor CI, so it is advisory.CI legs that must be read after my process exits — none of these run in a worktree:
teston all four Python versions, since this changesEngine.startand every keyed-store enginetest now passes through the new gate;
SecretRotationMetaStoreis implemented by all three shippedbackends (#1186), so the reconcile-plus-refusal path engages on each and only the hosted runners
exercise the server ones. They roll up into the required CI gate, so a red there blocks the
merge;
banditandpip-audit(no dependency changed, but the new raise path is new code);windows-service-smoke, which starts the engine under NSSM — the one leg that exercises a realservice start against the new gate.
Open questions for whoever picks this up
StoreKeyRotationOverdueError? I could notcheck. If it does, the drift gate goes red on this merge, which the item calls the safe outcome. If
it does not, the record went stale silently.
and records its own owner ratification as claimed by two sessions and unwitnessed. It asks that
the attribution be confirmed with the owner before being cited as a ruling. I have not confirmed
it; I am following the argument on its engineering merits, not on the claimed ratification.
[secret_rotation]completeness floor is owed, per the section above.🤖 Generated with Claude Code