diff --git a/src/main/services/team/opencode/bridge/OpenCodeBridgeCommandClient.ts b/src/main/services/team/opencode/bridge/OpenCodeBridgeCommandClient.ts index ab6ea9f6..3b49ccca 100644 --- a/src/main/services/team/opencode/bridge/OpenCodeBridgeCommandClient.ts +++ b/src/main/services/team/opencode/bridge/OpenCodeBridgeCommandClient.ts @@ -1,6 +1,6 @@ import { applyOpenCodeAutoUpdatePolicy } from '@main/services/runtime/openCodeAutoUpdatePolicy'; import { execCli } from '@main/utils/childProcess'; -import { randomUUID } from 'crypto'; +import { createHash, randomUUID } from 'crypto'; import { promises as fs } from 'fs'; import * as path from 'path'; @@ -68,6 +68,7 @@ const DEFAULT_STDERR_LIMIT_BYTES = 256_000; const WINDOWS_BATCH_EXTENSIONS = new Set(['.cmd', '.bat']); const EMPTY_STDOUT_READINESS_MAX_ATTEMPTS = 2; const EMPTY_STDOUT_READINESS_RETRY_DELAY_MS = 250; +const SAFE_BRIDGE_INPUT_FILE_REQUEST_ID = /^[A-Za-z0-9._-]{1,120}$/; export function resolveOpenCodeBridgeProcessCwd( binaryPath: string, @@ -278,26 +279,44 @@ export class OpenCodeBridgeCommandClient { outputPath: string ): Promise { const stdoutBytes = byteLength(stdout); - if (stdout.trim().length > 0) { - return { - content: stdout, - outputSource: 'stdout', - stdoutBytes, - outputFileBytes: null, - outputReadError: null, - }; - } try { const output = await fs.readFile(outputPath, 'utf8'); const outputFileBytes = byteLength(output); + if (output.trim().length > 0) { + return { + content: output, + outputSource: 'file', + stdoutBytes, + outputFileBytes, + outputReadError: null, + }; + } + if (stdout.trim().length > 0) { + return { + content: stdout, + outputSource: 'stdout', + stdoutBytes, + outputFileBytes, + outputReadError: null, + }; + } return { content: output, - outputSource: output.trim().length > 0 ? 'file' : 'none', + outputSource: 'none', stdoutBytes, outputFileBytes, outputReadError: null, }; } catch (error) { + if (stdout.trim().length > 0) { + return { + content: stdout, + outputSource: 'stdout', + stdoutBytes, + outputFileBytes: 0, + outputReadError: getBridgeOutputReadError(error), + }; + } return { content: stdout, outputSource: 'none', @@ -319,7 +338,7 @@ export class OpenCodeBridgeCommandClient { envelope: OpenCodeBridgeCommandEnvelope ): Promise { await fs.mkdir(this.tempDirectory, { recursive: true, mode: 0o700 }); - const inputPath = path.join(this.tempDirectory, `opencode-command-${envelope.requestId}.json`); + const inputPath = path.join(this.tempDirectory, buildBridgeInputFileName(envelope.requestId)); await fs.writeFile(inputPath, `${JSON.stringify(envelope, null, 2)}\n`, { encoding: 'utf8', mode: 0o600, @@ -412,6 +431,27 @@ function byteLength(value: string): number { return Buffer.byteLength(value, 'utf8'); } +function buildBridgeInputFileName(requestId: string): string { + const trimmed = requestId.trim(); + if (requestId === trimmed && SAFE_BRIDGE_INPUT_FILE_REQUEST_ID.test(trimmed)) { + return `opencode-command-${trimmed}.json`; + } + + const sanitized = + Array.from(trimmed, (char) => (isUnsafeBridgeInputFileNameChar(char) ? '_' : char)) + .join('') + .replace(/\s+/g, '_') + .replace(/_+/g, '_') + .replace(/^\.+/, '_') + .slice(0, 80) || 'request'; + const fingerprint = createHash('sha256').update(requestId).digest('hex').slice(0, 12); + return `opencode-command-${sanitized}-${fingerprint}.json`; +} + +function isUnsafeBridgeInputFileNameChar(char: string): boolean { + return char.charCodeAt(0) < 32 || '<>:"/\\|?*'.includes(char); +} + function getBridgeOutputReadError(error: unknown): string { if (error && typeof error === 'object' && 'code' in error) { const code = (error as { code?: unknown }).code; diff --git a/src/main/services/team/runtime/OpenCodeTeamRuntimeAdapter.ts b/src/main/services/team/runtime/OpenCodeTeamRuntimeAdapter.ts index 8fcbf273..fe77dbf5 100644 --- a/src/main/services/team/runtime/OpenCodeTeamRuntimeAdapter.ts +++ b/src/main/services/team/runtime/OpenCodeTeamRuntimeAdapter.ts @@ -255,7 +255,9 @@ export class OpenCodeTeamRuntimeAdapter implements TeamLaunchRuntimeAdapter { ); } - const skipReadinessPreflight = input.skipReadinessPreflight === true; + // App-managed OpenCode launch requires a fresh capability snapshot from + // readiness before any state-changing bridge command can run. + const skipReadinessPreflight = false; let selectedModel = input.model?.trim() ?? ''; let launchWarnings: string[] = []; if (!skipReadinessPreflight) { diff --git a/test/main/services/team/OpenCodeBridgeCommandClient.test.ts b/test/main/services/team/OpenCodeBridgeCommandClient.test.ts index e8ef5c10..98385ee3 100644 --- a/test/main/services/team/OpenCodeBridgeCommandClient.test.ts +++ b/test/main/services/team/OpenCodeBridgeCommandClient.test.ts @@ -115,6 +115,68 @@ describe('OpenCodeBridgeCommandClient', () => { }); }); + it('keeps bridge temp file names safe when requestId contains Windows path characters', async () => { + const requestId = 'req:windows/path\\unsafe*id?'; + runner.nextResult = { + stdout: `${JSON.stringify(bridgeSuccess({ requestId, data: { runId: 'run-1' } }))}\n`, + stderr: '', + exitCode: 0, + timedOut: false, + }; + const client = createClient(); + + const result = await client.execute( + 'opencode.launchTeam', + { runId: 'run-1' }, + { + cwd: '/tmp/project', + timeoutMs: 10_000, + requestId, + } + ); + + const inputPath = runner.calls[0].args[4]; + expect(result).toMatchObject({ + ok: true, + requestId, + }); + expect(path.dirname(inputPath)).toBe(tempDir); + expect(path.basename(inputPath)).not.toContain('/'); + expect(path.basename(inputPath)).not.toContain('\\'); + expect(path.basename(inputPath)).not.toContain(':'); + expect(JSON.parse(await runner.readInputEnvelope(0))).toMatchObject({ + requestId, + }); + }); + + it('prefers a non-empty output file over process stdout wrapper text', async () => { + runner.nextResult = { + stdout: '{"ok":true,"command":"opencode.launchTeam","requestId":"req-1","bytes":512}\n', + stderr: '', + exitCode: 0, + timedOut: false, + }; + runner.nextOutputFileContents = `${JSON.stringify( + bridgeSuccess({ data: { runId: 'run-1' } }) + )}\n`; + const client = createClient(); + + const result = await client.execute( + 'opencode.launchTeam', + { runId: 'run-1' }, + { + cwd: '/tmp/project', + timeoutMs: 10_000, + } + ); + + expect(result).toMatchObject({ + ok: true, + requestId: 'req-1', + command: 'opencode.launchTeam', + }); + }); + it('fails closed when stdout contains logs plus json', async () => { runner.nextResult = { stdout: 'debug token=secret\n{"ok":true}\n', diff --git a/test/main/services/team/OpenCodeTeamRuntimeAdapter.test.ts b/test/main/services/team/OpenCodeTeamRuntimeAdapter.test.ts index 5aa07b88..01788ce2 100644 --- a/test/main/services/team/OpenCodeTeamRuntimeAdapter.test.ts +++ b/test/main/services/team/OpenCodeTeamRuntimeAdapter.test.ts @@ -87,7 +87,7 @@ describe('OpenCodeTeamRuntimeAdapter', () => { }); }); - it('can rely on the launch bridge as the only readiness authority', async () => { + it('still runs readiness when a legacy caller asks to skip OpenCode preflight', async () => { const launchOpenCodeTeam = vi.fn< NonNullable >( @@ -126,23 +126,26 @@ describe('OpenCodeTeamRuntimeAdapter', () => { ); const bridge = bridgePort( readiness({ - state: 'unknown_error', - launchAllowed: false, - diagnostics: ['readiness should be skipped'], + state: 'ready', + launchAllowed: true, + diagnostics: ['readiness was required'], }), - { launchOpenCodeTeam } + { + getLastOpenCodeRuntimeSnapshot: vi.fn(() => runtimeSnapshot('cap-1')), + launchOpenCodeTeam, + } ); const adapter = new OpenCodeTeamRuntimeAdapter(bridge); const result = await adapter.launch(launchInput({ skipReadinessPreflight: true })); expect(result.teamLaunchState).toBe('clean_success'); - expect(bridge.checkOpenCodeTeamLaunchReadiness).not.toHaveBeenCalled(); + expect(bridge.checkOpenCodeTeamLaunchReadiness).toHaveBeenCalledTimes(1); expect(launchOpenCodeTeam).toHaveBeenCalledWith( expect.objectContaining({ selectedModel: 'openai/gpt-5.4-mini', skipPermissions: true, - expectedCapabilitySnapshotId: null, + expectedCapabilitySnapshotId: 'cap-1', }) ); expect(result.diagnostics).toEqual( diff --git a/test/main/utils/childProcess.test.ts b/test/main/utils/childProcess.test.ts index e26a6d01..3d0b48ca 100644 --- a/test/main/utils/childProcess.test.ts +++ b/test/main/utils/childProcess.test.ts @@ -1,10 +1,17 @@ // @vitest-environment node +import { + execCli, + killProcessTree, + killTrackedCliProcesses, + quoteWindowsCmdArg, + spawnCli, +} from '@main/utils/childProcess'; +import * as child from 'child_process'; +import { EventEmitter } from 'events'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'; import { tmpdir } from 'os'; import path from 'path'; -import { EventEmitter } from 'events'; - -import { describe, it, expect, beforeEach, afterEach, vi, type Mock } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from 'vitest'; // Mock the entire child_process module so that we can inspect how our helpers // invoke spawn/exec without hitting the real filesystem or spawning anything. @@ -19,17 +26,13 @@ vi.mock('child_process', async (importOriginal) => { }; }); -// Import after the mock call so that the mocked module is returned. -import * as child from 'child_process'; -import { - execCli, - killTrackedCliProcesses, - killProcessTree, - quoteWindowsCmdArg, - spawnCli, -} from '@main/utils/childProcess'; - type ExecCallback = (error: Error | null, stdout: string, stderr: string) => void; +type SpawnCliChild = ReturnType; +type ExecChild = ReturnType; + +function createMockProcess(): TProcess { + return new EventEmitter() as TProcess; +} // Helper to temporarily override process.platform function setPlatform(value: string) { @@ -153,7 +156,8 @@ describe('cli child process helpers', () => { describe('spawnCli', () => { it('calls spawn directly when path is ascii on windows', () => { setPlatform('win32'); - (child.spawn as unknown as Mock).mockReturnValue({} as any); + const fake = createMockProcess(); + (child.spawn as unknown as Mock).mockReturnValue(fake); const result = spawnCli('C:\\bin\\claude.exe', ['--version'], { cwd: 'x' }); expect(child.spawn).toHaveBeenCalledWith( @@ -164,13 +168,13 @@ describe('cli child process helpers', () => { env: expect.objectContaining({ CLAUDE_HOOK_JUDGE_MODE: 'true' }), }) ); - expect(result).toEqual({} as any); + expect(result).toBe(fake); }); it('hides spawned CLI windows by default but preserves explicit opt-out', () => { setPlatform('win32'); const spawnMock = child.spawn as unknown as Mock; - spawnMock.mockReturnValue({} as any); + spawnMock.mockReturnValue(createMockProcess()); spawnCli('C:\\bin\\claude.exe', ['--version']); expect(spawnMock.mock.calls[0][2]).toMatchObject({ windowsHide: true }); @@ -181,9 +185,9 @@ describe('cli child process helpers', () => { it('falls back to shell when spawn throws EINVAL', () => { setPlatform('win32'); - const error: any = new Error('spawn EINVAL'); + const error = new Error('spawn EINVAL') as NodeJS.ErrnoException; error.code = 'EINVAL'; - const fake = {} as any; + const fake = createMockProcess(); const spawnMock = child.spawn as unknown as Mock; spawnMock.mockImplementationOnce(() => { throw error; @@ -203,7 +207,7 @@ describe('cli child process helpers', () => { it('uses shell directly for Windows cmd launchers', () => { setPlatform('win32'); - const fake = {} as any; + const fake = createMockProcess(); const spawnMock = child.spawn as unknown as Mock; spawnMock.mockReturnValue(fake); @@ -216,7 +220,7 @@ describe('cli child process helpers', () => { it('runs generated Bun cmd launchers directly to preserve percent args', () => { setPlatform('win32'); - const fake = {} as any; + const fake = createMockProcess(); const spawnMock = child.spawn as unknown as Mock; spawnMock.mockReturnValue(fake); const { dir, launcher, target } = createGeneratedBunLauncher(); @@ -234,7 +238,7 @@ describe('cli child process helpers', () => { it('runs extensionless npm node cmd launchers directly', () => { setPlatform('win32'); - const fake = {} as any; + const fake = createMockProcess(); const spawnMock = child.spawn as unknown as Mock; spawnMock.mockReturnValue(fake); const { dir, launcher, target } = createExtensionlessNpmNodeLauncher(); @@ -271,7 +275,7 @@ describe('cli child process helpers', () => { it('uses shell directly when path contains non-ASCII on windows', () => { setPlatform('win32'); - const fake = {} as any; + const fake = createMockProcess(); const spawnMock = child.spawn as unknown as Mock; spawnMock.mockReturnValue(fake); @@ -288,7 +292,8 @@ describe('cli child process helpers', () => { it('does not use shell when not on windows', () => { setPlatform('linux'); - (child.spawn as unknown as Mock).mockReturnValue({} as any); + const fake = createMockProcess(); + (child.spawn as unknown as Mock).mockReturnValue(fake); const result = spawnCli('/usr/bin/claude', ['--help']); expect(child.spawn).toHaveBeenCalledWith( '/usr/bin/claude', @@ -297,7 +302,7 @@ describe('cli child process helpers', () => { env: expect.objectContaining({ CLAUDE_HOOK_JUDGE_MODE: 'true' }), }) ); - expect(result).toEqual({} as any); + expect(result).toBe(fake); }); it('kills tracked CLI processes on shutdown', () => { @@ -350,7 +355,7 @@ describe('cli child process helpers', () => { execFileMock.mockImplementation( (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); - return {} as any; + return createMockProcess(); } ); const result = await execCli('C:\\bin\\claude.exe', ['--version']); @@ -371,7 +376,7 @@ describe('cli child process helpers', () => { execFileMock.mockImplementation( (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); - return {} as any; + return createMockProcess(); } ); @@ -388,7 +393,7 @@ describe('cli child process helpers', () => { const execMock = child.exec as unknown as Mock; execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { cb(null, '0.0.8', ''); - return {} as any; + return createMockProcess(); }); const result = await execCli('C:\\runtime\\cli-dev.cmd', ['--version']); @@ -404,7 +409,7 @@ describe('cli child process helpers', () => { execFileMock.mockImplementation( (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); - return {} as any; + return createMockProcess(); } ); const { dir, launcher, target } = createGeneratedBunLauncher(); @@ -427,7 +432,7 @@ describe('cli child process helpers', () => { execFileMock.mockImplementation( (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); - return {} as any; + return createMockProcess(); } ); const { dir, launcher, target } = createExtensionlessNpmNodeLauncher(); @@ -443,13 +448,37 @@ describe('cli child process helpers', () => { } }); + it('executes npm native exe cmd launchers directly', async () => { + setPlatform('win32'); + const execFileMock = child.execFile as unknown as Mock; + const execMock = child.exec as unknown as Mock; + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { + cb(null, '{"ok":true}', ''); + return createMockProcess(); + } + ); + const { dir, launcher, target } = createNpmNativeExeLauncher(); + try { + const result = await execCli(launcher, ['runtime', 'providers', 'view']); + expect(execFileMock).toHaveBeenCalledTimes(1); + expect(execFileMock.mock.calls[0][0]).toBe(target); + expect(execFileMock.mock.calls[0][1]).toEqual(['runtime', 'providers', 'view']); + expect(execFileMock.mock.calls[0][2]).toMatchObject({ windowsHide: true }); + expect(execMock).not.toHaveBeenCalled(); + expect(result.stdout).toBe('{"ok":true}'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it('skips straight to shell when path contains non-ASCII on windows', async () => { setPlatform('win32'); const execFileMock = child.execFile as unknown as Mock; const execMock = child.exec as unknown as Mock; execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { cb(null, '1.2.3', ''); - return {} as any; + return createMockProcess(); }); const result = await execCli('C:\\Users\\Алексей\\AppData\\Roaming\\npm\\claude.cmd', [ @@ -466,7 +495,7 @@ describe('cli child process helpers', () => { const execMock = child.exec as unknown as Mock; execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); - return {} as any; + return createMockProcess(); }); await execCli('C:\\Users\\Алексей\\bin\\claude.cmd', ['--model', 'test%PATH%"arg']); @@ -485,7 +514,7 @@ describe('cli child process helpers', () => { const execMock = child.exec as unknown as Mock; execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); - return {} as any; + return createMockProcess(); }); await execCli('C:\\runtime\\cli-dev.cmd', [ @@ -505,9 +534,9 @@ describe('cli child process helpers', () => { it('shell: true cannot be overridden by caller options', () => { setPlatform('win32'); const spawnMock = child.spawn as unknown as Mock; - spawnMock.mockReturnValue({} as any); + spawnMock.mockReturnValue(createMockProcess()); - spawnCli('C:\\Users\\Алексей\\bin\\claude.cmd', ['--version'], { shell: false } as any); + spawnCli('C:\\Users\\Алексей\\bin\\claude.cmd', ['--version'], { shell: false }); // shell: true must win over caller's shell: false expect(spawnMock.mock.calls[0][1]).toMatchObject({ shell: true }); }); @@ -520,13 +549,13 @@ describe('cli child process helpers', () => { const err = new Error('spawn EINVAL') as Error & { code?: string }; err.code = 'EINVAL'; cb(err, '', ''); - return {} as any; + return createMockProcess(); } ); const execMock = child.exec as unknown as Mock; execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { cb(null, '2.3.4', ''); - return {} as any; + return createMockProcess(); }); // ASCII path — goes through execFile first, gets EINVAL, falls back to shell @@ -542,7 +571,7 @@ describe('cli child process helpers', () => { execFileMock.mockImplementation( (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(new Error('Command failed'), '{"error":"bad"}', 'bun: not found'); - return {} as any; + return createMockProcess(); } ); @@ -646,7 +675,7 @@ describe('cli child process helpers', () => { }); try { - killProcessTree({ pid: 200 } as any, 'SIGKILL'); + killProcessTree({ pid: 200 } as Parameters[0], 'SIGKILL'); expect(killSpy.mock.calls.map(([pid]) => pid)).toEqual( expect.arrayContaining([200, 201, 202])