Skip to content

feat(security): enforce the store DEK's calendar expiry, with a named opt-out (BACKLOG #1004) - #815

Draft
wshallwshall wants to merge 2 commits into
mainfrom
worktree-agent-a2c86c81e439fc4a4
Draft

feat(security): enforce the store DEK's calendar expiry, with a named opt-out (BACKLOG #1004)#815
wshallwshall wants to merge 2 commits into
mainfrom
worktree-agent-a2c86c81e439fc4a4

Conversation

@wshallwshall

@wshallwshall wshallwshall commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

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.

The reviewed label is on, and I did not put it there. It was applied by the owner at
2026-09-03 23:15 UTC, after my last push, so it names the current head 313ed3da3 rather than a
stale one. Draft status is now the only thing holding this PR, and the hold is real: the vault
half has not landed. Do not take the label as clearance to merge.

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 StoreKeyRotationOverdueError over the more obvious StoreKeyExpiredError for that reason.
It carries both a rotation token and an overdue token, it is accurate (the condition is computed
as days_overdue in a module named secret_rotation.py, beside an alert named
secret_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 gitignored
in this repository, so from an engine checkout git ls-files docs/security returns zero. I cannot
read 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_expiry appeared only in
docs/BACKLOG.md prose and nowhere in code, while the control needle enforce_grace_days returned
real engine hits. The calendar axis at pipeline/secret_rotation.py _maybe_escalate_dek still only
alerted; store/crypto.py still refused unconditionally on the usage axis. The defect stood.

What ships

  • enforce_store_key_expiry() in messagefoundry/pipeline/secret_rotation.py, raising
    StoreKeyRotationOverdueError. It reuses the arithmetic _maybe_escalate_dek already computes off
    the three shipped knobs (store_key_max_age_days, enforce_grace_days, warn_days), so the alert
    and the refusal can never disagree about what "overdue" means. A test pins that they report the
    same days_overdue.
  • The call site in Engine.start, sited OUTSIDE the blanket except Exception that guards the
    rotation-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 ASGI
    lifespan. The handler itself is untouched — its stated purpose is correct and narrowing it is a
    different decision.
  • An undetermined age REFUSES, and alerts as well. This is the second-order swallow the item
    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, default true. It suppresses the raise and
    nothing else — the reminder still fires, because an operator who accepted the risk still needs to
    be told the key is stale.
  • The opt-out is wired into security_loosenings(), so an off-by-choice posture is named on
    every boot and in GET /security/posture. Reported as what the site gives up, not as "a setting is
    off".

Default true on merit. The usage axis on the same key refuses unconditionally with no opt-out
at 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, deliberately

secret_rotation: SecretRotationSettings is required, matching every other parameter there. The
function'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_app stashes the settings on app.state the
way 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.py iterates [security], [store] and [auth] model fields.
It does not cover [secret_rotation], so a future bool added there could join silently. This PR
pins 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 helper
in that file conflicts with other open work.

The measuring-document edit, and which kind it is

docs/ASVS-L2-PHASE0-CHANGES.md said the engine "does not force-rotate or hard-expire a secret" and
that 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 a
refusal"
and the new docs/SECURITY-LOOSENING.md entry. Note that editing either
ASVS-L2-PHASE0-CHANGES.md sentence is itself one of the cell's re-score triggers, which is a third
reason the vault change cannot lag.

Proof — five arms plus a run mutation control

tests/test_store_key_calendar_expiry.py, 24 tests, all green.

Arm Covered by
Overdue calendar age refuses 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)
Undetermined age refuses 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_refusal
Opt-out disables it and announces itself test_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 starts
Nothing starts in silence (second commit) test_the_overdue_branch_does_not_double_alert_on_the_normal_path, test_the_overdue_branch_DOES_alert_when_nothing_else_has
A non-overdue key is unaffected test_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_loosening
Siting, not merely the raise test_the_refusal_is_sited_OUTSIDE_the_blanket_reconcile_handler (AST: locates the try by content, not line number, and asserts no enforce_store_key_expiry call inside it), with test_the_reconcile_handler_is_still_the_blanket_one_this_test_guards_against as its positive control — that one reds if the handler ever narrows or re-raises, which would make the siting test pin nothing

The mutation control was RUN, not asserted. Moving the call inside the guarded try and
re-running:

FAILED test_the_refusal_is_sited_OUTSIDE_the_blanket_reconcile_handler
FAILED test_an_overdue_key_aborts_Engine_start
FAILED test_an_undetermined_age_aborts_Engine_start
3 failed, 18 passed

and the swallowed run's captured log is exactly the defect being replaced:

ERROR messagefoundry.pipeline.engine: secret-rotation stamp reconcile failed; continuing with config dates

— 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 and pytest.raises fails, so their green is evidence only because of the
red above.

A silent path found and closed after the first commit (second commit)

The overdue branch leaned on _maybe_escalate_dek having already alerted on the same condition. That
holds 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 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.

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 extras scripts/worktree/new.ps1 installs
(dev,harness,fhir,dicom,x12,xml,webauthn plus the web console) under constraints.lock.

Check Result
ruff format --check . pass
ruff check . pass
mypy messagefoundry (strict) pass, 267 files
pytest tests/test_store_key_calendar_expiry.py 21 passed
Affected suites (posture defaults, alert SMTP TLS, client-network allowlist, memory-encryption readout, security config, all three secret-rotation suites) 215 passed, 1 skipped
Doc/posture sweep (-k "loosen or posture or security_config or documentation or docs or configuration") 576 passed, 2 skipped
tests/test_security_static.py, test_settings.py, test_secret_rotation_inventory.py, test_api_security_posture.py 217 passed
scripts/security/crypto_inventory_check.py OK, 77 sites, no drift (no new crypto import; tests/ is outside WALK_ROOTS)
scripts/docs/backlog_status_check.py OK, 657 items
scripts/docs/backlog_citation_check.py OK
messagefoundry security show smoke, both directions names the switch with the opt-out set; reports nothing at the default
pre-commit at commit time all hooks passed, including the ledger gate, SPDX, bandit and the leak guard

A second focused sweep over the suites most exposed to an Engine.start change — 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 -k sweep was started before the second commit and I killed it rather than report
it
: 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.py exits 1 on this
tree, 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.yaml nor CI, so it is advisory.

CI legs that must be read after my process exits — none of these run in a worktree:

  • test on all four Python versions, since this changes Engine.start and every keyed-store engine
    test now passes through the new gate;
  • the SQL Server and Postgres legs. SecretRotationMetaStore is implemented by all three shipped
    backends (#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;
  • bandit and pip-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 real
    service start against the new gate.

Open questions for whoever picks this up

  1. Does the 13.3.4 absence claim actually trip on StoreKeyRotationOverdueError? I could not
    check. 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.
  2. Ordering. The item's 2026-08-21 amendment says re-key on behaviour strictly before this lands,
    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.
  3. The [secret_rotation] completeness floor is owed, per the section above.

🤖 Generated with Claude Code

wshallwshall and others added 2 commits September 3, 2026 17:58
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>
@wshallwshall wshallwshall added the reviewed A reviewer has read this. Removed automatically when new commits arrive. label Sep 3, 2026
@github-actions github-actions Bot added ci-red A required check went red. Attribute it before retrying. and removed reviewed A reviewer has read this. Removed automatically when new commits arrive. labels Sep 4, 2026
@wshallwshall wshallwshall added the reviewed A reviewer has read this. Removed automatically when new commits arrive. label Sep 4, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Label disclosure, and a warning that matters more than the label

I did NOT apply the reviewed label on this pull request, and I deliberately withheld it. Another session applied it.

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 StoreKeyRotationOverdueError precisely so a name-keyed claim would trip and the gate would go red, which is the safe outcome. It could not verify that from an engine checkout, because the token set is vaulted. So the intended failure may simply not fire, and the recorded absence would go false silently with nothing reporting it.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying. reviewed A reviewer has read this. Removed automatically when new commits arrive.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant