diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..00271601 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-03-09 - Sanitize Error Messages for Shell Commands with Tokens +**Vulnerability:** Command execution error messages (like `git clone`) can leak sensitive authentication tokens injected into the URL/command if the raw `error.message` is returned directly to the client. +**Learning:** Always sanitize output strings (using regex or redaction) before sending error messages derived from system processes to the frontend API responses, especially when tokens are involved. +**Prevention:** Use `.replace(/https:\/\/[^@]+@/g, 'https://***@')` or similar patterns in catch blocks when handling process errors containing URLs with embedded credentials. diff --git a/pr_body.txt b/pr_body.txt new file mode 100644 index 00000000..cf05109b --- /dev/null +++ b/pr_body.txt @@ -0,0 +1,5 @@ +🚨 **Severity:** CRITICAL +💡 **Vulnerability:** The API endpoint `github-clone.ts` used `exec` for cloning repositories, which parses inputs via the shell, exposing the application to Command Injection vulnerabilities. Furthermore, failure cases directly exposed the `error.message` to the client, leading to a potential leak of the Github OAuth token embedded within the clone URL. +🎯 **Impact:** An attacker could execute arbitrary code on the server by crafting malicious inputs in the `repoUrl` or `repoName` payload. Additionally, token leakage compromises the Github account's security. +🔧 **Fix:** Replaced `exec` with `execFile`, utilizing an argument array `['clone', '--', authUrl, repoName]` which isolates inputs strictly as arguments and prevents option injection via `--`. Intercepted the error payload in the catch block to sanitize output with `.replace(/https:\/\/[^@]+@/g, 'https://***@')`, preventing secret spillage. Added a journal entry to `.jules/sentinel.md` regarding proper process output sanitization. +✅ **Verification:** Ran `pnpm run check` and `pnpm test` (vitest) to confirm functional stability. The patch uses exact replacement methodologies to ensure identical behavior with strict shell isolation. diff --git a/src/pages/api/github-clone.ts b/src/pages/api/github-clone.ts index 5ff437b8..9ccd3274 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 execFilePromise = util.promisify(execFile); export const POST: APIRoute = async ({ request }) => { try { @@ -32,8 +32,9 @@ export const POST: APIRoute = async ({ request }) => { // Assuming format https://github.com/user/repo.git const authUrl = repoUrl.replace('https://', `https://oauth2:${githubToken}@`); - // Clone the repository - const { stdout, stderr } = await execPromise(`git clone ${authUrl} ${repoName}`, { cwd: reposRoot }); + // Clone the repository securely using execFile to prevent command injection. + // We use '--' to prevent argument injection if the repo URL or name starts with '-'. + const { stdout, stderr } = await execFilePromise('git', ['clone', '--', authUrl, repoName], { cwd: reposRoot }); // Try to auto-sync it into the database try { @@ -53,7 +54,11 @@ export const POST: APIRoute = async ({ request }) => { }); } catch (error: any) { - return new Response(JSON.stringify({ error: error.message || 'Erreur lors du clonage' }), { + // 🛡️ Sentinel: Prevent GitHub token from leaking in error messages + const rawError = error.message || 'Erreur lors du clonage'; + const safeError = rawError.replace(/https:\/\/[^@]+@/g, 'https://***@'); + + return new Response(JSON.stringify({ error: safeError }), { status: 500, headers: { 'Content-Type': 'application/json' }, });