Repository navigation
feat: add embeddable end-user wallets - #2555
Merged
Merged
Conversation
Backend for the embeddable "Stripe for AI" SDK: lets third-party developers give their own end-users a credit wallet, AI access, and in-app top-ups through LLM Gateway. - db: end_customer / wallet / wallet_ledger; webhook_endpoint / platform_webhook_delivery; apiKey.keyType (platform_secret / platform_publishable / ephemeral_session) + wallet binding + expiry; log.endCustomerWalletId; project end-user fields; org margin balance + Connect account; new transaction types (3 additive migrations) - gateway: ephemeral-session resolver bills the bound wallet instead of org credits (chat, embeddings, moderations, videos; responses via chat), with a per-project origin allowlist - api: secret-key + ephemeral-session auth middleware; /v1/sessions(+refresh), /v1/wallet/top-up + balance, /v1/customers, /v1/connect/* (Stripe Connect onboarding + margin payout), /v1/webhooks, public /v1/config; Stripe webhook end-user top-up credit + margin accrual + refund reversal - worker: wallet-debit path; expired-session sweep; signed webhook delivery (HMAC + retries); wallet.low_balance emission; auto margin payout loop - keys-api: hide platform/ephemeral keys from the dashboard list + cap Companion SDK packages live in the llmgateway-templates repo. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughImplements embeddable end-user wallets and ephemeral sessions: DB schema and relations; Hono middlewares for session/platform auth; platform APIs for sessions, wallets, customers, webhooks, connect, and config; Stripe top-up/refund handling; gateway session application; and worker loops for debits, webhook delivery, cleanup, and payouts. ChangesEmbeddable SDK End-User Wallet
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Platform API
participant DB
participant Stripe
participant Worker
Client->>API: POST /v1/sessions (platform secret)
API->>DB: find-or-create endCustomer + wallet, insert ephemeral apiKey
API-->>Client: sessionToken (es_...)
Client->>API: POST /v1/wallet/top-up (with es_... token)
API->>DB: ensure end-customer Stripe customer
API->>Stripe: Create PaymentIntent (metadata includes walletId, netCredited)
Stripe-->>API: payment_intent.succeeded event
API->>DB: handleEndUserTopUpSucceeded -> insert walletLedger "topup", increment wallet.balance, accrue margin
API->>Worker: enqueueWebhookDeliveries(wallet.credited)
Worker->>DB: process batch logs -> decrement wallet.balance, insert walletLedger "usage_debit"
Worker->>API: enqueueWebhookDeliveries(wallet.low_balance)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (16)
apps/worker/src/worker.ts-1836-1846 (1)
1836-1846:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReject private-network webhook targets before fetching them.
This worker posts to whatever
delivery.endpoint.urlcontains. Combined with the platform-webhooks creation route accepting arbitrary URLs, a tenant can point a webhook at RFC1918, link-local, or metadata addresses and turn the delivery loop into an SSRF primitive. Validate endpoints against private CIDRs/internal hostnames at registration time and re-resolve before delivery to avoid DNS rebinding.🤖 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 `@apps/worker/src/worker.ts` around lines 1836 - 1846, The delivery code currently calls fetch(delivery.endpoint.url) directly which allows SSRF to internal/private addresses; before sending, resolve delivery.endpoint.url to IP(s) (e.g., using DNS lookup on the host portion) and reject any target whose resolved address falls into private, loopback, link-local, or metadata ranges (RFC1918: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16, ::1, fc00::/7, fe80::/10 and cloud provider metadata IPs) and also reject internal hostnames; if multiple A/AAAA results are returned ensure all are validated to prevent DNS rebinding, and only then perform the fetch using the validated IP/host; additionally ensure endpoint registration (where delivery.endpoint.url is accepted) performs the same validation to block private/internal targets up-front.apps/api/src/stripe.ts-97-114 (1)
97-114:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent duplicate Stripe customer creation for the same end-customer.
This read-create-update sequence is racy. Two concurrent requests for the same
endCustomerIdcan both observestripeCustomerId === null, create different Stripe customers, and whicheverUPDATElands last wins. That leaves an orphaned Stripe customer and breaks the one-customer-per-end-customer invariant. Claim the row atomically before calling Stripe, or do a conditional update/re-read so only one customer id can ever be persisted.🤖 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 `@apps/api/src/stripe.ts` around lines 97 - 114, The current read-create-update (endCustomer.stripeCustomerId check + customers.create + db.update(...).where(eq(tables.endCustomer.id, endCustomerId))) is racy; fix by claiming the row atomically before calling Stripe: inside a DB transaction lock or conditional update the endCustomer row (use SELECT ... FOR UPDATE or an UPDATE ... WHERE id = endCustomerId AND stripeCustomerId IS NULL RETURNING stripeCustomerId) to ensure only one caller proceeds to create a Stripe customer, then if the row was claimed create the Stripe customer with getStripe().customers.create and persist the returned id with an UPDATE that only writes when the row is still claimed; alternatively, if the conditional update returned an existing stripeCustomerId, return it immediately. Reference symbols: endCustomer, endCustomerId, getStripe(), db.update(...).where(eq(tables.endCustomer.id, endCustomerId)), tables.endCustomer.apps/worker/src/worker.ts-961-1002 (1)
961-1002:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCompute wallet
balanceAfterfrom the committed row, not the pre-read snapshot.
prevBalance/newBalanceare derived from afindFirst()before theUPDATE, butapps/api/src/stripe.tscan top up or reverse the same wallet concurrently. The decrement itself is safe, butwallet_ledger.balanceAfterand the low-balance crossing check can be stale or wrong if another writer changes the balance between the read and the update. Lock the wallet row or switch to anUPDATE ... RETURNINGflow that derives the final balance from the row version actually debited.🤖 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 `@apps/worker/src/worker.ts` around lines 961 - 1002, The code reads a wallet with tx.query.wallet.findFirst and computes prevBalance/newBalance before doing tx.update(tables.wallet).set(...), which can race with concurrent top-ups/reversals; change to an atomic UPDATE ... RETURNING (or lock the row) so the final balance is derived from the committed row actually debited. Specifically, replace the separate read (walletRecord / prevBalance / newBalance) + tx.update(...) with an UPDATE that subtracts costNumber and returns the resulting balance (use that returned balance for walletLedger.balanceAfter and for the WALLET_LOW_BALANCE_THRESHOLD crossing check), then insert into tables.walletLedger using the returned fields (walletId, organizationId, endCustomerId) rather than the pre-read snapshot.apps/api/src/stripe.ts-1459-1526 (1)
1459-1526:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPartial refunds currently wipe the full wallet top-up.
handleChargeRefundedroutes any wallet refund intohandleEndUserTopUpRefunded(topUp), but that helper never sees the actual Stripe refund amount or refund id. It always reversestopUp.netCreditedand the full developer margin once, so a partial refund removes the entire wallet credit and all accrued margin, and later partial refunds are skipped by the reversal dedupe. Mirror the org-credit refund path here: key off the Stripe refund id and reverse only the proportional net credit/margin for that refund.Also applies to: 1948-1956
apps/gateway/src/chat/chat.ts-1622-1628 (1)
1622-1628:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStrip developer dev-plan credits from the effective wallet org.
Lines 3498-3505, 3605-3611, and 3775-3782 still add
devPlanCreditsRemainingon top oforganization.credits. AfterwithWalletCredits(...), that lets an empty end-user wallet pass credit gating whenever the developer org still has dev-plan quota, even though the worker will bill the wallet path. Please zero out or ignore dev-plan credit balances for session-backed requests when building the effective organization snapshot.🤖 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 `@apps/gateway/src/chat/chat.ts` around lines 1622 - 1628, The effective organization snapshot is still inheriting dev-plan credits after withWalletCredits(...) which lets session-backed requests pass gating using developer devPlanCreditsRemaining; modify the code paths that compute the effective organization credits (where organization.credits and devPlanCreditsRemaining are combined — search for usages around withWalletCredits, devPlanCreditsRemaining, and functions that build the organization snapshot) to zero out or ignore devPlanCreditsRemaining whenever endUserWallet or a session-backed request is present (e.g., when endUserWallet is truthy or session flag is set) so the returned organization object uses only wallet credits for credit gating.apps/gateway/src/videos/videos.ts-679-684 (1)
679-684:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftScope video ownership to the end-user session.
After
applyEndUserSession()is enabled here, ephemeral browser sessions can reach all video routes, but the read paths at Line 3793 and Line 3877 still authorize only byproject.idviarequireVideoJobForProject()on Line 2177. NewvideoJobrows also do not persistendCustomerWalletIdor any end-customer owner on Line 3729. That lets one end-user session read another end-user’s video status/content within the same project if it learns thevideo_id. Persist session ownership onvideo_joband enforce it in the retrieval/content queries, or reject session tokens on the read routes until that owner check exists.🤖 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 `@apps/gateway/src/videos/videos.ts` around lines 679 - 684, After enabling applyEndUserSession(c, apiKey, baseProject, baseOrganization) you must persist and enforce end-user ownership: when creating a new video job record (the code path that inserts into video_job / constructs the videoJob) add and persist the session owner identifier (e.g., endCustomerWalletId) from the applied session, and update requireVideoJobForProject to also validate that the retrieved video job's endCustomerWalletId matches the current session's endCustomerWalletId (or reject ephemeral session tokens on read routes until this check exists); in short, modify the video job creation to store the session owner and extend requireVideoJobForProject and the read/content handlers to include owner-equality checks against the applied session from applyEndUserSession.apps/api/src/routes/platform-connect.ts-128-141 (1)
128-141:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
/payoutscurrently depends on/statusbeing called first.
stripeConnectOnboardedis only refreshed insideGET /status, but Line 180 blocks payouts on that cached column. A Stripe account that has just become payout-enabled will still get a 400 here until some client hits/statusto sync the flag. Re-check Stripe live in/payouts, or move this state sync out of the read-only status endpoint.Also applies to: 180-185
apps/api/src/routes/platform-session-refresh.ts-62-76 (1)
62-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh currently resets windowed spend limits.
Only
usageLimitandusageare copied to the rotated key. If the original session had aperiodUsageLimit, the client can refresh and start a fresh window withcurrentPeriodUsage = 0, which bypasses the intended spend cap.🩹 Preserve the full limit state on rotation
.values({ token, projectId: oldKey.projectId, description: oldKey.description, keyType: "ephemeral_session", endCustomerWalletId: session.walletId, expiresAt, // Carry the spend cap + accumulated usage forward so refreshing can't // reset it. usageLimit: oldKey.usageLimit, usage: oldKey.usage, + periodUsageLimit: oldKey.periodUsageLimit, + periodUsageDurationValue: oldKey.periodUsageDurationValue, + periodUsageDurationUnit: oldKey.periodUsageDurationUnit, + currentPeriodUsage: oldKey.currentPeriodUsage, + currentPeriodStartedAt: oldKey.currentPeriodStartedAt, createdBy: oldKey.createdBy, })🤖 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 `@apps/api/src/routes/platform-session-refresh.ts` around lines 62 - 76, The rotation only copies usageLimit and usage, which lets windowed limits be reset; when inserting the new API key in the db.insert(tables.apiKey).values(...) (the block creating newKey using token, oldKey, expiresAt, etc.) also copy the windowed-limit fields from oldKey (e.g. periodUsageLimit and currentPeriodUsage — and if your schema has periodResetAt/periodWindowStart or similarly named fields, copy those too) so the new ephemeral_session preserves the full limit state rather than starting a fresh window.apps/api/src/routes/platform-sessions.ts-102-122 (1)
102-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNarrow these catch blocks to the unique-constraint race only.
Both inserts currently swallow every database error and treat it as "someone else created the row first". That hides real failures like connectivity/FK issues and degrades the error into a generic re-read/500 path. Only catch the specific unique-violation case and let everything else propagate.
As per coding guidelines,
apps/{gateway,api}/src/**/*.{ts,tsx}: "Do not use broad try/catch in API handlers unless to check for specific errors; instead, let errors propagate and be handled by the global error handler."Also applies to: 137-151
🤖 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 `@apps/api/src/routes/platform-sessions.ts` around lines 102 - 122, The current catch blocks around the db.insert into tables.endCustomer (the block creating endCustomer and the similar block at the second instance) are too broad and swallow all DB errors; change them to only handle unique-constraint/duplicate-key errors (e.g., SQLSTATE '23505' for Postgres or vendor code 'ER_DUP_ENTRY' for MySQL) by checking the thrown error's code/sqlState (or specific error class) and only then perform the re-read via db.query.endCustomer.findFirst; for any other error rethrow it so it propagates to the global handler. Ensure you update both the insertion that assigns [endCustomer] and the other insert block referenced in the comment.Source: Coding guidelines
apps/api/src/routes/platform-sessions.ts-324-338 (1)
324-338:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the balance update and ledger write in one transaction.
/wallets/{id}/creditmutateswallet.balanceand then appends the ledger row in a separate statement. If the insert fails after the update, the wallet stays credited with no matching ledger entry. These writes need to succeed or fail together.Suggested direction
- const [updated] = await db - .update(tables.wallet) - .set({ balance: sql`${tables.wallet.balance} + ${amount}` }) - .where(eq(tables.wallet.id, id)) - .returning(); - - await db.insert(tables.walletLedger).values({ - walletId: wallet.id, - endCustomerId: wallet.endCustomerId, - organizationId: wallet.organizationId, - type: "adjustment", - amount: String(amount), - balanceAfter: updated.balance, - description: reason ?? "Server-side credit grant", - }); + const updated = await db.transaction(async (tx) => { + const [nextWallet] = await tx + .update(tables.wallet) + .set({ balance: sql`${tables.wallet.balance} + ${amount}` }) + .where(eq(tables.wallet.id, id)) + .returning(); + + await tx.insert(tables.walletLedger).values({ + walletId: wallet.id, + endCustomerId: wallet.endCustomerId, + organizationId: wallet.organizationId, + type: "adjustment", + amount: String(amount), + balanceAfter: nextWallet.balance, + description: reason ?? "Server-side credit grant", + }); + + return nextWallet; + });🤖 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 `@apps/api/src/routes/platform-sessions.ts` around lines 324 - 338, The wallet balance update and ledger insert must run inside a single DB transaction so they either both commit or both roll back: wrap the db.update to tables.wallet and the db.insert to tables.walletLedger in a single transaction (use db.transaction / db.$transaction API available in your DB client), execute the update using the transaction handle (so you get the updated row/table result from the transaction) and then insert the ledger row using that same transaction handle (referencing tables.wallet, tables.walletLedger, updated.balance, wallet.id, wallet.endCustomerId, wallet.organizationId, amount and reason) so the update and ledger insert succeed or fail together.apps/api/src/routes/platform-sessions.ts-248-258 (1)
248-258:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope wallet access to the authenticated project as well.
platformSecretAuthauthenticates a specificprojectId, but this helper only checksorganizationId. A platform secret from project A can therefore read or credit wallets from project B in the same org if it knows the wallet id. The lookup should enforce both org and project ownership.Suggested fix
-/** Load a wallet and assert it belongs to the authenticated platform key's org. */ -async function loadWalletForPlatform(walletId: string, organizationId: string) { +/** Load a wallet and assert it belongs to the authenticated platform key's org/project. */ +async function loadWalletForPlatform( + walletId: string, + organizationId: string, + projectId: string, +) { const wallet = await db.query.wallet.findFirst({ - where: { id: { eq: walletId } }, + where: { + id: { eq: walletId }, + organizationId: { eq: organizationId }, + projectId: { eq: projectId }, + }, }); - if (!wallet || wallet.organizationId !== organizationId) { + if (!wallet) { throw new HTTPException(404, { - message: "Wallet not found in this organization", + message: "Wallet not found in this project", }); } return wallet; }🤖 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 `@apps/api/src/routes/platform-sessions.ts` around lines 248 - 258, The loadWalletForPlatform helper only checks organizationId but must also enforce project ownership; update the function loadWalletForPlatform(walletId: string, organizationId: string) to accept the authenticated projectId (e.g. add parameter projectId: string) and change the DB lookup (db.query.wallet.findFirst / the where clause) to require both organizationId and projectId match the wallet, or alternatively check wallet.projectId === projectId after fetch and throw the same HTTPException(404, ...) when it mismatches; update any callers of loadWalletForPlatform to pass the current projectId from platformSecretAuth.apps/api/src/routes/platform-sessions.ts-179-209 (1)
179-209:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCreate the session key and its scope atomically.
The
api_keyrow is inserted before the IAM rule and usage-limit writes. If either follow-up statement fails, this endpoint returns an error but leaves behind an active session token without the requested model restriction or spend cap. Wrap the whole mint flow in a single transaction and setusageLimiton the initial insert.Suggested direction
- const [sessionKey] = await db - .insert(tables.apiKey) - .values({ - token, - projectId: platformKey.projectId, - description: `End-user session for ${endCustomer.externalId}`, - keyType: "ephemeral_session", - endCustomerWalletId: wallet.id, - expiresAt, - createdBy: platformKey.createdBy, - }) - .returning(); - - if (scope?.models && scope.models.length > 0) { - await db.insert(tables.apiKeyIamRule).values({ - apiKeyId: sessionKey.id, - ruleType: "allow_models", - ruleValue: { models: scope.models }, - }); - } - - if (scope?.maxSpend) { - await db - .update(tables.apiKey) - .set({ usageLimit: String(scope.maxSpend) }) - .where(eq(tables.apiKey.id, sessionKey.id)); - } + const sessionKey = await db.transaction(async (tx) => { + const [created] = await tx + .insert(tables.apiKey) + .values({ + token, + projectId: platformKey.projectId, + description: `End-user session for ${endCustomer.externalId}`, + keyType: "ephemeral_session", + endCustomerWalletId: wallet.id, + expiresAt, + createdBy: platformKey.createdBy, + usageLimit: scope?.maxSpend ? String(scope.maxSpend) : undefined, + }) + .returning(); + + if (scope?.models?.length) { + await tx.insert(tables.apiKeyIamRule).values({ + apiKeyId: created.id, + ruleType: "allow_models", + ruleValue: { models: scope.models }, + }); + } + + return created; + });🤖 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 `@apps/api/src/routes/platform-sessions.ts` around lines 179 - 209, Wrap the entire session mint flow in a single database transaction so the api key insert, optional apiKeyIamRule insert, and optional usageLimit update are atomic; instead of inserting the api_key row first and updating it later, include usageLimit on the initial db.insert(tables.apiKey).values(...) when scope?.maxSpend is present, and perform the conditional db.insert(tables.apiKeyIamRule).values(...) (using apiKeyId: sessionKey.id and ruleType/ruleValue) inside the same transaction, ensuring any error rolls back the api_key creation; use the same db transaction context for all operations that currently reference sessionKey, tables.apiKey, and tables.apiKeyIamRule.packages/db/migrations/1780710942_adorable_moonstone.sql-9-14 (1)
9-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConstrain webhook status/attempt fields at the database boundary.
Both tables use free-form
textstatuses, and deliveryattemptshas no lower bound. A bad write can silently bypass pending-delivery queries or corrupt retry behavior. Add check constraints for valid status values and non-negative attempts.Suggested constraints
CREATE TABLE "platform_webhook_delivery" ( @@ "status" text DEFAULT 'pending' NOT NULL, "attempts" integer DEFAULT 0 NOT NULL, @@ ); @@ CREATE TABLE "webhook_endpoint" ( @@ "status" text DEFAULT 'active' NOT NULL ); +ALTER TABLE "platform_webhook_delivery" +ADD CONSTRAINT "platform_webhook_delivery_status_chk" +CHECK ("status" IN ('pending', 'delivered', 'failed')); +ALTER TABLE "platform_webhook_delivery" +ADD CONSTRAINT "platform_webhook_delivery_attempts_non_negative_chk" +CHECK ("attempts" >= 0); +ALTER TABLE "webhook_endpoint" +ADD CONSTRAINT "webhook_endpoint_status_chk" +CHECK ("status" IN ('active', 'disabled'));Also applies to: 26-27
🤖 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/db/migrations/1780710942_adorable_moonstone.sql` around lines 9 - 14, Add DB-level CHECK constraints on the webhook tables to ensure "status" only contains allowed values and "attempts" is non-negative: add a CHECK constraint like CHECK (status IN ('pending','sent','failed','cancelled')) (use an appropriate set of statuses your app uses) and add CHECK (attempts >= 0) for the "attempts" column; name the constraints clearly (e.g. chk_webhook_status, chk_webhook_attempts) and add the same constraints to the other table referenced in the diff (the second table at lines 26-27) while preserving existing defaults (e.g. DEFAULT 'pending') and column definitions ("status", "attempts", "next_attempt_at", "last_attempt_at", "response_status", "last_error").apps/api/src/lib/platform-secret-auth.ts-52-58 (1)
52-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInactive projects/organizations currently pass authentication.
The middleware only blocks
deletedentities;inactivestill succeeds. If "active only" is intended, enforce strict active checks for both project and organization.Suggested condition update
- if (row.project.status === "deleted") { + if (row.project.status !== "active") { throw new HTTPException(403, { message: "Project is not active" }); } - if (row.project.organization?.status === "deleted") { + if (row.project.organization?.status !== "active") { throw new HTTPException(403, { message: "Organization is not active" }); }🤖 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 `@apps/api/src/lib/platform-secret-auth.ts` around lines 52 - 58, The current checks only throw when status === "deleted", allowing "inactive" to authenticate; update the logic in the platform-secret-auth middleware where row.project and row.project.organization are validated so that you require an explicit active status (e.g., if row.project.status !== "active" and if row.project.organization?.status !== "active") and throw HTTPException(403, { message: "Project is not active" }) / HTTPException(403, { message: "Organization is not active" }) using the existing HTTPException class to enforce strict active-only access.apps/api/src/lib/end-user-session-auth.ts-52-64 (1)
52-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject non-active end customers/projects during session auth.
endUserSessionAuthaccepts tokens even when related entities are no longer active, because it only validates wallet status. Add explicit checks forkey.wallet.endCustomer.status === "active"andkey.wallet.project?.status === "active"before setting context.Suggested guard block
if (!key || !key.endCustomerWalletId || !key.wallet) { throw new HTTPException(401, { message: "Invalid session token" }); } + + if (key.wallet.endCustomer?.status !== "active") { + throw new HTTPException(403, { + message: "End customer is not active", + }); + } + + if (key.wallet.project?.status !== "active") { + throw new HTTPException(403, { + message: "Project is not active", + }); + }🤖 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 `@apps/api/src/lib/end-user-session-auth.ts` around lines 52 - 64, endUserSessionAuth currently only checks key.wallet.status and allows sessions for end customers or projects that are inactive; add explicit guards to reject tokens when key.wallet.endCustomer.status !== "active" or when key.wallet.project exists and key.wallet.project.status !== "active". Update the validation block that inspects key and key.wallet (the same area that throws HTTPException for invalid/expired/frozen sessions) to throw HTTPException(401, { message: "End customer is inactive" }) for a non-active endCustomer and HTTPException(401, { message: "Project is inactive" }) when a project exists but is not active, before setting context or returning the session.packages/db/migrations/1780710405_even_madripoor.sql-1-3 (1)
1-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce Connect onboarding state consistency.
stripe_connect_onboardedcan be set totruewhilestripe_connect_account_idremainsNULL, which creates an invalid org state for Connect flows. Add a DB check to keep these fields consistent.Suggested migration tweak
ALTER TABLE "organization" ADD COLUMN "stripe_connect_account_id" text;--> statement-breakpoint ALTER TABLE "organization" ADD COLUMN "stripe_connect_onboarded" boolean DEFAULT false NOT NULL;--> statement-breakpoint ALTER TABLE "organization" ADD CONSTRAINT "organization_stripe_connect_account_id_key" UNIQUE("stripe_connect_account_id"); +ALTER TABLE "organization" +ADD CONSTRAINT "organization_connect_onboarded_requires_account_id_chk" +CHECK ( + NOT stripe_connect_onboarded + OR stripe_connect_account_id IS NOT NULL +);🤖 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/db/migrations/1780710405_even_madripoor.sql` around lines 1 - 3, Add a DB CHECK constraint to prevent stripe_connect_onboarded = true while stripe_connect_account_id IS NULL: in the migration, after adding the two columns (stripe_connect_account_id and stripe_connect_onboarded) add an ALTER TABLE "organization" ... ADD CONSTRAINT (e.g. organization_stripe_connect_onboarded_consistency_check) CHECK (NOT (stripe_connect_onboarded AND stripe_connect_account_id IS NULL)); ensure you add any necessary data backfill or updates to existing rows before adding the constraint so the check will not fail at creation.
🟡 Minor comments (1)
apps/gateway/src/lib/end-user-session.ts-46-49 (1)
46-49:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse 401/403 instead of 500 for unbound session tokens.
An unbound ephemeral token is an auth failure, not an internal server error. Returning 500 here can trigger false incident signals and retry noise.
Suggested change
if (!apiKey.endCustomerWalletId) { - throw new HTTPException(500, { + throw new HTTPException(401, { message: "Session token is not bound to a wallet", }); }🤖 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 `@apps/gateway/src/lib/end-user-session.ts` around lines 46 - 49, The check in end-user-session.ts that throws new HTTPException(500, ...) when apiKey.endCustomerWalletId is missing should return an auth error instead of a server error: change the thrown status from 500 to an appropriate auth status (use 401 Unauthorized for an unbound/invalid ephemeral token, or 403 Forbidden if you consider it a rights issue) by updating the HTTPException(...) call in the block that references apiKey.endCustomerWalletId; ensure the error message remains descriptive and adjust any tests/consumers that expect a 500 accordingly.
🤖 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 `@apps/api/src/routes/platform-connect.ts`:
- Around line 198-224: The current flow calls getStripe().transfers.create,
db.update(tables.organization) and db.insert(tables.transaction) as separate
side effects which can double-debit on retries and reuse the same idempotency
key later; fix by creating a persisted payout record with a unique idempotency
key before calling Stripe and do the local bookkeeping inside a single DB
transaction: insert a new payout/transaction row (or a new payouts table) with a
unique constraint on the generated payout_id/idempotency_key, SELECT FOR UPDATE
the organization row to verify and decrement endUserMarginBalance atomically
(using the same sql`GREATEST(...)` logic) and mark the payout as “pending”; then
call getStripe().transfers.create using the persisted unique idempotency key,
and upon successful transfer update the persisted payout/transaction row with
transfer.id and status="completed" (or status="failed" on error) — this ensures
idempotent retries, prevents double-debits, and avoids idempotency key
collisions.
In `@apps/api/src/routes/platform-webhooks.ts`:
- Around line 37-39: The webhook registration schema currently allows any
absolute URL (schema: z.object with url: z.string().url()), which enables SSRF;
update the validation for the url field used in platform-webhooks.ts (and the
other schema blocks mentioned) to require HTTPS and reject
private/reserved/DNS-rebindable destinations via a custom Zod refinement: parse
the URL, enforce protocol === 'https:', resolve and validate the hostname is not
an IP in RFC1918/reserved ranges nor a loopback/metadata address, and reject
hostnames that resolve to such addresses (or use a safe-hoster allowlist). Also
add a note to re-validate the same checks at delivery time in the worker that
POSTs webhooks to ensure DNS rebinding protection.
In `@apps/api/src/stripe.ts`:
- Around line 1361-1425: The wallet top-up is non-atomic: balance is updated
before the wallet_ledger/topup row and org transaction are committed, so retries
or crashes can double-credit; wrap the entire sequence (check/insert ledger
dedupe, update tables.wallet balance, insert tables.walletLedger, update
tables.organization endUserMarginBalance, insert tables.transaction) inside a
single DB transaction (use db.transaction or equivalent) and enforce idempotency
with a unique constraint on (stripePaymentIntentId, type) in wallet_ledger;
implement the ledger insert within that transaction using an
upsert/conflict-do-nothing or catch unique-violation to avoid double-processing
(use paymentIntent.id, walletId, developerMargin, netCredited, and the tables.*
symbols to locate the statements).
In `@apps/worker/src/worker.ts`:
- Around line 1942-1985: processMarginPayouts currently reads
endUserMarginBalance and then issues the Stripe transfer, which can cause
overpayment if concurrent changes occur; change it to atomically reserve the
payout first by performing a conditional DB update that decrements the balance
only if balance >= amount (use an UPDATE on organization with a WHERE that
checks organization.endUserMarginBalance >= amount and check rowsAffected),
aborting if the update didn't claim funds; then call
getStripe().transfers.create to perform the transfer; on transfer failure
restore the reserved amount (increment the balance back) and on success proceed
to insert the transaction and mark completion. Reference: processMarginPayouts,
organization, getStripe().transfers.create, and ensure you handle rowsAffected
and rollback on error.
In `@packages/db/src/schema.ts`:
- Around line 618-632: The wallet ledger currently only has a non-unique index
on stripePaymentIntentId (see wallet_ledger_stripe_payment_intent_id_idx) so
concurrent webhook deliveries can insert duplicate top-up rows; add a UNIQUE
partial index/constraint for top-up rows so stripePaymentIntentId is unique when
type === "topup" (e.g., create a unique index on table.stripePaymentIntentId
WHERE type = 'topup' or a unique composite on (stripePaymentIntentId, type)
filtered to type='topup') to enforce idempotency at the DB layer.
---
Major comments:
In `@apps/api/src/lib/end-user-session-auth.ts`:
- Around line 52-64: endUserSessionAuth currently only checks key.wallet.status
and allows sessions for end customers or projects that are inactive; add
explicit guards to reject tokens when key.wallet.endCustomer.status !== "active"
or when key.wallet.project exists and key.wallet.project.status !== "active".
Update the validation block that inspects key and key.wallet (the same area that
throws HTTPException for invalid/expired/frozen sessions) to throw
HTTPException(401, { message: "End customer is inactive" }) for a non-active
endCustomer and HTTPException(401, { message: "Project is inactive" }) when a
project exists but is not active, before setting context or returning the
session.
In `@apps/api/src/lib/platform-secret-auth.ts`:
- Around line 52-58: The current checks only throw when status === "deleted",
allowing "inactive" to authenticate; update the logic in the
platform-secret-auth middleware where row.project and row.project.organization
are validated so that you require an explicit active status (e.g., if
row.project.status !== "active" and if row.project.organization?.status !==
"active") and throw HTTPException(403, { message: "Project is not active" }) /
HTTPException(403, { message: "Organization is not active" }) using the existing
HTTPException class to enforce strict active-only access.
In `@apps/api/src/routes/platform-session-refresh.ts`:
- Around line 62-76: The rotation only copies usageLimit and usage, which lets
windowed limits be reset; when inserting the new API key in the
db.insert(tables.apiKey).values(...) (the block creating newKey using token,
oldKey, expiresAt, etc.) also copy the windowed-limit fields from oldKey (e.g.
periodUsageLimit and currentPeriodUsage — and if your schema has
periodResetAt/periodWindowStart or similarly named fields, copy those too) so
the new ephemeral_session preserves the full limit state rather than starting a
fresh window.
In `@apps/api/src/routes/platform-sessions.ts`:
- Around line 102-122: The current catch blocks around the db.insert into
tables.endCustomer (the block creating endCustomer and the similar block at the
second instance) are too broad and swallow all DB errors; change them to only
handle unique-constraint/duplicate-key errors (e.g., SQLSTATE '23505' for
Postgres or vendor code 'ER_DUP_ENTRY' for MySQL) by checking the thrown error's
code/sqlState (or specific error class) and only then perform the re-read via
db.query.endCustomer.findFirst; for any other error rethrow it so it propagates
to the global handler. Ensure you update both the insertion that assigns
[endCustomer] and the other insert block referenced in the comment.
- Around line 324-338: The wallet balance update and ledger insert must run
inside a single DB transaction so they either both commit or both roll back:
wrap the db.update to tables.wallet and the db.insert to tables.walletLedger in
a single transaction (use db.transaction / db.$transaction API available in your
DB client), execute the update using the transaction handle (so you get the
updated row/table result from the transaction) and then insert the ledger row
using that same transaction handle (referencing tables.wallet,
tables.walletLedger, updated.balance, wallet.id, wallet.endCustomerId,
wallet.organizationId, amount and reason) so the update and ledger insert
succeed or fail together.
- Around line 248-258: The loadWalletForPlatform helper only checks
organizationId but must also enforce project ownership; update the function
loadWalletForPlatform(walletId: string, organizationId: string) to accept the
authenticated projectId (e.g. add parameter projectId: string) and change the DB
lookup (db.query.wallet.findFirst / the where clause) to require both
organizationId and projectId match the wallet, or alternatively check
wallet.projectId === projectId after fetch and throw the same HTTPException(404,
...) when it mismatches; update any callers of loadWalletForPlatform to pass the
current projectId from platformSecretAuth.
- Around line 179-209: Wrap the entire session mint flow in a single database
transaction so the api key insert, optional apiKeyIamRule insert, and optional
usageLimit update are atomic; instead of inserting the api_key row first and
updating it later, include usageLimit on the initial
db.insert(tables.apiKey).values(...) when scope?.maxSpend is present, and
perform the conditional db.insert(tables.apiKeyIamRule).values(...) (using
apiKeyId: sessionKey.id and ruleType/ruleValue) inside the same transaction,
ensuring any error rolls back the api_key creation; use the same db transaction
context for all operations that currently reference sessionKey, tables.apiKey,
and tables.apiKeyIamRule.
In `@apps/api/src/stripe.ts`:
- Around line 97-114: The current read-create-update
(endCustomer.stripeCustomerId check + customers.create +
db.update(...).where(eq(tables.endCustomer.id, endCustomerId))) is racy; fix by
claiming the row atomically before calling Stripe: inside a DB transaction lock
or conditional update the endCustomer row (use SELECT ... FOR UPDATE or an
UPDATE ... WHERE id = endCustomerId AND stripeCustomerId IS NULL RETURNING
stripeCustomerId) to ensure only one caller proceeds to create a Stripe
customer, then if the row was claimed create the Stripe customer with
getStripe().customers.create and persist the returned id with an UPDATE that
only writes when the row is still claimed; alternatively, if the conditional
update returned an existing stripeCustomerId, return it immediately. Reference
symbols: endCustomer, endCustomerId, getStripe(),
db.update(...).where(eq(tables.endCustomer.id, endCustomerId)),
tables.endCustomer.
In `@apps/gateway/src/chat/chat.ts`:
- Around line 1622-1628: The effective organization snapshot is still inheriting
dev-plan credits after withWalletCredits(...) which lets session-backed requests
pass gating using developer devPlanCreditsRemaining; modify the code paths that
compute the effective organization credits (where organization.credits and
devPlanCreditsRemaining are combined — search for usages around
withWalletCredits, devPlanCreditsRemaining, and functions that build the
organization snapshot) to zero out or ignore devPlanCreditsRemaining whenever
endUserWallet or a session-backed request is present (e.g., when endUserWallet
is truthy or session flag is set) so the returned organization object uses only
wallet credits for credit gating.
In `@apps/gateway/src/videos/videos.ts`:
- Around line 679-684: After enabling applyEndUserSession(c, apiKey,
baseProject, baseOrganization) you must persist and enforce end-user ownership:
when creating a new video job record (the code path that inserts into video_job
/ constructs the videoJob) add and persist the session owner identifier (e.g.,
endCustomerWalletId) from the applied session, and update
requireVideoJobForProject to also validate that the retrieved video job's
endCustomerWalletId matches the current session's endCustomerWalletId (or reject
ephemeral session tokens on read routes until this check exists); in short,
modify the video job creation to store the session owner and extend
requireVideoJobForProject and the read/content handlers to include
owner-equality checks against the applied session from applyEndUserSession.
In `@apps/worker/src/worker.ts`:
- Around line 1836-1846: The delivery code currently calls
fetch(delivery.endpoint.url) directly which allows SSRF to internal/private
addresses; before sending, resolve delivery.endpoint.url to IP(s) (e.g., using
DNS lookup on the host portion) and reject any target whose resolved address
falls into private, loopback, link-local, or metadata ranges (RFC1918:
10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16, ::1,
fc00::/7, fe80::/10 and cloud provider metadata IPs) and also reject internal
hostnames; if multiple A/AAAA results are returned ensure all are validated to
prevent DNS rebinding, and only then perform the fetch using the validated
IP/host; additionally ensure endpoint registration (where delivery.endpoint.url
is accepted) performs the same validation to block private/internal targets
up-front.
- Around line 961-1002: The code reads a wallet with tx.query.wallet.findFirst
and computes prevBalance/newBalance before doing
tx.update(tables.wallet).set(...), which can race with concurrent
top-ups/reversals; change to an atomic UPDATE ... RETURNING (or lock the row) so
the final balance is derived from the committed row actually debited.
Specifically, replace the separate read (walletRecord / prevBalance /
newBalance) + tx.update(...) with an UPDATE that subtracts costNumber and
returns the resulting balance (use that returned balance for
walletLedger.balanceAfter and for the WALLET_LOW_BALANCE_THRESHOLD crossing
check), then insert into tables.walletLedger using the returned fields
(walletId, organizationId, endCustomerId) rather than the pre-read snapshot.
In `@packages/db/migrations/1780710405_even_madripoor.sql`:
- Around line 1-3: Add a DB CHECK constraint to prevent stripe_connect_onboarded
= true while stripe_connect_account_id IS NULL: in the migration, after adding
the two columns (stripe_connect_account_id and stripe_connect_onboarded) add an
ALTER TABLE "organization" ... ADD CONSTRAINT (e.g.
organization_stripe_connect_onboarded_consistency_check) CHECK (NOT
(stripe_connect_onboarded AND stripe_connect_account_id IS NULL)); ensure you
add any necessary data backfill or updates to existing rows before adding the
constraint so the check will not fail at creation.
In `@packages/db/migrations/1780710942_adorable_moonstone.sql`:
- Around line 9-14: Add DB-level CHECK constraints on the webhook tables to
ensure "status" only contains allowed values and "attempts" is non-negative: add
a CHECK constraint like CHECK (status IN
('pending','sent','failed','cancelled')) (use an appropriate set of statuses
your app uses) and add CHECK (attempts >= 0) for the "attempts" column; name the
constraints clearly (e.g. chk_webhook_status, chk_webhook_attempts) and add the
same constraints to the other table referenced in the diff (the second table at
lines 26-27) while preserving existing defaults (e.g. DEFAULT 'pending') and
column definitions ("status", "attempts", "next_attempt_at", "last_attempt_at",
"response_status", "last_error").
---
Minor comments:
In `@apps/gateway/src/lib/end-user-session.ts`:
- Around line 46-49: The check in end-user-session.ts that throws new
HTTPException(500, ...) when apiKey.endCustomerWalletId is missing should return
an auth error instead of a server error: change the thrown status from 500 to an
appropriate auth status (use 401 Unauthorized for an unbound/invalid ephemeral
token, or 403 Forbidden if you consider it a rights issue) by updating the
HTTPException(...) call in the block that references apiKey.endCustomerWalletId;
ensure the error message remains descriptive and adjust any tests/consumers that
expect a 500 accordingly.
🪄 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: f7dd19b1-1625-47c1-bdf2-bf235c41235a
📒 Files selected for processing (35)
apps/api/src/index.tsapps/api/src/lib/end-user-session-auth.tsapps/api/src/lib/platform-secret-auth.tsapps/api/src/routes/keys-api.tsapps/api/src/routes/organization.tsapps/api/src/routes/platform-connect.tsapps/api/src/routes/platform-customers.tsapps/api/src/routes/platform-session-refresh.tsapps/api/src/routes/platform-sessions.tsapps/api/src/routes/platform-wallet.tsapps/api/src/routes/platform-webhooks.tsapps/api/src/routes/public-config.tsapps/api/src/stripe.tsapps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/create-log-entry.tsapps/gateway/src/embeddings/embeddings.tsapps/gateway/src/lib/api-key-usage-limits.spec.tsapps/gateway/src/lib/cached-queries.tsapps/gateway/src/lib/end-user-session.tsapps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/moderations/moderations.tsapps/gateway/src/videos/videos.tsapps/worker/src/worker.tspackages/db/migrations/1780705855_fat_firedrake.sqlpackages/db/migrations/1780710405_even_madripoor.sqlpackages/db/migrations/1780710942_adorable_moonstone.sqlpackages/db/migrations/meta/1780705855_snapshot.jsonpackages/db/migrations/meta/1780710405_snapshot.jsonpackages/db/migrations/meta/1780710942_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/index.tspackages/db/src/relations.tspackages/db/src/schema.tspackages/db/src/types.tspackages/db/src/webhook-helpers.ts
Two bugs found running the embeddable SDK end-to-end:
- platform-sessions is mounted at the bare `/v1` prefix, so its blanket
`use("*", platformSecretAuth)` registered as `/v1/*` and leaked onto sibling
routers — wrongly demanding a secret key on the public `/v1/config` and the
es_-authenticated `/v1/wallet` and `/v1/sessions/refresh`. Scope the
middleware to the routes this router owns (`/sessions`, `/wallets/*`).
- The IAM `allow_models` check compares against the canonical model id
(e.g. `gpt-4o-mini`), not the `provider/model` request form, so a session
scoped to `openai/gpt-4o-mini` rejected every chat with 403. Normalize the
scope so developers can pass either form.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verified each review finding against current code; fixed the still-valid ones. Idempotency / atomicity: - stripe.ts: credit wallet + ledger + margin accrual now run in one transaction; add a unique partial index on wallet_ledger(stripePaymentIntentId) WHERE type='topup' so concurrent webhook deliveries can't double-credit (update-first serializes on the wallet row; 23505 is treated as already-processed). - platform-sessions: mint flow (api key + IAM rule) is atomic, so a partial failure can't leave an unscoped full-access session; usageLimit folded into the insert. Wallet credit grant wraps update+ledger in a transaction. Money correctness (reserve-first): - worker auto-payout + platform-connect manual payout: conditional decrement (WHERE balance >= amount RETURNING), abort if unclaimed, restore on transfer failure — closes the manual-vs-auto payout race. - worker wallet debit: derive balanceAfter + low-balance crossing from an atomic UPDATE ... RETURNING instead of a stale pre-read. SSRF: - new browser-safe @llmgateway/shared assertSafeWebhookUrl/isPrivateOrReservedIp; enforced at webhook registration and at delivery, where the worker also resolves DNS and rejects private/reserved addresses (DNS-rebinding guard). Auth / tenancy: - platform-secret-auth + end-user-session-auth: reject inactive/blocked/deleted projects, orgs, and end customers (not just deleted). - loadWalletForPlatform enforces projectId, not just org. - videos: new videoJob.endCustomerWalletId; read routes reject cross-end-user access within a shared project. - platform-sessions insert catches only swallow unique violations now. - session refresh copies windowed-limit fields forward. - end-user-session: unbound ephemeral token returns 401, not 500. Build (12/12) + eslint clean. Migration is additive (one column, one unique partial index, one FK). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/stripe.ts (1)
1528-1543:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefund handler lacks atomicity—same issue fixed in
handleEndUserTopUpSucceeded.The wallet debit, ledger insert, and margin claw-back are separate DB operations not wrapped in a transaction. This allows:
Double-debit on crash recovery: If the process crashes after line 1532 (balance decremented) but before line 1543 (ledger inserted), a webhook retry will pass the idempotency check and decrement the balance again.
Race on concurrent refunds: Two refund webhooks for the same
stripePaymentIntentIdcan both pass thealreadyReversedcheck, both read the same balance, and both decrement—double-debiting the wallet.Wrap the wallet update, ledger insert, and margin claw-back in a single transaction with the ledger insert acting as the authoritative idempotency guard (similar to the top-up handler).
🔧 Suggested fix
- const [updated] = await db - .update(tables.wallet) - .set({ balance: sql`${tables.wallet.balance} - ${reversal}` }) - .where(eq(tables.wallet.id, topUp.walletId)) - .returning(); - - await db.insert(tables.walletLedger).values({ - walletId: topUp.walletId, - endCustomerId: topUp.endCustomerId, - organizationId: topUp.organizationId, - type: "reversal", - amount: String(-reversal), - balanceAfter: updated.balance, - stripePaymentIntentId: topUp.stripePaymentIntentId, - description: "End-user top-up refund", - }); - - if (developerMargin > 0) { - await db - .update(tables.organization) - .set({ - endUserMarginBalance: sql`GREATEST(${tables.organization.endUserMarginBalance} - ${developerMargin}, 0)`, - }) - .where(eq(tables.organization.id, topUp.organizationId)); - - await db.insert(tables.transaction).values({ - organizationId: topUp.organizationId, - type: "end_user_refund", - amount: String(developerMargin), - creditAmount: String(developerMargin), - status: "completed", - stripePaymentIntentId: topUp.stripePaymentIntentId, - description: `End-user top-up refund margin claw-back (wallet ${topUp.walletId})`, - }); - } + try { + await db.transaction(async (tx) => { + const [updated] = await tx + .update(tables.wallet) + .set({ balance: sql`${tables.wallet.balance} - ${reversal}` }) + .where(eq(tables.wallet.id, topUp.walletId)) + .returning(); + + await tx.insert(tables.walletLedger).values({ + walletId: topUp.walletId, + endCustomerId: topUp.endCustomerId, + organizationId: topUp.organizationId, + type: "reversal", + amount: String(-reversal), + balanceAfter: updated.balance, + stripePaymentIntentId: topUp.stripePaymentIntentId, + description: "End-user top-up refund", + }); + + if (developerMargin > 0) { + await tx + .update(tables.organization) + .set({ + endUserMarginBalance: sql`GREATEST(${tables.organization.endUserMarginBalance} - ${developerMargin}, 0)`, + }) + .where(eq(tables.organization.id, topUp.organizationId)); + + await tx.insert(tables.transaction).values({ + organizationId: topUp.organizationId, + type: "end_user_refund", + amount: String(developerMargin), + creditAmount: String(developerMargin), + status: "completed", + stripePaymentIntentId: topUp.stripePaymentIntentId, + description: `End-user top-up refund margin claw-back (wallet ${topUp.walletId})`, + }); + } + }); + } catch (err) { + const code = + (err as { code?: string; cause?: { code?: string } })?.code ?? + (err as { cause?: { code?: string } })?.cause?.code; + if (code === "23505") { + logger.info( + `Skipping duplicate end-user refund for wallet ${topUp.walletId} (concurrent delivery for ${topUp.stripePaymentIntentId})`, + ); + return; + } + throw err; + }Note: This also requires adding a unique partial index on
wallet_ledger(stripePaymentIntentId) WHERE type='reversal'to make the ledger insert the authoritative idempotency guard (analogous to the existingtopupindex).🤖 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 `@apps/api/src/stripe.ts` around lines 1528 - 1543, The refund handler currently performs the wallet balance update, walletLedger insert, and margin claw-back as separate DB operations (see the update on tables.wallet, insert into tables.walletLedger using topUp.stripePaymentIntentId and amount String(-reversal)), which can lead to double-debits and race conditions; wrap the wallet update, ledger insert, and margin claw-back in a single database transaction (use the same DB transaction pattern used in handleEndUserTopUpSucceeded), perform the ledger.insert inside the transaction as the authoritative idempotency guard (fail the transaction if the insert conflicts), and add a unique partial DB index on wallet_ledger(stripePaymentIntentId) WHERE type='reversal' so the insert enforces idempotency for reversals.
🤖 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/shared/src/url-safety.ts`:
- Around line 50-68: The IPv6 check omits the multicast range ff00::/8, so
update the IPv6 handling block (where the variable host is inspected) to treat
addresses starting with "ff" as unsafe; add a check like host.startsWith("ff")
or a case-insensitive /^ff/ test (placed alongside the existing
ULA/link-local/loopback checks) and return true for multicast addresses so
they're treated as private/blocked before falling through to the mapped IPv4
handling or returning false.
---
Outside diff comments:
In `@apps/api/src/stripe.ts`:
- Around line 1528-1543: The refund handler currently performs the wallet
balance update, walletLedger insert, and margin claw-back as separate DB
operations (see the update on tables.wallet, insert into tables.walletLedger
using topUp.stripePaymentIntentId and amount String(-reversal)), which can lead
to double-debits and race conditions; wrap the wallet update, ledger insert, and
margin claw-back in a single database transaction (use the same DB transaction
pattern used in handleEndUserTopUpSucceeded), perform the ledger.insert inside
the transaction as the authoritative idempotency guard (fail the transaction if
the insert conflicts), and add a unique partial DB index on
wallet_ledger(stripePaymentIntentId) WHERE type='reversal' so the insert
enforces idempotency for reversals.
🪄 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: 83eb06e3-d9c4-4c41-805a-f07aaab4c4b8
📒 Files selected for processing (16)
apps/api/src/lib/end-user-session-auth.tsapps/api/src/lib/platform-secret-auth.tsapps/api/src/routes/platform-connect.tsapps/api/src/routes/platform-session-refresh.tsapps/api/src/routes/platform-sessions.tsapps/api/src/routes/platform-webhooks.tsapps/api/src/stripe.tsapps/gateway/src/lib/end-user-session.tsapps/gateway/src/videos/videos.tsapps/worker/src/worker.tspackages/db/migrations/1780768844_embeddable_hardening.sqlpackages/db/migrations/meta/1780768844_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.tspackages/shared/src/index.tspackages/shared/src/url-safety.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/db/migrations/meta/_journal.json
- apps/api/src/lib/platform-secret-auth.ts
- apps/api/src/lib/end-user-session-auth.ts
- apps/api/src/routes/platform-webhooks.ts
- apps/api/src/routes/platform-session-refresh.ts
- packages/db/src/schema.ts
- apps/gateway/src/lib/end-user-session.ts
- apps/api/src/routes/platform-connect.ts
- apps/api/src/routes/platform-sessions.ts
- apps/worker/src/worker.ts
- Squash the branch's 4 incremental migrations into a single 1780775243_embeddable_end_user_wallets migration (CI allows at most one new migration per PR). Contents are identical: all wallet/end-customer/ webhook tables, the additive columns, video_job.end_customer_wallet_id, and the unique partial topup-idempotency index. - Omit embeddable-internal columns (org margin/Stripe-Connect, project end-user settings, api-key keyType/expiresAt/endCustomerWalletId) from the Serialized* types: they aren't part of the dashboard-facing API surface, so the UI's generated client types now match (fixes the api-keys/dashboard type errors that broke build/lint/test/generate/autofix). - Regenerate apps/ui/src/lib/api/v1.d.ts to include the new platform routes (the generate job's `pnpm build && git diff --exit-code` requires it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/migrations/1780775243_embeddable_end_user_wallets.sql (1)
58-58:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd a refund-scoped idempotency key to
wallet_ledger.Line 100 only protects
type = 'topup', but the table stores no refund-level external identifier beyondstripe_payment_intent_id. That means refund/reversal inserts still cannot be deduped safely at the DB layer, so a retried refund webhook can write multiple ledger rows and drift wallet balances. Persist a refund-scoped id (for example a Stripe refund id, or another refund-event identifier) and make it unique for refund/reversal rows.Possible migration shape
CREATE TABLE "wallet_ledger" ( "id" text PRIMARY KEY, "created_at" timestamp DEFAULT now() NOT NULL, "wallet_id" text NOT NULL, "end_customer_id" text NOT NULL, "organization_id" text NOT NULL, "type" text NOT NULL, "amount" numeric NOT NULL, "balance_after" numeric NOT NULL, "gross_paid" numeric, "platform_fee" numeric, "developer_margin" numeric, "net_credited" numeric, "stripe_payment_intent_id" text, + "stripe_refund_id" text, "gateway_log_id" text, "description" text ); ... CREATE UNIQUE INDEX "wallet_ledger_topup_payment_intent_unique" ON "wallet_ledger" ("stripe_payment_intent_id") WHERE "type" = 'topup'; +CREATE UNIQUE INDEX "wallet_ledger_refund_refund_id_unique" ON "wallet_ledger" ("stripe_refund_id") WHERE "stripe_refund_id" IS NOT NULL;Also applies to: 100-100
🤖 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/db/migrations/1780775243_embeddable_end_user_wallets.sql` at line 58, Add a refund-scoped idempotency column and DB uniqueness constraint so refund/reversal ledger rows can be deduped: add a new text column (e.g. refund_external_id) to the wallet_ledger table and populate it for refund/reversal inserts, then create a partial unique index enforcing uniqueness of refund_external_id for rows where type IN ('refund','reversal') (similar to the existing topup uniqueness that uses stripe_payment_intent_id). Update any insert paths that create refund/reversal rows (wallet_ledger inserts) to set refund_external_id with the external refund identifier so retries will be rejected by the DB.
🤖 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.
Outside diff comments:
In `@packages/db/migrations/1780775243_embeddable_end_user_wallets.sql`:
- Line 58: Add a refund-scoped idempotency column and DB uniqueness constraint
so refund/reversal ledger rows can be deduped: add a new text column (e.g.
refund_external_id) to the wallet_ledger table and populate it for
refund/reversal inserts, then create a partial unique index enforcing uniqueness
of refund_external_id for rows where type IN ('refund','reversal') (similar to
the existing topup uniqueness that uses stripe_payment_intent_id). Update any
insert paths that create refund/reversal rows (wallet_ledger inserts) to set
refund_external_id with the external refund identifier so retries will be
rejected by the DB.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9df60a5b-47c4-43d2-ba0a-e2c62b12dc57
⛔ Files ignored due to path filters (1)
apps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (4)
packages/db/migrations/1780775243_embeddable_end_user_wallets.sqlpackages/db/migrations/meta/1780775243_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/types.ts
✅ Files skipped from review due to trivial changes (1)
- packages/db/migrations/meta/_journal.json
1. Margin payout idempotency key was margin_payout_<org>_<cents> in both the auto-payout worker loop and the manual /v1/connect/payouts endpoint. Two distinct payouts of the same cents value within Stripe's idempotency window would collide: Stripe replays the first transfer (no money moves) while we still debit endUserMarginBalance and record a transaction — silently losing the developer's funds. Now keyed on a fresh per-payout shortid ref (still safe for single-call SDK network retries). 2. End-user refund reversal was check-then-act with no DB guard and no transaction; duplicate/concurrent charge.refunded deliveries could double- reverse (debit the wallet twice + claw back margin twice). Added a unique partial index on wallet_ledger(stripe_payment_intent_id) WHERE type='reversal' and wrapped the reversal in a transaction that locks+re-reads the wallet for the balance clamp and catches 23505 like the top-up path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI 'Ensure at most one new migration' allows only one new .sql per PR. Fold the reversal-index migration into the feature migration by resetting migrations to main and regenerating, so the whole feature is one file again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- apps/docs: Embeddable SDK feature page (architecture, quickstart for @llmgateway/server + @llmgateway/elements + @llmgateway/client, buying credits, webhooks, payouts, security model), linking the template. - apps/ui: engineering blog post "Stripe for AI" with hero image, linking the embeddable-credits template and the docs page. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 75.8%, saving 802.7 KB.
|
Member
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 25.3%, saving 64.8 KB.
|
Member
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 14.2%, saving 27.2 KB.
|
steebchen
changed the title
This was referenced Jun 8, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backend for the embeddable "Stripe for AI" SDK: third-party developers can give their own end-users a credit wallet, AI access, and in-app top-ups through LLM Gateway. Companion SDK packages live in
llmgateway-templatesPR #7.Browser
es_...tokens now live in a dedicatedend_user_sessiontable. Each end customer gets one hidden aggregateapi_keywithkeyType = "end_user_customer", solog.apiKeyIdandapiKeyHourlyStatsrepresent the stable end user/customer rather than each short-lived browser session.log.endUserSessionIdandlog.endCustomerWalletIdkeep the concrete session and billed wallet.Changes
end_user_session; addslog.endUserSessionIdandvideoJob.endUserSessionId; uses hiddenapiKey.keyType = "end_user_customer"keys uniquely bound to one activeendCustomerWalletId./v1/sessionsensures customer, wallet, and one hidden aggregate key per end customer, then insertsend_user_session; refresh rotates session rows without creating API keys.es_...auth resolves throughend_user_session, uses the customer aggregate API key forapiKeyId, enforces session JSON scope, and logs session + wallet IDs.log.endCustomerWalletId; API-key usage counters skip session traffic; session usage counters update onend_user_session;apiKeyHourlyStatsincludes developer keys and hidden end-user customer keys, not per-session keys.@llmgateway/elementsAPI: use thetestboolean for Stripe test mode instead of asking developers to pass LLM Gateway's Stripe publishable key.Migration Notes
logtable.log.end_user_session_id,log.end_customer_wallet_id, andlog.end_customer_idcolumns.CREATE INDEX CONCURRENTLY IF NOT EXISTSand run ahead of deploy in production.Verification
pnpm formatpnpm buildpnpm push-testpnpm test:unit -- apps/api/src/routes/platform-sessions.spec.ts apps/api/src/routes/activity.spec.ts apps/gateway/src/lib/end-user-session.spec.ts apps/worker/src/log-processing.spec.ts(expanded to 111 files; 1823 passed, 2 skipped)