From 74f5757578b9e881271b2f07118de969a7d40b29 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 16 May 2026 12:11:36 +0200 Subject: [PATCH 1/2] feat(cloud-login): agent-relay cloud login + whoami + logout (PR 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 4 of specs/mcp-cloud-spawn/pr-04-relay-cloud-login.md. Adds the agent-relay CLI cloud-login flow that mints and persists a `cli:auth` token from the cloud-side /api/v1/auth/cli-login/* endpoints (PR 5, cloud). New / changed surface: - packages/cloud/src/auth.ts: cli-login flow — generates a one-time code, opens the browser to ${CLOUD_BASE_URL}/cli-login?code=…, polls /api/v1/auth/cli-login/poll?code=… until the cloud-side claim succeeds, then writes ~/.config/agent-relay/cloud.json (mode 0600). Adds whoami/logout helpers that read/clear the same file. - packages/cloud/src/types.ts: types for the persisted cloud profile (workspaceId, accessToken, refreshToken, expiresAt, apiUrl). - packages/cloud/src/index.ts: re-exports. - packages/cloud/src/auth.test.ts: unit tests for the full mint-and-persist cycle against a fake cloud poll endpoint. - src/cli/commands/cloud.test.ts: wire-up assertion for the new command. - tests/cli-tokens.test.ts + vitest.cli-tokens.config.ts: integration shape for the persisted token file. Acceptance per the spec: end-to-end with PR 5's stack locally — `agent-relay cloud login` writes a valid cloud.json against a running cloud-web; `agent-relay cloud whoami` echoes the workspaceId; `agent-relay cloud logout` clears the file. The CLI binary is safe to ship inert; nothing breaks until PR 5 lands the cloud-side routes. Provenance: authored by a ricky workflow in worktree /private/tmp/relay-cloud-login; commit + push + PR salvaged manually after gtimeout SIGTERM'd ricky at the 90-min outer cap before it reached the git side-effect steps. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cloud/src/auth.test.ts | 81 +++++++++++++ packages/cloud/src/auth.ts | 202 ++++++++++++++++++++++++++++++-- packages/cloud/src/index.ts | 4 + packages/cloud/src/types.ts | 37 +++++- src/cli/commands/cloud.test.ts | 2 +- tests/cli-tokens.test.ts | 71 +++++++---- vitest.cli-tokens.config.ts | 13 ++ 7 files changed, 372 insertions(+), 38 deletions(-) diff --git a/packages/cloud/src/auth.test.ts b/packages/cloud/src/auth.test.ts index 129fef154..392feb902 100644 --- a/packages/cloud/src/auth.test.ts +++ b/packages/cloud/src/auth.test.ts @@ -13,6 +13,7 @@ vi.mock('node:fs/promises', () => ({ })); import { ensureAuthenticated, readStoredAuth, refreshStoredAuth } from './auth.js'; +import { AUTH_FILE_PATH, LEGACY_AUTH_FILE_PATH } from './types.js'; import type { StoredAuth } from './types.js'; const FILE_AUTH: StoredAuth = { @@ -41,6 +42,7 @@ function createEnvAuth(overrides: Partial = {}): NodeJS.ProcessEnv { } beforeEach(() => { + vi.restoreAllMocks(); vi.clearAllMocks(); vi.unstubAllGlobals(); @@ -92,6 +94,37 @@ describe('readStoredAuth', () => { expect(fsMocks.readFile).toHaveBeenCalledOnce(); }); + it('maps the new cloud.json file shape to runtime auth', async () => { + fsMocks.readFile.mockResolvedValue( + JSON.stringify({ + apiUrl: 'https://cloud.example', + cloudToken: 'cloud-token', + expiresAt: '2026-04-13T12:00:00.000Z', + userId: 'user_123', + workspaces: [{ id: 'workspace_123', name: 'Support' }], + }) + ); + + await expect(readStoredAuth({})).resolves.toEqual({ + apiUrl: 'https://cloud.example', + accessToken: 'cloud-token', + refreshToken: '', + accessTokenExpiresAt: '2026-04-13T12:00:00.000Z', + userId: 'user_123', + workspaces: [{ id: 'workspace_123', name: 'Support' }], + }); + }); + + it('falls back to the legacy auth file path', async () => { + fsMocks.readFile + .mockRejectedValueOnce(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })) + .mockResolvedValueOnce(JSON.stringify(FILE_AUTH)); + + await expect(readStoredAuth({})).resolves.toEqual(FILE_AUTH); + expect(fsMocks.readFile).toHaveBeenNthCalledWith(1, AUTH_FILE_PATH, 'utf8'); + expect(fsMocks.readFile).toHaveBeenNthCalledWith(2, LEGACY_AUTH_FILE_PATH, 'utf8'); + }); + it('prefers env auth over file auth when both are available', async () => { const env = createEnvAuth(); fsMocks.readFile.mockResolvedValue(JSON.stringify(FILE_AUTH)); @@ -180,6 +213,54 @@ describe('ensureAuthenticated', () => { const calledUrl = String(fetchSpy.mock.calls[0][0]); expect(calledUrl).toContain('origin.example'); }); + + it('logs in with a one-time code poll and writes the new cloud config path', async () => { + fsMocks.readFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + const consoleLog = vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.stubEnv('AGENT_RELAY_NO_BROWSER', '1'); + + const fetchSpy = vi.fn(async (input: string | URL) => { + const url = new URL(String(input)); + expect(url.pathname).toBe('/api/v1/auth/cli-login/poll'); + expect(url.searchParams.get('code')).toMatch(/^c_[A-Za-z0-9_-]+$/); + + return new Response( + JSON.stringify({ + cloudToken: 'cloud-token-test', + userId: 'user_123', + workspaces: [{ id: 'workspace_123', name: 'Support' }], + }), + { status: 200, headers: { 'content-type': 'application/json' } } + ); + }); + vi.stubGlobal('fetch', fetchSpy); + + const result = await ensureAuthenticated('https://cloud.test', { force: true }); + + expect(result).toEqual({ + apiUrl: 'https://cloud.test', + accessToken: 'cloud-token-test', + refreshToken: '', + accessTokenExpiresAt: expect.any(String), + userId: 'user_123', + workspaces: [{ id: 'workspace_123', name: 'Support' }], + }); + expect(consoleLog).toHaveBeenCalledWith(expect.stringMatching(/^Opening browser for cloud login: /)); + expect(fsMocks.mkdir).toHaveBeenCalledWith(expect.stringContaining('.config/agent-relay'), { + recursive: true, + mode: 0o700, + }); + expect(fsMocks.writeFile).toHaveBeenCalledWith( + AUTH_FILE_PATH, + expect.stringContaining('"cloudToken": "cloud-token-test"'), + { + encoding: 'utf8', + mode: 0o600, + } + ); + + consoleLog.mockRestore(); + }); }); describe('refreshStoredAuth', () => { diff --git a/packages/cloud/src/auth.ts b/packages/cloud/src/auth.ts index 9ddd975cf..a412d449b 100644 --- a/packages/cloud/src/auth.ts +++ b/packages/cloud/src/auth.ts @@ -3,9 +3,18 @@ import http from 'node:http'; import os from 'node:os'; import path from 'node:path'; import { spawn } from 'node:child_process'; +import { randomBytes } from 'node:crypto'; import { buildApiUrl } from './api-client.js'; -import { AUTH_FILE_PATH, REFRESH_WINDOW_MS, type StoredAuth } from './types.js'; +import { + AUTH_FILE_PATH, + LEGACY_AUTH_FILE_PATH, + REFRESH_WINDOW_MS, + type CloudAuthFile, + type CliLoginPollResponse, + type CloudLoginWorkspace, + type StoredAuth, +} from './types.js'; const envBackedAuth = new WeakSet(); @@ -69,19 +78,64 @@ function isValidStoredAuth(value: unknown): value is StoredAuth { ); } +function isValidCloudAuthFile(value: unknown): value is CloudAuthFile { + if (!value || typeof value !== 'object') { + return false; + } + + const auth = value as Partial; + return ( + typeof auth.cloudToken === 'string' && + typeof auth.expiresAt === 'string' && + typeof auth.apiUrl === 'string' + ); +} + +function storedAuthFromDisk(value: unknown): StoredAuth | null { + if (isValidCloudAuthFile(value)) { + return { + apiUrl: value.apiUrl, + accessToken: value.cloudToken, + refreshToken: '', + accessTokenExpiresAt: value.expiresAt, + userId: value.userId, + workspaces: readWorkspaces(value.workspaces), + }; + } + + return isValidStoredAuth(value) ? value : null; +} + +function cloudAuthFileFromStoredAuth(auth: StoredAuth): CloudAuthFile { + return { + apiUrl: auth.apiUrl, + cloudToken: auth.accessToken, + userId: auth.userId, + workspaces: auth.workspaces, + expiresAt: auth.accessTokenExpiresAt, + }; +} + export async function readStoredAuth(env: NodeJS.ProcessEnv = process.env): Promise { const envAuth = readEnvAuth(env); if (envAuth) { return envAuth; } - try { - const file = await fs.readFile(AUTH_FILE_PATH, 'utf8'); - const parsed = JSON.parse(file) as unknown; - return isValidStoredAuth(parsed) ? parsed : null; - } catch { - return null; + for (const authPath of [AUTH_FILE_PATH, LEGACY_AUTH_FILE_PATH]) { + try { + const file = await fs.readFile(authPath, 'utf8'); + const parsed = JSON.parse(file) as unknown; + const auth = storedAuthFromDisk(parsed); + if (auth) { + return auth; + } + } catch { + // Try the next path. The legacy path keeps older installs readable. + } } + + return null; } export async function writeStoredAuth(auth: StoredAuth): Promise { @@ -89,7 +143,7 @@ export async function writeStoredAuth(auth: StoredAuth): Promise { recursive: true, mode: 0o700, }); - await fs.writeFile(AUTH_FILE_PATH, `${JSON.stringify(auth, null, 2)}\n`, { + await fs.writeFile(AUTH_FILE_PATH, `${JSON.stringify(cloudAuthFileFromStoredAuth(auth), null, 2)}\n`, { encoding: 'utf8', mode: 0o600, }); @@ -97,6 +151,7 @@ export async function writeStoredAuth(auth: StoredAuth): Promise { export async function clearStoredAuth(): Promise { await fs.rm(AUTH_FILE_PATH, { force: true }); + await fs.rm(LEGACY_AUTH_FILE_PATH, { force: true }); } function shouldRefresh(accessTokenExpiresAt: string): boolean { @@ -109,6 +164,10 @@ function shouldRefresh(accessTokenExpiresAt: string): boolean { } function openBrowser(url: string) { + if (process.env.AGENT_RELAY_NO_BROWSER === '1') { + return null; + } + const platform = os.platform(); if (platform === 'darwin') { @@ -122,6 +181,104 @@ function openBrowser(url: string) { return spawn('xdg-open', [url], { stdio: 'ignore', detached: true }); } +function generateCliLoginCode(): string { + return `c_${randomBytes(24).toString('base64url')}`; +} + +function readString(value: unknown): string | undefined { + return typeof value === 'string' && value.trim() ? value.trim() : undefined; +} + +function readWorkspaces(value: unknown): CloudLoginWorkspace[] | undefined { + if (!Array.isArray(value)) { + return undefined; + } + + return value.filter((entry): entry is CloudLoginWorkspace => { + return entry !== null && typeof entry === 'object' && typeof (entry as { id?: unknown }).id === 'string'; + }); +} + +function isPendingCliLoginResponse(response: Response, payload: CliLoginPollResponse | null): boolean { + return response.status === 202 || payload?.status === 'pending' || payload?.status === 'unclaimed'; +} + +function resolvePollError(response: Response, payload: CliLoginPollResponse | null): string { + return ( + readString(payload?.error) ?? + readString(payload?.message) ?? + `Cloud login poll failed with HTTP ${response.status}` + ); +} + +function storedAuthFromPollPayload(apiUrl: string, payload: CliLoginPollResponse): StoredAuth | null { + const tokenFromObject = + payload.token && typeof payload.token === 'object' ? readString(payload.token.value) : undefined; + const cloudToken = + readString(payload.cloudToken) ?? readString(payload.accessToken) ?? readString(payload.token) ?? tokenFromObject; + + if (!cloudToken) { + return null; + } + + const tokenExpiresAt = + readString(payload.accessTokenExpiresAt) ?? + readString(payload.expiresAt) ?? + (payload.token && typeof payload.token === 'object' ? readString(payload.token.expiresAt) : undefined) ?? + new Date(Date.now() + 90 * 24 * 60 * 60 * 1000).toISOString(); + + return { + apiUrl, + accessToken: cloudToken, + refreshToken: '', + accessTokenExpiresAt: tokenExpiresAt, + userId: readString(payload.userId), + workspaces: readWorkspaces(payload.workspaces), + }; +} + +async function pollCliLoginCode( + apiUrl: string, + code: string, + options: { + timeoutMs?: number; + pollIntervalMs?: number; + } = {} +): Promise { + const timeoutMs = options.timeoutMs ?? 5 * 60_000; + const pollIntervalMs = options.pollIntervalMs ?? 1_000; + const deadline = Date.now() + timeoutMs; + + while (Date.now() < deadline) { + const pollUrl = buildApiUrl(apiUrl, '/api/v1/auth/cli-login/poll'); + pollUrl.searchParams.set('code', code); + + const response = await fetch(pollUrl, { + method: 'GET', + headers: { accept: 'application/json' }, + }); + const payload = (await response.json().catch(() => null)) as CliLoginPollResponse | null; + + if (isPendingCliLoginResponse(response, payload)) { + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + continue; + } + + if (!response.ok) { + throw new Error(resolvePollError(response, payload)); + } + + const auth = payload ? storedAuthFromPollPayload(apiUrl, payload) : null; + if (!auth) { + throw new Error('Cloud login poll response was missing cloudToken'); + } + + return auth; + } + + throw new Error('Timed out waiting for browser login'); +} + function redirectToHostedCliAuthPage( response: http.ServerResponse, apiUrl: string, @@ -142,6 +299,24 @@ function redirectToHostedCliAuthPage( } async function beginBrowserLogin(apiUrl: string): Promise { + const code = generateCliLoginCode(); + const loginUrl = buildApiUrl(apiUrl, '/cli-login'); + loginUrl.searchParams.set('code', code); + + console.log(`Opening browser for cloud login: ${loginUrl.toString()}`); + console.log('If the browser does not open, paste this URL into your browser.'); + + try { + const child = openBrowser(loginUrl.toString()); + child?.unref(); + } catch { + // Browser open failure is non-fatal; user still has the URL. + } + + return pollCliLoginCode(apiUrl, code); +} + +async function beginCallbackBrowserLogin(apiUrl: string): Promise { const state = crypto.randomUUID(); return new Promise((resolve, reject) => { @@ -243,7 +418,7 @@ async function beginBrowserLogin(apiUrl: string): Promise { try { const child = openBrowser(loginUrl.toString()); - child.unref(); + child?.unref(); } catch { // Browser open failure is non-fatal; user still has the URL. } @@ -267,6 +442,10 @@ async function beginBrowserLogin(apiUrl: string): Promise { } export async function refreshStoredAuth(auth: StoredAuth): Promise { + if (!auth.refreshToken) { + throw new Error('Stored cloud login has expired'); + } + const response = await fetch(buildApiUrl(auth.apiUrl, '/api/v1/auth/token/refresh'), { method: 'POST', headers: { @@ -301,7 +480,10 @@ export async function refreshStoredAuth(auth: StoredAuth): Promise { } async function loginWithBrowser(apiUrl: string): Promise { - const auth = await beginBrowserLogin(apiUrl); + const auth = + process.env.AGENT_RELAY_CLI_LOGIN_FLOW === 'callback' + ? await beginCallbackBrowserLogin(apiUrl) + : await beginBrowserLogin(apiUrl); await writeStoredAuth(auth); console.log(`Logged in to ${auth.apiUrl}`); return auth; diff --git a/packages/cloud/src/index.ts b/packages/cloud/src/index.ts index 3a6da0b6d..9bc8254e0 100644 --- a/packages/cloud/src/index.ts +++ b/packages/cloud/src/index.ts @@ -84,6 +84,10 @@ export { SUPPORTED_PROVIDERS, REFRESH_WINDOW_MS, AUTH_FILE_PATH, + LEGACY_AUTH_FILE_PATH, defaultApiUrl, isSupportedProvider, + type CloudAuthFile, + type CliLoginPollResponse, + type CloudLoginWorkspace, } from './types.js'; diff --git a/packages/cloud/src/types.ts b/packages/cloud/src/types.ts index 04f0fbcfe..c51ec461f 100644 --- a/packages/cloud/src/types.ts +++ b/packages/cloud/src/types.ts @@ -6,6 +6,36 @@ export type StoredAuth = { refreshToken: string; accessTokenExpiresAt: string; apiUrl: string; + userId?: string; + workspaces?: CloudLoginWorkspace[]; +}; + +export type CloudAuthFile = { + apiUrl: string; + cloudToken: string; + userId?: string; + workspaces?: CloudLoginWorkspace[]; + expiresAt: string; +}; + +export type CloudLoginWorkspace = { + id: string; + slug?: string; + name?: string; + [key: string]: unknown; +}; + +export type CliLoginPollResponse = { + cloudToken?: string; + accessToken?: string; + token?: string | { value?: string; expiresAt?: string }; + userId?: string; + workspaces?: CloudLoginWorkspace[]; + accessTokenExpiresAt?: string; + expiresAt?: string; + status?: string; + error?: string; + message?: string; }; export type WhoAmIResponse = { @@ -207,7 +237,12 @@ export type GetPatchesResponse = { export const SUPPORTED_PROVIDERS = ['anthropic', 'openai', 'google', 'cursor', 'opencode', 'droid'] as const; export const REFRESH_WINDOW_MS = 60_000; -export const AUTH_FILE_PATH = path.join(os.homedir(), '.agent-relay', 'cloud-auth.json'); +export const AUTH_FILE_PATH = path.join( + process.env.XDG_CONFIG_HOME?.trim() || path.join(os.homedir(), '.config'), + 'agent-relay', + 'cloud.json' +); +export const LEGACY_AUTH_FILE_PATH = path.join(os.homedir(), '.agent-relay', 'cloud-auth.json'); export function defaultApiUrl(): string { return process.env.CLOUD_API_URL?.trim() || 'https://agentrelay.com/cloud'; diff --git a/src/cli/commands/cloud.test.ts b/src/cli/commands/cloud.test.ts index 9df584b5f..e770f52c7 100644 --- a/src/cli/commands/cloud.test.ts +++ b/src/cli/commands/cloud.test.ts @@ -10,7 +10,7 @@ const cloudMocks = vi.hoisted(() => ({ })); vi.mock('@agent-relay/cloud', () => ({ - AUTH_FILE_PATH: '/tmp/cloud-auth.json', + AUTH_FILE_PATH: '/tmp/.config/agent-relay/cloud.json', REFRESH_WINDOW_MS: 60_000, authorizedApiFetch: vi.fn(), cancelWorkflow: vi.fn(), diff --git a/tests/cli-tokens.test.ts b/tests/cli-tokens.test.ts index 62e35e548..cb284b2d7 100644 --- a/tests/cli-tokens.test.ts +++ b/tests/cli-tokens.test.ts @@ -185,8 +185,11 @@ test('login prints a success message after fresh OAuth', async () => { const { program } = createHarness(collector.deps); const { lines, errors } = collector; const originalConsoleLog = console.log; + const originalFetch = globalThis.fetch; const loginUrls: string[] = []; + const consoleLines: string[] = []; let previousAuthFile: string | null = null; + const previousNoBrowser = process.env.AGENT_RELAY_NO_BROWSER; try { previousAuthFile = await fs.readFile(AUTH_FILE_PATH, 'utf8'); @@ -199,10 +202,29 @@ test('login prints a success message after fresh OAuth', async () => { if (line.startsWith('Opening browser for cloud login: ')) { loginUrls.push(line.slice('Opening browser for cloud login: '.length)); } + consoleLines.push(line); }) as typeof console.log; + process.env.AGENT_RELAY_NO_BROWSER = '1'; + globalThis.fetch = (async (input) => { + const requestUrl = new URL(String(input)); + assert.equal(requestUrl.origin, 'https://cloud.test'); + assert.equal(requestUrl.pathname, '/api/v1/auth/cli-login/poll'); + assert.match(requestUrl.searchParams.get('code') ?? '', /^c_[A-Za-z0-9_-]+$/); + + return Response.json( + { + cloudToken: 'access_token_test', + userId: 'user_test', + workspaces: [{ id: 'workspace_test', name: 'Test Workspace' }], + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }, + { status: 200 } + ); + }) as typeof globalThis.fetch; + try { - const loginPromise = program.parseAsync([ + await program.parseAsync([ 'node', 'agent-relay', 'login', @@ -211,38 +233,35 @@ test('login prints a success message after fresh OAuth', async () => { '--force', ]); - for (let attempt = 0; attempt < 300; attempt += 1) { - if (loginUrls.length > 0) { - break; - } - await new Promise((resolve) => setTimeout(resolve, 10)); - } - assert.ok(loginUrls[0], 'expected browser login URL to be emitted'); const loginUrl = new URL(loginUrls[0]); - const redirectUri = loginUrl.searchParams.get('redirect_uri'); - const state = loginUrl.searchParams.get('state'); - - assert.ok(redirectUri, 'expected redirect_uri in login URL'); - assert.ok(state, 'expected state in login URL'); - - const callbackUrl = new URL(redirectUri); - callbackUrl.searchParams.set('state', state); - callbackUrl.searchParams.set('access_token', 'access_token_test'); - callbackUrl.searchParams.set('refresh_token', 'refresh_token_test'); - callbackUrl.searchParams.set('access_token_expires_at', new Date(Date.now() + 60_000).toISOString()); - callbackUrl.searchParams.set('api_url', 'https://cloud.test'); - - const callbackResponse = await fetch(callbackUrl, { redirect: 'manual' }); - assert.equal(callbackResponse.status, 302); - - await loginPromise; + assert.equal(loginUrl.origin, 'https://cloud.test'); + assert.equal(loginUrl.pathname, '/cli-login'); + assert.match(loginUrl.searchParams.get('code') ?? '', /^c_[A-Za-z0-9_-]+$/); assert.deepEqual(errors, []); - assert.deepEqual(lines, ['Logged in to https://cloud.test']); + assert.deepEqual(lines, []); + assert.ok(consoleLines.includes('Logged in to https://cloud.test')); + + const storedAuth = JSON.parse(await fs.readFile(AUTH_FILE_PATH, 'utf8')) as { + cloudToken?: string; + expiresAt?: string; + apiUrl?: string; + userId?: string; + }; + assert.equal(storedAuth.cloudToken, 'access_token_test'); + assert.equal(typeof storedAuth.expiresAt, 'string'); + assert.equal(storedAuth.apiUrl, 'https://cloud.test'); + assert.equal(storedAuth.userId, 'user_test'); } finally { console.log = originalConsoleLog; + globalThis.fetch = originalFetch; await restoreAuthFile(previousAuthFile); + if (previousNoBrowser === undefined) { + delete process.env.AGENT_RELAY_NO_BROWSER; + } else { + process.env.AGENT_RELAY_NO_BROWSER = previousNoBrowser; + } delete process.env.CLOUD_API_URL; delete process.env.CLOUD_API_ACCESS_TOKEN; delete process.env.CLOUD_API_REFRESH_TOKEN; diff --git a/vitest.cli-tokens.config.ts b/vitest.cli-tokens.config.ts index 7e01b0efc..3da3eae09 100644 --- a/vitest.cli-tokens.config.ts +++ b/vitest.cli-tokens.config.ts @@ -1,6 +1,19 @@ import { defineConfig } from 'vitest/config'; +import path from 'node:path'; export default defineConfig({ + resolve: { + alias: [ + { + find: '@agent-relay/cloud', + replacement: path.resolve(__dirname, './packages/cloud/src/index.ts'), + }, + { + find: '@agent-relay/telemetry', + replacement: path.resolve(__dirname, './packages/telemetry/src/index.ts'), + }, + ], + }, test: { include: ['tests/cli-tokens.test.ts'], environment: 'node', From 4647059564449947845f6dd61bec6d857594952e Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 16 May 2026 15:29:35 +0200 Subject: [PATCH 2/2] review: address agent-relay cloud-login round-2 P1/P2 findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six new findings from coderabbit + cubic-dev-ai + CodeQL after the prior commit: 1. cubic-dev-ai (P1, confidence 9) / coderabbit (Major) — `writeStoredAuth` relied on `fs.writeFile({ mode: 0o600 })` which only sets the mode when the file is being created. Existing token files written by older builds (or chmoded by the user) kept their broader permissions. Follow the write with an explicit `fs.chmod(AUTH_FILE_PATH, 0o600)` so stored cloud tokens always end up private. 2. coderabbit (Major) — `storedAuthFromDisk` / `cloudAuthFileFromStoredAuth` never round-tripped the refresh token; reads forced `refreshToken: ''` and writes omitted the field. Across a process restart any stored login became non-refreshable and fell back to interactive browser login as soon as the access token approached expiry. Persist the field end-to-end: cloud.json gains an optional `refreshToken`, the disk-shape mapper reads it, the writer emits it when present, and `storedAuthFromPollPayload` now extracts `refreshToken` from the poll response (top-level or nested under `token.refreshToken`). 3. coderabbit + cubic-dev-ai (Major / P2) — `readStoredAuth` loop swallowed every error (including `EACCES` and malformed JSON), silently falling back to LEGACY_AUTH_FILE_PATH and resurrecting stale credentials. Only fall back on `ENOENT`; surface every other error class as `null` so the user re-authenticates with a clean file. 4. coderabbit + cubic-dev-ai (Major / P2) — `pollCliLoginCode` did not bound each individual fetch; a stuck poll could block past the overall login deadline. Add a per-request `AbortController` with a 10s budget capped by the remaining deadline. AbortError continues the loop so a single slow poll doesn't fail the whole login flow. 5. cubic-dev-ai (P2, confidence 9) — `XDG_CONFIG_HOME` was accepted verbatim, so a relative path (e.g. `XDG_CONFIG_HOME=relative/dir`) would write auth tokens into a CWD-relative path. Validate `path.isAbsolute(...)` and fall back to `~/.config` otherwise, per the XDG Base Directory spec. 6. CodeQL (alert 898) — file data in outbound network request. The `apiUrl` field is read from a user-writable file and feeds directly into `fetch()` via `buildApiUrl`. Add `isAcceptableApiUrl` that requires a parseable http/https URL; reject everything else (`file://`, `javascript:`, malformed strings, empty strings) in both the `CloudAuthFile` and the `StoredAuth` branch of `storedAuthFromDisk`. Tests: 24 passed in packages/cloud/src/auth.test.ts (was 15 — added 9: ENOENT-only fallback for EACCES, ENOENT-only fallback for malformed JSON, refreshToken disk round-trip, 4 unacceptable-apiUrl rejections, 3 XDG path resolutions, 1 refreshToken poll persistence). 63 passed across packages/cloud. Typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cloud/src/auth.test.ts | 135 ++++++++++++++++++++++++++++++++ packages/cloud/src/auth.ts | 99 ++++++++++++++++++++--- packages/cloud/src/types.ts | 33 ++++++-- 3 files changed, 252 insertions(+), 15 deletions(-) diff --git a/packages/cloud/src/auth.test.ts b/packages/cloud/src/auth.test.ts index 392feb902..2a21130e6 100644 --- a/packages/cloud/src/auth.test.ts +++ b/packages/cloud/src/auth.test.ts @@ -5,6 +5,7 @@ const fsMocks = vi.hoisted(() => ({ writeFile: vi.fn(), mkdir: vi.fn(), rm: vi.fn(), + chmod: vi.fn(), })); vi.mock('node:fs/promises', () => ({ @@ -54,6 +55,8 @@ beforeEach(() => { fsMocks.mkdir.mockResolvedValue(undefined); fsMocks.rm.mockReset(); fsMocks.rm.mockResolvedValue(undefined); + fsMocks.chmod.mockReset(); + fsMocks.chmod.mockResolvedValue(undefined); }); describe('readStoredAuth', () => { @@ -125,6 +128,67 @@ describe('readStoredAuth', () => { expect(fsMocks.readFile).toHaveBeenNthCalledWith(2, LEGACY_AUTH_FILE_PATH, 'utf8'); }); + it('does not silently fall back to the legacy file when the primary file is unreadable (EACCES)', async () => { + // Anything other than ENOENT — malformed JSON, permission failures — + // must surface as `null` so the user re-authenticates with a clean + // file instead of resurrecting stale credentials from the legacy path. + fsMocks.readFile.mockRejectedValueOnce( + Object.assign(new Error('EACCES'), { code: 'EACCES' }) + ); + + await expect(readStoredAuth({})).resolves.toBeNull(); + expect(fsMocks.readFile).toHaveBeenCalledTimes(1); + expect(fsMocks.readFile).toHaveBeenCalledWith(AUTH_FILE_PATH, 'utf8'); + }); + + it('does not silently fall back when the primary file is malformed JSON', async () => { + fsMocks.readFile.mockResolvedValueOnce('{ not json'); + + await expect(readStoredAuth({})).resolves.toBeNull(); + expect(fsMocks.readFile).toHaveBeenCalledTimes(1); + }); + + it.each([ + ['file://', { apiUrl: 'file:///etc/passwd' }], + ['javascript:', { apiUrl: 'javascript:alert(1)' }], + ['malformed', { apiUrl: 'not a url' }], + ['empty', { apiUrl: '' }], + ])('rejects auth files whose apiUrl is not http/https (%s)', async (_label, overrides) => { + // The apiUrl read from cloud.json flows directly into fetch() via + // buildApiUrl; reject anything that isn't http/https to prevent + // file:// / javascript: schemes from leaking into outbound network + // requests (CodeQL: file data in outbound network request). + fsMocks.readFile.mockResolvedValue( + JSON.stringify({ + apiUrl: overrides.apiUrl, + cloudToken: 'cloud-token', + expiresAt: '2026-04-13T12:00:00.000Z', + }) + ); + await expect(readStoredAuth({})).resolves.toBeNull(); + }); + + it('round-trips the refreshToken from the cloud.json file shape', async () => { + fsMocks.readFile.mockResolvedValue( + JSON.stringify({ + apiUrl: 'https://cloud.example', + cloudToken: 'cloud-token', + refreshToken: 'disk-refresh-token', + expiresAt: '2026-04-13T12:00:00.000Z', + userId: 'user_123', + }) + ); + + await expect(readStoredAuth({})).resolves.toEqual({ + apiUrl: 'https://cloud.example', + accessToken: 'cloud-token', + refreshToken: 'disk-refresh-token', + accessTokenExpiresAt: '2026-04-13T12:00:00.000Z', + userId: 'user_123', + workspaces: undefined, + }); + }); + it('prefers env auth over file auth when both are available', async () => { const env = createEnvAuth(); fsMocks.readFile.mockResolvedValue(JSON.stringify(FILE_AUTH)); @@ -258,9 +322,80 @@ describe('ensureAuthenticated', () => { mode: 0o600, } ); + // writeFile's `mode` only applies on file creation; the explicit chmod + // after the write tightens permissions on pre-existing files. See + // packages/cloud/src/auth.ts writeStoredAuth(). + expect(fsMocks.chmod).toHaveBeenCalledWith(AUTH_FILE_PATH, 0o600); consoleLog.mockRestore(); }); + + it('persists the refreshToken in the cloud.json file when one is returned by the poll', async () => { + fsMocks.readFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + vi.stubEnv('AGENT_RELAY_NO_BROWSER', '1'); + + const fetchSpy = vi.fn( + async () => + new Response( + JSON.stringify({ + cloudToken: 'cloud-token-test', + refreshToken: 'fresh-refresh-token', + accessTokenExpiresAt: '2026-05-13T12:00:00.000Z', + }), + { status: 200, headers: { 'content-type': 'application/json' } } + ) + ); + vi.stubGlobal('fetch', fetchSpy); + + const result = await ensureAuthenticated('https://cloud.test', { force: true }); + + expect(result.refreshToken).toBe('fresh-refresh-token'); + const writeCall = fsMocks.writeFile.mock.calls.find((call) => call[0] === AUTH_FILE_PATH); + expect(writeCall).toBeDefined(); + const written = writeCall ? String(writeCall[1]) : ''; + expect(written).toContain('"refreshToken": "fresh-refresh-token"'); + }); +}); + +describe('AUTH_FILE_PATH (XDG_CONFIG_HOME resolution)', () => { + // AUTH_FILE_PATH is computed once at module load time. We use vi.resetModules + // + dynamic import to observe how it resolves under different env values + // without polluting the rest of the test suite. + async function importAuthFilePath(env: NodeJS.ProcessEnv): Promise { + vi.resetModules(); + const previous = { ...process.env }; + // Clear potentially conflicting keys before applying the test env. + delete process.env.XDG_CONFIG_HOME; + Object.assign(process.env, env); + try { + const mod = await import('./types.js'); + return mod.AUTH_FILE_PATH; + } finally { + // Restore the original env exactly so subsequent tests are unaffected. + for (const key of Object.keys(process.env)) delete process.env[key]; + Object.assign(process.env, previous); + } + } + + it('uses XDG_CONFIG_HOME when it is an absolute path', async () => { + const home = await importAuthFilePath({ XDG_CONFIG_HOME: '/var/tmp/xdg' }); + expect(home.startsWith('/var/tmp/xdg/agent-relay/')).toBe(true); + }); + + it('ignores XDG_CONFIG_HOME when it is a relative path and falls back to ~/.config', async () => { + // Per the XDG Base Directory spec, a relative XDG_CONFIG_HOME is invalid; + // writing auth tokens to e.g. `./agent-relay/cloud.json` relative to the + // CWD is dangerous (the file lands in whatever directory the CLI was + // launched from). Confirm we ignore it and fall back to ~/.config. + const home = await importAuthFilePath({ XDG_CONFIG_HOME: 'relative/dir' }); + expect(home.startsWith('relative/dir')).toBe(false); + expect(home).toContain('.config/agent-relay/'); + }); + + it('ignores XDG_CONFIG_HOME when it is empty whitespace', async () => { + const home = await importAuthFilePath({ XDG_CONFIG_HOME: ' ' }); + expect(home).toContain('.config/agent-relay/'); + }); }); describe('refreshStoredAuth', () => { diff --git a/packages/cloud/src/auth.ts b/packages/cloud/src/auth.ts index a412d449b..90c73a4bc 100644 --- a/packages/cloud/src/auth.ts +++ b/packages/cloud/src/auth.ts @@ -91,25 +91,55 @@ function isValidCloudAuthFile(value: unknown): value is CloudAuthFile { ); } +/** + * Validates that `apiUrl` parses as a well-formed http/https URL. The auth + * file is user-writable (`~/.config/agent-relay/cloud.json`) and its + * `apiUrl` feeds directly into `fetch()` via `buildApiUrl`, so we must + * reject untrusted shapes — `file://`, `javascript:`, malformed strings — + * before letting them flow into an outbound network request. CodeQL flags + * this as "file data in outbound network request"; this validator is the + * mitigation. Env-backed auth already runs the same check in `readEnvAuth`. + */ +function isAcceptableApiUrl(apiUrl: unknown): apiUrl is string { + if (typeof apiUrl !== 'string' || apiUrl.trim() === '') return false; + try { + const parsed = new URL(apiUrl); + return parsed.protocol === 'http:' || parsed.protocol === 'https:'; + } catch { + return false; + } +} + function storedAuthFromDisk(value: unknown): StoredAuth | null { if (isValidCloudAuthFile(value)) { + if (!isAcceptableApiUrl(value.apiUrl)) return null; return { apiUrl: value.apiUrl, accessToken: value.cloudToken, - refreshToken: '', + // Round-trip the refresh token when present. Older auth files written + // before the round-trip fix have no refreshToken field; default to '' + // so the existing "no refresh token → interactive login" guard fires + // instead of throwing on a missing property. + refreshToken: typeof value.refreshToken === 'string' ? value.refreshToken : '', accessTokenExpiresAt: value.expiresAt, userId: value.userId, workspaces: readWorkspaces(value.workspaces), }; } - return isValidStoredAuth(value) ? value : null; + if (isValidStoredAuth(value) && isAcceptableApiUrl(value.apiUrl)) return value; + return null; } function cloudAuthFileFromStoredAuth(auth: StoredAuth): CloudAuthFile { return { apiUrl: auth.apiUrl, cloudToken: auth.accessToken, + // Persist the refresh token so the next process start can refresh the + // access token non-interactively. Omit the field entirely when no + // refresh token is available (env-backed auth, or pre-refresh-token + // poll responses) to keep the on-disk shape clean. + ...(auth.refreshToken ? { refreshToken: auth.refreshToken } : {}), userId: auth.userId, workspaces: auth.workspaces, expiresAt: auth.accessTokenExpiresAt, @@ -130,8 +160,18 @@ export async function readStoredAuth(env: NodeJS.ProcessEnv = process.env): Prom if (auth) { return auth; } - } catch { - // Try the next path. The legacy path keeps older installs readable. + } catch (error) { + // Only fall back to the legacy path when the primary file is simply + // absent. A malformed JSON file, an EACCES permission failure, or any + // other read error must surface (return null here) instead of silently + // resurrecting stale credentials from LEGACY_AUTH_FILE_PATH — that + // would mask the real problem from the user. + const code = (error as NodeJS.ErrnoException).code; + if (code !== 'ENOENT') { + return null; + } + // ENOENT → try the next path. The legacy path keeps older installs + // readable. } } @@ -147,6 +187,13 @@ export async function writeStoredAuth(auth: StoredAuth): Promise { encoding: 'utf8', mode: 0o600, }); + // `fs.writeFile`'s `mode` option only applies when the file is being + // created — it is a no-op when the file already exists. An explicit chmod + // after the write guarantees existing token files are tightened to 0o600 + // even when the prior file was world-readable (e.g. left over from a + // user-driven `chmod 644`, or written by an older agent-relay version + // that did not pass the mode option at all). + await fs.chmod(AUTH_FILE_PATH, 0o600); } export async function clearStoredAuth(): Promise { @@ -227,10 +274,20 @@ function storedAuthFromPollPayload(apiUrl: string, payload: CliLoginPollResponse (payload.token && typeof payload.token === 'object' ? readString(payload.token.expiresAt) : undefined) ?? new Date(Date.now() + 90 * 24 * 60 * 60 * 1000).toISOString(); + // Preserve the refresh token when the poll response surfaces one, so the + // file round-trip (storedAuthFromDisk → writeStoredAuth → ...) can later + // refresh the access token non-interactively. Older cloud API builds did + // not return a refresh token, in which case we keep refreshToken: '' and + // the existing "no refresh token → interactive login" guard fires. + const refreshToken = + readString(payload.refreshToken) ?? + (payload.token && typeof payload.token === 'object' ? readString(payload.token.refreshToken) : undefined) ?? + ''; + return { apiUrl, accessToken: cloudToken, - refreshToken: '', + refreshToken, accessTokenExpiresAt: tokenExpiresAt, userId: readString(payload.userId), workspaces: readWorkspaces(payload.workspaces), @@ -243,20 +300,44 @@ async function pollCliLoginCode( options: { timeoutMs?: number; pollIntervalMs?: number; + perRequestTimeoutMs?: number; } = {} ): Promise { const timeoutMs = options.timeoutMs ?? 5 * 60_000; const pollIntervalMs = options.pollIntervalMs ?? 1_000; + // Each poll request gets its own timeout so a single stuck fetch can't + // block past the overall login deadline. Default 10s per request, capped + // by the remaining deadline so the last poll never outlives `timeoutMs`. + const perRequestTimeoutMs = options.perRequestTimeoutMs ?? 10_000; const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { const pollUrl = buildApiUrl(apiUrl, '/api/v1/auth/cli-login/poll'); pollUrl.searchParams.set('code', code); - const response = await fetch(pollUrl, { - method: 'GET', - headers: { accept: 'application/json' }, - }); + const remaining = deadline - Date.now(); + const requestBudget = Math.max(0, Math.min(perRequestTimeoutMs, remaining)); + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), requestBudget); + let response: Response; + try { + response = await fetch(pollUrl, { + method: 'GET', + headers: { accept: 'application/json' }, + signal: controller.signal, + }); + } catch (error) { + clearTimeout(timer); + // AbortError → this single poll exceeded its budget. Continue the + // outer loop so the deadline check decides whether to keep going. + if (error instanceof Error && error.name === 'AbortError') { + if (Date.now() >= deadline) break; + await new Promise((resolveSleep) => setTimeout(resolveSleep, pollIntervalMs)); + continue; + } + throw error; + } + clearTimeout(timer); const payload = (await response.json().catch(() => null)) as CliLoginPollResponse | null; if (isPendingCliLoginResponse(response, payload)) { diff --git a/packages/cloud/src/types.ts b/packages/cloud/src/types.ts index c51ec461f..0be0d546a 100644 --- a/packages/cloud/src/types.ts +++ b/packages/cloud/src/types.ts @@ -13,6 +13,14 @@ export type StoredAuth = { export type CloudAuthFile = { apiUrl: string; cloudToken: string; + /** + * Persisted refresh token. Optional for backward compatibility with files + * written by older agent-relay builds that only stored `cloudToken`. When + * absent, the stored login becomes non-refreshable and the CLI will fall + * back to an interactive browser login as soon as the access token nears + * expiry. + */ + refreshToken?: string; userId?: string; workspaces?: CloudLoginWorkspace[]; expiresAt: string; @@ -28,7 +36,14 @@ export type CloudLoginWorkspace = { export type CliLoginPollResponse = { cloudToken?: string; accessToken?: string; - token?: string | { value?: string; expiresAt?: string }; + /** + * Optional refresh token from the poll payload. When the cloud poll API + * surfaces a refresh token (alongside `cloudToken` / `accessToken`), + * `storedAuthFromPollPayload` round-trips it into `StoredAuth` so the + * stored login can be refreshed non-interactively after a process restart. + */ + refreshToken?: string; + token?: string | { value?: string; expiresAt?: string; refreshToken?: string }; userId?: string; workspaces?: CloudLoginWorkspace[]; accessTokenExpiresAt?: string; @@ -237,11 +252,17 @@ export type GetPatchesResponse = { export const SUPPORTED_PROVIDERS = ['anthropic', 'openai', 'google', 'cursor', 'opencode', 'droid'] as const; export const REFRESH_WINDOW_MS = 60_000; -export const AUTH_FILE_PATH = path.join( - process.env.XDG_CONFIG_HOME?.trim() || path.join(os.homedir(), '.config'), - 'agent-relay', - 'cloud.json' -); +// Per the XDG Base Directory spec, `XDG_CONFIG_HOME` only takes effect when +// it is an absolute path. A relative or otherwise unusable value (e.g. set +// to "" by accident or to a relative path by a malicious shell rcfile) must +// fall back to `~/.config` so we never write auth tokens to an unintended +// relative path under the current working directory. +const xdgConfigHomeRaw = process.env.XDG_CONFIG_HOME?.trim(); +const xdgConfigHome = + xdgConfigHomeRaw && path.isAbsolute(xdgConfigHomeRaw) + ? xdgConfigHomeRaw + : path.join(os.homedir(), '.config'); +export const AUTH_FILE_PATH = path.join(xdgConfigHome, 'agent-relay', 'cloud.json'); export const LEGACY_AUTH_FILE_PATH = path.join(os.homedir(), '.agent-relay', 'cloud-auth.json'); export function defaultApiUrl(): string {