fix(x402): harden replay and settlement binding#236
Conversation
|
@greptile-apps please review |
Greptile SummaryThis PR hardens the x402 replay and settlement binding across TypeScript, Go, and PHP: it makes replay reservations atomic and fail-closed before broadcast, adds route/memo binding to the exact-scheme challenge, replaces process-local
Confidence Score: 5/5Safe to merge. The replay-and-settlement hardening is implemented consistently across all targeted SDKs with correct fail-closed semantics; the observations flagged are maintenance notes, not runtime defects. The core invariants are correctly implemented and thoroughly tested across Go (970 tests), TypeScript, PHP, and the cross-SDK harness. The two flagged items are a hard-coded facilitator error-string whitelist that could silently block retries on unlisted pre-broadcast reasons, and a potential panic in cosign() if a malformed decoded transaction has NumRequiredSignatures larger than AccountKeys. Neither is a present defect in the changed paths. typescript/packages/pay-kit/src/adapters/x402.ts (RELEASE_SAFE_SETTLE_REASONS coupling) and go/paykit/adapters/x402/exact.go (Signers() call order in cosign). Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant Middleware
participant Adapter as x402 Adapter
participant Store as Replay Store
participant Facilitator
Client->>Middleware: POST /pay (X-Payment credential)
Middleware->>Adapter: VerifyAndSettle(req)
Adapter->>Adapter: assertPaymentHeaderWithinCap()
Adapter->>Facilitator: verify(payload, requirements)
Facilitator-->>Adapter: isValid true
Adapter->>Adapter: payloadKey(header, payload, requirements)
Adapter->>Store: reserve(consumed-key, true, 300s)
alt key already exists
Store-->>Adapter: false
Adapter-->>Client: 402 x402_payment_replayed
else key reserved
Store-->>Adapter: true
Adapter->>Facilitator: settle(payload, requirements)
alt settle throws
note over Adapter,Store: Reservation KEPT fail-closed
Adapter-->>Client: 402 settlement_failed
else failure in RELEASE_SAFE_SETTLE_REASONS
Adapter->>Store: delete(consumed-key)
Adapter-->>Client: 402 reason
else success
Adapter-->>Client: 200 OK with x-payment-response
end
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant Middleware
participant Adapter as x402 Adapter
participant Store as Replay Store
participant Facilitator
Client->>Middleware: POST /pay (X-Payment credential)
Middleware->>Adapter: VerifyAndSettle(req)
Adapter->>Adapter: assertPaymentHeaderWithinCap()
Adapter->>Facilitator: verify(payload, requirements)
Facilitator-->>Adapter: isValid true
Adapter->>Adapter: payloadKey(header, payload, requirements)
Adapter->>Store: reserve(consumed-key, true, 300s)
alt key already exists
Store-->>Adapter: false
Adapter-->>Client: 402 x402_payment_replayed
else key reserved
Store-->>Adapter: true
Adapter->>Facilitator: settle(payload, requirements)
alt settle throws
note over Adapter,Store: Reservation KEPT fail-closed
Adapter-->>Client: 402 settlement_failed
else failure in RELEASE_SAFE_SETTLE_REASONS
Adapter->>Store: delete(consumed-key)
Adapter-->>Client: 402 reason
else success
Adapter-->>Client: 200 OK with x-payment-response
end
end
Reviews (8): Last reviewed commit: "Merge remote-tracking branch 'origin/fix..." | Re-trigger Greptile |
|
@greptile-apps please review |
|
@greptile-apps please re-review the updated replay hardening. |
…mbiguous broadcast Bind the exact adapter's replay reservation to a canonical payload identity (sha256 over the decoded message plus the pinned requirements) so a re-encoded transaction cannot dodge the reserve-before-settle claim. Keep the reservation when settlement throws, since a facilitator can raise after broadcast and its side-effect boundary is ambiguous; only release on definitively pre-broadcast verification failures enumerated in RELEASE_SAFE_SETTLE_REASONS. Add the reserve-capable replay-store module the adapters depend on, a shared short-TTL single-flight challenge blockhash cache, an oversized-header cap, and the upto same-channel route binding with documented TTL-boundary handling. The store production/durability policy and the demo-signer-on-mainnet boundary revalidation live in the config layer (shared TS leaf), so the adapters here require only a reserve-capable replayStore and fail closed when one is absent.
Create a single run-scoped temp directory and export it to every spawned adapter as PAY_KIT_HARNESS_REPLAY_STORE_DIR so per-language x402 servers share one durable replay root instead of splitting workers onto process-local stores. Remove the directory on exit and on SIGINT/SIGHUP/SIGTERM, and cover the signal-cleanup path with a test.
Wire the x402 exact adapter and its replay store through the Laravel service provider and the Symfony DI extension, and fail closed off localnet when an x402-accepting app has no replay adapter injected. Skip x402 DI entirely for mpp-only apps so they boot without an x402 replay store. Harden the exact verifier's u64 handling and canonical replay identity, and boot the PHP harness server against the run-scoped shared replay directory.
Reserve the canonical replay identity before settlement on the exact adapter so a concurrent duplicate cannot slip through the settle window, and keep the reservation across ambiguous broadcast outcomes. Stop the x402 upto path from reusing the exact reserve-before-settle setup, and thread the replay-store capability through the client and middleware.
…boot The rebuild dropped the localnet replay-store default, so an x402 config with no explicit replayStore threw ConfigurationError at adapter construction even on localnet, breaking playground-api (createPayKit accept:['x402','mpp'] network:'localnet' with no store). Resolve the reserving replay store once in x402-shared: an injected store wins, localnet (or the explicit off-localnet opt-in) gets a process-local in-memory store mirroring the MPP session adapter, and off localnet without the opt-in still fails closed. Both exact and upto route through it.
…resAt The upto replay reservation used max(300, channel.expiresAt - now) as its TTL. expiresAt is payer-signed and unbounded above, so a far-future value minted effectively permanent route/consumed keys. Clamp the TTL to a 24h hard ceiling above the completion-window floor so a channel authorization cannot pin replay keys indefinitely.
7f65011 to
3cd4761
Compare
|
@greptileai review |
Reconcile #238 (MPP subscription) onto #237's nominal replay-store policy, which the maintainer chose as the single config-layer model. #238's flag-based policy is removed; all its unique behaviour is preserved on top of #237's model. config.ts: - Delete #238's validateReplayStore + requiresSettlementReplayStore. - Keep #237's assertReplayStorePolicy as the single replay policy (isAtomicReplayStore / isProductionReplayStore / declareProductionReplayStore / isAuthorizedUnsafeMemoryReplayStore). - x402 config coverage stays adapter-only: the x402 store cannot satisfy the putIfAbsent production marker and #237's own tests require x402-only configs to accept a legacy Store.Store, so the mpp-only gate is left in place and x402 replay is fenced by the x402 adapter (#236). Never silently accepts a non-durable x402 mainnet store. - Preserve #238's validators, inMemoryStoresAllowed, freezePayKitConfig / freezePayKitSigner snapshot, 32-byte challenge-secret check, expiresIn date check, and demo-signer-by-address detection (now also enforced at the assertReplayStorePolicy boundary). validatePayKitConfig runs the replay/signer policy before the challenge-secret length so mainnet/store misconfig reports first. subscription reconciliation: - Subscription adapter requires a durable shared store via #237's nominal isProductionReplayStore rather than isShared/isDurable flags; the devnet in-memory opt-in (createUnsafeMemoryReplayStore, isAuthorizedUnsafe) is rejected for subscription activation and never accepted on mainnet. - The atomic view (createAtomicReplayStoreView) now exposes reserve() aliasing putIfAbsent, so #238's reserve-based subscription server runs unchanged against the operator's putIfAbsent production store. subscription-replay-store.ts is removed. session store: keep #237's low-level durability guard (sessionStoreDurability) and adopt #238's more robust global-registry enumerable isMemorySessionStore brand (survives package-copy and shallow-spread). tests: rewrite #238 tests that asserted the deleted validateReplayStore messages to the nominal assertReplayStorePolicy model; every replay/session/subscription money-path test still asserts fail-closed. Broaden the harness fail-closed signature to accept #237's Go "forbidden on mainnet" wording.
… mpp replay-store hardening Rehearsal-only reconciliations surfaced by composing the x402 (#236) and mpp replay-store (union base) leaves: - PayKitServiceProvider: drop the duplicate `use PayKit\Store\Store` the x402 import merge introduced. - PayKitExtension: register MppAdapter with an explicit class (2-arg) so the lazy mppFactory ServiceClosureArgument resolves it in an mpp-only container (1-arg left the class empty -> ReflectionException). - RequirePaymentTest: x402-only construction on devnet now acknowledges the single-process replay scope via PAY_KIT_ALLOW_INMEMORY_REPLAY_STORE, since #236 requires a durable shared x402 store off-localnet. - ReplayStoreInjectionTest: construct the Symfony listener with a named x402: arg (union base pins mpp before mppFactory by reflection), and opt the mpp-only localnet framework tests into allow_unsafe_memory_store (the mpp adapter requires an explicit opt-in even on localnet, by design).
…e combined tree Composing #236 (x402 reserve lifecycle) with the mpp nominal replay-store model (#237) surfaced three TS fallouts: - adapters/x402-shared: when a config injects an mpp-style atomic putIfAbsent store (the shared process-local store a dual x402+mpp config provisions from the unsafe opt-in), x402 now upgrades it to the reserve surface via the atomic view instead of rejecting it, so both protocols fence the same markers. Fixes the openapi x402+mpp dual-protocol config. - x402-adapter/x402-shared tests: pin accept: ['x402'] so the pure-x402 adapter tests use the reserve store rather than defaulting to the mpp putIfAbsent policy (which correctly rejects a reserve-only store). - mpp server index/store: prettier formatting the auto-merge left unformatted.
…into fix/x402-replay-hardening # Conflicts: # go/paykit/types.go # php/src/Frameworks/Laravel/PayKitServiceProvider.php # php/src/Frameworks/Laravel/RequirePaymentMiddleware.php # php/src/Frameworks/Symfony/DependencyInjection/PayKitExtension.php # php/src/Frameworks/Symfony/EventListener/RequirePaymentListener.php # typescript/packages/pay-kit/src/__tests__/x402.test.ts # typescript/packages/pay-kit/src/replay-store.ts
…into fix/x402-replay-hardening # Conflicts: # typescript/packages/pay-kit/src/replay-store.ts
… into fix/x402-replay-hardening # Conflicts: # harness/src/process.ts # harness/test/process.test.ts
…rface A dual x402+mpp config can inject (or auto-provision via the unsafe opt-in) an mpp-style atomic putIfAbsent store. Instead of rejecting it, wrap it in the atomic replay-store view so x402's reserve-before-settle fences the same consumed-signature markers as mpp. Reserve-capable stores pass through; non-atomic stores still fail closed.
…ore policy The mpp replay-store policy (declared atomic stores only) rejects the reserve-only memory store these pure-x402 fixtures inject. Pin accept: ['x402'] so the fixtures exercise the x402 reserve contract they test.
x402-only devnet constructions acknowledge single-process replay scope via PAY_KIT_ALLOW_INMEMORY_REPLAY_STORE; mpp-only localnet framework tests opt into allow_unsafe_memory_store (the mpp adapter requires the explicit opt-in even on localnet); the Symfony listener is constructed with a named x402: argument (mpp/mppFactory keep their reflected positions).
…er-entry An empty reservation list would let the per-entry TTL assertions pass vacuously; require at least one entry so the guard cannot false-green.
… into fix/x402-replay-hardening
… into fix/x402-replay-hardening
… into fix/x402-replay-hardening
…e probes The asserted-skip roster's honesty gate fired here: this leaf's tree carries the php/lua native fail-closed store guards, so labeling them absent is no longer true. Adopt the reconciled native-guard framework (probes key on each guard's in-tree marker and assert both the missing-store and the non-durable/ non-shared rejections), with the ruby marker keyed on the SHARED-store predicate its capability leaf introduces, so that probe stays honestly pending until its full subject lands. Here: 12 passed, 8 pending-skips with owned reasons.
… into fix/x402-replay-hardening
… into fix/x402-replay-hardening
… into fix/x402-replay-hardening
… into fix/x402-replay-hardening
Summary
Security invariants
Verification
axum,client, Clippy-D warnings, rustfmtBase is #219 per the protocol-split delivery plan. This is intentionally a protocol PR because the same replay and settlement invariant spans multiple SDKs.