From 5a8d21904deff9d454c9d7a2e54f22120ebe8124 Mon Sep 17 00:00:00 2001
From: Waleed <walif6@gmail.com>
Date: Fri, 24 Jul 2026 11:32:27 -0700
Subject: [PATCH] fix(sso): surface DNS verification failures and the provider
 auto-append gotcha (#5931)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(sso): surface DNS verification failures and the provider auto-append gotcha

Post-merge audit follow-ups for verified domains (#5909):

- The host field handed admins the FQDN `_sim-challenge.acme.com`. GoDaddy,
  Namecheap, Hover and most cPanel panels append the zone to whatever is typed,
  yielding `_sim-challenge.acme.com.acme.com` — the record looks right in their
  panel but never verifies, and our 422 tells them to wait 48 hours. Add a hint
  under the field (and a docs callout) telling those admins to enter just the
  label.
- DNS failures were logged at debug, but production log level is ERROR, so a
  blocked-egress or SERVFAIL condition was invisible to us and misreported to the
  admin as "record not found yet". Log infrastructure-class failures at warn with
  the DNS error code; keep the genuinely-absent codes at debug.
- Trim the joined TXT value before comparing: several DNS panels pad the stored
  string, which otherwise fails an exact match forever.
- Lower the resolver to 2s/1 try. c-ares multiplies timeout across servers and
  retries by ~7x, so the previous 5s/2-try config could block a verify request
  for ~35s when resolvers are unreachable.
- Cover `checkDomainTxtRecord` — the function that decides whether the gate opens
  had no tests. Adds exact match, chunk-joined value, match among unrelated
  records, padded value, near-miss, another org's token, absent record,
  infrastructure failure, and empty-response cases.

* fix(sso): log DNS infrastructure failures at error so prod actually surfaces them

Production's default minimum log level is ERROR, so the warn introduced in the
previous commit was still filtered out — the fault stayed invisible exactly as
before. A resolver failure that is not 'record absent' is a genuine
infrastructure error, so ERROR is both the visible and the honest severity.

* fix(sso): state the zone-removal rule instead of a wrong subdomain hint

The hint computed the bare label as the first segment of the challenge host, so
for a subdomain like eng.acme.com it advised entering `_sim-challenge` when the
host relative to the acme.com zone is `_sim-challenge.eng` — following it would
publish the record on the wrong name and verification would never succeed, the
exact failure the hint exists to prevent. Deriving the real zone needs the Public
Suffix List (acme.co.uk defeats naive label-stripping), so state the rule instead:
enter the host with the trailing zone removed. Docs show both the apex and the
subdomain form.
---
 .../platform/enterprise/verified-domains.mdx  |  6 +-
 .../sim/ee/sso/components/domain-settings.tsx | 10 ++-
 .../lib/auth/sso/domain-verification.test.ts  | 81 ++++++++++++++++++-
 apps/sim/lib/auth/sso/domain-verification.ts  | 47 +++++++++--
 4 files changed, 132 insertions(+), 12 deletions(-)

diff --git a/apps/docs/content/docs/en/platform/enterprise/verified-domains.mdx b/apps/docs/content/docs/en/platform/enterprise/verified-domains.mdx
index d56dc3230c..fe91e17e83 100644
--- a/apps/docs/content/docs/en/platform/enterprise/verified-domains.mdx
+++ b/apps/docs/content/docs/en/platform/enterprise/verified-domains.mdx
@@ -23,7 +23,11 @@ Go to **Settings → Security → Verified domains** in your organization settin
 3. Add that TXT record at your DNS provider.
 4. Click **Verify**. Sim looks up the record; on success the domain is marked **Verified**.
 
-DNS changes can take up to 48 hours to propagate — if verification does not succeed immediately, wait and retry. You can remove the TXT record after the domain is verified; the verification persists.
+<Callout type="warning">
+  Some DNS providers — GoDaddy, Namecheap, Hover, and most cPanel panels — append your zone to whatever you type in the host field. If yours does, enter the host with the trailing zone removed, or you will end up with `_sim-challenge.acme.com.acme.com` and verification will never succeed. Managing the `acme.com` zone, `_sim-challenge.acme.com` becomes `_sim-challenge`; verifying the subdomain `eng.acme.com` from that same zone, `_sim-challenge.eng.acme.com` becomes `_sim-challenge.eng`. Cloudflare and Route 53 take the full host as shown.
+</Callout>
+
+DNS changes can take up to 48 hours to propagate — if verification does not succeed immediately, wait and retry. Keep the TXT record published: leaving it in place means the domain stays verifiable if you ever need to verify it again.
 
 Add each domain you own separately. Subdomains (`eng.acme.com`) are verified independently of the apex.
 
diff --git a/apps/sim/ee/sso/components/domain-settings.tsx b/apps/sim/ee/sso/components/domain-settings.tsx
index 4185d96aeb..04f10e6a8d 100644
--- a/apps/sim/ee/sso/components/domain-settings.tsx
+++ b/apps/sim/ee/sso/components/domain-settings.tsx
@@ -21,13 +21,15 @@ interface DomainSettingsProps {
 interface CopyFieldProps {
   label: string
   value: string
+  hint?: string
 }
 
-function CopyField({ label, value }: CopyFieldProps) {
+function CopyField({ label, value, hint }: CopyFieldProps) {
   return (
     <div className='flex flex-col gap-1'>
       <span className='text-[var(--text-muted)] text-caption'>{label}</span>
       <ChipCopyInput value={value} copyLabel={`Copy ${label}`} inputClassName='font-mono' />
+      {hint ? <span className='text-[var(--text-muted)] text-caption'>{hint}</span> : null}
     </div>
   )
 }
@@ -71,7 +73,11 @@ function DomainRow({ organizationId, domain, onRemove }: DomainRowProps) {
             Add this TXT record at your DNS provider, then verify. DNS changes can take up to 48
             hours to propagate.
           </p>
-          <CopyField label='Host / name' value={domain.challengeHost} />
+          <CopyField
+            label='Host / name'
+            value={domain.challengeHost}
+            hint='Some DNS providers append your zone automatically. If yours does, enter this host with the trailing zone removed.'
+          />
           <CopyField label='Value' value={domain.txtRecordValue} />
           <div>
             <Button size='sm' onClick={handleVerify} disabled={verifyDomain.isPending}>
diff --git a/apps/sim/lib/auth/sso/domain-verification.test.ts b/apps/sim/lib/auth/sso/domain-verification.test.ts
index 3781924c13..e44dfcc8f3 100644
--- a/apps/sim/lib/auth/sso/domain-verification.test.ts
+++ b/apps/sim/lib/auth/sso/domain-verification.test.ts
@@ -1,10 +1,24 @@
 /**
  * @vitest-environment node
  */
-import { describe, expect, it } from 'vitest'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockResolveTxt, mockSetServers } = vi.hoisted(() => ({
+  mockResolveTxt: vi.fn(),
+  mockSetServers: vi.fn(),
+}))
+
+vi.mock('node:dns/promises', () => ({
+  Resolver: class {
+    resolveTxt = mockResolveTxt
+    setServers = mockSetServers
+  },
+}))
+
 import {
   buildChallengeHost,
   buildTxtRecordValue,
+  checkDomainTxtRecord,
   generateVerificationToken,
   SSO_CHALLENGE_HOST_PREFIX,
   toDomainResponse,
@@ -76,4 +90,69 @@ describe('domain-verification helpers', () => {
       expect(verified.verifiedAt).toBe(verifiedAt.toISOString())
     })
   })
+
+  describe('checkDomainTxtRecord', () => {
+    const TOKEN = 'tok-123'
+    const EXPECTED = buildTxtRecordValue(TOKEN)
+
+    beforeEach(() => {
+      vi.clearAllMocks()
+    })
+
+    it('queries the challenge host for the domain', async () => {
+      mockResolveTxt.mockResolvedValue([[EXPECTED]])
+      await checkDomainTxtRecord('acme.com', TOKEN)
+      expect(mockResolveTxt).toHaveBeenCalledWith('_sim-challenge.acme.com')
+    })
+
+    it('verifies when the exact value is published', async () => {
+      mockResolveTxt.mockResolvedValue([[EXPECTED]])
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(true)
+    })
+
+    it('joins a value split across 255-char chunks before comparing', async () => {
+      const midpoint = Math.floor(EXPECTED.length / 2)
+      mockResolveTxt.mockResolvedValue([[EXPECTED.slice(0, midpoint), EXPECTED.slice(midpoint)]])
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(true)
+    })
+
+    it('finds the match among unrelated TXT records on the same host', async () => {
+      mockResolveTxt.mockResolvedValue([
+        ['v=spf1 include:_spf.google.com ~all'],
+        ['facebook-domain-verification=abc123'],
+        [EXPECTED],
+      ])
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(true)
+    })
+
+    it('tolerates padding a DNS panel added around the value', async () => {
+      mockResolveTxt.mockResolvedValue([[`  ${EXPECTED}  `]])
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(true)
+    })
+
+    it('rejects a near-miss value (no partial or prefix match)', async () => {
+      mockResolveTxt.mockResolvedValue([[`${EXPECTED}extra`], [EXPECTED.slice(0, -1)]])
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(false)
+    })
+
+    it('rejects another org token published on the same host', async () => {
+      mockResolveTxt.mockResolvedValue([[buildTxtRecordValue('someone-elses-token')]])
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(false)
+    })
+
+    it('returns false (never throws) when the record is absent', async () => {
+      mockResolveTxt.mockRejectedValue(Object.assign(new Error('no data'), { code: 'ENODATA' }))
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(false)
+    })
+
+    it('returns false (never throws) when resolution fails for an infrastructure reason', async () => {
+      mockResolveTxt.mockRejectedValue(Object.assign(new Error('timeout'), { code: 'ETIMEOUT' }))
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(false)
+    })
+
+    it('returns false when the host has no TXT records at all', async () => {
+      mockResolveTxt.mockResolvedValue([])
+      await expect(checkDomainTxtRecord('acme.com', TOKEN)).resolves.toBe(false)
+    })
+  })
 })
diff --git a/apps/sim/lib/auth/sso/domain-verification.ts b/apps/sim/lib/auth/sso/domain-verification.ts
index 76391991c8..79e8d1ef57 100644
--- a/apps/sim/lib/auth/sso/domain-verification.ts
+++ b/apps/sim/lib/auth/sso/domain-verification.ts
@@ -54,14 +54,29 @@ const TXT_VALUE_PREFIX = 'sim-domain-verification='
  * depend on (or get poisoned by) the host's local resolver/split-horizon DNS. */
 const VERIFICATION_NAMESERVERS = ['1.1.1.1', '8.8.8.8']
 
-const DNS_TIMEOUT_MS = 5000
+/**
+ * Per-attempt timeout. c-ares multiplies this across servers and retries by
+ * more than the nominal `tries` (measured ~7x with two servers), so keep the
+ * base low: 2s x 1 try over two servers bounds a fully-unreachable-resolver
+ * lookup at a few seconds rather than the ~35s a 5s/2-try config produced.
+ */
+const DNS_TIMEOUT_MS = 2000
+
+/**
+ * DNS error codes that genuinely mean "the record is not published yet" — the
+ * expected state while an admin is still adding it. Anything else (timeout,
+ * refused, SERVFAIL) indicates an infrastructure problem on our side and is
+ * logged loudly, because it is otherwise indistinguishable to the admin from a
+ * missing record.
+ */
+const RECORD_ABSENT_DNS_CODES = new Set(['ENODATA', 'ENOTFOUND', 'NXDOMAIN'])
 
 /**
  * Shared resolver pinned to the public nameservers. Its config is fully static
  * and `resolveTxt` is safe to call concurrently, so a single module-scope
  * instance avoids re-allocating one per verification.
  */
-const verificationResolver = new Resolver({ timeout: DNS_TIMEOUT_MS, tries: 2 })
+const verificationResolver = new Resolver({ timeout: DNS_TIMEOUT_MS, tries: 1 })
 verificationResolver.setServers(VERIFICATION_NAMESERVERS)
 
 /** The fully-qualified host an org must create the TXT record on. */
@@ -95,13 +110,29 @@ export async function checkDomainTxtRecord(domain: string, token: string): Promi
 
   try {
     const records = await verificationResolver.resolveTxt(host)
-    // Each TXT record may be split into multiple strings — join the chunks.
-    return records.some((chunks) => chunks.join('') === expected)
+    // Each TXT record may be split into 255-char chunks — join before comparing.
+    // Trim the joined value: several DNS panels pad the stored string, which
+    // would otherwise fail an exact match forever with no way for the admin to
+    // tell why. Concatenation happens first, so trimming cannot corrupt a
+    // legitimate chunk boundary.
+    return records.some((chunks) => chunks.join('').trim() === expected)
   } catch (error) {
-    logger.debug('TXT verification lookup failed (treated as unverified)', {
-      host,
-      error: getErrorMessage(error),
-    })
+    const code = (error as NodeJS.ErrnoException)?.code
+    if (code && RECORD_ABSENT_DNS_CODES.has(code)) {
+      logger.debug('TXT verification record not published yet', { host, code })
+    } else {
+      // Not a missing record — our resolver path itself is failing (blocked
+      // egress, timeout, SERVFAIL). Log at ERROR, not warn: the default minimum
+      // level in production is ERROR, so anything below it is dropped and the
+      // fault stays invisible while the admin is told their record "isn't
+      // published yet". This is a genuine infrastructure fault, so ERROR is also
+      // the honest severity.
+      logger.error('TXT verification lookup failed for an infrastructure reason', {
+        host,
+        code,
+        error: getErrorMessage(error),
+      })
+    }
     return false
   }
 }