fix(evidence): loud error when retired-keys config is unusable (v0.1.25.35)#200
Merged
Conversation
…25.35) The active-key nbf clamp (v0.1.25.33) only fires when a retired entry has a usable window. If cycles.evidence.signing.retired-keys is configured but produces ZERO usable entries — malformed JSON, or every entry dropped for bad bounds — there is nothing to clamp against, so the active key publishes unbounded at the configured nbf (default 0 = since epoch). That silently reverts a rotated server to the never-rotated posture: pre-rotation evidence won't resolve, and the current key could resolve a backdated envelope as authentic. JwksController now distinguishes "retired-keys not configured" (blank, no noise) from "configured but yielded zero usable entries", logging a clear ERROR for the latter that names the consequence. Deliberately NOT fail-closed (that would also break verification of all current evidence); the active key still publishes — the never-fail-closed guarantee is retained. Surfaced by the codex review of the rotation blog post (cycles-docs#724). JwksControllerTest +1 (OutputCaptureExtension: configured-but-unusable logs at ERROR and still publishes the active key). Full mvn verify green, jacoco 95% gate met. Observability-only — no wire/spec change, no change to the JWK Set.
Review finding (High): the ERROR keyed off parseRetiredKeys returning empty, but the parser only validates integral/in-range bounds — JwksDocuments drops more on emission (empty/inverted window where exp_ms<=nbf_ms, malformed hex, duplicate kid, overlap). An empty window (nbf_ms==exp_ms) passes the parser (so the list is non-empty -> no ERROR), is excluded from the clamp/WARN (which filter exp>nbf), and is dropped by JwksDocuments -> only the unbounded active key publishes, with NO signal. The exact silent collapse this change closes. Base the decision on what JwksDocuments actually publishes: count retired keys in the emitted set (publishedRetiredCount, reusing jwkSet's full emission validation) and fire the ERROR when retired-keys is configured + raw-hex signer + zero published retired keys. The WARN/clamp basis is unchanged. JwksControllerTest +1 (empty window nbf==exp: parser-passing, emission-dropped, ERROR still fires); existing test updated to the new "no publishable retired entries" message. JwksController 100% line-covered; full mvn verify green, jacoco 95% gate met.
…e key is bounded Review finding (Low): the ERROR claimed the active key "remains unbounded" and backdating could resolve whenever zero retired keys are published. But the JwksDocuments clamp filters exp>nbf, NOT key material — so a retired entry with a valid window but malformed hex (not publishable) still clamps the active key up. In that case the active key IS bounded; the real consequence is missing rotation history, not backdating. Split the message on activeKeyWindowPredatesRetirement (whether a valid window clamped the active key): bounded -> report missing history without a backdating claim; no valid window -> warn the active key has no rotation boundary and a backdated envelope could resolve. The detection (published-count basis) is unchanged. JwksControllerTest +1 (valid-window + malformed-hex: active clamped to the exp, bounded ERROR, asserts no "backdated"). JwksController 100% line-covered; full mvn verify green, jacoco 95% gate met.
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
1 participant
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
Closes a silent gap in the v0.1.25.33 rotation-history publication, surfaced by the codex review of the rotation blog post (runcycles/cycles-docs#724).
The active-key
nbfclamp only bounds the active key when a retired entry contributes a valid window. Ifcycles.evidence.signing.retired-keysis configured (non-blank) but no retired key is publishable, rotation history isn't served and — depending on the config — the active key can publish unbounded, so pre-rotation evidence won't resolve and a backdated envelope could resolve as authentic. Previously this collapsed silently to the never-rotated posture.Change
JwksControllernow signals this, with detection and wording made precise over review:JwksDocumentsactually publishes, not parser output. An entry can pass the lenient parser (integral, in-range bounds) yet be dropped on emission (empty/inverted windowexp_ms <= nbf_ms, malformed hex, duplicate kid, overlap).publishedRetiredCount(...)reusesjwkSet's full emission validation; the ERROR fires when retired-keys is configured + raw-hex signer + zero published retired keys.v0.1.25.34 → v0.1.25.35. Observability-only: no wire/spec change, no change to the published JWK Set.Tests
JwksControllerTest+3 (OutputCaptureExtension, level-checked atERROR):nbf_ms == exp_ms) dropped on emission → unbounded ERROR (the case parser-output detection missed);JwksController100% line-covered; fullmvn verifygreen; jacoco 95% gate met. Codex-reviewed across three rounds (High → Low → Low), all findings applied.