Skip to content

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

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 #32

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-13 - Command Injection in Docker Logs via URL Parameters
**Vulnerability:** Command injection and potential flag injection due to using `execSync` with unsanitized URL parameters (`tail` and `id`) string-concatenated in `docker logs`.
**Learning:** Even internal toolings or APIs querying basic container information can be vectors for critical command injections if parameters derived from `url.searchParams` are concatenated into strings executed by the shell.
**Prevention:** Always use `execFileSync`, `execFile`, or `spawn` with an array of arguments, and ensure input parameters do not start with a hyphen (`-`) to mitigate flag injection vulnerabilities in CLI tools like docker. Also, catch blocks should sanitize error outputs rather than throwing raw internal stack traces directly.
18 changes: 13 additions & 5 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,5 @@
import type { APIRoute } from 'astro';
import { execSync } from 'child_process';
import { execFileSync } from 'child_process';
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

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

medium

Switching to the asynchronous execFile is recommended to avoid blocking the Node.js event loop in this API route. This requires importing promisify to handle the callback-based API with async/await.

import { execFile } from 'child_process';
import { promisify } from 'util';

const execFileAsync = promisify(execFile);


export const GET: APIRoute = async ({ url }) => {
try {
Expand All @@ -21,18 +21,25 @@ export const GET: APIRoute = async ({ url }) => {
});
}

// Validation des entrées pour éviter l'injection de flags
if (containerId.startsWith('-') || tail.startsWith('-')) {
return new Response(JSON.stringify({ error: "Paramètres invalides" }), {
status: 400,
headers: { 'Content-Type': 'application/json' }
});
}

// Commande Docker pour récupérer les logs
const command = `docker logs --tail ${tail} ${containerId}`;
let logs = [];
try {
const output = execSync(command, { stdio: ['pipe', 'pipe', 'pipe'] }).toString();
const output = execFileSync('docker', ['logs', '--tail', tail, containerId], { stdio: ['pipe', 'pipe', 'pipe'], encoding: 'utf8' });
logs = output.trim().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 {
throw err;
throw new Error("Erreur d'exécution de la commande");
}
}
Comment on lines +35 to 44
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

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

high

The current implementation has two significant issues:

  1. Missing Logs: Using execFileSync with separate pipes for stdout and stderr means that any logs written to stderr by the containerized application are ignored on success.
  2. Information Leak: If the command fails, the catch block returns the raw stderr from the Docker CLI (lines 39-40), which can leak internal system details (e.g., "No such container") and contradicts the goal of sanitizing error messages.

Using execFile allows capturing both streams and provides a cleaner way to handle errors uniformly.

        const { stdout, stderr } = await execFileAsync('docker', ['logs', '--tail', tail, containerId], { encoding: 'utf8' });
        const output = stdout + stderr;
        logs = output.trim() ? output.trim().split('\n') : [];
    } catch (err: any) {
        throw new Error("Erreur d'exécution de la commande");
    }


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 }), {
// Ne pas fuiter les stack traces internes
return new Response(JSON.stringify({ error: "Logs indisponibles" }), {
status: 500,
headers: { 'Content-Type': 'application/json' }
});
Expand Down