diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 4ab8a5c0c..4f4179b78 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -568,13 +568,25 @@ See [Environment Variables](../guide/env-vars.md) for the full guide. ## GitHub +Deploy-on-GitHub works from **any** Void login — Google, GitHub, or other SSO. The first time you connect GitHub, Void links your GitHub identity to your current account (a one-time step, independent of how you logged in); it never creates a second account. + +### `void github link` + +``` +void github link +``` + +Link your current Void account to a GitHub identity. Opens your browser to authorize Void on GitHub (a localhost + PKCE handshake, the same mechanics as `void auth login`), then binds that GitHub identity to the logged-in account. Requires an authenticated CLI (`void auth login` first). + +You normally don't need to run this directly — `void github install` runs the link automatically when your account has no GitHub identity yet. Run it on its own to link ahead of time, or to link a GitHub identity without installing the App. If that GitHub account is already linked to a different Void account, the command fails and asks you to use a different GitHub account. + ### `void github install` ``` void github install ``` -Open the GitHub App install page in your browser. After installing, run `void github connect` to link a repository to your project. +Open the GitHub App install page in your browser. If your account has no linked GitHub identity yet, `void github install` first runs the GitHub link automatically (browser authorize), then continues. After installing, run `void github connect` to link a repository to your project. ### `void github installations` diff --git a/packages/void/src/cli/auth.ts b/packages/void/src/cli/auth.ts index 59e819c57..6cbe28ab6 100644 --- a/packages/void/src/cli/auth.ts +++ b/packages/void/src/cli/auth.ts @@ -163,11 +163,11 @@ function base64Url(buffer: Buffer): string { return buffer.toString('base64').replaceAll('+', '-').replaceAll('/', '_').replaceAll('=', ''); } -function createCodeVerifier(): string { +export function createCodeVerifier(): string { return base64Url(randomBytes(32)); } -function createCodeChallenge(verifier: string): string { +export function createCodeChallenge(verifier: string): string { return base64Url(createHash('sha256').update(verifier).digest()); } @@ -263,28 +263,58 @@ export function openBrowser( } /** - * Start a local HTTP server, open the browser to the platform login page, - * and wait for the OAuth callback with a short-lived code. The CLI exchanges - * the code itself so the localhost callback never accepts bearer tokens. + * Start a local HTTP callback server, open the browser to a caller-built + * authorize URL, and wait for the redirect back to `http://localhost:PORT/callback`. * - * `onUrl` is invoked with the login URL once the local server is listening, - * so callers can show the URL to the user as a manual fallback when the - * browser fails to open (common on WSL2, headless SSH, Docker, etc.). + * The server validates the `state` it appended to the callback URL against the + * incoming request, then runs the caller's `finalize` hook INSIDE the request + * handler (so the browser only ever sees the success page after the underlying + * exchange/link actually succeeded). On finalize success it renders the success + * HTML and resolves with finalize's return value; on finalize failure it + * renders the same "could not complete" failure page the login flow uses and + * rejects with the thrown error. The server is always closed before settling. + * + * `onUrl` is invoked with the authorize URL once the local server is listening, + * so callers can show the URL to the user as a manual fallback when the browser + * fails to open (common on WSL2, headless SSH, Docker, etc.). To match the + * login flow it fires immediately before the browser is opened. + * + * `execFile` is injectable so tests can avoid launching a real browser. */ -export async function runLogin( - root: string = process.cwd(), - provider = 'github', - onUrl?: (url: string) => void, -): Promise { +export async function awaitBrowserCallback(options: { + /** Caller-generated; appended to the callback URL and validated on the incoming request. */ + state: string; + /** Given `http://localhost:PORT/callback?state=…`, return the URL to open. */ + buildAuthUrl: (callback: URL) => Promise | string; + /** + * Runs INSIDE the request handler, after state/error/validate checks pass and + * BEFORE the success page is rendered. Resolve to settle the outer promise + * with this value (success page shown); throw to render the failure page and + * reject the outer promise with the thrown error. + */ + finalize: (params: URLSearchParams) => Promise; + /** Map an `?error=…` query to a user message (return null to ignore). */ + describeError?: (params: URLSearchParams) => string | null; + /** + * Reject an otherwise-valid callback request (renders a 400 page) WITHOUT + * settling, so the server keeps listening for a subsequent valid callback. + * Used by login to reject `?token=` and missing-`code` requests. Return a + * message to reject, or null to accept. + */ + validate?: (params: URLSearchParams) => string | null; + /** Optional, invoked with the authorize URL before the browser is opened. */ + onUrl?: (url: string) => void; + /** Default 5 minutes. */ + timeoutMs?: number; + /** Injectable for tests; defaults to the real `node:child_process` `execFile`. */ + execFile?: typeof import('node:child_process').execFile; +}): Promise { const http = await import('node:http'); - const { execFile } = await import('node:child_process'); + const { state, buildAuthUrl, finalize, describeError, validate, onUrl } = options; + const timeoutMs = options.timeoutMs ?? 5 * 60 * 1000; + const execFile = options.execFile ?? (await import('node:child_process')).execFile; - const apiUrl = getApiUrl(root); - const state = randomBytes(32).toString('hex'); - const codeVerifier = createCodeVerifier(); - const codeChallenge = createCodeChallenge(codeVerifier); - - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { let settled = false; const fail = (error: Error): void => { @@ -296,13 +326,13 @@ export async function runLogin( reject(error); }; - const succeed = (token: string): void => { + const succeed = (result: T): void => { if (settled) { return; } settled = true; server.close(); - resolve(token); + resolve(result); }; const server = http.createServer(async (req, res) => { @@ -319,79 +349,134 @@ export async function runLogin( return; } - const error = url.searchParams.get('error'); - if (error === 'signup_restricted') { + const errorMessage = describeError?.(url.searchParams); + if (errorMessage) { res.writeHead(403, { 'Content-Type': 'text/html' }); - res.end( - page('Signup restricted', 'Your email is not on the approved list for this environment.'), - ); - fail( - new Error( - 'Signup restricted: your email is not on the approved list for this environment.', - ), - ); - return; - } - - if (url.searchParams.has('token')) { - res.writeHead(400, { 'Content-Type': 'text/html' }); - res.end(page('Login failed', 'Invalid login callback. Please try again.')); + res.end(page('Login failed', errorMessage)); + fail(new Error(errorMessage)); return; } - const code = url.searchParams.get('code'); - if (!code) { + // Reject-but-keep-listening: invalid request shapes (e.g. a stray + // `?token=`, or a redirect that arrived without a code). The server + // stays open so the genuine callback can still complete. + const invalidMessage = validate?.(url.searchParams); + if (invalidMessage) { res.writeHead(400, { 'Content-Type': 'text/html' }); - res.end(page('Login failed', 'No authorization code received. Please try again.')); + res.end(page('Login failed', invalidMessage)); return; } - let token: string; + // Finalize INSIDE the request path so the browser only sees the success + // page once the underlying exchange/link succeeds. A finalize failure + // (expired code, API error, 409 conflict, etc.) renders the failure page + // and rejects — matching the base login flow's exchange-in-request-path + // behavior. Guard against the timeout racing the in-flight finalize. + let result: T; try { - token = await exchangeLoginCode(apiUrl, code, state, codeVerifier); + result = await finalize(url.searchParams); } catch (err) { + if (settled) { + return; + } res.writeHead(400, { 'Content-Type': 'text/html' }); res.end(page('Login failed', 'Could not complete login. Please try again.')); fail(err instanceof Error ? err : new Error('auth: Failed to complete login.')); return; } + if (settled) { + return; + } res.writeHead(200, { 'Content-Type': 'text/html' }); res.end(page('Login successful!', 'You can close this tab and return to the terminal.')); - succeed(token); + succeed(result); }); // Close immediately after callback — don't wait for keep-alive timeout server.on('connection', (socket) => socket.unref()); - server.listen(0, '127.0.0.1', () => { + server.listen(0, '127.0.0.1', async () => { const addr = server.address(); if (!addr || typeof addr === 'string') { - reject(new Error('auth: Failed to start local callback server.')); + fail(new Error('auth: Failed to start local callback server.')); return; } const port = addr.port; - const redirect = new URL(`http://localhost:${port}/callback`); - redirect.searchParams.set('state', state); + const callback = new URL(`http://localhost:${port}/callback`); + callback.searchParams.set('state', state); - const loginUrl = new URL('auth/login', apiUrl.endsWith('/') ? apiUrl : `${apiUrl}/`); - loginUrl.searchParams.set('redirect', redirect.href); - loginUrl.searchParams.set('provider', provider); - loginUrl.searchParams.set('code_challenge', codeChallenge); - loginUrl.searchParams.set('code_challenge_method', 'S256'); + let authUrl: string; + try { + authUrl = await buildAuthUrl(callback); + } catch (err) { + fail(err instanceof Error ? err : new Error('auth: Failed to build authorize URL.')); + return; + } - onUrl?.(loginUrl.href); - openBrowser(loginUrl.href, execFile); + onUrl?.(authUrl); + openBrowser(authUrl, execFile); }); - // 5-minute timeout - const timeout = setTimeout( - () => { - fail(new Error('auth: Login timed out after 5 minutes. Try again.')); - }, - 5 * 60 * 1000, - ); + const timeout = setTimeout(() => { + fail(new Error('auth: Login timed out after 5 minutes. Try again.')); + }, timeoutMs); timeout.unref(); }); } + +/** + * Start a local HTTP server, open the browser to the platform login page, + * and wait for the OAuth callback with a short-lived code. The CLI exchanges + * the code itself so the localhost callback never accepts bearer tokens. + * + * `onUrl` is invoked with the login URL once the local server is listening, + * so callers can show the URL to the user as a manual fallback when the + * browser fails to open (common on WSL2, headless SSH, Docker, etc.). + */ +export async function runLogin( + root: string = process.cwd(), + provider = 'github', + onUrl?: (url: string) => void, +): Promise { + const { execFile } = await import('node:child_process'); + + const apiUrl = getApiUrl(root); + const state = randomBytes(32).toString('hex'); + const codeVerifier = createCodeVerifier(); + const codeChallenge = createCodeChallenge(codeVerifier); + + // The exchange runs INSIDE the request handler (via finalize), so an exchange + // failure renders the failure page — exactly as the base login flow did. + return awaitBrowserCallback({ + state, + execFile, + onUrl, + buildAuthUrl: (callback) => { + const loginUrl = new URL('auth/login', apiUrl.endsWith('/') ? apiUrl : `${apiUrl}/`); + loginUrl.searchParams.set('redirect', callback.href); + loginUrl.searchParams.set('provider', provider); + loginUrl.searchParams.set('code_challenge', codeChallenge); + loginUrl.searchParams.set('code_challenge_method', 'S256'); + return loginUrl.href; + }, + describeError: (p) => + p.get('error') === 'signup_restricted' + ? 'Signup restricted: your email is not on the approved list for this environment.' + : null, + // Keep listening but reject these request shapes (preserves the original + // localhost server behavior): a stray `?token=` is never honored, and a + // callback without a `code` is ignored until the real one arrives. + validate: (p) => { + if (p.has('token')) { + return 'Invalid login callback. Please try again.'; + } + if (!p.get('code')) { + return 'No authorization code received. Please try again.'; + } + return null; + }, + finalize: (params) => exchangeLoginCode(apiUrl, params.get('code')!, state, codeVerifier), + }); +} diff --git a/packages/void/src/cli/cli.ts b/packages/void/src/cli/cli.ts index 486e285c3..6c702b1ce 100644 --- a/packages/void/src/cli/cli.ts +++ b/packages/void/src/cli/cli.ts @@ -201,7 +201,7 @@ if (helpPath) { }, { name: 'github', - hint: 'install | installations | connect', + hint: 'install | installations | connect | link', description: 'Connect a GitHub repository for automatic deploys', }, { diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index c37aaeaa6..5600e7945 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -929,4 +929,43 @@ export class PlatformClient { return res.json(); } + + async getGithubLinkUrl(redirect: string, codeChallenge: string): Promise<{ url: string }> { + const params = new URLSearchParams({ + redirect, + code_challenge: codeChallenge, + code_challenge_method: 'S256', + }); + const res = await fetch(`${this.baseUrl}/github/link-url?${params}`, { + headers: { + ...this.authHeaders, + }, + }); + + if (!res.ok) { + await throwPlatformApiError('Failed to start GitHub link', res); + } + + return res.json(); + } + + async completeGithubLink( + linkCode: string, + codeVerifier: string, + ): Promise<{ status: 'linked' | 'already_linked' }> { + const res = await fetch(`${this.baseUrl}/github/link/complete`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + ...this.authHeaders, + }, + body: JSON.stringify({ link_code: linkCode, code_verifier: codeVerifier }), + }); + + if (!res.ok) { + await throwPlatformApiError('Failed to complete GitHub link', res); + } + + return res.json(); + } } diff --git a/packages/void/src/cli/github-args.ts b/packages/void/src/cli/github-args.ts index 1b1e988c9..90729f39f 100644 --- a/packages/void/src/cli/github-args.ts +++ b/packages/void/src/cli/github-args.ts @@ -8,11 +8,13 @@ export const githubSubcommands: Array = [ arg: '[project]', description: 'Connect a repository to a project for auto-deploy', }, + { name: 'link', description: 'Link your GitHub account to Void' }, ]; export type GithubArgs = | { subcommand: 'install' } | { subcommand: 'installations' } + | { subcommand: 'link' } | { subcommand: 'connect'; projectName: string; @@ -39,6 +41,13 @@ export function parseGithubArgs(args: Array): GithubArgs { return { subcommand }; } + if (subcommand === 'link') { + if (rest.length > 0) { + throw new Error(`github: Unexpected argument '${rest[0]}'.`); + } + return { subcommand }; + } + if (subcommand === 'connect') { let projectName = ''; let installationId: string | undefined; @@ -111,5 +120,5 @@ export function parseGithubArgs(args: Array): GithubArgs { return { subcommand, projectName, installationId, repo, branch, executor }; } - throw new Error('github: Usage: void github .'); + throw new Error('github: Usage: void github .'); } diff --git a/packages/void/src/cli/github-cmd.ts b/packages/void/src/cli/github-cmd.ts index 76136c756..53b66f713 100644 --- a/packages/void/src/cli/github-cmd.ts +++ b/packages/void/src/cli/github-cmd.ts @@ -1,5 +1,12 @@ +import { randomBytes } from 'node:crypto'; import { cancel, isCancel, log, select, text } from '@clack/prompts'; -import { openBrowser, getToken } from './auth.ts'; +import { + awaitBrowserCallback, + createCodeChallenge, + createCodeVerifier, + openBrowser, + getToken, +} from './auth.ts'; import { PlatformApiError, PlatformClient, @@ -107,6 +114,8 @@ export async function runGithubCommand(root: string, args: GithubArgs): Promise< return runInstallations(client); case 'connect': return runConnect(root, client, args); + case 'link': + return runGithubLink(root, client); } } @@ -114,18 +123,54 @@ export async function runGithubCommand(root: string, args: GithubArgs): Promise< // install // --------------------------------------------------------------------------- -async function runInstall(root: string, client: PlatformClient): Promise { +/** + * `void github install` — print the GitHub App install URL and open it. + * + * If the account has no linked GitHub identity, the server returns a 403 with + * the stable code `github_identity_required` (Task 3). Rather than telling the + * user to `void auth login` (which risks forking into a second account), we run + * the link flow inline and retry the install URL exactly once. + * + * `linkIdentity` is injectable so tests can supply a fake linker instead of + * starting a real localhost callback server / browser. + */ +export async function runInstall( + root: string, + client: PlatformClient, + linkIdentity: (root: string, client: PlatformClient) => Promise = runGithubLink, +): Promise { let url: string; try { ({ url } = await client.getGithubInstallUrl()); } catch (err) { - if (err instanceof PlatformApiError && err.status === 403) { - log.error( - 'Your account has no linked GitHub identity. Run `void auth login` and choose GitHub.', - ); - process.exit(1); + if ( + err instanceof PlatformApiError && + err.status === 403 && + err.code === 'github_identity_required' + ) { + log.message('Your Void account needs a linked GitHub identity. Starting the GitHub link…'); + await linkIdentity(root, client); + try { + ({ url } = await client.getGithubInstallUrl()); + } catch (retryErr) { + // Only the SAME identity-required 403 means linking didn't take — show + // the actionable link/install guidance. Any other failure (401, 426, + // 500, a different 403) is the real error and must surface normally. + if ( + retryErr instanceof PlatformApiError && + retryErr.status === 403 && + retryErr.code === 'github_identity_required' + ) { + log.error( + 'Could not start the GitHub App install after linking. Try `void github link`, then `void github install` again.', + ); + process.exit(1); + } + throw retryErr; + } + } else { + throw err; } - throw err; } log.message(`Open this URL to install the Void GitHub App:\n${pc.cyan(url)}`); @@ -134,6 +179,70 @@ async function runInstall(root: string, client: PlatformClient): Promise { log.message('After installing, run `void github connect` to link a repository to your project.'); } +// --------------------------------------------------------------------------- +// link +// --------------------------------------------------------------------------- + +/** + * Link the current Void account (the logged-in CLI token holder) to a GitHub + * identity. Drives the platform's 3-endpoint link flow with a localhost + + * PKCE handshake, mirroring `void auth login`. + * + * Exported so Task 5 can call it (e.g. when `github install` returns 403 for a + * GitHub-less account, prompt to link and retry). The success path is + * non-exiting so a caller can continue; only error paths exit/throw. + */ +export async function runGithubLink(root: string, client: PlatformClient): Promise { + void root; // Reserved for the signature Task 5 reuses; routing uses `client`. + + const verifier = createCodeVerifier(); + const challenge = createCodeChallenge(verifier); + const state = randomBytes(32).toString('hex'); + + // completeGithubLink runs INSIDE the request handler (via finalize), so the + // browser only shows the success page after the link actually succeeds; a 409 + // conflict rejects out of awaitBrowserCallback and is mapped below. + let status: 'linked' | 'already_linked'; + try { + ({ status } = await awaitBrowserCallback<{ status: 'linked' | 'already_linked' }>({ + state, + buildAuthUrl: async (callback) => { + callback.searchParams.set('state', state); + const { url } = await client.getGithubLinkUrl(callback.href, challenge); + return url; + }, + describeError: (p) => (p.get('error') ? 'GitHub authorization failed.' : null), + onUrl: (url) => log.message(`Open this URL to link your GitHub account:\n${pc.cyan(url)}`), + finalize: async (params) => { + const linkCode = params.get('link_code'); + if (!linkCode) { + throw new Error('github: link callback did not return a link code.'); + } + return client.completeGithubLink(linkCode, verifier); + }, + })); + } catch (err) { + if (err instanceof PlatformApiError && err.code === 'github_already_linked') { + log.error('Your Void account is already linked to a different GitHub account.'); + process.exit(1); + } + if ( + err instanceof PlatformApiError && + (err.status === 409 || err.code === 'github_identity_conflict') + ) { + log.error('This GitHub account is already linked to another Void account.'); + process.exit(1); + } + throw err; + } + + if (status === 'already_linked') { + log.success('GitHub account already linked.'); + } else { + log.success('GitHub account linked.'); + } +} + // --------------------------------------------------------------------------- // installations // --------------------------------------------------------------------------- diff --git a/packages/void/src/cli/help.ts b/packages/void/src/cli/help.ts index e2bce200a..79487c6c6 100644 --- a/packages/void/src/cli/help.ts +++ b/packages/void/src/cli/help.ts @@ -200,6 +200,7 @@ const HELP_TREE: HelpTree = { command: 'connect [project]', description: 'Connect a repository to a project for auto-deploy', }, + { command: 'link', description: 'Link your GitHub account to Void' }, ], children: { install: basicSubcommand( @@ -210,6 +211,7 @@ const HELP_TREE: HelpTree = { 'List your GitHub App installations.', 'void github installations', ), + link: basicSubcommand('Link your GitHub account to Void.', 'void github link'), connect: { summary: 'Connect a repository to a project for automatic deploys.', usage: ['void github connect [project] [options]'], diff --git a/packages/void/test/unit/auth-callback.test.ts b/packages/void/test/unit/auth-callback.test.ts new file mode 100644 index 000000000..6742ce8bf --- /dev/null +++ b/packages/void/test/unit/auth-callback.test.ts @@ -0,0 +1,194 @@ +import { describe, expect, it, vi } from 'vite-plus/test'; +import { awaitBrowserCallback } from '../../src/cli/auth.ts'; + +// A no-op stand-in for node:child_process.execFile so the helper never tries +// to launch a real browser during tests. +const noopExecFile = vi.fn() as unknown as typeof import('node:child_process').execFile; + +/** + * Start awaitBrowserCallback and resolve once it hands us the authorize URL + * (built from the localhost callback). Returns the callback URL so the test can + * drive the localhost server directly. + */ +function startCallback(options?: { + describeError?: (params: URLSearchParams) => string | null; + finalize?: (params: URLSearchParams) => Promise; + state?: string; +}): { + resultPromise: Promise; + callbackPromise: Promise; + state: string; +} { + const state = options?.state ?? 'test-state-123'; + let resolveCallback!: (url: URL) => void; + const callbackPromise = new Promise((resolve) => { + resolveCallback = resolve; + }); + + const finalize = + options?.finalize ?? ((params: URLSearchParams) => Promise.resolve(params as unknown as T)); + + const resultPromise = awaitBrowserCallback({ + state, + execFile: noopExecFile, + describeError: options?.describeError, + finalize, + buildAuthUrl: (callback) => { + resolveCallback(callback); + return 'https://example.test/authorize'; + }, + }); + + return { resultPromise, callbackPromise, state }; +} + +describe('awaitBrowserCallback', () => { + it('runs finalize in the request path and resolves with its return value', async () => { + const finalize = vi.fn(async (params: URLSearchParams) => ({ + linked: params.get('link_code'), + })); + const { resultPromise, callbackPromise, state } = startCallback({ finalize }); + const callback = await callbackPromise; + + // The helper appends the caller-supplied state to the callback URL. + expect(callback.pathname).toBe('/callback'); + expect(callback.searchParams.get('state')).toBe(state); + expect(callback.protocol).toBe('http:'); + expect(callback.hostname).toBe('localhost'); + + const hit = new URL(callback); + hit.searchParams.set('link_code', 'abc'); + + const res = await fetch(hit); + expect(res.status).toBe(200); + + // finalize ran inside the request handler with the callback params, and the + // outer promise resolves with finalize's return value. + expect(finalize).toHaveBeenCalledTimes(1); + expect(finalize.mock.calls[0]![0].get('link_code')).toBe('abc'); + expect(finalize.mock.calls[0]![0].get('state')).toBe(state); + + await expect(resultPromise).resolves.toEqual({ linked: 'abc' }); + }); + + it('renders the failure page and rejects when finalize throws (F3 regression pin)', async () => { + const finalizeError = new Error('exchange blew up'); + const finalize = vi.fn(async () => { + throw finalizeError; + }); + const { resultPromise, callbackPromise, state } = startCallback({ finalize }); + const callback = await callbackPromise; + + // Attach the rejection assertion before triggering the request so the + // handler is in place when the promise settles (no unhandled rejection). + const rejection = expect(resultPromise).rejects.toBe(finalizeError); + + const hit = new URL(callback); + hit.searchParams.set('state', state); + hit.searchParams.set('link_code', 'abc'); + + const res = await fetch(hit); + // The browser must NOT see the success page when finalize fails. + expect(res.status).toBe(400); + const body = await res.text(); + expect(body).not.toContain('successful'); + expect(body).toContain('Could not complete'); + + expect(finalize).toHaveBeenCalledTimes(1); + await rejection; + }); + + it('returns 400 and keeps listening on a state mismatch (does not call finalize)', async () => { + const finalize = vi.fn(async (params: URLSearchParams) => params); + const { resultPromise, callbackPromise, state } = startCallback({ finalize }); + const callback = await callbackPromise; + + const bad = new URL(callback); + bad.searchParams.set('state', 'wrong-state'); + bad.searchParams.set('link_code', 'attacker'); + + const badRes = await fetch(bad); + expect(badRes.status).toBe(400); + + // A state mismatch is rejected before finalize runs. + expect(finalize).not.toHaveBeenCalled(); + + // The genuine callback still completes — the server stayed open. + const good = new URL(callback); + good.searchParams.set('state', state); + good.searchParams.set('link_code', 'real'); + + const goodRes = await fetch(good); + expect(goodRes.status).toBe(200); + + const params = await resultPromise; + expect(finalize).toHaveBeenCalledTimes(1); + expect(params.get('link_code')).toBe('real'); + }); + + it('renders 403 and rejects when describeError returns a message', async () => { + const { resultPromise, callbackPromise, state } = startCallback({ + describeError: (p) => (p.get('error') ? 'GitHub authorization failed.' : null), + }); + const callback = await callbackPromise; + + // Attach the rejection assertion before triggering the request so the + // handler is in place when the promise settles (no unhandled rejection). + const rejection = expect(resultPromise).rejects.toThrow('GitHub authorization failed.'); + + const hit = new URL(callback); + hit.searchParams.set('state', state); + hit.searchParams.set('error', 'access_denied'); + + const res = await fetch(hit); + expect(res.status).toBe(403); + + await rejection; + }); + + it('rejects after the timeout elapses', async () => { + const resultPromise = awaitBrowserCallback({ + state: 'timeout-state', + execFile: noopExecFile, + timeoutMs: 10, + finalize: async (params) => params, + buildAuthUrl: (callback) => + `https://example.test/authorize?cb=${encodeURIComponent(callback.href)}`, + }); + + await expect(resultPromise).rejects.toThrow(/timed out/); + }); + + it('does not open a real browser (execFile is injected)', async () => { + const execFile = vi.fn() as unknown as typeof import('node:child_process').execFile; + let resolveCallback!: (url: URL) => void; + const callbackPromise = new Promise((resolve) => { + resolveCallback = resolve; + }); + + const resultPromise = awaitBrowserCallback({ + state: 's', + execFile, + finalize: async (params) => params, + buildAuthUrl: (callback) => { + resolveCallback(callback); + return 'https://example.test/authorize'; + }, + }); + + const callback = await callbackPromise; + const hit = new URL(callback); + hit.searchParams.set('state', 's'); + await fetch(hit); + await resultPromise; + + // By the time the flow completes, the injected execFile (never a real + // browser) was invoked with the authorize URL as a single argv element. + expect(execFile).toHaveBeenCalledTimes(1); + expect(execFile).toHaveBeenCalledWith( + expect.anything(), + expect.arrayContaining(['https://example.test/authorize']), + expect.any(Function), + ); + }); +}); diff --git a/packages/void/test/unit/auth-login.test.ts b/packages/void/test/unit/auth-login.test.ts index ab94b8099..fe301ecbf 100644 --- a/packages/void/test/unit/auth-login.test.ts +++ b/packages/void/test/unit/auth-login.test.ts @@ -138,6 +138,46 @@ describe('runLogin', () => { await expect(loginPromise).resolves.toBe('real-token'); }); + it('shows the failure page and throws when the code exchange fails (F3 regression)', async () => { + // Override fetch so the localhost callback still hits the real server, but + // the API token exchange returns a non-ok response (expired code, etc.). + vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => { + const url = + input instanceof Request ? input.url : input instanceof URL ? input.href : input.toString(); + if (url.startsWith('http://localhost:')) { + return originalFetch(input, init); + } + return new Response('expired', { status: 400 }); + }); + + let resolveLoginUrl!: (url: string) => void; + const loginUrlPromise = new Promise((resolve) => { + resolveLoginUrl = resolve; + }); + + const loginPromise = runLogin('/project', 'github', resolveLoginUrl); + // Attach the rejection assertion up front so the exchange failure doesn't + // surface as an unhandled rejection. + const rejection = expect(loginPromise).rejects.toThrow(/Failed to exchange login code/); + + const loginUrl = new URL(await loginUrlPromise); + const redirect = loginUrl.searchParams.get('redirect'); + expect(redirect).toBeTruthy(); + + const validUrl = new URL(redirect!); + validUrl.searchParams.set('code', 'real-code'); + + const validResponse = await fetch(validUrl); + // The exchange ran INSIDE the request handler and failed → failure page, + // never the success page. + expect(validResponse.status).toBe(400); + const body = await validResponse.text(); + expect(body).not.toContain('successful'); + expect(body).toContain('Could not complete'); + + await rejection; + }); + it('sends Cloudflare Access headers when exchanging the login code', async () => { process.env.CF_ACCESS_TOKEN = 'cf-access-token'; diff --git a/packages/void/test/unit/github-args.test.ts b/packages/void/test/unit/github-args.test.ts index 9d410595a..15522124e 100644 --- a/packages/void/test/unit/github-args.test.ts +++ b/packages/void/test/unit/github-args.test.ts @@ -285,16 +285,34 @@ describe('parseGithubArgs', () => { }); }); + describe('link', () => { + it('parses link with no args', () => { + expect(parseGithubArgs(['link'])).toEqual({ subcommand: 'link' }); + }); + + it('rejects extra positional', () => { + expect(() => parseGithubArgs(['link', 'extra'])).toThrow( + "github: Unexpected argument 'extra'.", + ); + }); + + it('rejects a flag', () => { + expect(() => parseGithubArgs(['link', '--flag'])).toThrow( + "github: Unexpected argument '--flag'.", + ); + }); + }); + describe('unknown subcommand', () => { it('throws on empty args', () => { expect(() => parseGithubArgs([])).toThrow( - 'github: Usage: void github .', + 'github: Usage: void github .', ); }); it('throws on unknown subcommand', () => { expect(() => parseGithubArgs(['unknown'])).toThrow( - 'github: Usage: void github .', + 'github: Usage: void github .', ); }); }); diff --git a/packages/void/test/unit/github-client.test.ts b/packages/void/test/unit/github-client.test.ts index c61284cd1..de25cc6e4 100644 --- a/packages/void/test/unit/github-client.test.ts +++ b/packages/void/test/unit/github-client.test.ts @@ -177,3 +177,77 @@ describe('PlatformClient.connectGithub', () => { }); }); }); + +describe('PlatformClient.getGithubLinkUrl', () => { + it('GETs link-url with redirect + S256 challenge query, auth headers, returns { url }', async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ url: 'https://github.com/login/oauth/authorize?client_id=x' }), + ); + + const client = makeClient(); + const redirect = 'http://localhost:54321/callback?state=abc'; + const result = await client.getGithubLinkUrl(redirect, 'challenge-xyz'); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + const parsed = new URL(url); + expect(parsed.origin + parsed.pathname).toBe(`${apiUrl}/github/link-url`); + expect(parsed.searchParams.get('redirect')).toBe(redirect); + expect(parsed.searchParams.get('code_challenge')).toBe('challenge-xyz'); + expect(parsed.searchParams.get('code_challenge_method')).toBe('S256'); + expect(init.method).toBeUndefined(); + const headers = init.headers as Record; + expect(headers.Authorization).toBe(`Bearer ${token}`); + expect(result).toEqual({ url: 'https://github.com/login/oauth/authorize?client_id=x' }); + }); + + it('throws PlatformApiError on non-2xx', async () => { + fetchMock.mockResolvedValueOnce(new Response('forbidden', { status: 403 })); + const client = makeClient(); + await expect(client.getGithubLinkUrl('http://localhost:1/callback', 'c')).rejects.toMatchObject( + { + name: 'PlatformApiError', + status: 403, + }, + ); + }); +}); + +describe('PlatformClient.completeGithubLink', () => { + it('POSTs link/complete with exact JSON body + auth headers, returns { status }', async () => { + fetchMock.mockResolvedValueOnce(jsonResponse({ status: 'linked' })); + + const client = makeClient(); + const result = await client.completeGithubLink('link-code-1', 'verifier-1'); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe(`${apiUrl}/github/link/complete`); + expect(init.method).toBe('POST'); + const headers = init.headers as Record; + expect(headers['Content-Type']).toBe('application/json'); + expect(headers.Authorization).toBe(`Bearer ${token}`); + expect(JSON.parse(init.body as string)).toEqual({ + link_code: 'link-code-1', + code_verifier: 'verifier-1', + }); + expect(result).toEqual({ status: 'linked' }); + }); + + it('returns already_linked status', async () => { + fetchMock.mockResolvedValueOnce(jsonResponse({ status: 'already_linked' })); + const client = makeClient(); + const result = await client.completeGithubLink('link-code-2', 'verifier-2'); + expect(result).toEqual({ status: 'already_linked' }); + }); + + it('throws PlatformApiError with status 409 and code on github_identity_conflict', async () => { + fetchMock.mockResolvedValueOnce(jsonResponse({ error: 'github_identity_conflict' }, 409)); + const client = makeClient(); + await expect(client.completeGithubLink('link-code-3', 'verifier-3')).rejects.toMatchObject({ + name: 'PlatformApiError', + status: 409, + code: 'github_identity_conflict', + }); + }); +}); diff --git a/packages/void/test/unit/github-install-cmd.test.ts b/packages/void/test/unit/github-install-cmd.test.ts new file mode 100644 index 000000000..fd2620150 --- /dev/null +++ b/packages/void/test/unit/github-install-cmd.test.ts @@ -0,0 +1,142 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test'; +import { PlatformApiError, type PlatformClient } from '../../src/cli/client.ts'; +import { runInstall } from '../../src/cli/github-cmd.ts'; + +// Mock node:child_process so openBrowser never launches a real browser on the +// success path. +const mockExecFile = vi.fn(); +vi.mock('node:child_process', () => ({ + execFile: (...args: Array) => mockExecFile(...args), +})); + +let exitSpy: ReturnType; + +function identityRequired(): PlatformApiError { + return new PlatformApiError( + 'Failed to get GitHub install URL', + 403, + JSON.stringify({ error: 'github_identity_required' }), + ); +} + +beforeEach(() => { + mockExecFile.mockReset(); + // process.exit -> throw so we can assert the error-path exit without killing + // the test runner. + exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); +}); + +afterEach(() => { + exitSpy.mockRestore(); + vi.unstubAllGlobals(); +}); + +describe('runInstall', () => { + it('auto-links then retries getGithubInstallUrl once on a github_identity_required 403', async () => { + const getGithubInstallUrl = vi + .fn() + .mockRejectedValueOnce(identityRequired()) + .mockResolvedValueOnce({ url: 'https://github.com/apps/void/installations/new' }); + const client = { getGithubInstallUrl } as unknown as PlatformClient; + const linkIdentity = vi.fn(async () => {}); + + await runInstall('/project', client, linkIdentity); + + expect(linkIdentity).toHaveBeenCalledTimes(1); + expect(linkIdentity).toHaveBeenCalledWith('/project', client); + expect(getGithubInstallUrl).toHaveBeenCalledTimes(2); + // The install URL was opened in the browser on the success retry. + expect(mockExecFile).toHaveBeenCalledTimes(1); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it('rethrows a non-identity 403 without auto-linking', async () => { + const err = new PlatformApiError( + 'Failed to get GitHub install URL', + 403, + JSON.stringify({ error: 'something_else' }), + ); + const getGithubInstallUrl = vi.fn().mockRejectedValue(err); + const client = { getGithubInstallUrl } as unknown as PlatformClient; + const linkIdentity = vi.fn(async () => {}); + + await expect(runInstall('/project', client, linkIdentity)).rejects.toBe(err); + + expect(linkIdentity).not.toHaveBeenCalled(); + expect(getGithubInstallUrl).toHaveBeenCalledTimes(1); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it('rethrows a non-403 error without auto-linking', async () => { + const err = new PlatformApiError('Failed to get GitHub install URL', 500, 'boom'); + const getGithubInstallUrl = vi.fn().mockRejectedValue(err); + const client = { getGithubInstallUrl } as unknown as PlatformClient; + const linkIdentity = vi.fn(async () => {}); + + await expect(runInstall('/project', client, linkIdentity)).rejects.toBe(err); + + expect(linkIdentity).not.toHaveBeenCalled(); + expect(getGithubInstallUrl).toHaveBeenCalledTimes(1); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it('exits(1) with actionable guidance when the retry still fails after linking', async () => { + const getGithubInstallUrl = vi + .fn() + .mockRejectedValueOnce(identityRequired()) + .mockRejectedValueOnce(identityRequired()); + const client = { getGithubInstallUrl } as unknown as PlatformClient; + const linkIdentity = vi.fn(async () => {}); + + await expect(runInstall('/project', client, linkIdentity)).rejects.toThrow('process.exit(1)'); + + expect(linkIdentity).toHaveBeenCalledTimes(1); + expect(getGithubInstallUrl).toHaveBeenCalledTimes(2); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it('rethrows a non-identity error from the post-link retry (F4)', async () => { + // After linking, the install URL fails again — but with a DIFFERENT error + // (a 500). The generic "try void github link" guidance must NOT swallow it; + // the real error has to surface through normal handling. + const retryErr = new PlatformApiError('Failed to get GitHub install URL', 500, 'boom'); + const getGithubInstallUrl = vi + .fn() + .mockRejectedValueOnce(identityRequired()) + .mockRejectedValueOnce(retryErr); + const client = { getGithubInstallUrl } as unknown as PlatformClient; + const linkIdentity = vi.fn(async () => {}); + + await expect(runInstall('/project', client, linkIdentity)).rejects.toBe(retryErr); + + expect(linkIdentity).toHaveBeenCalledTimes(1); + expect(getGithubInstallUrl).toHaveBeenCalledTimes(2); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it('rethrows a 426 cli_outdated from the post-link retry (F4)', async () => { + const retryErr = new PlatformApiError( + 'Failed to get GitHub install URL', + 426, + JSON.stringify({ error: 'cli_outdated', message: 'Upgrade: npm i -g void@latest' }), + ); + const getGithubInstallUrl = vi + .fn() + .mockRejectedValueOnce(identityRequired()) + .mockRejectedValueOnce(retryErr); + const client = { getGithubInstallUrl } as unknown as PlatformClient; + const linkIdentity = vi.fn(async () => {}); + + await expect(runInstall('/project', client, linkIdentity)).rejects.toBe(retryErr); + + expect(linkIdentity).toHaveBeenCalledTimes(1); + expect(getGithubInstallUrl).toHaveBeenCalledTimes(2); + expect(exitSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/void/test/unit/github-link-cmd.test.ts b/packages/void/test/unit/github-link-cmd.test.ts new file mode 100644 index 000000000..3cfdfd8b8 --- /dev/null +++ b/packages/void/test/unit/github-link-cmd.test.ts @@ -0,0 +1,178 @@ +import { log } from '@clack/prompts'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test'; +import { PlatformApiError, type PlatformClient } from '../../src/cli/client.ts'; +import { runGithubLink } from '../../src/cli/github-cmd.ts'; + +// Mock node:child_process so openBrowser never launches a real browser; the +// localhost callback server is driven directly by fetch in each test. +const mockExecFile = vi.fn(); +vi.mock('node:child_process', () => ({ + execFile: (...args: Array) => mockExecFile(...args), +})); + +const originalFetch = globalThis.fetch.bind(globalThis); + +type FakeClient = Pick; + +let exitSpy: ReturnType; + +beforeEach(() => { + mockExecFile.mockReset(); + // process.exit -> throw so we can assert the error-path exit without killing + // the test runner. + exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); +}); + +afterEach(() => { + exitSpy.mockRestore(); + vi.unstubAllGlobals(); +}); + +/** + * Run runGithubLink against a fake client, intercepting the authorize URL the + * command requests (via the fake getGithubLinkUrl) so the test can POST back to + * the localhost callback with whatever query it wants. + * + * `completeGithubLink` and the callback query are configurable per test. + */ +function drive(opts: { + completeGithubLink: FakeClient['completeGithubLink']; + callbackQuery: (callbackUrl: URL) => URL; +}): Promise { + let resolveCallback!: (url: URL) => void; + const callbackReady = new Promise((resolve) => { + resolveCallback = resolve; + }); + + const client: FakeClient = { + getGithubLinkUrl: vi.fn(async (redirect: string) => { + // `redirect` is the localhost callback URL (with the command's state). + resolveCallback(new URL(redirect)); + return { url: 'https://github.com/login/oauth/authorize?client_id=x' }; + }), + completeGithubLink: opts.completeGithubLink, + }; + + const run = runGithubLink('/project', client as unknown as PlatformClient); + + // Once the command has built the callback URL, fire the simulated browser + // redirect back at the localhost server. Swallow fetch errors: on the + // error paths the server is torn down mid-response (process.exit / throw), + // which can surface as ECONNRESET on the in-flight request — irrelevant to + // what we're asserting (the command's outcome). + const fired = callbackReady.then(async (callbackUrl) => { + const hit = opts.callbackQuery(callbackUrl); + await originalFetch(hit).catch(() => {}); + }); + + // Settle on the command's outcome, but make sure the fetch is also drained so + // no dangling promise leaks across tests. + return run.finally(() => fired.catch(() => {})); +} + +describe('runGithubLink', () => { + it('completes the link on a successful callback (status: linked)', async () => { + const completeGithubLink = vi.fn(async () => ({ status: 'linked' as const })); + + await drive({ + completeGithubLink, + callbackQuery: (cb) => { + const hit = new URL(cb); // already carries the command's state + hit.searchParams.set('link_code', 'lc-123'); + return hit; + }, + }); + + expect(completeGithubLink).toHaveBeenCalledTimes(1); + expect(completeGithubLink).toHaveBeenCalledWith('lc-123', expect.any(String)); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it('exits(1) on a 409 github_identity_conflict', async () => { + const completeGithubLink = vi.fn(async () => { + throw new PlatformApiError( + 'Failed to complete GitHub link', + 409, + JSON.stringify({ error: 'github_identity_conflict' }), + ); + }); + + await expect( + drive({ + completeGithubLink, + callbackQuery: (cb) => { + const hit = new URL(cb); + hit.searchParams.set('link_code', 'lc-409'); + return hit; + }, + }), + ).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it('exits(1) with a distinct message on a 409 github_already_linked', async () => { + const errorSpy = vi.spyOn(log, 'error').mockImplementation(() => {}); + const completeGithubLink = vi.fn(async () => { + throw new PlatformApiError( + 'Failed to complete GitHub link', + 409, + JSON.stringify({ error: 'github_already_linked' }), + ); + }); + + await expect( + drive({ + completeGithubLink, + callbackQuery: (cb) => { + const hit = new URL(cb); + hit.searchParams.set('link_code', 'lc-already'); + return hit; + }, + }), + ).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + // The new code maps to the "your account, different GitHub" message — + // distinct from the other-user github_identity_conflict wording. + expect(errorSpy).toHaveBeenCalledWith( + 'Your Void account is already linked to a different GitHub account.', + ); + errorSpy.mockRestore(); + }); + + it('throws when the callback returns no link code', async () => { + const completeGithubLink = vi.fn(async () => ({ status: 'linked' as const })); + + await expect( + drive({ + completeGithubLink, + // valid state, but no link_code in the query + callbackQuery: (cb) => new URL(cb), + }), + ).rejects.toThrow('did not return a link code'); + + expect(completeGithubLink).not.toHaveBeenCalled(); + }); + + it('re-throws non-conflict PlatformApiErrors', async () => { + const completeGithubLink = vi.fn(async () => { + throw new PlatformApiError('Failed to complete GitHub link', 500, 'boom'); + }); + + await expect( + drive({ + completeGithubLink, + callbackQuery: (cb) => { + const hit = new URL(cb); + hit.searchParams.set('link_code', 'lc-500'); + return hit; + }, + }), + ).rejects.toMatchObject({ name: 'PlatformApiError', status: 500 }); + + expect(exitSpy).not.toHaveBeenCalled(); + }); +});