Skip to content

🤖 AI: Fix Caching of Pub Keys - #27

Merged
ejinotti-mdsol merged 4 commits into
masterfrom
fix/expire-cached-public-keys
Sep 10, 2026
Merged

ejinotti-mdsol merged 4 commits into
masterfrom
fix/expire-cached-public-keys

Conversation

@ejinotti-mdsol

@ejinotti-mdsol ejinotti-mdsol commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Problem

Cached public keys never expired. After an application rotated its key,
long-running processes continued rejecting its requests until restarted.
Validation failures also omitted the application UUID claimed by the request,
making rejections harder to diagnose.

Changes

Replace the process-lifetime key map with a bounded, expiring LRU cache:

  • Derive expiry from response Cache-Control: max-age, accounting for Age
    and time spent fetching the response.
  • Do not cache responses marked no-cache or no-store.
  • Use a 60-second fallback when no usable cache lifetime is supplied.
  • Cache only successfully parsed public keys from HTTP 200 responses;
    unsuccessful lookups, including 404 responses, are not cached.
  • Consider all Cache-Control field lines, use the shortest repeated
    max-age, and handle unreadable headers and overflowing values safely.
  • Limit the cache to 10,000 entries by default, configurable through
    ConfigFileSection::pubkey_cache_capacity. Zero uses the default.

Add AttemptedMAuthIdentity for rejection diagnostics. It records the UUID
claimed in the signature header and is included in rejection logs. The Required
layer exposes it through the 401 response extensions; the Optional layer places
it in request extensions alongside the validation error.

This is not an authenticated identity. Trust decisions must continue to use
ValidatedRequestDetails. When both signature headers are present, diagnostic
identity extraction prefers V2.

Implementation notes

The cache remains process-wide to preserve entries across the existing service
cloning behavior. The validation services' custom Clone implementations
reconstruct MAuthInfo from configuration; MAuthInfo itself derives Clone.
The first configuration loaded determines cache capacity, matching the existing
HTTP client's initialization behavior.

Response headers are inspected before consuming the JSON body. Concurrent cache
misses are not coalesced, so simultaneous requests for an expired key can each
fetch it. A rotated key becomes available after cache expiry; this change does
not add an immediate refresh on signature failure.

Feature gating and dead-code annotations also allow Clippy to pass with and
without incoming-validation support.

Compatibility

Bump the crate version to 0.8.0. Adding pubkey_cache_capacity breaks
existing exhaustive ConfigFileSection struct literals; add
pubkey_cache_capacity: None to retain the default capacity.

Validation

  • cargo fmt --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • Clippy with no default features, with axum-service alone, and with
    tracing-otel-31 alone
  • cargo test --all-features: 39 tests and 4 doctests pass

Tests cover cache-header parsing, expiry boundaries, overflow handling, LRU
eviction, and diagnostic identity extraction. A local HTTP server test verifies
that a cached key is reused, a rotated signature initially fails, and expiry
triggers a refetch that restores successful validation. Expiry is controlled
without sleeping, and temporary signing keys are generated in memory.

Middleware tests verify that Required validation returns a 401 with the claimed
identity without invoking the handler, and Optional validation passes the
claimed identity and error to the handler without marking the request as
authenticated. Both V1 and V2 signature headers are covered.

@ejinotti-mdsol ejinotti-mdsol changed the title claude: fix pub key caching to actually expire 🤖 AI: Fix Caching of Pub Keys Sep 9, 2026
@ejinotti-mdsol
ejinotti-mdsol requested a balanced review from Copilot September 9, 2026 15:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Repeated cache directives can be ignored, and oversized cache-age values can panic the process.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds bounded, expiration-aware public-key caching and improves diagnostics for rejected MAuth requests.

Changes:

  • Honors MAuth cache headers with TTL and LRU eviction.
  • Records claimed application UUIDs on validation failures.
  • Adds configurable cache capacity, tests, documentation, and a version bump.
File summaries
File Description
src/validate_incoming.rs Implements expiring LRU caching and attempted identity extraction.
src/protocol_test_suite.rs Updates test configuration construction.
src/lib.rs Defines and initializes the process-wide cache.
src/config.rs Adds cache-capacity configuration.
src/axum_service.rs Logs and exposes attempted identities on rejection.
README.md Documents identity diagnostics and caching behavior.
Cargo.toml Adds the LRU dependency and bumps the version.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/validate_incoming.rs Outdated
Comment thread src/validate_incoming.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The cache semantics, diagnostic propagation, feature gating, and edge cases are comprehensively implemented and tested.

Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@ykitamura-mdsol ykitamura-mdsol left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copilot seems to be happy 👍

@ejinotti-mdsol

Copy link
Copy Markdown
Contributor Author

thanks @ykitamura-mdsol! i actually reviewed this myself as well 😉

@ejinotti-mdsol
ejinotti-mdsol merged commit 6fff437 into master Sep 10, 2026
6 checks passed
@ejinotti-mdsol
ejinotti-mdsol deleted the fix/expire-cached-public-keys branch September 10, 2026 10:55
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.

3 participants