Skip to content

🛡️ Sentinel: [CRITICAL] Fix command injection in github clone API #18

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 github clone API #18

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 @@
## 2025-03-01 - Command Injection in github-clone.ts
**Vulnerability:** Command injection and argument injection in `src/pages/api/github-clone.ts`. The API endpoint accepted `repoUrl` and `repoName` from JSON body and concatenated them directly into a `git clone` shell command using `exec`. A malicious user could provide a `repoName` like `; rm -rf /;` to execute arbitrary commands on the server. Furthermore, they could provide a string starting with `-` to inject flags to `git`.
**Learning:** Shell-based command execution (like `exec`) paired with user input is a critical security risk. Node's `exec` passes the entire command string to a shell, making it trivial to inject additional commands.
**Prevention:** Always use `execFile` (or `spawn`) and pass arguments as an array to ensure the system executes only the intended executable with arguments securely escaped by the OS. Additionally, explicitly validate that user-controlled input intended as arguments (like URLs or folder names) does not start with a hyphen (`-`) to prevent flag/argument injection against the target executable itself.
16 changes: 13 additions & 3 deletions src/pages/api/github-clone.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -16,6 +16,15 @@ export const POST: APIRoute = async ({ request }) => {
return new Response(JSON.stringify({ error: 'repoUrl et repoName requis' }), { status: 400 });
}

if (typeof repoUrl !== 'string' || typeof repoName !== 'string') {
return new Response(JSON.stringify({ error: 'Type invalide pour repoUrl ou repoName' }), { status: 400 });
}

// Security: Prevent argument injection
if (repoUrl.trim().startsWith('-') || repoName.trim().startsWith('-')) {
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

security-high high

Validating only the leading hyphen is insufficient for repoName. Since this value is used as a directory name in path.join and passed to git clone, it should be strictly validated to prevent path traversal (e.g., ..) and ensure it doesn't contain path separators. This prevents a malicious user from cloning a repository into an arbitrary location on the server.

Suggested change
if (repoUrl.trim().startsWith('-') || repoName.trim().startsWith('-')) {
if (repoUrl.trim().startsWith('-') || repoName.trim().startsWith('-') || repoName.includes('..') || repoName.includes('/') || repoName.includes('\\')) {

return new Response(JSON.stringify({ error: 'Format de repoUrl ou repoName invalide' }), { status: 400 });
}

const githubToken = await getConfig('githubToken', true);
if (!githubToken || githubToken.trim() === '') {
return new Response(JSON.stringify({ error: 'Jeton GitHub manquant' }), { status: 400 });
Expand All @@ -33,7 +42,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 });
// Security: Use execFile to prevent command injection
const { stdout, stderr } = await execFilePromise('git', ['clone', authUrl, repoName], { cwd: reposRoot });
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

security-high high

The authUrl contains the sensitive githubToken. When execFilePromise fails (e.g., due to an invalid URL or network error), the error object's message property typically includes the full command line with all arguments. This error message is then returned directly to the client in the catch block at line 66, leaking the token. You should catch the error here and throw a generic one to ensure the token is never exposed.

Suggested change
const { stdout, stderr } = await execFilePromise('git', ['clone', authUrl, repoName], { cwd: reposRoot });
const { stdout, stderr } = await execFilePromise('git', ['clone', authUrl, repoName], { cwd: reposRoot }).catch(() => {
throw new Error('Erreur lors du clonage du dépôt. Veuillez vérifier l\'URL et vos accès.');
});


// Try to auto-sync it into the database
try {
Expand Down