Sign requests with RFC 9421 HTTP Message Signatures - #39
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52265a962b
ℹ️ 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
Replaces body-only EIP-191 authentication with request-bound RFC 9421 signatures across Core, x402, and CLI.
Changes:
- Adds signature creation, parsing, expiry, digest, and signer verification.
- Updates x402 and CLI request-signing flows.
- Refreshes tests, documentation, dependencies, and release metadata.
Unresolved issues include bodyless JSON request rejection, case-sensitive discount matching, and an incompatible global AJV override.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
core/src/signature.ts |
Implements signature profile. |
core/src/verify.ts |
Verifies signed requests. |
core/src/index.ts |
Exports new APIs. |
core/tests/signature.test.ts |
Tests signature handling. |
core/tests/verify.test.ts |
Tests request verification. |
core/tests/exports.test.ts |
Tests public exports. |
x402/src/client.ts |
Signs x402 retries. |
x402/src/hooks.ts |
Verifies incoming requests. |
x402/src/protocol.ts |
Defines signature headers. |
x402/src/index.ts |
Exports protocol constants. |
x402/tests/client.test.ts |
Tests client signing. |
x402/tests/client-e2e.test.ts |
Adds end-to-end verification. |
x402/tests/hooks.test.ts |
Tests server hooks. |
x402/package.json |
Adds test preparation. |
x402/DOCS.md |
Documents the new flow. |
cli/src/prove.ts |
Creates proof headers. |
cli/src/index.ts |
Updates prove command. |
cli/test/prove.test.ts |
Tests CLI proofs. |
cli/README.md |
Updates CLI usage. |
cli/REGISTRATION.md |
Updates signing guidance. |
cli/package.json |
Adds Core dependency. |
skills/integrate-agentkit/SKILL.md |
Updates Core integration guidance. |
skills/integrate-agentkit-x402/SKILL.md |
Updates x402 integration guidance. |
skills/agentkit-x402/SKILL.md |
Updates agent signing workflow. |
package.json |
Adds AJV override. |
bun.lock |
Updates dependency resolution. |
.changeset/rfc9421-signatures.md |
Records release changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Override ajv to v8 (root-hoisted v6 from eslint broke ajv-draft-04 via incur -> @readme/openapi-parser) and build core before x402/cli tests, since the workspace symlink resolves through core's dist/ exports map. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The X-AgentKit bare EIP-191 body signature was a permanent, universal bearer credential (no audience, method, path, expiry, or nonce binding) and a cross-protocol signing oracle. Requests are now signed under a closed RFC 9421 profile covering @method, @authority, @path, @query, and content-digest (RFC 9530), with created/expires/nonce/keyid/tag params, EIP-191 over the signature base, and recovered-signer == keyid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… protection The hooks now verify against the request's real method and URL through a single core verifyRequest call (dropping the duplicate address recovery), and enforce single-use nonces via restored hasUsedNonce/recordNonce storage methods. The client signs the RFC 9421 signature base and retries with Signature-Input, Signature, and Content-Digest instead of X-AgentKit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
prove now takes <method> <url> [body] and returns the Content-Digest, Signature-Input, and Signature header values instead of a bare EIP-191 body signature, using the shared profile implementation from @worldcoin/agentkit-core. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Single-use signatures via a nonce move to a follow-up PR. Replay of a byte-identical request is bounded by the five-minute created/expires window until then; docs state this explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rfaced addresses verifyRequest now returned the lowercase wire keyid while extractPayer reads the payment payload's from verbatim (usually EIP-55 checksummed), so the pendingDiscounts lookup never matched and discount recovery silently stopped firing. Discount keys now lowercase both sides, and core surfaces EIP-55 checksummed addresses (results, error addresses, lookups), restoring the pre-RFC-9421 observable behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
deriveComponents interpolated the caller's method string raw, so a direct createSignatureHeaders caller could inject extra lines into the base being signed. Verification was never spoofable (the verifier rebuilds a fixed six-line base from a real Request) and existing callers were guarded upstream, but core is the public signing API, so the letters-only check now lives at the shared choke point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Body normalization ran before the method check, so a bodyless GET carrying Content-Type: application/json (a common HTTP-client default) passed undefined into normalizeAgentkitJsonBody and was rejected before verification. Determine the method first, skip body retrieval for GET/HEAD, and treat a missing adapter body as the signed empty body — matching what clients sign. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Five seconds only covered well-synced clocks; laptops after sleep, VMs, and containers without NTP are routinely seconds fast and would fail every request with SIGNATURE_NOT_YET_VALID. Slow clocks were already tolerated for up to 300 seconds via the age check, so this only evens out the asymmetry and extends the worst-case replay window marginally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d8b5eb2 to
78bce2b
Compare
paolodamico
left a comment
There was a problem hiding this comment.
wouldn't this break clients still using old agentkit versions (pre-0.2)?
| export const requestBodyInputSchema = z.string().describe('Exact UTF-8 request body to sign') | ||
| export const methodInputSchema = z | ||
| .string() | ||
| .regex(/^[A-Za-z]+$/, 'Invalid HTTP method') |
There was a problem hiding this comment.
should this have case normalization?
There was a problem hiding this comment.
It's already normalised one layer down
There was a problem hiding this comment.
Will move it up to here to make clearer
| body?: string | Uint8Array | ||
| /** The agent's address; becomes the lowercase `keyid`. */ | ||
| address: string | ||
| /** EIP-191 signer over the UTF-8 signature base. */ |
There was a problem hiding this comment.
high why is an EIP-191 signer being used here? doesn't RFC 9421 already specify the signature scheme completely?
There was a problem hiding this comment.
No, the signature scheme is plug and play. You can use any. It only specifies the signature base, header format etc.
I kept EIP-191 signatures because secp256k1 is required with the address book
| import { AGENTKIT, normalizeAgentkitBody, normalizeAgentkitRequestBody } from './protocol' | ||
|
|
||
| export type AgentkitSigner = { | ||
| /** The agent's address; becomes the signature keyid. */ |
There was a problem hiding this comment.
should we start using public keys instead of addresses?
There was a problem hiding this comment.
why? The address book is already keyed by address and the public key is implicit in the address
| if (parsedBody !== undefined) { | ||
| body = | ||
| contentType === 'application/json' || contentType?.endsWith('+json') | ||
| ? normalizeAgentkitJsonBody(parsedBody) |
There was a problem hiding this comment.
😅 this looks like a footgun, "normalizing" bodies generally creates a lot of hard-to-debug discrepancies, it's much better to compute the digest from raw bytes (and more performant)
There was a problem hiding this comment.
the rawBytes are not available here, the HTTPAdapter from Coinbases's x402 package only gives us the already parsed body (getBody())
m1guelpf
left a comment
There was a problem hiding this comment.
LGTM except for paolo's comments
Resolves conflicts between the RFC 9421 signature work and the lookupId rename + lookup cache from new-cli: - verify.ts keeps the RFC 9421 verification flow and adopts the lookupNullifierHash -> lookupId rename plus the 60s lookup cache - VerifiedAgentRequest.nullifierHash renamed to lookupId - RFC 9421 headers (Signature-Input, Signature, Content-Digest) supersede the AgentKit header rename from new-cli - docs and skills keep the RFC 9421 flow with lookup ID terminology Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stacked on #38. Replaces the bare EIP-191 body signature with RFC 9421 HTTP Message Signatures + RFC 9530 Content-Digest.
Why
The
X-AgentKitheader contained a signature over the body of a request only. This had two issues::provewould sign any stringpersonal_sign. This meant that a signature from another protocol - e.g. SIWE - could be used to impersonate an agent signatureHow it works now
The client signs a canonical signature base covering the request's method, host, path, query, and a digest of the body, plus a validity window. The server rebuilds that base from the request it actually received and recovers the signer — so changing anything covered breaks verification. Three headers carry the scheme:
Content-DigestSignature-Inputcreated,expires,keyid(agent address),tag="agentkit". Itself part of the signed base, so it can't be tampered with.SignatureThis is an implementation of RFC 9421, which has been adopted by Cloudflare and others.
Next steps (follow-up PRs)
created/expireswindow, and the docs state this explicitly. Follow up PR to add nonce support and storage.