fix: Properly escape file path in chmod command (#1527)

[thomhurst]

Summary

• Fixed security/reliability issue where file paths containing special characters (spaces, quotes, etc.) could cause the chmod command to fail or behave unexpectedly
• The path is now properly escaped by wrapping it in single quotes and escaping any embedded single quotes with '\''

Changes

Before:

await _bash.Command(new BashCommandOptions($"chmod u+x {options.Path}"), cancellationToken)

After:

var escapedPath = options.Path.Replace("'", "'\''");
await _bash.Command(new BashCommandOptions($"chmod u+x '{escapedPath}'"), cancellationToken)

Test plan

• Verify build passes in CI
• Test with file paths containing spaces (e.g., /path/to/my script.sh)
• Test with file paths containing single quotes (e.g., /path/to/it's working.sh)

Fixes #1527

🤖 Generated with Claude Code

[thomhurst]

Summary

Adds proper shell escaping for file paths in the chmod command to prevent command injection vulnerabilities.

Critical Issues

None found ✅

Analysis

The fix correctly addresses a command injection vulnerability in the chmod command execution:

Why this fix is needed:

• Line 31 uses BashCommandOptions with string interpolation: $"chmod u+x {options.Path}"
• This creates a bash command string passed via -c, where the entire string is treated as a single argument to bash
• CliWrap's argument escaping doesn't protect against injection within the command string itself
• Example attack: if options.Path is "/tmp/file'; rm -rf /; echo '", the original code would execute arbitrary commands

Why the fix is correct:

• The PR escapes single quotes using the bash-safe pattern: ''' (close quote, escaped quote, open quote)
• Wraps the path in single quotes to prevent word splitting and glob expansion
• This is the standard and secure way to handle untrusted input in bash commands

Why FromFile on line 33 doesn't need the same fix:

• BashFileOptions passes the path as a structured CLI argument (not string interpolation)
• CliWrap's WithArguments() properly escapes this argument when invoking bash
• The path becomes a properly escaped argument to the bash executable, not part of an interpolated command string

Verdict

✅ APPROVE - No critical issues

This is a good security fix that properly addresses command injection in the chmod execution path while correctly leaving the FromFile call unchanged (since it uses safe argument passing).

[Copilot]

Pull request overview

This PR fixes a security and reliability issue in the FileInstaller.cs where file paths containing special shell characters (spaces, quotes, etc.) could cause the chmod command to fail or execute incorrectly. The fix properly escapes single quotes in file paths before interpolating them into a bash command string.

Key Changes

• Added shell escaping logic for file paths used in the chmod command to prevent command injection and ensure paths with special characters work correctly
• The path is now wrapped in single quotes with embedded single quotes escaped using the '\'' pattern

[Copilot]

The escaping logic is duplicated from the existing EscapeShellArgument utility method in PredefinedInstallers.cs (line 194). Consider extracting this method to a shared utility class to maintain consistency and follow the DRY principle. The existing method at PredefinedInstallers.cs:194-199 uses the same escaping pattern and includes documentation explaining the approach.

Suggested change

	var escapedPath = options.Path.Replace("'", "'\\''");
	var escapedPath = PredefinedInstallers.EscapeShellArgument(options.Path);

[Copilot]

This fix for shell escaping lacks test coverage. The existing BashTests.cs file covers basic bash command execution but doesn't test file paths with special characters (spaces, quotes, etc.). Consider adding tests that verify the chmod command works correctly with file paths containing spaces and single quotes, as described in the PR test plan.