From abd27f2d5f501325c978a8702340e9fdd84fb384 Mon Sep 17 00:00:00 2001 From: Wanjohi Date: Sat, 19 Sep 2026 01:39:56 +0300 Subject: [PATCH 1/2] fix(billing): a customer may already exist, and may belong to somebody else MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registering a team assumed creating a subscription would create the customer it names. It does not — the customer has to exist first, and creating one fails if the address is already taken, which it often is: a checkout taken before the team existed leaves one behind, and so does making one by hand. So the lookup is now in three steps. A customer already carrying this team id is used. Otherwise one is found by address and adopted. Otherwise one is made. **A customer carrying a different team's id is left alone.** Taking it would move where that subscription is billed, and the team that lost it would go quiet rather than fail — which is the kind of thing found a month later in a revenue figure that does not add up. A team whose owner has no address yet gets no customer, reported rather than guessed around: an invented address makes a customer nobody can be reached at, and the next call fixes it once there is a real one. Checked against the provider rather than only in tests: all four paths — new, repeated, taken by another team, and no address — behave as written. --- packages/core/src/billing/polar.ts | 93 +++++++++++++++++++++++------- packages/core/src/team/index.ts | 8 ++- 2 files changed, 78 insertions(+), 23 deletions(-) diff --git a/packages/core/src/billing/polar.ts b/packages/core/src/billing/polar.ts index 5e517115..8c2e09ba 100644 --- a/packages/core/src/billing/polar.ts +++ b/packages/core/src/billing/polar.ts @@ -87,30 +87,79 @@ export namespace Polar { * fixes it. That is also why it is safe to call on a team that already has * one. */ - export const ensureFree = fn(z.object({ teamId: z.string() }), async (input) => { - const { freeProductId } = settings(); - if (!freeProductId) { - return { created: false, reason: 'no free product configured' as const }; - } - - try { - const existing = await client().customers.getStateExternal({ - externalId: input.teamId - }); - if (existing.activeSubscriptions.length > 0) { - return { created: false, reason: 'already subscribed' as const }; + export const ensureFree = fn( + z.object({ teamId: z.string(), email: z.email().optional() }), + async (input) => { + const { freeProductId } = settings(); + if (!freeProductId) { + return { created: false, reason: 'no free product configured' as const }; } - } catch { - // No such customer yet, which is the ordinary case the first time. - // Creating the subscription below makes one. - } - await client().subscriptions.create({ - productId: freeProductId, - externalCustomerId: input.teamId - }); - return { created: true, reason: 'created' as const }; - }); + // Already ours, and already subscribed to something. + try { + const state = await client().customers.getStateExternal({ + externalId: input.teamId + }); + if (state.activeSubscriptions.length > 0) { + return { created: false, reason: 'already subscribed' as const }; + } + await client().subscriptions.create({ + productId: freeProductId, + externalCustomerId: input.teamId + }); + return { created: true, reason: 'subscribed an existing customer' as const }; + } catch { + // No customer carries this team id yet, which is the ordinary + // case the first time. Fall through and find or make one. + } + + // A customer may already exist under this address without being + // linked to anything of ours — made by hand, or left behind by a + // checkout taken before the team existed. Their addresses are unique, + // so creating a second one is refused rather than allowed, and the + // only way forward is to adopt the one that is there. + let customerId: string | null = null; + if (input.email) { + const found = await client().customers.list({ email: input.email, limit: 2 }); + const existing = found.result.items.at(0); + if (existing) { + // **Never take one that belongs to another team.** Moving an + // external id would move where a subscription is billed, and + // the team losing it would go quiet rather than error. + if (existing.externalId && existing.externalId !== input.teamId) { + return { created: false, reason: 'address belongs to another team' as const }; + } + if (!existing.externalId) { + await client().customers.update({ + id: existing.id, + customerUpdate: { externalId: input.teamId } + }); + } + customerId = existing.id; + } + } + + if (!customerId) { + if (!input.email) { + // Without an address there is nothing to look up and nothing + // to create with, and guessing one would make a customer + // nobody can be reached at. + return { created: false, reason: 'no email to create a customer with' as const }; + } + const made = await client().customers.create({ + email: input.email, + externalId: input.teamId + }); + customerId = made.id; + } + + await client().subscriptions.create({ + productId: freeProductId, + externalCustomerId: input.teamId + }); + return { created: true, reason: 'created' as const }; + } + ); /** * A checkout for a team, as the customer they already are. diff --git a/packages/core/src/team/index.ts b/packages/core/src/team/index.ts index a85d82ca..4c4cc381 100644 --- a/packages/core/src/team/index.ts +++ b/packages/core/src/team/index.ts @@ -7,6 +7,7 @@ import { Database } from '../db/index.js'; import { Examples } from '../examples.js'; import { fn } from '../fn.js'; import { Identifier } from '../id.js'; +import { User } from '../user/index.js'; import { TeamMemberTable } from './member.sql.js'; import { TeamTable } from './team.sql.js'; @@ -91,7 +92,12 @@ export namespace Team { // idempotent. Database.effect(async () => { try { - await Polar.ensureFree({ teamId: input.id }); + // The owner's address, so a customer can be found or made. A team + // created by somebody with no verified address gets no customer + // yet, which is a state `ensureFree` reports rather than guesses + // its way out of. + const owner = await User.fromID(ownerId); + await Polar.ensureFree({ teamId: input.id, email: owner?.email ?? undefined }); } catch (error) { // eslint-disable-next-line no-console console.error('could not register team with the payment provider:', error); From a09ad1e09af9e87d7c7f286a457a0c061ee88d17 Mon Sep 17 00:00:00 2001 From: Wanjohi Date: Sat, 19 Sep 2026 01:42:42 +0300 Subject: [PATCH 2/2] fix(billing): read the payload that was actually sent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deliveries arrived intact, verified correctly, and applied to nobody. Checking the signature here rather than through the SDK means the body is exactly what was sent, and what is sent is snake_case. The SDK's parser renames fields to camelCase on the way through, so the reader written against it looked for `externalId` in a payload that says `external_id` — found nothing, decided the delivery was about a customer we did not create, and acknowledged it. That is the worst shape a bug of this kind can take. Every visible signal was healthy: a 200 back to the provider, no retries, no errors, and a plan that silently never changed. Both spellings are now read, so neither path can regress the other. --- packages/core/src/billing/polar.test.ts | 30 +++++++++++++++++++++++++ packages/core/src/billing/polar.ts | 19 +++++++++++----- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/packages/core/src/billing/polar.test.ts b/packages/core/src/billing/polar.test.ts index 989460fa..0e65e27b 100644 --- a/packages/core/src/billing/polar.test.ts +++ b/packages/core/src/billing/polar.test.ts @@ -172,6 +172,36 @@ describe('Both signing schemes', () => { expect(delivery.standing).toEqual({ plan: 'paid', status: 'active' }); }); + test('a raw snake_case payload is read, not just the SDK\u2019s camelCase', async () => { + // Verifying the signature ourselves hands back exactly what was sent, + // which is snake_case; the SDK's parser renames fields on the way + // through. Reading one spelling made every delivery verify correctly and + // then apply to nobody, which looks identical to working. + const { Webhook } = await import('standardwebhooks'); + const secret = `whsec_${Buffer.from('b'.repeat(32)).toString('base64')}`; + configure({ POLAR_WEBHOOK_SECRET: secret }); + + const body = JSON.stringify({ + type: 'subscription.revoked', + data: { customer: { external_id: 'tem_snake' }, product_id: PAID } + }); + const id = 'msg_snake'; + const timestamp = new Date(); + const signature = new Webhook(secret).sign(id, timestamp, body); + + const delivery = Polar.receive({ + body, + headers: { + 'webhook-id': id, + 'webhook-timestamp': Math.floor(timestamp.getTime() / 1000).toString(), + 'webhook-signature': signature + } + }); + + expect(delivery.teamId).toBe('tem_snake'); + expect(delivery.standing).toEqual({ plan: 'free', status: 'revoked' }); + }); + test('a tampered body under a valid-looking signature is refused', () => { const secret = `whsec_${Buffer.from('a'.repeat(32)).toString('base64')}`; configure({ POLAR_WEBHOOK_SECRET: secret }); diff --git a/packages/core/src/billing/polar.ts b/packages/core/src/billing/polar.ts index 8c2e09ba..9cb06b6c 100644 --- a/packages/core/src/billing/polar.ts +++ b/packages/core/src/billing/polar.ts @@ -341,18 +341,27 @@ export namespace Polar { throw error; } + // Both spellings, for both paths. The SDK's parser renames fields to + // camelCase on the way through; verifying the signature ourselves + // hands back exactly what was sent, which is snake_case. Reading only + // one spelling makes every delivery arrive intact, verify correctly, + // and then quietly apply to nobody. const data = event.data ?? {}; - const customer = data.customer as { externalId?: string | null } | undefined; + const customer = data.customer as + | { externalId?: string | null; external_id?: string | null } + | undefined; // `externalId` is the team id we put on the customer. A delivery // without one is about a customer created some other way — by hand in // their dashboard, most likely — and there is nothing here it can // change. - const teamId = customer?.externalId ?? null; + const teamId = customer?.externalId ?? customer?.external_id ?? null; - // Both spellings, because which one a payload carries depends on - // whether the product was expanded into it. const product = data.product as { id?: string } | undefined; - const productId = (data.productId as string | undefined) ?? product?.id ?? null; + const productId = + (data.productId as string | undefined) ?? + (data.product_id as string | undefined) ?? + product?.id ?? + null; return { type: event.type, teamId, standing: standingFor(event.type, productId) }; }