getsentry/XcodeBuildMCP PR #289 hardens shell escaping in MCP tool execution

getsentry/XcodeBuildMCP accepted MCP tool parameters that could reach /bin/sh -c through unsafe shell escaping. PR #289 fixes the central useShell=true path by replacing hand-rolled double-quote wrapping with POSIX single-quote escaping, then hardens two adjacent injection surfaces in log predicates and generated TypeScript source.

I submitted the fix, regression tests and documentation for the remaining manual shell construction sites. The PR was merged on 26 April 2026.

The vulnerable path

XcodeBuildMCP is an MCP server for driving Xcode build, test, simulator and project-discovery workflows from an AI client. That design gives the MCP server local developer privileges by intention. It can call xcodebuild, inspect app bundles, launch simulators and read project state. The security question is whether tool parameters remain data as they pass through those operations.

The command injection issue sat in platform detection. MCP tool handlers such as build_run_sim, build_sim, test_sim and simulator-defaults-refresh accepted parameters including scheme, projectPath and workspacePath as plain z.string() values. Those values flowed into inferPlatform(), then into detectPlatformFromScheme() in src/utils/platform-detection.ts.

At platform-detection.ts:84, the platform detector invoked the executor roughly as follows:

executor([
  "xcodebuild",
  "-showBuildSettings",
  "-scheme",
  scheme,
  "-project",
  path,
], "Platform Detection", true)

The final true was the important part. It selected the shell path in defaultExecutor() rather than the normal argument-array path. The old implementation built a command string by wrapping every argument in double quotes, escaping only " and \\, then passing the result to /bin/sh -c.

That is not shell escaping. Inside POSIX double quotes, command substitution still runs. A value such as $(id) remains active. Backtick substitution remains active too. The argument looks quoted to a reader but remains executable to the shell.

The data flow was therefore simple:

1. An MCP client supplies a scheme, projectPath or workspacePath string.
2. The tool handler accepts it without content restrictions.
3. Platform detection passes it into an xcodebuild command with useShell=true.
4. The executor converts the argument array into a shell command string.
5. /bin/sh -c evaluates command substitution before xcodebuild receives its arguments.

The practical impact was CWE-78: OS command injection with the privileges of the developer running the MCP server.

Why the MCP boundary mattered

This was not a remotely exposed HTTP service. XcodeBuildMCP uses stdio transport, so an attacker does not simply connect over the network and send a command. That narrows the exposure but does not remove it.

The realistic attack path is agent-mediated input. A malicious repository, issue, README, build log or prompt could influence an AI client into calling an MCP tool with attacker-controlled parameters. If the client auto-approves tool calls, or if the user approves a plausible build action without inspecting every argument, the payload reaches the local toolchain.

That model is now common enough that NVD has already recorded MCP-specific command injection cases. CVE-2025-5277 describes an aws-mcp-server issue where a crafted prompt accessed by an MCP client could run arbitrary commands on the host. CVE-2025-52573 covers command injection in an iOS simulator MCP server before version 1.3.3. The edge varies by project, but the shape is consistent: the model is persuaded to call a tool, the tool treats the model's parameters as trusted input and the sink is a local privileged operation.

The Model Context Protocol security guidance emphasises user consent and control over tool execution. That is necessary, but it is not a substitute for input handling at the sink. Consent prompts are a user-interface boundary. Shell parsing is a language boundary. Confusing the two is how local developer tooling becomes a command execution surface with a friendly chat box in front of it.

Invariant Labs' work on MCP tool poisoning makes the same broader point from another angle: the tool layer is part of the prompt surface. If tool descriptions, repository content or surrounding context can shape which tool is called and with which arguments, then every parser behind that tool must assume hostile input.

What PR #289 changes

The central fix is small and deliberately boring. PR #289 adds src/utils/shell-escape.ts and changes src/utils/command.ts so the shell path uses POSIX single-quote escaping:

command.map((arg) => shellEscapeArg(arg)).join(" ")

Single-quoted strings in POSIX shells do not perform variable expansion, command substitution or metacharacter processing. Embedded single quotes are represented by closing the quote, adding an escaped literal quote and reopening the quote. This is the same basic approach used by shlex.quote() in Python and Shellwords.shellescape() in Ruby.

The difference matters. Double-quote wrapping tries to preserve spaces while leaving parts of the shell language alive. Single-quote escaping makes the argument inert, except for the carefully handled single quote case itself.

The regression tests in shell-escape.test.ts covered simple strings, empty input, embedded single quotes, $(), backticks, semicolons, pipes, newlines, backslashes and realistic malicious app paths. That coverage is important because shell escaping bugs usually hide in the cases that look like punctuation rather than data.

Predicate interpolation was a separate injection surface

The PR also hardened src/utils/log_capture.ts. This was not OS command injection because the predicate is passed as an argument with useShell=false. It was still injection, just at the NSPredicate layer.

Before the patch, bundleId and custom subsystem filters were interpolated into double-quoted NSPredicate string literals. A value like this could break out of the intended string context:

const maliciousBundleId = 'io.evil" OR 1==1 OR subsystem == "x';

The fix adds escapePredicateString(), escaping backslashes before double quotes, then uses it for values placed inside predicate string literals. The order matters because escaping quotes first and backslashes second can corrupt the escape sequence just created.

The regression tests validate the expected predicate form. The malicious bundle ID remains inside one string literal:

subsystem == "io.evil\" OR 1==1 OR subsystem == \"x"

That is the right scope for the fix. Shell escaping would be irrelevant here because the shell is not the parser receiving the string. NSPredicate escaping is the parser-specific control.

Version generation needed source-level escaping

The third change was in scripts/generate-version.ts. The version generator read values from package.json and emitted src/version.ts. Before the patch, values were interpolated directly into single-quoted TypeScript source strings:

`export const version = '${pkg.version}';\n`

If a package metadata value were compromised, that could turn data into generated source code. The merged fix adds two layers.

First, version fields are validated with a strict semver-like whitelist at lines 19 to 28 of the diff:

const VERSION_REGEX = /^v?[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.\-]+)?(\+[a-zA-Z0-9.\-]+)?$/;

Second, all emitted string literals use JSON.stringify() at lines 44 to 53, including packageName, repositoryOwner and repositoryName after the rebase reconciliation:

`export const version = ${JSON.stringify(pkg.version)};\n`

Validation limits what version strings can be. JSON.stringify() makes the generated code safe even if a future field contains quotes, newlines or backslashes. The two defences cover different failure modes, which is exactly what generated source deserves.

What remained documented

PR #289 fixed the central executor path, but it did not claim that every shell invocation in the repository disappeared. Two manual construction sites remained documented in tests:

Those paths bypassed the central useShell=true escaping change because they manually invoked /bin/sh -c while using the executor's normal useShell=false mode. In other words, the executor safely spawned /bin/sh, then the supplied -c string recreated the vulnerable shell boundary inside the argument list.

The follow-up shape is straightforward:

executor(["defaults", "read", `${appPath}/Info`, "CFBundleIdentifier"])

That removes the shell entirely. Escaping is useful when a shell is unavoidable. Not invoking a shell is better when the target command already accepts arguments.

The tests named these as unfixed vectors rather than hiding them behind the main fix. That made review more useful. It separated the vulnerability fixed by the PR from related debt that should not be confused with a completed guarantee.

Validation

The merged test set was broad enough to catch the intended failures without pretending to be exhaustive. New regression coverage included:

The reported run covered 138 files and 1,542 tests. Of those, 1,527 passed, none failed and 15 were pre-existing skips. tsc --noEmit completed without type errors and the production build succeeded.

The pattern in AI developer tools

This finding sits in the same family as several other AI tooling bugs I have written up: LightRAG's Memgraph Cypher injection, full-stack-ai-agent-template's webhook SSRF and CodeGraphContext's write-Cypher visualisation endpoint. The sinks differ. The route into them is increasingly familiar.

AI-facing developer tools accept flexible natural-language-driven input, convert it into structured tool parameters and then perform local operations with real authority. That authority may be a database write, an HTTP request to a private address or a shell command on a workstation. The model is not the security boundary. The tool handler is.

XcodeBuildMCP's fixed path was especially easy to underestimate because the dangerous value was a build setting, not a field named command. Shells do not care about our naming conventions. If user-controlled bytes reach /bin/sh -c, every argument is potentially syntax until proven otherwise.

The older rule still holds: pass argument arrays by default, escape for the parser you are actually invoking and avoid shells unless there is no simpler interface. The newer complication is that the caller may be an AI agent confidently laundering hostile text into a well-typed tool call. The type says string. The shell hears a programme.