Follow-up to #289.
PR #289 fixes the useShell=true code path in defaultExecutor by routing through shellEscapeArg. That fix does not cover call sites that construct their own /bin/sh -c strings and pass them to the executor with useShell=false as ['/bin/sh', '-c', command]. Each of these interpolates user-controlled paths into the shell string and is currently vulnerable to CWE-78.
Affected sites
src/utils/bundle-id.ts — defaults read "${appPath}/Info" CFBundleIdentifier and the PlistBuddy fallbacksrc/mcp/tools/project-discovery/get_mac_bundle_id.ts — same patternsrc/mcp/tools/macos/build_macos.ts — same patternsrc/utils/macos-steps.ts — same pattern
Regression tests already documenting these vectors
The PR carries UNFIXED: test cases for these sites in:
src/utils/__tests__/bundle-id-injection.test.tssrc/utils/__tests__/mac-bundle-id-injection.test.ts
Their assertions (e.g. expect(cmdString).toContain('$(id)')) intentionally still document the injection vectors so the audit trail remains visible. They should be flipped from UNFIXED: to safe assertions as each site is fixed.
Remediation options
For each site, either:
- Stop using
/bin/sh -c and call defaults / PlistBuddy with an arg array directly (preferred — removes shell parsing entirely), or
- Apply
shellEscapeArg (from src/utils/command.ts) to every interpolated value.
Option (1) is a small design-shape decision per call site (whether the helper takes a path or a parsed-out arg array), which is why these were deliberately left out of the #289 rebase scope.
Why this matters
After #289 lands, one shell-injection variant is closed and four documented variants remain live. The vulnerability is only partially mitigated until these are addressed.
Follow-up to #289.
PR #289 fixes the
useShell=truecode path indefaultExecutorby routing throughshellEscapeArg. That fix does not cover call sites that construct their own/bin/sh -cstrings and pass them to the executor withuseShell=falseas['/bin/sh', '-c', command]. Each of these interpolates user-controlled paths into the shell string and is currently vulnerable to CWE-78.Affected sites
src/utils/bundle-id.ts—defaults read "${appPath}/Info" CFBundleIdentifierand the PlistBuddy fallbacksrc/mcp/tools/project-discovery/get_mac_bundle_id.ts— same patternsrc/mcp/tools/macos/build_macos.ts— same patternsrc/utils/macos-steps.ts— same patternRegression tests already documenting these vectors
The PR carries
UNFIXED:test cases for these sites in:src/utils/__tests__/bundle-id-injection.test.tssrc/utils/__tests__/mac-bundle-id-injection.test.tsTheir assertions (e.g.
expect(cmdString).toContain('$(id)')) intentionally still document the injection vectors so the audit trail remains visible. They should be flipped fromUNFIXED:to safe assertions as each site is fixed.Remediation options
For each site, either:
/bin/sh -cand calldefaults/PlistBuddywith an arg array directly (preferred — removes shell parsing entirely), orshellEscapeArg(fromsrc/utils/command.ts) to every interpolated value.Option (1) is a small design-shape decision per call site (whether the helper takes a path or a parsed-out arg array), which is why these were deliberately left out of the #289 rebase scope.
Why this matters
After #289 lands, one shell-injection variant is closed and four documented variants remain live. The vulnerability is only partially mitigated until these are addressed.