Implement managed proxy support for Cloudflare custom hostnames#1030
Open
goldflag wants to merge 3 commits into
Open
Implement managed proxy support for Cloudflare custom hostnames#1030goldflag wants to merge 3 commits into
goldflag wants to merge 3 commits into
Conversation
- Added environment variables for Cloudflare API integration in `.env.example`. - Introduced proxy management functionality in the API, including endpoints to enable, disable, and check the status of managed proxies for sites. - Updated site deletion logic to remove associated Cloudflare hostnames and proxy domains when a site is deleted. - Enhanced server logic to start and stop the Cloudflare reconciliation service and manage proxy domain refresh. - Updated database schema to include fields for proxy domain management, ensuring proper tracking and handling of custom hostnames.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Cloudflare-backed managed proxy support for sites: configuration and schema updates, Cloudflare client helpers, proxy enable/status/disable APIs, hostname rewriting, lifecycle reconciliation, and teardown paths for site deletion and Stripe subscription deletion. ChangesManaged Proxy Support
Sequence Diagram(s)Proxy enablement sequenceDiagram
participant enableProxy
participant createCustomHostname
participant sitesTable
participant proxyDomains
enableProxy->>createCustomHostname: create Cloudflare custom hostname
createCustomHostname-->>enableProxy: CustomHostname
enableProxy->>sitesTable: update proxyDomain, proxyEnabled, proxyStatus, proxyCfHostnameId
enableProxy->>proxyDomains: addProxyDomain(domain)
Cloudflare reconciliation sequenceDiagram
participant cloudflareReconciliationService
participant sitesTable
participant listCustomHostnames
participant deleteCustomHostname
participant proxyDomains
cloudflareReconciliationService->>sitesTable: load live proxyDomain values
cloudflareReconciliationService->>listCustomHostnames: list Cloudflare custom hostnames
cloudflareReconciliationService->>deleteCustomHostname: delete orphan hostname
cloudflareReconciliationService->>proxyDomains: removeProxyDomain(orphan.hostname)
Stripe teardown sequenceDiagram
participant teardownProxiesForStripeCustomer
participant deleteCustomHostname
participant sitesTable
participant proxyDomains
teardownProxiesForStripeCustomer->>deleteCustomHostname: delete stored Cloudflare custom hostname
teardownProxiesForStripeCustomer->>sitesTable: clear proxyDomain, proxyEnabled, proxyStatus, proxyCfHostnameId
teardownProxiesForStripeCustomer->>proxyDomains: removeProxyDomain(hostname)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 |
![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: 11
🧹 Nitpick comments (2)
server/src/services/cloudflare/cloudflareReconciliationService.ts (1)
86-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the implicit
anyforcfHostnames.
let cfHostnames;drops strict typing on the main reconcile path. Please type the variable explicitly or keep the awaited result as aconstinside thetry.Suggested fix
- let cfHostnames; + let cfHostnames: Awaited<ReturnType<typeof listCustomHostnames>>; try { cfHostnames = await listCustomHostnames();As per coding guidelines,
**/*.{ts,tsx}should "Use TypeScript with strict typing throughout both client and server".🤖 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 `@server/src/services/cloudflare/cloudflareReconciliationService.ts` around lines 86 - 88, The main reconcile path in cloudflareReconciliationService is losing strict typing because cfHostnames is declared without a type. Update the listCustomHostnames handling in the reconciliation flow to either declare cfHostnames with an explicit type or keep the awaited result as a const inside the try block, so the Cloudflare reconciliation logic stays strictly typed and avoids implicit any.Source: Coding guidelines
server/src/api/sites/proxy/getProxyStatus.ts (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the Fastify request contract fully explicit.
This handler only declares
Params, sorequest.queryandrequest.bodyremain implicit. Use an explicit route type for all three shapes to stay aligned with the API typing rule. As per coding guidelines, "Type Fastify requests with explicitParams,Querystring, andBodyshapes in route handlers."🤖 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 `@server/src/api/sites/proxy/getProxyStatus.ts` at line 11, The getProxyStatus route handler only types request.Params, leaving request.query and request.body implicit; update the FastifyRequest contract on getProxyStatus to explicitly declare Params, Querystring, and Body shapes. Use the existing getProxyStatus symbol to locate the handler and keep the route typing aligned with the API typing rule even if query/body are empty objects.Source: Coding guidelines
🤖 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 `@server/src/api/sites/deleteSite.ts`:
- Around line 16-34: The managed-proxy teardown in deleteSite is happening
before the site deletion is durable, which can leave the site row live while the
Cloudflare hostname and in-memory proxy state are already removed. Move the
deleteCustomHostname and removeProxyDomain work in deleteSite to run only after
the DB delete has succeeded/committed, or otherwise make the DB deletion the
durable step before any external cleanup. Use the existing siteRow,
deleteCustomHostname, and removeProxyDomain flow as the anchor when relocating
the cleanup.
In `@server/src/api/sites/proxy/enableProxy.ts`:
- Around line 59-97: The domain conflict check in enableProxy is only advisory
and can race, so move to a reservation/locking step before
createCustomHostname() to ensure only one request can claim a proxyDomain at a
time. Use the existing enableProxy flow around the conflicting query,
createCustomHostname(), and the sites update to atomically reserve the domain in
the database first, then proceed with Cloudflare creation only after the
reservation succeeds; if reservation fails, return the conflict response without
making the external write.
- Around line 54-103: The enableProxy flow in enableProxy.ts is overwriting an
existing proxy configuration in place without cleaning up the previous one.
Before updating the sites row, check whether the current site already has a
different proxyDomain/proxyCfHostnameId, and if so tear down the old Cloudflare
hostname and remove the old domain from the in-process proxy state before
calling createCustomHostname, db.update(sites), and addProxyDomain(domain). If
domain swaps are not meant to be supported, add a guard that rejects updates
when an active proxy already exists instead of replacing it silently.
In `@server/src/api/sites/proxy/getProxyStatus.ts`:
- Around line 12-15: Reject partially numeric siteId values by replacing the
loose parseInt-based parsing in getProxyStatus with strict validation before any
database access. Update the request.params.siteId handling so only fully numeric
positive IDs are accepted, and keep the existing 400 response path for invalid
input. Make sure the stricter check is applied consistently in getProxyStatus
and any related site-ID reads or updates that rely on the parsed siteId.
In `@server/src/api/sites/proxy/utils.ts`:
- Line 32: The hostname validation in HOSTNAME_REGEX still allows labels that
start or end with -, so update the pattern used by isValidProxyDomain() in
utils.ts to reject any dot-separated label with a leading or trailing hyphen.
Make sure the validation catches cases like a.-proxy.example and
a.proxy-.example before they reach the Cloudflare path, so the handler returns a
400 instead of failing later.
In `@server/src/db/postgres/schema.ts`:
- Around line 105-108: The proxy-domain uniqueness constraint in the schema is
currently case-sensitive, so duplicate hostnames with different casing can slip
through. Update the `sites` table definition in `schema.ts` so the invariant is
enforced case-insensitively, either by normalizing `table.proxyDomain` at the
schema boundary or by changing `sites_proxy_domain_unique` to use a
case-insensitive expression/index; make sure the
`unique(...).on(table.proxyDomain)` constraint still clearly identifies the
`proxyDomain` field as the source of truth.
In `@server/src/index.ts`:
- Around line 515-520: The server starts listening before the proxy-domain set
is fully warmed, which lets `rewriteUrl` miss managed-proxy requests right after
restart. Update `startProxyDomainRefresh()` in `server/src/lib/proxyDomains.ts`
to await the initial `refreshProxyDomains()` before returning, then keep the
interval setup after that. In `server/src/index.ts`, ensure the startup sequence
awaits `startProxyDomainRefresh()` before `server.listen(...)` so the set is
ready before traffic is served.
In `@server/src/lib/cloudflare.ts`:
- Around line 76-101: cfFetch currently lets stalled requests and low-level
fetch failures escape without normalization. Update cfFetch to use an
AbortSignal timeout for the fetch call, then wrap the fetch in try/catch and
convert network TypeError and abort/DOMException failures into CloudflareError
with an appropriate status/message. Keep the existing Cloudflare API error
handling after a successful response, and use the cfFetch symbol to locate the
change.
In `@server/src/lib/proxyDomains.ts`:
- Around line 38-52: The refreshProxyDomains() rebuild can overwrite concurrent
in-process updates from addProxyDomain() and removeProxyDomain(). Update
refreshProxyDomains() so it does not blindly clear and repopulate proxyDomains
from a stale DB snapshot after async waits; instead add a freshness
guard/version check or merge strategy that preserves newer eager changes, and
keep rewriteUrl() relying on the live set. Use the proxyDomains, addProxyDomain,
removeProxyDomain, and refreshProxyDomains symbols to locate the fix.
In `@server/src/services/cloudflare/cloudflareReconciliationService.ts`:
- Around line 160-177: The managed-proxy reconciliation path currently skips
sites with no Cloudflare hostname id and deletes the hostname before clearing
the DB state. Update cloudflareReconciliationService’s proxied-site loop to
select all rows that are still marked as managed/proxied for the organization,
persist the disabled site state in sites first, and then best-effort call
deleteCustomHostname only when proxyCfHostnameId is present. Keep the fix
localized around the proxied query and the for-of loop so the DB always reflects
disabled status even if the Cloudflare delete fails or the id is missing.
- Around line 96-103: The safety guard in CloudflareReconciliationService only
aborts when the zone size exceeds MAX_DELETES_PER_RUN, so a populated zone with
live.size === 0 can still be fully wiped on smaller deployments. Update the
reconciliation check in cloudflareReconciliationService to always stop the sweep
whenever live is empty and cfHostnames has any entries, and keep the existing
logger.error message and early return behavior tied to that guard.
---
Nitpick comments:
In `@server/src/api/sites/proxy/getProxyStatus.ts`:
- Line 11: The getProxyStatus route handler only types request.Params, leaving
request.query and request.body implicit; update the FastifyRequest contract on
getProxyStatus to explicitly declare Params, Querystring, and Body shapes. Use
the existing getProxyStatus symbol to locate the handler and keep the route
typing aligned with the API typing rule even if query/body are empty objects.
In `@server/src/services/cloudflare/cloudflareReconciliationService.ts`:
- Around line 86-88: The main reconcile path in cloudflareReconciliationService
is losing strict typing because cfHostnames is declared without a type. Update
the listCustomHostnames handling in the reconciliation flow to either declare
cfHostnames with an explicit type or keep the awaited result as a const inside
the try block, so the Cloudflare reconciliation logic stays strictly typed and
avoids implicit any.
🪄 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: aa243cbc-3023-44f2-9dda-e30d32bd9bdd
📒 Files selected for processing (15)
server/.env.exampleserver/src/api/sites/deleteSite.tsserver/src/api/sites/index.tsserver/src/api/sites/proxy/disableProxy.tsserver/src/api/sites/proxy/enableProxy.tsserver/src/api/sites/proxy/getProxyStatus.tsserver/src/api/sites/proxy/index.tsserver/src/api/sites/proxy/utils.tsserver/src/api/stripe/webhook.tsserver/src/cluster.tsserver/src/db/postgres/schema.tsserver/src/index.tsserver/src/lib/cloudflare.tsserver/src/lib/proxyDomains.tsserver/src/services/cloudflare/cloudflareReconciliationService.ts
Comment on lines
+16
to
+34
| // If this site had a managed proxy, tear down its Cloudflare hostname so we stop being | ||
| // billed. Best-effort: failures are logged and collected by the daily reconcile sweep, | ||
| // and must never block the site deletion itself. | ||
| const [siteRow] = await db | ||
| .select({ proxyDomain: sites.proxyDomain, proxyCfHostnameId: sites.proxyCfHostnameId }) | ||
| .from(sites) | ||
| .where(eq(sites.siteId, Number(id))) | ||
| .limit(1); | ||
|
|
||
| if (siteRow?.proxyCfHostnameId && isCloudflareConfigured) { | ||
| try { | ||
| await deleteCustomHostname(siteRow.proxyCfHostnameId); | ||
| } catch (error) { | ||
| logger.error(error as Error, `Failed to delete Cloudflare hostname for deleted site ${id}; reconcile will retry`); | ||
| } | ||
| } | ||
| if (siteRow?.proxyDomain) { | ||
| removeProxyDomain(siteRow.proxyDomain); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don't tear down the proxy before the site delete succeeds.
This runs before the deletion work at Lines 36-46. If one of those calls fails after deleteCustomHostname(...) succeeds, the site row can remain live while its managed-proxy hostname has already been removed from Cloudflare and from memory. The reconcile job only deletes orphans; it will not recreate the missing hostname.
Move the Cloudflare/in-memory teardown after the site delete commits, or make the DB delete and proxy-state clear the durable step before any external delete.
🤖 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 `@server/src/api/sites/deleteSite.ts` around lines 16 - 34, The managed-proxy
teardown in deleteSite is happening before the site deletion is durable, which
can leave the site row live while the Cloudflare hostname and in-memory proxy
state are already removed. Move the deleteCustomHostname and removeProxyDomain
work in deleteSite to run only after the DB delete has succeeded/committed, or
otherwise make the DB deletion the durable step before any external cleanup. Use
the existing siteRow, deleteCustomHostname, and removeProxyDomain flow as the
anchor when relocating the cleanup.
Comment on lines
+54
to
+103
| const site = await db.query.sites.findFirst({ where: eq(sites.siteId, siteId) }); | ||
| if (!site) { | ||
| return reply.status(404).send({ success: false, error: "Site not found" }); | ||
| } | ||
|
|
||
| // The hostname can back only one site. Reject if another site already claims it. | ||
| const conflicting = await db | ||
| .select({ siteId: sites.siteId }) | ||
| .from(sites) | ||
| .where(and(eq(sites.proxyDomain, domain), ne(sites.siteId, siteId))) | ||
| .limit(1); | ||
| if (conflicting.length > 0) { | ||
| return reply.status(409).send({ success: false, error: "That domain is already in use by another site" }); | ||
| } | ||
|
|
||
| // Register the custom hostname with Cloudflare. If it already exists (e.g. a leftover | ||
| // from a previous attempt), reuse it rather than failing. | ||
| let customHostname: CustomHostname; | ||
| try { | ||
| customHostname = await createCustomHostname(domain); | ||
| } catch (error) { | ||
| if (error instanceof CloudflareError && (error.code === 1406 || error.code === 1407)) { | ||
| const existing = await findCustomHostnameByName(domain); | ||
| if (!existing) { | ||
| logger.error(error, `Cloudflare reported ${domain} exists but it could not be found`); | ||
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | ||
| } | ||
| customHostname = existing; | ||
| } else { | ||
| logger.error(error as Error, `Failed to create Cloudflare custom hostname for ${domain}`); | ||
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | ||
| } | ||
| } | ||
|
|
||
| await db | ||
| .update(sites) | ||
| .set({ | ||
| proxyDomain: domain, | ||
| proxyEnabled: true, | ||
| proxyStatus: mapCloudflareStatus(customHostname), | ||
| proxyCfHostnameId: customHostname.id, | ||
| updatedAt: new Date().toISOString(), | ||
| }) | ||
| .where(eq(sites.siteId, siteId)); | ||
|
|
||
| // Make the domain serve traffic immediately in this process; other processes pick it | ||
| // up on their next refresh. | ||
| addProxyDomain(domain); | ||
|
|
||
| return reply.status(200).send({ success: true, ...buildProxyResponse(domain, siteId, customHostname) }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don't overwrite an existing proxy without tearing the old one down.
If this site already has a different proxyDomain / proxyCfHostnameId, Lines 88-97 replace that state in place. The previous Cloudflare hostname is left behind, and the old domain stays routable in this process until the next refresh because only addProxyDomain(domain) runs here.
Low-risk guard if domain swaps are not intentionally supported
const site = await db.query.sites.findFirst({ where: eq(sites.siteId, siteId) });
if (!site) {
return reply.status(404).send({ success: false, error: "Site not found" });
}
+
+ if (site.proxyEnabled && site.proxyDomain && site.proxyDomain !== domain) {
+ return reply.status(409).send({
+ success: false,
+ error: "Disable the existing proxy before assigning a new domain",
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const site = await db.query.sites.findFirst({ where: eq(sites.siteId, siteId) }); | |
| if (!site) { | |
| return reply.status(404).send({ success: false, error: "Site not found" }); | |
| } | |
| // The hostname can back only one site. Reject if another site already claims it. | |
| const conflicting = await db | |
| .select({ siteId: sites.siteId }) | |
| .from(sites) | |
| .where(and(eq(sites.proxyDomain, domain), ne(sites.siteId, siteId))) | |
| .limit(1); | |
| if (conflicting.length > 0) { | |
| return reply.status(409).send({ success: false, error: "That domain is already in use by another site" }); | |
| } | |
| // Register the custom hostname with Cloudflare. If it already exists (e.g. a leftover | |
| // from a previous attempt), reuse it rather than failing. | |
| let customHostname: CustomHostname; | |
| try { | |
| customHostname = await createCustomHostname(domain); | |
| } catch (error) { | |
| if (error instanceof CloudflareError && (error.code === 1406 || error.code === 1407)) { | |
| const existing = await findCustomHostnameByName(domain); | |
| if (!existing) { | |
| logger.error(error, `Cloudflare reported ${domain} exists but it could not be found`); | |
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | |
| } | |
| customHostname = existing; | |
| } else { | |
| logger.error(error as Error, `Failed to create Cloudflare custom hostname for ${domain}`); | |
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | |
| } | |
| } | |
| await db | |
| .update(sites) | |
| .set({ | |
| proxyDomain: domain, | |
| proxyEnabled: true, | |
| proxyStatus: mapCloudflareStatus(customHostname), | |
| proxyCfHostnameId: customHostname.id, | |
| updatedAt: new Date().toISOString(), | |
| }) | |
| .where(eq(sites.siteId, siteId)); | |
| // Make the domain serve traffic immediately in this process; other processes pick it | |
| // up on their next refresh. | |
| addProxyDomain(domain); | |
| return reply.status(200).send({ success: true, ...buildProxyResponse(domain, siteId, customHostname) }); | |
| const site = await db.query.sites.findFirst({ where: eq(sites.siteId, siteId) }); | |
| if (!site) { | |
| return reply.status(404).send({ success: false, error: "Site not found" }); | |
| } | |
| if (site.proxyEnabled && site.proxyDomain && site.proxyDomain !== domain) { | |
| return reply.status(409).send({ | |
| success: false, | |
| error: "Disable the existing proxy before assigning a new domain", | |
| }); | |
| } | |
| // The hostname can back only one site. Reject if another site already claims it. | |
| const conflicting = await db | |
| .select({ siteId: sites.siteId }) | |
| .from(sites) | |
| .where(and(eq(sites.proxyDomain, domain), ne(sites.siteId, siteId))) | |
| .limit(1); | |
| if (conflicting.length > 0) { | |
| return reply.status(409).send({ success: false, error: "That domain is already in use by another site" }); | |
| } | |
| // Register the custom hostname with Cloudflare. If it already exists (e.g. a leftover | |
| // from a previous attempt), reuse it rather than failing. | |
| let customHostname: CustomHostname; | |
| try { | |
| customHostname = await createCustomHostname(domain); | |
| } catch (error) { | |
| if (error instanceof CloudflareError && (error.code === 1406 || error.code === 1407)) { | |
| const existing = await findCustomHostnameByName(domain); | |
| if (!existing) { | |
| logger.error(error, `Cloudflare reported ${domain} exists but it could not be found`); | |
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | |
| } | |
| customHostname = existing; | |
| } else { | |
| logger.error(error as Error, `Failed to create Cloudflare custom hostname for ${domain}`); | |
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | |
| } | |
| } | |
| await db | |
| .update(sites) | |
| .set({ | |
| proxyDomain: domain, | |
| proxyEnabled: true, | |
| proxyStatus: mapCloudflareStatus(customHostname), | |
| proxyCfHostnameId: customHostname.id, | |
| updatedAt: new Date().toISOString(), | |
| }) | |
| .where(eq(sites.siteId, siteId)); | |
| // Make the domain serve traffic immediately in this process; other processes pick it | |
| // up on their next refresh. | |
| addProxyDomain(domain); | |
| return reply.status(200).send({ success: true, ...buildProxyResponse(domain, siteId, customHostname) }); |
🤖 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 `@server/src/api/sites/proxy/enableProxy.ts` around lines 54 - 103, The
enableProxy flow in enableProxy.ts is overwriting an existing proxy
configuration in place without cleaning up the previous one. Before updating the
sites row, check whether the current site already has a different
proxyDomain/proxyCfHostnameId, and if so tear down the old Cloudflare hostname
and remove the old domain from the in-process proxy state before calling
createCustomHostname, db.update(sites), and addProxyDomain(domain). If domain
swaps are not meant to be supported, add a guard that rejects updates when an
active proxy already exists instead of replacing it silently.
Comment on lines
+59
to
+97
| // The hostname can back only one site. Reject if another site already claims it. | ||
| const conflicting = await db | ||
| .select({ siteId: sites.siteId }) | ||
| .from(sites) | ||
| .where(and(eq(sites.proxyDomain, domain), ne(sites.siteId, siteId))) | ||
| .limit(1); | ||
| if (conflicting.length > 0) { | ||
| return reply.status(409).send({ success: false, error: "That domain is already in use by another site" }); | ||
| } | ||
|
|
||
| // Register the custom hostname with Cloudflare. If it already exists (e.g. a leftover | ||
| // from a previous attempt), reuse it rather than failing. | ||
| let customHostname: CustomHostname; | ||
| try { | ||
| customHostname = await createCustomHostname(domain); | ||
| } catch (error) { | ||
| if (error instanceof CloudflareError && (error.code === 1406 || error.code === 1407)) { | ||
| const existing = await findCustomHostnameByName(domain); | ||
| if (!existing) { | ||
| logger.error(error, `Cloudflare reported ${domain} exists but it could not be found`); | ||
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | ||
| } | ||
| customHostname = existing; | ||
| } else { | ||
| logger.error(error as Error, `Failed to create Cloudflare custom hostname for ${domain}`); | ||
| return reply.status(502).send({ success: false, error: "Failed to register domain with Cloudflare" }); | ||
| } | ||
| } | ||
|
|
||
| await db | ||
| .update(sites) | ||
| .set({ | ||
| proxyDomain: domain, | ||
| proxyEnabled: true, | ||
| proxyStatus: mapCloudflareStatus(customHostname), | ||
| proxyCfHostnameId: customHostname.id, | ||
| updatedAt: new Date().toISOString(), | ||
| }) | ||
| .where(eq(sites.siteId, siteId)); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Reserve the domain before creating the Cloudflare hostname.
The check on Lines 60-64 is only advisory. Two concurrent requests can both pass it and both call createCustomHostname(). If the DB later rejects one update, you've already performed a non-idempotent external write and leaked a custom hostname; if it doesn't, two sites can claim the same host.
🤖 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 `@server/src/api/sites/proxy/enableProxy.ts` around lines 59 - 97, The domain
conflict check in enableProxy is only advisory and can race, so move to a
reservation/locking step before createCustomHostname() to ensure only one
request can claim a proxyDomain at a time. Use the existing enableProxy flow
around the conflicting query, createCustomHostname(), and the sites update to
atomically reserve the domain in the database first, then proceed with
Cloudflare creation only after the reservation succeeds; if reservation fails,
return the conflict response without making the external write.
Comment on lines
+12
to
+15
| const siteId = parseInt(request.params.siteId, 10); | ||
| if (isNaN(siteId) || siteId <= 0) { | ||
| return reply.status(400).send({ success: false, error: "Invalid site ID" }); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject partially numeric siteId values.
parseInt accepts prefixes, so /sites/12abc/proxy is treated as site 12. That lets this handler read or update the wrong row on Lines 17, 43, and 51. Parse the param strictly before the database call.
Suggested fix
- const siteId = parseInt(request.params.siteId, 10);
- if (isNaN(siteId) || siteId <= 0) {
+ const rawSiteId = request.params.siteId;
+ if (!/^[1-9]\d*$/.test(rawSiteId)) {
return reply.status(400).send({ success: false, error: "Invalid site ID" });
}
+ const siteId = Number(rawSiteId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const siteId = parseInt(request.params.siteId, 10); | |
| if (isNaN(siteId) || siteId <= 0) { | |
| return reply.status(400).send({ success: false, error: "Invalid site ID" }); | |
| } | |
| const rawSiteId = request.params.siteId; | |
| if (!/^[1-9]\d*$/.test(rawSiteId)) { | |
| return reply.status(400).send({ success: false, error: "Invalid site ID" }); | |
| } | |
| const siteId = Number(rawSiteId); |
🤖 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 `@server/src/api/sites/proxy/getProxyStatus.ts` around lines 12 - 15, Reject
partially numeric siteId values by replacing the loose parseInt-based parsing in
getProxyStatus with strict validation before any database access. Update the
request.params.siteId handling so only fully numeric positive IDs are accepted,
and keep the existing 400 response path for invalid input. Make sure the
stricter check is applied consistently in getProxyStatus and any related site-ID
reads or updates that rely on the parsed siteId.
| * A first-party hostname under the customer's own domain. Requires at least one dot | ||
| * (it must be a subdomain/host, not a bare label) and stays within DNS length limits. | ||
| */ | ||
| const HOSTNAME_REGEX = /^(?=.{1,253}$)(?!-)[A-Za-z0-9-]{1,63}(?:\.[A-Za-z0-9-]{1,63})+$/; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject labels that start or end with -.
This pattern still accepts invalid hostnames like a.-proxy.example and a.proxy-.example. Those pass isValidProxyDomain(), then fail later at Cloudflare as a 502 instead of a 400 from this handler.
Suggested fix
-const HOSTNAME_REGEX = /^(?=.{1,253}$)(?!-)[A-Za-z0-9-]{1,63}(?:\.[A-Za-z0-9-]{1,63})+$/;
+const HOSTNAME_REGEX =
+ /^(?=.{1,253}$)(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)+[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?$/;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const HOSTNAME_REGEX = /^(?=.{1,253}$)(?!-)[A-Za-z0-9-]{1,63}(?:\.[A-Za-z0-9-]{1,63})+$/; | |
| const HOSTNAME_REGEX = | |
| /^(?=.{1,253}$)(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)+[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?$/; |
🤖 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 `@server/src/api/sites/proxy/utils.ts` at line 32, The hostname validation in
HOSTNAME_REGEX still allows labels that start or end with -, so update the
pattern used by isValidProxyDomain() in utils.ts to reject any dot-separated
label with a leading or trailing hyphen. Make sure the validation catches cases
like a.-proxy.example and a.proxy-.example before they reach the Cloudflare
path, so the handler returns a 400 instead of failing later.
Comment on lines
+515
to
520
| // Keep the managed-proxy hostname set warm for the rewriteUrl hook (request-serving | ||
| // process: workers, or this process in single-process mode). | ||
| startProxyDomainRefresh(); | ||
|
|
||
| // Start the server first | ||
| await server.listen({ port: 3001, host: "0.0.0.0" }); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Warm the proxy-domain set before listening.
startProxyDomainRefresh() fires the initial refresh in the background and returns immediately, but rewriteUrl depends on that set for every managed-proxy request. Right after a restart, those requests can miss the rewrite and 404 until the async warm-up finishes.
Suggested direction
- startProxyDomainRefresh();
+ await startProxyDomainRefresh();// server/src/lib/proxyDomains.ts
export async function startProxyDomainRefresh(): Promise<void> {
await refreshProxyDomains();
if (!refreshTimer) {
refreshTimer = setInterval(() => void refreshProxyDomains(), REFRESH_INTERVAL_MS);
refreshTimer.unref?.();
}
}🤖 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 `@server/src/index.ts` around lines 515 - 520, The server starts listening
before the proxy-domain set is fully warmed, which lets `rewriteUrl` miss
managed-proxy requests right after restart. Update `startProxyDomainRefresh()`
in `server/src/lib/proxyDomains.ts` to await the initial `refreshProxyDomains()`
before returning, then keep the interval setup after that. In
`server/src/index.ts`, ensure the startup sequence awaits
`startProxyDomainRefresh()` before `server.listen(...)` so the set is ready
before traffic is served.
Comment on lines
+76
to
+101
| async function cfFetch<T>(path: string, init?: RequestInit): Promise<CfEnvelope<T>> { | ||
| if (!isCloudflareConfigured) { | ||
| throw new CloudflareError("Cloudflare is not configured", 500); | ||
| } | ||
|
|
||
| const res = await fetch(`${API_BASE}${path}`, { | ||
| ...init, | ||
| headers: { | ||
| Authorization: `Bearer ${API_TOKEN}`, | ||
| "Content-Type": "application/json", | ||
| ...(init?.headers || {}), | ||
| }, | ||
| }); | ||
|
|
||
| const body = (await res.json().catch(() => null)) as CfEnvelope<T> | null; | ||
|
|
||
| if (!body || !body.success) { | ||
| const firstError = body?.errors?.[0]; | ||
| throw new CloudflareError( | ||
| firstError?.message || `Cloudflare API error (HTTP ${res.status})`, | ||
| res.status, | ||
| firstError?.code | ||
| ); | ||
| } | ||
|
|
||
| return body; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
Add a timeout and normalize transport errors in cfFetch.
The current implementation lacks a request timeout and fails to catch network-level rejections. A stalled call can hang request handlers or the reconciliation loop, and raw fetch errors escape as generic uncaught rejections instead of CloudflareError. Wrap the fetch call in a try/catch block that propagates TypeError (network failures) and DOMException (abort signals) as CloudflareError instances, and enforce a reasonable timeout using AbortSignal.
Current code
async function cfFetch<T>(path: string, init?: RequestInit): Promise<CfEnvelope<T>> {
if (!isCloudflareConfigured) {
throw new CloudflareError("Cloudflare is not configured", 500);
}
const res = await fetch(`${API_BASE}${path}`, {
...init,
headers: {
Authorization: `Bearer ${API_TOKEN}`,
"Content-Type": "application/json",
...(init?.headers || {}),
},
});
const body = (await res.json().catch(() => null)) as CfEnvelope<T> | null;
if (!body || !body.success) {
const firstError = body?.errors?.[0];
throw new CloudflareError(
firstError?.message || `Cloudflare API error (HTTP ${res.status})`,
res.status,
firstError?.code
);
}
return body;
}
🤖 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 `@server/src/lib/cloudflare.ts` around lines 76 - 101, cfFetch currently lets
stalled requests and low-level fetch failures escape without normalization.
Update cfFetch to use an AbortSignal timeout for the fetch call, then wrap the
fetch in try/catch and convert network TypeError and abort/DOMException failures
into CloudflareError with an appropriate status/message. Keep the existing
Cloudflare API error handling after a successful response, and use the cfFetch
symbol to locate the change.
Comment on lines
+38
to
+52
| export async function refreshProxyDomains(): Promise<void> { | ||
| try { | ||
| const rows = await db | ||
| .select({ proxyDomain: sites.proxyDomain }) | ||
| .from(sites) | ||
| .where(and(eq(sites.proxyEnabled, true), isNotNull(sites.proxyDomain))); | ||
|
|
||
| // Rebuild from the DB snapshot. The query above is the only await; the swap below | ||
| // is synchronous, so no request can observe a half-built set. | ||
| proxyDomains.clear(); | ||
| for (const row of rows) { | ||
| if (row.proxyDomain) { | ||
| proxyDomains.add(row.proxyDomain.toLowerCase()); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The refresh path can clobber eager add/remove updates.
If refreshProxyDomains() starts its query, then addProxyDomain() or removeProxyDomain() runs before the query resolves, Lines 47-52 rebuild the set from an older snapshot and undo the in-process change. That can temporarily drop a newly enabled domain or resurrect a just-disabled one until the next 60s poll, which breaks rewriteUrl() host matching.
One way to fence off stale refreshes
-const proxyDomains = new Set<string>();
+let proxyDomains = new Set<string>();
+let proxyDomainsVersion = 0;
export function addProxyDomain(host: string): void {
proxyDomains.add(host.toLowerCase());
+ proxyDomainsVersion += 1;
}
export function removeProxyDomain(host: string): void {
proxyDomains.delete(host.toLowerCase());
+ proxyDomainsVersion += 1;
}
export async function refreshProxyDomains(): Promise<void> {
try {
+ const versionAtStart = proxyDomainsVersion;
const rows = await db
.select({ proxyDomain: sites.proxyDomain })
.from(sites)
.where(and(eq(sites.proxyEnabled, true), isNotNull(sites.proxyDomain)));
- proxyDomains.clear();
+ const nextProxyDomains = new Set<string>();
for (const row of rows) {
if (row.proxyDomain) {
- proxyDomains.add(row.proxyDomain.toLowerCase());
+ nextProxyDomains.add(row.proxyDomain.toLowerCase());
}
}
+
+ if (versionAtStart === proxyDomainsVersion) {
+ proxyDomains = nextProxyDomains;
+ }
} catch (error) {🤖 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 `@server/src/lib/proxyDomains.ts` around lines 38 - 52, The
refreshProxyDomains() rebuild can overwrite concurrent in-process updates from
addProxyDomain() and removeProxyDomain(). Update refreshProxyDomains() so it
does not blindly clear and repopulate proxyDomains from a stale DB snapshot
after async waits; instead add a freshness guard/version check or merge strategy
that preserves newer eager changes, and keep rewriteUrl() relying on the live
set. Use the proxyDomains, addProxyDomain, removeProxyDomain, and
refreshProxyDomains symbols to locate the fix.
Comment on lines
+96
to
+103
| // Defensive guard: an empty live set against a populated zone almost certainly means | ||
| // a bad read, not that every customer churned at once. Refuse to wipe the zone. | ||
| if (live.size === 0 && cfHostnames.length > this.MAX_DELETES_PER_RUN) { | ||
| this.logger.error( | ||
| `Reconciliation found 0 live hostnames but ${cfHostnames.length} in Cloudflare — aborting as a safety measure.` | ||
| ); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Abort any populated-zone sweep when the live set is empty.
The current guard only aborts when Cloudflare has more than MAX_DELETES_PER_RUN hostnames. On a smaller deployment, one bad DB read that returns zero live rows would still delete every active hostname in the zone.
Suggested fix
- if (live.size === 0 && cfHostnames.length > this.MAX_DELETES_PER_RUN) {
+ if (live.size === 0 && cfHostnames.length > 0) {
this.logger.error(
`Reconciliation found 0 live hostnames but ${cfHostnames.length} in Cloudflare — aborting as a safety measure.`
);
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Defensive guard: an empty live set against a populated zone almost certainly means | |
| // a bad read, not that every customer churned at once. Refuse to wipe the zone. | |
| if (live.size === 0 && cfHostnames.length > this.MAX_DELETES_PER_RUN) { | |
| this.logger.error( | |
| `Reconciliation found 0 live hostnames but ${cfHostnames.length} in Cloudflare — aborting as a safety measure.` | |
| ); | |
| return; | |
| } | |
| // Defensive guard: an empty live set against a populated zone almost certainly means | |
| // a bad read, not that every customer churned at once. Refuse to wipe the zone. | |
| if (live.size === 0 && cfHostnames.length > 0) { | |
| this.logger.error( | |
| `Reconciliation found 0 live hostnames but ${cfHostnames.length} in Cloudflare — aborting as a safety measure.` | |
| ); | |
| return; | |
| } |
🤖 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 `@server/src/services/cloudflare/cloudflareReconciliationService.ts` around
lines 96 - 103, The safety guard in CloudflareReconciliationService only aborts
when the zone size exceeds MAX_DELETES_PER_RUN, so a populated zone with
live.size === 0 can still be fully wiped on smaller deployments. Update the
reconciliation check in cloudflareReconciliationService to always stop the sweep
whenever live is empty and cfHostnames has any entries, and keep the existing
logger.error message and early return behavior tied to that guard.
Comment on lines
+160
to
+177
| const proxied = await db | ||
| .select({ | ||
| siteId: sites.siteId, | ||
| proxyDomain: sites.proxyDomain, | ||
| proxyCfHostnameId: sites.proxyCfHostnameId, | ||
| }) | ||
| .from(sites) | ||
| .where(and(inArray(sites.organizationId, orgIds), isNotNull(sites.proxyCfHostnameId))); | ||
|
|
||
| for (const site of proxied) { | ||
| try { | ||
| if (site.proxyCfHostnameId) { | ||
| await deleteCustomHostname(site.proxyCfHostnameId); | ||
| } | ||
| await db | ||
| .update(sites) | ||
| .set({ proxyDomain: null, proxyEnabled: false, proxyStatus: null, proxyCfHostnameId: null }) | ||
| .where(eq(sites.siteId, site.siteId)); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Disable the site in the DB even when the Cloudflare hostname id is missing, and do that before the delete call.
This path only visits rows with proxyCfHostnameId, so any churned site stuck with proxyEnabled/proxyDomain but no hostname id is skipped entirely. It also deletes in Cloudflare before clearing sites, so a DB write failure leaves the site marked live even though the hostname is already gone.
Select all managed-proxy rows for the customer, persist the disabled state first, then best-effort delete the Cloudflare hostname when an id 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 `@server/src/services/cloudflare/cloudflareReconciliationService.ts` around
lines 160 - 177, The managed-proxy reconciliation path currently skips sites
with no Cloudflare hostname id and deletes the hostname before clearing the DB
state. Update cloudflareReconciliationService’s proxied-site loop to select all
rows that are still marked as managed/proxied for the organization, persist the
disabled site state in sites first, and then best-effort call
deleteCustomHostname only when proxyCfHostnameId is present. Keep the fix
localized around the proxied query and the for-of loop so the DB always reflects
disabled status even if the Cloudflare delete fails or the id is missing.
- Added new translation keys and refined existing entries in Czech, German, English, Spanish, French, Italian, Japanese, and other languages to enhance user experience with API functionalities. - Included translations for filters, parameters, and error handling, ensuring comprehensive coverage of new features related to API interactions and data management. - Improved consistency and clarity across all supported languages, supporting the latest updates in the application.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@client/src/components/SiteSettings/CustomDomainTab.tsx`:
- Around line 103-105: The CustomDomainTab state check is too broad and treats
an omitted configured field as false. Update the conditional in the
CustomDomainTab component to only enter the “not configured” branch when
configured is explicitly false, or otherwise make sure the API always returns
configured for valid responses. Use the existing data?.configured check in the
component to locate and tighten this logic.
- Line 4: The CustomDomainTab component is importing the wrong next-intl hook
for runtime user-facing text. Replace useExtracted with useTranslations in the
CustomDomainTab.tsx import and update the component to use the standard
useTranslations() hook for all translated strings, keeping the existing
translation keys and usage consistent.
🪄 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: d0910491-7dd8-413e-a5ed-88efcc358e86
📒 Files selected for processing (17)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/uk.jsonclient/messages/zh.jsonclient/src/api/admin/endpoints/index.tsclient/src/api/admin/endpoints/proxy.tsclient/src/api/admin/hooks/useProxy.tsclient/src/components/SiteSettings/CustomDomainTab.tsxclient/src/components/SiteSettings/SiteSettings.tsx
✅ Files skipped from review due to trivial changes (1)
- client/src/api/admin/endpoints/index.ts
| "use client"; | ||
|
|
||
| import { Check, Copy, Globe, Loader2, RefreshCw, ShieldAlert } from "lucide-react"; | ||
| import { useExtracted } from "next-intl"; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does next-intl v4.8.3 export a hook named useExtracted, and what is the recommended hook for component translations?
💡 Result:
Yes, next-intl v4.8.3 exports an experimental hook named useExtracted [1][2][3]. The recommended hook for component translations is useTranslations [4][5]. Additional context: The useExtracted hook is an experimental feature designed to automate message extraction from source files [1][2]. It integrates with the Next.js build process (via Turbopack or Webpack) to compile calls into useTranslations during development and build time [1][2]. Because it is experimental, it must be explicitly enabled in your next.config.ts file [1]. The standard and primary hook for consuming translations within React components remains useTranslations, which is the established API for most use cases [4][5][6].
Citations:
- 1: https://next-intl.dev/docs/usage/extraction
- 2: https://next-intl.dev/blog/use-extracted
- 3: https://github.com/amannn/next-intl/blob/main/CHANGELOG.md
- 4: https://next-intl.dev/docs/usage/translations
- 5: https://github.com/amannn/next-intl/blob/main/docs/src/pages/docs/usage/translations.mdx
- 6: https://next-intl.dev/docs/usage
Replace useExtracted with useTranslations.
The project guidelines require using useTranslations() for user-facing strings. While useExtracted is an available experimental hook in next-intl, it is intended for build-time message extraction and is not the standard API for component translations.
import { useTranslations } from "next-intl";🤖 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 `@client/src/components/SiteSettings/CustomDomainTab.tsx` at line 4, The
CustomDomainTab component is importing the wrong next-intl hook for runtime
user-facing text. Replace useExtracted with useTranslations in the
CustomDomainTab.tsx import and update the component to use the standard
useTranslations() hook for all translated strings, keeping the existing
translation keys and usage consistent.
Source: Coding guidelines
Comment on lines
+103
to
+105
| // Feature isn't enabled on this server (no Cloudflare credentials configured). | ||
| if (!data?.configured) { | ||
| return ( |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t treat missing configured as “not configured.”
if (!data?.configured) will show the “not configured” state when configured is omitted/undefined, even for valid proxy responses. Use an explicit check (data?.configured === false) or align the API contract to always send configured.
Suggested fix
- if (!data?.configured) {
+ if (data?.configured === false) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Feature isn't enabled on this server (no Cloudflare credentials configured). | |
| if (!data?.configured) { | |
| return ( | |
| // Feature isn't enabled on this server (no Cloudflare credentials configured). | |
| if (data?.configured === false) { | |
| return ( |
🤖 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 `@client/src/components/SiteSettings/CustomDomainTab.tsx` around lines 103 -
105, The CustomDomainTab state check is too broad and treats an omitted
configured field as false. Update the conditional in the CustomDomainTab
component to only enter the “not configured” branch when configured is
explicitly false, or otherwise make sure the API always returns configured for
valid responses. Use the existing data?.configured check in the component to
locate and tighten this logic.
…uration details - Updated the response structure to include `configured` and `cnameTarget` fields, providing clearer information about Cloudflare settings. - Improved the handling of proxy status reporting, ensuring accurate responses based on the current configuration and status of the site. - Refactored the logic to maintain consistency in the API response format, enhancing clarity for users interacting with the proxy status endpoint.
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
1 participant
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.
.env.example.Summary by CodeRabbit