🛡️ Sentinel: [CRITICAL] Fix command injection in id-lookups#69
Conversation
Replaced `execSync` with `execFileSync` to bypass shell interpolation and prevent command injection. Used `--` delimiters to protect against option injection. Co-authored-by: Donach <39565367+Donach@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Severity: CRITICAL
Vulnerability: A potential command injection vulnerability existed in
packages/core/src/unix/id-lookups.tswhere dynamic variables (groupName,username) were passed toexecSyncvia string interpolation. An attacker could exploit this by passing unvalidated inputs to execute arbitrary shell commands.Impact: Arbitrary shell command execution on the host system.
Fix: Refactored
getGidFromGroupName,getUidFromUsername, andgetHomedirFromUsernameto useexecFileSync, which spawns the executable directly without an intermediate shell. Passed inputs safely as an array of arguments, using the--delimiter to prevent arguments starting with a dash from being parsed as options.Verification: Ran
pnpm lint,pnpm test, andpnpm buildto verify the refactoring logic and ensure that no regressions were introduced.PR created automatically by Jules for task 9318451421827350139 started by @Donach