From 5af3b7aca9425c640d7360cbcac61047c57287c4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 10 Aug 2026 04:01:46 -0700 Subject: [PATCH] fix: close Claude adapter edge cases --- .../claude-child-process-environment.test.ts | 29 +++++++ .../claude-child-process-environment.ts | 32 ++++++-- .../claude-stream-json-connection.test.ts | 8 +- .../claude/claude-stream-json-connection.ts | 50 ++---------- src/main/claude/claude-stream-json-stdout.ts | 80 +++++++++++++++++++ .../claude/claude-structured-dispatch.test.ts | 20 +++++ src/main/claude/claude-structured-dispatch.ts | 38 +++++++-- .../claude-structured-inbound-control.test.ts | 46 +++++++++++ .../claude-structured-inbound-control.ts | 12 +-- 9 files changed, 252 insertions(+), 63 deletions(-) create mode 100644 src/main/claude/claude-child-process-environment.test.ts create mode 100644 src/main/claude/claude-stream-json-stdout.ts create mode 100644 src/main/claude/claude-structured-inbound-control.test.ts diff --git a/src/main/claude/claude-child-process-environment.test.ts b/src/main/claude/claude-child-process-environment.test.ts new file mode 100644 index 00000000000..9dc7007f07b --- /dev/null +++ b/src/main/claude/claude-child-process-environment.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest' +import { buildClaudeChildProcessEnv } from './claude-child-process-environment' + +describe('Claude child process environment', () => { + it('strips case-insensitive inherited auth and session stamps on Windows', () => { + const env = buildClaudeChildProcessEnv( + { + ANTHROPIC_AUTH_TOKEN: 'configured-token', + Claude_Code_Session_Id: 'configured-session' + }, + { + platform: 'win32', + inheritedEnv: { + anthropic_api_key: 'inherited-key', + Anthropic_Custom_Headers: 'Authorization: inherited', + claude_code_child_session: '1', + CLAUDE_CODE_SESSION_ID: 'inherited-session', + SAFE_VALUE: 'preserved' + } + } + ) + + expect(env).toEqual({ + ANTHROPIC_AUTH_TOKEN: 'configured-token', + Claude_Code_Session_Id: 'configured-session', + SAFE_VALUE: 'preserved' + }) + }) +}) diff --git a/src/main/claude/claude-child-process-environment.ts b/src/main/claude/claude-child-process-environment.ts index 003d85b5b6d..ad5e854d4ff 100644 --- a/src/main/claude/claude-child-process-environment.ts +++ b/src/main/claude/claude-child-process-environment.ts @@ -1,4 +1,4 @@ -import { applyClaudeEnvPatch } from '../claude-accounts/environment' +import { CLAUDE_AUTH_ENV_VARS, applyClaudeEnvPatch } from '../claude-accounts/environment' const CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS = [ 'CLAUDE_CODE_CHILD_SESSION', @@ -6,9 +6,9 @@ const CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS = [ 'CLAUDE_CODE_BRIDGE_SESSION_ID' ] as const -function cloneProcessEnv(): Record { +function cloneProcessEnv(source: NodeJS.ProcessEnv): Record { const env: Record = {} - for (const [key, value] of Object.entries(process.env)) { + for (const [key, value] of Object.entries(source)) { if (value !== undefined) { env[key] = value } @@ -17,11 +17,31 @@ function cloneProcessEnv(): Record { } export function buildClaudeChildProcessEnv( - configuredEnv: Record = {} + configuredEnv: Record = {}, + options: { inheritedEnv?: NodeJS.ProcessEnv; platform?: NodeJS.Platform } = {} ): Record { - const env = applyClaudeEnvPatch(cloneProcessEnv(), {}, { stripAuthEnv: true }) + const inheritedEnv = options.inheritedEnv ?? process.env + const platform = options.platform ?? process.platform + const env = applyClaudeEnvPatch(cloneProcessEnv(inheritedEnv), {}, { stripAuthEnv: true }) + if (platform === 'win32') { + const authKeys = new Set(CLAUDE_AUTH_ENV_VARS.map((key) => key.toUpperCase())) + for (const [key, value] of Object.entries(env)) { + const normalized = key.toUpperCase() + if ( + authKeys.has(normalized) || + (normalized === 'ANTHROPIC_CUSTOM_HEADERS' && + /authorization|x-api-key|api-key|bearer/i.test(value)) + ) { + delete env[key] + } + } + } for (const key of CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS) { - delete env[key] + for (const inheritedKey of Object.keys(env)) { + if (inheritedKey === key || (platform === 'win32' && inheritedKey.toUpperCase() === key)) { + delete env[inheritedKey] + } + } } return Object.assign(env, configuredEnv) } diff --git a/src/main/claude/claude-stream-json-connection.test.ts b/src/main/claude/claude-stream-json-connection.test.ts index e46e8154922..40d8ae149b8 100644 --- a/src/main/claude/claude-stream-json-connection.test.ts +++ b/src/main/claude/claude-stream-json-connection.test.ts @@ -119,15 +119,17 @@ describe('Claude stream-json connection', () => { it('fails closed on malformed provider output', async () => { const process = fakeSpawn() const onExit = vi.fn() + const onMessage = vi.fn() await openClaudeStreamJsonConnection( { command: 'claude', args: [], cwd: '/work' }, - { onExit }, + { onExit, onMessage }, process.spawnImpl ) - process.child.stdout.write('{not-json}\n') + process.child.stdout.write('{not-json}\n{"type":"assistant"}\n') await new Promise((resolve) => setImmediate(resolve)) - expect(process.child.kill).toHaveBeenCalled() + expect(process.child.kill).toHaveBeenCalledTimes(1) + expect(onMessage).not.toHaveBeenCalled() expect(onExit).toHaveBeenCalledWith(expect.objectContaining({ message: expect.any(String) })) }) diff --git a/src/main/claude/claude-stream-json-connection.ts b/src/main/claude/claude-stream-json-connection.ts index e84a39fc7e8..f086a22d831 100644 --- a/src/main/claude/claude-stream-json-connection.ts +++ b/src/main/claude/claude-stream-json-connection.ts @@ -2,6 +2,7 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' import { waitForProcessExitUntil } from '../codex/codex-process-exit-deadline' import { killCodexAppServerProcessTree } from '../codex/codex-app-server-session' import { buildClaudeChildProcessEnv } from './claude-child-process-environment' +import { attachClaudeStreamJsonStdout } from './claude-stream-json-stdout' export type ClaudeStreamJsonLaunch = { command: string @@ -64,17 +65,14 @@ type PendingRequest = { reject: (error: Error) => void timer: ReturnType } - function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value) } - function exitError(stderrTail: string, cause?: Error): Error { const detail = stderrTail.trim() const message = detail ? `claude stream-json exited: ${detail}` : 'claude stream-json exited' return cause ? new Error(message, { cause }) : new Error(message) } - export async function openClaudeStreamJsonConnection( launch: ClaudeStreamJsonLaunch, handlers: ClaudeStreamJsonConnectionHandlers = {}, @@ -89,7 +87,6 @@ export async function openClaudeStreamJsonConnection( const pending = new Map() let nextRequestId = 1 let stderrTail = '' - let stdoutBuffer = '' let exited = false let closing = false let terminalError: Error | null = null @@ -179,41 +176,13 @@ export async function openClaudeStreamJsonConnection( } handlers.onMessage?.(message) } - - const parseLine = (line: string): void => { - if (!line.trim()) { - return - } - let parsed: unknown - try { - parsed = JSON.parse(line) - } catch (error) { + const stdout = attachClaudeStreamJsonStdout({ + stdout: child.stdout, + maxLineBytes: STDOUT_LINE_MAX_BYTES, + onMessage: dispatchMessage, + onFailure: (error) => { killCodexAppServerProcessTree(child) - handleUnexpectedEnd(new Error('claude emitted invalid stream-json', { cause: error })) - return - } - if (isRecord(parsed)) { - dispatchMessage(parsed) - } - } - - child.stdout.setEncoding('utf8').on('data', (chunk: string) => { - stdoutBuffer += chunk - let newline = stdoutBuffer.indexOf('\n') - while (newline !== -1) { - const line = stdoutBuffer.slice(0, newline) - stdoutBuffer = stdoutBuffer.slice(newline + 1) - if (Buffer.byteLength(line, 'utf8') > STDOUT_LINE_MAX_BYTES) { - killCodexAppServerProcessTree(child) - handleUnexpectedEnd(new Error('claude emitted an oversized stream-json line')) - return - } - parseLine(line) - newline = stdoutBuffer.indexOf('\n') - } - if (Buffer.byteLength(stdoutBuffer, 'utf8') > STDOUT_LINE_MAX_BYTES) { - killCodexAppServerProcessTree(child) - handleUnexpectedEnd(new Error('claude emitted an oversized stream-json line')) + handleUnexpectedEnd(error) } }) child.stderr.setEncoding('utf8').on('data', (chunk: string) => { @@ -225,10 +194,7 @@ export async function openClaudeStreamJsonConnection( }) child.on('close', () => { markExited() - if (stdoutBuffer.trim()) { - parseLine(stdoutBuffer) - stdoutBuffer = '' - } + stdout.flush() handleUnexpectedEnd() }) child.stdin.on('error', (error) => { diff --git a/src/main/claude/claude-stream-json-stdout.ts b/src/main/claude/claude-stream-json-stdout.ts new file mode 100644 index 00000000000..1d15fa1bb28 --- /dev/null +++ b/src/main/claude/claude-stream-json-stdout.ts @@ -0,0 +1,80 @@ +import type { Readable } from 'node:stream' + +export type ClaudeStreamJsonStdout = { + flush: () => void +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +export function attachClaudeStreamJsonStdout(input: { + stdout: Readable + maxLineBytes: number + onMessage: (message: Record) => void + onFailure: (error: Error) => void +}): ClaudeStreamJsonStdout { + let buffer = '' + let stopped = false + + const stop = (): void => { + if (stopped) { + return + } + stopped = true + buffer = '' + input.stdout.removeListener('data', onData) + input.stdout.pause() + } + const fail = (error: Error): void => { + stop() + input.onFailure(error) + } + const parseLine = (line: string): void => { + if (!line.trim()) { + return + } + try { + const parsed: unknown = JSON.parse(line) + if (isRecord(parsed)) { + input.onMessage(parsed) + } + } catch (error) { + fail(new Error('claude emitted invalid stream-json', { cause: error })) + } + } + const onData = (chunk: string): void => { + if (stopped) { + return + } + buffer += chunk + let newline = buffer.indexOf('\n') + while (newline !== -1) { + const line = buffer.slice(0, newline) + buffer = buffer.slice(newline + 1) + if (Buffer.byteLength(line, 'utf8') > input.maxLineBytes) { + fail(new Error('claude emitted an oversized stream-json line')) + return + } + parseLine(line) + if (stopped) { + return + } + newline = buffer.indexOf('\n') + } + if (Buffer.byteLength(buffer, 'utf8') > input.maxLineBytes) { + fail(new Error('claude emitted an oversized stream-json line')) + } + } + + input.stdout.setEncoding('utf8').on('data', onData) + return { + flush: () => { + if (!stopped && buffer.trim()) { + const line = buffer + buffer = '' + parseLine(line) + } + } + } +} diff --git a/src/main/claude/claude-structured-dispatch.test.ts b/src/main/claude/claude-structured-dispatch.test.ts index 53fabdb7675..69a107e5d2e 100644 --- a/src/main/claude/claude-structured-dispatch.test.ts +++ b/src/main/claude/claude-structured-dispatch.test.ts @@ -64,4 +64,24 @@ describe('Claude structured dispatch image limits', () => { await rm(directory, { recursive: true, force: true }) } }) + + it('rejects a local image by actual bytes read beyond the per-image cap', async () => { + const directory = await mkdtemp(join(tmpdir(), 'orca-claude-image-')) + try { + const path = join(directory, 'oversized.png') + await writeFile(path, Buffer.alloc(5 * 1024 * 1024 + 1)) + const session = sessionFor() + const body = userMessage([{ type: 'image-ref', path }]) + + await expect( + dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }, 1) + ).resolves.toEqual({ + state: 'rejected', + reason: `Claude image must be a non-empty file no larger than ${5 * 1024 * 1024} bytes` + }) + expect(session.connection.send).not.toHaveBeenCalled() + } finally { + await rm(directory, { recursive: true, force: true }) + } + }) }) diff --git a/src/main/claude/claude-structured-dispatch.ts b/src/main/claude/claude-structured-dispatch.ts index 21293e6d14c..da95e373f82 100644 --- a/src/main/claude/claude-structured-dispatch.ts +++ b/src/main/claude/claude-structured-dispatch.ts @@ -1,5 +1,5 @@ import { extname } from 'node:path' -import { readFile, stat } from 'node:fs/promises' +import { open } from 'node:fs/promises' import type { AgentJournalMessageItem } from '../../shared/agent-session-journal-types' import type { NativeChatBlock } from '../../shared/native-chat-types' import type { AgentSessionDispatchOutcome } from '../native-chat/agent-session-wire/structured-agent-session-adapter' @@ -15,6 +15,33 @@ type ImageBudget = { localBytes: number } +async function readClaudeImage(path: string): Promise { + const file = await open(path, 'r') + try { + const info = await file.stat() + if (!info.isFile()) { + throw new Error('Claude image must be a file') + } + const buffer = Buffer.allocUnsafe(MAX_IMAGE_BYTES + 1) + let bytesRead = 0 + while (bytesRead < buffer.length) { + const result = await file.read(buffer, bytesRead, buffer.length - bytesRead, bytesRead) + if (result.bytesRead === 0) { + break + } + bytesRead += result.bytesRead + } + if (bytesRead === 0 || bytesRead > MAX_IMAGE_BYTES) { + throw new Error( + `Claude image must be a non-empty file no larger than ${MAX_IMAGE_BYTES} bytes` + ) + } + return buffer.subarray(0, bytesRead) + } finally { + await file.close() + } +} + const IMAGE_MIME_BY_EXTENSION: Record = { '.gif': 'image/gif', '.jpeg': 'image/jpeg', @@ -56,11 +83,8 @@ async function imageContent( if (!block.path) { throw new Error('image reference has neither a path nor a URL') } - const info = await stat(block.path) - if (!info.isFile() || info.size > MAX_IMAGE_BYTES) { - throw new Error(`Claude image must be a file no larger than ${MAX_IMAGE_BYTES} bytes`) - } - budget.localBytes += info.size + const data = await readClaudeImage(block.path) + budget.localBytes += data.byteLength if (budget.localBytes > MAX_TOTAL_IMAGE_BYTES) { throw new Error(`Claude images must total no more than ${MAX_TOTAL_IMAGE_BYTES} bytes`) } @@ -73,7 +97,7 @@ async function imageContent( source: { type: 'base64', media_type: mediaType, - data: (await readFile(block.path)).toString('base64') + data: data.toString('base64') } } } diff --git a/src/main/claude/claude-structured-inbound-control.test.ts b/src/main/claude/claude-structured-inbound-control.test.ts new file mode 100644 index 00000000000..0ece52dcdd0 --- /dev/null +++ b/src/main/claude/claude-structured-inbound-control.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it, vi } from 'vitest' +import { createClaudeAcquisitionAttempt } from './claude-structured-session-state' +import { ClaudePromptRegistry } from './claude-structured-prompt-replies' +import { handleClaudeInboundControl } from './claude-structured-inbound-control' + +function rejectingAttempt() { + const attempt = createClaudeAcquisitionAttempt(new ClaudePromptRegistry()) + const respond = vi.fn().mockRejectedValue(new Error('connection closed')) + const respondWithError = vi.fn().mockRejectedValue(new Error('connection closed')) + attempt.connection = { respond, respondWithError } as never + return { attempt, respond, respondWithError } +} + +describe('Claude inbound control', () => { + it('consumes rejected writes while declining unsupported controls', async () => { + const dialog = rejectingAttempt() + handleClaudeInboundControl({ + sessionId: 'session-1', + attempt: dialog.attempt, + request: { + type: 'control_request', + request_id: 'dialog-1', + request: { subtype: 'request_user_dialog' } + }, + emit: vi.fn() + }) + const unsupported = rejectingAttempt() + handleClaudeInboundControl({ + sessionId: 'session-1', + attempt: unsupported.attempt, + request: { + type: 'control_request', + request_id: 'control-1', + request: { subtype: 'future_control' } + }, + emit: vi.fn() + }) + await new Promise((resolve) => setImmediate(resolve)) + + expect(dialog.respond).toHaveBeenCalledWith('dialog-1', { behavior: 'cancelled' }) + expect(unsupported.respondWithError).toHaveBeenCalledWith( + 'control-1', + 'Orca does not handle future_control' + ) + }) +}) diff --git a/src/main/claude/claude-structured-inbound-control.ts b/src/main/claude/claude-structured-inbound-control.ts index 26f7e1d8355..3e53a0f6fc7 100644 --- a/src/main/claude/claude-structured-inbound-control.ts +++ b/src/main/claude/claude-structured-inbound-control.ts @@ -12,15 +12,17 @@ export function handleClaudeInboundControl(input: { }): void { const connection = input.attempt.connection if (input.request.request.subtype === 'request_user_dialog') { - void connection?.respond(input.request.request_id, { behavior: 'cancelled' }) + void connection?.respond(input.request.request_id, { behavior: 'cancelled' }).catch(() => {}) return } const prompt = input.attempt.prompts.register(input.request) if (!prompt) { - void connection?.respondWithError( - input.request.request_id, - `Orca does not handle ${input.request.request.subtype}` - ) + void connection + ?.respondWithError( + input.request.request_id, + `Orca does not handle ${input.request.request.subtype}` + ) + .catch(() => {}) return } input.emit({ type: 'prompt', sessionId: input.sessionId, prompt })