Public: anyone can read this; the crew can change it.
A payment webhook delivered twice changes nothing the second time: one test per event the forge handles #11876
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/11866-a-payment-webhook-delivered-twice-change → main 3 commits fork 1 thread · 1 unresolved
2 files
changed +170
−4
src/http/stripe_hook.ts +8 −4
| @@ -5,7 +5,7 @@ | ||
| 5 | 5 | * checked with `verifyStripeSignature` against the sealed webhook secret, and |
| 6 | 6 | * only then parsed. A bad or stale signature is 400 and counted per UTC day |
| 7 | 7 | * (`stripe_hook_refused`). A verified event's id is stored once; a repeated id |
| 8 | * is 200 and does nothing, and so is an event type nobody acts on yet. | |
| 8 | * is 200 saying "already handled" and does nothing; so is an event type nobody acts on yet. | |
| 9 | 9 | * `checkout.session.completed` binds its forge (`billing/checkout_completed.ts`). |
| 10 | 10 | * A `customer.subscription.*` or `invoice.*` event only names a subscription: the |
| 11 | 11 | * hook re-reads it from Stripe and writes the entitlement from that, one |
| @@ -53,8 +53,11 @@ export function setStripeHookCaps(app: App, perClient: number, wholeForge: numbe | ||
| 53 | 53 | bucketsOf(app, perClient, wholeForge); |
| 54 | 54 | } |
| 55 | 55 | |
| 56 | const answer = (status: number, headers?: Record<string, string>): Response => | |
| 57 | new Response(null, { status, headers: { "cache-control": "no-store", ...headers } }); | |
| 56 | const answer = (status: number, headers?: Record<string, string>, body: string | null = null): Response => | |
| 57 | new Response(body, { status, headers: { "cache-control": "no-store", ...headers } }); | |
| 58 | ||
| 59 | /** What a redelivered event id is told: a 200, so Stripe stops, and these words, so a person reading the log knows. */ | |
| 60 | export const ALREADY_HANDLED = "already handled"; | |
| 58 | 61 | |
| 59 | 62 | /** Bad and stale signatures refused today and earlier, by kind. */ |
| 60 | 63 | export function refusedCount(app: App, kind: "bad" | "stale"): number { |
| @@ -128,7 +131,8 @@ export async function stripeHook(ctx: Ctx): Promise<Response> { | ||
| 128 | 131 | if (settled?.state === "spawned") bindCheckout(app, event); |
| 129 | 132 | } |
| 130 | 133 | } |
| 131 | if (fresh && (type.startsWith("customer.subscription.") || type.startsWith("invoice."))) { | |
| 134 | if (!fresh) return answer(200, { "content-type": "text/plain; charset=utf-8" }, ALREADY_HANDLED); | |
| 135 | if ((type.startsWith("customer.subscription.") || type.startsWith("invoice."))) { | |
Non-blocking: drop the redundant outer parentheses on this condition. |
||
| 132 | 136 | try { |
| 133 | 137 | await syncSubscription(app, subscriptionIdOf(event)); |
| 134 | 138 | } catch (cause) { |
test/billing-replay.test.ts added +162
| @@ -0,0 +1,162 @@ | ||
| 1 | // A payment webhook delivered twice changes nothing the second time (#11866): one case per event the forge handles. | |
| 2 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { sealKey, sealWebhook } from "../src/billing/connection.ts"; | |
| 8 | import { lapseFor } from "../src/billing/lapse.ts"; | |
| 9 | import { changesOf, getStored } from "../src/billing/store.ts"; | |
| 10 | import { setStripeFetch } from "../src/billing/sync.ts"; | |
| 11 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 12 | import { refusedCount, ALREADY_HANDLED } from "../src/http/stripe_hook.ts"; | |
| 13 | import { usePlacement, useProbe } from "../src/spawn/door.ts"; | |
| 14 | import { ok } from "../src/kernel/result.ts"; | |
| 15 | import { placeOrder } from "../src/actions/order.ts"; | |
| 16 | import { useCheckout, type CheckoutProvider } from "../src/billing/checkout.ts"; | |
| 17 | import { createUser } from "../src/core/users.ts"; | |
| 18 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 19 | ||
| 20 | const secret = "whsec_replay_test_secret"; | |
| 21 | let dir: string; | |
| 22 | let app: App; | |
| 23 | let servers: Servers; | |
| 24 | let status = "active"; | |
| 25 | let placed = 0; | |
| 26 | let mails = 0; | |
| 27 | let n = 0; | |
| 28 | ||
| 29 | async function sign(body: string, key = secret, t = Math.floor(Date.now() / 1000)): Promise<string> { | |
| 30 | const k = await crypto.subtle.importKey("raw", new TextEncoder().encode(key), { name: "HMAC", hash: "SHA-256" }, false, ["sign"]); | |
| 31 | const mac = new Uint8Array(await crypto.subtle.sign("HMAC", k, new TextEncoder().encode(`${t}.${body}`))); | |
| 32 | return `t=${t},v1=${Buffer.from(mac).toString("hex")}`; | |
| 33 | } | |
| 34 | const post = (body: string, header: string) => | |
| 35 | fetch(`http://127.0.0.1:${servers.port}/hooks/stripe`, { method: "POST", headers: { "stripe-signature": header, "x-forwarded-for": `10.8.0.${++n}` }, body }); | |
| 36 | const count = (sql: string): number => app.db.query<{ n: number }, []>(sql).get()?.n ?? 0; | |
| 37 | ||
| 38 | const fake: CheckoutProvider = { | |
| 39 | name: "fake", | |
| 40 | async create(req) { const id = `cs_replay_${req.orderId.slice(4, 16)}`; return ok({ id, url: `https://pay.fake.test/${id}` }); }, | |
| 41 | async state() { return ok({ paid: true, customer: "cus_1", subscription: "sub_1" }); }, | |
| 42 | }; | |
| 43 | ||
| 44 | beforeAll(async () => { | |
| 45 | dir = await mkdtemp(join(tmpdir(), "billing-replay-")); | |
| 46 | app = createApp({ dataDir: dir, port: 0, forgeRepo: "alice/forge-code", mailer: async () => { mails++; return true; } }); | |
| 47 | servers = await serve(app); | |
| 48 | sealKey(app.db, dir, "sk_test_abcdefgh12345678"); | |
| 49 | sealWebhook(app.db, dir, secret); | |
| 50 | const a = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true }); | |
| 51 | if (!a.ok) throw new Error("user"); | |
| 52 | const code = await createRepo(app, a.value, { name: "forge-code", visibility: "public" }); | |
| 53 | if (!code.ok) throw new Error("code repo"); | |
| 54 | await commitToBranch(app, a.value, code.value, { message: "v1", files: [{ path: "src/main.ts", content: "console.log('forge')" }] }); | |
| 55 | useProbe(app, () => async () => true); | |
| 56 | usePlacement(app, { place: async (plan) => ok({ url: `https://${plan.name}.fake.test`, bodyId: `fake-${++placed}` }), retire: async () => ok() }); | |
| 57 | useCheckout(app, fake); | |
| 58 | setStripeFetch(app, (async () => | |
| 59 | new Response(JSON.stringify({ id: "sub_1", status, customer: "cus_1", metadata: { forge: "acme", plan: "medium" }, items: { data: [{ current_period_end: 1900000000 }] } }), { status: 200 })) as unknown as typeof fetch); | |
| 60 | }, 30_000); | |
| 61 | afterAll(async () => { | |
| 62 | setStripeFetch(app, undefined); | |
| 63 | await servers.stop(); | |
| 64 | app.close(); | |
| 65 | await rm(dir, { recursive: true, force: true }); | |
| 66 | }); | |
| 67 | ||
| 68 | /** Deliver the same signed body twice; the first is handled, the second is told so. */ | |
| 69 | async function twice(body: string): Promise<void> { | |
| 70 | const header = await sign(body); | |
| 71 | const first = await post(body, header); | |
| 72 | expect(first.status).toBe(200); | |
| 73 | expect(await first.text()).not.toContain(ALREADY_HANDLED); | |
| 74 | const second = await post(body, header); | |
| 75 | expect(second.status).toBe(200); | |
| 76 | expect(await second.text()).toContain(ALREADY_HANDLED); | |
| 77 | } | |
| 78 | ||
| 79 | describe("a replayed webhook changes nothing", () => { | |
| 80 | test("checkout completed: one forge, one order row, one entitlement change, no mail", async () => { | |
| 81 | const placedOrder = await placeOrder(app, { name: "shop", plan: "small", email: "[email protected]", address: "203.0.113.9" }, new URL(`http://127.0.0.1:${servers.port}/`)); | |
| 82 | if (!placedOrder.ok) throw new Error("order"); | |
| 83 | const order = app.db.query<{ id: string; session_id: string }, []>("SELECT id, session_id FROM forge_order").get()!; | |
| 84 | const body = JSON.stringify({ id: "evt_checkout", type: "checkout.session.completed", data: { object: { id: order.session_id, metadata: { order: order.id }, customer: "cus_9", subscription: "sub_shop", mode: "subscription", payment_status: "paid" } } }); | |
| 85 | const header = await sign(body); | |
| 86 | const placedStart = placed; | |
| 87 | expect((await post(body, header)).status).toBe(200); | |
| 88 | expect(placed - placedStart).toBe(1); | |
| 89 | const orders = count("SELECT COUNT(*) AS n FROM forge_order"); | |
| 90 | const changes = changesOf(app.db, "shop").length; | |
| 91 | const events = count("SELECT COUNT(*) AS n FROM stripe_event"); | |
| 92 | const placedBefore = placed; | |
| 93 | // The hook sends no mail at all, so this only guards against one being added later. | |
| 94 | const mailsBefore = mails; | |
| 95 | const again = await post(body, header); | |
| 96 | expect(again.status).toBe(200); | |
| 97 | expect(await again.text()).toContain(ALREADY_HANDLED); | |
| 98 | expect(placed).toBe(placedBefore); | |
| 99 | expect(count("SELECT COUNT(*) AS n FROM forge_order")).toBe(orders); | |
| 100 | expect(changesOf(app.db, "shop").length).toBe(changes); | |
| 101 | expect(count("SELECT COUNT(*) AS n FROM stripe_event")).toBe(events); | |
| 102 | expect(mails).toBe(mailsBefore); | |
| 103 | }); | |
| 104 | ||
| 105 | test("payment failed: the grace clock does not move on the second delivery", async () => { | |
| 106 | status = "active"; | |
| 107 | await twice(JSON.stringify({ id: "evt_active", type: "customer.subscription.updated", data: { object: { id: "sub_1" } } })); | |
| 108 | status = "past_due"; | |
| 109 | const body = JSON.stringify({ id: "evt_failed", type: "invoice.payment_failed", data: { object: { subscription: "sub_1" } } }); | |
| 110 | const header = await sign(body); | |
| 111 | expect((await post(body, header)).status).toBe(200); | |
| 112 | const t0 = Date.now(); | |
| 113 | const lapse = lapseFor(app.db, "acme", t0); | |
| 114 | expect(lapse.state).toBe("grace"); | |
| 115 | const changes = changesOf(app.db, "acme").length; | |
| 116 | const stored = getStored(app.db, "acme"); | |
| 117 | await Bun.sleep(15); | |
| 118 | const again = await post(body, header); | |
| 119 | expect(again.status).toBe(200); | |
| 120 | expect(await again.text()).toContain(ALREADY_HANDLED); | |
| 121 | expect(lapseFor(app.db, "acme", t0)).toEqual(lapse); | |
| 122 | expect(changesOf(app.db, "acme").length).toBe(changes); | |
| 123 | expect(getStored(app.db, "acme")).toEqual(stored); | |
| 124 | }); | |
| 125 | ||
| 126 | test("subscription cancelled: second delivery adds no change and keeps the lapse", async () => { | |
| 127 | status = "canceled"; | |
| 128 | const body = JSON.stringify({ id: "evt_cancel", type: "customer.subscription.deleted", data: { object: { id: "sub_1" } } }); | |
| 129 | const header = await sign(body); | |
| 130 | expect((await post(body, header)).status).toBe(200); | |
| 131 | const lapse = lapseFor(app.db, "acme"); | |
| 132 | expect(lapse.state).toBe("none"); | |
| 133 | const changes = changesOf(app.db, "acme").length; | |
| 134 | await Bun.sleep(15); | |
| 135 | const again = await post(body, header); | |
| 136 | expect(await again.text()).toContain(ALREADY_HANDLED); | |
| 137 | expect(lapseFor(app.db, "acme")).toEqual(lapse); | |
| 138 | expect(changesOf(app.db, "acme").length).toBe(changes); | |
| 139 | expect(mails).toBe(0); | |
| 140 | }); | |
| 141 | }); | |
| 142 | ||
| 143 | describe("the guard", () => { | |
| 144 | const body = JSON.stringify({ id: "evt_guard", type: "customer.subscription.updated", data: { object: { id: "sub_1" } } }); | |
| 145 | ||
| 146 | test("a bad signature is refused, counted, and its event is not kept", async () => { | |
| 147 | const before = refusedCount(app, "bad"); | |
| 148 | const res = await post(body, await sign(body, "whsec_someone_else")); | |
| 149 | expect(res.status).toBe(400); | |
| 150 | expect(refusedCount(app, "bad")).toBe(before + 1); | |
| 151 | expect(count("SELECT COUNT(*) AS n FROM stripe_event WHERE id = 'evt_guard'")).toBe(0); | |
| 152 | }); | |
| 153 | ||
| 154 | test("a validly signed but old timestamp is refused as stale", async () => { | |
| 155 | const before = refusedCount(app, "stale"); | |
| 156 | const old = Math.floor(Date.now() / 1000) - 3600; | |
| 157 | const res = await post(body, await sign(body, secret, old)); | |
| 158 | expect(res.status).toBe(400); | |
| 159 | expect(refusedCount(app, "stale")).toBe(before + 1); | |
| 160 | expect(count("SELECT COUNT(*) AS n FROM stripe_event WHERE id = 'evt_guard'")).toBe(0); | |
| 161 | }); | |
| 162 | }); |
new above
offline
freshis always true here after the early return above; removefresh &&so the condition is not dead.