Renew and charge .link domain#3882
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds an admin domain-renewal flow: a client form posts normalized domains to a new admin API which creates a renewal invoice and starts a Stripe PaymentIntent; the charge-succeeded webhook extends expiry from the later of current expiry or today. Also adjusts Stripe descriptor handling and an error log. ChangesDomain Renewal Feature
Sequence Diagram(s)sequenceDiagram
participant AdminUI as Admin Dashboard (RenewDomain)
participant AdminAPI as POST /api/admin/renew-domain
participant Prisma as Prisma DB
participant Stripe as Stripe (PaymentIntent)
AdminUI->>AdminAPI: POST { domain }
AdminAPI->>Prisma: lookup registeredDomain, check processing invoices
AdminAPI->>Prisma: create domainRenewal invoice (transaction)
AdminAPI->>Stripe: createPaymentIntent(idempotencyKey, amount)
Stripe-->>AdminAPI: PaymentIntent (status)
AdminAPI-->>AdminUI: JSON { ok, message, invoiceId?, paymentIntentStatus? }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/dynadot/set-renew-option.ts (1)
52-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not swallow renewal failures in a payment-adjacent path.
At Line 52, the catch logs the error but then returns normally. This can let upstream logic proceed as if renewal succeeded, causing charge/renewal inconsistency. Re-throw (or return an explicit failure result) after logging.
Suggested fix
} catch (error) { await log({ message: `Failed to set renew option for ${domain}: ${error instanceof Error ? error.message : String(error)}`, type: "errors", mention: true, }); console.error(error); + throw error instanceof Error + ? error + : new Error(`Failed to set renew option for ${domain}`); }🤖 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/web/lib/dynadot/set-renew-option.ts` around lines 52 - 60, The catch in set-renew-option.ts (inside the setRenewOption / rename-renewal logic) currently logs failures but swallows the error, letting upstream code assume success; after the log/console.error in the catch block, re-throw the caught error (or return a clearly failing result object) so callers receive the failure. Locate the catch block that logs `Failed to set renew option for ${domain}` and change it to rethrow the original error (or return an explicit failure sentinel) immediately after logging so payment/renewal flow can handle the failure upstream.
🧹 Nitpick comments (4)
apps/web/app/(ee)/api/admin/renew-domain/route.ts (2)
47-102: 🏗️ Heavy liftTOCTOU on pending-invoice check and a race in invoice-number generation.
Two concerns live in the same flow:
- The duplicate-renewal guard (lines 47–72) reads pending invoices and then creates a new one — two concurrent admin requests for the same slug can both pass the check before either commits, producing two parallel
processinginvoices and two PaymentIntents for the same domain.- The invoice number is computed as
count + 1inside$transaction(lines 82–88). Under Prisma's/Postgres' defaultREAD COMMITTEDisolation, concurrent transactions can read the same count and emit duplicate invoice numbers for the workspace.Both are low-probability in an admin-only single-user flow but are correctness hazards. Consider:
- A unique partial index on
(workspaceId, slug)forprocessing domainRenewalinvoices, and rely on a unique-violation to reject duplicates atomically.- Generating invoice numbers from a Postgres sequence or a unique constraint on
(workspaceId, number)with retry-on-conflict.🤖 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/web/app/`(ee)/api/admin/renew-domain/route.ts around lines 47 - 102, The current flow reads pending invoices with prisma.invoice.findMany and checks hasPendingForSlug, then creates a new invoice inside prisma.$transaction where the invoice number is computed from tx.invoice.count — both steps are vulnerable to TOCTOU and concurrent-transaction races; fix by adding a DB-side guard and a retry-on-conflict path: (1) add a unique partial index (or unique constraint) in the DB for processing domainRenewal per workspace/slug to atomically prevent duplicate processing invoices and propagate unique-violation errors to the app, and (2) stop computing invoice numbers via count and instead use a database sequence or enforce uniqueness on (workspaceId, number) and implement a retry loop around the tx that catches unique-violation errors from tx.invoice.create (or number-constraint violations) to regenerate the number and retry creating the invoice; update error handling around createId/invoice.create to handle and surface conflicts gracefully.
14-17: ⚡ Quick winUse
parsed.error.issues[0]?.messageinstead ofparsed.error.messagefor validation errors.In Zod 4, accessing
error.messagedirectly returns a JSON-stringified issues array, not a user-friendly error string. The codebase usesparsed.error.issues[0]?.messageelsewhere (e.g., invite-partner-sheet.tsx) to extract the first issue's message. This API route should follow the same pattern for consistency.♻️ Proposed fix
const parsed = schema.safeParse(await req.json()); if (!parsed.success) { - return NextResponse.json({ error: parsed.error.message }, { status: 400 }); + return NextResponse.json( + { error: parsed.error.issues[0]?.message ?? "Invalid request body." }, + { status: 400 }, + ); }🤖 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/web/app/`(ee)/api/admin/renew-domain/route.ts around lines 14 - 17, The validation error response uses parsed.error.message which in Zod v4 yields a JSON string of issues; change the response to use the first issue message instead: read parsed.error.issues[0]?.message and return that (falling back to a generic message if undefined) in the NextResponse.json call where schema.safeParse(await req.json()) is handled so the API route returns a user-friendly validation string.apps/web/app/(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx (1)
11-39: 💤 Low valueReset the input on success to prevent accidental double-charges.
After a successful submission, the typed domain is still in the input and the form remains submittable — clicking again will fire another renewal attempt. The server's pending-invoice check (409) mitigates duplicate charges, but resetting on success is a cheap UX guard against repeated submissions while the first webhook is still in flight.
♻️ Proposed change (use a ref or pass the form to reset)
<form - action={async (data) => { + action={async (data) => { const res = await fetch("/api/admin/renew-domain", { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify({ domain: data.get("domain"), }), }); const json = await res.json(); if (!res.ok) { toast.error(json.error ?? "Request failed"); return; } toast.success(json.message ?? "Renewal initiated", { description: json.invoiceId ? `Invoice: ${json.invoiceId}${ json.paymentIntentStatus ? ` · Payment status: ${json.paymentIntentStatus}` : "" }` : undefined, }); + // Clear the input so the admin doesn't accidentally resubmit + const form = document.getElementById("renew-domain")?.closest("form"); + (form as HTMLFormElement | null)?.reset(); }} >🤖 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/web/app/`(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx around lines 11 - 39, The form leaves the domain input populated after a successful submission, allowing accidental duplicate renewals; update the <form> in renew-domain.tsx to reset the inputs on success by attaching a ref to the <form> element (use useRef<HTMLFormElement | null>), assign ref={formRef} to the <form>, and call formRef.current?.reset() immediately after the toast.success call (or before returning) so the Form component's input is cleared; reference the existing async form action block and the <Form /> component when making the change.apps/web/lib/stripe/create-payment-intent.ts (1)
54-55: 💤 Low valueFix incorrect assumption about
statement_descriptor_suffixlength constraint.The 22-character limit applies to the entire concatenated descriptor (account prefix + "*" + space + suffix), not the suffix alone. Slicing to 22 characters will cause Stripe to truncate the suffix to fit the total budget. For example, with an account prefix of 10 characters, the suffix is effectively limited to ~10 characters. Instead of a fixed 22-char slice, trim the descriptor based on the account's configured statement descriptor prefix length (e.g.,
max_suffix_length = 22 - len(prefix) - 2). Alternatively, document that callers must pass concise values (e.g., short business names) that fit the actual constraint after accounting for the prefix.🤖 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/web/lib/stripe/create-payment-intent.ts` around lines 54 - 55, The current fixed slice to 22 chars for statement_descriptor_suffix is wrong because Stripe limits the entire concatenated descriptor (prefix + "*" + " " + suffix) to 22 chars; update the code in create-payment-intent (where statement_descriptor_suffix is set) to compute max_suffix_length = 22 - (prefix.length) - 2 and then slice the provided statementDescriptor to that length (or clamp to 0), using the account's configured statement descriptor prefix (e.g., statement_descriptor_prefix) to determine prefix.length; alternatively add a clear comment or validation that callers must provide a short statementDescriptor that fits after the prefix rather than slicing to 22 unconditionally.
🤖 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/web/app/`(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx:
- Around line 49-62: The input in the RenewDomain component is marked
aria-invalid="true" and lacks a label; remove the hardcoded aria-invalid
attribute and instead bind aria-invalid to the actual validation state (e.g.,
aria-invalid={Boolean(formErrors.domain) || isInvalid}) or omit it until
validation exists, and add a <label htmlFor="renew-domain"> (either visible or
visually-hidden) that references the id "renew-domain" so the input has an
associated label; update any validation state/variable names you have (e.g.,
formErrors, isInvalid, pending) and keep the existing id/name
("renew-domain"/"domain") and className usage (cn(...)) when making these
changes.
In `@apps/web/app/`(ee)/admin.dub.co/(dashboard)/page.tsx:
- Around line 48-53: Update the user-facing paragraph in page.tsx (the <p
className="text-sm text-neutral-500"> node) to remove the extra article and
clarify which dashboard to use: change "already expired at the Dynadot" to
"already expired at Dynadot" and replace "renew it in the dashboard" with "renew
it in the admin dashboard" so the copy reads clearly and points to the correct
place.
---
Outside diff comments:
In `@apps/web/lib/dynadot/set-renew-option.ts`:
- Around line 52-60: The catch in set-renew-option.ts (inside the setRenewOption
/ rename-renewal logic) currently logs failures but swallows the error, letting
upstream code assume success; after the log/console.error in the catch block,
re-throw the caught error (or return a clearly failing result object) so callers
receive the failure. Locate the catch block that logs `Failed to set renew
option for ${domain}` and change it to rethrow the original error (or return an
explicit failure sentinel) immediately after logging so payment/renewal flow can
handle the failure upstream.
---
Nitpick comments:
In `@apps/web/app/`(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx:
- Around line 11-39: The form leaves the domain input populated after a
successful submission, allowing accidental duplicate renewals; update the <form>
in renew-domain.tsx to reset the inputs on success by attaching a ref to the
<form> element (use useRef<HTMLFormElement | null>), assign ref={formRef} to the
<form>, and call formRef.current?.reset() immediately after the toast.success
call (or before returning) so the Form component's input is cleared; reference
the existing async form action block and the <Form /> component when making the
change.
In `@apps/web/app/`(ee)/api/admin/renew-domain/route.ts:
- Around line 47-102: The current flow reads pending invoices with
prisma.invoice.findMany and checks hasPendingForSlug, then creates a new invoice
inside prisma.$transaction where the invoice number is computed from
tx.invoice.count — both steps are vulnerable to TOCTOU and
concurrent-transaction races; fix by adding a DB-side guard and a
retry-on-conflict path: (1) add a unique partial index (or unique constraint) in
the DB for processing domainRenewal per workspace/slug to atomically prevent
duplicate processing invoices and propagate unique-violation errors to the app,
and (2) stop computing invoice numbers via count and instead use a database
sequence or enforce uniqueness on (workspaceId, number) and implement a retry
loop around the tx that catches unique-violation errors from tx.invoice.create
(or number-constraint violations) to regenerate the number and retry creating
the invoice; update error handling around createId/invoice.create to handle and
surface conflicts gracefully.
- Around line 14-17: The validation error response uses parsed.error.message
which in Zod v4 yields a JSON string of issues; change the response to use the
first issue message instead: read parsed.error.issues[0]?.message and return
that (falling back to a generic message if undefined) in the NextResponse.json
call where schema.safeParse(await req.json()) is handled so the API route
returns a user-friendly validation string.
In `@apps/web/lib/stripe/create-payment-intent.ts`:
- Around line 54-55: The current fixed slice to 22 chars for
statement_descriptor_suffix is wrong because Stripe limits the entire
concatenated descriptor (prefix + "*" + " " + suffix) to 22 chars; update the
code in create-payment-intent (where statement_descriptor_suffix is set) to
compute max_suffix_length = 22 - (prefix.length) - 2 and then slice the provided
statementDescriptor to that length (or clamp to 0), using the account's
configured statement descriptor prefix (e.g., statement_descriptor_prefix) to
determine prefix.length; alternatively add a clear comment or validation that
callers must provide a short statementDescriptor that fits after the prefix
rather than slicing to 22 unconditionally.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: da57a132-65b9-43d5-9cff-60850ace39fb
📒 Files selected for processing (6)
apps/web/app/(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsxapps/web/app/(ee)/admin.dub.co/(dashboard)/page.tsxapps/web/app/(ee)/api/admin/renew-domain/route.tsapps/web/app/(ee)/api/stripe/webhook/charge-succeeded.tsapps/web/lib/dynadot/set-renew-option.tsapps/web/lib/stripe/create-payment-intent.ts
|
@coderabbitai full review. any breaking changes or security issues? |
🧩 Analysis chain
🏁 Script executed: Length of output: 113462
🔒 Security Issues1. [HIGH] PaymentIntent
|
| # | Severity | Issue |
|---|---|---|
| 1 | 🔴 High | PaymentIntent client_secret returned in API response |
| 2 | 🟠 Medium | TOCTOU race allows duplicate invoice+charge creation |
| 3 | 🟠 Medium | Invoice number collision under concurrent requests |
| 4 | 🟡 Medium | statement_descriptor_suffix changes visible bank statement text |
| 5 | 🟡 Low | Expiry renewal base change is a semantic behavioral shift |
| 6 | 🟢 Low | aria-invalid="true" hardcoded on domain input |
The most urgent fix before merge is #1 — remove paymentIntentClientSecret from the response, as the PI is already confirmed server-side and the secret is not needed by the admin UI.
✅ Actions performed
Full review triggered.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apps/web/app/(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx (1)
48-62:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a label and stop hardcoding the field as invalid.
aria-invalid="true"makes assistive tech announce this input as invalid on first render, and the field still has no associated label. Please either wirearia-invalidto real validation state or remove it until validation exists, and add a visible orsr-onlylabel forrenew-domain.🤖 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/web/app/`(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx around lines 48 - 62, The input with id="renew-domain" is hardcoded as invalid and lacks a label; remove the static aria-invalid="true" or wire it to a real validation state (e.g., aria-invalid={isInvalid}) coming from component state/props, and add a visible or screen-reader-only <label htmlFor="renew-domain"> (or a Label component) associated with name="domain"/id="renew-domain" so the field is announced correctly and only reported invalid when validation actually fails; also ensure any styling that depends on pending or validation (the cn(...) usage) reflects the new isInvalid state.
🤖 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/web/app/`(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx:
- Around line 12-35: The form action handler (the async function passed to
action used in the component) currently assumes fetch and res.json() always
succeed; wrap the whole submit flow in a try/catch, perform the POST to
"/api/admin/renew-domain" inside that try, and parse the response body
defensively (try res.json(), fall back to res.text() or a default object if JSON
parsing fails) before checking res.ok; on network or parse errors call
toast.error with a clear fallback message and on success call toast.success with
the same message/invoice/paymentIntentStatus logic but using the safely parsed
data.
In `@apps/web/app/`(ee)/api/admin/renew-domain/route.ts:
- Around line 89-95: The current invoiceNumber creation reads tx.invoice.count
and adds 1 (totalInvoices, invoiceNumber), which is race-prone; change it to a
DB-backed monotonic counter/sequence per workspace instead: add or use a
dedicated counter (e.g., workspace.invoiceCounter or a DB sequence keyed by
workspaceId) and atomically increment it inside the same transaction (use a row
lock / UPDATE ... RETURNING or the DB sequence) to produce the next sequential
number, then format with registered.project.invoicePrefix and padStart; update
the code paths that currently use totalInvoices/invoiceNumber to fetch the
returned incremented counter value instead of counting rows.
- Around line 47-109: The current duplicate-check (pendingInvoices /
hasPendingForSlug) is racey because it runs outside the transaction; move the
“is there already an active renewal for this domain?” check inside the same
prisma.$transaction that creates the invoice (use tx.invoice.findFirst or count
filtering by workspaceId, type:"domainRenewal", status:"processing" and
registeredDomains containing registered.slug) and abort the transaction if
found, then create the invoice in that same tx (refer to prisma.$transaction,
tx.invoice.findFirst, tx.invoice.create, registered.slug, registered.projectId).
Additionally add a DB-enforced invariant: introduce a domain-specific unique
constraint (e.g., add a domain_slug column or a new InvoiceDomain table and
create a UNIQUE partial index for status='processing' per workspace/domain) so
concurrent transactions cannot both succeed even if the application-level check
races.
- Around line 150-159: The response in the renew-domain route returns
paymentIntent.client_secret and can leak a sensitive secret while not handling
requires_action; update the handler (the code that builds the NextResponse.json
returning invoiceId, paymentIntentStatus and paymentIntent.client_secret) to NOT
include paymentIntent.client_secret for non-succeeded payment intents — instead
treat any intent with status !== 'succeeded' as a failure and return success:
false (or a clear error) so the client won’t receive the client_secret;
alternatively, if you prefer to support client-side confirmation, implement an
explicit Stripe.js confirmation flow triggered when paymentIntent.status ===
'requires_action' and only return the client_secret when you also implement that
frontend confirmation path.
---
Duplicate comments:
In `@apps/web/app/`(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsx:
- Around line 48-62: The input with id="renew-domain" is hardcoded as invalid
and lacks a label; remove the static aria-invalid="true" or wire it to a real
validation state (e.g., aria-invalid={isInvalid}) coming from component
state/props, and add a visible or screen-reader-only <label
htmlFor="renew-domain"> (or a Label component) associated with
name="domain"/id="renew-domain" so the field is announced correctly and only
reported invalid when validation actually fails; also ensure any styling that
depends on pending or validation (the cn(...) usage) reflects the new isInvalid
state.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d76e605-4764-4443-b97d-a6e0b7df90bb
📒 Files selected for processing (6)
apps/web/app/(ee)/admin.dub.co/(dashboard)/components/renew-domain.tsxapps/web/app/(ee)/admin.dub.co/(dashboard)/page.tsxapps/web/app/(ee)/api/admin/renew-domain/route.tsapps/web/app/(ee)/api/stripe/webhook/charge-succeeded.tsapps/web/lib/dynadot/set-renew-option.tsapps/web/lib/stripe/create-payment-intent.ts
1 participant
Summary by CodeRabbit
New Features
Bug Fixes