feat(paykit): harden subscription checkout sessions#202
Conversation
|
@Pascoooo is attempting to deploy a commit to the maxktz Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds subscription quantity support throughout the subscribe flow and Stripe provider, introduces checkout "hardening" options (promotion codes, automatic tax, billing address, tax ID collection, idempotency), returns ChangesSubscription Quantity and Checkout Session Hardening
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/paykit/src/subscription/subscription.api.ts (1)
37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRoute
expireCheckoutSessionthrough the provider-customer mapping layer
ctx.customer.stripeCustomerIdhard-codes Stripe in a generic API path. If this method is meant to stay provider-agnostic, centralize the provider customer lookup first; otherwise the API surface is Stripe-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/subscription/subscription.api.ts` around lines 37 - 40, The expireCheckoutSession path is hard-coding Stripe via ctx.customer.stripeCustomerId in a generic API, so route the lookup through the provider-customer mapping layer instead. Update the subscription API flow in expireCheckoutSession to resolve the provider customer first using the existing customer/provider mapping helpers or customer model methods, then use that mapped identifier for the rest of the logic. Keep the NOT_FOUND behavior in place, but ensure the symbols around ctx.customer and the provider-customer resolution are used so the API stays provider-agnostic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/paykit/src/stripe/stripe-provider.ts`:
- Around line 735-739: The checkout-session ownership check in stripe-provider
should treat session.customer as authoritative instead of allowing
client_reference_id to pass on its own. Update the isCustomerSession logic in
the relevant checkout-session validation path so it first checks
sessionCustomerId against data.providerCustomerId, and only uses
session.client_reference_id when session.customer is absent; keep the change
localized around getStripeCustomerId(session.customer) and the isCustomerSession
guard.
In `@packages/paykit/src/subscription/subscription.service.ts`:
- Around line 801-817: The quantity-change transaction in
subscription.service.ts only syncs subscription data and billing state, so
prorated invoices from handleQuantityChange are not saved. Update the
transaction in the same flow that calls
deleteScheduledSubscriptionsInGroupIfNeeded, syncSubscriptionFromProvider, and
syncSubscriptionBillingState to also persist providerResult.invoice by invoking
upsertInvoiceRecord when an invoice is present. Keep the invoice write inside
the existing ctx.database.transaction so billing history stays consistent with
the subscription update.
In `@packages/paykit/src/subscription/subscription.types.ts`:
- Around line 33-40: The subscribe request schema currently lets client callers
override checkout policy through the checkout field, which is then passed into
createSubscriptionCheckout. Update subscribeBodySchema and the /subscribe client
flow so merchant-controlled settings like automaticTax,
billingAddressCollection, taxIdCollection, and idempotencyKey are not accepted
from clients; keep those values server-side and only allow safe client inputs in
the subscription types and checkout creation path.
---
Nitpick comments:
In `@packages/paykit/src/subscription/subscription.api.ts`:
- Around line 37-40: The expireCheckoutSession path is hard-coding Stripe via
ctx.customer.stripeCustomerId in a generic API, so route the lookup through the
provider-customer mapping layer instead. Update the subscription API flow in
expireCheckoutSession to resolve the provider customer first using the existing
customer/provider mapping helpers or customer model methods, then use that
mapped identifier for the rest of the logic. Keep the NOT_FOUND behavior in
place, but ensure the symbols around ctx.customer and the provider-customer
resolution are used so the API stays provider-agnostic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a07646d3-85d7-4e37-b449-ab96346d1036
📒 Files selected for processing (16)
.changeset/checkout-session-hardening.md.changeset/subscription-quantity.mdpackages/paykit/src/api/methods.tspackages/paykit/src/core/create-paykit.tspackages/paykit/src/core/errors.tspackages/paykit/src/index.tspackages/paykit/src/providers/provider.tspackages/paykit/src/stripe/__tests__/stripe-provider.test.tspackages/paykit/src/stripe/stripe-provider.tspackages/paykit/src/subscription/__tests__/subscription.service.test.tspackages/paykit/src/subscription/__tests__/subscription.types.test.tspackages/paykit/src/subscription/subscription.api.tspackages/paykit/src/subscription/subscription.service.tspackages/paykit/src/subscription/subscription.types.tspackages/paykit/src/types/events.tspackages/paykit/src/types/instance.ts
| const sessionCustomerId = getStripeCustomerId(session.customer); | ||
| const isCustomerSession = | ||
| sessionCustomerId === data.providerCustomerId || | ||
| session.client_reference_id === data.providerCustomerId; | ||
| if (!isCustomerSession) { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/paykit/src/stripe/stripe-provider.ts"
echo "== Outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== Relevant line numbers =="
nl -ba "$FILE" | sed -n '700,770p'
echo
echo "== Search expireCheckoutSession usages =="
rg -n "expireCheckoutSession|isCustomerSession|client_reference_id|session\.customer|getStripeCustomerId" packages/paykit -SRepository: getpaykit/paykit
Length of output: 2771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/paykit/src/stripe/stripe-provider.ts"
echo "== Numbered excerpt around ownership check =="
python3 - <<'PY'
from pathlib import Path
path = Path("packages/paykit/src/stripe/stripe-provider.ts")
lines = path.read_text().splitlines()
for i in range(700, 771):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
PY
echo
echo "== Search relevant usages/tests =="
rg -n "expireCheckoutSession|isCustomerSession|client_reference_id|session\.customer|getStripeCustomerId|checkout session" packages/paykit -SRepository: getpaykit/paykit
Length of output: 8038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/paykit/src/stripe/stripe-provider.ts"
echo "== Ownership-related helpers around checkout/webhook logic =="
python3 - <<'PY'
from pathlib import Path
path = Path("packages/paykit/src/stripe/stripe-provider.ts")
lines = path.read_text().splitlines()
for start, end in [(250, 390), (650, 705)]:
print(f"\n--- {start}-{end} ---")
for i in range(start, end + 1):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
PY
echo
echo "== Stripe provider test excerpt =="
python3 - <<'PY'
from pathlib import Path
path = Path("packages/paykit/src/stripe/__tests__/stripe-provider.test.ts")
lines = path.read_text().splitlines()
for start, end in [(1, 120), (190, 320)]:
print(f"\n--- {start}-{end} ---")
for i in range(start, end + 1):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: getpaykit/paykit
Length of output: 16827
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Subscription API =="
python3 - <<'PY'
from pathlib import Path
path = Path("packages/paykit/src/subscription/subscription.api.ts")
lines = path.read_text().splitlines()
for i, line in enumerate(lines, 1):
print(f"{i:4d}: {line}")
PY
echo
echo "== Provider interface =="
python3 - <<'PY'
from pathlib import Path
path = Path("packages/paykit/src/providers/provider.ts")
lines = path.read_text().splitlines()
for i, line in enumerate(lines, 1):
if 120 <= i <= 180:
print(f"{i:4d}: {line}")
PY
echo
echo "== Search for other checkout session create/expire flows =="
rg -n "checkout\.sessions\.create|expireCheckoutSession\(" packages/paykit/src -SRepository: getpaykit/paykit
Length of output: 5906
🌐 Web query:
Stripe Checkout Session client_reference_id versus customer ownership semantics
💡 Result:
In Stripe Checkout, client_reference_id and the customer field serve distinct, non-overlapping purposes regarding ownership and reconciliation [1][2]. client_reference_id The client_reference_id is an optional, free-form string (up to 200 characters) designed for your internal system's reconciliation [1][3]. It acts as a bridge between Stripe and your database, allowing you to associate a Checkout Session with a specific entity—such as an internal user ID, cart ID, or order ID—that exists outside of Stripe [1][3]. It is not managed or interpreted by Stripe; it is simply passed through the session and included in webhook events like checkout.session.completed, making it ideal for tracking payments back to your own records [2][3]. customer field The customer field defines the actual ownership of the payment within the Stripe ecosystem [2]. When you pass an existing Stripe Customer ID to this field, you are instructing Stripe to attribute the resulting PaymentIntent or Subscription to that specific Customer object [2][4]. If you do not provide a customer ID, Stripe may create a new Customer object automatically depending on your configuration (such as in subscription mode or when customer_creation is enabled) [2]. Key Differences Reconciliation vs. Ownership: Use client_reference_id to link a session to your internal business logic (e.g., matching a payment to a row in your SQL database) [1][5]. Use the customer field to manage the lifecycle, saved payment methods, and historical data of a customer within Stripe [2][4]. Data Flow: client_reference_id is a custom tag you apply [1]. The customer field dictates how Stripe structures and stores the payment record [2]. Best Practice for Linking: If you need to ensure a payment is tied to a user who already exists in your system, you should typically create or retrieve that user's Stripe Customer ID first and pass that ID into the customer field [5][6]. You can additionally use client_reference_id (or the metadata field) to store your own internal primary key, which ensures robust cross-referencing in your webhooks [5][6].
Citations:
- 1: https://docs.stripe.com/api/checkout/sessions/create
- 2: https://docs.stripe.com/api/checkout/sessions/object
- 3: https://docs.stripe.com/payment-links/url-parameters
- 4: https://docs.stripe.com/payments/existing-customers
- 5: https://stackoverflow.com/questions/79421507/how-to-retrieve-stripe-customer-id-at-checkout-session-creation
- 6: https://www.tomaszezula.com/stripe-payment-links-and-custom-parameters/
Make customer authoritative for checkout-session ownership checks.
If session.customer is present, only fall back to client_reference_id when it’s absent; client_reference_id is reconciliation metadata, not ownership, so the current || can accept a mismatched session.
🛡️ Proposed fix
const sessionCustomerId = getStripeCustomerId(session.customer);
- const isCustomerSession =
- sessionCustomerId === data.providerCustomerId ||
- session.client_reference_id === data.providerCustomerId;
+ const isCustomerSession = sessionCustomerId
+ ? sessionCustomerId === data.providerCustomerId
+ : session.client_reference_id === data.providerCustomerId;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sessionCustomerId = getStripeCustomerId(session.customer); | |
| const isCustomerSession = | |
| sessionCustomerId === data.providerCustomerId || | |
| session.client_reference_id === data.providerCustomerId; | |
| if (!isCustomerSession) { | |
| const sessionCustomerId = getStripeCustomerId(session.customer); | |
| const isCustomerSession = sessionCustomerId | |
| ? sessionCustomerId === data.providerCustomerId | |
| : session.client_reference_id === data.providerCustomerId; | |
| if (!isCustomerSession) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/stripe/stripe-provider.ts` around lines 735 - 739, The
checkout-session ownership check in stripe-provider should treat
session.customer as authoritative instead of allowing client_reference_id to
pass on its own. Update the isCustomerSession logic in the relevant
checkout-session validation path so it first checks sessionCustomerId against
data.providerCustomerId, and only uses session.client_reference_id when
session.customer is absent; keep the change localized around
getStripeCustomerId(session.customer) and the isCustomerSession guard.
| await ctx.database.transaction(async (tx) => { | ||
| await deleteScheduledSubscriptionsInGroupIfNeeded(tx, subCtx); | ||
| await syncSubscriptionFromProvider(tx, { | ||
| providerSubscription: providerResult.subscription!, | ||
| subscriptionId: activeSubscription.id, | ||
| }); | ||
| await syncSubscriptionBillingState(tx, { | ||
| currentPeriodEndAt: providerResult.subscription!.currentPeriodEndAt, | ||
| currentPeriodStartAt: providerResult.subscription!.currentPeriodStartAt, | ||
| quantity: providerResult.subscription!.quantity ?? subCtx.quantity, | ||
| stripeSubscriptionId: providerResult.subscription!.providerSubscriptionId, | ||
| stripeSubscriptionScheduleId: | ||
| providerResult.subscription!.providerSubscriptionScheduleId ?? null, | ||
| status: providerResult.subscription!.status, | ||
| subscriptionId: activeSubscription.id, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist invoices from quantity updates.
handleQuantityChange returns providerResult.invoice, but the transaction only syncs subscription state. Quantity updates can generate prorated invoices, so billing history misses them unless this path also calls upsertInvoiceRecord.
💾 Proposed fix
await syncSubscriptionBillingState(tx, {
currentPeriodEndAt: providerResult.subscription!.currentPeriodEndAt,
currentPeriodStartAt: providerResult.subscription!.currentPeriodStartAt,
quantity: providerResult.subscription!.quantity ?? subCtx.quantity,
stripeSubscriptionId: providerResult.subscription!.providerSubscriptionId,
stripeSubscriptionScheduleId:
providerResult.subscription!.providerSubscriptionScheduleId ?? null,
status: providerResult.subscription!.status,
subscriptionId: activeSubscription.id,
});
+ if (providerResult.invoice) {
+ await upsertInvoiceRecord(tx, {
+ customerId: subCtx.customerId,
+ invoice: providerResult.invoice,
+ providerId: subCtx.providerId,
+ subscriptionId: activeSubscription.id,
+ });
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await ctx.database.transaction(async (tx) => { | |
| await deleteScheduledSubscriptionsInGroupIfNeeded(tx, subCtx); | |
| await syncSubscriptionFromProvider(tx, { | |
| providerSubscription: providerResult.subscription!, | |
| subscriptionId: activeSubscription.id, | |
| }); | |
| await syncSubscriptionBillingState(tx, { | |
| currentPeriodEndAt: providerResult.subscription!.currentPeriodEndAt, | |
| currentPeriodStartAt: providerResult.subscription!.currentPeriodStartAt, | |
| quantity: providerResult.subscription!.quantity ?? subCtx.quantity, | |
| stripeSubscriptionId: providerResult.subscription!.providerSubscriptionId, | |
| stripeSubscriptionScheduleId: | |
| providerResult.subscription!.providerSubscriptionScheduleId ?? null, | |
| status: providerResult.subscription!.status, | |
| subscriptionId: activeSubscription.id, | |
| }); | |
| }); | |
| await ctx.database.transaction(async (tx) => { | |
| await deleteScheduledSubscriptionsInGroupIfNeeded(tx, subCtx); | |
| await syncSubscriptionFromProvider(tx, { | |
| providerSubscription: providerResult.subscription!, | |
| subscriptionId: activeSubscription.id, | |
| }); | |
| await syncSubscriptionBillingState(tx, { | |
| currentPeriodEndAt: providerResult.subscription!.currentPeriodEndAt, | |
| currentPeriodStartAt: providerResult.subscription!.currentPeriodStartAt, | |
| quantity: providerResult.subscription!.quantity ?? subCtx.quantity, | |
| stripeSubscriptionId: providerResult.subscription!.providerSubscriptionId, | |
| stripeSubscriptionScheduleId: | |
| providerResult.subscription!.providerSubscriptionScheduleId ?? null, | |
| status: providerResult.subscription!.status, | |
| subscriptionId: activeSubscription.id, | |
| }); | |
| if (providerResult.invoice) { | |
| await upsertInvoiceRecord(tx, { | |
| customerId: subCtx.customerId, | |
| invoice: providerResult.invoice, | |
| providerId: subCtx.providerId, | |
| subscriptionId: activeSubscription.id, | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/subscription/subscription.service.ts` around lines 801 -
817, The quantity-change transaction in subscription.service.ts only syncs
subscription data and billing state, so prorated invoices from
handleQuantityChange are not saved. Update the transaction in the same flow that
calls deleteScheduledSubscriptionsInGroupIfNeeded, syncSubscriptionFromProvider,
and syncSubscriptionBillingState to also persist providerResult.invoice by
invoking upsertInvoiceRecord when an invoice is present. Keep the invoice write
inside the existing ctx.database.transaction so billing history stays consistent
with the subscription update.
| export const subscribeBodySchema = z.object({ | ||
| planId: z.string(), | ||
| checkout: subscriptionCheckoutOptionsSchema.optional(), | ||
| forceCheckout: z.boolean().optional(), | ||
| quantity: z.number().int().positive().optional(), | ||
| successUrl: returnUrl(), | ||
| cancelUrl: returnUrl().optional(), | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the subscribe route exposes checkout to clients and forwards it unfiltered.
fd subscription.api.ts -t f packages/paykit/src --exec sed -n '1,60p' {}
rg -nP -C3 'route:\s*\{|client:\s*true|ctx\.input\.checkout' packages/paykit/src/subscription/subscription.api.tsRepository: getpaykit/paykit
Length of output: 2399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the subscription types and service flow around checkout handling.
fd subscription.types.ts packages/paykit/src -t f -x sh -c 'echo "==== {} ===="; sed -n "1,220p" "{}"'
fd subscription.service.ts packages/paykit/src -t f -x sh -c 'echo "==== {} ===="; sed -n "1,260p" "{}"'
# Find the checkout schema fields and where they are consumed.
rg -n -C3 'subscriptionCheckoutOptionsSchema|automaticTax|billingAddressCollection|taxIdCollection|idempotencyKey|createSubscriptionCheckout|subscribeToPlan' packages/paykit/srcRepository: getpaykit/paykit
Length of output: 35358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for public docs or examples that explain who controls subscription checkout options.
rg -n -C2 'allowPromotionCodes|billingAddressCollection|automaticTax|taxIdCollection|idempotencyKey|checkout options|subscribeBodySchema|/subscribe' README.md packages/paykit/src docs . 2>/dev/null || trueRepository: getpaykit/paykit
Length of output: 33244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the subscription billing docs around the server/client examples.
sed -n '90,180p' apps/web/content/docs/subscription-billing.mdxRepository: getpaykit/paykit
Length of output: 3214
Client callers can override checkout policy here packages/paykit/src/subscription/subscription.types.ts:33-40 — checkout is accepted on the client: true /subscribe route and passed straight through to createSubscriptionCheckout, so callers can weaken automaticTax, billingAddressCollection, taxIdCollection, or supply their own idempotencyKey. If these settings are meant to be merchant policy, keep them server-side and expose only safe client inputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/subscription/subscription.types.ts` around lines 33 - 40,
The subscribe request schema currently lets client callers override checkout
policy through the checkout field, which is then passed into
createSubscriptionCheckout. Update subscribeBodySchema and the /subscribe client
flow so merchant-controlled settings like automaticTax,
billingAddressCollection, taxIdCollection, and idempotencyKey are not accepted
from clients; keep those values server-side and only allow safe client inputs in
the subscription types and checkout creation path.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/paykit/src/core/errors.ts">
<violation number="1" location="packages/paykit/src/core/errors.ts:26">
P2: New public error code uses non-standard spelling `NOT_EXPIREABLE` instead of `NOT_EXPIRABLE`. Since `PayKitErrorCode` is a public API surface and this constant is newly introduced, it should be corrected before release to avoid locking in a misspelled public constant.</violation>
</file>
<file name="packages/paykit/src/subscription/__tests__/subscription.service.test.ts">
<violation number="1" location="packages/paykit/src/subscription/__tests__/subscription.service.test.ts:281">
P3: Missing assertion for the update `where` clause leaves subscription scoping unverified.</violation>
</file>
<file name="packages/paykit/src/subscription/subscription.types.ts">
<violation number="1" location="packages/paykit/src/subscription/subscription.types.ts:37">
P2: Subscription quantity accepts unbounded positive integers, risking silent precision loss for values above JavaScript's safe integer range before Zod validation even runs, which could propagate incorrect billing quantities to Stripe.</violation>
</file>
<file name="packages/paykit/src/types/events.ts">
<violation number="1" location="packages/paykit/src/types/events.ts:30">
P2: The customer.updated event subscription projection is missing the newly modeled `quantity` field, making quantity-only changes invisible to event consumers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| PROVIDER_CUSTOMER_NOT_FOUND: "Customer not found in provider", | ||
| PROVIDER_CHECKOUT_SESSION_CUSTOMER_MISMATCH: | ||
| "Provider checkout session does not belong to customer", | ||
| PROVIDER_CHECKOUT_SESSION_NOT_EXPIREABLE: "Provider checkout session cannot be expired", |
There was a problem hiding this comment.
P2: New public error code uses non-standard spelling NOT_EXPIREABLE instead of NOT_EXPIRABLE. Since PayKitErrorCode is a public API surface and this constant is newly introduced, it should be corrected before release to avoid locking in a misspelled public constant.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/core/errors.ts, line 26:
<comment>New public error code uses non-standard spelling `NOT_EXPIREABLE` instead of `NOT_EXPIRABLE`. Since `PayKitErrorCode` is a public API surface and this constant is newly introduced, it should be corrected before release to avoid locking in a misspelled public constant.</comment>
<file context>
@@ -21,6 +21,10 @@ export const PAYKIT_ERROR_CODES = defineErrorCodes({
PROVIDER_CUSTOMER_NOT_FOUND: "Customer not found in provider",
+ PROVIDER_CHECKOUT_SESSION_CUSTOMER_MISMATCH:
+ "Provider checkout session does not belong to customer",
+ PROVIDER_CHECKOUT_SESSION_NOT_EXPIREABLE: "Provider checkout session cannot be expired",
+ PROVIDER_CHECKOUT_SESSION_NOT_FOUND: "Provider checkout session not found",
PROVIDER_SESSION_INVALID: "Provider session did not include a URL",
</file context>
| planId: z.string(), | ||
| checkout: subscriptionCheckoutOptionsSchema.optional(), | ||
| forceCheckout: z.boolean().optional(), | ||
| quantity: z.number().int().positive().optional(), |
There was a problem hiding this comment.
P2: Subscription quantity accepts unbounded positive integers, risking silent precision loss for values above JavaScript's safe integer range before Zod validation even runs, which could propagate incorrect billing quantities to Stripe.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/subscription/subscription.types.ts, line 37:
<comment>Subscription quantity accepts unbounded positive integers, risking silent precision loss for values above JavaScript's safe integer range before Zod validation even runs, which could propagate incorrect billing quantities to Stripe.</comment>
<file context>
@@ -3,21 +3,65 @@ import * as z from "zod";
planId: z.string(),
+ checkout: subscriptionCheckoutOptionsSchema.optional(),
forceCheckout: z.boolean().optional(),
+ quantity: z.number().int().positive().optional(),
successUrl: returnUrl(),
cancelUrl: returnUrl().optional(),
</file context>
| quantity: z.number().int().positive().optional(), | |
| quantity: z.number().int().positive().max(Number.MAX_SAFE_INTEGER).optional(), |
| providerProduct?: Record<string, string> | null; | ||
| providerSubscriptionId: string; | ||
| providerSubscriptionScheduleId?: string | null; | ||
| quantity?: number; |
There was a problem hiding this comment.
P2: The customer.updated event subscription projection is missing the newly modeled quantity field, making quantity-only changes invisible to event consumers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/types/events.ts, line 30:
<comment>The customer.updated event subscription projection is missing the newly modeled `quantity` field, making quantity-only changes invisible to event consumers.</comment>
<file context>
@@ -27,6 +27,7 @@ export interface NormalizedSubscription {
providerProduct?: Record<string, string> | null;
providerSubscriptionId: string;
providerSubscriptionScheduleId?: string | null;
+ quantity?: number;
status: string;
}
</file context>
| subscriptionId: "sub_local_123", | ||
| }); | ||
|
|
||
| expect(update.set).toHaveBeenCalledWith(expect.objectContaining({ quantity: 3 })); |
There was a problem hiding this comment.
P3: Missing assertion for the update where clause leaves subscription scoping unverified.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/subscription/__tests__/subscription.service.test.ts, line 281:
<comment>Missing assertion for the update `where` clause leaves subscription scoping unverified.</comment>
<file context>
@@ -0,0 +1,311 @@
+ subscriptionId: "sub_local_123",
+ });
+
+ expect(update.set).toHaveBeenCalledWith(expect.objectContaining({ quantity: 3 }));
+ });
+
</file context>
1 participant
Summary
This PR hardens subscription Checkout Sessions without adding app-specific billing behavior.
It is stacked on #201 (
subscription-quantity). After #201 merges, this branch should be rebased so this PR contains only the checkout hardening commit.Changes
checkoutSessionIdfrom subscription checkout results.expireCheckoutSessionPayKit method that verifies provider customer ownership before expiring a Stripe Checkout Session.paykitjsrelease.Out of scope
Verification
pnpm --filter paykitjs typecheckpnpm lintpnpm test:unitpnpm --filter paykitjs buildpnpm exec oxfmt --check <touched files>Note:
pnpm format:checkcurrently fails on this Windows shell because the quoted glob is not expanded foroxfmt, so touched files were checked explicitly instead.Summary by cubic
Hardens subscription Checkout Sessions and adds quantity-aware subscriptions across
paykitjs. Adds a secure server method to expire Stripe sessions, returnscheckoutSessionIdfrom subscribe, and avoids top‑level eval during runtime init.New Features
expireCheckoutSessionthat verifies customer ownership, treats already-expired sessions as idempotent, and expires only open sessions.subscribenow returnscheckoutSessionIdand supportsquantity; quantity is persisted and synced across create/update/schedule flows.Migration
expireCheckoutSession; addquantitytocreateSubscription,updateSubscription,scheduleSubscriptionChange, andcreateSubscriptionCheckout; includequantityin returned subscriptions.subscribeacceptscheckoutandquantity; handlecheckoutSessionIdin the result.Written for commit 0dc710c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes