Refactor XcodeBuildMCP into a CLI first tool #197
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Log the actual command that will be executed | ||
| const displayCommand = | ||
| useShell && escapedCommand.length === 3 ? escapedCommand[2] : [executable, ...args].join(' '); | ||
| log('info', `Executing ${logPrefix ?? ''} command: ${displayCommand}`); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 days ago
General approach: avoid running paths derived from process.cwd(), configuration, or PATH through a shell command string. Instead, pass the executable path and its arguments directly to spawn, letting Node escape/handle them without shell interpretation. For functionality that currently calls the executor with useShell: true for simple command + args, switch to useShell: false. This completely sidesteps the need to manually quote arguments and eliminates shell injection from tainted paths.
Best concrete fix here: the problematic flow originates from isAxeAtLeastVersion, which calls the executor with true for useShell. That causes [axePath, '--version'] to be converted into a single /bin/sh -c command string, which is what CodeQL flags at spawn(executable, args, spawnOpts). There is no need for shell semantics when running axe --version; it is just an executable with one argument. Changing that call to use false (or omit the flag and rely on the default) ensures that defaultExecutor invokes spawn(axePath, ['--version'], spawnOpts) without the intermediate shell command string, removing the vulnerability. This change preserves all existing functionality: the same executable is run with the same argument, we just stop involving /bin/sh -c.
Concretely:
- In
src/utils/axe-helpers.ts, editisAxeAtLeastVersionso that it callsexec([axePath, '--version'], 'AXe Version', false)instead of passingtrue. No other logic changes are needed. - No changes are required in
src/utils/command.tsitself, because the non-shell path is already safe.
| @@ -162,7 +162,8 @@ | ||
|
|
||
| const exec = executor ?? getDefaultCommandExecutor(); | ||
| try { | ||
| const res = await exec([axePath, '--version'], 'AXe Version', true); | ||
| // Use direct execution without a shell to avoid shell interpretation of the axe path. | ||
| const res = await exec([axePath, '--version'], 'AXe Version', false); | ||
| if (!res.success) return false; | ||
|
|
||
| const output = res.output ?? ''; |
commit: |
1 participant
Breaking
mcpargument must be passedNew
Note
Medium Risk
Medium risk due to a breaking invocation change (
xcodebuildmcp mcp) and updates to CLI/daemon startup + command spawning that can affect how tools execute and connect to the daemon.Overview
Shifts
xcodebuildmcpto a unified CLI-first interface where MCP server mode is started explicitly via themcpsubcommand, and updates all user/developer docs and install snippets to passmcp(including quick-install links and testing commands).Adds a new
docs/CLI.mdguide describing direct tool invocation, daemon lifecycle commands, and per-workspace daemon behavior; updates architecture docs accordingly.Includes several CLI/daemon fixes called out in the changelog: honor
XCODEBUILDMCP_SOCKEToverrides when auto-starting the daemon, avoid daemon crashes by stopping log-file output after stream errors, and stop routing tool commands throughshby default (preventingspawn sh ENOENT).Written by Cursor Bugbot for commit b04c37a. This will update automatically on new commits. Configure here.