🛡️ Sentinel: [CRITICAL] Fix Command Injection in Docker Logs API#104
🛡️ Sentinel: [CRITICAL] Fix Command Injection in Docker Logs API#104bobdivx wants to merge 1 commit into
Conversation
🚨 Severity: CRITICAL 💡 Vulnerability: Unsanitized user inputs (containerId and tail) were being directly interpolated into a shell string executed via execSync, leading to arbitrary Command Injection. 🎯 Impact: An attacker could execute arbitrary shell commands on the backend server by injecting shell operators. 🔧 Fix: - Replaced execSync with execFileAsync to pass arguments directly to the binary, bypassing shell evaluation. - Implemented strict regex validation for containerId and tail. - Prevented potential error details/stack traces from leaking into the JSON response. - Added learning to .jules/sentinel.md. ✅ Verification: Run pnpm test and pnpm run check. Try sending malicious payload in ?id=mycontainer;ls and verify it is safely rejected. Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a command injection vulnerability in the Docker logs API endpoint by replacing execSync with execFileAsync, introducing strict regex validation for the containerId and tail parameters, and improving error handling to prevent internal stack trace leaks. The review feedback points out that handling err.stderr in the catch block of execFileAsync and returning it as normal logs with a 200 OK status is problematic, as execution failures should instead throw an error and trigger a 500 Internal Server Error response.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } catch (err: any) { | ||
| // Certains logs sortent sur stderr, checkons stderr si stdout est vide ou si erreur | ||
| if (err.stderr) { | ||
| logs = err.stderr.toString().trim().split('\n'); | ||
| } else { | ||
| throw err; | ||
| } | ||
| if (err.stderr) { | ||
| logs = err.stderr.toString().trim().split("\n").filter(Boolean); | ||
| } else { | ||
| console.error("Docker logs error:", err); | ||
| throw new Error("Erreur lors de la récupération des logs"); | ||
| } | ||
| } |
There was a problem hiding this comment.
With the migration to execFileAsync, when the command runs successfully (exit code 0), both stdout and stderr are captured and handled in the try block.
The catch block is only entered if the command fails (non-zero exit code or spawn error). In case of a failure (e.g., container not found, docker daemon down), err.stderr will contain the error message (e.g., Error: No such container).
By checking if (err.stderr) and setting logs = err.stderr..., the API will return a 200 OK response with the error message treated as container logs. This bypasses the UI's error handling (which expects a non-2xx status or an { error } field to display a red error box) and displays the error message as a normal log line.
Instead, any failure in execFileAsync should be treated as a true error: we should log it and throw an error so that the outer catch block handles it and returns a 500 Internal Server Error response.
} catch (err: any) {
console.error("Docker logs error:", err);
throw new Error("Erreur lors de la récupération des logs");
}1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: Unsanitized user inputs (
containerIdandtail) were being directly interpolated into a shell string executed viaexecSyncinsrc/pages/api/docker-logs.ts, leading to arbitrary Command Injection.🎯 Impact: An attacker could execute arbitrary shell commands on the backend server by injecting shell operators (like
;,&&) via API request parameters.🔧 Fix:
execSyncwithexecFileAsyncto pass arguments directly to the binary, bypassing shell evaluation.containerId(/^[a-zA-Z0-9_.-]+$/) andtail(/^\d+$/or'all')..jules/sentinel.md.✅ Verification: Run
pnpm testandpnpm run check. Try sending malicious payload in?id=mycontainer;lsand verify it is safely rejected as a 400 Bad Request.PR created automatically by Jules for task 12150813474250741277 started by @bobdivx