From 5364b9ee127192d71f75b4128fa9ae5187e871cb Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 20:19:04 -0800 Subject: [PATCH 01/13] feat(cloud-worker): mount subscription logins for the run and write refreshed tokens back When the assignment payload carries cliCredentials (a short-lived run-scoped reference), the worker exchanges it with its worker token for the CLI auth files, writes each 0600 in a 0700 dir inside the run dir, points CODEX_HOME/GROK_HOME at it, sets AGENT_RELAY_SUBSCRIPTION_LOGINS for relayflows precedence, writes back a file the CLI refreshed, and deletes the run dir after the run. Co-Authored-By: Claude Opus 5.5 --- .../cloud-worker-cli-credentials.test.ts | 181 ++++++++++++++++++ .../commands/cloud-worker-cli-credentials.ts | 149 ++++++++++++++ packages/cli/src/cli/commands/cloud-worker.ts | 36 +++- packages/cloud/src/worker.ts | 11 ++ 4 files changed, 375 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts create mode 100644 packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts diff --git a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts new file mode 100644 index 0000000000..f228145791 --- /dev/null +++ b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts @@ -0,0 +1,181 @@ +import { EventEmitter } from 'node:events'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { describe, expect, it, vi } from 'vitest'; + +import { createDefaultAssignmentRunner } from './cloud-worker.js'; +import { + materializeCliCredentials, + SUBSCRIPTION_LOGINS_ENV, + writeBackCliCredentials, +} from './cloud-worker-cli-credentials.js'; + +const worker = { + baseUrl: 'https://control.test', + workerId: 'wrk_1', + workerToken: 'sh_wrk_secret', + name: 'w', + heartbeatIntervalMs: 30_000, + registeredAt: '2026-11-03T00:00:00.000Z', + updatedAt: '2026-11-03T00:00:00.000Z', +} as never; + +const CODEX = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-1' } }); +const GROK = JSON.stringify({ 'https://auth.x.ai::c': { key: 'k', refresh_token: 'xrt-secret' } }); +const credentials = { + ref: 'sh_clicred_ref', + path: '/api/v1/workers/{workerId}/assignments/run_1/cli-credentials', +}; + +function controlPlane(files: unknown[]) { + const calls: Array<{ url: string; method: string; headers: Headers; body?: string }> = []; + const fetchImpl = vi.fn(async (url: string, init: RequestInit = {}) => { + calls.push({ + url, + method: init.method ?? 'GET', + headers: new Headers(init.headers), + body: typeof init.body === 'string' ? init.body : undefined, + }); + if ((init.method ?? 'GET') === 'GET') { + return new Response(JSON.stringify({ files, skipped: [] }), { headers: { 'content-type': 'application/json' } }); + } + return new Response(JSON.stringify({ updated: true }), { headers: { 'content-type': 'application/json' } }); + }); + return { calls, fetchImpl: fetchImpl as unknown as typeof fetch }; +} + +const codexFile = { + credentialId: 'c-codex', + provider: 'openai', + cli: 'codex', + homeEnv: 'CODEX_HOME', + directory: '.codex', + file: 'auth.json', + content: CODEX, +}; + +describe('cloud worker CLI logins', () => { + it('writes each auth file 0600 in a 0700 dir inside the run dir and points the CLI at it', async () => { + const runDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-creds-')); + try { + const cp = controlPlane([ + codexFile, + { ...codexFile, credentialId: 'c-grok', provider: 'xai', cli: 'grok', homeEnv: 'GROK_HOME', directory: '.grok', content: GROK }, + { ...codexFile, credentialId: 'evil', homeEnv: 'PATH' }, + { ...codexFile, credentialId: 'evil2', directory: '../../etc' }, + ]); + const log = vi.fn(); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl, log }); + expect(cp.calls[0]!.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials'); + expect(cp.calls[0]!.headers.get('authorization')).toBe('Bearer sh_wrk_secret'); + expect(cp.calls[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); + expect(materialized.env).toEqual({ + CODEX_HOME: path.join(runDir, '.cli-home', '.codex'), + GROK_HOME: path.join(runDir, '.cli-home', '.grok'), + [SUBSCRIPTION_LOGINS_ENV]: 'codex,grok', + }); + const codexPath = path.join(runDir, '.cli-home', '.codex', 'auth.json'); + expect(fs.readFileSync(codexPath, 'utf-8')).toBe(CODEX); + expect(fs.statSync(codexPath).mode & 0o777).toBe(0o600); + expect(fs.statSync(path.dirname(codexPath)).mode & 0o777).toBe(0o700); + expect(fs.statSync(path.join(runDir, '.cli-home')).mode & 0o777).toBe(0o700); + expect(materialized.files.map((file) => file.credentialId)).toEqual(['c-codex', 'c-grok']); + expect(fs.existsSync(path.join(runDir, 'etc'))).toBe(false); + expect(JSON.stringify(log.mock.calls)).not.toContain('rt-secret'); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('refuses a reference path that leaves the control plane', async () => { + const cp = controlPlane([]); + await expect( + materializeCliCredentials({ + worker, + credentials: { ...credentials, path: 'https://evil.test/steal' }, + runDir: os.tmpdir(), + fetchImpl: cp.fetchImpl, + }) + ).rejects.toThrow(/relative \/api\//); + expect(cp.calls).toHaveLength(0); + }); + + it('writes back only files the CLI changed', async () => { + const runDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-creds-')); + try { + const cp = controlPlane([codexFile, { ...codexFile, credentialId: 'c-grok', cli: 'grok', homeEnv: 'GROK_HOME', directory: '.grok', content: GROK }]); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const refreshed = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-2' } }); + fs.writeFileSync(materialized.files[0]!.filePath, refreshed); + expect(await writeBackCliCredentials({ worker, credentials, materialized, fetchImpl: cp.fetchImpl })).toBe(1); + const put = cp.calls.find((call) => call.method === 'PUT')!; + expect(put.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials/c-codex'); + expect(put.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); + expect(JSON.parse(put.body!)).toEqual({ content: refreshed }); + expect(cp.calls.filter((call) => call.method === 'PUT')).toHaveLength(1); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('the assignment runner mounts logins for relayflows, writes back, and deletes the run dir', async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-creds-home-')); + try { + const cp = controlPlane([codexFile]); + let seenEnv: NodeJS.ProcessEnv = {}; + let fileDuringRun = ''; + const spawnProcess = vi.fn((_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + seenEnv = options.env; + const authPath = path.join(options.env.CODEX_HOME!, 'auth.json'); + fileDuringRun = fs.readFileSync(authPath, 'utf-8'); + // The CLI refreshes its token during the run. + fs.writeFileSync(authPath, JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-3' } })); + const child = new EventEmitter() as EventEmitter & { killed: boolean; kill: () => boolean }; + child.killed = false; + child.kill = () => true; + queueMicrotask(() => child.emit('exit', 0, null)); + return child; + }) as never; + const runner = createDefaultAssignmentRunner({ + log: vi.fn(), + error: vi.fn(), + exit: vi.fn() as never, + env: { AGENT_RELAY_HOME: home, CODEX_HOME: '/home/user/.codex' }, + spawnProcess, + now: () => new Date(), + cwd: () => home, + fetchImpl: cp.fetchImpl, + resolveRelayflowsCliEntrypoint: () => '/opt/relayflows/dist/cli.js', + }); + await runner({ + assignment: { runId: 'run_1' } as never, + payload: { + runId: 'run_1', + workspaceId: 'ws', + relayWorkspaceId: 'rw', + relaycastApiKey: 'rk', + relayfileUrl: '', + relayfileToken: '', + workflow: 'version: "1.0"\n', + fileType: 'yaml', + sourceFileType: 'yaml', + workflowFileName: 'workflow.yaml', + cliCredentials: credentials, + }, + worker, + signal: new AbortController().signal, + }); + const runDir = path.join(home, 'cloud-workers', 'runs', 'run_1'); + expect(seenEnv.CODEX_HOME).toBe(path.join(runDir, '.cli-home', '.codex')); + expect(seenEnv[SUBSCRIPTION_LOGINS_ENV]).toBe('codex'); + expect(fileDuringRun).toBe(CODEX); + const put = cp.calls.find((call) => call.method === 'PUT'); + expect(JSON.parse(put!.body!).content).toContain('rt-secret-3'); + expect(fs.existsSync(runDir)).toBe(false); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts new file mode 100644 index 0000000000..fadbf69c00 --- /dev/null +++ b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts @@ -0,0 +1,149 @@ +// Subscription-login materialisation for `agent-relay cloud worker`. +// +// A control plane that holds Codex/Grok subscription logins for the workspace +// (self-hosted relaycontrol; hosted Cloud mounts the same files in its own +// sandboxes) puts `cliCredentials` in the assignment payload: a short-lived, +// run-scoped reference, never the files. Before spawning relayflows, the worker +// exchanges the reference (with its worker token) for the auth files, writes +// each one 0600 into a 0700 directory inside the run's private dir, and points +// the CLI at it (CODEX_HOME / GROK_HOME). After the run it writes back a file +// the CLI refreshed, so the stored login keeps a live refresh token. The run +// dir, and the files with it, are deleted after the run. +// +// Nothing here logs a file's content or the reference. +import fsp from 'node:fs/promises'; +import path from 'node:path'; + +import type { CloudWorkerRecord } from '@agent-relay/cloud'; + +export type CliCredentialsPayload = { + ref: string; + expiresAt?: string; + /** Path template with `{workerId}`. */ + path: string; + credentials?: Array<{ credentialId: string; provider: string; cli: string }>; +}; + +type CredentialFile = { + credentialId: string; + provider: string; + cli: string; + homeEnv: string; + directory: string; + file: string; + content: string; +}; + +export type MaterializedCliCredentials = { + env: Record; + files: Array<{ credentialId: string; cli: string; filePath: string; original: string }>; +}; + +/** The only env names a materialised login may set. */ +const ALLOWED_HOME_ENV = new Set(['CODEX_HOME', 'GROK_HOME']); +const SAFE_SEGMENT = /^\.?[A-Za-z0-9_-]{1,64}(\.json)?$/; + +/** Env flag relayflows reads to give the subscription precedence over API keys. */ +export const SUBSCRIPTION_LOGINS_ENV = 'AGENT_RELAY_SUBSCRIPTION_LOGINS'; + +export function isCliCredentialsPayload(value: unknown): value is CliCredentialsPayload { + if (!value || typeof value !== 'object' || Array.isArray(value)) return false; + const candidate = value as Record; + return typeof candidate.ref === 'string' && candidate.ref.length > 0 && typeof candidate.path === 'string'; +} + +function credentialUrl(worker: CloudWorkerRecord, template: string, suffix = ''): string { + const relative = template.replace('{workerId}', encodeURIComponent(worker.workerId)); + // Only same-origin, relative API paths: the reference is never sent elsewhere. + if (!relative.startsWith('/api/')) throw new Error('cliCredentials.path must be a relative /api/ path'); + return new URL(`${relative}${suffix}`, worker.baseUrl.endsWith('/') ? worker.baseUrl : `${worker.baseUrl}/`).toString(); +} + +function headers(worker: CloudWorkerRecord, ref: string, json = false): Headers { + const result = new Headers({ Authorization: `Bearer ${worker.workerToken}`, 'X-Cli-Credential-Ref': ref }); + if (json) result.set('Content-Type', 'application/json'); + return result; +} + +export async function materializeCliCredentials(input: { + worker: CloudWorkerRecord; + credentials: CliCredentialsPayload; + runDir: string; + fetchImpl: typeof fetch; + signal?: AbortSignal; + log?: (...args: unknown[]) => void; +}): Promise { + const response = await input.fetchImpl(credentialUrl(input.worker, input.credentials.path), { + method: 'GET', + headers: headers(input.worker, input.credentials.ref), + signal: input.signal, + }); + if (!response.ok) { + throw new Error(`Could not load the run's CLI logins (HTTP ${response.status}).`); + } + const body = (await response.json()) as { files?: CredentialFile[]; skipped?: Array<{ reason?: string }> }; + const home = path.join(input.runDir, '.cli-home'); + await fsp.mkdir(home, { recursive: true, mode: 0o700 }); + await fsp.chmod(home, 0o700).catch(() => undefined); + const env: Record = {}; + const files: MaterializedCliCredentials['files'] = []; + for (const entry of body.files ?? []) { + if ( + !ALLOWED_HOME_ENV.has(entry.homeEnv) || + !SAFE_SEGMENT.test(entry.directory) || + !SAFE_SEGMENT.test(entry.file) || + typeof entry.content !== 'string' + ) { + input.log?.(`[cloud-worker] skipping a CLI login with an unexpected layout (${entry.cli ?? 'unknown'})`); + continue; + } + const directory = path.join(home, entry.directory); + await fsp.mkdir(directory, { recursive: true, mode: 0o700 }); + await fsp.chmod(directory, 0o700).catch(() => undefined); + const filePath = path.join(directory, entry.file); + await fsp.writeFile(filePath, entry.content, { encoding: 'utf-8', mode: 0o600 }); + await fsp.chmod(filePath, 0o600).catch(() => undefined); + env[entry.homeEnv] = directory; + files.push({ credentialId: entry.credentialId, cli: entry.cli, filePath, original: entry.content }); + } + if (files.length) env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); + if (body.skipped?.length) { + input.log?.(`[cloud-worker] ${body.skipped.length} CLI login(s) unavailable for this run`); + } + return { env, files }; +} + +/** After the run: send back any auth file the CLI rewrote (token refresh). */ +export async function writeBackCliCredentials(input: { + worker: CloudWorkerRecord; + credentials: CliCredentialsPayload; + materialized: MaterializedCliCredentials; + fetchImpl: typeof fetch; + log?: (...args: unknown[]) => void; +}): Promise { + let written = 0; + for (const file of input.materialized.files) { + let current: string; + try { + current = await fsp.readFile(file.filePath, 'utf-8'); + } catch { + continue; + } + if (current === file.original) continue; + try { + const response = await input.fetchImpl( + credentialUrl(input.worker, input.credentials.path, `/${encodeURIComponent(file.credentialId)}`), + { + method: 'PUT', + headers: headers(input.worker, input.credentials.ref, true), + body: JSON.stringify({ content: current }), + } + ); + if (response.ok) written += 1; + else input.log?.(`[cloud-worker] ${file.cli} login write-back failed (HTTP ${response.status})`); + } catch { + input.log?.(`[cloud-worker] ${file.cli} login write-back failed`); + } + } + return written; +} diff --git a/packages/cli/src/cli/commands/cloud-worker.ts b/packages/cli/src/cli/commands/cloud-worker.ts index 604a429bf8..559686d1c8 100644 --- a/packages/cli/src/cli/commands/cloud-worker.ts +++ b/packages/cli/src/cli/commands/cloud-worker.ts @@ -20,6 +20,12 @@ import { } from '@agent-relay/cloud'; import { defaultExit } from '../lib/exit.js'; +import { + isCliCredentialsPayload, + materializeCliCredentials, + writeBackCliCredentials, + type MaterializedCliCredentials, +} from './cloud-worker-cli-credentials.js'; type ExitFn = (code: number) => never; @@ -80,11 +86,15 @@ function safePathSegment(value: string): string { function buildWorkerRuntimeEnv( payload: WorkerWorkflowPayload, - deps: CloudWorkerDependencies + deps: CloudWorkerDependencies, + cliCredentialEnv: Record = {} ): NodeJS.ProcessEnv { return { ...deps.env, ...payload.envSecrets, + // Materialised subscription logins (CODEX_HOME/GROK_HOME pointing into the + // run dir) win over anything inherited or delivered under those names. + ...cliCredentialEnv, AGENT_RELAY_CLOUD_WORKER_RUN_ID: payload.runId, RELAY_WORKSPACE_ID: payload.relayWorkspaceId, RELAY_API_KEY: payload.relaycastApiKey, @@ -217,6 +227,8 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex // directory/env and then invokes relayflows with the equivalent run flags. const runDir = path.join(cloudWorkerStateDir(deps.env), 'runs', payload.runId); const keepRunDir = deps.env.AGENT_RELAY_WORKER_KEEP_RUN_DIR === '1'; + const cliCredentials = isCliCredentialsPayload(payload.cliCredentials) ? payload.cliCredentials : null; + let materialized: MaterializedCliCredentials | null = null; try { await fsp.mkdir(runDir, { recursive: true, mode: 0o700 }); await fsp.chmod(runDir, 0o700).catch(() => undefined); @@ -225,12 +237,23 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex const workflowPath = path.join(runDir, safeFileName(payload.workflowFileName)); await writeSecretFile(workflowPath, payload.workflow); + if (cliCredentials) { + materialized = await materializeCliCredentials({ + worker, + credentials: cliCredentials, + runDir, + fetchImpl: deps.fetchImpl, + signal, + log: deps.log, + }); + } + const relayflowsCli = deps.resolveRelayflowsCliEntrypoint(); const result = await runChild({ command: process.execPath, args: relayflowsArgs(relayflowsCli, workflowPath, payload), cwd: runDir, - env: buildWorkerRuntimeEnv(payload, deps), + env: buildWorkerRuntimeEnv(payload, deps, materialized?.env), deps, signal, }); @@ -245,6 +268,15 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex summary: `Workflow ${payload.runId} completed.`, }; } finally { + if (cliCredentials && materialized?.files.length) { + await writeBackCliCredentials({ + worker, + credentials: cliCredentials, + materialized, + fetchImpl: deps.fetchImpl, + log: deps.log, + }).catch(() => undefined); + } if (!keepRunDir) { await fsp.rm(runDir, { recursive: true, force: true }).catch(() => undefined); } diff --git a/packages/cloud/src/worker.ts b/packages/cloud/src/worker.ts index 21f4208055..11024c7683 100644 --- a/packages/cloud/src/worker.ts +++ b/packages/cloud/src/worker.ts @@ -21,6 +21,17 @@ export type WorkerWorkflowPayload = { sourceFileType: WorkerFileType | 'workflow'; workflowFileName: string; envSecrets?: Record; + /** + * Subscription logins for the run: a short-lived reference the worker + * exchanges, with its worker token, for CLI auth files (see + * packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts). + */ + cliCredentials?: { + ref: string; + expiresAt?: string; + path: string; + credentials?: Array<{ credentialId: string; provider: string; cli: string }>; + }; metadata?: Record; s3CodeKey?: string; paths?: Array<{ From a04bb89f4cadb783b4c77c4e0f8c48f07513c030 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 20:21:36 -0800 Subject: [PATCH 02/13] style: prettier Co-Authored-By: Claude Opus 5.5 --- .../cloud-worker-cli-credentials.test.ts | 62 ++++++++++++++++--- .../commands/cloud-worker-cli-credentials.ts | 12 +++- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts index f228145791..906fda7cc8 100644 --- a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts @@ -39,9 +39,13 @@ function controlPlane(files: unknown[]) { body: typeof init.body === 'string' ? init.body : undefined, }); if ((init.method ?? 'GET') === 'GET') { - return new Response(JSON.stringify({ files, skipped: [] }), { headers: { 'content-type': 'application/json' } }); + return new Response(JSON.stringify({ files, skipped: [] }), { + headers: { 'content-type': 'application/json' }, + }); } - return new Response(JSON.stringify({ updated: true }), { headers: { 'content-type': 'application/json' } }); + return new Response(JSON.stringify({ updated: true }), { + headers: { 'content-type': 'application/json' }, + }); }); return { calls, fetchImpl: fetchImpl as unknown as typeof fetch }; } @@ -62,13 +66,29 @@ describe('cloud worker CLI logins', () => { try { const cp = controlPlane([ codexFile, - { ...codexFile, credentialId: 'c-grok', provider: 'xai', cli: 'grok', homeEnv: 'GROK_HOME', directory: '.grok', content: GROK }, + { + ...codexFile, + credentialId: 'c-grok', + provider: 'xai', + cli: 'grok', + homeEnv: 'GROK_HOME', + directory: '.grok', + content: GROK, + }, { ...codexFile, credentialId: 'evil', homeEnv: 'PATH' }, { ...codexFile, credentialId: 'evil2', directory: '../../etc' }, ]); const log = vi.fn(); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl, log }); - expect(cp.calls[0]!.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials'); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + log, + }); + expect(cp.calls[0]!.url).toBe( + 'https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials' + ); expect(cp.calls[0]!.headers.get('authorization')).toBe('Bearer sh_wrk_secret'); expect(cp.calls[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); expect(materialized.env).toEqual({ @@ -105,13 +125,32 @@ describe('cloud worker CLI logins', () => { it('writes back only files the CLI changed', async () => { const runDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-creds-')); try { - const cp = controlPlane([codexFile, { ...codexFile, credentialId: 'c-grok', cli: 'grok', homeEnv: 'GROK_HOME', directory: '.grok', content: GROK }]); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const cp = controlPlane([ + codexFile, + { + ...codexFile, + credentialId: 'c-grok', + cli: 'grok', + homeEnv: 'GROK_HOME', + directory: '.grok', + content: GROK, + }, + ]); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + }); const refreshed = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-2' } }); fs.writeFileSync(materialized.files[0]!.filePath, refreshed); - expect(await writeBackCliCredentials({ worker, credentials, materialized, fetchImpl: cp.fetchImpl })).toBe(1); + expect( + await writeBackCliCredentials({ worker, credentials, materialized, fetchImpl: cp.fetchImpl }) + ).toBe(1); const put = cp.calls.find((call) => call.method === 'PUT')!; - expect(put.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials/c-codex'); + expect(put.url).toBe( + 'https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials/c-codex' + ); expect(put.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); expect(JSON.parse(put.body!)).toEqual({ content: refreshed }); expect(cp.calls.filter((call) => call.method === 'PUT')).toHaveLength(1); @@ -131,7 +170,10 @@ describe('cloud worker CLI logins', () => { const authPath = path.join(options.env.CODEX_HOME!, 'auth.json'); fileDuringRun = fs.readFileSync(authPath, 'utf-8'); // The CLI refreshes its token during the run. - fs.writeFileSync(authPath, JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-3' } })); + fs.writeFileSync( + authPath, + JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-3' } }) + ); const child = new EventEmitter() as EventEmitter & { killed: boolean; kill: () => boolean }; child.killed = false; child.kill = () => true; diff --git a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts index fadbf69c00..3fe61abff1 100644 --- a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts @@ -56,7 +56,10 @@ function credentialUrl(worker: CloudWorkerRecord, template: string, suffix = '') const relative = template.replace('{workerId}', encodeURIComponent(worker.workerId)); // Only same-origin, relative API paths: the reference is never sent elsewhere. if (!relative.startsWith('/api/')) throw new Error('cliCredentials.path must be a relative /api/ path'); - return new URL(`${relative}${suffix}`, worker.baseUrl.endsWith('/') ? worker.baseUrl : `${worker.baseUrl}/`).toString(); + return new URL( + `${relative}${suffix}`, + worker.baseUrl.endsWith('/') ? worker.baseUrl : `${worker.baseUrl}/` + ).toString(); } function headers(worker: CloudWorkerRecord, ref: string, json = false): Headers { @@ -94,7 +97,9 @@ export async function materializeCliCredentials(input: { !SAFE_SEGMENT.test(entry.file) || typeof entry.content !== 'string' ) { - input.log?.(`[cloud-worker] skipping a CLI login with an unexpected layout (${entry.cli ?? 'unknown'})`); + input.log?.( + `[cloud-worker] skipping a CLI login with an unexpected layout (${entry.cli ?? 'unknown'})` + ); continue; } const directory = path.join(home, entry.directory); @@ -106,7 +111,8 @@ export async function materializeCliCredentials(input: { env[entry.homeEnv] = directory; files.push({ credentialId: entry.credentialId, cli: entry.cli, filePath, original: entry.content }); } - if (files.length) env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); + if (files.length) + env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); if (body.skipped?.length) { input.log?.(`[cloud-worker] ${body.skipped.length} CLI login(s) unavailable for this run`); } From 185bb75d10ddf1d6664d9af989b3babe534148cc Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 20:42:12 -0800 Subject: [PATCH 03/13] fix(cloud-worker): review fixes for subscription logins Move the helper to lib; accept only the exact credential route under the worker's base path (prefix kept, no other origin or route); mount a file only for a credential the assignment listed with that CLI's own layout; read write-backs without following symlinks; retry write-back with a timeout, report a login that could not be stored in the run summary, and store it for failed runs too; remove the login dir even when the run dir is kept; JSDoc; changelog. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 + .../cloud-worker-cli-credentials.test.ts | 223 -------------- .../commands/cloud-worker-cli-credentials.ts | 155 ---------- packages/cli/src/cli/commands/cloud-worker.ts | 36 ++- .../lib/cloud-worker-cli-credentials.test.ts | 274 ++++++++++++++++++ .../cli/lib/cloud-worker-cli-credentials.ts | 249 ++++++++++++++++ 6 files changed, 553 insertions(+), 388 deletions(-) delete mode 100644 packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts delete mode 100644 packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts create mode 100644 packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts create mode 100644 packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 02df606fe5..d0ae7aa270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased - Major] +### Added + +- `agent-relay cloud worker` mounts the workspace's Codex and Grok subscription logins for a run when the control plane offers them (`cliCredentials` in the assignment): each auth file is written 0600 under the run's private directory as `$CODEX_HOME/auth.json` / `$GROK_HOME/auth.json`, a file the CLI refreshed during the run is written back, and the files are removed after the run. + ### Changed - The core (`agent-relay-probe`) talks to one Agent Relay site, fixed when it is built: `AGENT_RELAY_SITE_URL` at build time (`https://agentrelay.com` when unset; an https origin, or `http://localhost[:port]` / `http://127.0.0.1[:port]`), and a build for a site the core could not trust fails to build. Nothing changes the site at run time, in any build. diff --git a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts deleted file mode 100644 index 906fda7cc8..0000000000 --- a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.test.ts +++ /dev/null @@ -1,223 +0,0 @@ -import { EventEmitter } from 'node:events'; -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; - -import { describe, expect, it, vi } from 'vitest'; - -import { createDefaultAssignmentRunner } from './cloud-worker.js'; -import { - materializeCliCredentials, - SUBSCRIPTION_LOGINS_ENV, - writeBackCliCredentials, -} from './cloud-worker-cli-credentials.js'; - -const worker = { - baseUrl: 'https://control.test', - workerId: 'wrk_1', - workerToken: 'sh_wrk_secret', - name: 'w', - heartbeatIntervalMs: 30_000, - registeredAt: '2026-11-03T00:00:00.000Z', - updatedAt: '2026-11-03T00:00:00.000Z', -} as never; - -const CODEX = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-1' } }); -const GROK = JSON.stringify({ 'https://auth.x.ai::c': { key: 'k', refresh_token: 'xrt-secret' } }); -const credentials = { - ref: 'sh_clicred_ref', - path: '/api/v1/workers/{workerId}/assignments/run_1/cli-credentials', -}; - -function controlPlane(files: unknown[]) { - const calls: Array<{ url: string; method: string; headers: Headers; body?: string }> = []; - const fetchImpl = vi.fn(async (url: string, init: RequestInit = {}) => { - calls.push({ - url, - method: init.method ?? 'GET', - headers: new Headers(init.headers), - body: typeof init.body === 'string' ? init.body : undefined, - }); - if ((init.method ?? 'GET') === 'GET') { - return new Response(JSON.stringify({ files, skipped: [] }), { - headers: { 'content-type': 'application/json' }, - }); - } - return new Response(JSON.stringify({ updated: true }), { - headers: { 'content-type': 'application/json' }, - }); - }); - return { calls, fetchImpl: fetchImpl as unknown as typeof fetch }; -} - -const codexFile = { - credentialId: 'c-codex', - provider: 'openai', - cli: 'codex', - homeEnv: 'CODEX_HOME', - directory: '.codex', - file: 'auth.json', - content: CODEX, -}; - -describe('cloud worker CLI logins', () => { - it('writes each auth file 0600 in a 0700 dir inside the run dir and points the CLI at it', async () => { - const runDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-creds-')); - try { - const cp = controlPlane([ - codexFile, - { - ...codexFile, - credentialId: 'c-grok', - provider: 'xai', - cli: 'grok', - homeEnv: 'GROK_HOME', - directory: '.grok', - content: GROK, - }, - { ...codexFile, credentialId: 'evil', homeEnv: 'PATH' }, - { ...codexFile, credentialId: 'evil2', directory: '../../etc' }, - ]); - const log = vi.fn(); - const materialized = await materializeCliCredentials({ - worker, - credentials, - runDir, - fetchImpl: cp.fetchImpl, - log, - }); - expect(cp.calls[0]!.url).toBe( - 'https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials' - ); - expect(cp.calls[0]!.headers.get('authorization')).toBe('Bearer sh_wrk_secret'); - expect(cp.calls[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); - expect(materialized.env).toEqual({ - CODEX_HOME: path.join(runDir, '.cli-home', '.codex'), - GROK_HOME: path.join(runDir, '.cli-home', '.grok'), - [SUBSCRIPTION_LOGINS_ENV]: 'codex,grok', - }); - const codexPath = path.join(runDir, '.cli-home', '.codex', 'auth.json'); - expect(fs.readFileSync(codexPath, 'utf-8')).toBe(CODEX); - expect(fs.statSync(codexPath).mode & 0o777).toBe(0o600); - expect(fs.statSync(path.dirname(codexPath)).mode & 0o777).toBe(0o700); - expect(fs.statSync(path.join(runDir, '.cli-home')).mode & 0o777).toBe(0o700); - expect(materialized.files.map((file) => file.credentialId)).toEqual(['c-codex', 'c-grok']); - expect(fs.existsSync(path.join(runDir, 'etc'))).toBe(false); - expect(JSON.stringify(log.mock.calls)).not.toContain('rt-secret'); - } finally { - fs.rmSync(runDir, { recursive: true, force: true }); - } - }); - - it('refuses a reference path that leaves the control plane', async () => { - const cp = controlPlane([]); - await expect( - materializeCliCredentials({ - worker, - credentials: { ...credentials, path: 'https://evil.test/steal' }, - runDir: os.tmpdir(), - fetchImpl: cp.fetchImpl, - }) - ).rejects.toThrow(/relative \/api\//); - expect(cp.calls).toHaveLength(0); - }); - - it('writes back only files the CLI changed', async () => { - const runDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-creds-')); - try { - const cp = controlPlane([ - codexFile, - { - ...codexFile, - credentialId: 'c-grok', - cli: 'grok', - homeEnv: 'GROK_HOME', - directory: '.grok', - content: GROK, - }, - ]); - const materialized = await materializeCliCredentials({ - worker, - credentials, - runDir, - fetchImpl: cp.fetchImpl, - }); - const refreshed = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-2' } }); - fs.writeFileSync(materialized.files[0]!.filePath, refreshed); - expect( - await writeBackCliCredentials({ worker, credentials, materialized, fetchImpl: cp.fetchImpl }) - ).toBe(1); - const put = cp.calls.find((call) => call.method === 'PUT')!; - expect(put.url).toBe( - 'https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials/c-codex' - ); - expect(put.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); - expect(JSON.parse(put.body!)).toEqual({ content: refreshed }); - expect(cp.calls.filter((call) => call.method === 'PUT')).toHaveLength(1); - } finally { - fs.rmSync(runDir, { recursive: true, force: true }); - } - }); - - it('the assignment runner mounts logins for relayflows, writes back, and deletes the run dir', async () => { - const home = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-creds-home-')); - try { - const cp = controlPlane([codexFile]); - let seenEnv: NodeJS.ProcessEnv = {}; - let fileDuringRun = ''; - const spawnProcess = vi.fn((_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { - seenEnv = options.env; - const authPath = path.join(options.env.CODEX_HOME!, 'auth.json'); - fileDuringRun = fs.readFileSync(authPath, 'utf-8'); - // The CLI refreshes its token during the run. - fs.writeFileSync( - authPath, - JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-3' } }) - ); - const child = new EventEmitter() as EventEmitter & { killed: boolean; kill: () => boolean }; - child.killed = false; - child.kill = () => true; - queueMicrotask(() => child.emit('exit', 0, null)); - return child; - }) as never; - const runner = createDefaultAssignmentRunner({ - log: vi.fn(), - error: vi.fn(), - exit: vi.fn() as never, - env: { AGENT_RELAY_HOME: home, CODEX_HOME: '/home/user/.codex' }, - spawnProcess, - now: () => new Date(), - cwd: () => home, - fetchImpl: cp.fetchImpl, - resolveRelayflowsCliEntrypoint: () => '/opt/relayflows/dist/cli.js', - }); - await runner({ - assignment: { runId: 'run_1' } as never, - payload: { - runId: 'run_1', - workspaceId: 'ws', - relayWorkspaceId: 'rw', - relaycastApiKey: 'rk', - relayfileUrl: '', - relayfileToken: '', - workflow: 'version: "1.0"\n', - fileType: 'yaml', - sourceFileType: 'yaml', - workflowFileName: 'workflow.yaml', - cliCredentials: credentials, - }, - worker, - signal: new AbortController().signal, - }); - const runDir = path.join(home, 'cloud-workers', 'runs', 'run_1'); - expect(seenEnv.CODEX_HOME).toBe(path.join(runDir, '.cli-home', '.codex')); - expect(seenEnv[SUBSCRIPTION_LOGINS_ENV]).toBe('codex'); - expect(fileDuringRun).toBe(CODEX); - const put = cp.calls.find((call) => call.method === 'PUT'); - expect(JSON.parse(put!.body!).content).toContain('rt-secret-3'); - expect(fs.existsSync(runDir)).toBe(false); - } finally { - fs.rmSync(home, { recursive: true, force: true }); - } - }); -}); diff --git a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts deleted file mode 100644 index 3fe61abff1..0000000000 --- a/packages/cli/src/cli/commands/cloud-worker-cli-credentials.ts +++ /dev/null @@ -1,155 +0,0 @@ -// Subscription-login materialisation for `agent-relay cloud worker`. -// -// A control plane that holds Codex/Grok subscription logins for the workspace -// (self-hosted relaycontrol; hosted Cloud mounts the same files in its own -// sandboxes) puts `cliCredentials` in the assignment payload: a short-lived, -// run-scoped reference, never the files. Before spawning relayflows, the worker -// exchanges the reference (with its worker token) for the auth files, writes -// each one 0600 into a 0700 directory inside the run's private dir, and points -// the CLI at it (CODEX_HOME / GROK_HOME). After the run it writes back a file -// the CLI refreshed, so the stored login keeps a live refresh token. The run -// dir, and the files with it, are deleted after the run. -// -// Nothing here logs a file's content or the reference. -import fsp from 'node:fs/promises'; -import path from 'node:path'; - -import type { CloudWorkerRecord } from '@agent-relay/cloud'; - -export type CliCredentialsPayload = { - ref: string; - expiresAt?: string; - /** Path template with `{workerId}`. */ - path: string; - credentials?: Array<{ credentialId: string; provider: string; cli: string }>; -}; - -type CredentialFile = { - credentialId: string; - provider: string; - cli: string; - homeEnv: string; - directory: string; - file: string; - content: string; -}; - -export type MaterializedCliCredentials = { - env: Record; - files: Array<{ credentialId: string; cli: string; filePath: string; original: string }>; -}; - -/** The only env names a materialised login may set. */ -const ALLOWED_HOME_ENV = new Set(['CODEX_HOME', 'GROK_HOME']); -const SAFE_SEGMENT = /^\.?[A-Za-z0-9_-]{1,64}(\.json)?$/; - -/** Env flag relayflows reads to give the subscription precedence over API keys. */ -export const SUBSCRIPTION_LOGINS_ENV = 'AGENT_RELAY_SUBSCRIPTION_LOGINS'; - -export function isCliCredentialsPayload(value: unknown): value is CliCredentialsPayload { - if (!value || typeof value !== 'object' || Array.isArray(value)) return false; - const candidate = value as Record; - return typeof candidate.ref === 'string' && candidate.ref.length > 0 && typeof candidate.path === 'string'; -} - -function credentialUrl(worker: CloudWorkerRecord, template: string, suffix = ''): string { - const relative = template.replace('{workerId}', encodeURIComponent(worker.workerId)); - // Only same-origin, relative API paths: the reference is never sent elsewhere. - if (!relative.startsWith('/api/')) throw new Error('cliCredentials.path must be a relative /api/ path'); - return new URL( - `${relative}${suffix}`, - worker.baseUrl.endsWith('/') ? worker.baseUrl : `${worker.baseUrl}/` - ).toString(); -} - -function headers(worker: CloudWorkerRecord, ref: string, json = false): Headers { - const result = new Headers({ Authorization: `Bearer ${worker.workerToken}`, 'X-Cli-Credential-Ref': ref }); - if (json) result.set('Content-Type', 'application/json'); - return result; -} - -export async function materializeCliCredentials(input: { - worker: CloudWorkerRecord; - credentials: CliCredentialsPayload; - runDir: string; - fetchImpl: typeof fetch; - signal?: AbortSignal; - log?: (...args: unknown[]) => void; -}): Promise { - const response = await input.fetchImpl(credentialUrl(input.worker, input.credentials.path), { - method: 'GET', - headers: headers(input.worker, input.credentials.ref), - signal: input.signal, - }); - if (!response.ok) { - throw new Error(`Could not load the run's CLI logins (HTTP ${response.status}).`); - } - const body = (await response.json()) as { files?: CredentialFile[]; skipped?: Array<{ reason?: string }> }; - const home = path.join(input.runDir, '.cli-home'); - await fsp.mkdir(home, { recursive: true, mode: 0o700 }); - await fsp.chmod(home, 0o700).catch(() => undefined); - const env: Record = {}; - const files: MaterializedCliCredentials['files'] = []; - for (const entry of body.files ?? []) { - if ( - !ALLOWED_HOME_ENV.has(entry.homeEnv) || - !SAFE_SEGMENT.test(entry.directory) || - !SAFE_SEGMENT.test(entry.file) || - typeof entry.content !== 'string' - ) { - input.log?.( - `[cloud-worker] skipping a CLI login with an unexpected layout (${entry.cli ?? 'unknown'})` - ); - continue; - } - const directory = path.join(home, entry.directory); - await fsp.mkdir(directory, { recursive: true, mode: 0o700 }); - await fsp.chmod(directory, 0o700).catch(() => undefined); - const filePath = path.join(directory, entry.file); - await fsp.writeFile(filePath, entry.content, { encoding: 'utf-8', mode: 0o600 }); - await fsp.chmod(filePath, 0o600).catch(() => undefined); - env[entry.homeEnv] = directory; - files.push({ credentialId: entry.credentialId, cli: entry.cli, filePath, original: entry.content }); - } - if (files.length) - env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); - if (body.skipped?.length) { - input.log?.(`[cloud-worker] ${body.skipped.length} CLI login(s) unavailable for this run`); - } - return { env, files }; -} - -/** After the run: send back any auth file the CLI rewrote (token refresh). */ -export async function writeBackCliCredentials(input: { - worker: CloudWorkerRecord; - credentials: CliCredentialsPayload; - materialized: MaterializedCliCredentials; - fetchImpl: typeof fetch; - log?: (...args: unknown[]) => void; -}): Promise { - let written = 0; - for (const file of input.materialized.files) { - let current: string; - try { - current = await fsp.readFile(file.filePath, 'utf-8'); - } catch { - continue; - } - if (current === file.original) continue; - try { - const response = await input.fetchImpl( - credentialUrl(input.worker, input.credentials.path, `/${encodeURIComponent(file.credentialId)}`), - { - method: 'PUT', - headers: headers(input.worker, input.credentials.ref, true), - body: JSON.stringify({ content: current }), - } - ); - if (response.ok) written += 1; - else input.log?.(`[cloud-worker] ${file.cli} login write-back failed (HTTP ${response.status})`); - } catch { - input.log?.(`[cloud-worker] ${file.cli} login write-back failed`); - } - } - return written; -} diff --git a/packages/cli/src/cli/commands/cloud-worker.ts b/packages/cli/src/cli/commands/cloud-worker.ts index 559686d1c8..c615025aa3 100644 --- a/packages/cli/src/cli/commands/cloud-worker.ts +++ b/packages/cli/src/cli/commands/cloud-worker.ts @@ -25,7 +25,7 @@ import { materializeCliCredentials, writeBackCliCredentials, type MaterializedCliCredentials, -} from './cloud-worker-cli-credentials.js'; +} from '../lib/cloud-worker-cli-credentials.js'; type ExitFn = (code: number) => never; @@ -229,6 +229,7 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex const keepRunDir = deps.env.AGENT_RELAY_WORKER_KEEP_RUN_DIR === '1'; const cliCredentials = isCliCredentialsPayload(payload.cliCredentials) ? payload.cliCredentials : null; let materialized: MaterializedCliCredentials | null = null; + let writeBackDone = false; try { await fsp.mkdir(runDir, { recursive: true, mode: 0o700 }); await fsp.chmod(runDir, 0o700).catch(() => undefined); @@ -262,25 +263,40 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex throw new Error(`Workflow runner exited with code ${result.exitCode}`); } + const failedWriteBacks = await storeRefreshedLogins(); return { exitCode: result.exitCode, durationMs: result.durationMs, - summary: `Workflow ${payload.runId} completed.`, + summary: failedWriteBacks.length + ? `Workflow ${payload.runId} completed. The refreshed ${failedWriteBacks.join(', ')} login could not be stored; sign in again if the next run fails.` + : `Workflow ${payload.runId} completed.`, }; } finally { - if (cliCredentials && materialized?.files.length) { - await writeBackCliCredentials({ - worker, - credentials: cliCredentials, - materialized, - fetchImpl: deps.fetchImpl, - log: deps.log, - }).catch(() => undefined); + // A failed or cancelled run still stores a refreshed login (Grok rotates + // its single-use refresh token), then the login files are removed even + // when the run directory is kept for debugging. + await storeRefreshedLogins(); + if (materialized) { + await fsp.rm(materialized.home, { recursive: true, force: true }).catch(() => undefined); } if (!keepRunDir) { await fsp.rm(runDir, { recursive: true, force: true }).catch(() => undefined); } } + + /** Writes back refreshed logins once; returns the CLIs whose login could not be stored. */ + async function storeRefreshedLogins(): Promise { + if (!cliCredentials || !materialized?.files.length || writeBackDone) return []; + writeBackDone = true; + const outcome = await writeBackCliCredentials({ + worker, + credentials: cliCredentials, + materialized, + fetchImpl: deps.fetchImpl, + log: deps.log, + }).catch(() => ({ written: 0, failed: materialized?.files ?? [] })); + return outcome.failed.map((file) => file.cli); + } }; } diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts new file mode 100644 index 0000000000..8b6f0c3ecb --- /dev/null +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -0,0 +1,274 @@ +import { EventEmitter } from 'node:events'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { describe, expect, it, vi } from 'vitest'; + +import { createDefaultAssignmentRunner } from '../commands/cloud-worker.js'; +import { + materializeCliCredentials, + SUBSCRIPTION_LOGINS_ENV, + writeBackCliCredentials, +} from './cloud-worker-cli-credentials.js'; + +const posix = process.platform !== 'win32'; + +function workerFor(baseUrl = 'https://control.test') { + return { + baseUrl, + workerId: 'wrk_1', + workerToken: 'sh_wrk_secret', + name: 'w', + heartbeatIntervalMs: 30_000, + registeredAt: '2026-11-03T00:00:00.000Z', + updatedAt: '2026-11-03T00:00:00.000Z', + } as never; +} +const worker = workerFor(); + +const CODEX = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-1' } }); +const GROK = JSON.stringify({ 'https://auth.x.ai::c': { key: 'k', refresh_token: 'xrt-secret' } }); +const credentials = { + ref: 'sh_clicred_ref', + path: '/api/v1/workers/{workerId}/assignments/run_1/cli-credentials', + credentials: [ + { credentialId: 'c-codex', provider: 'openai', cli: 'codex' }, + { credentialId: 'c-grok', provider: 'xai', cli: 'grok' }, + ], +}; + +const codexFile = { + credentialId: 'c-codex', + provider: 'openai', + cli: 'codex', + homeEnv: 'CODEX_HOME', + directory: '.codex', + file: 'auth.json', + content: CODEX, +}; +const grokFile = { + credentialId: 'c-grok', + provider: 'xai', + cli: 'grok', + homeEnv: 'GROK_HOME', + directory: '.grok', + file: 'auth.json', + content: GROK, +}; + +function controlPlane(files: unknown[], putStatus = 200) { + const calls: Array<{ url: string; method: string; headers: Headers; body?: string }> = []; + const fetchImpl = vi.fn(async (url: string, init: RequestInit = {}) => { + calls.push({ + url, + method: init.method ?? 'GET', + headers: new Headers(init.headers), + body: typeof init.body === 'string' ? init.body : undefined, + }); + if ((init.method ?? 'GET') === 'GET') { + return new Response(JSON.stringify({ files, skipped: [] }), { + headers: { 'content-type': 'application/json' }, + }); + } + return new Response(JSON.stringify({ updated: putStatus === 200 }), { + status: putStatus, + headers: { 'content-type': 'application/json' }, + }); + }); + return { calls, fetchImpl: fetchImpl as unknown as typeof fetch }; +} + +function tmp(prefix: string) { + return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); +} + +describe('cloud worker CLI logins', () => { + it('writes each auth file 0600 in a 0700 dir inside the run dir and points the CLI at it', async () => { + const runDir = tmp('cli-creds-'); + try { + const cp = controlPlane([codexFile, grokFile]); + const log = vi.fn(); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl, log }); + expect(cp.calls[0]!.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials'); + expect(cp.calls[0]!.headers.get('authorization')).toBe('Bearer sh_wrk_secret'); + expect(cp.calls[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); + expect(materialized.home).toBe(path.join(runDir, '.cli-home')); + expect(materialized.env).toEqual({ + CODEX_HOME: path.join(runDir, '.cli-home', '.codex'), + GROK_HOME: path.join(runDir, '.cli-home', '.grok'), + [SUBSCRIPTION_LOGINS_ENV]: 'codex,grok', + }); + const codexPath = path.join(runDir, '.cli-home', '.codex', 'auth.json'); + expect(fs.readFileSync(codexPath, 'utf-8')).toBe(CODEX); + if (posix) { + expect(fs.statSync(codexPath).mode & 0o777).toBe(0o600); + expect(fs.statSync(path.dirname(codexPath)).mode & 0o777).toBe(0o700); + expect(fs.statSync(path.join(runDir, '.cli-home')).mode & 0o777).toBe(0o700); + } + expect(JSON.stringify(log.mock.calls)).not.toContain('rt-secret'); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('mounts only files that match a credential the assignment listed, with its own CLI layout', async () => { + const runDir = tmp('cli-creds-'); + try { + const cp = controlPlane([ + { ...codexFile, homeEnv: 'GROK_HOME', directory: '.grok' }, + { ...codexFile, credentialId: 'c-grok' }, + { ...codexFile, credentialId: 'not-listed' }, + { ...codexFile, homeEnv: 'PATH' }, + { ...codexFile, directory: '../../etc' }, + { ...codexFile, cli: 'claude' }, + ]); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + expect(materialized.files).toEqual([]); + expect(materialized.env).toEqual({}); + expect(fs.existsSync(path.join(runDir, '.cli-home', '.grok'))).toBe(false); + expect(fs.existsSync(path.join(runDir, 'etc'))).toBe(false); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('keeps the base path prefix and refuses any path but the credential route', async () => { + const runDir = tmp('cli-creds-'); + try { + const prefixed = controlPlane([codexFile]); + await materializeCliCredentials({ + worker: workerFor('https://agentrelay.test/cloud/'), + credentials, + runDir, + fetchImpl: prefixed.fetchImpl, + }); + expect(prefixed.calls[0]!.url).toBe( + 'https://agentrelay.test/cloud/api/v1/workers/wrk_1/assignments/run_1/cli-credentials' + ); + for (const hostile of [ + 'https://evil.test/steal', + '/api/\\evil.example/x', + '/api/v1/workers/{workerId}/assignments/run_1/../../../admin', + '/api/v1/workers/{workerId}/assignments/run_1/cli-credentials?x=1', + '//evil.test/api/v1/workers/{workerId}/assignments/run_1/cli-credentials', + ]) { + const cp = controlPlane([]); + await expect( + materializeCliCredentials({ worker, credentials: { ...credentials, path: hostile }, runDir, fetchImpl: cp.fetchImpl }) + ).rejects.toThrow(/credential route/); + expect(cp.calls).toHaveLength(0); + } + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('writes back only changed regular files, never through a symlink', async () => { + const runDir = tmp('cli-creds-'); + try { + const cp = controlPlane([codexFile, grokFile]); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const refreshed = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-2' } }); + fs.writeFileSync(materialized.files[0]!.filePath, refreshed); + if (posix) { + const secret = path.join(runDir, 'host-secret.txt'); + fs.writeFileSync(secret, 'not a login'); + fs.rmSync(materialized.files[1]!.filePath); + fs.symlinkSync(secret, materialized.files[1]!.filePath); + } + const outcome = await writeBackCliCredentials({ worker, credentials, materialized, fetchImpl: cp.fetchImpl }); + expect(outcome).toEqual({ written: 1, failed: [] }); + const puts = cp.calls.filter((call) => call.method === 'PUT'); + expect(puts).toHaveLength(1); + expect(puts[0]!.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials/c-codex'); + expect(puts[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); + expect(JSON.parse(puts[0]!.body!)).toEqual({ content: refreshed }); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('retries a failed write-back and reports it when it cannot be stored', async () => { + const runDir = tmp('cli-creds-'); + try { + const cp = controlPlane([codexFile], 503); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + fs.writeFileSync(materialized.files[0]!.filePath, '{"changed":true}'); + const outcome = await writeBackCliCredentials({ + worker, + credentials, + materialized, + fetchImpl: cp.fetchImpl, + retryDelayMs: 1, + }); + expect(outcome).toEqual({ written: 0, failed: [{ credentialId: 'c-codex', cli: 'codex' }] }); + expect(cp.calls.filter((call) => call.method === 'PUT')).toHaveLength(3); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('the assignment runner mounts logins for relayflows, writes back, and removes the logins', async () => { + const home = tmp('cli-creds-home-'); + try { + const cp = controlPlane([codexFile]); + let seenEnv: NodeJS.ProcessEnv = {}; + let fileDuringRun = ''; + const spawnProcess = vi.fn((_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + seenEnv = options.env; + const authPath = path.join(options.env.CODEX_HOME!, 'auth.json'); + fileDuringRun = fs.readFileSync(authPath, 'utf-8'); + // The CLI refreshes its token during the run. + fs.writeFileSync(authPath, JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-3' } })); + const child = new EventEmitter() as EventEmitter & { killed: boolean; kill: () => boolean }; + child.killed = false; + child.kill = () => true; + queueMicrotask(() => child.emit('exit', 0, null)); + return child; + }) as never; + const runner = createDefaultAssignmentRunner({ + log: vi.fn(), + error: vi.fn(), + exit: vi.fn() as never, + // The run dir is kept, but the logins must still be removed. + env: { AGENT_RELAY_HOME: home, CODEX_HOME: '/home/user/.codex', AGENT_RELAY_WORKER_KEEP_RUN_DIR: '1' }, + spawnProcess, + now: () => new Date(), + cwd: () => home, + fetchImpl: cp.fetchImpl, + resolveRelayflowsCliEntrypoint: () => '/opt/relayflows/dist/cli.js', + }); + const result = await runner({ + assignment: { runId: 'run_1' } as never, + payload: { + runId: 'run_1', + workspaceId: 'ws', + relayWorkspaceId: 'rw', + relaycastApiKey: 'rk', + relayfileUrl: '', + relayfileToken: '', + workflow: 'version: "1.0"\n', + fileType: 'yaml', + sourceFileType: 'yaml', + workflowFileName: 'workflow.yaml', + cliCredentials: credentials, + }, + worker, + signal: new AbortController().signal, + }); + const runDir = path.join(home, 'cloud-workers', 'runs', 'run_1'); + expect(seenEnv.CODEX_HOME).toBe(path.join(runDir, '.cli-home', '.codex')); + expect(seenEnv[SUBSCRIPTION_LOGINS_ENV]).toBe('codex'); + expect(fileDuringRun).toBe(CODEX); + const puts = cp.calls.filter((call) => call.method === 'PUT'); + expect(puts).toHaveLength(1); + expect(JSON.parse(puts[0]!.body!).content).toContain('rt-secret-3'); + expect(result.summary).toBe('Workflow run_1 completed.'); + expect(fs.existsSync(runDir)).toBe(true); + expect(fs.existsSync(path.join(runDir, '.cli-home'))).toBe(false); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts new file mode 100644 index 0000000000..21730fd1f1 --- /dev/null +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -0,0 +1,249 @@ +// Subscription-login materialisation for `agent-relay cloud worker`. +// +// A control plane that holds Codex/Grok subscription logins for the workspace +// (self-hosted relaycontrol; hosted Cloud mounts the same files in its own +// sandboxes) puts `cliCredentials` in the assignment payload: a short-lived, +// run-scoped reference, never the files. Before spawning relayflows, the worker +// exchanges the reference (with its worker token) for the auth files, writes +// each one 0600 into a 0700 directory inside the run's private dir, and points +// the CLI at it (CODEX_HOME / GROK_HOME). After the run it writes back a file +// the CLI refreshed, so the stored login keeps a live refresh token. The login +// directory is deleted after the run (even when the run dir is kept). +// +// Nothing here logs a file's content or the reference. +import fs from 'node:fs'; +import fsp from 'node:fs/promises'; +import path from 'node:path'; + +import type { CloudWorkerRecord } from '@agent-relay/cloud'; + +/** The `cliCredentials` field of an assignment payload. */ +export type CliCredentialsPayload = { + ref: string; + expiresAt?: string; + /** `/api/v1/workers/{workerId}/assignments//cli-credentials`. */ + path: string; + credentials?: Array<{ credentialId: string; provider: string; cli: string }>; +}; + +type CredentialFile = { + credentialId: string; + provider: string; + cli: string; + homeEnv: string; + directory: string; + file: string; + content: string; +}; + +/** What was mounted for a run. */ +export type MaterializedCliCredentials = { + /** Env for the relayflows process (CODEX_HOME/GROK_HOME, AGENT_RELAY_SUBSCRIPTION_LOGINS). */ + env: Record; + /** The private directory holding every mounted login; delete it after the run. */ + home: string; + files: Array<{ credentialId: string; cli: string; filePath: string; original: string }>; +}; + +/** Result of a write-back pass. */ +export type CliCredentialWriteBack = { written: number; failed: Array<{ credentialId: string; cli: string }> }; + +/** The only layouts a login may use: each CLI's own home variable, directory and file. */ +const CLI_LAYOUTS: Record = { + codex: { homeEnv: 'CODEX_HOME', directory: '.codex', file: 'auth.json' }, + grok: { homeEnv: 'GROK_HOME', directory: '.grok', file: 'auth.json' }, +}; + +/** Env flag relayflows reads to give the subscription precedence over API keys. */ +export const SUBSCRIPTION_LOGINS_ENV = 'AGENT_RELAY_SUBSCRIPTION_LOGINS'; + +const CREDENTIAL_PATH = /^\/api\/v1\/workers\/\{workerId\}\/assignments\/[A-Za-z0-9_-]{1,128}\/cli-credentials$/; +const CREDENTIAL_ID = /^[A-Za-z0-9_-]{1,128}$/; +const WRITE_BACK_TIMEOUT_MS = 15_000; +const WRITE_BACK_ATTEMPTS = 3; + +/** + * Whether an assignment payload's `cliCredentials` has the expected shape. + * + * @param value - The raw `payload.cliCredentials` value. + * @returns True when it carries a reference and a credential path. + */ +export function isCliCredentialsPayload(value: unknown): value is CliCredentialsPayload { + if (!value || typeof value !== 'object' || Array.isArray(value)) return false; + const candidate = value as Record; + return typeof candidate.ref === 'string' && candidate.ref.length > 0 && typeof candidate.path === 'string'; +} + +/** + * Builds a credential URL under the worker's API base, keeping any base path + * prefix (e.g. `/cloud`). Only the exact credential route is accepted, so the + * worker token and reference can never be sent to another origin or route. + */ +function credentialUrl(worker: CloudWorkerRecord, template: string, credentialId?: string): string { + if (!CREDENTIAL_PATH.test(template)) { + throw new Error('cliCredentials.path is not the worker credential route'); + } + if (credentialId !== undefined && !CREDENTIAL_ID.test(credentialId)) { + throw new Error('Invalid CLI credential id'); + } + const base = new URL(worker.baseUrl); + const relative = template.replace('{workerId}', encodeURIComponent(worker.workerId)); + const url = new URL( + `${worker.baseUrl.replace(/\/+$/, '')}${relative}${credentialId === undefined ? '' : `/${credentialId}`}` + ); + if (url.origin !== base.origin || url.search || url.hash) { + throw new Error('cliCredentials.path resolves outside the control plane'); + } + return url.toString(); +} + +function headers(worker: CloudWorkerRecord, ref: string, json = false): Headers { + const result = new Headers({ Authorization: `Bearer ${worker.workerToken}`, 'X-Cli-Credential-Ref': ref }); + if (json) result.set('Content-Type', 'application/json'); + return result; +} + +/** + * Fetches the run's subscription logins and writes each auth file 0600 into a + * 0700 directory under `/.cli-home`. + * + * A file is mounted only when it matches a credential the assignment payload + * listed, with that credential's CLI and the CLI's own layout; anything else is + * skipped (and logged by CLI name only). + * + * @param input.worker - The authenticated worker (token sent with the request). + * @param input.credentials - The payload's `cliCredentials`. + * @param input.runDir - The run's private directory. + * @returns The env for relayflows, the login directory, and the mounted files. + * @throws When the control plane refuses the reference or the path is not the credential route. + */ +export async function materializeCliCredentials(input: { + worker: CloudWorkerRecord; + credentials: CliCredentialsPayload; + runDir: string; + fetchImpl: typeof fetch; + signal?: AbortSignal; + log?: (...args: unknown[]) => void; +}): Promise { + const response = await input.fetchImpl(credentialUrl(input.worker, input.credentials.path), { + method: 'GET', + headers: headers(input.worker, input.credentials.ref), + signal: input.signal, + }); + if (!response.ok) { + throw new Error(`Could not load the run's CLI logins (HTTP ${response.status}).`); + } + const body = (await response.json()) as { files?: CredentialFile[]; skipped?: Array<{ reason?: string }> }; + const expected = new Map((input.credentials.credentials ?? []).map((entry) => [entry.credentialId, entry.cli])); + const home = path.join(input.runDir, '.cli-home'); + await fsp.mkdir(home, { recursive: true, mode: 0o700 }); + await fsp.chmod(home, 0o700).catch(() => undefined); + const env: Record = {}; + const files: MaterializedCliCredentials['files'] = []; + for (const entry of body.files ?? []) { + const layout = CLI_LAYOUTS[entry.cli]; + const valid = + layout !== undefined && + typeof entry.credentialId === 'string' && + CREDENTIAL_ID.test(entry.credentialId) && + expected.get(entry.credentialId) === entry.cli && + entry.homeEnv === layout.homeEnv && + entry.directory === layout.directory && + entry.file === layout.file && + typeof entry.content === 'string' && + !(layout.homeEnv in env); + if (!valid || !layout) { + input.log?.(`[cloud-worker] skipping a CLI login that does not match the assignment (${String(entry.cli)})`); + continue; + } + const directory = path.join(home, layout.directory); + await fsp.mkdir(directory, { recursive: true, mode: 0o700 }); + await fsp.chmod(directory, 0o700).catch(() => undefined); + const filePath = path.join(directory, layout.file); + await fsp.writeFile(filePath, entry.content, { encoding: 'utf-8', mode: 0o600 }); + await fsp.chmod(filePath, 0o600).catch(() => undefined); + env[layout.homeEnv] = directory; + files.push({ credentialId: entry.credentialId, cli: entry.cli, filePath, original: entry.content }); + } + if (files.length) env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); + if (body.skipped?.length) { + input.log?.(`[cloud-worker] ${body.skipped.length} CLI login(s) unavailable for this run`); + } + return { env, home, files }; +} + +/** Reads a regular file without following a symlink a workflow may have planted. */ +async function readRegularFile(filePath: string): Promise { + const noFollow = fs.constants.O_NOFOLLOW ?? 0; + if (!noFollow) { + const stat = await fsp.lstat(filePath).catch(() => null); + if (!stat?.isFile()) return null; + } + let handle: fsp.FileHandle; + try { + handle = await fsp.open(filePath, fs.constants.O_RDONLY | noFollow); + } catch { + return null; + } + try { + const stat = await handle.stat(); + if (!stat.isFile() || stat.size > 256 * 1024) return null; + return await handle.readFile({ encoding: 'utf-8' }); + } finally { + await handle.close(); + } +} + +/** + * After the run, sends back each auth file the CLI rewrote (its own token + * refresh), so the stored login keeps a live refresh token. Each PUT has its + * own timeout and is retried; it never waits on the run's abort signal, so a + * cancelled run still stores a rotated (single-use) refresh token. + * + * @returns How many files were stored, and which changed files could not be. + */ +export async function writeBackCliCredentials(input: { + worker: CloudWorkerRecord; + credentials: CliCredentialsPayload; + materialized: MaterializedCliCredentials; + fetchImpl: typeof fetch; + log?: (...args: unknown[]) => void; + retryDelayMs?: number; +}): Promise { + const result: CliCredentialWriteBack = { written: 0, failed: [] }; + for (const file of input.materialized.files) { + const current = await readRegularFile(file.filePath); + if (current === null || current === file.original) continue; + let stored = false; + for (let attempt = 1; attempt <= WRITE_BACK_ATTEMPTS && !stored; attempt += 1) { + try { + const response = await input.fetchImpl( + credentialUrl(input.worker, input.credentials.path, file.credentialId), + { + method: 'PUT', + headers: headers(input.worker, input.credentials.ref, true), + body: JSON.stringify({ content: current }), + signal: AbortSignal.timeout(WRITE_BACK_TIMEOUT_MS), + } + ); + if (response.ok) stored = true; + // A 4xx will not succeed on retry. + else if (response.status < 500 && response.status !== 429) break; + } catch { + // Network failure or timeout: retry. + } + if (!stored && attempt < WRITE_BACK_ATTEMPTS) { + await new Promise((resolve) => setTimeout(resolve, (input.retryDelayMs ?? 1_000) * attempt)); + } + } + if (stored) { + result.written += 1; + } else { + result.failed.push({ credentialId: file.credentialId, cli: file.cli }); + input.log?.( + `[cloud-worker] the refreshed ${file.cli} login could not be stored; sign in again if the next run fails` + ); + } + } + return result; +} From 728f63ca886bd5b08683c34442a892ded3f580e5 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 20:42:42 -0800 Subject: [PATCH 04/13] style: prettier Co-Authored-By: Claude Opus 5.5 --- .../lib/cloud-worker-cli-credentials.test.ts | 62 ++++++++++++++++--- .../cli/lib/cloud-worker-cli-credentials.ts | 19 ++++-- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index 8b6f0c3ecb..6826f150f1 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -89,8 +89,16 @@ describe('cloud worker CLI logins', () => { try { const cp = controlPlane([codexFile, grokFile]); const log = vi.fn(); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl, log }); - expect(cp.calls[0]!.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials'); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + log, + }); + expect(cp.calls[0]!.url).toBe( + 'https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials' + ); expect(cp.calls[0]!.headers.get('authorization')).toBe('Bearer sh_wrk_secret'); expect(cp.calls[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); expect(materialized.home).toBe(path.join(runDir, '.cli-home')); @@ -123,7 +131,12 @@ describe('cloud worker CLI logins', () => { { ...codexFile, directory: '../../etc' }, { ...codexFile, cli: 'claude' }, ]); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + }); expect(materialized.files).toEqual([]); expect(materialized.env).toEqual({}); expect(fs.existsSync(path.join(runDir, '.cli-home', '.grok'))).toBe(false); @@ -155,7 +168,12 @@ describe('cloud worker CLI logins', () => { ]) { const cp = controlPlane([]); await expect( - materializeCliCredentials({ worker, credentials: { ...credentials, path: hostile }, runDir, fetchImpl: cp.fetchImpl }) + materializeCliCredentials({ + worker, + credentials: { ...credentials, path: hostile }, + runDir, + fetchImpl: cp.fetchImpl, + }) ).rejects.toThrow(/credential route/); expect(cp.calls).toHaveLength(0); } @@ -168,7 +186,12 @@ describe('cloud worker CLI logins', () => { const runDir = tmp('cli-creds-'); try { const cp = controlPlane([codexFile, grokFile]); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + }); const refreshed = JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-2' } }); fs.writeFileSync(materialized.files[0]!.filePath, refreshed); if (posix) { @@ -177,11 +200,18 @@ describe('cloud worker CLI logins', () => { fs.rmSync(materialized.files[1]!.filePath); fs.symlinkSync(secret, materialized.files[1]!.filePath); } - const outcome = await writeBackCliCredentials({ worker, credentials, materialized, fetchImpl: cp.fetchImpl }); + const outcome = await writeBackCliCredentials({ + worker, + credentials, + materialized, + fetchImpl: cp.fetchImpl, + }); expect(outcome).toEqual({ written: 1, failed: [] }); const puts = cp.calls.filter((call) => call.method === 'PUT'); expect(puts).toHaveLength(1); - expect(puts[0]!.url).toBe('https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials/c-codex'); + expect(puts[0]!.url).toBe( + 'https://control.test/api/v1/workers/wrk_1/assignments/run_1/cli-credentials/c-codex' + ); expect(puts[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); expect(JSON.parse(puts[0]!.body!)).toEqual({ content: refreshed }); } finally { @@ -193,7 +223,12 @@ describe('cloud worker CLI logins', () => { const runDir = tmp('cli-creds-'); try { const cp = controlPlane([codexFile], 503); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + }); fs.writeFileSync(materialized.files[0]!.filePath, '{"changed":true}'); const outcome = await writeBackCliCredentials({ worker, @@ -220,7 +255,10 @@ describe('cloud worker CLI logins', () => { const authPath = path.join(options.env.CODEX_HOME!, 'auth.json'); fileDuringRun = fs.readFileSync(authPath, 'utf-8'); // The CLI refreshes its token during the run. - fs.writeFileSync(authPath, JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-3' } })); + fs.writeFileSync( + authPath, + JSON.stringify({ auth_mode: 'chatgpt', tokens: { refresh_token: 'rt-secret-3' } }) + ); const child = new EventEmitter() as EventEmitter & { killed: boolean; kill: () => boolean }; child.killed = false; child.kill = () => true; @@ -232,7 +270,11 @@ describe('cloud worker CLI logins', () => { error: vi.fn(), exit: vi.fn() as never, // The run dir is kept, but the logins must still be removed. - env: { AGENT_RELAY_HOME: home, CODEX_HOME: '/home/user/.codex', AGENT_RELAY_WORKER_KEEP_RUN_DIR: '1' }, + env: { + AGENT_RELAY_HOME: home, + CODEX_HOME: '/home/user/.codex', + AGENT_RELAY_WORKER_KEEP_RUN_DIR: '1', + }, spawnProcess, now: () => new Date(), cwd: () => home, diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index 21730fd1f1..daf901307a 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -46,7 +46,10 @@ export type MaterializedCliCredentials = { }; /** Result of a write-back pass. */ -export type CliCredentialWriteBack = { written: number; failed: Array<{ credentialId: string; cli: string }> }; +export type CliCredentialWriteBack = { + written: number; + failed: Array<{ credentialId: string; cli: string }>; +}; /** The only layouts a login may use: each CLI's own home variable, directory and file. */ const CLI_LAYOUTS: Record = { @@ -57,7 +60,8 @@ const CLI_LAYOUTS: Record }; - const expected = new Map((input.credentials.credentials ?? []).map((entry) => [entry.credentialId, entry.cli])); + const expected = new Map( + (input.credentials.credentials ?? []).map((entry) => [entry.credentialId, entry.cli]) + ); const home = path.join(input.runDir, '.cli-home'); await fsp.mkdir(home, { recursive: true, mode: 0o700 }); await fsp.chmod(home, 0o700).catch(() => undefined); @@ -153,7 +159,9 @@ export async function materializeCliCredentials(input: { typeof entry.content === 'string' && !(layout.homeEnv in env); if (!valid || !layout) { - input.log?.(`[cloud-worker] skipping a CLI login that does not match the assignment (${String(entry.cli)})`); + input.log?.( + `[cloud-worker] skipping a CLI login that does not match the assignment (${String(entry.cli)})` + ); continue; } const directory = path.join(home, layout.directory); @@ -165,7 +173,8 @@ export async function materializeCliCredentials(input: { env[layout.homeEnv] = directory; files.push({ credentialId: entry.credentialId, cli: entry.cli, filePath, original: entry.content }); } - if (files.length) env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); + if (files.length) + env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); if (body.skipped?.length) { input.log?.(`[cloud-worker] ${body.skipped.length} CLI login(s) unavailable for this run`); } From f003902b7045613c92e13d1ab838aa00ee9a0f8c Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:07:37 -0800 Subject: [PATCH 05/13] fix(cloud-worker): second review pass on subscription logins Require the offered-credential list; create the login dir fresh and write files with O_EXCL so a planted symlink is never followed; refuse redirects; skip malformed entries; remove the login dir whenever logins were offered and log a failed removal; report an unstored login on failed runs too; clarify that relayflows precedence needs the relayflows release with #65; split the changelog entry. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +- packages/cli/src/cli/commands/cloud-worker.ts | 20 +++- .../lib/cloud-worker-cli-credentials.test.ts | 93 +++++++++++++++++++ .../cli/lib/cloud-worker-cli-credentials.ts | 79 +++++++++++++--- 4 files changed, 175 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0ae7aa270..14a752478f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `agent-relay cloud worker` mounts the workspace's Codex and Grok subscription logins for a run when the control plane offers them (`cliCredentials` in the assignment): each auth file is written 0600 under the run's private directory as `$CODEX_HOME/auth.json` / `$GROK_HOME/auth.json`, a file the CLI refreshed during the run is written back, and the files are removed after the run. +- `agent-relay cloud worker` can run Codex and Grok jobs with the workspace's subscription logins when the control plane provides them; the login files exist only while the run does. +- `agent-relay cloud worker` saves refreshed Codex and Grok login tokens for later runs. ### Changed diff --git a/packages/cli/src/cli/commands/cloud-worker.ts b/packages/cli/src/cli/commands/cloud-worker.ts index c615025aa3..d6072a4b88 100644 --- a/packages/cli/src/cli/commands/cloud-worker.ts +++ b/packages/cli/src/cli/commands/cloud-worker.ts @@ -271,13 +271,23 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex ? `Workflow ${payload.runId} completed. The refreshed ${failedWriteBacks.join(', ')} login could not be stored; sign in again if the next run fails.` : `Workflow ${payload.runId} completed.`, }; - } finally { + } catch (error) { // A failed or cancelled run still stores a refreshed login (Grok rotates - // its single-use refresh token), then the login files are removed even - // when the run directory is kept for debugging. + // its single-use refresh token), and says so when it could not. + const failedWriteBacks = await storeRefreshedLogins(); + if (failedWriteBacks.length && error instanceof Error) { + error.message = `${error.message} The refreshed ${failedWriteBacks.join(', ')} login could not be stored; sign in again if the next run fails.`; + } + throw error; + } finally { await storeRefreshedLogins(); - if (materialized) { - await fsp.rm(materialized.home, { recursive: true, force: true }).catch(() => undefined); + // The login files are removed even when the run directory is kept for + // debugging, and even when materialisation failed partway. + if (cliCredentials) { + const loginHome = materialized?.home ?? path.join(runDir, '.cli-home'); + await fsp.rm(loginHome, { recursive: true, force: true }).catch(() => { + deps.error('[cloud-worker] could not remove the run\'s CLI login directory; delete it manually'); + }); } if (!keepRunDir) { await fsp.rm(runDir, { recursive: true, force: true }).catch(() => undefined); diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index 6826f150f1..a72cc69132 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -111,6 +111,7 @@ describe('cloud worker CLI logins', () => { expect(fs.readFileSync(codexPath, 'utf-8')).toBe(CODEX); if (posix) { expect(fs.statSync(codexPath).mode & 0o777).toBe(0o600); + expect(fs.statSync(path.join(runDir, '.cli-home', '.grok', 'auth.json')).mode & 0o777).toBe(0o600); expect(fs.statSync(path.dirname(codexPath)).mode & 0o777).toBe(0o700); expect(fs.statSync(path.join(runDir, '.cli-home')).mode & 0o777).toBe(0o700); } @@ -313,4 +314,96 @@ describe('cloud worker CLI logins', () => { fs.rmSync(home, { recursive: true, force: true }); } }); + + it('fails clearly when the reference has no credential list, and skips malformed entries', async () => { + const runDir = tmp('cli-creds-'); + try { + const cp = controlPlane([codexFile]); + await expect( + materializeCliCredentials({ + worker, + credentials: { ref: credentials.ref, path: credentials.path }, + runDir, + fetchImpl: cp.fetchImpl, + }) + ).rejects.toThrow(/must list the CLI logins/); + const malformed = controlPlane([null, 7, codexFile]); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: malformed.fetchImpl, + }); + expect(materialized.files.map((file) => file.credentialId)).toEqual(['c-codex']); + expect(malformed.calls[0]!.url).toContain('/cli-credentials'); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + } + }); + + it('never writes a login through a symlink planted in the run dir', async () => { + if (!posix) return; + const runDir = tmp('cli-creds-'); + const outside = tmp('cli-creds-outside-'); + try { + fs.symlinkSync(outside, path.join(runDir, '.cli-home')); + const cp = controlPlane([codexFile]); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + expect(fs.readdirSync(outside)).toEqual([]); + expect(fs.lstatSync(path.join(runDir, '.cli-home')).isSymbolicLink()).toBe(false); + expect(fs.readFileSync(materialized.files[0]!.filePath, 'utf-8')).toBe(CODEX); + } finally { + fs.rmSync(runDir, { recursive: true, force: true }); + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + + it('removes the login dir and reports an unstored login when the run fails', async () => { + const home = tmp('cli-creds-home-'); + try { + const cp = controlPlane([codexFile], 400); + const spawnProcess = vi.fn((_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + fs.writeFileSync(path.join(options.env.CODEX_HOME!, 'auth.json'), '{"refreshed":true}'); + const child = new EventEmitter() as EventEmitter & { killed: boolean; kill: () => boolean }; + child.killed = false; + child.kill = () => true; + queueMicrotask(() => child.emit('exit', 1, null)); + return child; + }) as never; + const runner = createDefaultAssignmentRunner({ + log: vi.fn(), + error: vi.fn(), + exit: vi.fn() as never, + env: { AGENT_RELAY_HOME: home, AGENT_RELAY_WORKER_KEEP_RUN_DIR: '1' }, + spawnProcess, + now: () => new Date(), + cwd: () => home, + fetchImpl: cp.fetchImpl, + resolveRelayflowsCliEntrypoint: () => '/opt/relayflows/dist/cli.js', + }); + await expect( + runner({ + assignment: { runId: 'run_2' } as never, + payload: { + runId: 'run_2', + workspaceId: 'ws', + relayWorkspaceId: 'rw', + relaycastApiKey: 'rk', + relayfileUrl: '', + relayfileToken: '', + workflow: 'version: "1.0"\n', + fileType: 'yaml', + sourceFileType: 'yaml', + workflowFileName: 'workflow.yaml', + cliCredentials: credentials, + }, + worker, + signal: new AbortController().signal, + }) + ).rejects.toThrow(/exited with code 1.*codex login could not be stored/); + expect(fs.existsSync(path.join(home, 'cloud-workers', 'runs', 'run_2', '.cli-home'))).toBe(false); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); }); diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index daf901307a..cbc682ac77 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -57,7 +57,12 @@ const CLI_LAYOUTS: Record 0 && typeof candidate.path === 'string'; } +/** The listed credentials, or an error: a reference without its list cannot be matched safely. */ +function offeredCredentials(credentials: CliCredentialsPayload): Map { + const list: unknown = credentials.credentials; + if ( + !Array.isArray(list) || + !list.every( + (entry) => + entry !== null && + typeof entry === 'object' && + typeof (entry as Record).credentialId === 'string' && + typeof (entry as Record).cli === 'string' + ) + ) { + throw new Error('cliCredentials.credentials must list the CLI logins offered for this run.'); + } + return new Map(list.map((entry: { credentialId: string; cli: string }) => [entry.credentialId, entry.cli])); +} + +/** + * Creates `/.cli-home` fresh: anything already there (a directory or a + * symlink planted by the workflow archive) is removed first, so every path + * below it is created by the worker and no write follows a link. + */ +async function freshLoginHome(runDir: string): Promise { + const home = path.join(runDir, '.cli-home'); + await fsp.rm(home, { recursive: true, force: true }); + await fsp.mkdir(home, { mode: 0o700 }); + await fsp.chmod(home, 0o700).catch(() => undefined); + const stat = await fsp.lstat(home); + if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error('Could not create the CLI login directory'); + return home; +} + /** * Builds a credential URL under the worker's API base, keeping any base path * prefix (e.g. `/cloud`). Only the exact credential route is accepted, so the @@ -133,21 +171,27 @@ export async function materializeCliCredentials(input: { method: 'GET', headers: headers(input.worker, input.credentials.ref), signal: input.signal, + // A redirect would resend the worker token and reference elsewhere. + redirect: 'error', }); if (!response.ok) { throw new Error(`Could not load the run's CLI logins (HTTP ${response.status}).`); } - const body = (await response.json()) as { files?: CredentialFile[]; skipped?: Array<{ reason?: string }> }; - const expected = new Map( - (input.credentials.credentials ?? []).map((entry) => [entry.credentialId, entry.cli]) - ); - const home = path.join(input.runDir, '.cli-home'); - await fsp.mkdir(home, { recursive: true, mode: 0o700 }); - await fsp.chmod(home, 0o700).catch(() => undefined); + const body = (await response.json().catch(() => null)) as { + files?: unknown; + skipped?: unknown; + } | null; + const expected = offeredCredentials(input.credentials); + const home = await freshLoginHome(input.runDir); const env: Record = {}; const files: MaterializedCliCredentials['files'] = []; - for (const entry of body.files ?? []) { - const layout = CLI_LAYOUTS[entry.cli]; + const entries: Array | null> = Array.isArray(body?.files) ? body.files : []; + for (const entry of entries) { + if (!entry || typeof entry !== 'object') { + input.log?.('[cloud-worker] skipping a malformed CLI login entry'); + continue; + } + const layout = typeof entry.cli === 'string' ? CLI_LAYOUTS[entry.cli] : undefined; const valid = layout !== undefined && typeof entry.credentialId === 'string' && @@ -165,17 +209,23 @@ export async function materializeCliCredentials(input: { continue; } const directory = path.join(home, layout.directory); - await fsp.mkdir(directory, { recursive: true, mode: 0o700 }); + await fsp.mkdir(directory, { mode: 0o700 }); await fsp.chmod(directory, 0o700).catch(() => undefined); const filePath = path.join(directory, layout.file); - await fsp.writeFile(filePath, entry.content, { encoding: 'utf-8', mode: 0o600 }); + // 'wx' = O_CREAT | O_EXCL: fails on anything already at the path, a symlink included. + await fsp.writeFile(filePath, entry.content as string, { encoding: 'utf-8', mode: 0o600, flag: 'wx' }); await fsp.chmod(filePath, 0o600).catch(() => undefined); env[layout.homeEnv] = directory; - files.push({ credentialId: entry.credentialId, cli: entry.cli, filePath, original: entry.content }); + files.push({ + credentialId: entry.credentialId as string, + cli: entry.cli as string, + filePath, + original: entry.content as string, + }); } if (files.length) env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); - if (body.skipped?.length) { + if (Array.isArray(body?.skipped) && body.skipped.length) { input.log?.(`[cloud-worker] ${body.skipped.length} CLI login(s) unavailable for this run`); } return { env, home, files }; @@ -233,6 +283,7 @@ export async function writeBackCliCredentials(input: { headers: headers(input.worker, input.credentials.ref, true), body: JSON.stringify({ content: current }), signal: AbortSignal.timeout(WRITE_BACK_TIMEOUT_MS), + redirect: 'error', } ); if (response.ok) stored = true; From a25c23a46d52d1d5bf029d355a543a79add67895 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:08:21 -0800 Subject: [PATCH 06/13] style: prettier Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/cloud-worker.ts | 2 +- .../cli/src/cli/lib/cloud-worker-cli-credentials.test.ts | 7 ++++++- packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts | 3 ++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/cli/commands/cloud-worker.ts b/packages/cli/src/cli/commands/cloud-worker.ts index d6072a4b88..b36b9722a4 100644 --- a/packages/cli/src/cli/commands/cloud-worker.ts +++ b/packages/cli/src/cli/commands/cloud-worker.ts @@ -286,7 +286,7 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex if (cliCredentials) { const loginHome = materialized?.home ?? path.join(runDir, '.cli-home'); await fsp.rm(loginHome, { recursive: true, force: true }).catch(() => { - deps.error('[cloud-worker] could not remove the run\'s CLI login directory; delete it manually'); + deps.error("[cloud-worker] could not remove the run's CLI login directory; delete it manually"); }); } if (!keepRunDir) { diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index a72cc69132..86e567f793 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -348,7 +348,12 @@ describe('cloud worker CLI logins', () => { try { fs.symlinkSync(outside, path.join(runDir, '.cli-home')); const cp = controlPlane([codexFile]); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + }); expect(fs.readdirSync(outside)).toEqual([]); expect(fs.lstatSync(path.join(runDir, '.cli-home')).isSymbolicLink()).toBe(false); expect(fs.readFileSync(materialized.files[0]!.filePath, 'utf-8')).toBe(CODEX); diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index cbc682ac77..f000ab8dd4 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -112,7 +112,8 @@ async function freshLoginHome(runDir: string): Promise { await fsp.mkdir(home, { mode: 0o700 }); await fsp.chmod(home, 0o700).catch(() => undefined); const stat = await fsp.lstat(home); - if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error('Could not create the CLI login directory'); + if (!stat.isDirectory() || stat.isSymbolicLink()) + throw new Error('Could not create the CLI login directory'); return home; } From 80230db76e90b14ae66947872d0278f6e4c21de1 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:24:29 -0800 Subject: [PATCH 07/13] fix(cloud-worker): third review pass on subscription logins Fail the run on an unreadable or invalid login response; check the credential list before the request; keep logins in a sibling of the run dir so no archive entry collides; give every CLI an empty run-scoped home and always set AGENT_RELAY_SUBSCRIPTION_LOGINS so nothing falls back to host logins; own-property layout lookup; restore permissions before retrying removal; lead the error with the login warning when it would be truncated. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/cloud-worker.ts | 12 +++- .../lib/cloud-worker-cli-credentials.test.ts | 48 +++++++++---- .../cli/lib/cloud-worker-cli-credentials.ts | 68 +++++++++++++++---- 3 files changed, 99 insertions(+), 29 deletions(-) diff --git a/packages/cli/src/cli/commands/cloud-worker.ts b/packages/cli/src/cli/commands/cloud-worker.ts index b36b9722a4..7fde393e8b 100644 --- a/packages/cli/src/cli/commands/cloud-worker.ts +++ b/packages/cli/src/cli/commands/cloud-worker.ts @@ -22,7 +22,9 @@ import { import { defaultExit } from '../lib/exit.js'; import { isCliCredentialsPayload, + cliLoginHome, materializeCliCredentials, + removeLoginHome, writeBackCliCredentials, type MaterializedCliCredentials, } from '../lib/cloud-worker-cli-credentials.js'; @@ -276,7 +278,11 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex // its single-use refresh token), and says so when it could not. const failedWriteBacks = await storeRefreshedLogins(); if (failedWriteBacks.length && error instanceof Error) { - error.message = `${error.message} The refreshed ${failedWriteBacks.join(', ')} login could not be stored; sign in again if the next run fails.`; + const warning = `The refreshed ${failedWriteBacks.join(', ')} login could not be stored; sign in again if the next run fails.`; + // Assignment reports keep the first 500 characters: lead with the + // warning when the original error would push it out. + error.message = + error.message.length + warning.length + 1 > 500 ? `${warning} ${error.message}` : `${error.message} ${warning}`; } throw error; } finally { @@ -284,8 +290,8 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex // The login files are removed even when the run directory is kept for // debugging, and even when materialisation failed partway. if (cliCredentials) { - const loginHome = materialized?.home ?? path.join(runDir, '.cli-home'); - await fsp.rm(loginHome, { recursive: true, force: true }).catch(() => { + const loginHome = materialized?.home ?? cliLoginHome(runDir); + await removeLoginHome(loginHome).catch(() => { deps.error("[cloud-worker] could not remove the run's CLI login directory; delete it manually"); }); } diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index 86e567f793..a0d4d82e52 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -7,6 +7,7 @@ import { describe, expect, it, vi } from 'vitest'; import { createDefaultAssignmentRunner } from '../commands/cloud-worker.js'; import { + cliLoginHome, materializeCliCredentials, SUBSCRIPTION_LOGINS_ENV, writeBackCliCredentials, @@ -101,19 +102,19 @@ describe('cloud worker CLI logins', () => { ); expect(cp.calls[0]!.headers.get('authorization')).toBe('Bearer sh_wrk_secret'); expect(cp.calls[0]!.headers.get('x-cli-credential-ref')).toBe('sh_clicred_ref'); - expect(materialized.home).toBe(path.join(runDir, '.cli-home')); + expect(materialized.home).toBe(cliLoginHome(runDir)); expect(materialized.env).toEqual({ - CODEX_HOME: path.join(runDir, '.cli-home', '.codex'), - GROK_HOME: path.join(runDir, '.cli-home', '.grok'), + CODEX_HOME: path.join(cliLoginHome(runDir), '.codex'), + GROK_HOME: path.join(cliLoginHome(runDir), '.grok'), [SUBSCRIPTION_LOGINS_ENV]: 'codex,grok', }); - const codexPath = path.join(runDir, '.cli-home', '.codex', 'auth.json'); + const codexPath = path.join(cliLoginHome(runDir), '.codex', 'auth.json'); expect(fs.readFileSync(codexPath, 'utf-8')).toBe(CODEX); if (posix) { expect(fs.statSync(codexPath).mode & 0o777).toBe(0o600); - expect(fs.statSync(path.join(runDir, '.cli-home', '.grok', 'auth.json')).mode & 0o777).toBe(0o600); + expect(fs.statSync(path.join(cliLoginHome(runDir), '.grok', 'auth.json')).mode & 0o777).toBe(0o600); expect(fs.statSync(path.dirname(codexPath)).mode & 0o777).toBe(0o700); - expect(fs.statSync(path.join(runDir, '.cli-home')).mode & 0o777).toBe(0o700); + expect(fs.statSync(cliLoginHome(runDir)).mode & 0o777).toBe(0o700); } expect(JSON.stringify(log.mock.calls)).not.toContain('rt-secret'); } finally { @@ -139,8 +140,13 @@ describe('cloud worker CLI logins', () => { fetchImpl: cp.fetchImpl, }); expect(materialized.files).toEqual([]); - expect(materialized.env).toEqual({}); - expect(fs.existsSync(path.join(runDir, '.cli-home', '.grok'))).toBe(false); + // No login mounted: each CLI home is an empty run-scoped dir, never the host's. + expect(materialized.env).toEqual({ + CODEX_HOME: path.join(cliLoginHome(runDir), '.codex'), + GROK_HOME: path.join(cliLoginHome(runDir), '.grok'), + [SUBSCRIPTION_LOGINS_ENV]: '', + }); + expect(fs.readdirSync(path.join(cliLoginHome(runDir), '.grok'))).toEqual([]); expect(fs.existsSync(path.join(runDir, 'etc'))).toBe(false); } finally { fs.rmSync(runDir, { recursive: true, force: true }); @@ -301,7 +307,7 @@ describe('cloud worker CLI logins', () => { signal: new AbortController().signal, }); const runDir = path.join(home, 'cloud-workers', 'runs', 'run_1'); - expect(seenEnv.CODEX_HOME).toBe(path.join(runDir, '.cli-home', '.codex')); + expect(seenEnv.CODEX_HOME).toBe(path.join(cliLoginHome(runDir), '.codex')); expect(seenEnv[SUBSCRIPTION_LOGINS_ENV]).toBe('codex'); expect(fileDuringRun).toBe(CODEX); const puts = cp.calls.filter((call) => call.method === 'PUT'); @@ -309,7 +315,7 @@ describe('cloud worker CLI logins', () => { expect(JSON.parse(puts[0]!.body!).content).toContain('rt-secret-3'); expect(result.summary).toBe('Workflow run_1 completed.'); expect(fs.existsSync(runDir)).toBe(true); - expect(fs.existsSync(path.join(runDir, '.cli-home'))).toBe(false); + expect(fs.existsSync(cliLoginHome(runDir))).toBe(false); } finally { fs.rmSync(home, { recursive: true, force: true }); } @@ -335,7 +341,20 @@ describe('cloud worker CLI logins', () => { fetchImpl: malformed.fetchImpl, }); expect(materialized.files.map((file) => file.credentialId)).toEqual(['c-codex']); - expect(malformed.calls[0]!.url).toContain('/cli-credentials'); + expect(materialized.env[SUBSCRIPTION_LOGINS_ENV]).toBe('codex'); + expect(materialized.env.GROK_HOME).toBe(path.join(cliLoginHome(runDir), '.grok')); + // The list is checked before any request; an unreadable response fails the run. + expect(cp.calls).toHaveLength(0); + const html = vi.fn(async () => new Response('oops', { status: 200 })) as unknown as typeof fetch; + await expect(materializeCliCredentials({ worker, credentials, runDir, fetchImpl: html })).rejects.toThrow( + /unreadable/ + ); + const shapeless = vi.fn(async () => new Response('{"files":"nope"}', { status: 200 })) as unknown as typeof fetch; + await expect(materializeCliCredentials({ worker, credentials, runDir, fetchImpl: shapeless })).rejects.toThrow( + /invalid/ + ); + const proto = controlPlane([{ ...codexFile, cli: '__proto__' }, { ...codexFile, cli: 'toString' }]); + expect((await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: proto.fetchImpl })).files).toEqual([]); } finally { fs.rmSync(runDir, { recursive: true, force: true }); } @@ -346,7 +365,7 @@ describe('cloud worker CLI logins', () => { const runDir = tmp('cli-creds-'); const outside = tmp('cli-creds-outside-'); try { - fs.symlinkSync(outside, path.join(runDir, '.cli-home')); + fs.symlinkSync(outside, cliLoginHome(runDir)); const cp = controlPlane([codexFile]); const materialized = await materializeCliCredentials({ worker, @@ -355,9 +374,10 @@ describe('cloud worker CLI logins', () => { fetchImpl: cp.fetchImpl, }); expect(fs.readdirSync(outside)).toEqual([]); - expect(fs.lstatSync(path.join(runDir, '.cli-home')).isSymbolicLink()).toBe(false); + expect(fs.lstatSync(cliLoginHome(runDir)).isSymbolicLink()).toBe(false); expect(fs.readFileSync(materialized.files[0]!.filePath, 'utf-8')).toBe(CODEX); } finally { + fs.rmSync(cliLoginHome(runDir), { recursive: true, force: true }); fs.rmSync(runDir, { recursive: true, force: true }); fs.rmSync(outside, { recursive: true, force: true }); } @@ -406,7 +426,7 @@ describe('cloud worker CLI logins', () => { signal: new AbortController().signal, }) ).rejects.toThrow(/exited with code 1.*codex login could not be stored/); - expect(fs.existsSync(path.join(home, 'cloud-workers', 'runs', 'run_2', '.cli-home'))).toBe(false); + expect(fs.existsSync(cliLoginHome(path.join(home, 'cloud-workers', 'runs', 'run_2')))).toBe(false); } finally { fs.rmSync(home, { recursive: true, force: true }); } diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index f000ab8dd4..513c90d04f 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -106,8 +106,13 @@ function offeredCredentials(credentials: CliCredentialsPayload): Map { - const home = path.join(runDir, '.cli-home'); + const home = cliLoginHome(runDir); await fsp.rm(home, { recursive: true, force: true }); await fsp.mkdir(home, { mode: 0o700 }); await fsp.chmod(home, 0o700).catch(() => undefined); @@ -148,7 +153,7 @@ function headers(worker: CloudWorkerRecord, ref: string, json = false): Headers /** * Fetches the run's subscription logins and writes each auth file 0600 into a - * 0700 directory under `/.cli-home`. + * 0700 directory in the run's login dir (a sibling of `runDir`). * * A file is mounted only when it matches a credential the assignment payload * listed, with that credential's CLI and the CLI's own layout; anything else is @@ -168,6 +173,8 @@ export async function materializeCliCredentials(input: { signal?: AbortSignal; log?: (...args: unknown[]) => void; }): Promise { + // Validate the payload before any request, so a bad list is reported as such. + const expected = offeredCredentials(input.credentials); const response = await input.fetchImpl(credentialUrl(input.worker, input.credentials.path), { method: 'GET', headers: headers(input.worker, input.credentials.ref), @@ -178,21 +185,28 @@ export async function materializeCliCredentials(input: { if (!response.ok) { throw new Error(`Could not load the run's CLI logins (HTTP ${response.status}).`); } - const body = (await response.json().catch(() => null)) as { - files?: unknown; - skipped?: unknown; - } | null; - const expected = offeredCredentials(input.credentials); + // An unreadable response fails the run: starting without the offered login + // could silently fall back to inherited credentials. + let body: { files?: unknown; skipped?: unknown }; + try { + body = (await response.json()) as { files?: unknown; skipped?: unknown }; + } catch { + throw new Error("The control plane returned an unreadable CLI login response."); + } + if (!body || typeof body !== 'object' || !Array.isArray(body.files)) { + throw new Error('The control plane returned an invalid CLI login response.'); + } const home = await freshLoginHome(input.runDir); const env: Record = {}; const files: MaterializedCliCredentials['files'] = []; - const entries: Array | null> = Array.isArray(body?.files) ? body.files : []; + const entries: Array | null> = body.files; for (const entry of entries) { if (!entry || typeof entry !== 'object') { input.log?.('[cloud-worker] skipping a malformed CLI login entry'); continue; } - const layout = typeof entry.cli === 'string' ? CLI_LAYOUTS[entry.cli] : undefined; + const layout = + typeof entry.cli === 'string' && Object.hasOwn(CLI_LAYOUTS, entry.cli) ? CLI_LAYOUTS[entry.cli] : undefined; const valid = layout !== undefined && typeof entry.credentialId === 'string' && @@ -224,9 +238,17 @@ export async function materializeCliCredentials(input: { original: entry.content as string, }); } - if (files.length) - env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); - if (Array.isArray(body?.skipped) && body.skipped.length) { + // Every CLI home points into the run's login dir, empty when nothing mounted, + // so a CLI never falls back to a login on the worker host (or its ~/.codex). + for (const layout of Object.values(CLI_LAYOUTS)) { + if (layout.homeEnv in env) continue; + const directory = path.join(home, layout.directory); + await fsp.mkdir(directory, { mode: 0o700 }); + env[layout.homeEnv] = directory; + } + // Always set, so an inherited value never claims a login this run lacks. + env[SUBSCRIPTION_LOGINS_ENV] = [...new Set(files.map((file) => file.cli))].sort().join(','); + if (Array.isArray(body.skipped) && body.skipped.length) { input.log?.(`[cloud-worker] ${body.skipped.length} CLI login(s) unavailable for this run`); } return { env, home, files }; @@ -308,3 +330,25 @@ export async function writeBackCliCredentials(input: { } return result; } + +/** Makes a login tree removable again (a workflow may have chmod-ed it) and removes it. */ +export async function removeLoginHome(home: string): Promise { + try { + await fsp.rm(home, { recursive: true, force: true }); + return; + } catch { + // Restore owner permissions and retry once. + } + const restore = async (target: string): Promise => { + const stat = await fsp.lstat(target).catch(() => null); + if (!stat || stat.isSymbolicLink()) return; + await fsp.chmod(target, stat.isDirectory() ? 0o700 : 0o600).catch(() => undefined); + if (stat.isDirectory()) { + for (const name of await fsp.readdir(target).catch(() => [] as string[])) { + await restore(path.join(target, name)); + } + } + }; + await restore(home); + await fsp.rm(home, { recursive: true, force: true }); +} From dbd7d18a9bb135fc638927c204e690b2c95537cb Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:24:53 -0800 Subject: [PATCH 08/13] style: prettier Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/cloud-worker.ts | 4 ++- .../lib/cloud-worker-cli-credentials.test.ts | 29 ++++++++++++------- .../cli/lib/cloud-worker-cli-credentials.ts | 6 ++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/cli/commands/cloud-worker.ts b/packages/cli/src/cli/commands/cloud-worker.ts index 7fde393e8b..ca3506ea8b 100644 --- a/packages/cli/src/cli/commands/cloud-worker.ts +++ b/packages/cli/src/cli/commands/cloud-worker.ts @@ -282,7 +282,9 @@ export function createDefaultAssignmentRunner(deps: CloudWorkerDependencies): Ex // Assignment reports keep the first 500 characters: lead with the // warning when the original error would push it out. error.message = - error.message.length + warning.length + 1 > 500 ? `${warning} ${error.message}` : `${error.message} ${warning}`; + error.message.length + warning.length + 1 > 500 + ? `${warning} ${error.message}` + : `${error.message} ${warning}`; } throw error; } finally { diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index a0d4d82e52..530d1b8d51 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -345,16 +345,25 @@ describe('cloud worker CLI logins', () => { expect(materialized.env.GROK_HOME).toBe(path.join(cliLoginHome(runDir), '.grok')); // The list is checked before any request; an unreadable response fails the run. expect(cp.calls).toHaveLength(0); - const html = vi.fn(async () => new Response('oops', { status: 200 })) as unknown as typeof fetch; - await expect(materializeCliCredentials({ worker, credentials, runDir, fetchImpl: html })).rejects.toThrow( - /unreadable/ - ); - const shapeless = vi.fn(async () => new Response('{"files":"nope"}', { status: 200 })) as unknown as typeof fetch; - await expect(materializeCliCredentials({ worker, credentials, runDir, fetchImpl: shapeless })).rejects.toThrow( - /invalid/ - ); - const proto = controlPlane([{ ...codexFile, cli: '__proto__' }, { ...codexFile, cli: 'toString' }]); - expect((await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: proto.fetchImpl })).files).toEqual([]); + const html = vi.fn( + async () => new Response('oops', { status: 200 }) + ) as unknown as typeof fetch; + await expect( + materializeCliCredentials({ worker, credentials, runDir, fetchImpl: html }) + ).rejects.toThrow(/unreadable/); + const shapeless = vi.fn( + async () => new Response('{"files":"nope"}', { status: 200 }) + ) as unknown as typeof fetch; + await expect( + materializeCliCredentials({ worker, credentials, runDir, fetchImpl: shapeless }) + ).rejects.toThrow(/invalid/); + const proto = controlPlane([ + { ...codexFile, cli: '__proto__' }, + { ...codexFile, cli: 'toString' }, + ]); + expect( + (await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: proto.fetchImpl })).files + ).toEqual([]); } finally { fs.rmSync(runDir, { recursive: true, force: true }); } diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index 513c90d04f..482680fc68 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -191,7 +191,7 @@ export async function materializeCliCredentials(input: { try { body = (await response.json()) as { files?: unknown; skipped?: unknown }; } catch { - throw new Error("The control plane returned an unreadable CLI login response."); + throw new Error('The control plane returned an unreadable CLI login response.'); } if (!body || typeof body !== 'object' || !Array.isArray(body.files)) { throw new Error('The control plane returned an invalid CLI login response.'); @@ -206,7 +206,9 @@ export async function materializeCliCredentials(input: { continue; } const layout = - typeof entry.cli === 'string' && Object.hasOwn(CLI_LAYOUTS, entry.cli) ? CLI_LAYOUTS[entry.cli] : undefined; + typeof entry.cli === 'string' && Object.hasOwn(CLI_LAYOUTS, entry.cli) + ? CLI_LAYOUTS[entry.cli] + : undefined; const valid = layout !== undefined && typeof entry.credentialId === 'string' && From 432461ff9f246e377632805e7044a18247abfc59 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:45:50 -0800 Subject: [PATCH 09/13] fix(cloud-worker): clean test login dirs; no-follow permission restore; doc placement Co-Authored-By: Claude Opus 5.5 --- .../lib/cloud-worker-cli-credentials.test.ts | 14 ++++++- .../cli/lib/cloud-worker-cli-credentials.ts | 39 +++++++++++++------ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index 530d1b8d51..01d3d0046f 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -3,7 +3,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { createDefaultAssignmentRunner } from '../commands/cloud-worker.js'; import { @@ -80,9 +80,19 @@ function controlPlane(files: unknown[], putStatus = 200) { return { calls, fetchImpl: fetchImpl as unknown as typeof fetch }; } +// Each run dir lives in its own temp parent, so the sibling login dir is +// removed with it after every test. +const tempParents: string[] = []; function tmp(prefix: string) { - return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const parent = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempParents.push(parent); + const runDir = path.join(parent, 'run'); + fs.mkdirSync(runDir, { mode: 0o700 }); + return runDir; } +afterEach(() => { + for (const parent of tempParents.splice(0)) fs.rmSync(parent, { recursive: true, force: true }); +}); describe('cloud worker CLI logins', () => { it('writes each auth file 0600 in a 0700 dir inside the run dir and points the CLI at it', async () => { diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index 482680fc68..d4918f0d33 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -102,15 +102,21 @@ function offeredCredentials(credentials: CliCredentialsPayload): Map/.cli-home` fresh: anything already there (a directory or a - * symlink planted by the workflow archive) is removed first, so every path - * below it is created by the worker and no write follows a link. + * The run's login directory: a sibling of the run dir (`..cli-home`), + * so no code-archive entry can collide with it. Pure path helper. + * + * @param runDir - The run's private directory. + * @returns The login directory path. */ export function cliLoginHome(runDir: string): string { - // A sibling of the run dir, so no code-archive entry can collide with it. return path.join(path.dirname(runDir), `.${path.basename(runDir)}.cli-home`); } +/** + * Creates the login directory fresh: anything already there (a directory or a + * symlink) is removed first, so every path below it is created by the worker + * and no write follows a link. + */ async function freshLoginHome(runDir: string): Promise { const home = cliLoginHome(runDir); await fsp.rm(home, { recursive: true, force: true }); @@ -341,14 +347,25 @@ export async function removeLoginHome(home: string): Promise { } catch { // Restore owner permissions and retry once. } + // Only directories need restoring (removing a file needs its parent's + // permissions). Each is opened with O_NOFOLLOW | O_DIRECTORY and chmod-ed + // through that handle, so a symlink swapped in is never followed. + const noFollowDir = (fs.constants.O_NOFOLLOW ?? 0) | (fs.constants.O_DIRECTORY ?? 0); const restore = async (target: string): Promise => { - const stat = await fsp.lstat(target).catch(() => null); - if (!stat || stat.isSymbolicLink()) return; - await fsp.chmod(target, stat.isDirectory() ? 0o700 : 0o600).catch(() => undefined); - if (stat.isDirectory()) { - for (const name of await fsp.readdir(target).catch(() => [] as string[])) { - await restore(path.join(target, name)); - } + let handle: fsp.FileHandle; + try { + handle = await fsp.open(target, fs.constants.O_RDONLY | noFollowDir); + } catch { + return; + } + try { + if (!(await handle.stat()).isDirectory()) return; + await handle.chmod(0o700).catch(() => undefined); + } finally { + await handle.close(); + } + for (const entry of await fsp.readdir(target, { withFileTypes: true }).catch(() => [])) { + if (entry.isDirectory()) await restore(path.join(target, entry.name)); } }; await restore(home); From bc0f528d06a7e2a291fc51577b535efdf6eac1cf Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:58:47 -0800 Subject: [PATCH 10/13] fix(cloud-worker): login dirs under cli-logins/, restore unreadable dirs before removal Co-Authored-By: Claude Opus 5.5 --- .../lib/cloud-worker-cli-credentials.test.ts | 18 ++++++++-- .../cli/lib/cloud-worker-cli-credentials.ts | 34 ++++++++++++------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index 01d3d0046f..78a110ae9a 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -9,6 +9,7 @@ import { createDefaultAssignmentRunner } from '../commands/cloud-worker.js'; import { cliLoginHome, materializeCliCredentials, + removeLoginHome, SUBSCRIPTION_LOGINS_ENV, writeBackCliCredentials, } from './cloud-worker-cli-credentials.js'; @@ -86,8 +87,9 @@ const tempParents: string[] = []; function tmp(prefix: string) { const parent = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); tempParents.push(parent); - const runDir = path.join(parent, 'run'); - fs.mkdirSync(runDir, { mode: 0o700 }); + // /runs/run, so the login dir (/cli-logins/run) stays inside the parent. + const runDir = path.join(parent, 'runs', 'run'); + fs.mkdirSync(runDir, { recursive: true, mode: 0o700 }); return runDir; } afterEach(() => { @@ -450,4 +452,16 @@ describe('cloud worker CLI logins', () => { fs.rmSync(home, { recursive: true, force: true }); } }); + + it('removes a login dir the workflow made unreadable', async () => { + if (!posix || process.getuid?.() === 0) return; + const runDir = tmp('cli-creds-'); + const cp = controlPlane([codexFile]); + const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + fs.chmodSync(path.join(materialized.home, '.codex'), 0o000); + await removeLoginHome(materialized.home); + expect(fs.existsSync(materialized.home)).toBe(false); + expect(materialized.home).toBe(path.join(path.dirname(path.dirname(runDir)), 'cli-logins', 'run')); + }); }); + diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index d4918f0d33..559950d7c7 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -102,14 +102,15 @@ function offeredCredentials(credentials: CliCredentialsPayload): Map.cli-home`), - * so no code-archive entry can collide with it. Pure path helper. + * The run's login directory: `/cli-logins/`, outside `runs/`, + * so neither a code-archive entry nor another run's directory can collide + * with it. Pure path helper. * * @param runDir - The run's private directory. * @returns The login directory path. */ export function cliLoginHome(runDir: string): string { - return path.join(path.dirname(runDir), `.${path.basename(runDir)}.cli-home`); + return path.join(path.dirname(path.dirname(runDir)), 'cli-logins', path.basename(runDir)); } /** @@ -119,6 +120,7 @@ export function cliLoginHome(runDir: string): string { */ async function freshLoginHome(runDir: string): Promise { const home = cliLoginHome(runDir); + await fsp.mkdir(path.dirname(home), { recursive: true, mode: 0o700 }); await fsp.rm(home, { recursive: true, force: true }); await fsp.mkdir(home, { mode: 0o700 }); await fsp.chmod(home, 0o700).catch(() => undefined); @@ -348,21 +350,29 @@ export async function removeLoginHome(home: string): Promise { // Restore owner permissions and retry once. } // Only directories need restoring (removing a file needs its parent's - // permissions). Each is opened with O_NOFOLLOW | O_DIRECTORY and chmod-ed - // through that handle, so a symlink swapped in is never followed. + // permissions). A readable directory is opened with O_NOFOLLOW | O_DIRECTORY + // and chmod-ed through that handle; one the workflow made unreadable (mode + // 000) cannot be opened, so after lstat confirms a real directory it is + // chmod-ed by path. Node has no descriptor-relative traversal, so the walk + // itself is by path; racing it needs a live same-user process. const noFollowDir = (fs.constants.O_NOFOLLOW ?? 0) | (fs.constants.O_DIRECTORY ?? 0); const restore = async (target: string): Promise => { - let handle: fsp.FileHandle; + const stat = await fsp.lstat(target).catch(() => null); + if (!stat || stat.isSymbolicLink() || !stat.isDirectory()) return; + let handle: fsp.FileHandle | null = null; try { handle = await fsp.open(target, fs.constants.O_RDONLY | noFollowDir); } catch { - return; + handle = null; } - try { - if (!(await handle.stat()).isDirectory()) return; - await handle.chmod(0o700).catch(() => undefined); - } finally { - await handle.close(); + if (handle) { + try { + if ((await handle.stat()).isDirectory()) await handle.chmod(0o700).catch(() => undefined); + } finally { + await handle.close(); + } + } else { + await fsp.chmod(target, 0o700).catch(() => undefined); } for (const entry of await fsp.readdir(target, { withFileTypes: true }).catch(() => [])) { if (entry.isDirectory()) await restore(path.join(target, entry.name)); From 7f7e9bf56c27482f41860e83519b737e1530fde5 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:59:04 -0800 Subject: [PATCH 11/13] test(cloud-worker): create the login parent before planting the symlink Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index 78a110ae9a..9656248323 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -386,6 +386,7 @@ describe('cloud worker CLI logins', () => { const runDir = tmp('cli-creds-'); const outside = tmp('cli-creds-outside-'); try { + fs.mkdirSync(path.dirname(cliLoginHome(runDir)), { recursive: true }); fs.symlinkSync(outside, cliLoginHome(runDir)); const cp = controlPlane([codexFile]); const materialized = await materializeCliCredentials({ From 4c9c625ac3bbbb749fb9921affb0644f3a426d71 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 21:59:20 -0800 Subject: [PATCH 12/13] style: prettier Co-Authored-By: Claude Opus 5.5 --- .../cli/src/cli/lib/cloud-worker-cli-credentials.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts index 9656248323..16a9564ced 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.test.ts @@ -458,11 +458,15 @@ describe('cloud worker CLI logins', () => { if (!posix || process.getuid?.() === 0) return; const runDir = tmp('cli-creds-'); const cp = controlPlane([codexFile]); - const materialized = await materializeCliCredentials({ worker, credentials, runDir, fetchImpl: cp.fetchImpl }); + const materialized = await materializeCliCredentials({ + worker, + credentials, + runDir, + fetchImpl: cp.fetchImpl, + }); fs.chmodSync(path.join(materialized.home, '.codex'), 0o000); await removeLoginHome(materialized.home); expect(fs.existsSync(materialized.home)).toBe(false); expect(materialized.home).toBe(path.join(path.dirname(path.dirname(runDir)), 'cli-logins', 'run')); }); }); - From 0bbcfa878d048a3c7717bbc1780e558b7af5102e Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 3 Nov 2026 22:11:29 -0800 Subject: [PATCH 13/13] fix(cloud-worker): restore login-dir permissions with chmod -R (no symlink follow, no check/use race) Co-Authored-By: Claude Opus 5.5 --- .../cli/lib/cloud-worker-cli-credentials.ts | 41 +++++-------------- 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts index 559950d7c7..7378e1efe5 100644 --- a/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts +++ b/packages/cli/src/cli/lib/cloud-worker-cli-credentials.ts @@ -14,6 +14,10 @@ import fs from 'node:fs'; import fsp from 'node:fs/promises'; import path from 'node:path'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; + +const execFileAsync = promisify(execFile); import type { CloudWorkerRecord } from '@agent-relay/cloud'; @@ -349,35 +353,12 @@ export async function removeLoginHome(home: string): Promise { } catch { // Restore owner permissions and retry once. } - // Only directories need restoring (removing a file needs its parent's - // permissions). A readable directory is opened with O_NOFOLLOW | O_DIRECTORY - // and chmod-ed through that handle; one the workflow made unreadable (mode - // 000) cannot be opened, so after lstat confirms a real directory it is - // chmod-ed by path. Node has no descriptor-relative traversal, so the walk - // itself is by path; racing it needs a live same-user process. - const noFollowDir = (fs.constants.O_NOFOLLOW ?? 0) | (fs.constants.O_DIRECTORY ?? 0); - const restore = async (target: string): Promise => { - const stat = await fsp.lstat(target).catch(() => null); - if (!stat || stat.isSymbolicLink() || !stat.isDirectory()) return; - let handle: fsp.FileHandle | null = null; - try { - handle = await fsp.open(target, fs.constants.O_RDONLY | noFollowDir); - } catch { - handle = null; - } - if (handle) { - try { - if ((await handle.stat()).isDirectory()) await handle.chmod(0o700).catch(() => undefined); - } finally { - await handle.close(); - } - } else { - await fsp.chmod(target, 0o700).catch(() => undefined); - } - for (const entry of await fsp.readdir(target, { withFileTypes: true }).catch(() => [])) { - if (entry.isDirectory()) await restore(path.join(target, entry.name)); - } - }; - await restore(home); + // Restore owner permissions with chmod(1) rather than a check-then-chmod in + // this process: `chmod -R` (default -P) never follows symlinks, applies the + // mode to a directory before descending, so a mode-000 directory made by + // the workflow is repaired too, and leaves no check/use race in this code. + if (process.platform !== 'win32') { + await execFileAsync('chmod', ['-R', 'u+rwX', '--', home]).catch(() => undefined); + } await fsp.rm(home, { recursive: true, force: true }); }