From b6bfa78b7619eb412cb127af8a1e29aaf4f25457 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 17:37:15 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20command=20injection=20in=20logs=20via=20execFile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Migrated `execSync` to `execFile` in `src/pages/api/docker-logs.ts` and `src/pages/api/forge-logs.ts`. - Validated inputs to prevent argument injection (`-`). - Improved performance by not blocking the event loop. Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com> --- .jules/sentinel.md | 4 ++++ src/pages/api/docker-logs.ts | 28 +++++++++++++++++----------- src/pages/api/forge-logs.ts | 8 ++++++-- 3 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..c8c87cec --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-09 - Command Injection in docker logs via execSync +**Vulnerability:** Command injection and event loop blocking in `src/pages/api/docker-logs.ts` and `src/pages/api/forge-logs.ts` due to string concatenation with user input inside `execSync()`. +**Learning:** Using `execSync(command)` with user-derived inputs (like `tail` and `containerId`) allows arbitrary command execution if an attacker uses shell metacharacters like `;` or `&&`. Furthermore, synchronous execution blocks the main thread. +**Prevention:** Always use `execFile` or `spawn` with an array of arguments for OS commands, which prevents the shell from interpreting metacharacters. Additionally, validate inputs (e.g., rejecting strings starting with `-`) to prevent argument injection. diff --git a/src/pages/api/docker-logs.ts b/src/pages/api/docker-logs.ts index cb72e09e..5d86d87a 100644 --- a/src/pages/api/docker-logs.ts +++ b/src/pages/api/docker-logs.ts @@ -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 { @@ -14,23 +17,26 @@ 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" }), { + // Validation de sécurité : éviter l'injection de drapeaux ou commandes + if (!containerId || containerId.startsWith('-') || tail.startsWith('-')) { + return new Response(JSON.stringify({ error: "ID du conteneur manquant ou 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'); + // Utiliser execFile au lieu de execSync pour éviter les injections de commande et le blocage de l'event loop + const { stdout, stderr } = await execFileAsync('docker', ['logs', '--tail', tail, containerId]); + + // docker logs écrit souvent sur stderr (ou stdout + stderr combinés) + const combined = (stdout + '\n' + stderr).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'); + if (err.stderr || err.stdout) { + const combined = ((err.stdout || '') + '\n' + (err.stderr || '')).trim(); + logs = combined ? combined.split('\n') : []; } else { throw err; } diff --git a/src/pages/api/forge-logs.ts b/src/pages/api/forge-logs.ts index 68638bee..38435401 100644 --- a/src/pages/api/forge-logs.ts +++ b/src/pages/api/forge-logs.ts @@ -1,6 +1,9 @@ import type { APIRoute } from 'astro'; -import { execSync } from 'child_process'; +import { execFile } from 'child_process'; +import { promisify } from 'util'; import path from 'path'; + +const execFileAsync = promisify(execFile); export const GET: APIRoute = async ({ url }) => { try { @@ -12,7 +15,8 @@ export const GET: APIRoute = async ({ url }) => { let output = ""; try { - output = execSync(`tail -n ${lines} ${filePath}`).toString(); + const { stdout } = await execFileAsync('tail', ['-n', lines.toString(), filePath]); + output = stdout; } catch (e) { output = "Erreur lors de la lecture du fichier log."; }