diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c9fbbbe..e4677be9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: pull_request: branches: [main] +permissions: + contents: read + jobs: build-and-test: runs-on: ubuntu-latest diff --git a/.github/workflows/sentry.yml b/.github/workflows/sentry.yml index cd14ac86..61e627e2 100644 --- a/.github/workflows/sentry.yml +++ b/.github/workflows/sentry.yml @@ -4,6 +4,9 @@ on: tags: - 'v*' +permissions: + contents: read + jobs: release: runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 3abdc60f..b09664c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Fixed `stop_mac_app` app-name targeting so it no longer terminates unrelated processes whose command lines contain the app name, and reject unsafe process IDs before execution ([#306](https://github.com/getsentry/XcodeBuildMCP/issues/306)). - Fixed incremental `xcodemake` builds when DerivedData uses an absolute path by updating the pinned wrapper and delegating Makefile reuse to it so its argument and freshness checks are always applied ([#466](https://github.com/getsentry/XcodeBuildMCP/issues/466)). - Fixed the scheduled Warden sweep to authenticate through OpenRouter and track the current v0 action release ([#483](https://github.com/getsentry/XcodeBuildMCP/issues/483)). +- Fixed shell-mode command execution so environment-derived arguments never become shell source, and restricted CI and Sentry release workflows to read-only repository access. ## [2.6.2] diff --git a/src/utils/__tests__/command.test.ts b/src/utils/__tests__/command.test.ts index 4b03ebeb..cc93b81d 100644 --- a/src/utils/__tests__/command.test.ts +++ b/src/utils/__tests__/command.test.ts @@ -1,7 +1,69 @@ +import { chmod, mkdtemp, rm, writeFile } from 'fs/promises'; +import { tmpdir } from 'os'; +import { join } from 'path'; import { describe, expect, it } from 'vitest'; import { __getRealCommandExecutor } from '../command.ts'; describe('defaultExecutor', () => { + it('passes arguments literally when shell execution is requested', async () => { + const executor = __getRealCommandExecutor(); + const argumentsWithMetacharacters = [ + '$(printf injected)', + '`printf injected`', + 'value; printf injected', + '$HOME', + "single'quote", + ]; + + const result = await executor( + ['/usr/bin/printf', '%s\n', ...argumentsWithMetacharacters], + 'Shell Argument Test', + true, + ); + + expect(result).toMatchObject({ + success: true, + exitCode: 0, + output: `${argumentsWithMetacharacters.join('\n')}\n`, + }); + }); + + it('treats a leading-dash executable as a command name when shell execution is requested', async () => { + const executableDirectory = await mkdtemp(join(tmpdir(), 'xcodebuildmcp-command-')); + const executablePath = join(executableDirectory, '-c'); + await writeFile(executablePath, '#!/bin/sh\nprintf "%s\\n" "$@"\n', 'utf8'); + await chmod(executablePath, 0o700); + + try { + const executor = __getRealCommandExecutor(); + const result = await executor(['-c', 'literal argument'], 'Shell Executable Test', true, { + env: { PATH: executableDirectory }, + }); + + expect(result).toMatchObject({ + success: true, + exitCode: 0, + output: 'literal argument\n', + }); + } finally { + await rm(executableDirectory, { recursive: true, force: true }); + } + }); + + it('returns an exit response when a shell-mode executable is missing', async () => { + const executor = __getRealCommandExecutor(); + const result = await executor( + ['xcodebuildmcp-command-that-does-not-exist'], + 'Missing Shell Executable Test', + true, + ); + + expect(result).toMatchObject({ + success: false, + exitCode: 127, + }); + }); + it('settles after exit even when child close is delayed', async () => { const executor = __getRealCommandExecutor(); const startedAt = Date.now(); diff --git a/src/utils/command.ts b/src/utils/command.ts index 0c53719c..54f889f2 100644 --- a/src/utils/command.ts +++ b/src/utils/command.ts @@ -18,17 +18,15 @@ async function defaultExecutor( opts?: CommandExecOptions, detached: boolean = false, ): Promise { - let escapedCommand = command; - if (useShell) { - const commandString = command.map((arg) => shellEscapeArg(arg)).join(' '); + let executable = command[0]; + let args = command.slice(1); - escapedCommand = ['/bin/sh', '-c', commandString]; + if (useShell) { + executable = '/usr/bin/env'; + args = ['--', ...command]; } return new Promise((resolve, reject) => { - let executable = escapedCommand[0]; - let args = escapedCommand.slice(1); - if (!useShell && executable === 'xcodebuild') { const xcrunPath = '/usr/bin/xcrun'; if (existsSync(xcrunPath)) { @@ -37,8 +35,9 @@ async function defaultExecutor( } } - const displayCommand = - useShell && escapedCommand.length === 3 ? escapedCommand[2] : [executable, ...args].join(' '); + const displayCommand = useShell + ? command.map((arg) => shellEscapeArg(arg)).join(' ') + : [executable, ...args].join(' '); log('debug', `Executing ${logPrefix ?? ''} command: ${displayCommand}`); const emitTranscript = transcriptEmitterStorage.getStore();