π‘οΈ Sentinel: [CRITICAL] Fix command injection in unix system queries - #165
Conversation
Replaced `execSync` with `execFileSync` in `packages/core/src/unix/system-queries.ts` to execute shell commands like `id` and `getent`. This mitigates command injection risks because arguments are passed directly to the executable instead of being evaluated inside an implicit shell environment. Appended a journal entry in `.jules/sentinel.md` documenting this security pattern.
|
π 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: The daemon used
execSyncfromnode:child_processinpackages/core/src/unix/system-queries.tsto execute shell commands likegetentandidwhich took dynamic parameters likeusernameorgroupName. Due to the usage of double quotes around parameters being interpreted by a shell environment, this resulted in a command injection vulnerability.π― Impact: If an attacker can control the
usernameorgroupNameinputs, they could break out of the double quotes and execute arbitrary shell commands on the host operating system with the privileges of the daemon.π§ Fix: Replaced
execSyncwithexecFileSync, completely removing the implicit shell execution. It now securely passes arguments directly to the executable binaries (idandgetent), eliminating the command injection vector. Unused imports insystem-queries.tswere also cleaned up.β Verification: Verified by running the relevant core unix tests (
pnpm --filter @agor/core test -- src/unix/system-queries.ts src/unix/group-manager.test.ts src/unix/user-manager.test.ts src/unix/run-as-user.test.ts) and workspace linter (pnpm -w run lint:fix). All checks passed cleanly.Note: The journal file
.jules/sentinel.mdwas also updated with these learnings.PR created automatically by Jules for task 6320002034556739945 started by @Donach