-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Description
Background
During PR #35 review, Copilot flagged potential shell injection concerns in XcodeClient.swift. While the current implementation uses values from internal configuration (not user input), hardening the shell command execution would improve defense-in-depth.
Current Patterns
Several methods in XcodeClient construct shell commands via string interpolation:
rm -frcommands (lines 146, 185, 222) - file deletion via shellxcodebuildcommands (line 224-225) - build execution with piped outputaltoolupload command (line 162) - App Store upload
Proposed Changes
Option A: Use FileManager for file operations
Replace rm -rf shell commands with FileManager.removeItem(atPath:) for safer file deletion.
Option B: Use Process API with argument arrays
For complex commands like xcodebuild, use Process with properly separated arguments instead of shell string interpolation.
Option C: Input validation
Add validation to ensure scheme names, paths, and other interpolated values don't contain shell metacharacters.
Files to Review
Sources/Xproject/Utilities/XcodeClient.swiftSources/Xproject/Utilities/CommandExecutor.swift(if changes affect the executor interface)
Acceptance Criteria
- Replace
rmshell commands withFileManageroperations - Review all shell command constructions for potential injection points
- Consider input validation for configuration-derived values
- Maintain existing functionality and test coverage
References
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels