mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
fix: harden Claude structured adapter
This commit is contained in:
@@ -0,0 +1,27 @@
|
||||
import { applyClaudeEnvPatch } from '../claude-accounts/environment'
|
||||
|
||||
const CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS = [
|
||||
'CLAUDE_CODE_CHILD_SESSION',
|
||||
'CLAUDE_CODE_SESSION_ID',
|
||||
'CLAUDE_CODE_BRIDGE_SESSION_ID'
|
||||
] as const
|
||||
|
||||
function cloneProcessEnv(): Record<string, string> {
|
||||
const env: Record<string, string> = {}
|
||||
for (const [key, value] of Object.entries(process.env)) {
|
||||
if (value !== undefined) {
|
||||
env[key] = value
|
||||
}
|
||||
}
|
||||
return env
|
||||
}
|
||||
|
||||
export function buildClaudeChildProcessEnv(
|
||||
configuredEnv: Record<string, string> = {}
|
||||
): Record<string, string> {
|
||||
const env = applyClaudeEnvPatch(cloneProcessEnv(), {}, { stripAuthEnv: true })
|
||||
for (const key of CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS) {
|
||||
delete env[key]
|
||||
}
|
||||
return Object.assign(env, configuredEnv)
|
||||
}
|
||||
@@ -1,6 +1,6 @@
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { PassThrough } from 'node:stream'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { spawn } from 'node:child_process'
|
||||
import {
|
||||
openClaudeStreamJsonConnection,
|
||||
@@ -22,8 +22,11 @@ function fakeSpawn() {
|
||||
child.stdout = new PassThrough()
|
||||
child.stderr = new PassThrough()
|
||||
child.kill = vi.fn(() => true)
|
||||
const spawnImpl = vi.fn(() => child) as unknown as typeof spawn
|
||||
return { child, spawnImpl }
|
||||
const spawnMock = vi.fn(
|
||||
(_command: string, _args: readonly string[], _options: { env?: NodeJS.ProcessEnv }) => child
|
||||
)
|
||||
const spawnImpl = spawnMock as unknown as typeof spawn
|
||||
return { child, spawnImpl, spawnMock }
|
||||
}
|
||||
|
||||
function writtenFrames(child: FakeChild): Promise<Record<string, unknown>[]> {
|
||||
@@ -42,6 +45,8 @@ function writtenFrames(child: FakeChild): Promise<Record<string, unknown>[]> {
|
||||
}
|
||||
|
||||
describe('Claude stream-json connection', () => {
|
||||
afterEach(() => vi.unstubAllEnvs())
|
||||
|
||||
it('spawns in the pinned workspace and routes acknowledged control requests', async () => {
|
||||
const process = fakeSpawn()
|
||||
const connection = await openClaudeStreamJsonConnection(
|
||||
@@ -125,4 +130,51 @@ describe('Claude stream-json connection', () => {
|
||||
expect(process.child.kill).toHaveBeenCalled()
|
||||
expect(onExit).toHaveBeenCalledWith(expect.objectContaining({ message: expect.any(String) }))
|
||||
})
|
||||
|
||||
it('uses configured auth and strips inherited auth and child-session stamps', async () => {
|
||||
vi.stubEnv('ANTHROPIC_API_KEY', 'inherited-key')
|
||||
vi.stubEnv('CLAUDE_CODE_CHILD_SESSION', '1')
|
||||
vi.stubEnv('CLAUDE_CODE_SESSION_ID', 'inherited-session')
|
||||
const process = fakeSpawn()
|
||||
|
||||
await openClaudeStreamJsonConnection(
|
||||
{
|
||||
command: 'claude',
|
||||
args: [],
|
||||
cwd: '/work',
|
||||
env: {
|
||||
ANTHROPIC_AUTH_TOKEN: 'configured-token',
|
||||
ANTHROPIC_BASE_URL: 'https://gateway.example.test',
|
||||
CLAUDE_CODE_SESSION_ID: 'configured-session'
|
||||
}
|
||||
},
|
||||
{},
|
||||
process.spawnImpl
|
||||
)
|
||||
|
||||
const env = process.spawnMock.mock.calls[0]?.[2]?.env
|
||||
expect(env).toMatchObject({
|
||||
ANTHROPIC_AUTH_TOKEN: 'configured-token',
|
||||
ANTHROPIC_BASE_URL: 'https://gateway.example.test',
|
||||
CLAUDE_CODE_SESSION_ID: 'configured-session'
|
||||
})
|
||||
expect(env?.ANTHROPIC_API_KEY).toBeUndefined()
|
||||
expect(env?.CLAUDE_CODE_CHILD_SESSION).toBeUndefined()
|
||||
})
|
||||
|
||||
it('settles concurrent and repeated closes after a spawn failure', async () => {
|
||||
const process = fakeSpawn()
|
||||
const connection = await openClaudeStreamJsonConnection(
|
||||
{ command: 'missing-claude', args: [], cwd: '/work' },
|
||||
{},
|
||||
process.spawnImpl
|
||||
)
|
||||
process.child.emit('error', new Error('spawn missing-claude ENOENT'))
|
||||
|
||||
await expect(Promise.all([connection.close(), connection.close()])).resolves.toEqual([
|
||||
undefined,
|
||||
undefined
|
||||
])
|
||||
await expect(connection.close()).resolves.toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,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'
|
||||
|
||||
export type ClaudeStreamJsonLaunch = {
|
||||
command: string
|
||||
@@ -81,7 +82,7 @@ export async function openClaudeStreamJsonConnection(
|
||||
): Promise<ClaudeStreamJsonConnection> {
|
||||
const child = spawnImpl(launch.command, launch.args, {
|
||||
cwd: launch.cwd,
|
||||
env: { ...process.env, ...launch.env },
|
||||
env: buildClaudeChildProcessEnv(launch.env),
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
windowsHide: true
|
||||
}) as ChildProcessWithoutNullStreams
|
||||
@@ -93,13 +94,17 @@ export async function openClaudeStreamJsonConnection(
|
||||
let closing = false
|
||||
let terminalError: Error | null = null
|
||||
let writeChain: Promise<void> = Promise.resolve()
|
||||
let closePromise: Promise<void> | null = null
|
||||
|
||||
let settleExit = (): void => {}
|
||||
const exitPromise = new Promise<void>((resolve) => {
|
||||
child.on('exit', () => {
|
||||
exited = true
|
||||
resolve()
|
||||
})
|
||||
settleExit = resolve
|
||||
})
|
||||
const markExited = (): void => {
|
||||
exited = true
|
||||
settleExit()
|
||||
}
|
||||
child.on('exit', markExited)
|
||||
|
||||
const failPending = (error: Error): void => {
|
||||
for (const waiter of pending.values()) {
|
||||
@@ -215,10 +220,11 @@ export async function openClaudeStreamJsonConnection(
|
||||
stderrTail = (stderrTail + chunk).slice(-STDERR_TAIL_MAX_BYTES)
|
||||
})
|
||||
child.on('error', (error) => {
|
||||
exited = true
|
||||
markExited()
|
||||
handleUnexpectedEnd(error)
|
||||
})
|
||||
child.on('close', () => {
|
||||
markExited()
|
||||
if (stdoutBuffer.trim()) {
|
||||
parseLine(stdoutBuffer)
|
||||
stdoutBuffer = ''
|
||||
@@ -261,25 +267,24 @@ export async function openClaudeStreamJsonConnection(
|
||||
return promise
|
||||
}
|
||||
|
||||
const close = async (): Promise<void> => {
|
||||
if (closing) {
|
||||
await exitPromise
|
||||
return
|
||||
}
|
||||
closing = true
|
||||
try {
|
||||
child.stdin.end()
|
||||
} catch {
|
||||
// The reap below still owns the process.
|
||||
}
|
||||
if (!exited) {
|
||||
await waitForProcessExitUntil(exitPromise, GRACEFUL_EXIT_MS)
|
||||
if (!exited) {
|
||||
killCodexAppServerProcessTree(child)
|
||||
await waitForProcessExitUntil(exitPromise, FORCED_EXIT_MS)
|
||||
const close = (): Promise<void> => {
|
||||
closePromise ??= (async () => {
|
||||
closing = true
|
||||
try {
|
||||
child.stdin.end()
|
||||
} catch {
|
||||
// The reap below still owns the process.
|
||||
}
|
||||
}
|
||||
failPending(new Error('claude stream-json connection closed'))
|
||||
if (!exited) {
|
||||
await waitForProcessExitUntil(exitPromise, GRACEFUL_EXIT_MS)
|
||||
if (!exited) {
|
||||
killCodexAppServerProcessTree(child)
|
||||
await waitForProcessExitUntil(exitPromise, FORCED_EXIT_MS)
|
||||
}
|
||||
}
|
||||
failPending(new Error('claude stream-json connection closed'))
|
||||
})()
|
||||
return closePromise
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { applyClaudePromptAnswer } from './claude-structured-prompt-replies'
|
||||
import { ClaudeControlRequestError } from './claude-stream-json-connection'
|
||||
import type { ClaudeSession } from './claude-structured-session-state'
|
||||
|
||||
export async function cancelClaudeTurn(
|
||||
@@ -8,8 +9,11 @@ export async function cancelClaudeTurn(
|
||||
try {
|
||||
await session.connection.request('interrupt', {}, { timeoutMs })
|
||||
return { cancelled: true }
|
||||
} catch {
|
||||
return { cancelled: false }
|
||||
} catch (error) {
|
||||
if (error instanceof ClaudeControlRequestError) {
|
||||
return { cancelled: false }
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
import { mkdtemp, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { AgentJournalMessageItem } from '../../shared/agent-session-journal-types'
|
||||
import { dispatchClaudeTurn } from './claude-structured-dispatch'
|
||||
import type { ClaudeSession } from './claude-structured-session-state'
|
||||
|
||||
function sessionFor(send = vi.fn().mockResolvedValue(undefined)): ClaudeSession {
|
||||
return {
|
||||
connection: { send } as unknown as ClaudeSession['connection'],
|
||||
providerSessionId: 'provider-session',
|
||||
leafUuid: null,
|
||||
fence: 1,
|
||||
prompts: {} as ClaudeSession['prompts'],
|
||||
dispatchWaiters: [],
|
||||
options: new Map(),
|
||||
reportedOptions: {},
|
||||
events: undefined
|
||||
}
|
||||
}
|
||||
|
||||
function userMessage(blocks: AgentJournalMessageItem['blocks']): AgentJournalMessageItem {
|
||||
return { kind: 'message', role: 'user', blocks }
|
||||
}
|
||||
|
||||
describe('Claude structured dispatch image limits', () => {
|
||||
it('rejects more than twenty URL images before sending', async () => {
|
||||
const session = sessionFor()
|
||||
const body = userMessage(
|
||||
Array.from({ length: 21 }, (_, index) => ({
|
||||
type: 'image-ref' as const,
|
||||
url: `https://example.test/${index}.png`
|
||||
}))
|
||||
)
|
||||
|
||||
await expect(
|
||||
dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }, 1)
|
||||
).resolves.toEqual({ state: 'rejected', reason: 'Claude messages support at most 20 images' })
|
||||
expect(session.connection.send).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects local images whose aggregate size exceeds twenty MiB', async () => {
|
||||
const directory = await mkdtemp(join(tmpdir(), 'orca-claude-images-'))
|
||||
try {
|
||||
const paths = await Promise.all(
|
||||
Array.from({ length: 5 }, async (_, index) => {
|
||||
const path = join(directory, `${index}.png`)
|
||||
await writeFile(path, Buffer.alloc(5 * 1024 * 1024))
|
||||
return path
|
||||
})
|
||||
)
|
||||
const session = sessionFor()
|
||||
const body = userMessage(paths.map((path) => ({ type: 'image-ref' as const, path })))
|
||||
|
||||
await expect(
|
||||
dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }, 1)
|
||||
).resolves.toEqual({
|
||||
state: 'rejected',
|
||||
reason: `Claude images must total no more than ${20 * 1024 * 1024} bytes`
|
||||
})
|
||||
expect(session.connection.send).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
await rm(directory, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -7,6 +7,13 @@ import type { ClaudeSession } from './claude-structured-session-state'
|
||||
import { readClaudeFrameString } from './claude-structured-init-proof'
|
||||
|
||||
const MAX_IMAGE_BYTES = 5 * 1024 * 1024
|
||||
const MAX_IMAGE_COUNT = 20
|
||||
const MAX_TOTAL_IMAGE_BYTES = 20 * 1024 * 1024
|
||||
|
||||
type ImageBudget = {
|
||||
count: number
|
||||
localBytes: number
|
||||
}
|
||||
|
||||
const IMAGE_MIME_BY_EXTENSION: Record<string, string> = {
|
||||
'.gif': 'image/gif',
|
||||
@@ -36,8 +43,13 @@ export function resolveClaudeReplayWaiter(
|
||||
}
|
||||
|
||||
async function imageContent(
|
||||
block: Extract<NativeChatBlock, { type: 'image-ref' }>
|
||||
block: Extract<NativeChatBlock, { type: 'image-ref' }>,
|
||||
budget: ImageBudget
|
||||
): Promise<unknown> {
|
||||
budget.count += 1
|
||||
if (budget.count > MAX_IMAGE_COUNT) {
|
||||
throw new Error(`Claude messages support at most ${MAX_IMAGE_COUNT} images`)
|
||||
}
|
||||
if (block.url) {
|
||||
return { type: 'image', source: { type: 'url', url: block.url } }
|
||||
}
|
||||
@@ -48,6 +60,10 @@ async function imageContent(
|
||||
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
|
||||
if (budget.localBytes > MAX_TOTAL_IMAGE_BYTES) {
|
||||
throw new Error(`Claude images must total no more than ${MAX_TOTAL_IMAGE_BYTES} bytes`)
|
||||
}
|
||||
const mediaType = IMAGE_MIME_BY_EXTENSION[extname(block.path).toLowerCase()]
|
||||
if (!mediaType) {
|
||||
throw new Error(`Claude does not support the image type ${extname(block.path)}`)
|
||||
@@ -67,11 +83,12 @@ async function messageContent(body: AgentJournalMessageItem): Promise<unknown[]>
|
||||
throw new Error('Claude dispatch accepts only user messages')
|
||||
}
|
||||
const content: unknown[] = []
|
||||
const imageBudget: ImageBudget = { count: 0, localBytes: 0 }
|
||||
for (const block of body.blocks as NativeChatBlock[]) {
|
||||
if (block.type === 'text' && block.text.length > 0) {
|
||||
content.push({ type: 'text', text: block.text })
|
||||
} else if (block.type === 'image-ref') {
|
||||
content.push(await imageContent(block))
|
||||
content.push(await imageContent(block, imageBudget))
|
||||
}
|
||||
}
|
||||
if (content.length === 0) {
|
||||
|
||||
@@ -27,6 +27,7 @@ export type ClaudeStructuredLaunch = {
|
||||
command: string
|
||||
args: string[]
|
||||
cwd: string
|
||||
env?: Record<string, string>
|
||||
claudeConfigDir: string
|
||||
providerSessionId: string
|
||||
resumeLeafUuid: string | null
|
||||
|
||||
@@ -9,6 +9,7 @@ import type {
|
||||
ClaudeStreamJsonLaunch,
|
||||
openClaudeStreamJsonConnection
|
||||
} from './claude-stream-json-connection'
|
||||
import { ClaudeControlRequestError } from './claude-stream-json-connection'
|
||||
import { CLAUDE_SPAWN_TOKEN_ENV } from './claude-structured-owner-identity'
|
||||
import { encodeClaudeQuestionOptionId } from './claude-structured-prompt-replies'
|
||||
import {
|
||||
@@ -202,6 +203,27 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => {
|
||||
expect(events[0]).toMatchObject({ type: 'message', message: { subtype: 'init' } })
|
||||
})
|
||||
|
||||
it('forwards configured launch environment while keeping ownership pins authoritative', async () => {
|
||||
const claude = fakeClaude()
|
||||
const adapter = adapterFor(claude, {
|
||||
env: {
|
||||
ANTHROPIC_AUTH_TOKEN: 'configured-token',
|
||||
ANTHROPIC_BASE_URL: 'https://gateway.example.test',
|
||||
CLAUDE_CONFIG_DIR: '/wrong/account',
|
||||
[CLAUDE_SPAWN_TOKEN_ENV]: 'wrong-token'
|
||||
}
|
||||
})
|
||||
|
||||
await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' })
|
||||
|
||||
expect(claude.connections[0].launch.env).toEqual({
|
||||
ANTHROPIC_AUTH_TOKEN: 'configured-token',
|
||||
ANTHROPIC_BASE_URL: 'https://gateway.example.test',
|
||||
CLAUDE_CONFIG_DIR: '/accounts/claude',
|
||||
[CLAUDE_SPAWN_TOKEN_ENV]: 'spawn-9'
|
||||
})
|
||||
})
|
||||
|
||||
it('records only non-secret effective auth-lane diagnostics', async () => {
|
||||
const claude = fakeClaude({
|
||||
settings: {
|
||||
@@ -319,11 +341,18 @@ describe('ClaudeStructuredSessionAdapter turns and controls', () => {
|
||||
])
|
||||
|
||||
claude.routes.interrupt = () => {
|
||||
throw new Error('not running')
|
||||
throw new ClaudeControlRequestError('interrupt', 'not running')
|
||||
}
|
||||
await expect(
|
||||
adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-2', fence: 7 })
|
||||
).resolves.toEqual({ cancelled: false })
|
||||
|
||||
claude.routes.interrupt = () => {
|
||||
throw new Error('claude interrupt request timed out')
|
||||
}
|
||||
await expect(
|
||||
adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-3', fence: 7 })
|
||||
).rejects.toThrow('timed out')
|
||||
})
|
||||
|
||||
it('hydrates live model choices and maps the resolved current model to its CLI id', async () => {
|
||||
|
||||
@@ -117,6 +117,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
|
||||
args: launch.args,
|
||||
cwd: launch.cwd,
|
||||
env: {
|
||||
...launch.env,
|
||||
[CLAUDE_SPAWN_TOKEN_ENV]: input.spawnToken,
|
||||
CLAUDE_CONFIG_DIR: launch.claudeConfigDir
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user