Repository navigation
feat(publish): custom domain model + settings UI + DNS instructions (PR1) - #1695
Conversation
…PR1) - Pure core: normalizeHostname, validateCustomDomain, buildDnsInstructions in packages/lib/src/validators/custom-domain.ts with 29 unit tests (apex/subdomain heuristic, RFC-1123 charset/length, pagespace.* block) - DB: custom_domains table (id, driveId FK cascade, hostname unique, status pending/verified/failed, createdAt); migration 0168 - API: GET/POST /api/drives/[driveId]/domains + DELETE .../[domainId] — owner/admin gated, CSRF on writes, 409 on duplicate, audited 21 route contract tests (mocked DB) - UI: Custom Domains card added to drive Settings → General; add/remove domains, DNS instructions (A+AAAA for apex, CNAME for sub) from NEXT_PUBLIC_PUBLISH_EDGE_IPV4/IPV6/CNAME_TARGET env vars, status badge with 'pending DNS verification' affordance - Exports: @pagespace/lib/validators/custom-domain, @pagespace/db/schema/custom-domains added to package.json DNS verify, Fly cert provisioning, Caddy routing: deferred to PR2-4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LCj8wakYJbhBcyH9cn93Sa
📝 WalkthroughWalkthroughAdds end-to-end custom domain management for drives: a new ChangesCustom Domains Feature
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over User,SWR: Add Domain
User->>CustomDomainsCard: types hostname + clicks Add
CustomDomainsCard->>handleAddDomain: hostname string
handleAddDomain->>normalizeHostname: canonicalize input
handleAddDomain->>validateCustomDomain: check hostname validity
validateCustomDomain-->>handleAddDomain: valid or toast 400
handleAddDomain->>API: POST /api/drives/:driveId/domains
API->>AuthLayer: authenticateRequestWithOptions (CSRF)
AuthLayer-->>API: principal or 401/403
API->>customDomainsDB: insert { driveId, hostname }
customDomainsDB-->>API: row or unique constraint error
API->>auditRequest: add-custom-domain event
API-->>handleAddDomain: 201 / 409 / 500
handleAddDomain->>SWR: mutateDomains()
SWR-->>CustomDomainsCard: refreshed domain list
end
rect rgba(205, 92, 92, 0.5)
Note over User,SWR: Remove Domain
User->>DomainRow: clicks Delete
DomainRow->>handleRemoveDomain: domainId
handleRemoveDomain->>API: DELETE /api/drives/:driveId/domains/:domainId
API->>customDomainsDB: delete where id + driveId
customDomainsDB-->>API: deleted row or 404
API->>auditRequest: remove-custom-domain event
API-->>handleRemoveDomain: 200 { deleted: true } or error
handleRemoveDomain->>SWR: mutateDomains()
SWR-->>CustomDomainsCard: refreshed domain list
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be7ea2c78c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isAddingDomain) return; | ||
| setIsAddingDomain(true); | ||
| try { | ||
| await fetchWithAuth(`/api/drives/${driveId}/domains`, { |
There was a problem hiding this comment.
When the POST returns 409/400/403, fetchWithAuth only resolves to a Response rather than throwing (unlike the post/fetchJSON helpers used elsewhere), so this path still clears the input, revalidates, and shows the success toast. In the duplicate-hostname case from the new API, users are told the domain was added even though the server rejected it; check response.ok or use the JSON helper before showing success.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the latest commit. handleAddDomain now checks response.ok before clearing the input or showing the success toast. On failure it reads the JSON error body and surfaces: "That domain is already registered" for 409, the server error message for other 4xx, and a generic fallback for network errors.
| status: text('status', { enum: ['pending', 'verified', 'failed'] }).default('pending').notNull(), | ||
| createdAt: timestamp('created_at', { mode: 'date' }).defaultNow().notNull(), | ||
| }, (table) => ({ | ||
| hostnameKey: uniqueIndex('custom_domains_hostname_key').on(table.hostname), |
There was a problem hiding this comment.
Because every inserted row starts as pending and this index is global, any drive admin can add someone-elses-domain.com without proving DNS control and the unique constraint will block the real owner from adding it to their drive; the only custom_domains writes in this change are add/delete, so there is no verifier or cleanup path to release those claims. Make the uniqueness conditional on verified ownership or require a verification token before reserving the hostname.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is a known limitation acknowledged by design for PR1. The unique constraint on hostname uses a first-claim model — standard for domain verification flows (e.g., how GitHub Pages and Vercel work). A squatted hostname can be trivially reclaimed once DNS verification lands in PR2: we can simply add a policy to release pending claims older than a TTL (e.g. 24h) or require the user to prove control via a TXT record before the unique constraint is enforced. Changing the uniqueness scheme in PR1 would require DNS-verification logic to be correct and safe, which is the explicit scope of PR2. Leaving this as-is per the epic plan.
| } | ||
|
|
||
| // Extract the leftmost label as the CNAME name (e.g. "www" from "www.acme.com"). | ||
| const name = hostname.split('.')[0]; |
There was a problem hiding this comment.
For accepted deep subdomains like docs.blog.acme.com, taking only hostname.split('.')[0] tells a user managing the acme.com zone to create docs CNAME ..., which points docs.acme.com rather than the requested hostname. The CNAME name needs to include the full relative host portion (or display the FQDN) so DNS setup works for multi-label subdomains.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Changed from hostname.split(".")[0] to labels.slice(0, -2).join(".") so docs.blog.acme.com correctly produces name=docs.blog rather than just docs. Added a test case for this: preserves all subdomain labels for deep subdomains.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/src/app/api/drives/`[driveId]/domains/route.ts:
- Around line 63-66: The `await request.json()` call can throw an error when the
JSON is malformed, and this error is not caught, resulting in a 500 response
instead of the appropriate 400. Wrap the `await request.json()` call in a
try-catch block to handle JSON parsing errors before Zod validation with
`addDomainSchema.safeParse()` runs. When a JSON parsing error is caught, return
a NextResponse.json with a 400 status code and an appropriate error message.
Apply this same fix to all similar locations in the file where request bodies
are parsed (also at lines 93-99).
In `@apps/web/src/app/dashboard/`[driveId]/settings/general/page.tsx:
- Around line 401-407: The Input component for the domain field and the
icon-only delete buttons lack proper accessibility labels for screen readers.
Add an aria-label attribute to the Input element that describes its purpose
(e.g., "Enter custom domain"), and add aria-label attributes to all icon-only
delete action buttons (the ones referenced at lines 469-476) to describe their
function (e.g., "Remove domain"). This ensures screen reader users can
understand the purpose of these interactive elements.
In `@packages/db/src/schema/custom-domains.ts`:
- Line 10: The status field in the custom-domains schema currently uses text
with an enum option for TypeScript type safety, but does not enforce the
constraint at the database layer. To fix this, use pgEnum from Drizzle ORM to
create a PostgreSQL enum type for the status values ('pending', 'verified',
'failed'), then reference this enum in the status field definition instead of
using text with the enum option. This will ensure that the database itself
enforces the constraint and prevents invalid status values from being inserted
directly at the database level.
In `@packages/lib/src/validators/custom-domain.ts`:
- Around line 147-151: The current implementation of extracting the CNAME name
from hostname only takes the leftmost label using hostname.split('.')[0], which
loses information for deep subdomains. Instead of taking only the first element,
modify the logic to extract all labels except the last one (the root domain),
then rejoin them with dots to preserve the full subdomain path. This ensures
that for a hostname like docs.blog.acme.io, the name becomes docs.blog rather
than just docs.
🪄 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: 3616765c-882b-41ce-9150-d47e24060a01
📒 Files selected for processing (14)
apps/web/src/app/api/drives/[driveId]/domains/[domainId]/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/[domainId]/route.tsapps/web/src/app/api/drives/[driveId]/domains/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/route.tsapps/web/src/app/dashboard/[driveId]/settings/general/page.tsxpackages/db/drizzle/0168_smart_eddie_brock.sqlpackages/db/drizzle/meta/0168_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/package.jsonpackages/db/src/schema.tspackages/db/src/schema/custom-domains.tspackages/lib/package.jsonpackages/lib/src/validators/__tests__/custom-domain.test.tspackages/lib/src/validators/custom-domain.ts
- Fix CNAME name for deep subdomains: use labels.slice(0,-2).join('.')
instead of split('.')[0] so docs.blog.acme.com → name='docs.blog'
- Add test case for deep-subdomain CNAME preservation
- Use pgEnum('custom_domain_status') for DB-level constraint enforcement;
regenerate migration 0168 with CREATE TYPE ... AS ENUM(...)
- Wrap request.json() in try-catch for 400 on malformed JSON bodies
- Fix handleAddDomain: check response.ok and surface server error messages
(409 → 'already registered', other 4xx → server error text, network → fallback)
- Add accessibility: sr-only Label[for=new-custom-domain] on hostname input,
aria-label='Remove domain {hostname}' on icon-only delete button
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LCj8wakYJbhBcyH9cn93Sa
- Use pg error code '23505' for unique constraint detection (matches existing pattern in reactions/calendar routes; more reliable than string match) - Remove unreachable z.ZodError catch (safeParse never throws) - Simplify LABEL_PATTERN: second alternate was redundant — first arm already matches single-char labels via optional group - Remove unused driveId prop from CustomDomainsCard interface Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LCj8wakYJbhBcyH9cn93Sa
1 participant
Summary
PR1 of 5 in the Custom Domain + SSL Provisioning epic. Lays the full data/API/UI foundation; DNS verification, Fly cert provisioning, Caddy routing, and canonical/OG wiring are deferred to PR2–5.
packages/lib/src/validators/custom-domain.ts):normalizeHostname(strips scheme/path/port/trailing-dot),validateCustomDomain(RFC-1123 charset+length, rejectspagespace.ai/pagespace.site/*.pagespace.*),buildDnsInstructions(apex → A + AAAA, 3+-label → CNAME, all subdomain labels preserved vialabels.slice(0,-2).join('.')). 30 unit tests.packages/db/src/schema/custom-domains.ts):custom_domainstable —id,driveId(FK → drives, cascade),hostname(globally unique, normalized),status(pgEnum:pending/verified/failed, defaultpending),createdAt. Migration0168.apps/web/src/app/api/drives/[driveId]/domains/):GETlist +POSTadd (+DELETEat/[domainId]). Owner/admin gated, CSRF on writes, 409 on duplicate hostname (Postgres error code23505), audited. 21 route contract tests (mocked DB).NEXT_PUBLIC_PUBLISH_EDGE_IPV4/IPV6/CNAME_TARGETenv vars. Status badge readsPending DNSwith copy noting verification comes in a later PR. Malformed-JSON body → 400; non-OK response → toast with server error text.Acceptance criteria
db:generate;@pagespace/db/schema/custom-domains+@pagespace/lib/validators/custom-domainexports addedbun run lint✅ ·bun run typecheck✅ ·bun run test:unit(new tests) ✅ ·bun run build✅Out of scope (later PRs)
DNS verification/polling, Fly cert API, Caddy routing config, canonical/OG/sitemap host changes.
Test plan
docs.example.com→ verify it appears withPending DNSbadge and DNS row shows CNAME instructionexample.com→ verify DNS row shows A + AAAA instructionsacme.pagespace.site→ verify rejected client-side with validation error🤖 Generated with Claude Code
https://claude.ai/code/session_01LCj8wakYJbhBcyH9cn93Sa