fix(opencode): run bridge commands portably

This commit is contained in:
777genius 2026-05-22 16:49:23 +03:00
parent 587c974c63
commit 97b4d86749
5 changed files with 194 additions and 58 deletions

View file

@ -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<OpenCodeBridgeOutputReadResult> {
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<TBody>
): Promise<string> {
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;

View file

@ -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) {

View file

@ -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',

View file

@ -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<OpenCodeTeamRuntimeBridgePort['launchOpenCodeTeam']>
>(
@ -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(

View file

@ -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<typeof spawnCli>;
type ExecChild = ReturnType<typeof child.exec>;
function createMockProcess<TProcess>(): 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<SpawnCliChild>();
(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<SpawnCliChild>());
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<SpawnCliChild>();
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<SpawnCliChild>();
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<SpawnCliChild>();
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<SpawnCliChild>();
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<SpawnCliChild>();
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<SpawnCliChild>();
(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<ExecChild>();
}
);
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<ExecChild>();
}
);
@ -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<ExecChild>();
});
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<ExecChild>();
}
);
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<ExecChild>();
}
);
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<ExecChild>();
}
);
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<ExecChild>();
});
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<ExecChild>();
});
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<ExecChild>();
});
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<SpawnCliChild>());
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<ExecChild>();
}
);
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<ExecChild>();
});
// 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<ExecChild>();
}
);
@ -646,7 +675,7 @@ describe('cli child process helpers', () => {
});
try {
killProcessTree({ pid: 200 } as any, 'SIGKILL');
killProcessTree({ pid: 200 } as Parameters<typeof killProcessTree>[0], 'SIGKILL');
expect(killSpy.mock.calls.map(([pid]) => pid)).toEqual(
expect.arrayContaining([200, 201, 202])