Skip to content

🛡️ Sentinel: [CRITICAL] Fix command injection in docker logs API #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bobdivx wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

🛡️ Sentinel: [CRITICAL] Fix command injection in docker logs API #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-05-18 - Fix Command Injection in API execution
**Vulnerability:** The `src/pages/api/docker-logs.ts` API route had a CRITICAL command injection vulnerability where unsanitized user inputs (`containerId` and `tail`) were directly interpolated into a string executed by `execSync`.
**Learning:** Shell interpreters like `execSync` evaluate meta-characters natively, permitting command breakouts.
**Prevention:** Always use safe primitives like `execFile` or `execFileAsync` where parameters are passed explicitly as an array. Furthermore, thoroughly validate dynamic inputs using regex (e.g., `/^[a-zA-Z0-9_.-]+$/`) to reject special characters, prohibit starting hyphens (`-`) to mitigate flag injection, and use the `--` double-dash operator to demarcate options from positional arguments.
28 changes: 18 additions & 10 deletions src/pages/api/docker-logs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import type { APIRoute } from 'astro';
import { execSync } from 'child_process';
import { execFile } from 'child_process';
import { promisify } from 'util';

const execFileAsync = promisify(execFile);

export const GET: APIRoute = async ({ url }) => {
try {
Expand All @@ -14,21 +17,25 @@ export const GET: APIRoute = async ({ url }) => {
const containerId = url.searchParams.get('id');
const tail = url.searchParams.get('tail') || '100';

if (!containerId) {
return new Response(JSON.stringify({ error: "ID du conteneur manquant" }), {
if (!containerId || !/^[a-zA-Z0-9_.-]+$/.test(containerId) || containerId.startsWith('-')) {
return new Response(JSON.stringify({ error: "ID du conteneur invalide ou manquant" }), {
status: 400,
headers: { 'Content-Type': 'application/json' }
});
}

if (!/^\d+$/.test(tail) && tail !== 'all') {
return new Response(JSON.stringify({ error: "Paramètre 'tail' invalide" }), {
status: 400,
headers: { 'Content-Type': 'application/json' }
});
}

// Commande Docker pour récupérer les logs
const command = `docker logs --tail ${tail} ${containerId}`;
let logs = [];
let logs: string[] = [];
try {
const output = execSync(command, { stdio: ['pipe', 'pipe', 'pipe'] }).toString();
logs = output.trim().split('\n');
const { stdout, stderr } = await execFileAsync('docker', ['logs', '--tail', tail, '--', containerId]);
logs = (stdout || stderr).trim().split('\n');
Comment on lines +36 to +37

@gemini-code-assist gemini-code-assist Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using (stdout || stderr) means that if stdout is non-empty, stderr is completely ignored. Since Docker containers can write to both streams, any logs written to stderr (such as error messages or warnings) will be lost if the container also outputs to stdout. Additionally, if both streams are empty, "".trim().split('\n') will produce [""] instead of an empty array.

We should combine both streams and handle empty logs correctly.

        const { stdout, stderr } = await execFileAsync('docker', ['logs', '--tail', tail, '--', containerId]);
        const combined = [stdout, stderr].filter(Boolean).join('\n').trim();
        logs = combined ? combined.split('\n') : [];

} 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 {
Expand All @@ -41,7 +48,8 @@ export const GET: APIRoute = async ({ url }) => {
headers: { 'Content-Type': 'application/json' }
});
} catch (error: any) {
return new Response(JSON.stringify({ error: "Logs indisponibles: " + error.message }), {
// Sanitize error message to prevent leaking sensitive information
return new Response(JSON.stringify({ error: "Logs indisponibles: Une erreur inattendue s'est produite." }), {
status: 500,
headers: { 'Content-Type': 'application/json' }
});
Expand Down