From fbd094d3e3f156db45543c6ed528345d770cc20a Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 18:32:08 -0700 Subject: [PATCH 1/4] fix(desktop): stop the OAuth connect callback from failing on a bare-path callback URL The desktop connect launcher passed better-auth a same-origin path as its callbackURL. Better Auth stores that value verbatim in the OAuth state, and the callback's credential-draft reader parsed it with a bare `new URL()`, which rejects a path. That throw happened inside the `account.create.before` database hook, which better-auth's OAuth callback does not guard, so the provider redirect landed on a 500 after authorization had already succeeded. Send an absolute URL from the connect page, matching the workspace-scoped branch and every other connect surface, and accept a path-absolute callback URL in the draft reader so the shape can never fail the callback again. Protocol-relative and malformed values still throw, keeping an unreadable binding loud. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/desktop/connect/connect-launcher.tsx | 17 ++- apps/sim/app/desktop/connect/page.test.tsx | 131 ++++++++++++++++++ apps/sim/app/desktop/connect/page.tsx | 2 +- .../lib/credentials/draft-processor.test.ts | 12 ++ apps/sim/lib/credentials/draft-processor.ts | 31 ++++- 5 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 apps/sim/app/desktop/connect/page.test.tsx diff --git a/apps/sim/app/desktop/connect/connect-launcher.tsx b/apps/sim/app/desktop/connect/connect-launcher.tsx index c1c8bfde4a4..fdb6276aea3 100644 --- a/apps/sim/app/desktop/connect/connect-launcher.tsx +++ b/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(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=`, 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 diff --git a/apps/sim/app/desktop/connect/page.test.tsx b/apps/sim/app/desktop/connect/page.test.tsx new file mode 100644 index 00000000000..0eae3c0c6cd --- /dev/null +++ b/apps/sim/app/desktop/connect/page.test.tsx @@ -0,0 +1,131 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockGetSession, mockRedirect } = vi.hoisted(() => ({ + mockGetSession: vi.fn(), + mockRedirect: vi.fn((url: string) => { + throw new Error(`NEXT_REDIRECT:${url}`) + }), +})) + +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: () => 'https://sim.test', +})) + +/** 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) { + return { searchParams: Promise.resolve(params) } +} + +async function renderPage(params: Record) { + const result = (await DesktopConnectPage(pageProps(params))) as unknown as { + type: { name: string } + props: Record + } + return result +} + +describe('DesktopConnectPage', () => { + beforeEach(() => { + vi.clearAllMocks() + 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('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() + }) +}) diff --git a/apps/sim/app/desktop/connect/page.tsx b/apps/sim/app/desktop/connect/page.tsx index 4e473134b6b..d37ea9d0e75 100644 --- a/apps/sim/app/desktop/connect/page.tsx +++ b/apps/sim/app/desktop/connect/page.tsx @@ -125,7 +125,7 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec return ( ) } diff --git a/apps/sim/lib/credentials/draft-processor.test.ts b/apps/sim/lib/credentials/draft-processor.test.ts index 00401d165d4..579f02375c4 100644 --- a/apps/sim/lib/credentials/draft-processor.test.ts +++ b/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() }) }) diff --git a/apps/sim/lib/credentials/draft-processor.ts b/apps/sim/lib/credentials/draft-processor.ts index c375dd8d48c..e49f1214787 100644 --- a/apps/sim/lib/credentials/draft-processor.ts +++ b/apps/sim/lib/credentials/draft-processor.ts @@ -25,13 +25,40 @@ type AvailableOAuthCredentialDraftBinding = Extract< const oauthCredentialDraftBindings = new WeakMap() -/** Extracts a draft binding from Better Auth state and rejects malformed callback state. */ +/** + * Base a path-absolute callback URL is resolved against. Only the query string + * is ever read, so the origin reaches nothing — an RFC 2606 `.invalid` host says + * so at a glance, and keeps this parse independent of `NEXT_PUBLIC_APP_URL`, + * whose absence would otherwise turn a state read into a configuration throw. + */ +const CALLBACK_URL_RESOLUTION_BASE = 'http://callback.invalid' + +/** + * Extracts a draft binding from Better Auth state and rejects malformed callback state. + * + * Better Auth documents `callbackURL` as a reference relative to the app + * (`/dashboard`) and stores whatever it is handed verbatim, so a path and a full + * URL are equally legitimate — these two shapes are what is accepted. Bare + * `new URL()` rejects the path form, and because this runs inside the + * `account.create.before` database hook, which Better Auth's OAuth callback does + * not guard, that rejection surfaced as a 500 on the callback rather than a + * failed connection. + * + * A network-path reference (`//host/path`, RFC 3986 §4.2) is not a path and + * still throws, as does anything else malformed: a callback URL we cannot read + * must stay loud rather than read as "carried no draft", which would fall back + * to guessing the draft from the user and provider alone. + */ export function parseCredentialDraftIdFromCallbackUrl(callbackUrl: unknown): string | undefined { if (callbackUrl === undefined) return undefined if (typeof callbackUrl !== 'string') { throw new Error('OAuth state callback URL must be a string') } - return new URL(callbackUrl).searchParams.get(OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM) ?? undefined + const isPathAbsolute = callbackUrl.startsWith('/') && !callbackUrl.startsWith('//') + const url = isPathAbsolute + ? new URL(callbackUrl, CALLBACK_URL_RESOLUTION_BASE) + : new URL(callbackUrl) + return url.searchParams.get(OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM) ?? undefined } /** Reads an exact draft binding without falling back when OAuth state is unavailable. */ From d9b50f140de26d8d19e0f7a466197f462ab85803 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 18:46:24 -0700 Subject: [PATCH 2/4] fix(desktop): compose the connect completion URL through the URL API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concatenating `getBaseUrl()` with the completion path leaves the result dependent on how the deployment spelled `NEXT_PUBLIC_APP_URL`: the helper only adds a missing protocol, so a trailing slash produced `//desktop/connect/complete`, a pathname that matches no route. The completion page is what bounces the OAuth result to the desktop app's loopback, so that typo would have stranded the flow just past the callback it was meant to fix. Both callback URLs in the page — the launcher's and the workspace-scoped authorize redirect's — now go through one helper that resolves the path against the base with `new URL`, matching how the same function already builds the authorize URL, with coverage for a trailing-slash base. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/app/desktop/connect/page.test.tsx | 33 ++++++++++++++++++++-- apps/sim/app/desktop/connect/page.tsx | 18 ++++++++---- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/apps/sim/app/desktop/connect/page.test.tsx b/apps/sim/app/desktop/connect/page.test.tsx index 0eae3c0c6cd..d44f769d668 100644 --- a/apps/sim/app/desktop/connect/page.test.tsx +++ b/apps/sim/app/desktop/connect/page.test.tsx @@ -3,11 +3,13 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGetSession, mockRedirect } = vi.hoisted(() => ({ +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', () => ({ @@ -21,7 +23,7 @@ vi.mock('@/lib/auth/auth-client', () => ({ })) vi.mock('@/lib/core/utils/urls', () => ({ - getBaseUrl: () => 'https://sim.test', + getBaseUrl: () => baseUrl.value, })) /** Keeps the landing-page barrel the real shell pulls in out of this graph. */ @@ -57,6 +59,7 @@ async function renderPage(params: Record) { describe('DesktopConnectPage', () => { beforeEach(() => { vi.clearAllMocks() + baseUrl.value = 'https://sim.test' mockGetSession.mockResolvedValue({ user: { id: 'user-1', email: 'user@example.com' } }) }) @@ -93,6 +96,32 @@ describe('DesktopConnectPage', () => { 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( diff --git a/apps/sim/app/desktop/connect/page.tsx b/apps/sim/app/desktop/connect/page.tsx index d37ea9d0e75..9110d733599 100644 --- a/apps/sim/app/desktop/connect/page.tsx +++ b/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 ( ) } From 5e8e10be5d75d776d73bf2b44ea99135ae9e97d3 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 19:09:20 -0700 Subject: [PATCH 3/4] fix(urls): give base URLs the no-trailing-slash form their call sites assume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getBaseUrl()` returned `NEXT_PUBLIC_APP_URL` as the operator spelled it, while almost every consumer builds `${base}/path`. A base configured with a trailing slash therefore produced a `//path` pathname that matches no route, and broke the `startsWith(`${base}/`)` prefix checks that decide whether a redirect target is our own — the OAuth authorize route rejected its own completion callback and fell back to the workspace page, so the desktop handoff never ran on those deployments. The previous commit fixed one such URL; this fixes the reason it was wrong, for the ~30 concatenation sites that share the assumption. `normalizeBaseUrl` now strips trailing slashes alongside the protocol it already added, which is the invariant SITE_URL has always documented. A path-prefixed base keeps its path. `getInternalApiBaseUrl` gets the same treatment, since its callers concatenate identically. `@sim/testing`'s urls mock is a hand-written mirror of this module, so it moves in step. `internal-api-base-url.test.ts` now unmocks the module it names — otherwise it asserts against that mirror and any drift between the two passes unnoticed. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/utils/internal-api-base-url.test.ts | 21 +++++++++++- apps/sim/lib/core/utils/urls.test.ts | 32 +++++++++++++++++++ apps/sim/lib/core/utils/urls.ts | 23 +++++++++---- packages/testing/src/mocks/urls.mock.ts | 17 +++++++--- 4 files changed, 82 insertions(+), 11 deletions(-) diff --git a/apps/sim/lib/core/utils/internal-api-base-url.test.ts b/apps/sim/lib/core/utils/internal-api-base-url.test.ts index 4b346111a0c..dff09ac175e 100644 --- a/apps/sim/lib/core/utils/internal-api-base-url.test.ts +++ b/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, diff --git a/apps/sim/lib/core/utils/urls.test.ts b/apps/sim/lib/core/utils/urls.test.ts index bcbe797df6e..c0659e93747 100644 --- a/apps/sim/lib/core/utils/urls.test.ts +++ b/apps/sim/lib/core/utils/urls.test.ts @@ -56,6 +56,38 @@ 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' + ) + } + }) + + it('keeps the path of a path-prefixed base URL', () => { + mockGetEnv.mockImplementation((key) => + key === 'NEXT_PUBLIC_APP_URL' ? 'https://example.com/sim/' : undefined + ) + expect(getBaseUrl()).toBe('https://example.com/sim') + }) + + 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 diff --git a/apps/sim/lib/core/utils/urls.ts b/apps/sim/lib/core/utils/urls.ts index 8db88438988..31235e2b88d 100644 --- a/apps/sim/lib/core/utils/urls.ts +++ b/apps/sim/lib/core/utils/urls.ts @@ -12,13 +12,22 @@ 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. A path-prefixed base (`https://host/sim/`) keeps its path. + */ 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 +98,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) } /** diff --git a/packages/testing/src/mocks/urls.mock.ts b/packages/testing/src/mocks/urls.mock.ts index 8889ee23860..1daf6540f94 100644 --- a/packages/testing/src/mocks/urls.mock.ts +++ b/packages/testing/src/mocks/urls.mock.ts @@ -24,6 +24,17 @@ function hasHttpProtocol(url: string): boolean { return /^https?:\/\//i.test(url) } +/** + * Mirrors the real module's `normalizeBaseUrl`: protocol-less values get + * https:// under isProd, then trailing slashes are stripped so `${base}/path` + * stays single-slashed at every call site. + */ +function normalizeBaseUrl(url: string): string { + const protocol = envFlagsMock.isProd ? 'https://' : 'http://' + const withProtocol = hasHttpProtocol(url) ? url : `${protocol}${url}` + return withProtocol.replace(/\/+$/, '') +} + function getBaseUrlImpl(): string { const baseUrl = readEnv('NEXT_PUBLIC_APP_URL')?.trim() if (!baseUrl) { @@ -31,9 +42,7 @@ function getBaseUrlImpl(): string { 'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly' ) } - // Mirrors the real module: protocol-less values get https:// under isProd. - const protocol = envFlagsMock.isProd ? 'https://' : 'http://' - return hasHttpProtocol(baseUrl) ? baseUrl : `${protocol}${baseUrl}` + return normalizeBaseUrl(baseUrl) } function getInternalApiBaseUrlImpl(): string { @@ -47,7 +56,7 @@ function getInternalApiBaseUrlImpl(): string { 'INTERNAL_API_BASE_URL must include protocol (http:// or https://), e.g. http://sim-app.default.svc.cluster.local:3000' ) } - return internalBaseUrl + return normalizeBaseUrl(internalBaseUrl) } function ensureAbsoluteUrlImpl(pathOrUrl: string): string { From e57f284afd6872bc68d5ab3c62cf0713f1314a33 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 19:25:27 -0700 Subject: [PATCH 4/4] chore(urls): stop claiming path-prefixed base URLs are supported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit's doc and test said a path-prefixed base keeps its path. That reads as support for a deployment shape the app does not have: there is no Next `basePath`, so routes are served at the origin root and such a value could not address them however the base were normalized. Every documented example is origin-only. Says only what is true — trailing slashes are the one spelling absorbed — and reframes the test as pinning the trim's shape rather than asserting a path-prefixed deployment works. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/lib/core/utils/urls.test.ts | 11 ++++++++--- apps/sim/lib/core/utils/urls.ts | 6 +++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/core/utils/urls.test.ts b/apps/sim/lib/core/utils/urls.test.ts index c0659e93747..0f472f3d9a9 100644 --- a/apps/sim/lib/core/utils/urls.test.ts +++ b/apps/sim/lib/core/utils/urls.test.ts @@ -74,11 +74,16 @@ describe('getBaseUrl', () => { } }) - it('keeps the path of a path-prefixed base URL', () => { + /** + * 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/sim/' : undefined + key === 'NEXT_PUBLIC_APP_URL' ? 'https://example.com/a/b/' : undefined ) - expect(getBaseUrl()).toBe('https://example.com/sim') + expect(getBaseUrl()).toBe('https://example.com/a/b') }) it('adds the protocol and strips the trailing slash together', () => { diff --git a/apps/sim/lib/core/utils/urls.ts b/apps/sim/lib/core/utils/urls.ts index 31235e2b88d..08d6a9c7673 100644 --- a/apps/sim/lib/core/utils/urls.ts +++ b/apps/sim/lib/core/utils/urls.ts @@ -22,7 +22,11 @@ function hasHttpProtocol(url: string): boolean { * 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. A path-prefixed base (`https://host/sim/`) keeps its path. + * 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 { const protocol = isProd ? 'https://' : 'http://'