diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..8c12418c --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-18 - Prevent Command Injection and Token Leaks in Git Operations +**Vulnerability:** The API route `src/pages/api/github-clone.ts` used `exec` to run `git clone`, interpolating unvalidated user inputs (`repoUrl` and `repoName`) and an authentication URL that contained a sensitive GitHub token. This exposed the system to command injection risks and risked leaking the token if the shell command failed and the resulting exception message containing the command was sent to the client. +**Learning:** Using shell execution (`exec` or `execSync`) with string concatenation for commands that include secrets or user inputs is highly dangerous. A failed command will include the raw string (including the secret) in the `error.message`. +**Prevention:** Always use `execFile` or `spawn` with an array of arguments, particularly when executing system commands. Use `--` to separate flags from positional arguments, preventing argument injection. In error handling paths, deliberately sanitize `error.message` to replace any known sensitive secrets (like tokens or passwords) with placeholders (e.g., `***`) before returning the message in an API response. diff --git a/src/pages/api/github-clone.ts b/src/pages/api/github-clone.ts index 5ff437b8..6c8ef0ff 100644 --- a/src/pages/api/github-clone.ts +++ b/src/pages/api/github-clone.ts @@ -1,12 +1,12 @@ import type { APIRoute } from 'astro'; -import { exec } from 'node:child_process'; +import { execFile } from 'node:child_process'; import util from 'node:util'; import fs from 'node:fs'; import path from 'node:path'; import { getReposRootResolved } from '../../lib/forge-repos'; import { getConfig } from '../../lib/config-db'; -const execPromise = util.promisify(exec); +const execFileAsync = util.promisify(execFile); export const POST: APIRoute = async ({ request }) => { try { @@ -33,7 +33,8 @@ export const POST: APIRoute = async ({ request }) => { const authUrl = repoUrl.replace('https://', `https://oauth2:${githubToken}@`); // Clone the repository - const { stdout, stderr } = await execPromise(`git clone ${authUrl} ${repoName}`, { cwd: reposRoot }); + // Use execFile with array arguments to prevent command injection + const { stdout, stderr } = await execFileAsync('git', ['clone', '--', authUrl, repoName], { cwd: reposRoot }); // Try to auto-sync it into the database try { @@ -53,7 +54,13 @@ export const POST: APIRoute = async ({ request }) => { }); } catch (error: any) { - return new Response(JSON.stringify({ error: error.message || 'Erreur lors du clonage' }), { + const githubToken = await getConfig('githubToken', true); + // Secure error handling: prevent leaking the github token + const safeError = error.message + ? (githubToken ? error.message.replaceAll(githubToken, '***') : error.message) + : 'Erreur lors du clonage'; + + return new Response(JSON.stringify({ error: safeError }), { status: 500, headers: { 'Content-Type': 'application/json' }, });