fix(desktop): stop the OAuth connect callback from failing on a bare-path callback URL

apps/sim/app/desktop/connect/connect-launcher.tsx

@@ -8,8 +8,13 @@ import { DesktopHandoffShell } from '@/app/desktop/components/desktop-handoff-sh
 
 interface ConnectLauncherProps {
   providerId: string
-  /** Same-origin path better-auth returns the browser to after the callback. */
-  completePath: string
+  /**
+   * Absolute URL better-auth returns the browser to after the callback. Better
+   * Auth stores it verbatim in the OAuth state and the callback reads the
+   * credential draft back off it, so a bare path would be parsed without an
+   * origin — keep this a full URL, as every other connect surface passes.
+   */
+  completeUrl: string
 }
 
 /**
@@ -19,7 +24,7 @@ interface ConnectLauncherProps {
  * leaves for the provider immediately, so the UI is just a brief interstitial
  * plus an error state with retry.
  */
-export function ConnectLauncher({ providerId, completePath }: ConnectLauncherProps) {
+export function ConnectLauncher({ providerId, completeUrl }: ConnectLauncherProps) {
   const startedRef = useRef(false)
   const [error, setError] = useState<string | null>(null)
 
@@ -28,18 +33,18 @@ export function ConnectLauncher({ providerId, completePath }: ConnectLauncherPro
     try {
       await client.oauth2.link({
         providerId,
-        callbackURL: completePath,
+        callbackURL: completeUrl,
         // Failed flows bounce to the same complete page (which forwards the
         // failure to the loopback) instead of waiting out the handoff TTL.
         // Do NOT bake in a query param here: better-auth appends its own
         // `&error=<code>`, and a second `error` key deserializes to an array
         // that the complete page can't read — so it would look like success.
-        errorCallbackURL: completePath,
+        errorCallbackURL: completeUrl,
       })
     } catch (err) {
       setError(getErrorMessage(err, 'Could not start the connection.'))
     }
-  }, [providerId, completePath])
+  }, [providerId, completeUrl])
 
   useEffect(() => {
     if (startedRef.current) return

apps/sim/app/desktop/connect/page.test.tsx

@@ -0,0 +1,160 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockGetSession, mockRedirect, baseUrl } = vi.hoisted(() => ({
+  mockGetSession: vi.fn(),
+  mockRedirect: vi.fn((url: string) => {
+    throw new Error(`NEXT_REDIRECT:${url}`)
+  }),
+  /** Mutable so a test can give the deployment a trailing-slash base URL. */
+  baseUrl: { value: 'https://sim.test' },
+}))
+
+vi.mock('@/lib/auth', () => ({
+  auth: { api: { getSession: mockGetSession } },
+  getSession: vi.fn(),
+}))
+
+vi.mock('@/lib/auth/auth-client', () => ({
+  client: { oauth2: { link: vi.fn() } },
+  signOut: vi.fn(),
+}))
+
+vi.mock('@/lib/core/utils/urls', () => ({
+  getBaseUrl: () => baseUrl.value,
+}))
+
+/** Keeps the landing-page barrel the real shell pulls in out of this graph. */
+vi.mock('@/app/desktop/components/desktop-handoff-shell', () => ({
+  DesktopHandoffShell: () => null,
+}))
+
+vi.mock('next/navigation', () => ({
+  redirect: mockRedirect,
+}))
+
+vi.mock('next/headers', () => ({
+  headers: vi.fn(async () => new Headers()),
+}))
+
+import DesktopConnectPage from '@/app/desktop/connect/page'
+
+const VALID_STATE = 'a'.repeat(32)
+const PORT = '57979'
+
+function pageProps(params: Record<string, string>) {
+  return { searchParams: Promise.resolve(params) }
+}
+
+async function renderPage(params: Record<string, string>) {
+  const result = (await DesktopConnectPage(pageProps(params))) as unknown as {
+    type: { name: string }
+    props: Record<string, unknown>
+  }
+  return result
+}
+
+describe('DesktopConnectPage', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    baseUrl.value = 'https://sim.test'
+    mockGetSession.mockResolvedValue({ user: { id: 'user-1', email: 'user@example.com' } })
+  })
+
+  it('hands the launcher an absolute complete URL so the callback can read the draft back', async () => {
+    // Better Auth stores `callbackURL` verbatim, and the OAuth callback parses it
+    // with `new URL`. A bare path threw there, failing the whole callback with a
+    // 500 after the provider had already authorized.
+    const result = await renderPage({
+      provider: 'google-email',
+      state: VALID_STATE,
+      port: PORT,
+      draftId: 'draft-1',
+    })
+
+    expect(result.type.name).toBe('ConnectLauncher')
+    expect(result.props.providerId).toBe('google-email')
+
+    const completeUrl = new URL(result.props.completeUrl as string)
+    expect(completeUrl.origin).toBe('https://sim.test')
+    expect(completeUrl.pathname).toBe('/desktop/connect/complete')
+    expect(completeUrl.searchParams.get('state')).toBe(VALID_STATE)
+    expect(completeUrl.searchParams.get('port')).toBe(PORT)
+    expect(completeUrl.searchParams.get('credentialDraftId')).toBe('draft-1')
+  })
+
+  it('keeps the complete URL absolute when no draft rides along', async () => {
+    const result = await renderPage({
+      provider: 'google-email',
+      state: VALID_STATE,
+      port: PORT,
+    })
+
+    expect(result.type.name).toBe('ConnectLauncher')
+    expect(() => new URL(result.props.completeUrl as string)).not.toThrow()
+  })
+
+  it('keeps the completion route intact when the deployment base URL has a trailing slash', async () => {
+    // `//desktop/connect/complete` matches no route, so the provider result
+    // would never reach the loopback and the connect would hang.
+    baseUrl.value = 'https://sim.test/'
+
+    const launcher = await renderPage({
+      provider: 'google-email',
+      state: VALID_STATE,
+      port: PORT,
+    })
+    expect(new URL(launcher.props.completeUrl as string).pathname).toBe('/desktop/connect/complete')
+
+    await expect(
+      DesktopConnectPage(
+        pageProps({
+          provider: 'google-email',
+          state: VALID_STATE,
+          port: PORT,
+          workspaceId: 'workspace-1',
+        })
+      )
+    ).rejects.toThrow('NEXT_REDIRECT:')
+    const callbackUrl = new URL(mockRedirect.mock.calls[0][0]).searchParams.get('callbackURL')
+    expect(new URL(callbackUrl as string).pathname).toBe('/desktop/connect/complete')
+  })
+
+  it('sends a workspace-scoped connect to the authorize route with an absolute callback', async () => {
+    await expect(
+      DesktopConnectPage(
+        pageProps({
+          provider: 'google-email',
+          state: VALID_STATE,
+          port: PORT,
+          workspaceId: 'workspace-1',
+        })
+      )
+    ).rejects.toThrow('NEXT_REDIRECT:')
+
+    const authorize = new URL(mockRedirect.mock.calls[0][0])
+    expect(authorize.pathname).toBe('/api/auth/oauth2/authorize')
+    expect(authorize.searchParams.get('providerId')).toBe('google-email')
+    expect(authorize.searchParams.get('workspaceId')).toBe('workspace-1')
+    expect(authorize.searchParams.get('callbackURL')).toBe(
+      `https://sim.test/desktop/connect/complete?state=${VALID_STATE}&port=${PORT}`
+    )
+  })
+
+  it('rejects a malformed request without reading the session', async () => {
+    const invalid = [
+      { provider: 'Google', state: VALID_STATE, port: PORT },
+      { provider: 'google-email', state: 'short', port: PORT },
+      { provider: 'google-email', state: VALID_STATE },
+      { provider: 'google-email', state: VALID_STATE, port: PORT, draftId: 'bad draft' },
+    ]
+
+    for (const params of invalid) {
+      const result = await renderPage(params)
+      expect(result.type.name).toBe('InvalidRequest')
+    }
+    expect(mockGetSession).not.toHaveBeenCalled()
+  })
+})

apps/sim/app/desktop/connect/page.tsx

@@ -34,6 +34,17 @@ function InvalidRequest() {
   )
 }
 
+/**
+ * Absolute URL better-auth returns the browser to once the OAuth callback is
+ * done. Composed through the URL API rather than concatenated, so a trailing
+ * slash on `NEXT_PUBLIC_APP_URL` cannot yield a `//desktop/...` pathname that
+ * matches no route — this page is what bounces the result to the app's
+ * loopback, so a base-URL typo would otherwise strand the whole flow.
+ */
+function buildConnectCompleteUrl(state: string, port: number, draftId?: string): string {
+  return new URL(buildConnectCompletePath(state, port, draftId), getBaseUrl()).toString()
+}
+
 /**
  * Desktop OAuth-connect landing. The desktop app opens this page in the
  * system browser with the provider to connect, a one-time state, and the port
@@ -112,10 +123,7 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
     const authorize = new URL('/api/auth/oauth2/authorize', getBaseUrl())
     authorize.searchParams.set('providerId', providerId)
     authorize.searchParams.set('workspaceId', workspaceId)
-    authorize.searchParams.set(
-      'callbackURL',
-      `${getBaseUrl()}${buildConnectCompletePath(state, port)}`
-    )
+    authorize.searchParams.set('callbackURL', buildConnectCompleteUrl(state, port))
     if (credentialId) {
       authorize.searchParams.set('credentialId', credentialId)
     }
@@ -125,7 +133,7 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
   return (
     <ConnectLauncher
       providerId={providerId}
-      completePath={buildConnectCompletePath(state, port, draftId)}
+      completeUrl={buildConnectCompleteUrl(state, port, draftId)}
     />
   )
 }

apps/sim/lib/core/utils/internal-api-base-url.test.ts

@@ -12,7 +12,15 @@
  * @vitest-environment node
  */
 import { resetEnvMock, setEnv } from '@sim/testing'
-import { afterEach, describe, expect, it } from 'vitest'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+
+/**
+ * `vitest.setup.ts` mocks this module globally with a hand-written mirror, so
+ * without this the suite would assert against that mirror rather than the
+ * function it names — and any drift between the two would pass unnoticed.
+ */
+vi.unmock('@/lib/core/utils/urls')
+
 import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
 
 const PUBLIC_URL = 'https://sim.ai'
@@ -33,6 +41,17 @@ describe('getInternalApiBaseUrl', () => {
     expect(getInternalApiBaseUrl()).toBe(LOOPBACK)
   })
 
+  /** Callers concatenate `${base}/api/...`, exactly as they do with getBaseUrl(). */
+  it('strips a trailing slash from the internal URL', () => {
+    setEnv({
+      INTERNAL_API_BASE_URL: `${LOOPBACK}/`,
+      NEXT_PUBLIC_APP_URL: PUBLIC_URL,
+      DB_APP_NAME: 'sim',
+    })
+
+    expect(getInternalApiBaseUrl()).toBe(LOOPBACK)
+  })
+
   it('IGNORES the internal URL on a Trigger.dev worker and falls back to the public URL', () => {
     setEnv({
       INTERNAL_API_BASE_URL: LOOPBACK,

apps/sim/lib/core/utils/urls.test.ts

@@ -56,6 +56,43 @@ describe('getBaseUrl', () => {
     expect(getBaseUrl()).toBe('https://app.example.com')
   })
 
+  /**
+   * Call sites build `${getBaseUrl()}/path`, so a trailing slash would give them
+   * a `//path` pathname that matches no route — and would break the
+   * `startsWith(`${base}/`)` prefix checks that decide whether a redirect target
+   * is our own, silently sending those redirects to their fallback instead.
+   */
+  it('strips trailing slashes so concatenated paths stay single-slashed', () => {
+    for (const configured of ['https://app.example.com/', 'https://app.example.com///']) {
+      mockGetEnv.mockImplementation((key) =>
+        key === 'NEXT_PUBLIC_APP_URL' ? configured : undefined
+      )
+      expect(getBaseUrl()).toBe('https://app.example.com')
+      expect(new URL(`${getBaseUrl()}/desktop/connect/complete`).pathname).toBe(
+        '/desktop/connect/complete'
+      )
+    }
+  })
+
+  /**
+   * Pins the trim's shape — it must not eat more than the trailing slashes.
+   * Not a claim that a path-prefixed deployment works: the app declares no Next
+   * `basePath`, so such a value could not address its routes either way.
+   */
+  it('trims only trailing slashes, never interior ones', () => {
+    mockGetEnv.mockImplementation((key) =>
+      key === 'NEXT_PUBLIC_APP_URL' ? 'https://example.com/a/b/' : undefined
+    )
+    expect(getBaseUrl()).toBe('https://example.com/a/b')
+  })
+
+  it('adds the protocol and strips the trailing slash together', () => {
+    mockGetEnv.mockImplementation((key) =>
+      key === 'NEXT_PUBLIC_APP_URL' ? 'app.example.com/' : undefined
+    )
+    expect(getBaseUrl()).toBe('http://app.example.com')
+  })
+
   /**
    * Never guesses from `window.location.origin`: an opaque origin (a sandboxed
    * iframe) serializes to the truthy string `'null'`, which would silently

apps/sim/lib/core/utils/urls.ts

@@ -12,13 +12,26 @@ function hasHttpProtocol(url: string): boolean {
   return /^https?:\/\//i.test(url)
 }
 
+/**
+ * Brings a configured base URL to the no-trailing-slash form {@link SITE_URL}
+ * documents: adds the protocol when the operator omitted it, then strips
+ * trailing slashes.
+ *
+ * Call sites overwhelmingly build URLs as `${base}/path`, so a base spelled
+ * `https://host/` gives every one of them a `//path` pathname that matches no
+ * route, and breaks the `startsWith(`${base}/`)` prefix checks that decide
+ * whether a redirect target is our own. Normalizing once here is what lets
+ * those call sites stay simple instead of each defending against the operator's
+ * spelling.
+ *
+ * Trailing slashes are the only spelling this absorbs. The app declares no Next
+ * `basePath`, so its routes are served at the origin root and a path-prefixed
+ * value could not address them however this normalized it.
+ */
 function normalizeBaseUrl(url: string): string {
-  if (hasHttpProtocol(url)) {
-    return url
-  }
-
   const protocol = isProd ? 'https://' : 'http://'
-  return `${protocol}${url}`
+  const withProtocol = hasHttpProtocol(url) ? url : `${protocol}${url}`
+  return withProtocol.replace(/\/+$/, '')
 }
 
 /**
@@ -89,7 +102,9 @@ export function getInternalApiBaseUrl(): string {
     )
   }
 
-  return internalBaseUrl
+  // Protocol is proven present above, so this only trims trailing slashes —
+  // callers concatenate `${base}/api/...` exactly as they do with getBaseUrl().
+  return normalizeBaseUrl(internalBaseUrl)
 }
 
 /**

apps/sim/lib/credentials/draft-processor.test.ts

@@ -119,11 +119,23 @@ describe('parseCredentialDraftIdFromCallbackUrl', () => {
     ).toBe('draft-1')
   })
 
+  it('reads the relative callback URL Better Auth documents and stores verbatim', () => {
+    expect(
+      parseCredentialDraftIdFromCallbackUrl(
+        '/desktop/connect/complete?state=abc&port=57979&credentialDraftId=draft-1'
+      )
+    ).toBe('draft-1')
+    expect(
+      parseCredentialDraftIdFromCallbackUrl('/desktop/connect/complete?state=abc&port=57979')
+    ).toBeUndefined()
+  })
+
   it('fails closed for malformed or non-string callback state', () => {
     expect(() => parseCredentialDraftIdFromCallbackUrl({})).toThrow(
       'OAuth state callback URL must be a string'
     )
     expect(() => parseCredentialDraftIdFromCallbackUrl('not a URL')).toThrow()
+    expect(() => parseCredentialDraftIdFromCallbackUrl('//elsewhere.test/path')).toThrow()
   })
 })