Skip to content

fix: consume replay nonces atomically - #36

Open
realmehmetali wants to merge 4 commits into
worldcoin:mainfrom
realmehmetali:agent/atomic-nonce-consumption
Open

fix: consume replay nonces atomically#36
realmehmetali wants to merge 4 commits into
worldcoin:mainfrom
realmehmetali:agent/atomic-nonce-consumption

Conversation

@realmehmetali

Copy link
Copy Markdown

What changed

  • add consumeNonce(nonce, expiresAt) to AgentKitStorage as an atomic replay-protection primitive
  • prefer atomic nonce consumption after signature verification while retaining the legacy storage methods for compatibility
  • expire in-memory nonce records at the end of the challenge validity window
  • document a PostgreSQL INSERT ... ON CONFLICT DO NOTHING implementation and update the integration skill
  • add storage and concurrent replay tests

Why

The existing optional replay flow performs hasUsedNonce and recordNonce as separate operations. Two concurrent requests can both pass the read before either records the nonce. A single atomic check-and-insert lets persistent storage implementations enforce one-time challenge use without a TOCTOU window.

Nonce consumption happens after signature verification so malformed or invalidly signed requests cannot consume valid challenges. Existing storage implementations continue to work and can migrate incrementally.

Validation

  • bun test ./x402/tests — 12 tests passed
  • bun run --cwd x402 build
  • full monorepo build completed during development

AI assistance

This contribution was prepared with AI-assisted repository inspection, implementation, and test generation. The final diff and validation results were reviewed by the contributor.

@realmehmetali
realmehmetali marked this pull request as ready for review July 21, 2026 17:55

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02ebcda586

ℹ️ 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".

Comment thread x402/src/hooks.ts Outdated
: undefined

const validation = await validateAgentkitMessage(payload, resourceUri, { checkNonce })
const validation = await validateAgentkitMessage(payload, resourceUri)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject known replays before RPC signature verification

When storage exposes hasUsedNonce (including InMemoryAgentKitStorage and legacy persistent implementations), moving that lookup after verifyAgentkitSignature means a consumed but otherwise fresh header can be replayed repeatedly for up to five minutes and every request reaches EVM client.verifyMessage, which may perform configured RPC calls—particularly for ERC-1271 wallets. Retain the non-consuming early lookup as a fast rejection while still calling atomic consumeNonce only after successful signature verification, so invalid requests cannot consume new challenges.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 73905b3. Known nonces are now rejected through the non-consuming hasUsedNonce check before signature/RPC verification, while atomic consumeNonce remains after successful signature validation. I added an ERC-1271 regression proving replay rejection makes zero RPC calls, plus legacy storage coverage. All 14 x402 tests and the full monorepo build pass.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73905b352e

ℹ️ 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".

Comment thread x402/src/hooks.ts Outdated
if (storage?.recordNonce) {
await storage.recordNonce(payload.nonce)
if (storage?.consumeNonce) {
const consumed = await storage.consumeNonce(payload.nonce, getNonceExpiration(payload))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject nonces that expire during signature verification

When concurrent requests pass message validation just before issuedAt + 5 minutes (or expirationTime) and signature verification crosses that boundary, this call gives consumeNonce an already-expired timestamp. InMemoryAgentKitStorage stores the first nonce with that timestamp, then the second call immediately prunes it and also succeeds, so both replayed requests continue. Recheck temporal validity before consuming, or otherwise ensure an in-flight request cannot insert an immediately expirable replay record.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 16238e4. Nonce expiry is now recomputed immediately after signature verification and rejected at or past the boundary before consumption or AgentBook lookup; the built-in storage also rejects already-expired records defensively. The ERC-1271 regression begins 1 ms before expiry, advances time during the RPC, and proves zero consumption/access afterward. Focused tests pass 9/9, the full suite 32/32, and the full monorepo build passes.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16238e4392

ℹ️ 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".

Comment thread x402/DOCS.md
Comment on lines +450 to +453
`INSERT INTO agentkit_nonces (nonce, expires_at)
VALUES ($1, $2)
ON CONFLICT (nonce) DO NOTHING
RETURNING nonce`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject expired timestamps in the PostgreSQL example

The documented implementation inserts whenever the nonce is absent, even if expiresAt has already passed. Near the validity boundary, two requests can both pass the hook's pre-check, the first insert can then be removed by expiry cleanup, and a delayed second insert can also return success, allowing both replays through. Since AgentKitStorage now requires implementations to reject expired records, make the atomic statement condition insertion on expiresAt being later than the database's current time.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 607a36b. The PostgreSQL example now uses INSERT ... SELECT with WHERE $2::timestamptz > clock_timestamp() before ON CONFLICT, so a delayed expired request cannot recreate a nonce after cleanup. I also added a docs regression that guards the database-side predicate. The full 33-test suite and core/x402/CLI builds pass.

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.

1 participant