Implement RFC 9421 §5 Accept-Signature challenge-response negotiation#626
Implement RFC 9421 §5 Accept-Signature challenge-response negotiation#6262chanhaeng wants to merge 19 commits intofedify-dev:mainfrom
Accept-Signature challenge-response negotiation#626Conversation
issues-auto-labeler
bot
added
activitypub/interop
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades Fedify's HTTP signature handling by integrating RFC 9421 §5 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements RFC 9421 §5 Accept-Signature challenge-response negotiation for both inbound and outbound federation. The changes are comprehensive, touching on the federation handler, HTTP signature utilities, and adding new modules for Accept-Signature header processing. The implementation includes robust handling of 401 challenges, nonce-based replay protection, and maintains backward compatibility with legacy spec-swapping. The addition of extensive unit and integration tests ensures the new functionality is well-covered. My main feedback is a suggestion to improve the robustness of the nonce generation logic, specifically regarding base64 encoding.
Codecov Report❌ Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
|
@claude review |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR adds RFC 9421 §5 Accept-Signature challenge/response negotiation support to Fedify, both for outbound delivery retries (client side) and for inbox 401 challenge emission with optional nonce-based replay protection (server side).
Changes:
- Added
Accept-Signatureparsing/formatting/validation + challenge fulfillment utilities and exported them from@fedify/fedify/sig. - Extended RFC 9421 signing to support custom label/components and optional
nonce/tag, plus implemented outbounddoubleKnock()challenge-driven retry. - Added inbound
InboxChallengePolicyto emitAccept-Signaturechallenges and (optionally) store/verify one-time nonces in KV.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fedify/src/sig/mod.ts | Re-exports new Accept-Signature helpers and RFC 9421 signing options type. |
| packages/fedify/src/sig/http.ts | Adds RFC 9421 signing options (label, components, nonce, tag) and implements Accept-Signature-driven retry in doubleKnock(). |
| packages/fedify/src/sig/http.test.ts | Adds tests for RFC 9421 signing options and doubleKnock() Accept-Signature retry behavior. |
| packages/fedify/src/sig/accept.ts | New module implementing Accept-Signature parsing/formatting/validation and challenge fulfillment. |
| packages/fedify/src/sig/accept.test.ts | New tests covering Accept-Signature utilities. |
| packages/fedify/src/federation/middleware.ts | Wires InboxChallengePolicy and KV prefix into federation plumbing. |
| packages/fedify/src/federation/handler.ts | Emits Accept-Signature challenges on inbox 401s and adds nonce issuance/verification logic. |
| packages/fedify/src/federation/handler.test.ts | Adds tests for inbox challenge emission and nonce lifecycle (issue, consume, replay). |
| packages/fedify/src/federation/federation.ts | Introduces the InboxChallengePolicy public API in federation options. |
| examples/astro/deno.json | Adjusts TS module resolution for the Astro example. |
| docs/manual/send.md | Documents outbound Accept-Signature negotiation behavior. |
| docs/manual/inbox.md | Documents inbound inbox challenge emission and nonce replay protection. |
| CHANGES.md | Changelog entry for the new Accept-Signature negotiation features. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bc621b0bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements RFC 9421 §5 Accept-Signature challenge-response negotiation for both inbound and outbound federation. It introduces a new inboxChallengePolicy option to enable emitting challenges on 401 responses. On the outbound side, it handles these challenges by retrying requests with the required signature parameters. The implementation is robust, with comprehensive tests covering various scenarios, including security considerations like nonce replay and bypass attacks. The documentation has also been updated accordingly. My review includes one medium-severity suggestion to remove potentially unsafe fallback logic in the nonce verification function to improve security and clarity.
There was a problem hiding this comment.
Pull request overview
Adds RFC 9421 §5 Accept-Signature challenge/response negotiation support across both outbound delivery (doubleKnock) and inbound inbox verification, including optional one-time nonce replay protection backed by KV storage.
Changes:
- Added
Accept-Signatureparsing/formatting/validation helpers and exported them from thesigmodule surface. - Extended RFC 9421 signing to support configurable signature label, covered components, and metadata parameters (
nonce,tag), and taughtdoubleKnock()to honorAccept-Signaturechallenges before legacy spec-swap. - Added
InboxChallengePolicyto optionally emitAccept-Signaturechallenges on inbox401responses, with optional nonce issuance + verification, and documented the feature.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fedify/src/sig/mod.ts | Re-exports new accept.ts utilities and Rfc9421SignRequestOptions for public API access. |
| packages/fedify/src/sig/http.ts | Adds RFC 9421 signing options (label/components/nonce/tag) and implements outbound Accept-Signature-driven retry in doubleKnock(). |
| packages/fedify/src/sig/http.test.ts | Adds tests for RFC 9421 signing options and doubleKnock() challenge handling/fallback behavior. |
| packages/fedify/src/sig/accept.ts | New module for Accept-Signature structured field parsing, formatting, request validation, and fulfillment logic. |
| packages/fedify/src/sig/accept.test.ts | Unit tests for parsing/formatting/validation/fulfillment of Accept-Signature. |
| packages/fedify/src/federation/middleware.ts | Wires InboxChallengePolicy + KV prefix into federation plumbing. |
| packages/fedify/src/federation/handler.ts | Emits inbox Accept-Signature challenges on signature-verification 401s and adds nonce issuance/verification logic. |
| packages/fedify/src/federation/handler.test.ts | Adds inbox challenge/nonce issuance, consumption, replay-prevention, and bypass-regression tests. |
| packages/fedify/src/federation/federation.ts | Introduces InboxChallengePolicy in public federation options/types. |
| examples/astro/deno.json | Adjusts TypeScript module resolution to nodenext for the Astro example. |
| docs/manual/send.md | Documents outbound Accept-Signature negotiation behavior in doubleKnock(). |
| docs/manual/inbox.md | Documents inbound inbox Accept-Signature challenge emission + nonce option. |
| CHANGES.md | Changelog entry for new inbound/outbound Accept-Signature negotiation support. |
When deno check processes the full workspace, examples/astro's tsconfig.json (extending astro/tsconfigs/strict) sets moduleResolution to "Bundler". Since the Astro example imports @fedify/fedify, this setting leaks into the compilation context of accept.ts, causing @fxts/core's internal ReturnPipeType to incorrectly resolve pipe's return type. Override moduleResolution to "nodenext" in examples/astro/deno.json so the Astro tsconfig no longer affects how @fxts/core's types resolve.
- Remove duplicated components in `signRequestRfc9421` - Filter wider status in `doubleKnock`
- Remove unsafe fallback in `verifySignatureNonce` that scanned all Signature-Input entries when verifiedLabel is absent; non-RFC 9421 signatures do not support nonces so the check is skipped entirely - Defer nonce consumption until after `doesActorOwnKey` to avoid burning nonces on requests that will be rejected due to actor/key mismatch - Enforce a minimum component set (method, target-uri, authority) in `buildAcceptSignatureHeader` regardless of caller-supplied components
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements RFC 9421 §5 Accept-Signature challenge-response negotiation for both inbound and outbound federation, which is a significant enhancement for interoperability. The changes are well-structured, with new functionality encapsulated in dedicated modules and helper functions. The implementation correctly handles security considerations, such as deferring nonce verification to prevent replay attacks and misuse. The addition of comprehensive tests, including those for potential security vulnerabilities, is excellent and ensures the robustness of the new features. The documentation has also been updated clearly. Overall, this is a high-quality contribution.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4660cbba35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds RFC 9421 §5 Accept-Signature challenge/response negotiation to improve HTTP Signatures interoperability: outbound deliveries can retry using server-provided signature requirements, and inbound inboxes can optionally emit Accept-Signature challenges (including one-time nonces) on signature-verification 401s.
Changes:
- Added
Accept-Signatureparsing/formatting/fulfillment utilities and re-exported them from the sig module. - Extended RFC 9421 request signing to support custom signature label, covered components, and
nonce/tagparameters; outbounddoubleKnock()now performs a challenge-driven retry before legacy spec-swap. - Added inbound
InboxChallengePolicyto emitAccept-Signaturechallenges and (optionally) verify/consume replay-protection nonces stored in KV; updated docs and changelog.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fedify/src/sig/mod.ts | Re-exports new Accept-Signature utilities and RFC 9421 signing options as public API. |
| packages/fedify/src/sig/http.ts | Implements RFC 9421 signing options (label/components/nonce/tag), exposes verified signature label, and adds outbound Accept-Signature negotiation in doubleKnock(). |
| packages/fedify/src/sig/http.test.ts | Adds tests for RFC 9421 signing options and outbound Accept-Signature negotiation behavior. |
| packages/fedify/src/sig/accept.ts | New module to parse/format/validate Accept-Signature and fulfill challenges safely. |
| packages/fedify/src/sig/accept.test.ts | Unit tests for Accept-Signature parsing/formatting/validation/fulfillment. |
| packages/fedify/src/federation/middleware.ts | Threads new KV prefix and inbox challenge policy option through federation implementation. |
| packages/fedify/src/federation/handler.ts | Emits Accept-Signature on selected 401s and adds nonce issuance/verification using KV storage. |
| packages/fedify/src/federation/handler.test.ts | Adds tests for challenge emission and nonce issuance/consumption/replay behavior. |
| packages/fedify/src/federation/federation.ts | Introduces InboxChallengePolicy in federation configuration API. |
| examples/astro/deno.json | Updates TS module resolution setting for the Astro example. |
| docs/manual/send.md | Documents outbound Accept-Signature negotiation behavior. |
| docs/manual/inbox.md | Documents enabling inbound Accept-Signature challenges and optional nonce enforcement. |
| CHANGES.md | Changelog entry for Accept-Signature negotiation (inbound and outbound). |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
Implement
Accept-Signaturechallenge-response negotiation (RFC 9421 §5)for both inbound and outbound sides of Fedify's federation layer.
Related issue
Accept-Signatureparsing and challenge-driven re-signing indoubleKnock#583Accept-Signaturechallenges on inbox authentication failures #584Changes
Accept-Signaturenegotiation for both outbound and inbound paths. On the outbound side,doubleKnock()now parsesAccept-Signaturechallenges from401responses and retries with a compatible RFC 9421 signature before falling back to legacy spec-swap.InboxChallengePolicyoption inFederationOptionsenables emittingAccept-Signatureheaders on inbox401responses, with optional one-time nonce support for replay protection.Benefits
require specific RFC 9421 parameters
requirements and replay protection
transparent on outbound