Run chown and id without a shell

[ay901246]

What

osFileSystem.chown built id -g <user> and chown '<user>:<group>' '<path>' as strings and ran them with sh -c. Now the user, group and path go to id and chown as separate arguments, after --. Shell syntax in any of them stays plain text.

• The error messages do not change.
• homeDir still uses runCommand, because it needs the shell to expand ~user.
• The order is chown -- user:group path, not chown user:group -- path. BSD chown reads a -- after the first operand as a file name.

Checks

Check	Result
system package on macOS	Pass
The two new specs on the old code	Fail, as expected
All 8 chown specs as root in golang:1.26	Pass
CopyFile / CopyDir specs in golang:1.26	Fail only because the image has no lsof; not related

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 36d79c89-a395-4f71-8504-53422c37dbd4

📥 Commits

Reviewing files that changed from the base of the PR and between bf0c2ec and 9739a35.

📒 Files selected for processing (2)

• system/os_file_system_unix.go
• system/os_file_system_unix_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

chown now runs id and chown with separate arguments instead of shell-formatted commands. A new helper captures and trims command output, and returns empty output with an error when execution fails. Tests check shell syntax in paths and owner values.

Suggested reviewers: aramprice

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 9739a

The direct id and chown calls preserve shell syntax in inputs as ordinary operands, and the reviewed command forms are compatible with supported paths. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: running chown and id without a shell.
Description check	✅ Passed	The description directly explains the shell removal, argument handling, preserved behavior, and test results.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟢 Approval recommended

The implementation safely preserves arguments as literal values and tests both affected injection paths.

0 open findings

What changed in this PR

Removes shell invocation from Unix ownership changes to prevent command injection.

Changes:

• Executes id and chown with discrete arguments and --.
• Adds regression tests for shell syntax in paths and owners.

File	Description
system/​os_file_system_unix.go	Adds shell-free command execution for chown.
system/​os_file_system_unix_test.go	Verifies shell syntax is not executed.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[neddp]

Nice. :)