diff --git a/README.md b/README.md index 2f50dfc..e6828f1 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ Wire ` [auth] login` and the supporting OAuth runtime. cli-core ships the s npm install commander open ``` -`commander` is required when using `attachLoginCommand`. `open` is optional — when it's missing or `open()` throws, the authorize URL is surfaced via `onAuthorizeUrl` (or printed to stdout in human mode, stderr in `--json` / `--ndjson` mode) so the user can complete the flow manually. +`commander` is required when using `attachLoginCommand`. `open` is optional. The authorize URL is **always** surfaced via `onAuthorizeUrl` (or printed to stdout in human mode, stderr in `--json` / `--ndjson` mode) — even when the browser launch succeeds — because the launch can resolve cleanly yet open no actual browser (WSL silent no-op, headless Linux, locked-down corporate envs). WSL hosts get routed through `cmd.exe` directly so the user's real Windows browser opens. Headless Linux skips the launch entirely and relies on the URL print. #### Quick start (PKCE) @@ -229,10 +229,10 @@ Both attachers strip the standard `--json` / `--ndjson` / `--user` registrar fla | ------------------------ | -------------------------------------------------------------------------------------- | | `--read-only` | Threaded through to `resolveScopes` and the provider hooks via `readOnly`. | | `--callback-port ` | Override `preferredPort` per invocation. Validated as `[0..65535]`; `0` = OS-assigned. | -| `--json` | Machine-output mode. Authorize-URL fallback is routed to stderr. | -| `--ndjson` | Machine-output mode. Same fallback routing. | +| `--json` | Machine-output mode. Authorize-URL print is routed to stderr. | +| `--ndjson` | Machine-output mode. Same print routing. | -Under `--json` / `--ndjson`, the authorize-URL fallback (printed when `open` is missing or `open()` throws) goes to stderr so the JSON / NDJSON envelope on stdout stays clean. Pass `onAuthorizeUrl` to override the destination. The success / error HTML returned by `renderSuccess` / `renderError` is a render hook — every CLI brings its own template (no shared layout enforced). +Under `--json` / `--ndjson`, the always-printed authorize URL goes to stderr so the JSON / NDJSON envelope on stdout stays clean. Pass `onAuthorizeUrl` to override the destination. The success / error HTML returned by `renderSuccess` / `renderError` is a render hook — every CLI brings its own template (no shared layout enforced). #### Implementing `TokenStore` diff --git a/src/auth/flow.test.ts b/src/auth/flow.test.ts index a971c30..a510304 100644 --- a/src/auth/flow.test.ts +++ b/src/auth/flow.test.ts @@ -1,6 +1,27 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' -import { runOAuthFlow } from './flow.js' +// Partial-mock both modules so the default-opener branches (WSL → `cmd.exe`, +// headless Linux → no opener) can be exercised end-to-end through +// `runOAuthFlow`'s public surface. Tests that inject `openBrowser` never +// touch either mock, so the rest of the suite is unaffected. +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, readFileSync: vi.fn(actual.readFileSync) } +}) +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, execFile: vi.fn(actual.execFile) } +}) +// Mock the `open` peer-dep too, otherwise the `$BROWSER` test below falls +// through to the real `open` package, which captures `process.platform` at +// module-load (before our runtime stub) and on macOS spawns the real `open` +// command — launching live browser tabs from the test runner. +vi.mock('open', () => ({ default: vi.fn(async () => undefined) })) + +import { execFile } from 'node:child_process' +import { readFileSync } from 'node:fs' +import openBrowserModule from 'open' +import { type RunOAuthFlowOptions, runOAuthFlow } from './flow.js' import type { AuthProvider, TokenStore } from './types.js' type Account = { id: string; label?: string; email: string } @@ -68,6 +89,41 @@ function instrument(provider: Partial> = {}): { return { provider: wrapped, getRedirect: () => redirectUri } } +/** + * Fill in the boilerplate fields every test would otherwise repeat + * (`scopes`, `readOnly`, `flags`, `preferredPort`, `renderSuccess`, + * `renderError`, `timeoutMs`). Caller supplies the variants — only + * `provider` and `store` are required; `openBrowser`, `onAuthorizeUrl`, + * etc. all stay optional, matching `RunOAuthFlowOptions`. + */ +function flowOptions( + overrides: Partial> & + Pick, 'provider' | 'store'>, +): RunOAuthFlowOptions { + return { + scopes: [], + readOnly: false, + flags: {}, + preferredPort: 0, + renderSuccess, + renderError, + timeoutMs: 5000, + ...overrides, + } +} + +/** + * Drive the local callback server with a valid `code` + the `state` lifted + * from the authorize URL. Used as both an `openBrowser` and an + * `onAuthorizeUrl` stand-in across the suite. + */ +function driveCallback(getRedirect: () => string): (url: string) => Promise { + return async (url) => { + const state = new URL(url).searchParams.get('state') ?? '' + await fetch(`${getRedirect()}?code=abc&state=${state}`) + } +} + describe('runOAuthFlow', () => { it('drives prepare → authorize → exchange → validate → store and returns the result', async () => { const prepare = vi.fn(async () => ({ handshake: { dcrSecret: 'shh' } })) @@ -75,25 +131,17 @@ describe('runOAuthFlow', () => { const validateToken = vi.fn(async () => ({ id: '1', email: 'a@b' })) const { provider, getRedirect } = instrument({ prepare, exchangeCode, validateToken }) const store = fakeStore() + const openBrowser = vi.fn(driveCallback(getRedirect)) - const openBrowser = vi.fn(async (url: string) => { - const state = new URL(url).searchParams.get('state') ?? '' - await fetch(`${getRedirect()}?code=abc&state=${state}`) - }) - - const result = await runOAuthFlow({ - provider, - store, - scopes: ['read'], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser, - onAuthorizeUrl: () => undefined, - timeoutMs: 5000, - }) + const result = await runOAuthFlow( + flowOptions({ + provider, + store, + scopes: ['read'], + openBrowser, + onAuthorizeUrl: () => undefined, + }), + ) expect(result.token).toBe('tok-1') expect(result.account).toEqual({ id: '1', email: 'a@b' }) @@ -118,21 +166,9 @@ describe('runOAuthFlow', () => { }) const store = fakeStore() - const result = await runOAuthFlow({ - provider, - store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser: async (url) => { - const state = new URL(url).searchParams.get('state') ?? '' - await fetch(`${getRedirect()}?code=abc&state=${state}`) - }, - timeoutMs: 5000, - }) + const result = await runOAuthFlow( + flowOptions({ provider, store, openBrowser: driveCallback(getRedirect) }), + ) expect(result.account.id).toBe('99') expect(validateToken).not.toHaveBeenCalled() }) @@ -154,21 +190,9 @@ describe('runOAuthFlow', () => { }) const store = fakeStore() - await runOAuthFlow({ - provider, - store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser: async (url) => { - const state = new URL(url).searchParams.get('state') ?? '' - await fetch(`${getRedirect()}?code=abc&state=${state}`) - }, - timeoutMs: 5000, - }) + await runOAuthFlow( + flowOptions({ provider, store, openBrowser: driveCallback(getRedirect) }), + ) expect(validateToken).toHaveBeenCalledTimes(1) }) @@ -176,18 +200,14 @@ describe('runOAuthFlow', () => { const { provider } = instrument() const store = fakeStore() await expect( - runOAuthFlow({ - provider, - store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser: async () => {}, // never triggers a callback - timeoutMs: 50, - }), + runOAuthFlow( + flowOptions({ + provider, + store, + openBrowser: async () => {}, // never triggers a callback + timeoutMs: 50, + }), + ), ).rejects.toMatchObject({ code: 'AUTH_CALLBACK_TIMEOUT' }) }) @@ -195,30 +215,25 @@ describe('runOAuthFlow', () => { const { provider, getRedirect } = instrument() const store = fakeStore() - const result = await runOAuthFlow({ - provider, - store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser: async (url) => { - const state = new URL(url).searchParams.get('state') ?? '' - // Spurious requests (browser-extension prefetch, accidental - // reload) that don't match the expected state should leave the - // server listening rather than killing the in-flight flow. - const bad1 = await fetch(`${getRedirect()}?code=abc&state=wrong`) - expect(bad1.status).toBe(400) - const bad2 = await fetch(`${getRedirect()}?code=abc`) - expect(bad2.status).toBe(400) - // The legitimate redirect arriving after the noise should still - // settle the wait. - await fetch(`${getRedirect()}?code=abc&state=${state}`) - }, - timeoutMs: 5000, - }) + const result = await runOAuthFlow( + flowOptions({ + provider, + store, + openBrowser: async (url) => { + const state = new URL(url).searchParams.get('state') ?? '' + // Spurious requests (browser-extension prefetch, accidental + // reload) that don't match the expected state should leave the + // server listening rather than killing the in-flight flow. + const bad1 = await fetch(`${getRedirect()}?code=abc&state=wrong`) + expect(bad1.status).toBe(400) + const bad2 = await fetch(`${getRedirect()}?code=abc`) + expect(bad2.status).toBe(400) + // The legitimate redirect arriving after the noise should still + // settle the wait. + await fetch(`${getRedirect()}?code=abc&state=${state}`) + }, + }), + ) expect(result.token).toBe('tok-1') }) @@ -227,18 +242,9 @@ describe('runOAuthFlow', () => { const { provider } = instrument() const store = fakeStore() await expect( - runOAuthFlow({ - provider, - store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 70_000, - renderSuccess, - renderError, - openBrowser, - timeoutMs: 5000, - }), + runOAuthFlow( + flowOptions({ provider, store, openBrowser, preferredPort: 70_000 }), + ), ).rejects.toMatchObject({ code: 'AUTH_PORT_BIND_FAILED' }) expect(openBrowser).not.toHaveBeenCalled() }) @@ -250,52 +256,105 @@ describe('runOAuthFlow', () => { const setSpy = vi.spyOn(store, 'set') await expect( - runOAuthFlow({ + runOAuthFlow( + flowOptions({ + provider, + store, + openBrowser: async () => { + // Abort before the callback arrives — flow should reject + // with AUTH_OAUTH_FAILED rather than continue waiting. + controller.abort() + void getRedirect() // touch to silence unused-fn lint + }, + onAuthorizeUrl: () => undefined, + signal: controller.signal, + }), + ), + ).rejects.toMatchObject({ code: 'AUTH_OAUTH_FAILED' }) + expect(setSpy).not.toHaveBeenCalled() + }) + + it('always surfaces the authorize URL via onAuthorizeUrl, even when openBrowser succeeds', async () => { + // The browser spawn can resolve cleanly yet open no actual browser + // (WSL no-op, headless Linux, etc.), so the URL must reach the user + // on every successful run — not only on opener failure. + const { provider, getRedirect } = instrument() + const store = fakeStore() + const onAuthorizeUrl = vi.fn((_url: string) => undefined) + const openBrowser = vi.fn(driveCallback(getRedirect)) + const result = await runOAuthFlow( + flowOptions({ provider, store, openBrowser, onAuthorizeUrl }), + ) + expect(onAuthorizeUrl).toHaveBeenCalledTimes(1) + expect(onAuthorizeUrl.mock.calls[0][0]).toMatch(/^https:\/\/example\.com\/oauth\/authorize/) + expect(openBrowser).toHaveBeenCalledTimes(1) + expect(result.token).toBe('tok-1') + }) + + it('prints the authorize URL to stdout when no onAuthorizeUrl hook is supplied (TTY)', async () => { + // No-hook default surface: when the consumer doesn't pass + // `onAuthorizeUrl`, the URL still has to land somewhere — under a + // TTY we fall back to `console.log`. Pin this so a regression in + // the default path can't hide behind the hook-driven tests above. + const originalIsTTY = process.stdout.isTTY + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }) + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined) + + const { provider, getRedirect } = instrument() + const store = fakeStore() + + try { + const result = await runOAuthFlow( + flowOptions({ provider, store, openBrowser: driveCallback(getRedirect) }), + ) + expect(result.token).toBe('tok-1') + expect(logSpy).toHaveBeenCalledTimes(1) + expect(logSpy.mock.calls[0][0]).toMatch( + /^Open this URL in your browser:\n {2}https:\/\/example\.com\/oauth\/authorize/, + ) + } finally { + logSpy.mockRestore() + Object.defineProperty(process.stdout, 'isTTY', { + value: originalIsTTY, + configurable: true, + }) + } + }) + + it('isolates onAuthorizeUrl hook failures — a throwing hook does not abort login', async () => { + // The hook is purely an output channel. A buggy logger shouldn't + // be able to kill the auth flow. + const { provider, getRedirect } = instrument() + const store = fakeStore() + const onAuthorizeUrl = vi.fn((_url: string) => { + throw new Error('logger boom') + }) + const result = await runOAuthFlow( + flowOptions({ provider, store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser: async () => { - // Abort before the callback arrives — flow should reject - // with AUTH_OAUTH_FAILED rather than continue waiting. - controller.abort() - void getRedirect() // touch to silence unused-fn lint - }, - onAuthorizeUrl: () => undefined, - signal: controller.signal, - timeoutMs: 5000, + openBrowser: driveCallback(getRedirect), + onAuthorizeUrl, }), - ).rejects.toMatchObject({ code: 'AUTH_OAUTH_FAILED' }) - expect(setSpy).not.toHaveBeenCalled() + ) + expect(onAuthorizeUrl).toHaveBeenCalledTimes(1) + expect(result.token).toBe('tok-1') }) it('falls back to onAuthorizeUrl when the openBrowser opener throws', async () => { const { provider, getRedirect } = instrument() const store = fakeStore() - const onAuthorizeUrl = vi.fn(async (url: string) => { - // Drive the callback off the URL we received via the fallback path. - const state = new URL(url).searchParams.get('state') ?? '' - await fetch(`${getRedirect()}?code=abc&state=${state}`) - }) - const result = await runOAuthFlow({ - provider, - store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser: async () => { - throw new Error('opener boom') - }, - onAuthorizeUrl, - timeoutMs: 5000, - }) + const onAuthorizeUrl = vi.fn(driveCallback(getRedirect)) + const result = await runOAuthFlow( + flowOptions({ + provider, + store, + openBrowser: async () => { + throw new Error('opener boom') + }, + onAuthorizeUrl, + }), + ) expect(onAuthorizeUrl).toHaveBeenCalledTimes(1) expect(onAuthorizeUrl.mock.calls[0][0]).toMatch(/^https:\/\/example\.com\/oauth\/authorize/) expect(result.token).toBe('tok-1') @@ -317,22 +376,133 @@ describe('runOAuthFlow', () => { async setDefault() {}, } await expect( - runOAuthFlow({ - provider, - store, - scopes: [], - readOnly: false, - flags: {}, - preferredPort: 0, - renderSuccess, - renderError, - openBrowser: async (url) => { - const state = new URL(url).searchParams.get('state') ?? '' - await fetch(`${getRedirect()}?code=abc&state=${state}`) - }, - onAuthorizeUrl: () => undefined, - timeoutMs: 5000, - }), + runOAuthFlow( + flowOptions({ + provider, + store, + openBrowser: driveCallback(getRedirect), + onAuthorizeUrl: () => undefined, + }), + ), ).rejects.toMatchObject({ code: 'AUTH_STORE_WRITE_FAILED' }) }) }) + +describe('runOAuthFlow default opener selection', () => { + const originalPlatform = process.platform + + afterEach(() => { + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }) + vi.unstubAllEnvs() + // `mockRestore` reverts to the factory impl (`actual.readFileSync`, + // `actual.execFile`, the empty `open` mock). `mockReset` would + // strip the factory wrap and leave the next call returning + // undefined, which would silently break any test that forgets to + // set its own implementation. + vi.mocked(readFileSync).mockRestore() + vi.mocked(execFile).mockRestore() + vi.mocked(openBrowserModule).mockRestore() + }) + + function stubPlatform(value: NodeJS.Platform): void { + Object.defineProperty(process, 'platform', { value, configurable: true }) + } + + it('routes WSL via cmd.exe with the URL quoted and `%` doubled', async () => { + // Two escapes are load-bearing for cmd.exe: + // 1. quote the URL so `&` doesn't split the command line + // 2. double `%` so cmd.exe doesn't try to expand percent-encoded + // OAuth params (`%3A`, `%2F`, …) as env-var references. + // Use an authorize URL with both `&` and `%` so the assertion + // exercises both escapes. + stubPlatform('linux') + vi.mocked(readFileSync).mockReturnValue('Linux 5.15 #1 SMP microsoft-WSL2') + const execFileMock = vi.mocked(execFile) + execFileMock.mockImplementation((( + _cmd: string, + _args: readonly string[], + _opts: unknown, + cb: unknown, + ) => { + ;(cb as (err: Error | null) => void)(null) + return {} as never + }) as never) + + const { provider, getRedirect } = instrument({ + authorize: async (input) => ({ + authorizeUrl: `https://example.com/oauth/authorize?state=${input.state}&redirect_uri=http%3A%2F%2Flocalhost%3A8080`, + handshake: { codeVerifier: 'v1' }, + }), + }) + const store = fakeStore() + const onAuthorizeUrl = vi.fn(driveCallback(getRedirect)) + + await runOAuthFlow(flowOptions({ provider, store, onAuthorizeUrl })) + + expect(execFileMock).toHaveBeenCalledTimes(1) + const [cmd, args] = execFileMock.mock.calls[0] + const url = onAuthorizeUrl.mock.calls[0][0] as string + expect(cmd).toBe('cmd.exe') + expect(args).toEqual(['/c', 'start', '""', `"${url.replaceAll('%', '%%')}"`]) + // Sanity: the URL we built does contain both special chars. + expect(url).toMatch(/&/) + expect(url).toMatch(/%/) + }) + + it('skips the default opener entirely on headless Linux', async () => { + // No DISPLAY / WAYLAND_DISPLAY / BROWSER + non-WSL Linux → there's + // no working browser launch path. Don't pay the spawn cost; the URL + // print is the only surface. + stubPlatform('linux') + vi.mocked(readFileSync).mockImplementation(() => { + throw new Error('no /proc/version in this test env') + }) + vi.stubEnv('DISPLAY', '') + vi.stubEnv('WAYLAND_DISPLAY', '') + vi.stubEnv('BROWSER', '') + + const { provider, getRedirect } = instrument() + const store = fakeStore() + const onAuthorizeUrl = vi.fn(driveCallback(getRedirect)) + + const result = await runOAuthFlow(flowOptions({ provider, store, onAuthorizeUrl })) + + expect(result.token).toBe('tok-1') + expect(execFile).not.toHaveBeenCalled() + // Lock down "no opener taken": without this, an accidental fall-through + // to the `open` peer-dep would still pass since `onAuthorizeUrl` is + // driving the callback anyway. + expect(openBrowserModule).not.toHaveBeenCalled() + }) + + it('honours $BROWSER on Linux: routes through the `open` peer-dep, not cmd.exe', async () => { + // $BROWSER is the explicit user override Codespaces / custom + // remote-bridge setups use to point `open` at their own helper. + // When set, the headless short-circuit must not fire — let `open` + // handle it. The `open` package is mocked at module scope so the + // real binary never spawns (which would launch live browser tabs + // on macOS dev machines, since `open` captures `process.platform` + // at import time and ignores our runtime stub). + stubPlatform('linux') + vi.mocked(readFileSync).mockImplementation(() => { + throw new Error('not wsl') + }) + vi.stubEnv('DISPLAY', '') + vi.stubEnv('WAYLAND_DISPLAY', '') + vi.stubEnv('BROWSER', '/usr/local/bin/my-browser') + + const { provider, getRedirect } = instrument() + const store = fakeStore() + const onAuthorizeUrl = vi.fn(driveCallback(getRedirect)) + + const result = await runOAuthFlow(flowOptions({ provider, store, onAuthorizeUrl })) + + expect(result.token).toBe('tok-1') + expect(execFile).not.toHaveBeenCalled() + expect(openBrowserModule).toHaveBeenCalledTimes(1) + expect(openBrowserModule).toHaveBeenCalledWith(onAuthorizeUrl.mock.calls[0][0]) + }) +}) diff --git a/src/auth/flow.ts b/src/auth/flow.ts index 0e5d674..d95611b 100644 --- a/src/auth/flow.ts +++ b/src/auth/flow.ts @@ -1,9 +1,38 @@ +import { execFile } from 'node:child_process' +import { readFileSync } from 'node:fs' import { type IncomingMessage, type Server, type ServerResponse, createServer } from 'node:http' +import { promisify } from 'node:util' import { CliError, getErrorMessage } from '../errors.js' import { isStdoutTTY } from '../terminal.js' import { generateState } from './pkce.js' import type { AuthAccount, AuthProvider, TokenStore } from './types.js' +// WSL's `open` package routes through `xdg-open` / `wslview`, both of which +// silently no-op on headless WSL installs — the spawn resolves cleanly but no +// browser ever appears, so the OAuth callback wait runs to its 3-minute +// timeout. Detect at call time and route WSL through `cmd.exe` directly. +// Non-Linux platforms short-circuit before the fs read. +function isWsl(): boolean { + if (process.platform !== 'linux') return false + try { + return /microsoft/i.test(readFileSync('/proc/version', 'utf8')) + } catch { + return false + } +} + +// SSH sessions, containers, CI runners, headless servers — same failure mode +// as WSL but with no Windows side to bounce through. With no DISPLAY / +// WAYLAND_DISPLAY (and no $BROWSER override for Codespaces-style setups +// that route through a remote bridge), `xdg-open` will either error or +// no-op, so the spawn is pure noise — skip it and let the URL print do the +// work. +function isHeadlessLinux(): boolean { + if (process.platform !== 'linux') return false + if (process.env.BROWSER) return false + return !process.env.DISPLAY && !process.env.WAYLAND_DISPLAY +} + export type RunOAuthFlowOptions = { provider: AuthProvider store: TokenStore @@ -27,8 +56,15 @@ export type RunOAuthFlowOptions = { renderError: (message: string) => string /** Override the browser opener (tests). When omitted, dynamically imports `open`. */ openBrowser?: (url: string) => Promise - /** Print the authorize URL to stdout as a fallback when the browser can't open it. */ - onAuthorizeUrl?: (url: string) => void + /** + * Receives the authorize URL on every login attempt — not only when the + * browser launch fails — because the launch can resolve cleanly yet + * open no browser (WSL no-op, headless Linux, locked-down hosts). + * Treat as a fire-and-forget output channel: a sync hook is awaited, + * an async hook is awaited too, and any throw / rejection is swallowed + * so a buggy logger can never abort an otherwise-working login. + */ + onAuthorizeUrl?: (url: string) => void | Promise /** Callback timeout in ms. Default 3 minutes. */ timeoutMs?: number /** Cancellation signal (Ctrl-C wiring). */ @@ -424,22 +460,36 @@ async function openOrFallback( url: string, options: RunOAuthFlowOptions, ): Promise { - const opener = options.openBrowser ?? (await loadDefaultOpener()) - if (opener) { + // Surface the URL up-front, before attempting the browser spawn. The + // spawn can succeed yet open no browser (WSL without working interop, + // headless Linux, locked-down corporate envs, missing `open` peer), and + // we have no reliable signal that the user actually landed on the page + // — so printing here guarantees a copy-pasteable path on every platform. + // The hook is purely an output channel: isolate its failures so a buggy + // logger can't abort the login that the user is actually here to do. + if (options.onAuthorizeUrl) { try { - await opener(url) - return + await options.onAuthorizeUrl(url) } catch { - // Fall through to the URL print below. + // Hook errors don't propagate. } + } else if (isStdoutTTY()) console.log(`Open this URL in your browser:\n ${url}`) + + const opener = options.openBrowser ?? (await loadDefaultOpener()) + if (!opener) return + try { + await opener(url) + } catch { + // URL is already surfaced above. } - // No opener available, or the opener threw. Surface the URL so the user - // can finish the flow manually. - if (options.onAuthorizeUrl) options.onAuthorizeUrl(url) - else if (isStdoutTTY()) console.log(`Open this URL in your browser:\n ${url}`) } async function loadDefaultOpener(): Promise<((url: string) => Promise) | null> { + // WSL check must run before the headless check: WSL is `platform === 'linux'` + // and often has no DISPLAY, but `cmd.exe` does work and reaches the user's + // real Windows browser, so it's worth the spawn. + if (isWsl()) return openViaCmdExe + if (isHeadlessLinux()) return null try { const mod = (await import('open')) as { default: (url: string) => Promise } return async (url) => { @@ -449,3 +499,23 @@ async function loadDefaultOpener(): Promise<((url: string) => Promise) | n return null } } + +const execFileAsync = promisify(execFile) + +// Two layers of escaping are needed because `cmd.exe /c` is a shell: +// 1. Wrap the URL in literal double quotes. WSL interop only auto-quotes +// args that contain spaces; an OAuth URL (no spaces, plenty of `&`s) +// would otherwise be re-parsed by cmd.exe with `&` acting as a +// statement separator, so only the prefix up to the first `&` would +// reach `start`. +// 2. Double every `%` to `%%`. cmd.exe expands `%NAME%` even inside +// quoted strings; OAuth URLs are full of percent-encoded bytes +// (`%3A`, `%2F`, …) and a chance match against a defined env var +// (`%PATH%`, `%TEMP%`, …) would silently mangle the URL. +// `start ""` — the empty title arg is mandatory; otherwise `start` consumes +// the URL as a window title and never launches a browser. (`execFile`'s +// no-shell guarantee doesn't apply when the target is itself a shell.) +async function openViaCmdExe(url: string): Promise { + const escaped = url.replaceAll('%', '%%') + await execFileAsync('cmd.exe', ['/c', 'start', '""', `"${escaped}"`], { windowsHide: true }) +}