From 4e81d954e0f35c375e0b53d397e482abaeee80f0 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 23 Jun 2026 13:32:22 +0800 Subject: [PATCH 1/5] feat(cli): add void github link to link a GitHub identity Adds `void github link`, which links the logged-in Void account to a GitHub identity by driving the platform's 3-endpoint link flow with a localhost + PKCE handshake (mirrors `void auth login`). - Extract the inline localhost-callback server from `runLogin` into a shared, exported `awaitBrowserCallback` helper; refactor `runLogin` to use it (behavior-preserving). Export the PKCE helpers `createCodeVerifier` / `createCodeChallenge`. - Add `PlatformClient.getGithubLinkUrl` and `completeGithubLink`. - Add `runGithubLink(root, client)`; wire `link` into github args, dispatch, CLI hint, help, and docs. - New tests: `auth-callback.test.ts` (real coverage of the previously-untested localhost flow) and `github-link-cmd.test.ts` (conflict/missing-code paths), plus client + args coverage. Regression gate `auth-login` stays green. Co-Authored-By: Claude Opus 4.8 --- docs/reference/cli.md | 10 + packages/void/src/cli/auth.ts | 196 +++++++++++------- packages/void/src/cli/cli.ts | 2 +- packages/void/src/cli/client.ts | 39 ++++ packages/void/src/cli/github-args.ts | 11 +- packages/void/src/cli/github-cmd.ts | 68 +++++- packages/void/src/cli/help.ts | 2 + packages/void/test/unit/auth-callback.test.ts | 148 +++++++++++++ packages/void/test/unit/github-args.test.ts | 22 +- packages/void/test/unit/github-client.test.ts | 74 +++++++ .../void/test/unit/github-link-cmd.test.ts | 147 +++++++++++++ 11 files changed, 643 insertions(+), 76 deletions(-) create mode 100644 packages/void/test/unit/auth-callback.test.ts create mode 100644 packages/void/test/unit/github-link-cmd.test.ts diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 4ab8a5c0c..72acd29d1 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -568,6 +568,16 @@ See [Environment Variables](../guide/env-vars.md) for the full guide. ## GitHub +### `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). + +Run this when your account has no linked GitHub identity yet — for example if `void github install` reports that your account has no linked GitHub identity. If the 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` ``` diff --git a/packages/void/src/cli/auth.ts b/packages/void/src/cli/auth.ts index 59e819c57..fadf22d86 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,48 @@ 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, renders a success/error HTML page, and resolves with the + * callback request's search params (the caller reads `code` / `link_code` etc.). + * 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; + /** 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, 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,16 +316,16 @@ export async function runLogin( reject(error); }; - const succeed = (token: string): void => { + const succeed = (params: URLSearchParams): void => { if (settled) { return; } settled = true; server.close(); - resolve(token); + resolve(params); }; - const server = http.createServer(async (req, res) => { + const server = http.createServer((req, res) => { const url = new URL(req.url!, `http://localhost`); if (url.pathname !== '/callback') { res.writeHead(404); @@ -319,79 +339,113 @@ 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.', - ), - ); + res.end(page('Login failed', errorMessage)); + fail(new Error(errorMessage)); return; } - if (url.searchParams.has('token')) { + // 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', 'Invalid login callback. Please try again.')); - return; - } - - const code = url.searchParams.get('code'); - if (!code) { - res.writeHead(400, { 'Content-Type': 'text/html' }); - res.end(page('Login failed', 'No authorization code received. Please try again.')); - return; - } - - let token: string; - try { - token = await exchangeLoginCode(apiUrl, code, state, codeVerifier); - } catch (err) { - 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.')); + res.end(page('Login failed', invalidMessage)); 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(url.searchParams); }); // 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); + + const params = await 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; + }, + }); + + const code = params.get('code')!; + return exchangeLoginCode(apiUrl, 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..62b2eda76 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); } } @@ -134,6 +143,63 @@ 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'); + + const params = await awaitBrowserCallback({ + 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)}`), + }); + + const linkCode = params.get('link_code'); + if (!linkCode) { + throw new Error('github: link callback did not return a link code.'); + } + + let status: 'linked' | 'already_linked'; + try { + ({ status } = await client.completeGithubLink(linkCode, verifier)); + } catch (err) { + 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..9577bd8d7 --- /dev/null +++ b/packages/void/test/unit/auth-callback.test.ts @@ -0,0 +1,148 @@ +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; + 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 resultPromise = awaitBrowserCallback({ + state, + execFile: noopExecFile, + describeError: options?.describeError, + buildAuthUrl: (callback) => { + resolveCallback(callback); + return 'https://example.test/authorize'; + }, + }); + + return { resultPromise, callbackPromise, state }; +} + +describe('awaitBrowserCallback', () => { + it('resolves with the callback params on a valid /callback request', async () => { + const { resultPromise, callbackPromise, state } = startCallback(); + 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); + + const params = await resultPromise; + expect(params.get('link_code')).toBe('abc'); + expect(params.get('state')).toBe(state); + }); + + it('returns 400 and keeps listening on a state mismatch (does not resolve)', async () => { + const { resultPromise, callbackPromise, state } = startCallback(); + 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); + + // 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(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, + 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, + 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/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-link-cmd.test.ts b/packages/void/test/unit/github-link-cmd.test.ts new file mode 100644 index 000000000..819e74d56 --- /dev/null +++ b/packages/void/test/unit/github-link-cmd.test.ts @@ -0,0 +1,147 @@ +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('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(); + }); +}); From 230e9cb2fb1a478c2b11ee8123f4ca2ff1e73f76 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 23 Jun 2026 13:40:14 +0800 Subject: [PATCH 2/5] feat(cli): auto-link GitHub identity on `void github install` When `void github install` returns a 403 with the stable code `github_identity_required`, run the GitHub link flow inline and retry the install URL exactly once instead of telling the user to `void auth login` (which risks forking into a second account). The link step is injectable (`runInstall(root, client, linkIdentity)`) so tests can supply a fake linker without a real localhost server or browser. Only a `github_identity_required` 403 triggers the auto-link; every other error is rethrown unchanged. If the post-link retry still fails, exit(1) with actionable guidance. Co-Authored-By: Claude Opus 4.8 --- packages/void/src/cli/github-cmd.ts | 40 +++++-- .../void/test/unit/github-install-cmd.test.ts | 102 ++++++++++++++++++ 2 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 packages/void/test/unit/github-install-cmd.test.ts diff --git a/packages/void/src/cli/github-cmd.ts b/packages/void/src/cli/github-cmd.ts index 62b2eda76..8f312ba92 100644 --- a/packages/void/src/cli/github-cmd.ts +++ b/packages/void/src/cli/github-cmd.ts @@ -123,18 +123,44 @@ 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 { + log.error( + 'Could not start the GitHub App install after linking. Try `void github link`, then `void github install` again.', + ); + process.exit(1); + } + } else { + throw err; } - throw err; } log.message(`Open this URL to install the Void GitHub App:\n${pc.cyan(url)}`); 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..2bd7dd074 --- /dev/null +++ b/packages/void/test/unit/github-install-cmd.test.ts @@ -0,0 +1,102 @@ +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); + }); +}); From cc208b6d64d8c489809e92309695bf3949b025fe Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 23 Jun 2026 13:43:26 +0800 Subject: [PATCH 3/5] docs(cli): reconcile void github section for auto-link + any login provider void github install now auto-runs the link flow on github_identity_required (RFC 0074), so the install/link cross-references were stale. Clarify that Deploy-on-GitHub works from any login provider after a one-time identity link. Co-Authored-By: Claude Opus 4.8 --- docs/reference/cli.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 72acd29d1..4f4179b78 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -568,6 +568,8 @@ 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` ``` @@ -576,7 +578,7 @@ 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). -Run this when your account has no linked GitHub identity yet — for example if `void github install` reports that your account has no linked GitHub identity. If the GitHub account is already linked to a different Void account, the command fails and asks you to use a different GitHub account. +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` @@ -584,7 +586,7 @@ Run this when your account has no linked GitHub identity yet — for example if 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` From 025cc4b619d16c91b1f211d0e6820bf1ab73e3f0 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 23 Jun 2026 14:13:28 +0800 Subject: [PATCH 4/5] fix(cli): distinct message for github_already_linked vs github_identity_conflict The link-complete server now returns two distinct 409 codes (RFC 0074 F1): github_identity_conflict (the GitHub account belongs to another Void account) vs github_already_linked (your account already has a different GitHub identity). runGithubLink mapped any 409 to the former's wording; key on err.code so each case shows the right message. Co-Authored-By: Claude Opus 4.8 --- packages/void/src/cli/github-cmd.ts | 4 +++ .../void/test/unit/github-link-cmd.test.ts | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/packages/void/src/cli/github-cmd.ts b/packages/void/src/cli/github-cmd.ts index 8f312ba92..5c90f8ef0 100644 --- a/packages/void/src/cli/github-cmd.ts +++ b/packages/void/src/cli/github-cmd.ts @@ -209,6 +209,10 @@ export async function runGithubLink(root: string, client: PlatformClient): Promi try { ({ status } = await 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') diff --git a/packages/void/test/unit/github-link-cmd.test.ts b/packages/void/test/unit/github-link-cmd.test.ts index 819e74d56..3cfdfd8b8 100644 --- a/packages/void/test/unit/github-link-cmd.test.ts +++ b/packages/void/test/unit/github-link-cmd.test.ts @@ -1,3 +1,4 @@ +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'; @@ -112,6 +113,36 @@ describe('runGithubLink', () => { 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 })); From 63bd41bcae87dbee961ebfb6385496bb71831cee Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 23 Jun 2026 14:24:24 +0800 Subject: [PATCH 5/5] fix(cli): finalize browser callback in request path; rethrow real install retry errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F3: awaitBrowserCallback now runs a generic finalize() hook inside the request handler before the success page. On finalize success it shows the success page and resolves with the result; on finalize throw it shows the failure page and rejects. This restores base runLogin behavior (exchange in the request path, failure page on exchange failure) and makes `void github link` show success only after completeGithubLink succeeds. runLogin and runGithubLink refactored onto the hook. F4: runInstall's post-link retry no longer swallows all errors — it only keeps the actionable link/install guidance when the retry is still a 403 github_identity_required, and rethrows every other failure (401/426/500/other). Co-Authored-By: Claude Opus 4.8 --- packages/void/src/cli/auth.ts | 61 ++++++++++++----- packages/void/src/cli/github-cmd.ts | 57 +++++++++------- packages/void/test/unit/auth-callback.test.ts | 66 ++++++++++++++++--- packages/void/test/unit/auth-login.test.ts | 40 +++++++++++ .../void/test/unit/github-install-cmd.test.ts | 40 +++++++++++ 5 files changed, 217 insertions(+), 47 deletions(-) diff --git a/packages/void/src/cli/auth.ts b/packages/void/src/cli/auth.ts index fadf22d86..6cbe28ab6 100644 --- a/packages/void/src/cli/auth.ts +++ b/packages/void/src/cli/auth.ts @@ -267,9 +267,12 @@ export function openBrowser( * authorize URL, and wait for the redirect back to `http://localhost:PORT/callback`. * * The server validates the `state` it appended to the callback URL against the - * incoming request, renders a success/error HTML page, and resolves with the - * callback request's search params (the caller reads `code` / `link_code` etc.). - * The server is always closed before settling. + * 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 @@ -278,11 +281,18 @@ export function openBrowser( * * `execFile` is injectable so tests can avoid launching a real browser. */ -export async function awaitBrowserCallback(options: { +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; /** @@ -298,13 +308,13 @@ export async function awaitBrowserCallback(options: { timeoutMs?: number; /** Injectable for tests; defaults to the real `node:child_process` `execFile`. */ execFile?: typeof import('node:child_process').execFile; -}): Promise { +}): Promise { const http = await import('node:http'); - const { state, buildAuthUrl, describeError, validate, onUrl } = options; + 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; - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { let settled = false; const fail = (error: Error): void => { @@ -316,16 +326,16 @@ export async function awaitBrowserCallback(options: { reject(error); }; - const succeed = (params: URLSearchParams): void => { + const succeed = (result: T): void => { if (settled) { return; } settled = true; server.close(); - resolve(params); + resolve(result); }; - const server = http.createServer((req, res) => { + const server = http.createServer(async (req, res) => { const url = new URL(req.url!, `http://localhost`); if (url.pathname !== '/callback') { res.writeHead(404); @@ -357,9 +367,30 @@ export async function awaitBrowserCallback(options: { return; } + // 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 { + 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(url.searchParams); + succeed(result); }); // Close immediately after callback — don't wait for keep-alive timeout @@ -416,7 +447,9 @@ export async function runLogin( const codeVerifier = createCodeVerifier(); const codeChallenge = createCodeChallenge(codeVerifier); - const params = await awaitBrowserCallback({ + // 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, @@ -444,8 +477,6 @@ export async function runLogin( } return null; }, + finalize: (params) => exchangeLoginCode(apiUrl, params.get('code')!, state, codeVerifier), }); - - const code = params.get('code')!; - return exchangeLoginCode(apiUrl, code, state, codeVerifier); } diff --git a/packages/void/src/cli/github-cmd.ts b/packages/void/src/cli/github-cmd.ts index 5c90f8ef0..53b66f713 100644 --- a/packages/void/src/cli/github-cmd.ts +++ b/packages/void/src/cli/github-cmd.ts @@ -152,11 +152,21 @@ export async function runInstall( await linkIdentity(root, client); try { ({ url } = await client.getGithubInstallUrl()); - } catch { - log.error( - 'Could not start the GitHub App install after linking. Try `void github link`, then `void github install` again.', - ); - process.exit(1); + } 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; @@ -189,25 +199,28 @@ export async function runGithubLink(root: string, client: PlatformClient): Promi const challenge = createCodeChallenge(verifier); const state = randomBytes(32).toString('hex'); - const params = await awaitBrowserCallback({ - 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)}`), - }); - - const linkCode = params.get('link_code'); - if (!linkCode) { - throw new Error('github: link callback did not return a link code.'); - } - + // 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 client.completeGithubLink(linkCode, verifier)); + ({ 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.'); diff --git a/packages/void/test/unit/auth-callback.test.ts b/packages/void/test/unit/auth-callback.test.ts index 9577bd8d7..6742ce8bf 100644 --- a/packages/void/test/unit/auth-callback.test.ts +++ b/packages/void/test/unit/auth-callback.test.ts @@ -10,11 +10,12 @@ const noopExecFile = vi.fn() as unknown as typeof import('node:child_process').e * (built from the localhost callback). Returns the callback URL so the test can * drive the localhost server directly. */ -function startCallback(options?: { +function startCallback(options?: { describeError?: (params: URLSearchParams) => string | null; + finalize?: (params: URLSearchParams) => Promise; state?: string; }): { - resultPromise: Promise; + resultPromise: Promise; callbackPromise: Promise; state: string; } { @@ -24,10 +25,14 @@ function startCallback(options?: { resolveCallback = resolve; }); - const resultPromise = awaitBrowserCallback({ + 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'; @@ -38,8 +43,11 @@ function startCallback(options?: { } describe('awaitBrowserCallback', () => { - it('resolves with the callback params on a valid /callback request', async () => { - const { resultPromise, callbackPromise, state } = startCallback(); + 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. @@ -54,13 +62,45 @@ describe('awaitBrowserCallback', () => { const res = await fetch(hit); expect(res.status).toBe(200); - const params = await resultPromise; - expect(params.get('link_code')).toBe('abc'); - expect(params.get('state')).toBe(state); + // 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('returns 400 and keeps listening on a state mismatch (does not resolve)', async () => { - const { resultPromise, callbackPromise, state } = startCallback(); + 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); @@ -70,6 +110,9 @@ describe('awaitBrowserCallback', () => { 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); @@ -79,6 +122,7 @@ describe('awaitBrowserCallback', () => { expect(goodRes.status).toBe(200); const params = await resultPromise; + expect(finalize).toHaveBeenCalledTimes(1); expect(params.get('link_code')).toBe('real'); }); @@ -107,6 +151,7 @@ describe('awaitBrowserCallback', () => { state: 'timeout-state', execFile: noopExecFile, timeoutMs: 10, + finalize: async (params) => params, buildAuthUrl: (callback) => `https://example.test/authorize?cb=${encodeURIComponent(callback.href)}`, }); @@ -124,6 +169,7 @@ describe('awaitBrowserCallback', () => { const resultPromise = awaitBrowserCallback({ state: 's', execFile, + finalize: async (params) => params, buildAuthUrl: (callback) => { resolveCallback(callback); return 'https://example.test/authorize'; 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-install-cmd.test.ts b/packages/void/test/unit/github-install-cmd.test.ts index 2bd7dd074..fd2620150 100644 --- a/packages/void/test/unit/github-install-cmd.test.ts +++ b/packages/void/test/unit/github-install-cmd.test.ts @@ -99,4 +99,44 @@ describe('runInstall', () => { 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(); + }); });