From 9134fa88a17baa85aa322929dc9c8a8f03653ebd Mon Sep 17 00:00:00 2001 From: Georgy Butaev <41178744+g-but@users.noreply.github.com> Date: Sat, 29 Aug 2026 10:35:18 +0200 Subject: [PATCH] fix(payments): bound L402 verify retries per payment intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verify-retry branch sits deliberately outside the per-IP challenge budget, and that part is right: a payer who has genuinely paid must be able to retry until settlement is seen, and sharing the challenge budget would lock them out of their own purchase. But unbounded is not unbudgeted. Every verify on a non-terminal intent drives an outbound call — the recipient's NWC or LNURL relay, or mempool — so one valid token bought unlimited traffic aimed at someone else's infrastructure, and the per-IP limiter could not see it by design, because the thing being replayed is a token, not an address. rateLimitL402Verify gives it a budget of its own: 60 checks per minute, far more than an honest client polling a payment needs and far less than a loop, applied before the outbound call so a refused check never leaves the box. Keyed on the INTENT, not the preimage — the preimage is caller-supplied, so keying on it would have allowed exactly the bucket-rotation the per-IP limiter already fell to. The route crossed the 150-line gate on the way, so both branches moved to lib/api/l402-handlers: they carry different rate budgets and different failure surfaces, and neither is the route's business. Route is 36 lines. Refs bitbaum/orangecat#563 finding 7. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018waGt1ieA9TjpscqrbrnGb --- __tests__/unit/api/v1-pay-l402.test.ts | 66 +++++++- .../v1/pay/[entity_type]/[entity_id]/route.ts | 130 +------------- src/lib/api/l402-handlers.ts | 158 ++++++++++++++++++ src/lib/rate-limit.ts | 41 +++++ 4 files changed, 271 insertions(+), 124 deletions(-) create mode 100644 src/lib/api/l402-handlers.ts diff --git a/__tests__/unit/api/v1-pay-l402.test.ts b/__tests__/unit/api/v1-pay-l402.test.ts index dd3a0d7a5..d5e2e5758 100644 --- a/__tests__/unit/api/v1-pay-l402.test.ts +++ b/__tests__/unit/api/v1-pay-l402.test.ts @@ -65,11 +65,16 @@ jest.mock('@/lib/supabase/public', () => ({ createPublicClient: () => ({ _kind: jest.mock('@/lib/rate-limit', () => ({ rateLimitWriteAsync: jest.fn().mockResolvedValue({ success: true }), rateLimitPaymentRecipient: jest.fn().mockResolvedValue({ success: true }), + rateLimitL402Verify: jest.fn().mockResolvedValue({ success: true }), retryAfterSeconds: () => 1, })); import { GET } from '@/app/api/v1/pay/[entity_type]/[entity_id]/route'; -import { rateLimitPaymentRecipient } from '@/lib/rate-limit'; +import { + rateLimitL402Verify, + rateLimitPaymentRecipient, + rateLimitWriteAsync, +} from '@/lib/rate-limit'; const ENTITY_ID = '11111111-2222-3333-4444-555555555555'; const params = Promise.resolve({ entity_type: 'product', entity_id: ENTITY_ID }); @@ -246,7 +251,64 @@ describe('per-recipient invoice-spam limit', () => { it('refuses with 429 when that recipient is already being hammered', async () => { (rateLimitPaymentRecipient as jest.Mock).mockResolvedValue({ success: false }); - const res = await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID), { params }); + const res = await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID), { + params, + }); expect(res.status).toBe(429); }); }); + +/** + * The verify branch deliberately sits OUTSIDE the per-IP challenge budget: a + * payer who has genuinely paid must be able to retry until settlement is seen, + * and making them share the challenge budget would lock them out of their own + * purchase. + * + * But unbounded is not the same as unbudgeted. Every check on a non-terminal + * intent drives an outbound call to the recipient's LNURL/NWC relay or to + * mempool, so a single valid token bought unlimited traffic aimed at someone + * else's infrastructure — and the per-IP limiter could not see it, by design. + * + * bitbaum/orangecat#563 finding 7. + */ +describe('L402 verify-retry limit', () => { + const AUTH = { authorization: `L402 ${ENTITY_ID}.tok_abc:${'a'.repeat(64)}` }; + + beforeEach(() => { + (rateLimitL402Verify as jest.Mock).mockResolvedValue({ success: true }); + (rateLimitWriteAsync as jest.Mock).mockResolvedValue({ success: true }); + mockVerify.mockResolvedValue({ ok: false, status: 'pending' }); + }); + + it('keys the budget on the payment intent, which a caller cannot rotate', async () => { + await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID, AUTH), { params }); + expect(rateLimitL402Verify).toHaveBeenCalledWith(ENTITY_ID); + }); + + it('refuses with 429 once one token has hammered the relay', async () => { + (rateLimitL402Verify as jest.Mock).mockResolvedValue({ success: false }); + const res = await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID, AUTH), { + params, + }); + expect(res.status).toBe(429); + // Refused BEFORE the outbound check — the whole point is not making it. + expect(mockVerify).not.toHaveBeenCalled(); + }); + + it('still does not spend the challenge budget on a verify', async () => { + await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID, AUTH), { params }); + expect(rateLimitWriteAsync).not.toHaveBeenCalled(); + expect(rateLimitPaymentRecipient).not.toHaveBeenCalled(); + }); + + it('leaves the challenge branch on the per-IP budget, not the verify one', async () => { + mockCreateChallenge.mockResolvedValue({ + header: 'L402 token="i.t", invoice="lnbc1"', + payment_intent_id: 'i', + status_token: 't', + }); + await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID), { params }); + expect(rateLimitL402Verify).not.toHaveBeenCalled(); + expect(rateLimitWriteAsync).toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts b/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts index 8253ff519..ca1d79b56 100644 --- a/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts +++ b/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts @@ -1,18 +1,5 @@ -import { - apiBadRequest, - apiInternalError, - apiNotFound, - apiPaymentRequired, - apiRateLimited, - apiSuccess, -} from '@/lib/api/standardResponse'; -import { createL402Challenge, verifyL402Payment } from '@/domain/payments/l402'; import { parseL402Authorization } from '@/domain/payments/l402-codec'; -import { publicSupportCreateSchema } from '@/lib/validation/finance'; -import { createPublicClient } from '@/lib/supabase/public'; -import { rateLimitPaymentRecipient, rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit'; -import { logger } from '@/utils/logger'; -import { clientIpKey } from '@/lib/client-ip'; +import { handleL402Challenge, handleL402Verify } from '@/lib/api/l402-handlers'; /** * GET /api/v1/pay/{entity_type}/{entity_id}?amount_btc=X — HTTP 402 inline payment. @@ -29,120 +16,19 @@ import { clientIpKey } from '@/lib/client-ip'; * The JSON-contract flow (POST /api/v1/payments + polling) remains the * account-based sibling; this one exists so foreign agents that only speak * 402 can pay without learning our dialect. + * + * Both branches live in lib/api/l402-handlers — they carry different rate + * budgets and different failure surfaces, and neither is the route's business. */ - -function requestKey(request: Request): string { - return `l402:${clientIpKey(request)}`; -} - export async function GET( request: Request, { params }: { params: Promise<{ entity_type: string; entity_id: string }> } ) { const { entity_type, entity_id } = await params; + const target = { entityType: entity_type, entityId: entity_id }; - // Retry-with-proof branch first: verification is cheap and shouldn't share - // the challenge-creation rate budget. + // Retry-with-proof first: a caller holding credentials is answering a + // challenge we already issued, not asking for a new one. const creds = parseL402Authorization(request.headers.get('authorization')); - if (creds) { - try { - const result = await verifyL402Payment(creds, { - entityType: entity_type, - entityId: entity_id, - }); - if (result.ok) { - return apiSuccess( - { - status: result.status, - paid_at: result.paid_at, - verified_by: result.verified_by, - entity_type, - entity_id, - }, - { cache: 'NONE' } - ); - } - // Not paid yet — 402 again. The client already holds the invoice from - // the original challenge, so no new intent is created here. - return apiPaymentRequired('Invoice not paid yet — pay it, then retry with the preimage.', { - status: result.status, - }); - } catch (error) { - const message = error instanceof Error ? error.message : ''; - if (message.includes('not found')) { - return apiNotFound('Payment not found'); - } - logger.error('L402 verification failed', { error }); - return apiInternalError('Could not verify the payment.'); - } - } - - // Challenge branch: create an account-less intent and answer 402. - const limit = await rateLimitWriteAsync(requestKey(request)); - if (!limit.success) { - return apiRateLimited( - 'Too many payment requests. Try again shortly.', - retryAfterSeconds(limit) - ); - } - - // Per-IP is not enough here. Every challenge mints a REAL invoice through the - // recipient's own LNURL/NWC relay, so an attacker rotating IPs can still get - // a seller rate-limited or banned by their wallet provider. This bounds the - // damage to one recipient however many addresses it arrives from. - const recipientLimit = await rateLimitPaymentRecipient(entity_type, entity_id); - if (!recipientLimit.success) { - return apiRateLimited( - 'This page is receiving too many payment requests right now. Try again shortly.', - retryAfterSeconds(recipientLimit) - ); - } - - const url = new URL(request.url); - const parsed = publicSupportCreateSchema.safeParse({ - entity_type, - entity_id, - amount_btc: Number(url.searchParams.get('amount_btc')), - }); - if (!parsed.success) { - return apiBadRequest( - 'Invalid payment request — pass ?amount_btc= between 0.000001 and 1.', - parsed.error.errors - ); - } - - try { - const challenge = await createL402Challenge(createPublicClient(), parsed.data); - return apiPaymentRequired( - 'Pay the invoice, then retry with Authorization: L402 :.', - { - payment_intent_id: challenge.payment_intent_id, - token: `${challenge.payment_intent_id}.${challenge.status_token}`, - bolt11: challenge.bolt11, - onchain_address: challenge.onchain_address, - amount_btc: challenge.amount_btc, - method_label: challenge.method_label, - expires_in_seconds: challenge.expires_in_seconds, - }, - challenge.header - ); - } catch (error) { - const message = error instanceof Error ? error.message : 'Payment initiation failed'; - logger.error('L402 challenge creation failed', { error, entity_type, entity_id }); - // Same safe-error surface as the public support route — never leak internals. - const safeErrors: Array<[string, string]> = [ - ['not publicly available', 'This entity is not available for public payment.'], - ['cannot receive', 'This kind of entity cannot receive payments.'], - ['owner not found', 'This entity no longer has a receiving owner.'], - ['no wallet', 'The owner has not connected a Bitcoin wallet yet.'], - ['outside allowed range', 'That amount is outside the wallet provider’s allowed range.'], - ['LNURL', 'The Lightning Address could not create an invoice. Try again later.'], - ]; - for (const [pattern, safe] of safeErrors) { - if (message.toLowerCase().includes(pattern.toLowerCase())) { - return apiBadRequest(safe); - } - } - return apiInternalError('Could not create the payment challenge.'); - } + return creds ? handleL402Verify(creds, target) : handleL402Challenge(request, target); } diff --git a/src/lib/api/l402-handlers.ts b/src/lib/api/l402-handlers.ts new file mode 100644 index 000000000..69eb84224 --- /dev/null +++ b/src/lib/api/l402-handlers.ts @@ -0,0 +1,158 @@ +import { + apiBadRequest, + apiInternalError, + apiNotFound, + apiPaymentRequired, + apiRateLimited, + apiSuccess, +} from '@/lib/api/standardResponse'; +import { createL402Challenge, verifyL402Payment } from '@/domain/payments/l402'; +import type { L402Credentials } from '@/domain/payments/l402-codec'; +import { publicSupportCreateSchema } from '@/lib/validation/finance'; +import { createPublicClient } from '@/lib/supabase/public'; +import { + rateLimitL402Verify, + rateLimitPaymentRecipient, + rateLimitWriteAsync, + retryAfterSeconds, +} from '@/lib/rate-limit'; +import { logger } from '@/utils/logger'; +import { clientIpKey } from '@/lib/client-ip'; + +/** + * The two branches of GET /api/v1/pay/{type}/{id}, kept out of the route file + * so the route stays what it should be: parse, pick a branch, delegate. + */ + +interface PayTarget { + entityType: string; + entityId: string; +} + +/** + * Retry-with-proof. Deliberately OFF the challenge budget — a payer who has + * paid must be able to retry until settlement is seen, and sharing the + * challenge budget would lock them out of their own purchase. + * + * It gets a budget of its own instead. Each check on a non-terminal intent + * drives an outbound call to the recipient's relay or to mempool, so one valid + * token would otherwise buy unbounded traffic against someone else's + * infrastructure. Keyed on the INTENT, which a caller cannot vary without a + * valid status token for some other payment — so unlike the per-IP budget on + * the challenge branch, rotating addresses buys nothing here. + */ +export async function handleL402Verify( + creds: L402Credentials, + { entityType, entityId }: PayTarget +): Promise { + const verifyLimit = await rateLimitL402Verify(creds.paymentIntentId); + if (!verifyLimit.success) { + return apiRateLimited( + 'Too many verification checks for this payment. Try again shortly.', + retryAfterSeconds(verifyLimit) + ); + } + + try { + const result = await verifyL402Payment(creds, { entityType, entityId }); + if (result.ok) { + return apiSuccess( + { + status: result.status, + paid_at: result.paid_at, + verified_by: result.verified_by, + entity_type: entityType, + entity_id: entityId, + }, + { cache: 'NONE' } + ); + } + // Not paid yet — 402 again. The client already holds the invoice from the + // original challenge, so no new intent is created here. + return apiPaymentRequired('Invoice not paid yet — pay it, then retry with the preimage.', { + status: result.status, + }); + } catch (error) { + const message = error instanceof Error ? error.message : ''; + if (message.includes('not found')) { + return apiNotFound('Payment not found'); + } + logger.error('L402 verification failed', { error }); + return apiInternalError('Could not verify the payment.'); + } +} + +/** Never leak internals — same safe-error surface as the public support route. */ +const SAFE_CHALLENGE_ERRORS: Array<[string, string]> = [ + ['not publicly available', 'This entity is not available for public payment.'], + ['cannot receive', 'This kind of entity cannot receive payments.'], + ['owner not found', 'This entity no longer has a receiving owner.'], + ['no wallet', 'The owner has not connected a Bitcoin wallet yet.'], + ['outside allowed range', 'That amount is outside the wallet provider’s allowed range.'], + ['LNURL', 'The Lightning Address could not create an invoice. Try again later.'], +]; + +/** Create an account-less intent and answer 402 with the invoice. */ +export async function handleL402Challenge( + request: Request, + { entityType, entityId }: PayTarget +): Promise { + const limit = await rateLimitWriteAsync(`l402:${clientIpKey(request)}`); + if (!limit.success) { + return apiRateLimited( + 'Too many payment requests. Try again shortly.', + retryAfterSeconds(limit) + ); + } + + // Per-IP is not enough here. Every challenge mints a REAL invoice through the + // recipient's own LNURL/NWC relay, so an attacker rotating IPs can still get a + // seller rate-limited or banned by their wallet provider. This bounds the + // damage to one recipient however many addresses it arrives from. + const recipientLimit = await rateLimitPaymentRecipient(entityType, entityId); + if (!recipientLimit.success) { + return apiRateLimited( + 'This page is receiving too many payment requests right now. Try again shortly.', + retryAfterSeconds(recipientLimit) + ); + } + + const url = new URL(request.url); + const parsed = publicSupportCreateSchema.safeParse({ + entity_type: entityType, + entity_id: entityId, + amount_btc: Number(url.searchParams.get('amount_btc')), + }); + if (!parsed.success) { + return apiBadRequest( + 'Invalid payment request — pass ?amount_btc= between 0.000001 and 1.', + parsed.error.errors + ); + } + + try { + const challenge = await createL402Challenge(createPublicClient(), parsed.data); + return apiPaymentRequired( + 'Pay the invoice, then retry with Authorization: L402 :.', + { + payment_intent_id: challenge.payment_intent_id, + token: `${challenge.payment_intent_id}.${challenge.status_token}`, + bolt11: challenge.bolt11, + onchain_address: challenge.onchain_address, + amount_btc: challenge.amount_btc, + method_label: challenge.method_label, + expires_in_seconds: challenge.expires_in_seconds, + }, + challenge.header + ); + } catch (error) { + const message = error instanceof Error ? error.message : 'Payment initiation failed'; + logger.error('L402 challenge creation failed', { error, entityType, entityId }); + for (const [pattern, safe] of SAFE_CHALLENGE_ERRORS) { + if (message.toLowerCase().includes(pattern.toLowerCase())) { + return apiBadRequest(safe); + } + } + return apiInternalError('Could not create the payment challenge.'); + } +} diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 872f7d9a2..1f5c8a0cd 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -98,6 +98,9 @@ const upstashWriteLimiter = createUpstashLimiter('write', 30, '1 m'); // (even rotating IPs) can't flood a victim's NWC relay. Generous enough to // absorb a legitimately viral post's concurrent tippers. const upstashTipRecipientLimiter = createUpstashLimiter('tip-recipient', 20, '5 m'); +// Generous on purpose: a paying client polls, and refusing a real payer is +// worse than the outbound traffic this bounds. +const upstashL402VerifyLimiter = createUpstashLimiter('l402-verify', 60, '1 m'); // Public Ask-Cat / feedback endpoint: each submission costs a platform LLM // call, and the caller may be anonymous — so a tight per-IP budget on top of // the general limiter. 8 per 5 min is plenty for a real person having a @@ -166,6 +169,10 @@ const fallbackTipRecipientLimiter = new InMemoryRateLimiter({ windowMs: 5 * 60 * 1000, maxRequests: 20, }); +const fallbackL402VerifyLimiter = new InMemoryRateLimiter({ + windowMs: 60 * 1000, + maxRequests: 60, +}); const fallbackAskCatLimiter = new InMemoryRateLimiter({ windowMs: 5 * 60 * 1000, maxRequests: 8, @@ -275,6 +282,40 @@ export async function rateLimitPaymentRecipient( return rateLimitTipRecipient(`${entityType}:${entityId}`); } +/** + * Rate limit L402 verification PER TOKEN. + * + * The verify branch is deliberately unlimited: a payer who has paid must be + * able to retry until we see it, and sharing the challenge budget would let + * invoice-minting starve settlement confirmation. But "unlimited" is doing more + * than that. Each verify on a non-terminal intent drives an OUTBOUND call — the + * recipient's NWC or LNURL relay, or mempool — so one valid token buys unbounded + * traffic against someone else's infrastructure, and the per-IP limiter cannot + * see it because a token is the thing being replayed, not an address. + * + * Keyed on the intent, not the caller: 60 checks per minute is far more than an + * honest client polling a payment needs, and far less than a loop. The budget + * covers terminal intents too, even though those short-circuit in + * refreshPaymentStatus before any rail call — a cheap check is still a check, + * and one bound is easier to reason about than two. + * bitbaum/orangecat#563 finding 7. + */ +export async function rateLimitL402Verify(paymentIntentId: string): Promise { + // Keyed on the INTENT, not the preimage. The intent is what the outbound call + // is about, and it cannot be varied without a valid status token for a + // different payment — whereas a preimage is caller-supplied, so keying on it + // would let the same bucket-rotation trick the per-IP limiter already fell to. + // The id is not a secret (it is in the status route's own URL), so no hash. + const key = `l402-verify:${paymentIntentId}`; + + if (upstashL402VerifyLimiter) { + const result = await upstashL402VerifyLimiter.limit(key); + return toRateLimitResult(result); + } + + return fallbackL402VerifyLimiter.check(key); +} + /** * Rate limit public Ask-Cat / feedback submissions per IP. * 8 per 5 minutes — each one is a platform LLM call from a possibly anonymous