diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index e4024758..93826dd6 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -52,6 +52,7 @@ import { type WorkspaceTrustProvider, type WorkspaceTrustWorkspace, } from '@features/workspace-trust/main'; +import { validateTeamName } from '@main/ipc/guards'; import { ConfigManager } from '@main/services/infrastructure/ConfigManager'; import { NotificationManager } from '@main/services/infrastructure/NotificationManager'; import { prepareAgentChildProcessWritableEnv } from '@main/services/runtime/agentChildProcessPreflight'; @@ -636,8 +637,31 @@ const TEAMMATE_BOOTSTRAP_PROOF_TOKEN_ENV = 'CLAUDE_CODE_BOOTSTRAP_PROOF_TOKEN'; const NATIVE_APP_MANAGED_BOOTSTRAP_CONTEXT_ENV = 'CLAUDE_CODE_NATIVE_APP_MANAGED_BOOTSTRAP_CONTEXT_PATH'; +function resolveValidatedTeamStoragePath( + basePath: string, + teamName: string, + ...segments: string[] +): string { + const validated = validateTeamName(teamName); + if (!validated.valid || !validated.value) { + throw new Error(validated.error ?? 'Invalid teamName for team storage path'); + } + + const root = path.resolve(basePath); + const teamDir = path.resolve(root, validated.value); + if (!isPathWithinRoot(teamDir, root)) { + throw new Error(`Invalid team storage path for "${validated.value}"`); + } + + const target = path.resolve(teamDir, ...segments); + if (!isPathWithinRoot(target, teamDir)) { + throw new Error(`Invalid team storage child path for "${validated.value}"`); + } + return target; +} + function getTeamRuntimeEventsDir(teamName: string): string { - return path.join(getTeamsBasePath(), teamName, 'runtime'); + return resolveValidatedTeamStoragePath(getTeamsBasePath(), teamName, 'runtime'); } function isProcessBootstrapTransportDiagnostic(value: unknown): value is string { @@ -4993,7 +5017,7 @@ export class TeamProvisioningService { } private readPersistedTeamProcessRows(teamName: string): unknown[] | null { - const processesPath = path.join(getTeamsBasePath(), teamName, 'processes.json'); + const processesPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'processes.json'); let parsed: unknown; try { parsed = JSON.parse(fs.readFileSync(processesPath, 'utf8')) as unknown; @@ -14702,7 +14726,7 @@ export class TeamProvisioningService { bootstrapContextHash?: string; bootstrapBriefingHash?: string; }): Promise { - const configPath = path.join(getTeamsBasePath(), input.teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), input.teamName, 'config.json'); const raw = await tryReadRegularFileUtf8(configPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, maxBytes: TEAM_CONFIG_MAX_BYTES, @@ -19890,7 +19914,7 @@ export class TeamProvisioningService { try { const teamsBasePathsToProbe = getTeamsBasePathsToProbe(); for (const probe of teamsBasePathsToProbe) { - const configPath = path.join(probe.basePath, request.teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(probe.basePath, request.teamName, 'config.json'); if (await this.pathExists(configPath)) { const suffix = probe.location === 'configured' ? '' : ` (found under ${probe.basePath})`; throw new Error(`Team already exists${suffix}`); @@ -20199,8 +20223,8 @@ export class TeamProvisioningService { // member_briefing intentionally reads canonical team metadata/inboxes, so // createTeam must materialize those files before building the bootstrap spec. emitProvisioningCheckpoint(run, 'Persisting team metadata before spawn'); - const teamDir = path.join(getTeamsBasePath(), request.teamName); - const tasksDir = path.join(getTasksBasePath(), request.teamName); + const teamDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), request.teamName); + const tasksDir = this.resolveSafeTeamStoragePath(getTasksBasePath(), request.teamName); await fs.promises.mkdir(teamDir, { recursive: true }); await fs.promises.mkdir(tasksDir, { recursive: true }); await this.teamMetaStore.writeMeta(request.teamName, { @@ -20297,8 +20321,8 @@ export class TeamProvisioningService { }).catch(() => undefined); } await this.teamMetaStore.deleteMeta(request.teamName).catch(() => {}); - const teamDir = path.join(getTeamsBasePath(), request.teamName); - const tasksDir = path.join(getTasksBasePath(), request.teamName); + const teamDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), request.teamName); + const tasksDir = this.resolveSafeTeamStoragePath(getTasksBasePath(), request.teamName); await fs.promises.rm(teamDir, { recursive: true, force: true }).catch(() => {}); await fs.promises.rm(tasksDir, { recursive: true, force: true }).catch(() => {}); await removeDeterministicBootstrapSpecFile(run.bootstrapSpecPath).catch(() => {}); @@ -20402,8 +20426,8 @@ export class TeamProvisioningService { } catch (error) { // Clean up pre-saved meta files if spawn failed (instant failure, not transient) await this.teamMetaStore.deleteMeta(request.teamName).catch(() => {}); - const teamDir = path.join(getTeamsBasePath(), request.teamName); - const tasksDir = path.join(getTasksBasePath(), request.teamName); + const teamDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), request.teamName); + const tasksDir = this.resolveSafeTeamStoragePath(getTasksBasePath(), request.teamName); await fs.promises.rm(teamDir, { recursive: true, force: true }).catch(() => {}); await fs.promises.rm(tasksDir, { recursive: true, force: true }).catch(() => {}); await removeDeterministicBootstrapSpecFile(run.bootstrapSpecPath).catch(() => {}); @@ -20510,7 +20534,7 @@ export class TeamProvisioningService { ): Promise { const teamsBasePathsToProbe = getTeamsBasePathsToProbe(); for (const probe of teamsBasePathsToProbe) { - const configPath = path.join(probe.basePath, request.teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(probe.basePath, request.teamName, 'config.json'); if (await this.pathExists(configPath)) { const suffix = probe.location === 'configured' ? '' : ` (found under ${probe.basePath})`; throw new Error(`Team already exists${suffix}`); @@ -20529,8 +20553,8 @@ export class TeamProvisioningService { leadProviderId: launchRequest.providerId, members: materialized.members, }); - const teamDir = path.join(getTeamsBasePath(), launchRequest.teamName); - const tasksDir = path.join(getTasksBasePath(), launchRequest.teamName); + const teamDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), launchRequest.teamName); + const tasksDir = this.resolveSafeTeamStoragePath(getTasksBasePath(), launchRequest.teamName); await fs.promises.mkdir(teamDir, { recursive: true }); await fs.promises.mkdir(tasksDir, { recursive: true }); await this.teamMetaStore.writeMeta(launchRequest.teamName, { @@ -20568,7 +20592,7 @@ export class TeamProvisioningService { request: TeamLaunchRequest, onProgress: (progress: TeamProvisioningProgress) => void ): Promise { - const configPath = path.join(getTeamsBasePath(), request.teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), request.teamName, 'config.json'); const configRaw = await tryReadRegularFileUtf8(configPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, maxBytes: TEAM_CONFIG_MAX_BYTES, @@ -20842,7 +20866,7 @@ export class TeamProvisioningService { request: TeamCreateRequest, members: TeamCreateRequest['members'] ): Promise { - const configPath = path.join(getTeamsBasePath(), request.teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), request.teamName, 'config.json'); const config: TeamConfig = { name: request.displayName?.trim() || request.teamName, description: request.description, @@ -21078,7 +21102,7 @@ export class TeamProvisioningService { try { // Verify config.json exists — team must already be provisioned - const configPath = path.join(getTeamsBasePath(), request.teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), request.teamName, 'config.json'); const configRaw = await tryReadRegularFileUtf8(configPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, maxBytes: TEAM_CONFIG_MAX_BYTES, @@ -23927,7 +23951,12 @@ export class TeamProvisioningService { member: string, messages: { messageId: string }[] ): Promise { - const inboxPath = path.join(getTeamsBasePath(), teamName, 'inboxes', `${member}.json`); + const inboxDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'inboxes'); + const safeMemberName = this.normalizeSafeInboxBaseName(member); + if (!safeMemberName) { + return; + } + const inboxPath = this.resolveSafeInboxFilePath(inboxDir, `${safeMemberName}.json`); await withFileLock(inboxPath, async () => { await withInboxLock(inboxPath, async () => { @@ -24060,7 +24089,7 @@ export class TeamProvisioningService { * one-shot subagent instead of a persistent teammate. */ private async getRegisteredTeamMemberNames(teamName: string): Promise | null> { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); try { const raw = await tryReadRegularFileUtf8(configPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, @@ -24089,7 +24118,7 @@ export class TeamProvisioningService { const registeredNames = await this.getRegisteredTeamMemberNames(run.teamName); if (!registeredNames) { try { - await fs.promises.access(path.join(getTeamsBasePath(), run.teamName)); + await fs.promises.access(this.resolveSafeTeamStoragePath(getTeamsBasePath(), run.teamName)); } catch { return; } @@ -28946,7 +28975,12 @@ export class TeamProvisioningService { teamName: string, leadName: string ): Promise { - const inboxPath = path.join(getTeamsBasePath(), teamName, 'inboxes', `${leadName}.json`); + const inboxDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'inboxes'); + const safeLeadName = this.normalizeSafeInboxBaseName(leadName); + if (!safeLeadName) { + return []; + } + const inboxPath = this.resolveSafeInboxFilePath(inboxDir, `${safeLeadName}.json`); try { const raw = await tryReadRegularFileUtf8(inboxPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, @@ -29772,7 +29806,7 @@ export class TeamProvisioningService { return { snapshot: null, statuses: {} }; } - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); let configMembers = new Set(); let configBootstrapRunIds = new Map(); let leadName = 'team-lead'; @@ -31714,7 +31748,7 @@ export class TeamProvisioningService { } private readPersistedTeamProjectPath(teamName: string): string | null { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); try { const raw = fs.readFileSync(configPath, 'utf8'); const parsed = JSON.parse(raw) as { projectPath?: unknown }; @@ -31726,7 +31760,7 @@ export class TeamProvisioningService { } private readPersistedRuntimeMembers(teamName: string): PersistedRuntimeMemberLike[] { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); try { const raw = fs.readFileSync(configPath, 'utf8'); const parsed = JSON.parse(raw) as { members?: unknown }; @@ -33778,8 +33812,8 @@ export class TeamProvisioningService { toolNames = ['Edit', 'Write', 'NotebookEdit', 'Bash', 'Read', 'Grep', 'Glob']; } if (toolNames.length > 0) { - const settingsPath = path.join(projectCwd, '.claude', 'settings.local.json'); try { + const settingsPath = await this.resolveProjectClaudeSettingsPath(projectCwd); await this.addPermissionRulesToSettings(settingsPath, toolNames, 'allow'); logger.info( `[${run.teamName}] Applied setMode "${mode}" for ${agentId}: ${toolNames.join(', ')} in ${settingsPath}` @@ -33818,13 +33852,9 @@ export class TeamProvisioningService { } const behavior = suggestion.behavior ?? 'allow'; - // FACT: observed destinations are "localSettings" (project-level .claude/settings.local.json) - const settingsPath = - suggestion.destination === 'localSettings' - ? path.join(projectCwd, '.claude', 'settings.local.json') - : path.join(projectCwd, '.claude', 'settings.local.json'); // default to local - try { + // FACT: observed destinations are "localSettings"; default to project local settings. + const settingsPath = await this.resolveProjectClaudeSettingsPath(projectCwd); await this.addPermissionRulesToSettings(settingsPath, toolNames, behavior); logger.info( `[${run.teamName}] Added permission rules for ${agentId}: ${toolNames.join(', ')} -> ${behavior} in ${settingsPath}` @@ -33957,6 +33987,94 @@ export class TeamProvisioningService { return typeof firstQuestion?.question === 'string' ? { [firstQuestion.question]: message } : {}; } + private resolveSafeTeamStoragePath( + basePath: string, + teamName: string, + ...segments: string[] + ): string { + return resolveValidatedTeamStoragePath(basePath, teamName, ...segments); + } + + private async resolveProjectClaudeSettingsPath(projectCwd: string): Promise { + if (!path.isAbsolute(projectCwd)) { + throw new Error('Project cwd must be an absolute path'); + } + + const projectRoot = path.resolve(projectCwd); + let projectStat: fs.Stats; + try { + projectStat = await fs.promises.stat(projectRoot); + } catch { + throw new Error(`Project cwd does not exist: ${projectCwd}`); + } + if (!projectStat.isDirectory()) { + throw new Error(`Project cwd is not a directory: ${projectCwd}`); + } + + const realProjectRoot = await fs.promises.realpath(projectRoot); + const claudeDir = path.join(realProjectRoot, '.claude'); + let claudeDirStat: fs.Stats | null = null; + try { + claudeDirStat = await fs.promises.lstat(claudeDir); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + } + + if (claudeDirStat && !claudeDirStat.isDirectory() && !claudeDirStat.isSymbolicLink()) { + throw new Error('Project .claude path is not a directory'); + } + if (!claudeDirStat) { + await fs.promises.mkdir(claudeDir, { recursive: true }); + } + + const realClaudeDir = await fs.promises.realpath(claudeDir); + if (!isPathWithinRoot(realClaudeDir, realProjectRoot)) { + throw new Error('Project .claude directory resolves outside project cwd'); + } + const realClaudeStat = await fs.promises.stat(realClaudeDir); + if (!realClaudeStat.isDirectory()) { + throw new Error('Project .claude path is not a directory'); + } + + return path.join(claudeDir, 'settings.local.json'); + } + + private normalizeSafeInboxBaseName(baseName: string): string | null { + const trimmed = baseName.trim(); + return /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/.test(trimmed) ? trimmed : null; + } + + private isSafeInboxJsonFileName(fileName: string): boolean { + return ( + path.basename(fileName) === fileName && + /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}\.json$/.test(fileName) + ); + } + + private resolveSafeInboxFilePath(inboxDir: string, fileName: string): string { + if (!this.isSafeInboxJsonFileName(fileName)) { + throw new Error(`Invalid inbox file name: ${fileName}`); + } + const resolved = path.resolve(inboxDir, fileName); + if (!isPathWithinRoot(resolved, inboxDir)) { + throw new Error(`Invalid inbox file path: ${fileName}`); + } + return resolved; + } + + private isSafeAutoSuffixedInboxDuplicate(fileName: string, baseName: string): boolean { + if (!this.isSafeInboxJsonFileName(fileName)) { + return false; + } + const prefix = `${baseName}-`; + if (!fileName.startsWith(prefix) || !fileName.endsWith('.json')) { + return false; + } + const suffix = fileName.slice(prefix.length, -'.json'.length); + return /^\d+$/.test(suffix); + } /** * Safely add tool names to the permissions.allow (or deny) array in a Claude settings file. * Creates the file and parent directories if they don't exist. @@ -34015,8 +34133,8 @@ export class TeamProvisioningService { teamName: string, projectCwd: string ): Promise { - const settingsPath = path.join(projectCwd, '.claude', 'settings.local.json'); try { + const settingsPath = await this.resolveProjectClaudeSettingsPath(projectCwd); const allTools = [ ...AGENT_TEAMS_NAMESPACED_LEAD_BOOTSTRAP_TOOL_NAMES, 'Edit', @@ -34109,7 +34227,7 @@ export class TeamProvisioningService { // Best-effort: detect CLI-suffixed member names (alice-2, bob-2) that indicate // a stale config.json was present during launch (double-launch race). try { - const postLaunchConfigPath = path.join(getTeamsBasePath(), run.teamName, 'config.json'); + const postLaunchConfigPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), run.teamName, 'config.json'); const raw = await tryReadRegularFileUtf8(postLaunchConfigPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, maxBytes: TEAM_CONFIG_MAX_BYTES, @@ -35169,9 +35287,12 @@ export class TeamProvisioningService { * Emits progress updates as team files appear (config, inboxes, tasks). */ private startFilesystemMonitor(run: ProvisioningRun, request: TeamCreateRequest): void { - const configuredTeamDir = path.join(getTeamsBasePath(), run.teamName); - const defaultTeamDir = path.join(getAutoDetectedClaudeBasePath(), 'teams', run.teamName); - const tasksDir = path.join(getTasksBasePath(), run.teamName); + const configuredTeamDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), run.teamName); + const defaultTeamDir = this.resolveSafeTeamStoragePath( + path.join(getAutoDetectedClaudeBasePath(), 'teams'), + run.teamName + ); + const tasksDir = this.resolveSafeTeamStoragePath(getTasksBasePath(), run.teamName); const primaryProvisioningMembers = Array.isArray(run.effectiveMembers) ? run.effectiveMembers : request.members; @@ -35442,9 +35563,9 @@ export class TeamProvisioningService { } if (code === 0) { - const configuredConfigPath = path.join(getTeamsBasePath(), run.teamName, 'config.json'); + const configuredConfigPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), run.teamName, 'config.json'); const defaultTeamsBasePath = path.join(getAutoDetectedClaudeBasePath(), 'teams'); - const defaultConfigPath = path.join(defaultTeamsBasePath, run.teamName, 'config.json'); + const defaultConfigPath = this.resolveSafeTeamStoragePath(defaultTeamsBasePath, run.teamName, 'config.json'); const combinedLogs = buildCombinedLogs(run.stdoutBuffer, run.stderrBuffer); const cleanupHint = logsSuggestShutdownOrCleanup(combinedLogs) ? ' CLI output suggests the team was shut down / cleaned up, so no persisted config was left on disk.' @@ -35492,7 +35613,7 @@ export class TeamProvisioningService { ): Promise { const probes = run.teamsBasePathsToProbe.map((probe) => ({ ...probe, - configPath: path.join(probe.basePath, run.teamName, 'config.json'), + configPath: this.resolveSafeTeamStoragePath(probe.basePath, run.teamName, 'config.json'), })); const deadline = Date.now() + timeoutMs; @@ -35549,7 +35670,7 @@ export class TeamProvisioningService { if (run.expectedMembers.length === 0) { return []; } - const inboxDir = path.join(getTeamsBasePath(), run.teamName, 'inboxes'); + const inboxDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), run.teamName, 'inboxes'); const deadline = Date.now() + VERIFY_TIMEOUT_MS; let missing = new Set(run.expectedMembers); @@ -35559,7 +35680,12 @@ export class TeamProvisioningService { } const nextMissing = new Set(); for (const member of missing) { - const inboxPath = path.join(inboxDir, `${member}.json`); + const safeMemberName = this.normalizeSafeInboxBaseName(member); + if (!safeMemberName) { + nextMissing.add(member); + continue; + } + const inboxPath = this.resolveSafeInboxFilePath(inboxDir, `${safeMemberName}.json`); if (!(await this.pathExists(inboxPath))) { nextMissing.add(member); } @@ -35960,7 +36086,7 @@ export class TeamProvisioningService { * is interrupted. On failure, restorePrelaunchConfig() reverts to the backup. */ private async updateConfigProjectPath(teamName: string, cwd: string): Promise { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); try { const raw = await tryReadRegularFileUtf8(configPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, @@ -36142,7 +36268,7 @@ export class TeamProvisioningService { ): Promise { const MAX_SESSION_HISTORY = 5000; const MAX_PROJECT_PATH_HISTORY = 500; - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); try { const raw = await tryReadRegularFileUtf8(configPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, @@ -36229,7 +36355,7 @@ export class TeamProvisioningService { } private async cleanupCliAutoSuffixedMembers(teamName: string): Promise { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); const removedFromConfig: string[] = []; try { @@ -36370,7 +36496,7 @@ export class TeamProvisioningService { } private async assertConfigLeadOnlyForLaunch(teamName: string): Promise { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); const raw = await tryReadRegularFileUtf8(configPath, { timeoutMs: TEAM_JSON_READ_TIMEOUT_MS, maxBytes: TEAM_CONFIG_MAX_BYTES, @@ -36418,7 +36544,7 @@ export class TeamProvisioningService { } private async normalizeTeamConfigForLaunch(teamName: string, configRaw: string): Promise { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); const backupPath = `${configPath}.prelaunch.bak`; let parsed: unknown; @@ -36546,7 +36672,7 @@ export class TeamProvisioningService { * Restore config.json from prelaunch backup if launch fails after normalization. */ private async restorePrelaunchConfig(teamName: string): Promise { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); const backupPath = `${configPath}.prelaunch.bak`; try { const backupRaw = await tryReadRegularFileUtf8(backupPath, { @@ -36568,7 +36694,7 @@ export class TeamProvisioningService { * Remove the prelaunch backup file after a successful launch. */ async cleanupPrelaunchBackup(teamName: string): Promise { - const configPath = path.join(getTeamsBasePath(), teamName, 'config.json'); + const configPath = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'config.json'); const backupPath = `${configPath}.prelaunch.bak`; try { await fs.promises.unlink(backupPath); @@ -36583,7 +36709,7 @@ export class TeamProvisioningService { ): Promise { if (baseNames.size === 0) return; - const inboxDir = path.join(getTeamsBasePath(), teamName, 'inboxes'); + const inboxDir = this.resolveSafeTeamStoragePath(getTeamsBasePath(), teamName, 'inboxes'); let entries: string[]; try { entries = await fs.promises.readdir(inboxDir); @@ -36591,23 +36717,29 @@ export class TeamProvisioningService { return; } - const existing = new Set(entries.filter((e) => e.endsWith('.json') && !e.startsWith('.'))); + const existing = new Set( + entries.filter((entry) => this.isSafeInboxJsonFileName(entry) && !entry.startsWith('.')) + ); - for (const baseName of baseNames) { + for (const rawBaseName of baseNames) { + const baseName = this.normalizeSafeInboxBaseName(rawBaseName); + if (!baseName) { + continue; + } const canonicalFile = `${baseName}.json`; if (!existing.has(canonicalFile)) { continue; } - const duplicates = Array.from(existing) - .filter((file) => file.startsWith(`${baseName}-`) && file.endsWith('.json')) - .filter((file) => /-\d+\.json$/.test(file)); + const duplicates = Array.from(existing).filter((file) => + this.isSafeAutoSuffixedInboxDuplicate(file, baseName) + ); if (duplicates.length === 0) { continue; } - const canonicalPath = path.join(inboxDir, canonicalFile); + const canonicalPath = this.resolveSafeInboxFilePath(inboxDir, canonicalFile); let canonicalRaw: string; try { const raw = await tryReadRegularFileUtf8(canonicalPath, { @@ -36633,7 +36765,7 @@ export class TeamProvisioningService { const merged = [...canonicalList]; for (const dupFile of duplicates) { - const dupPath = path.join(inboxDir, dupFile); + const dupPath = this.resolveSafeInboxFilePath(inboxDir, dupFile); let dupRaw: string; try { const raw = await tryReadRegularFileUtf8(dupPath, { @@ -36700,7 +36832,7 @@ export class TeamProvisioningService { for (const dupFile of duplicates) { try { - await fs.promises.unlink(path.join(inboxDir, dupFile)); + await fs.promises.unlink(this.resolveSafeInboxFilePath(inboxDir, dupFile)); existing.delete(dupFile); } catch { // Best-effort cleanup. @@ -37696,7 +37828,7 @@ export class TeamProvisioningService { ); const teamName = 'mcp-validation-team'; const memberName = 'mcp-validation-member'; - const teamDir = path.join(claudeDir, 'teams', teamName); + const teamDir = resolveValidatedTeamStoragePath(path.join(claudeDir, 'teams'), teamName); await fs.promises.mkdir(teamDir, { recursive: true }); await fs.promises.writeFile( diff --git a/src/main/utils/childProcess.ts b/src/main/utils/childProcess.ts index e489b250..29f94bba 100644 --- a/src/main/utils/childProcess.ts +++ b/src/main/utils/childProcess.ts @@ -1,9 +1,7 @@ import { type ChildProcess, - exec, execFile, type ExecFileOptions, - type ExecOptions, spawn, type SpawnOptions, spawnSync, @@ -80,65 +78,14 @@ function execFileAsync( } /** - * Promise wrapper for exec. Used exclusively as a Windows shell fallback - * when execFile fails with EINVAL on non-ASCII binary paths. The command - * string is built from a known binary path + args, NOT from user input. + * cmd.exe fallback implemented through execFile so Node does not invoke an + * additional shell around the guarded command string. */ function execShellAsync( cmd: string, - options: ExecOptions = {} + options: ExecFileOptions = {} ): Promise<{ stdout: string; stderr: string }> { - return new Promise((resolve, reject) => { - const { timeout, killSignal, ...execOptions } = options; - const timeoutMs = typeof timeout === 'number' && timeout > 0 ? timeout : 0; - const timeoutSignal = normalizeKillSignal(killSignal); - let child: ChildProcess | null = null; - let settled = false; - let stdoutText = ''; - let stderrText = ''; - let timeoutHandle: ReturnType | null = null; - // eslint-disable-next-line sonarjs/os-command, security/detect-child-process -- cmd from known binaryPath+args, not user input (Windows EINVAL fallback) - child = exec(cmd, execOptions, (err, stdout, stderr) => { - if (settled) { - return; - } - settled = true; - timeoutHandle = cleanupTimedCliProcess(child, timeoutHandle); - if (err) - reject( - err instanceof Error ? err : new Error(typeof err === 'string' ? err : 'Unknown error') - ); - else resolve({ stdout: String(stdout), stderr: String(stderr) }); - }); - if (!settled) { - trackCliProcess(child); - if (timeoutMs > 0) { - child.stdout?.on('data', (chunk: Buffer | string) => { - stdoutText += chunk.toString(); - }); - child.stderr?.on('data', (chunk: Buffer | string) => { - stderrText += chunk.toString(); - }); - timeoutHandle = setTimeout(() => { - if (settled) { - return; - } - settled = true; - timeoutHandle = cleanupTimedCliProcess(child, timeoutHandle); - killProcessTree(child, timeoutSignal); - const error = new Error(`Command timed out after ${timeoutMs}ms: ${cmd}`); - Object.assign(error, { - killed: true, - signal: timeoutSignal, - stdout: stdoutText, - stderr: stderrText, - }); - reject(error); - }, timeoutMs); - timeoutHandle.unref?.(); - } - } - }); + return execFileAsync(getWindowsCmdPath(), ['/d', '/s', '/c', cmd], options); } function cleanupTimedCliProcess( @@ -300,6 +247,48 @@ function quoteArg(arg: string): string { return quoteWindowsCmdArg(arg); } +const WINDOWS_SHELL_UNSAFE_META_CHAR_RE = /[&|<>^]/u; + +function containsWindowsShellUnsafeControlChar(part: string): boolean { + for (let index = 0; index < part.length; index += 1) { + const code = part.charCodeAt(index); + if (code <= 0x1f || (code >= 0x7f && code <= 0x9f)) { + return true; + } + } + return false; +} + +function assertSafeWindowsShellFallbackPart(part: string): void { + if (containsWindowsShellUnsafeControlChar(part)) { + throw new Error('Unsafe Windows shell fallback argument: control characters are not allowed'); + } + if (WINDOWS_SHELL_UNSAFE_META_CHAR_RE.test(part)) { + throw new Error('Unsafe Windows shell fallback argument: shell metacharacters are not allowed'); + } +} + +function buildWindowsShellFallbackCommand(parts: string[]): string { + for (const part of parts) { + assertSafeWindowsShellFallbackPart(part); + } + return parts.map(quoteArg).join(' '); +} + +function getWindowsCmdPath(): string { + return path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'cmd.exe'); +} + +function spawnWindowsShellFallback( + cmd: string, + options: ReturnType> +): ReturnType { + return spawn(getWindowsCmdPath(), ['/d', '/s', '/c', cmd], { + ...options, + shell: false, + }); +} + /** Env vars injected into every spawned Claude CLI process. */ const CLI_ENV_DEFAULTS: Record = { CLAUDE_HOOK_JUDGE_MODE: 'true', @@ -408,8 +397,8 @@ export async function execCli( } // shell fallback (Windows only; others shouldn't reach here) - const cmd = [target, ...args].map(quoteArg).join(' '); - const shellResult = await execShellAsync(cmd, opts as unknown as ExecOptions); + const cmd = buildWindowsShellFallbackCommand([target, ...args]); + const shellResult = await execShellAsync(cmd, opts); return { stdout: String(shellResult.stdout), stderr: String(shellResult.stderr) }; } @@ -435,9 +424,8 @@ export function spawnCli( } if (process.platform === 'win32' && needsShell(binaryPath)) { - const cmd = [binaryPath, ...args].map(quoteArg).join(' '); - // eslint-disable-next-line sonarjs/os-command -- cmd from known binaryPath+args, not user input (Windows EINVAL fallback) - return trackCliProcess(spawn(cmd, { ...opts, shell: true })); + const cmd = buildWindowsShellFallbackCommand([binaryPath, ...args]); + return trackCliProcess(spawnWindowsShellFallback(cmd, opts)); } try { @@ -446,9 +434,8 @@ export function spawnCli( const code = err && typeof err === 'object' && 'code' in err ? (err as { code?: string }).code : undefined; if (process.platform === 'win32' && code === 'EINVAL') { - const cmd = [binaryPath, ...args].map(quoteArg).join(' '); - // eslint-disable-next-line sonarjs/os-command -- cmd from known binaryPath+args, not user input (Windows EINVAL fallback) - return trackCliProcess(spawn(cmd, { ...opts, shell: true })); + const cmd = buildWindowsShellFallbackCommand([binaryPath, ...args]); + return trackCliProcess(spawnWindowsShellFallback(cmd, opts)); } throw err; } diff --git a/test/main/services/team/TeamProvisioningService.test.ts b/test/main/services/team/TeamProvisioningService.test.ts index 0494ac95..3837ad3a 100644 --- a/test/main/services/team/TeamProvisioningService.test.ts +++ b/test/main/services/team/TeamProvisioningService.test.ts @@ -606,6 +606,13 @@ function createMemberSpawnStatusEntry( } type TeamProvisioningServicePrivateHarness = { + resolveProjectClaudeSettingsPath: (projectCwd: string) => Promise; + resolveSafeTeamStoragePath: ( + basePath: string, + teamName: string, + ...segments: string[] + ) => string; + mergeAndRemoveDuplicateInboxes: (teamName: string, baseNames: Set) => Promise; getLiveTeamAgentRuntimeMetadata: ( teamName: string ) => Promise>>; @@ -17946,6 +17953,112 @@ describe('TeamProvisioningService', () => { expect(settings.permissions?.allow).toEqual(['mcp__agent-teams__team_stop']); }); + it('resolves project Claude local settings only inside an absolute project cwd', async () => { + const svc = new TeamProvisioningService(); + const settingsPath = await privateHarness(svc).resolveProjectClaudeSettingsPath(tempClaudeRoot); + + expect(settingsPath).toBe( + path.join(fs.realpathSync(tempClaudeRoot), '.claude', 'settings.local.json') + ); + fs.writeFileSync(settingsPath, '{}', 'utf8'); + expect(fs.existsSync(path.join(tempClaudeRoot, '.claude', 'settings.local.json'))).toBe(true); + }); + + it('rejects unsafe project cwd values for local settings writes', async () => { + const svc = new TeamProvisioningService(); + const harness = privateHarness(svc); + const filePath = path.join(tempClaudeRoot, 'not-a-directory'); + fs.writeFileSync(filePath, 'x', 'utf8'); + + await expect(harness.resolveProjectClaudeSettingsPath('relative/project')).rejects.toThrow( + 'absolute path' + ); + await expect( + harness.resolveProjectClaudeSettingsPath(path.join(tempClaudeRoot, 'missing')) + ).rejects.toThrow('does not exist'); + await expect(harness.resolveProjectClaudeSettingsPath(filePath)).rejects.toThrow( + 'not a directory' + ); + }); + + it('rejects .claude symlink escapes for local settings writes', async () => { + const svc = new TeamProvisioningService(); + const harness = privateHarness(svc); + const projectDir = path.join(tempClaudeRoot, 'project-with-symlink'); + const outsideDir = path.join(tempClaudeRoot, 'outside-claude'); + fs.mkdirSync(projectDir, { recursive: true }); + fs.mkdirSync(outsideDir, { recursive: true }); + fs.symlinkSync( + outsideDir, + path.join(projectDir, '.claude'), + process.platform === 'win32' ? 'junction' : 'dir' + ); + + await expect(harness.resolveProjectClaudeSettingsPath(projectDir)).rejects.toThrow( + 'outside project cwd' + ); + }); + + it('resolves team storage paths only for validated team names', () => { + const svc = new TeamProvisioningService(); + const harness = privateHarness(svc); + + expect(harness.resolveSafeTeamStoragePath(tempTeamsBase, 'safe-team', 'config.json')).toBe( + path.join(tempTeamsBase, 'safe-team', 'config.json') + ); + expect(() => harness.resolveSafeTeamStoragePath(tempTeamsBase, '../bad')).toThrow( + /teamName contains invalid characters/i + ); + expect(() => harness.resolveSafeTeamStoragePath(tempTeamsBase, 'bad/name')).toThrow( + /teamName contains invalid characters/i + ); + expect(() => harness.resolveSafeTeamStoragePath(tempTeamsBase, 'bad\\name')).toThrow( + /teamName contains invalid characters/i + ); + }); + + it('cleans only expected auto-suffixed inbox duplicates', async () => { + const svc = new TeamProvisioningService(); + const harness = privateHarness(svc); + const teamName = 'safe-inbox-cleanup'; + const inboxDir = path.join(tempTeamsBase, teamName, 'inboxes'); + fs.mkdirSync(inboxDir, { recursive: true }); + fs.writeFileSync( + path.join(inboxDir, 'alice.json'), + JSON.stringify([{ messageId: 'm1', timestamp: '2026-01-01T00:00:00.000Z' }]), + 'utf8' + ); + fs.writeFileSync( + path.join(inboxDir, 'alice-2.json'), + JSON.stringify([{ messageId: 'm2', timestamp: '2026-01-02T00:00:00.000Z' }]), + 'utf8' + ); + fs.writeFileSync(path.join(inboxDir, 'alice-x.json'), '[]', 'utf8'); + fs.writeFileSync(path.join(inboxDir, 'bob-2.json'), '[]', 'utf8'); + + await harness.mergeAndRemoveDuplicateInboxes(teamName, new Set(['alice', '../escape'])); + + const merged = JSON.parse(fs.readFileSync(path.join(inboxDir, 'alice.json'), 'utf8')) as { + messageId?: string; + }[]; + expect(merged.map((message) => message.messageId)).toEqual(['m2', 'm1']); + expect(fs.existsSync(path.join(inboxDir, 'alice-2.json'))).toBe(false); + expect(fs.existsSync(path.join(inboxDir, 'alice-x.json'))).toBe(true); + expect(fs.existsSync(path.join(inboxDir, 'bob-2.json'))).toBe(true); + }); + + it('rejects unsafe team names before inbox cleanup paths are built', async () => { + const svc = new TeamProvisioningService(); + const harness = privateHarness(svc); + + await expect( + harness.mergeAndRemoveDuplicateInboxes('../bad', new Set(['alice'])) + ).rejects.toThrow(/teamName contains invalid characters/i); + await expect( + harness.mergeAndRemoveDuplicateInboxes('bad\\name', new Set(['alice'])) + ).rejects.toThrow(/teamName contains invalid characters/i); + }); + it('builds teammate AskUserQuestion permission responses with answers', () => { const svc = new TeamProvisioningService(); const toolInput = { diff --git a/test/main/utils/childProcess.test.ts b/test/main/utils/childProcess.test.ts index 3ed089db..76f2458b 100644 --- a/test/main/utils/childProcess.test.ts +++ b/test/main/utils/childProcess.test.ts @@ -199,13 +199,18 @@ describe('cli child process helpers', () => { env: { FOO: 'bar' }, }); expect(spawnMock).toHaveBeenCalledTimes(2); - const secondArg0 = spawnMock.mock.calls[1][0] as string; - expect(secondArg0).toMatch(/claude\.exe/); - expect(spawnMock.mock.calls[1][1]).toMatchObject({ shell: true, env: { FOO: 'bar' } }); + expect(spawnMock.mock.calls[1][0]).toMatch(/cmd\.exe$/i); + expect(spawnMock.mock.calls[1][1]).toEqual([ + '/d', + '/s', + '/c', + expect.stringMatching(/claude\.exe/), + ]); + expect(spawnMock.mock.calls[1][2]).toMatchObject({ shell: false, env: { FOO: 'bar' } }); expect(result).toBe(fake); }); - it('uses shell directly for Windows cmd launchers', () => { + it('uses cmd.exe directly for Windows cmd launcher shell fallback', () => { setPlatform('win32'); const fake = createMockProcess(); const spawnMock = child.spawn as unknown as Mock; @@ -213,8 +218,14 @@ describe('cli child process helpers', () => { const result = spawnCli('C:\\runtime\\cli-dev.cmd', ['--version']); expect(spawnMock).toHaveBeenCalledTimes(1); - expect(spawnMock.mock.calls[0][0]).toContain('cli-dev.cmd'); - expect(spawnMock.mock.calls[0][1]).toMatchObject({ shell: true }); + expect(spawnMock.mock.calls[0][0]).toMatch(/cmd\.exe$/i); + expect(spawnMock.mock.calls[0][1]).toEqual([ + '/d', + '/s', + '/c', + expect.stringContaining('cli-dev.cmd'), + ]); + expect(spawnMock.mock.calls[0][2]).toMatchObject({ shell: false }); expect(result).toBe(fake); }); @@ -282,14 +293,57 @@ describe('cli child process helpers', () => { const result = spawnCli('C:\\Users\\Алексей\\AppData\\Roaming\\npm\\claude.cmd', ['a', 'b'], { env: { FOO: 'bar' }, }); - // Non-ASCII detected upfront — single spawn call with shell: true + // Non-ASCII detected upfront, so launch through cmd.exe fallback once. expect(spawnMock).toHaveBeenCalledTimes(1); - const shellCmd = spawnMock.mock.calls[0][0] as string; + expect(spawnMock.mock.calls[0][0]).toMatch(/cmd\.exe$/i); + const shellCmd = spawnMock.mock.calls[0][1][3] as string; expect(shellCmd).toMatch(/claude\.cmd/); - expect(spawnMock.mock.calls[0][1]).toMatchObject({ shell: true, env: { FOO: 'bar' } }); + expect(spawnMock.mock.calls[0][2]).toMatchObject({ shell: false, env: { FOO: 'bar' } }); expect(result).toBe(fake); }); + it('rejects control characters only when Windows shell fallback is needed', () => { + setPlatform('win32'); + const spawnMock = child.spawn as unknown as Mock; + spawnMock.mockReturnValue(createMockProcess()); + + for (const unsafeArg of [ + 'safe\0bad', + 'safe\rbad', + 'safe\nbad', + 'safe\u001fbad', + 'safe\u0085bad', + ]) { + expect(() => spawnCli('C:\\Users\\Алексей\\bin\\claude.cmd', [unsafeArg])).toThrow( + 'control characters are not allowed' + ); + } + expect(spawnMock).not.toHaveBeenCalled(); + + spawnCli('C:\\bin\\claude.exe', ['safe\nargv']); + expect(spawnMock.mock.calls[0][0]).toBe('C:\\bin\\claude.exe'); + expect(spawnMock.mock.calls[0][1]).toEqual(['safe\nargv']); + expect(spawnMock.mock.calls[0][2]).not.toHaveProperty('shell'); + }); + + it('rejects shell metacharacters only when Windows shell fallback is needed', () => { + setPlatform('win32'); + const spawnMock = child.spawn as unknown as Mock; + spawnMock.mockReturnValue(createMockProcess()); + + for (const unsafeArg of ['safe&bad', 'safe|bad', 'safebad', 'safe^bad']) { + expect(() => spawnCli('C:\\Users\\Алексей\\bin\\claude.cmd', [unsafeArg])).toThrow( + 'shell metacharacters are not allowed' + ); + } + expect(spawnMock).not.toHaveBeenCalled(); + + spawnCli('C:\\bin\\claude.exe', ['safe&argv']); + expect(spawnMock.mock.calls[0][0]).toBe('C:\\bin\\claude.exe'); + expect(spawnMock.mock.calls[0][1]).toEqual(['safe&argv']); + expect(spawnMock.mock.calls[0][2]).not.toHaveProperty('shell'); + }); + it('does not use shell when not on windows', () => { setPlatform('linux'); const fake = createMockProcess(); @@ -387,18 +441,25 @@ describe('cli child process helpers', () => { expect(execFileMock.mock.calls[1][2]).toMatchObject({ windowsHide: false }); }); - it('skips straight to shell for Windows cmd launchers', async () => { + it('skips straight to cmd.exe fallback for Windows cmd launchers', 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) => { + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, '0.0.8', ''); return createMockProcess(); - }); + } + ); const result = await execCli('C:\\runtime\\cli-dev.cmd', ['--version']); - expect(execFileMock).not.toHaveBeenCalled(); - expect(execMock).toHaveBeenCalled(); + expect(execFileMock).toHaveBeenCalledWith( + expect.stringMatching(/cmd\.exe$/i), + ['/d', '/s', '/c', expect.stringContaining('cli-dev.cmd')], + expect.any(Object), + expect.any(Function) + ); + expect(execMock).not.toHaveBeenCalled(); expect(result.stdout).toBe('0.0.8'); }); @@ -429,19 +490,22 @@ describe('cli child process helpers', () => { 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) => { + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); return createMockProcess(); - }); + } + ); const { dir, launcher } = createGeneratedBunLauncher(); try { const result = await execCli(launcher, ['runtime', 'opencode-command'], { preferShellForWindowsBatch: true, }); - expect(execFileMock).not.toHaveBeenCalled(); - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock.mock.calls[0][0]).toContain('runtime'); - expect(execMock.mock.calls[0][0]).toContain('opencode-command'); + expect(execFileMock).toHaveBeenCalledTimes(1); + expect(execFileMock.mock.calls[0][0]).toMatch(/cmd\.exe$/i); + expect(execFileMock.mock.calls[0][1][3]).toContain('runtime'); + expect(execFileMock.mock.calls[0][1][3]).toContain('opencode-command'); + expect(execMock).not.toHaveBeenCalled(); expect(result.stdout).toBe('ok'); } finally { rmSync(dir, { recursive: true, force: true }); @@ -499,30 +563,38 @@ describe('cli child process helpers', () => { 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) => { + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, '1.2.3', ''); return createMockProcess(); - }); + } + ); const result = await execCli('C:\\Users\\Алексей\\AppData\\Roaming\\npm\\claude.cmd', [ '--version', ]); - // non-ASCII path detected upfront — execFile should NOT be called - expect(execFileMock).not.toHaveBeenCalled(); - expect(execMock).toHaveBeenCalled(); + expect(execFileMock).toHaveBeenCalledWith( + expect.stringMatching(/cmd\.exe$/i), + ['/d', '/s', '/c', expect.stringContaining('claude.cmd')], + expect.any(Object), + expect.any(Function) + ); + expect(execMock).not.toHaveBeenCalled(); expect(result.stdout).toBe('1.2.3'); }); it('escapes percent signs and quotes for cmd.exe in shell fallback', async () => { setPlatform('win32'); - const execMock = child.exec as unknown as Mock; - execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { + const execFileMock = child.execFile as unknown as Mock; + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); return createMockProcess(); - }); + } + ); await execCli('C:\\Users\\Алексей\\bin\\claude.cmd', ['--model', 'test%PATH%"arg']); - const shellCmd = execMock.mock.calls[0][0] as string; + const shellCmd = execFileMock.mock.calls[0][1][3] as string; // Keep % outside quoted chunks so cmd.exe does not expand it as an env var. expect(shellCmd).toContain('^%"PATH"^%'); expect(shellCmd).not.toContain('%PATH%'); @@ -534,11 +606,13 @@ describe('cli child process helpers', () => { it('keeps inline settings JSON as one argv-safe argument for Windows cmd launchers', async () => { setPlatform('win32'); - const execMock = child.exec as unknown as Mock; - execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { + const execFileMock = child.execFile as unknown as Mock; + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { cb(null, 'ok', ''); return createMockProcess(); - }); + } + ); await execCli('C:\\runtime\\cli-dev.cmd', [ '--settings', @@ -549,25 +623,25 @@ describe('cli child process helpers', () => { '--provider', 'codex', ]); - const shellCmd = execMock.mock.calls[0][0] as string; + const shellCmd = execFileMock.mock.calls[0][1][3] as string; expect(shellCmd).toContain('"{\\"codex\\":{\\"forced_login_method\\":\\"chatgpt\\"}}"'); expect(shellCmd).not.toContain('{""codex"":'); }); - it('shell: true cannot be overridden by caller options', () => { + it('does not pass caller shell options into cmd.exe fallback', () => { setPlatform('win32'); const spawnMock = child.spawn as unknown as Mock; spawnMock.mockReturnValue(createMockProcess()); - 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 }); + spawnCli('C:\\Users\\Алексей\\bin\\claude.cmd', ['--version'], { shell: true }); + expect(spawnMock.mock.calls[0][0]).toMatch(/cmd\.exe$/i); + expect(spawnMock.mock.calls[0][2]).toMatchObject({ shell: false }); }); it('falls back to shell when execFile throws EINVAL on windows', async () => { setPlatform('win32'); const execFileMock = child.execFile as unknown as Mock; - execFileMock.mockImplementation( + execFileMock.mockImplementationOnce( (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { const err = new Error('spawn EINVAL') as Error & { code?: string }; err.code = 'EINVAL'; @@ -575,19 +649,48 @@ describe('cli child process helpers', () => { return createMockProcess(); } ); - const execMock = child.exec as unknown as Mock; - execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { - cb(null, '2.3.4', ''); - return createMockProcess(); - }); + execFileMock.mockImplementationOnce( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { + cb(null, '2.3.4', ''); + return createMockProcess(); + } + ); // ASCII path — goes through execFile first, gets EINVAL, falls back to shell const result = await execCli('C:\\bin\\claude.exe', ['--version']); - expect(execFileMock).toHaveBeenCalled(); - expect(execMock).toHaveBeenCalled(); + expect(execFileMock).toHaveBeenCalledTimes(2); + expect(execFileMock.mock.calls[1][0]).toMatch(/cmd\.exe$/i); expect(result.stdout).toBe('2.3.4'); }); + it('rejects control characters when execCli needs Windows shell fallback', async () => { + setPlatform('win32'); + const execFileMock = child.execFile as unknown as Mock; + execFileMock.mockImplementationOnce( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { + const err = new Error('spawn EINVAL') as Error & { code?: string }; + err.code = 'EINVAL'; + cb(err, '', ''); + return createMockProcess(); + } + ); + + await expect(execCli('C:\\bin\\claude.exe', ['safe\rbad'])).rejects.toThrow( + 'control characters are not allowed' + ); + expect(execFileMock).toHaveBeenCalledTimes(1); + }); + + it('rejects shell metacharacters when execCli needs Windows shell fallback', async () => { + setPlatform('win32'); + const execFileMock = child.execFile as unknown as Mock; + + await expect( + execCli('C:\\Users\\Алексей\\bin\\claude.cmd', ['safe&bad']) + ).rejects.toThrow('shell metacharacters are not allowed'); + expect(execFileMock).not.toHaveBeenCalled(); + }); + it('preserves stdout and stderr on execFile failures', async () => { setPlatform('linux'); const execFileMock = child.execFile as unknown as Mock;