ReviewOS

also looking at this

stacks/uplink

feat(billing): issue the license from Stripe, not only from the thank-you page

#49
Merged glennmichael123 wants to merge feat/stripe-webhook into main
9 files +439 -2
9 days ago

Closes #48.

What was broken

issue() had exactly one caller: rendering /thanks?session_id=..., which is checkout's success_url. The redirect after payment was the only event in the system that ever created a license.

A buyer who closed the tab, lost the network at that moment, or paid by a method that settles after the redirect, was charged and got nothing. No row, no key, no email, and no way to ask: there is no resend path, and /license/check needs a key they do not have.

What it does

POST /api/billing/webhook takes Stripe's own copy of the checkout and calls the same issue().

Nothing about issuing is reimplemented. It was already idempotent: bySession returns the existing row, and the email only sends for a license created just now. So the webhook and the page can arrive in either order, or twice, and the buyer ends up with one license and one email. There is a test for exactly that, both orders, two deliveries each.

Status codes, because Stripe retries anything that is not 2xx

situationcodewhy
license issued, or already there200done
checkout.session.completed while still unpaid200async_payment_succeeded is already on its way
an event this endpoint does not handle200retrying will not make it interesting
bad or missing signature, body not an event400never improves
issuing threw, e.g. the database is down500retry may well work, and the purchase is real
no signing secret configured500it cannot verify, so it must not believe anyone

Two things worth reviewing closely

It verifies the bytes on the wire. rawBody(), not the parsed body: Stripe signs what it sent, and a re-serialized JSON body never matches the HMAC. The router documents keeping the raw bytes for exactly this. A test drives the action with a request whose parsed accessors count their reads, and asserts the raw body is read once and the parsed body never.

Verification is verifyStripe from @stacksjs/security, not the payments package's constructEvent. It is a plain HMAC check with no Stripe client behind it, so it cannot hit the SubtleCrypto trap that made the SDK path 401 every genuine delivery on Bun (stacksjs/stacks#2355). That failure is worth avoiding by construction rather than by remembering: Stripe retried, every retry 401'd, and nothing caught it because tests that call handlers directly never touch verification.

Config

STRIPE_WEBHOOK_SECRET is declared in config/env.ts, .env.example and config/payment.ts, whose comment already referred to "the account webhook secret above" that was not there. config/saas.ts carried the scaffold's 'your-webhook-endpoint', which described something that did not exist, and now names the real one.

storage/framework/auto-imports/actions.ts gains one line from buddy generate, which the route needs to typecheck. I deliberately left api-types.ts, client.ts and openapi.json alone: regenerating them here deletes tens of thousands of lines of unrelated routes, which looks like pre-existing drift between what is committed and what the generator now produces. Worth a separate look, not this PR.

Still needs a human

The endpoint has to be created in the Stripe dashboard, subscribed to checkout.session.completed and checkout.session.async_payment_succeeded, and its whsec_... put in the environment. Until then the endpoint answers 500 and issues nothing, which is the safe direction but is not the same as working.

The payment_method_types question from the issue is untouched, since it is a dashboard decision rather than a code one.

Verification

  • 139 unit tests pass, 11 new here
  • ./buddy lint and ./buddy typecheck clean

🤖 Generated with Claude Code

9 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.