feat(signing): PostgresReplayStore for distributed verifier deployments#1018
Merged
Conversation
The default InMemoryReplayStore is per-process — multi-instance verifier fleets leak replay protection across the boundary because each box has its own cache. RFC 9421's 5-minute window bounds the gap but it's plenty of time for an in-flight replay. PostgresReplayStore closes the hole using a (keyid, scope, nonce) primary key the verifier checks on every signed request. New exports from @adcp/client/signing/server: - PostgresReplayStore (against the structural PgQueryable interface; same pattern as PostgresTaskStore / PostgresStateStore) - getReplayStoreMigration(tableName?) — idempotent DDL with indexes on expires_at and (keyid, scope, expires_at) - sweepExpiredReplays(pool, options?) — Postgres has no native row-level TTL; callers schedule this helper themselves (cron, app timer, pg_cron) The insert path is one CTE statement that handles replay/cap/insert atomically. ON CONFLICT DO UPDATE WHERE existing-is-expired recycles expired rows in place — a same-nonce insert after the previous registration's TTL elapsed (but before the sweeper ran) correctly returns 'ok' rather than falsely 'replayed'. Concurrent same-nonce inserts (10 parallel) consistently produce exactly one 'ok' and the rest 'replayed', matching InMemoryReplayStore semantics. 13 integration tests against real Postgres (skipped when DATABASE_URL is unset, same convention as postgres-task-store.test.js): - Migration + custom table names + SQL-injection-shaped name rejection - Replay precedence + (keyid, scope) partitioning - TTL boundary (has() respects expiry; insert recycles expired nonces) - Cap precedence: replay wins over rate_abuse wins over ok - Sweeper: full + batched - Concurrency: 10 parallel inserts produce exactly one ok - E2E: signed request → verifier with PostgresReplayStore → second attempt rejected as replay Wire format unchanged. SDK minor: accumulates into Release PR #1014. Closes #1015. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three reviewers (security, code, protocol) cleared PR #1018 with no blockers but flagged consensus follow-ups: - Add `now`/`ttlSeconds` validation (security M2). PG `to_timestamp(NaN)` raises a parse error and `to_timestamp(±Infinity)` silently produces an `infinity` timestamp; a buggy `options.now()` injection point could DoS the verifier with PG errors. New `assertFiniteSeconds` helper rejects non-finite or out-of-range inputs at the three method boundaries with a clear TypeError. - Add `REPLAY_CACHE_MIGRATION` constant alongside `getReplayStoreMigration` to match the `MCP_TASKS_MIGRATION` / `ADCP_STATE_MIGRATION` convention in the sibling Postgres stores. - JSDoc note that `cap` is best-effort under concurrency — two concurrent inserts at `cap-1` can both observe the same MVCC snapshot and briefly overshoot. Matches `InMemoryReplayStore` semantics; the cap is a soft DoS guard, not a hard invariant. Five new tests: - Cap clears once entries pass expiry without the sweeper running (the prune-then-check semantic from `InMemoryReplayStore`, locked in so a future schema change can't quietly regress it — protocol blocker). - Concurrent recycle of an expired same-nonce: 10 parallel inserts past expiry produce exactly 1 `ok` and 9 `replayed` (the security review asked for this; the SQL was already correct, just untested). - `sweepExpiredReplays` with `batchSize` larger than the active expired count returns the actual count without error. - `now` / `ttlSeconds` validation: NaN, ±Infinity, negative all rejected. - End-to-end rate-abuse via the verifier pipeline: cap of 2, third unique signed request rejected with `request_signature_rate_abuse` (covers the conformance-vector-`negative/020` rejection path). `setCapHitForTesting` hook intentionally omitted — the protocol reviewer suggested option (a) "document the limitation" over (b) "add the hook," and white-box conformance grading against Postgres can use `cap: small` + real inserts to reach the cap-hit state. Test totals: 18/18 PostgresReplayStore integration tests + 175 existing signing tests — all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Local two-instance validation ( Distinct Three-reviewer pass (security, code, protocol — same parallel pattern as #1017). No blockers, one consensus blocker from protocol (untested invariant), several worth-fixing items. All addressed in
Test totals: 18/18 PostgresReplayStore integration tests (was 13) + 175 existing signing tests. All green. |
This was referenced Apr 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Postgres-backed
ReplayStoreso multi-instance verifier deployments share replay-protection state. Closes #1015.The default
InMemoryReplayStoreis per-process. On a fleet, an attacker who captures a signed request can replay it against a sibling instance whose cache hasn't seen the nonce — RFC 9421's 5-minute expiry bounds the window but that's plenty of time for an in-flight replay.PostgresReplayStorecloses the hole using a(keyid, scope, nonce)primary key the verifier checks on every signed request.This PR's changeset accumulates into Release PR #1014, which currently bumps to 5.20.0 from #1017. Merging this lands the work on
main; nothing publishes to npm until you merge the Release PR.What's worth a second pair of eyes
1. The single-statement insert handles replay → cap → insert atomically
src/lib/signing/postgres-replay-store.ts:148— one CTE that:'replayed'if an unexpired row already exists for(keyid, scope, nonce)'rate_abuse'if the active count for(keyid, scope)is at the cap'ok''replayed'on the concurrent-same-nonce loser branchResult precedence matches
InMemoryReplayStore: replay > rate_abuse > ok.2. Expired-row recycling — the bug I caught with the integration test
The first version of the SQL used
ON CONFLICT DO NOTHINGand inferred'replayed'from an emptyRETURNING. That broke the "previously-expired same-nonce" case: the row was still on disk (sweeper hadn't run),ON CONFLICTfired,RETURNINGwas empty, adapter returned'replayed'— but the InMemory store returns'ok'for that case (it prunes-then-checks). Switched toON CONFLICT DO UPDATE WHERE existing-is-expired, which refreshes the row in place when its previous TTL has elapsed. Now both stores agree.3. Sweeper is an exported helper, not a background timer
sweepExpiredReplays(pool, options?)is exported and callers schedule it themselves (cron, app timer,pg_cron, etc.). Deliberately avoiding a constructor-injected timer — those surprise people in tests and serverless. Doc shows three ways to schedule.4. Reuses
PgQueryablestructural typeImports from
src/lib/server/postgres-task-store.tsso the SDK stays free of a hardpgdependency. Matches thePostgresTaskStore/PostgresStateStorepattern already in the codebase.Test plan
npm run typecheck— cleannpm run format:check— cleannpm run build:lib— cleantest/lib/postgres-replay-store.test.js(run withDATABASE_URL=postgres://...):'ok', second insert same nonce'replayed'(keyid, scope)partitioning — same nonce on different scopes is not a replayhas()and byinsert()(expired same-nonce recycles correctly)isCapHitreflects active count vs cap; expires-out properly'ok'and 9'replayed'PostgresReplayStore→ first request verifies, second is rejected asrequest_signature_replayedDeferred follow-ups
@upstash/redisorioredis-shaped) as a sibling example — pattern is identical, no SDK interface change.pg_cronschema helper for users who'd rather have Postgres run the sweep — small follow-up doc, no code.🤖 Generated with Claude Code