🛡️ Sentinel: [CRITICAL] Fix command injection in child process execution#63
Conversation
Migrates `execSync` calls containing dynamic interpolation strings over to `execFileSync` to mitigate command injection risks. Specifically, in `apps/agor-cli/src/commands/admin/sync-unix.ts` and `apps/agor-daemon/src/services/terminals.ts`, user inputs or dynamic variables were previously embedded directly into shell commands via template literals. The updated code now invokes the binaries directly (via `execFileSync`) and passes the arguments explicitly as array elements along with `--` where necessary, which entirely bypasses shell execution and closes the command and argument injection vectors. The journal `.jules/sentinel.md` has been updated with these learnings. 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. |
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: Found
execSyncbeing used with string interpolation for dynamic variables (sudoUserinapps/agor-cli/src/commands/admin/sync-unix.tsandchownTo/envFileinapps/agor-daemon/src/services/terminals.ts). This pattern executes within a shell (/bin/sh), allowing for command injection if an attacker can manipulate these inputs.🎯 Impact: A threat actor breaking out of the string boundary could execute arbitrary system commands on the host machine. Given these functions interface with
sudoandgetent, this could easily lead to unconstrained privilege escalation or data exfiltration.🔧 Fix: Replaced
execSyncwithexecFileSync, completely bypassing the shell. Passed the binaries directly and utilized argument arrays (['passwd', '--', String(sudoUser)]) to ensure variables are strictly treated as string arguments.✅ Verification:
pnpm lintand fixed formatting gaps viapnpm lint:fix.@agor/cli,@agor/core, and@agor/daemonviaexport NODE_OPTIONS="--max-old-space-size=4096"; pnpm test..jules/sentinel.md.PR created automatically by Jules for task 13111848230379582701 started by @Donach