From 2686327a87ce1d8996c962be050c669c4d04b88f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 16:46:58 -0700 Subject: [PATCH 1/2] fix(cli): keep a path prefix in the login URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sim login` built both of its URLs with `new URL('/path', endpoint)`. A leading-slash path is absolute, so it resolves against the endpoint's origin and drops any path the endpoint carries: a deployment served at `https://host/sim` sent the browser to `https://host/cli/auth` and polled `https://host/api/cli/auth/poll`, neither of which exists there. Every other command builds its URL by concatenation and was unaffected, so the endpoint looked correct and login alone failed. Both now go through the client's `buildUrl`, which is exported for it rather than duplicated — one URL builder for the whole CLI is the point, since two of them is how the halves drifted apart. Its TSDoc now names the trap. --- packages/sim-cli/src/auth/device-flow.test.ts | 32 +++++++++++++++++++ packages/sim-cli/src/auth/device-flow.ts | 27 +++++++++------- packages/sim-cli/src/http/client.ts | 19 ++++++++++- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/packages/sim-cli/src/auth/device-flow.test.ts b/packages/sim-cli/src/auth/device-flow.test.ts index 6fa31cc3d80..16e98fc79ed 100644 --- a/packages/sim-cli/src/auth/device-flow.test.ts +++ b/packages/sim-cli/src/auth/device-flow.test.ts @@ -137,6 +137,38 @@ describe('createAuthRequest', () => { } }) + it('keeps a path prefix the endpoint carries, in both login URLs', async () => { + // Both URLs were built with `new URL('/path', endpoint)`. A leading-slash + // path is absolute, so it resolved against the ORIGIN and dropped the + // prefix: a deployment served at https://host/sim sent the browser to + // https://host/cli/auth and polled https://host/api/cli/auth/poll, neither + // of which exists there. Every other command concatenated and worked, so + // the endpoint looked correct and only login failed. + const prefixed = 'https://host.test/sim' + const auth = createAuthRequest() + + expect(buildApprovalUrl(prefixed, auth, 'platform')).toMatch( + /^https:\/\/host\.test\/sim\/cli\/auth\?/ + ) + + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ status: 'complete', key: { id: 'k', apiKey: 'sk' } }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', fetchMock) + + await pollForKey(prefixed, auth) + expect(fetchMock.mock.calls[0][0]).toBe('https://host.test/sim/api/cli/auth/poll') + }) + + it('omits an absent workspace rather than sending it blank', () => { + const auth = createAuthRequest() + expect(buildApprovalUrl(ENDPOINT, auth, 'platform')).not.toContain('workspace=') + expect(buildApprovalUrl(ENDPOINT, auth, 'platform', 'ws_1')).toContain('workspace=ws_1') + }) + it('never puts the poll secret in the browser URL', () => { const auth = createAuthRequest() const url = buildApprovalUrl(ENDPOINT, auth, 'platform', 'ws_1') diff --git a/packages/sim-cli/src/auth/device-flow.ts b/packages/sim-cli/src/auth/device-flow.ts index 1c72a2e9f34..da04111a15b 100644 --- a/packages/sim-cli/src/auth/device-flow.ts +++ b/packages/sim-cli/src/auth/device-flow.ts @@ -1,6 +1,6 @@ import { createHash, randomBytes, randomInt } from 'node:crypto' import { sleep } from '../helpers' -import { REDIRECT_STATUSES, redirectEndpoint, SimApiError } from '../http/client' +import { buildUrl, REDIRECT_STATUSES, redirectEndpoint, SimApiError } from '../http/client' import { USER_AGENT } from '../version' /** @@ -20,6 +20,12 @@ const PAIRING_ALPHABET = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789' const POLL_INTERVAL_MS = 2000 const POLL_TIMEOUT_MS = 15 * 60 * 1000 +/** The page the browser is sent to for approval. */ +const APPROVAL_PATH = '/cli/auth' + +/** The route the login poll targets; also the suffix a redirect target is measured against. */ +const POLL_PATH = '/api/cli/auth/poll' + /** * Poll statuses that leave the approval still redeemable, so the login should * keep waiting rather than making the user restart the browser handoff. @@ -89,13 +95,13 @@ export function buildApprovalUrl( scope: CliAuthScope, workspaceId?: string ): string { - const url = new URL('/cli/auth', endpoint) - url.searchParams.set('request', auth.request) - url.searchParams.set('challenge', auth.challenge) - url.searchParams.set('pairing', auth.pairing) - url.searchParams.set('scope', scope) - if (workspaceId) url.searchParams.set('workspace', workspaceId) - return url.toString() + return buildUrl(endpoint, APPROVAL_PATH, { + request: auth.request, + challenge: auth.challenge, + pairing: auth.pairing, + scope, + workspace: workspaceId, + }) } interface PollResponse { @@ -106,9 +112,6 @@ interface PollResponse { workspaceBound?: boolean } -/** The route the login poll targets; also the suffix a redirect target is measured against. */ -const POLL_PATH = '/api/cli/auth/poll' - /** * Explains a redirected poll rather than following it. * @@ -166,7 +169,7 @@ export async function pollForKey( let response: Response | null = null try { - response = await fetch(new URL(POLL_PATH, endpoint), { + response = await fetch(buildUrl(endpoint, POLL_PATH), { method: 'POST', headers: { 'content-type': 'application/json', diff --git a/packages/sim-cli/src/http/client.ts b/packages/sim-cli/src/http/client.ts index d7531971b3a..09cb52ad26e 100644 --- a/packages/sim-cli/src/http/client.ts +++ b/packages/sim-cli/src/http/client.ts @@ -53,7 +53,24 @@ export interface WorkspaceOptions { auth?: AuthRequirement } -function buildUrl(endpoint: string, path: string, query?: Record): string { +/** + * Joins an endpoint and a route into a request URL. + * + * Concatenation rather than `new URL(path, endpoint)`, which is the trap it + * exists to avoid: a leading-slash path is absolute, so `new URL()` resolves it + * against the endpoint's ORIGIN and silently drops any path the endpoint + * carries. A deployment served under a prefix — `https://host/sim` behind a + * proxy that fronts several apps — would have every request rewritten to + * `https://host/...`, losing the prefix that identifies it. + * + * Empty values are skipped rather than sent blank so an omitted optional + * parameter reads as absent, not as the empty string. + */ +export function buildUrl( + endpoint: string, + path: string, + query?: Record +): string { const url = new URL(`${endpoint}${path}`) for (const [key, value] of Object.entries(query ?? {})) { if (value === null || value === undefined || value === '') continue From 3885a15cf3c90c0893544a4d6a55c1509defa781 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 16:51:31 -0700 Subject: [PATCH 2/2] test(cli): restore fetch with spyOn so the stub cannot outlive its test vi.stubGlobal is not undone by restoreAllMocks, so the completed-auth response would have leaked into whatever ran next. The rest of this file already spies on globalThis.fetch, which the existing teardown restores. --- packages/sim-cli/src/auth/device-flow.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/sim-cli/src/auth/device-flow.test.ts b/packages/sim-cli/src/auth/device-flow.test.ts index 16e98fc79ed..42a9cf2a724 100644 --- a/packages/sim-cli/src/auth/device-flow.test.ts +++ b/packages/sim-cli/src/auth/device-flow.test.ts @@ -151,16 +151,18 @@ describe('createAuthRequest', () => { /^https:\/\/host\.test\/sim\/cli\/auth\?/ ) - const fetchMock = vi.fn().mockResolvedValue( + // `spyOn`, like the rest of this file: `restoreAllMocks` in teardown undoes + // it, whereas a `stubGlobal` would outlive the test and leak this + // completed-auth response into whatever ran next. + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue( new Response(JSON.stringify({ status: 'complete', key: { id: 'k', apiKey: 'sk' } }), { status: 200, headers: { 'content-type': 'application/json' }, }) ) - vi.stubGlobal('fetch', fetchMock) await pollForKey(prefixed, auth) - expect(fetchMock.mock.calls[0][0]).toBe('https://host.test/sim/api/cli/auth/poll') + expect(fetchSpy.mock.calls[0][0]).toBe('https://host.test/sim/api/cli/auth/poll') }) it('omits an absent workspace rather than sending it blank', () => {