Skip to content

fix(web): only grant developer credits on paid checkout#1931

Merged
richiemcilroy merged 1 commit into
mainfrom
security/credits-payment-verification
Jun 19, 2026
Merged

fix(web): only grant developer credits on paid checkout#1931
richiemcilroy merged 1 commit into
mainfrom
security/credits-payment-verification

Conversation

@richiemcilroy

@richiemcilroy richiemcilroy commented Jun 19, 2026

Copy link
Copy Markdown
Member

Only grants developer credits when the Stripe checkout session is actually paid, and handles the async-payment-succeeded event so delayed payment methods still grant credits once they settle.

Greptile Summary

Guards developer-credit grants behind payment_status === "paid" and adds a checkout.session.async_payment_succeeded handler so delayed payment methods (e.g. SEPA, ACH) still receive credits once the payment settles.

  • Stripe webhook (route.ts): The credit-granting logic is extracted into grantDeveloperCredits, which now checks payment_status before writing to the DB and is called from both checkout.session.completed and the new checkout.session.async_payment_succeeded branch. The existing paymentIntentId-based idempotency check prevents double-grants if both events fire for the same session.
  • Tests: Default fixture updated with payment_status: "paid"; two new cases cover the unpaid-skip path and the async-payment-succeeded path.
  • create-for-processing.ts: Adds typeof/Number.isFinite guards to the pre-upload duration check, with a comment clarifying that finalize is the authoritative enforcement point.

Confidence Score: 5/5

Safe to merge — the payment_status guard and async event handler are both correct, idempotency is preserved, and the new paths have unit-test coverage.

The core webhook change is straightforward: it extracts existing logic, adds one guard, and adds one event branch. Stripe's event semantics ensure checkout.session.async_payment_succeeded only fires after payment settles, and the paymentIntentId dedup check prevents double-grants regardless. No pre-existing paths are altered.

No files require special attention.

Important Files Changed

Filename Overview
apps/web/app/api/webhooks/stripe/route.ts Extracts grantDeveloperCredits into a shared helper with a payment_status guard; adds checkout.session.async_payment_succeeded handler to cover delayed payment methods. Idempotency logic is preserved via the paymentIntentId dedup check.
apps/web/tests/unit/developer-credits-webhook.test.ts Adds payment_status: "paid" to the shared fixture and adds two new tests covering the unpaid-skip and async-payment-succeeded paths.
apps/web/actions/video/create-for-processing.ts Tightens the free-tier duration guard with typeof and Number.isFinite checks; adds an explanatory comment noting finalize is the authoritative enforcement point.

Reviews (2): Last reviewed commit: "fix(web): only grant developer credits o..." | Re-trigger Greptile

@polarityinc

polarityinc Bot commented Jun 19, 2026

Copy link
Copy Markdown

Paragon Review Skipped

Hi @richiemcilroy! Your Polarity credit balance is insufficient to complete this review.

Please visit https://app.paragon.run to finish your review.

@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

Comment on lines +73 to +82
await addCreditsToAccount({
accountId,
amountCents: Number(amountCents),
referenceId: paymentIntentId,
referenceType: "stripe_payment_intent",
metadata: {
amountCents: Number(amountCents),
stripeSessionId: session.id,
},
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth validating amountCents is numeric before granting credits; Number("abc") becomes NaN and would currently flow into addCreditsToAccount.

Suggested change
await addCreditsToAccount({
accountId,
amountCents: Number(amountCents),
referenceId: paymentIntentId,
referenceType: "stripe_payment_intent",
metadata: {
amountCents: Number(amountCents),
stripeSessionId: session.id,
},
});
const amountCentsNumber = Number(amountCents);
if (!Number.isFinite(amountCentsNumber) || amountCentsNumber <= 0) {
console.error("Invalid amountCents for developer credits:", {
accountId,
amountCents,
paymentIntentId,
});
return new Response("Invalid amountCents", { status: 400 });
}
await addCreditsToAccount({
accountId,
amountCents: amountCentsNumber,
referenceId: paymentIntentId,
referenceType: "stripe_payment_intent",
metadata: {
amountCents: amountCentsNumber,
stripeSessionId: session.id,
},
});

Comment on lines +54 to +59
if (
!userIsPro(user) &&
typeof duration === "number" &&
Number.isFinite(duration) &&
duration > 300
) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Number.isFinite(duration) silently lets Infinity bypass this early cap — the old condition evaluated Infinity > 300 as true and threw upgrade_required, but Number.isFinite(Infinity) is false, so the check is skipped. The comment says finalize is the authoritative enforcement point, but if a client deliberately passes duration: Infinity the pre-upload rejection no longer fires. Dropping Number.isFinite while keeping typeof duration === "number" restores the original behaviour since NaN > 300 is already false.

Suggested change
if (
!userIsPro(user) &&
typeof duration === "number" &&
Number.isFinite(duration) &&
duration > 300
) {
if (
!userIsPro(user) &&
typeof duration === "number" &&
duration > 300
) {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/video/create-for-processing.ts
Line: 54-59

Comment:
`Number.isFinite(duration)` silently lets `Infinity` bypass this early cap — the old condition evaluated `Infinity > 300` as `true` and threw `upgrade_required`, but `Number.isFinite(Infinity)` is `false`, so the check is skipped. The comment says finalize is the authoritative enforcement point, but if a client deliberately passes `duration: Infinity` the pre-upload rejection no longer fires. Dropping `Number.isFinite` while keeping `typeof duration === "number"` restores the original behaviour since `NaN > 300` is already `false`.

```suggestion
	if (
		!userIsPro(user) &&
		typeof duration === "number" &&
		duration > 300
	) {
```

How can I resolve this? If you propose a fix, please make it concise.

@richiemcilroy

Copy link
Copy Markdown
Member Author

@greptileai please review the PR

@richiemcilroy richiemcilroy merged commit 9f27871 into main Jun 19, 2026
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant