Feat/issue 1122

[rewrite0w0]

Background

RequestContext.getSignedKey() currently calls verifyRequest() without a keyCache, so signing keys are fetched again across requests using the same key ID.

This change wires the existing KvKeyCache into RequestContext.getSignedKey() so that signing keys can be reused across requests while preserving the existing key rotation behavior.

Changes

• Update RequestContext.getSignedKey() in packages/fedify/src/federation/middleware.ts:

  • Create a KvKeyCache using the federation's public key KV prefix and TTL.
  • Pass the cache to verifyRequest().

• Add regression tests in packages/fedify/src/federation/middleware.test.ts:

  • Verify that a signing key is reused from the cache across requests.
  • Verify that unavailable keys are negatively cached.
  • Verify that a cached key is refetched when it no longer verifies, and that the refreshed key is subsequently cached.

Testing

• deno test -A packages/fedify/src/federation/middleware.test.ts
• mise check && mise fmt && mise test

Question

The issue mentions the possibility of adding an opt-out to GetSignedKeyOptions for callers that need a fresh key lookup.

Would an opt-out be useful here, or is using the key cache unconditionally the preferred behavior?

AI assistance

This PR description and test implementations were drafted with AI assistance (ChatGPT) and finalized after human review and testing.

[netlify]

✅ Deploy Preview for fedify-json-schema canceled.

Name	Link
🔨 Latest commit	89d1f9a
🔍 Latest deploy log	https://app.netlify.com/projects/fedify-json-schema/deploys/6ac0957cf8c65100089d1775

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

CONTRIBUTING.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 4a05cb27-f849-4ac1-a3be-aedf38c61274

📥 Commits

Reviewing files that changed from the base of the PR and between 49d937e and 89d1f9a.

📒 Files selected for processing (2)

• packages/fedify/src/federation/middleware.test.ts
• packages/fedify/src/federation/middleware.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

getSignedKey() now creates a KvKeyCache for public-key lookups and passes it to verifyRequest. Tests cover reuse of verified and unavailable keys, and refetching and reuse after key rotation.

Changes

Public-key cache

Layer / File(s)	Summary
Configure and verify cached public keys packages/fedify/src/federation/middleware.ts, packages/fedify/src/federation/middleware.test.ts	getSignedKey() configures a KvKeyCache with the selected loaders, tracer provider, and public-key TTL, then passes it to verifyRequest. Tests check reuse of verified and unavailable keys, plus refetch and reuse after key rotation.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 89d1f

Signing keys are now reused across requests, and a key that no longer verifies is refetched. Tests cover reuse, unavailable keys, and rotation. No merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 89d1f

Key reuse reduces remote lookups, but a previously trusted key can continue verifying requests after it is withdrawn. Refreshing after a signature mismatch supports rotation but does not detect requests signed with the old private key. Exposure depends on cache lifetime and the application's subsequent ownership checks.

Retained concerns

• Medium · security · inferred: Cross-request caching extends acceptance of formerly trusted keys. A request signed with an old private key can still verify against its cached public key after authoritative removal or replacement, without triggering rotation refresh. This can preserve access where authorization relies on getSignedKey without a subsequent ownership check. Newly written ordinary entries default to 30 days; legacy entries without expiry may last longer. Separate ownership verification mitigates key removal for callers that use it.

Security review details

Security Blast Radius

• inferred — The delayed-revocation exposure is tied to a previously cached key and possession of its corresponding private key. It can affect resources whose authorization accepts that sender through getSignedKey, across contexts using the same public-key cache namespace. This does not establish arbitrary-actor impersonation, cross-tenant access, or compromise of every federation endpoint.

Security Findings and Attack Paths

• inferred — A formerly trusted key is cached; its owner later withdraws or replaces it; a holder of the old private key submits a valid new signed request naming that key URL. The cached key still verifies, so mismatch-triggered refresh does not occur. Access can continue where the consumer treats that result as sufficient authorization. The rotation regression exercises the replacement private key, not this old-key path.

Trust Boundaries and Controls

• observed — Caching does not remove cryptographic signature verification or fresh-fetch ownership validation. For ordinary keys, getSignedKeyOwner separately resolves the claimed owner and checks its linkage through the selected loader, mitigating removal from that owner's key list. Cache generation 2 also separates entries from earlier versions that did not validate ownership.

Hardening Proposals

• proposed — Define an explicit authentication-freshness policy, including a cache-bypass option for revocation-sensitive callers or mandatory authoritative revalidation. Bound the accepted stale-key interval and account for legacy entries without expiry. Validate the policy against requests signed with a withdrawn key, not only requests signed with its replacement.
• proposed — If applications use different document loaders as distinct trust policies, isolate their cache namespaces or revalidate cached keys under the active policy. The shared key-URL namespace is established, but no concrete deployment using loaders as separate security domains was evidenced; this is a conditional hardening proposal, not an observed bypass.

🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title identifies this as a feature but does not say that it adds key caching to RequestContext.getSignedKey().	Use a concise title that names the change, such as “Cache signing keys in RequestContext.getSignedKey().”

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description explains the key-cache change, its rotation behavior, the regression tests, and the reported test commands.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines	Coverage Δ
packages/fedify/src/federation/middleware.ts	87.66% <100.00%> (+0.02%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[2chanhaeng]

Please use the title to summarize the intent of the change, and note which issue the PR addresses in the body. If the title only includes the issue number, GitHub cannot link the issue to the PR.
I think this PR needs a changelog entry because the number of remote lookups and the timing of key updates will change. These changes are visible to users.
I also think users should have an option to disable the cache. However, it doesn’t need to be implemented right away, so you can create a separate issue.
The PR description says AI was used, but the only commit that changed the code, 3ff6fa0, doesn’t have Assisted-by trailers in its message. Please add them.