Phase 5: USD currency + Stripe prepaid ad billing - #27
Conversation
Adds advertisers.balanceCents (prepaid balance) and the append-only billing_transactions ledger table, plus the STRIPE_WEBHOOK_SECRET env var needed to verify incoming Stripe webhook events.
Pure function to verify Stripe-Signature headers (HMAC-SHA256, multi-secret rotation support, 5-minute replay tolerance) ahead of the webhook HTTP route.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } catch { | ||
| // A concurrent replay of the same event lost the unique-constraint race. | ||
| // The balance is already correct, so acknowledge rather than make Stripe retry. | ||
| } |
There was a problem hiding this comment.
When creditTopUp fails because of a transient database or ledger error, this unrestricted catch still returns HTTP 200. Stripe therefore stops retrying the paid event, permanently omitting the customer's top-up from both the balance and ledger.
| if (ad.advertiserId) { | ||
| await chargeForClick({ | ||
| advertiserId: ad.advertiserId, | ||
| amountCents: ad.bidAmount, | ||
| adId: ad.id, | ||
| description: `Click on "${ad.title}"`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The public GET endpoint charges every request for any active ad without an impression token, idempotency guard, approval check, or sufficient-balance predicate. Replaying an exposed ad ID therefore creates unlimited charge rows and can drive the advertiser's prepaid balance arbitrarily negative. How this was verified: The rendered ad URL exposes the ID, and the unauthenticated route reaches an unconditional balance decrement with no intervening guard.
| advertiserId: ad.advertiserId, | ||
| amountCents: ad.bidAmount, | ||
| adId: ad.id, | ||
| description: `Click on "${ad.title}"`, |
There was a problem hiding this comment.
This English click description, along with the hard-coded Top-up description, is persisted and rendered verbatim on the localized billing page. Translators therefore cannot localize transaction history when another locale is enabled.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7ae77e6ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch { | ||
| // A concurrent replay of the same event lost the unique-constraint race. | ||
| // The balance is already correct, so acknowledge rather than make Stripe retry. | ||
| } |
There was a problem hiding this comment.
When creditTopUp throws because of a transient database outage, deadlock, migration mismatch, or any error other than the expected duplicate-session race, this catch still returns HTTP 200. Stripe therefore considers the paid event delivered and does not retry it, permanently leaving the customer charged without receiving balance. Only the verified duplicate constraint case should be acknowledged; other failures need to propagate or return a non-2xx response.
Useful? React with 👍 / 👎.
1 participant
Summary
The ad platform can now take money. Advertisers prepay a balance by card, each click deducts their bid, and ads stop serving at zero.
Currency migration (Plan A). The ad platform priced everything in £ despite being US-operated — harmless while the numbers were decorative, painful once Stripe is charging real cards. Added a single
formatUsd(cents)helper and moved the five ad-platform display sites onto it. The QuoteIQ CRM's £ pricing is deliberately untouched — different product, not this PR's call.Billing (Plan B). Prepaid wallet model:
advertisers.balance_centsis a cached figure, always derivable from a new append-onlybilling_transactionsledger (signed amounts,balance_after_centssnapshot, uniquestripe_session_id)selectAdsgates on a positive balance/advertise/billingshows balance, top-up ($25/$50/$100 + custom, $10–$500), and full transaction historyWhy prepaid rather than postpaid
No debt, no dunning, no failed-payment retries, no collections — and an advertiser cannot overspend, which caps the platform's exposure too. Postpaid is the grown-up model but it is a lot of failure-mode machinery for a platform with no advertisers yet, and the ledger makes migrating later straightforward.
Money-safety properties (reviewed and tested)
createTopUpSessionre-validates bounds server-side regardless.Test Plan
4242 4242 4242 4242; confirm balance credits within seconds, and that resending the event from the Stripe dashboard does not double-creditDeploy steps (manual, before this can take real money)
.env.localasSTRIPE_SECRET_KEY(start withsk_test_...)https://bsymbolic.com/api/stripe/webhookforcheckout.session.completed→ copy the signing secret intoSTRIPE_WEBHOOK_SECRETpm2 restart symbolic --update-envThe migration (
0015_youthful_gressill.sql) applies automatically via the deploy workflow. With no Stripe key set, top-ups fall back to a simulated link and no money moves.Known debt, deliberately not fixed here
The bid field is still named
bidPoundsinternally (AdWizard.tsx,adActions.ts). It predates this branch; renaming touches a Zod schema and its tests for zero user-visible benefit, so it stays as-is rather than risking the bid path for tidiness.🤖 Generated with Claude Code
Greptile Summary
The PR adds USD formatting and a prepaid Stripe wallet backed by an append-only billing ledger, then gates ad selection and debits advertisers for clicks.
Confidence Score: 2/5
The PR is not safe to merge until failed webhook credits remain retryable and the public click endpoint cannot be replayed to drain advertiser balances.
Genuine top-up database failures are acknowledged as successful with no recovery path, while the newly monetized public click URL permits unrestricted repeated debits and negative balances; the transaction descriptions also need non-blocking localization cleanup.
Files Needing Attention: src/app/api/stripe/webhook/route.ts, src/app/api/ads/click/route.ts, src/libs/billing.ts
Security Review
The public click redirect now performs an unrestricted monetary debit. Because requests carry no authentication, impression token, idempotency key, deduplication, or sufficient-balance predicate, replaying an active ad's URL can repeatedly charge its advertiser and drive the balance arbitrarily negative. How this was verified: The rendered URL exposes the ad ID, and tracing the unauthenticated GET through
chargeForClickreaches an unconditional balance decrement with no intervening guard.Important Files Changed
Sequence Diagram
Reviews (1): Last reviewed commit: "fix: translate billing errors and acknow..." | Re-trigger Greptile
Context used: