diff --git a/src/utils/execFileNoThrow.test.ts b/src/utils/execFileNoThrow.test.ts new file mode 100644 index 00000000..9fff427e --- /dev/null +++ b/src/utils/execFileNoThrow.test.ts @@ -0,0 +1,25 @@ +import { expect, test } from 'bun:test' +import { execFileNoThrowWithCwd } from './execFileNoThrow.js' + +test('execFileNoThrowWithCwd rejects shell-like executable names', async () => { + const result = await execFileNoThrowWithCwd('openclaude && whoami', []) + + expect(result.code).toBe(1) + expect(result.error).toContain('Unsafe executable') +}) + +test('execFileNoThrowWithCwd rejects cwd values with control characters', async () => { + const result = await execFileNoThrowWithCwd(process.execPath, ['--version'], { + cwd: 'C:\\repo\nmalicious', + }) + + expect(result.code).toBe(1) + expect(result.error).toContain('Unsafe working directory') +}) + +test('execFileNoThrowWithCwd rejects arguments with control characters', async () => { + const result = await execFileNoThrowWithCwd(process.execPath, ['--version\nmalicious']) + + expect(result.code).toBe(1) + expect(result.error).toContain('Unsafe argument') +}) diff --git a/src/utils/execFileNoThrow.ts b/src/utils/execFileNoThrow.ts index cc71fb8d..3b0c404d 100644 --- a/src/utils/execFileNoThrow.ts +++ b/src/utils/execFileNoThrow.ts @@ -3,6 +3,7 @@ // By using execa, Windows automatically gets shell escaping + BAT / CMD handling import { type ExecaError, execa } from 'execa' +import path from 'node:path' import { getCwd } from '../utils/cwd.js' import { logError } from './log.js' @@ -59,6 +60,46 @@ type ExecaResultWithError = { signal?: string } +const CONTROL_CHAR_PATTERN = /[\0\r\n]/ +const SAFE_BARE_EXECUTABLE_PATTERN = /^[A-Za-z0-9_.-]+$/ + +function hasPathSyntax(value: string): boolean { + return value.includes(path.sep) || value.includes('/') || path.isAbsolute(value) +} + +function validateExecutable(file: string): string | null { + const normalized = file.trim() + if (!normalized) { + return 'Unsafe executable: empty command' + } + if (CONTROL_CHAR_PATTERN.test(normalized)) { + return 'Unsafe executable: control characters are not allowed' + } + if (!hasPathSyntax(normalized) && !SAFE_BARE_EXECUTABLE_PATTERN.test(normalized)) { + return 'Unsafe executable: bare command names may only contain letters, numbers, ".", "_" and "-"' + } + return null +} + +function validateArgs(args: string[]): string | null { + for (const arg of args) { + if (CONTROL_CHAR_PATTERN.test(arg)) { + return 'Unsafe argument: control characters are not allowed' + } + } + return null +} + +function validateWorkingDirectory(cwd: string | undefined): string | null { + if (!cwd) { + return null + } + if (CONTROL_CHAR_PATTERN.test(cwd)) { + return 'Unsafe working directory: control characters are not allowed' + } + return null +} + /** * Extracts a human-readable error message from an execa result. * @@ -103,6 +144,21 @@ export function execFileNoThrowWithCwd( maxBuffer: 1_000_000, }, ): Promise<{ stdout: string; stderr: string; code: number; error?: string }> { + const executableError = validateExecutable(file) + if (executableError) { + return Promise.resolve({ stdout: '', stderr: '', code: 1, error: executableError }) + } + + const argsError = validateArgs(args) + if (argsError) { + return Promise.resolve({ stdout: '', stderr: '', code: 1, error: argsError }) + } + + const cwdError = validateWorkingDirectory(finalCwd) + if (cwdError) { + return Promise.resolve({ stdout: '', stderr: '', code: 1, error: cwdError }) + } + return new Promise(resolve => { // Use execa for cross-platform .bat/.cmd compatibility on Windows execa(file, args, { diff --git a/vscode-extension/openclaude-vscode/src/state.js b/vscode-extension/openclaude-vscode/src/state.js index 9f4e13f7..e17a12eb 100644 --- a/vscode-extension/openclaude-vscode/src/state.js +++ b/vscode-extension/openclaude-vscode/src/state.js @@ -109,6 +109,19 @@ function isLocalBaseUrl(baseUrl) { } } +function getHostname(baseUrl) { + const normalized = asNonEmptyString(baseUrl); + if (!normalized) { + return null; + } + + try { + return new URL(normalized).hostname.toLowerCase(); + } catch { + return null; + } +} + function resolveCommandCheckPath(command, workspacePath) { const normalized = asNonEmptyString(command); if (!normalized) { @@ -241,6 +254,7 @@ function hasCodexAlias(model) { function getOpenAICompatibleLabel(baseUrl, model) { const normalizedBaseUrl = (asNonEmptyString(baseUrl) || '').toLowerCase(); const normalizedModel = (asNonEmptyString(model) || '').toLowerCase(); + const hostname = getHostname(baseUrl); if (hasCodexBaseUrl(baseUrl) || (!baseUrl && hasCodexAlias(model))) { return 'Codex'; @@ -278,7 +292,7 @@ function getOpenAICompatibleLabel(baseUrl, model) { return 'Azure OpenAI'; } - if (normalizedBaseUrl.includes('api.openai.com') || !normalizedBaseUrl) { + if (hostname === 'api.openai.com' || !normalizedBaseUrl) { return 'OpenAI'; } diff --git a/vscode-extension/openclaude-vscode/src/state.test.js b/vscode-extension/openclaude-vscode/src/state.test.js index 5495df59..b3e0bcb9 100644 --- a/vscode-extension/openclaude-vscode/src/state.test.js +++ b/vscode-extension/openclaude-vscode/src/state.test.js @@ -158,6 +158,44 @@ test('describeProviderState reports LM Studio from openai profile base url', () ); }); +test('describeProviderState does not treat substring-matched OpenAI hosts as OpenAI', () => { + assert.deepEqual( + describeProviderState({ + shimEnabled: false, + env: { + CLAUDE_CODE_USE_OPENAI: '1', + OPENAI_BASE_URL: 'https://evil.example/path/api.openai.com/v1', + OPENAI_MODEL: 'gpt-4o', + }, + profile: null, + }), + { + label: 'OpenAI-compatible', + detail: 'gpt-4o', + source: 'env', + }, + ); +}); + +test('describeProviderState reports OpenAI when the parsed host is api.openai.com', () => { + assert.deepEqual( + describeProviderState({ + shimEnabled: false, + env: { + CLAUDE_CODE_USE_OPENAI: '1', + OPENAI_BASE_URL: 'https://api.openai.com/v1', + OPENAI_MODEL: 'gpt-4o', + }, + profile: null, + }), + { + label: 'OpenAI', + detail: 'gpt-4o', + source: 'env', + }, + ); +}); + test('describeProviderState reports environment-backed provider details', () => { assert.deepEqual( describeProviderState({