Pre-launch — Gluecron is in final validation. Public signups and git hosting for non-owner users open after launch review.
ccantynz/Gluecron.com
Commit8298a0f

fix(billing): scope Stripe events to the subscription on record

fix(billing): scope Stripe events to the subscription on record

customer.subscription.deleted updated userQuotas keyed on
stripeCustomerId alone, ignoring the deleted subscription's own id. One
Stripe customer can hold several subscriptions, and a plan change creates
the new one and deletes the old with no ordering guarantee between the
webhooks. So created(sub_NEW) routinely landed first and upgraded the
row, then deleted(sub_OLD) matched that same row and reset it to
planSlug "free" with a null subscription id.

That bills a customer while the product treats them as free, and it fails
in the direction least likely to be reported quickly — the downgrade
looks like an ordinary plan state rather than an error.

Two more sites had the same flaw, both keyed on customer alone:
reconcileSubscription's non-active branch, where a canceled or past_due
event for a superseded subscription stamped its id and status over the
live one, and invoice.payment_failed. All three now go through one shared
recordedSubscriptionScope helper rather than repeating the rule.

A row whose subscription id is NULL still matches: that is a row checkout
populated but reconcile never enriched, so the event is the only
subscription it could concern. Dropping that branch would silently
swallow real cancellations.

The active-upgrade branch is deliberately left UNSCOPED, and now says so
in the code. A newly-created subscription is by definition not yet the one
on record, so scoping it would reject the upgrade and strand a paying
customer on their old plan — the very failure this change exists to stop.
Stale events cannot reach it because a superseded subscription is not
active. Scoping guards stale NEGATIVE events; positive ones take the row.
The test pins that exemption to exactly one site, and it was the test that
caught the branch in the first place.

The drizzle SQL shape asserted here was inspected, not assumed.

Both fault classes proven by injection: restoring the customer-only match
fails 2 tests, dropping the NULL branch fails 1.

Suite: 3478 pass, 0 fail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ccantynz-alt committed on July 29, 2026Parent: 44b89b0
2 files changed+268−78298a0fa5d21a553a1d2dabdbe1023016211f827
2 changed files+268−7
Addedsrc/__tests__/stripe-subscription-scope.test.ts+176−0View fileUnifiedSplit
1/**
2 * Regression guard: a Stripe event must only touch the subscription the user
3 * actually has on record.
4 *
5 * `customer.subscription.deleted` updated userQuotas keyed on
6 * stripeCustomerId alone, ignoring the deleted subscription's own id. One
7 * Stripe customer can hold several subscriptions, and a plan change creates
8 * the new one and deletes the old — with no ordering guarantee between the
9 * two webhooks. So `created`(sub_NEW) routinely lands first, upgrading the
10 * row, and then `deleted`(sub_OLD) matched that same row and reset it to
11 * planSlug "free" with a null subscription id.
12 *
13 * The result is a customer who is being billed while the product treats
14 * them as free. It fails silently and in the direction least likely to be
15 * reported quickly, since the downgrade looks like an ordinary plan state.
16 *
17 * The same flaw sat in two more places, both keyed on customer alone:
18 * reconcileSubscription's non-active branch (a canceled or past_due event
19 * for a superseded subscription stamped its id and status over the live
20 * one) and invoice.payment_failed.
21 *
22 * These assert the shared scope condition rather than round-tripping a
23 * webhook, so no database is required and no module needs mocking.
24 */
25
26import { describe, it, expect } from "bun:test";
27import { readFileSync } from "node:fs";
28import { join } from "node:path";
29import { recordedSubscriptionScope } from "../routes/stripe-webhook";
30
31/**
32 * Flatten a drizzle SQL tree into labelled leaves.
33 *
34 * The shape was inspected rather than assumed: the condition is a nested
35 * SQL whose chunks are StringChunk (carrying `.value`), column references,
36 * and Param nodes. Guessing at this shape is what makes these tests wrong.
37 */
38type Leaf = { kind: "str" | "col" | "param"; value: string };
39
40function leaves(node: unknown, out: Leaf[] = []): Leaf[] {
41 if (!node) return out;
42 if (Array.isArray(node)) {
43 for (const n of node) leaves(n, out);
44 return out;
45 }
46 const anyNode = node as any;
47 const kind = anyNode?.constructor?.name;
48 if (kind === "SQL") return leaves(anyNode.queryChunks, out);
49 if (kind === "StringChunk") {
50 out.push({ kind: "str", value: String(anyNode.value ?? "") });
51 return out;
52 }
53 if (kind === "Param") {
54 out.push({ kind: "param", value: String(anyNode.value) });
55 return out;
56 }
57 if (anyNode?.name && anyNode?.table) {
58 out.push({ kind: "col", value: String(anyNode.name) });
59 }
60 return out;
61}
62
63const text = (ls: Leaf[]) =>
64 ls
65 .filter((l) => l.kind === "str")
66 .map((l) => l.value)
67 .join(" ");
68
69describe("recordedSubscriptionScope", () => {
70 it("returns no constraint when the event names no subscription", () => {
71 // A one-off invoice has no subscription; the customer match is all
72 // there is to go on, so the caller must not be over-constrained.
73 expect(recordedSubscriptionScope("")).toBeUndefined();
74 });
75
76 it("constrains on the subscription id the event carries", () => {
77 const ls = leaves(recordedSubscriptionScope("sub_ABC"));
78 expect(ls.some((l) => l.kind === "col" && l.value === "stripe_subscription_id")).toBe(true);
79 expect(ls.some((l) => l.kind === "param" && l.value === "sub_ABC")).toBe(true);
80 });
81
82 it("does not bind some other subscription's id", () => {
83 const ls = leaves(recordedSubscriptionScope("sub_NEW"));
84 expect(ls.some((l) => l.kind === "param" && l.value === "sub_OLD")).toBe(false);
85 });
86
87 it("also matches a row whose subscription id was never recorded", () => {
88 // Checkout populates the row; reconcile enriches it. A row caught
89 // between the two is NULL, and the event is the only subscription it
90 // could be about — so it must still be matched or a real cancellation
91 // would be dropped.
92 const sql = text(leaves(recordedSubscriptionScope("sub_ABC")));
93 expect(sql).toContain(" or ");
94 expect(sql.toLowerCase()).toContain("is null");
95 });
96
97 it("never references the customer column — that is the caller's job", () => {
98 const ls = leaves(recordedSubscriptionScope("sub_ABC"));
99 expect(ls.some((l) => l.kind === "col" && l.value === "stripe_customer_id")).toBe(false);
100 });
101});
102
103// --- every mutating site must apply the scope ------------------------------
104//
105// The bug was an absent constraint, so asserting the helper alone would not
106// have caught it. Strip ONLY line comments.
107
108describe("stripe-webhook applies the scope everywhere it mutates a plan", () => {
109 const src = (() => {
110 const raw = readFileSync(
111 join(import.meta.dir, "..", "routes", "stripe-webhook.ts"),
112 "utf8"
113 );
114 const stripped = raw
115 .split("\n")
116 .filter((l) => !l.trim().startsWith("//") && !l.trim().startsWith("*"))
117 .join("\n");
118 expect(stripped.length).toBeGreaterThan(0);
119 return stripped;
120 })();
121
122 /** Slice by anchor, never by character count — handlers move. */
123 function slice(startAnchor: string, endAnchor: string): string {
124 const start = src.indexOf(startAnchor);
125 expect(start).toBeGreaterThan(-1);
126 const end = src.indexOf(endAnchor, start + startAnchor.length);
127 expect(end).toBeGreaterThan(start);
128 const body = src.slice(start, end);
129 expect(body.length).toBeGreaterThan(0);
130 return body;
131 }
132
133 it("subscription.deleted scopes the downgrade", () => {
134 const body = slice(
135 `case "customer.subscription.deleted"`,
136 `case "invoice.payment_failed"`
137 );
138 expect(body).toContain('planSlug: "free"');
139 expect(body).toContain("recordedSubscriptionScope");
140 expect(body.indexOf("recordedSubscriptionScope")).toBeGreaterThan(
141 body.indexOf("stripeCustomerId, customerId")
142 );
143 });
144
145 it("invoice.payment_failed scopes the past_due flag", () => {
146 const body = slice(`case "invoice.payment_failed"`, "default:");
147 expect(body).toContain('stripeSubscriptionStatus: "past_due"');
148 expect(body).toContain("recordedSubscriptionScope");
149 });
150
151 it("reconcileSubscription scopes its non-active write", () => {
152 const body = slice("async function reconcileSubscription", "if (!slug)");
153 expect(body).toContain("stripeSubscriptionId: sub.id");
154 expect(body).toContain("recordedSubscriptionScope");
155 });
156
157 it("leaves exactly one unscoped update — the active-upgrade path", () => {
158 // Scoping guards against a STALE NEGATIVE event clobbering a live plan.
159 // The upgrade path must stay unscoped: a newly-created subscription is
160 // by definition not yet the one on record, so scoping it would reject
161 // the upgrade and strand a paying customer on their old plan. A
162 // superseded subscription is never active, so stale events cannot reach
163 // it. This pins that exemption to one known site.
164 const unscoped =
165 src.match(/\.where\(\s*eq\(userQuotas\.stripeCustomerId/g) ?? [];
166 expect(unscoped.length).toBe(1);
167
168 const at = src.search(/\.where\(\s*eq\(userQuotas\.stripeCustomerId/);
169 const block = src.slice(Math.max(0, at - 400), at);
170 expect(block.length).toBeGreaterThan(0);
171 // It is the active upgrade — sets a resolved paid slug, not a downgrade.
172 expect(block).toContain("planSlug: slug");
173 expect(block).not.toContain('planSlug: "free"');
174 expect(block).not.toContain('"past_due"');
175 });
176});
Modifiedsrc/routes/stripe-webhook.ts+92−7View fileUnifiedSplit
1818 */
1919
2020import { Hono } from "hono";
21import { eq, sql } from "drizzle-orm";
21import { eq, and, or, isNull, sql } from "drizzle-orm";
2222import { db } from "../db";
2323import { userQuotas } from "../db/schema";
2424import { reportError } from "../lib/observability";
168168 }
169169 case "customer.subscription.deleted": {
170170 const customerId = String(obj.customer ?? "");
171 const subId = String(obj.id ?? "");
171172 if (!customerId) return;
172 await db
173
174 // Scope the downgrade to the subscription we actually have on record.
175 // Matching on customer id alone downgraded PAYING customers: one
176 // Stripe customer can hold several subscriptions, and a plan change
177 // creates the new one and deletes the old. Webhook delivery order is
178 // not guaranteed, so `created`(sub_NEW) frequently lands before
179 // `deleted`(sub_OLD) — and the old subscription's deletion then wiped
180 // the row that had just been upgraded, silently dropping the customer
181 // to free while they were being billed.
182 //
183 // A row whose subscription id is NULL is still matched: that is a row
184 // checkout populated but reconcile never enriched, and the deletion is
185 // the only subscription it could refer to.
186 const affected = await db
173187 .update(userQuotas)
174188 .set({
175189 planSlug: "free",
178192 currentPeriodEnd: null,
179193 updatedAt: new Date(),
180194 })
181 .where(eq(userQuotas.stripeCustomerId, customerId));
182 console.log(`[stripe-webhook] downgraded customer=${customerId} to free`);
195 .where(
196 and(
197 eq(userQuotas.stripeCustomerId, customerId),
198 recordedSubscriptionScope(subId)
199 )
200 )
201 .returning({ userId: userQuotas.userId });
202
203 if (affected.length === 0) {
204 // Expected whenever a superseded subscription is cleaned up. Logged
205 // rather than silent so a genuine mismatch stays diagnosable.
206 console.log(
207 `[stripe-webhook] subscription.deleted sub=${subId} customer=${customerId} did not match the subscription on record — no downgrade`
208 );
209 } else {
210 console.log(
211 `[stripe-webhook] downgraded customer=${customerId} to free`
212 );
213 }
183214 return;
184215 }
185216 case "invoice.payment_failed": {
188219 // Mark status — don't downgrade immediately (Stripe retries billing
189220 // over a grace window; a future subscription.updated with status=
190221 // past_due/unpaid/canceled will do the actual plan move).
222 // Scoped like the two above: an invoice names its subscription, and a
223 // failed invoice for a superseded one must not flag the customer's
224 // current plan as past_due. A one-off invoice carries no subscription,
225 // in which case the customer match is all there is to go on.
226 const invoiceSubId = obj.subscription ? String(obj.subscription) : "";
191227 await db
192228 .update(userQuotas)
193229 .set({
194230 stripeSubscriptionStatus: "past_due",
195231 updatedAt: new Date(),
196232 })
197 .where(eq(userQuotas.stripeCustomerId, customerId));
233 .where(
234 and(
235 eq(userQuotas.stripeCustomerId, customerId),
236 recordedSubscriptionScope(invoiceSubId)
237 )
238 );
198239 return;
199240 }
200241 default:
203244 }
204245}
205246
247/**
248 * Restrict an update to the subscription this user actually has on record.
249 *
250 * One Stripe customer can hold several subscriptions, so `customer` alone
251 * does not identify which one an event is about. A plan change creates the
252 * new subscription and deletes the old, and webhook delivery order is not
253 * guaranteed — so `created`(sub_NEW) routinely arrives before
254 * `deleted`(sub_OLD). Keying only on the customer meant the old
255 * subscription's event landed on the row that had just been upgraded.
256 *
257 * A NULL subscription id on the row still matches: that is a row checkout
258 * populated but reconcile never enriched, so the event is the only
259 * subscription it could be about.
260 *
261 * Returns undefined when the event names no subscription (a one-off
262 * invoice), leaving the caller's customer match as the only constraint.
263 */
264export function recordedSubscriptionScope(eventSubscriptionId: string) {
265 if (!eventSubscriptionId) return undefined;
266 return or(
267 eq(userQuotas.stripeSubscriptionId, eventSubscriptionId),
268 isNull(userQuotas.stripeSubscriptionId)
269 );
270}
271
206272async function reconcileSubscription(
207273 subscriptionId: string,
208274 fallbackUserId: string | null
231297 // Non-active subscriptions don't grant a paid plan. We still record
232298 // their state so the user can see "past_due" in /settings/billing.
233299 if (customerId) {
300 // Same scoping rule as subscription.deleted. Without it, a `canceled`
301 // or `past_due` event for a SUPERSEDED subscription overwrote the row
302 // belonging to the customer's current, active one — stamping the dead
303 // subscription's id and status over a live paid plan.
234304 await db
235305 .update(userQuotas)
236306 .set({
239309 currentPeriodEnd: periodEnd,
240310 updatedAt: new Date(),
241311 })
242 .where(eq(userQuotas.stripeCustomerId, customerId));
312 .where(
313 and(
314 eq(userQuotas.stripeCustomerId, customerId),
315 recordedSubscriptionScope(sub.id)
316 )
317 );
243318 }
244319 return;
245320 }
265340 return;
266341 }
267342
268 // Last resort: locate by customer id and update in place
343 // Last resort: locate by customer id and update in place.
344 //
345 // Deliberately NOT scoped by recordedSubscriptionScope, unlike the three
346 // sites that write a downgrade or a failure status. This branch only runs
347 // for an active or trialing subscription with a resolved paid plan, and a
348 // newly-created subscription is by definition not yet the one on record —
349 // scoping here would reject the upgrade and leave a paying customer on
350 // their old plan, which is the very failure this change exists to stop.
351 // Stale events cannot reach this branch: a superseded subscription is not
352 // active. Positive events take the row; only negative ones must prove
353 // they concern the subscription it already holds.
269354 if (customerId) {
270355 await db
271356 .update(userQuotas)
272357