Feature/sessions 63 65 fix system complete

[alpsla]

No description provided.

[corgea]

SSRF ( 🔴 High ) - The code sends data to a URL from an external request without checking if it's trusted. This can let attackers make the server send data to bad places. View in Corgea ↗

More Details

🎟️Issue Explanation: The code sends data to a URL from an external request without checking if it's trusted. This can let attackers make the server send data to bad places.

- "request.webhook" URL is used directly in "fetch" without validation, risking data sent to attacker-controlled servers.
- An attacker can craft a request with a malicious "webhook" URL to capture sensitive analysis results.
- This bypass allows Server-Side Request Forgery (SSRF), where an attacker exploits the server to make unauthorized network requests.

We could not generate a fix for this.

[corgea]

OS Command Injection ( 🔴 Critical ) - The code runs a command using external input "endpoint" without safe handling, risking OS command injection that can let attackers run harmful system commands. View in Corgea ↗

More Details

🎟️Issue Explanation: The code runs a command using external input "endpoint" without safe handling, risking OS command injection that can let attackers run harmful system commands.

- The "endpoint" input goes directly into the OS command line without sanitization, allowing injection of extra commands.
- An attacker can exploit this by passing input like "endpoint"; rm -rf / #", running malicious commands after the intended one.
- This is dangerous because the code executes system-level commands, potentially compromising the system where it runs.

🪄Fix Explanation: The fix mitigates command injection by replacing string-based shell execution with "execFile", which treats arguments safely, and by validating "endpoint" to allow only safe HTTP/HTTPS URLs without shell metacharacters.
- Replaced "execAsync(`graphql-cop -t "${endpoint}" -o json`)" with "execFileAsync('graphql-cop', ['-t', endpoint, '-o', 'json'])" to avoid shell interpretation.
- Imported "execFile" dynamically and promisified it for use as "execFileAsync", ensuring async/await compatibility.
- Added regex validation "/^https?:\/\/[^\s]+$/i" to permit only HTTP/HTTPS URLs in "endpoint".
- Added regex check "/[;&|`$<>()\n]/" to block common shell metacharacters that could enable injection.
- If validation fails, the function safely returns "issues" without executing any commands.

💡Important Instructions: Ensure all other inputs to child process calls are similarly validated if they could be attacker-controlled; no other changes are needed specifically for this fix.

Suggested change

	const { stdout } = await execAsync(`graphql-cop -t "${endpoint}" -o json`, {
	timeout: 60000 // 1 minute timeout
	});
	// Parse graphql-cop JSON output
	const results = JSON.parse(stdout);
	for (const result of results) {
	issues.push({
	tool: 'graphql-cop',
	ruleId: `graphql-cop-${result.id || 'unknown'}`,
	// Use execFile with argument array to avoid shell interpretation and injection
	const { execFile } = await import('child_process');
	const execFileAsync = promisify(execFile);
	// Validate endpoint: allow only http/https URLs and reject shell metacharacters
	if (!/^https?:\/\/[^\s]+$/i.test(endpoint) || /[;&|`$<>()\n]/.test(endpoint)) {
	return issues;
	}
	const { stdout } = await execFileAsync('graphql-cop', ['-t', endpoint, '-o', 'json'], {
	timeout: 60000 // 1 minute timeout
	});

[corgea]

OS Command Injection ( 🔴 Critical ) - The code builds an OS command using user input without cleaning it, risking unintended command execution. This allows attackers to run harmful commands through the input path. View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds an OS command using user input without cleaning it, risking unintended command execution. This allows attackers to run harmful commands through the input path.

- The command "find "${repoPath}" ..." uses unfiltered "repoPath", letting special characters change the command's behavior.
- An attacker can set "repoPath" to something like "somepath"; rm -rf / #", causing destructive commands to run.
- Since this fetches Dockerfiles, files related to container security may be exposed or impacted by the malicious command execution.

We could not generate a fix for this.

[corgea]

OS Command Injection ( 🔴 Critical ) - This code runs a system command using user input without proper checks, allowing attackers to run unintended commands on the OS. This can be dangerous if input is malicious. View in Corgea ↗

More Details

🎟️Issue Explanation: This code runs a system command using user input without proper checks, allowing attackers to run unintended commands on the OS. This can be dangerous if input is malicious.

- The "execSync" runs "git checkout ${prBranch}", where "prBranch" comes from outside and can include harmful data.
- If "prBranch" is "branchName; rm -rf /", it runs "git checkout branchName" then deletes files, showing injection risk.
- Because it handles OS commands, this can lead to unauthorized code execution or data loss if exploited.

🪄Fix Explanation: The fix validates the branch name before using it in an OS command, preventing injection by allowing only safe characters, checking length, and disallowing leading hyphens, thus mitigating command injection risk.
The input "prBranch" is sanitized by trimming whitespace with "String(prBranch).trim()" before validation.
A strict regex "/^[A-Za-z0-9._\/-]+$/" permits only alphanumeric, dot, underscore, slash, and hyphen characters in "validatedPrBranch".
Additional checks reject branch names that start with "-", are empty, or exceed 255 characters to prevent flag injection or overly long input.
The command uses "execSync(`git checkout -- ${validatedPrBranch}`...)" with a safe branch name and the "--" separator to avoid confusing git with flags.

💡Important Instructions: Ensure all other uses of prBranch in this method or file use the validated validatedPrBranch variable to fully prevent injection risks here.

Suggested change

	execSync(`git checkout ${prBranch}`, { cwd: repoPath, stdio: 'ignore' });
	const modifiedFiles = new Set(getModifiedFilesBetweenBranches(repoPath, baseBranch, prBranch));
	console.log(` ✅ Repository ready`);
	console.log(` Base branch: ${baseBranch}`);
	console.log(` PR branch: ${prBranch}`);
	console.log(` Modified files: ${modifiedFiles.size}\n`);
	const validatedPrBranch = String(prBranch).trim();
	const safeBranchPattern = /^[A-Za-z0-9._\/-]+$/;
	if (!safeBranchPattern.test(validatedPrBranch) || validatedPrBranch.startsWith('-') || validatedPrBranch.length === 0 || validatedPrBranch.length > 255) {
	throw new Error('Invalid branch name supplied for checkout');
	}
	execSync(`git checkout -- ${validatedPrBranch}`, { cwd: repoPath, stdio: 'ignore' });

[corgea]

OS Command Injection ( 🔴 Critical ) - The code builds a command using user input without cleaning it, allowing attackers to change or add commands. This can lead to unexpected or harmful actions when running "execAsync". View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds a command using user input without cleaning it, allowing attackers to change or add commands. This can lead to unexpected or harmful actions when running "execAsync".

- The command string "cmd" uses "filePath" and "config.rulesets" directly, which can include special characters to alter the OS command.
- An attacker could inject commands by inputting something like "; rm -rf /" in "filePath", causing extra malicious commands to run.
- Since "execAsync" executes the command in the OS shell, untrusted input can lead to remote code execution or data loss risks.

🪄Fix Explanation: The fix replaces unsafe command concatenation with "execFile", validating "filePath" and each ruleset to reject dangerous characters, preventing command injection by treating inputs as separate arguments.
- Replaces "execAsync(cmd)" call with "execFile('spectral', args)", which safely handles arguments without shell interpolation.
- Validates "filePath" to ensure it is a string and disallows special shell characters using "unsafePattern" regex to prevent injection.
- Iterates over "config.rulesets", validating each item to be a string without unsafe chars or whitespace, then appends them as separate args.
- Builds argument list "args" to pass each parameter separately rather than constructing a shell command string.
- Adds "windowsHide: true" to "execFile" options for cleaner execution on Windows.

💡Important Instructions: Ensure upstream code properly sanitizes or constructs filePath and config.rulesets as strings to avoid unnecessary rejections. No further changes needed for this fix.

Suggested change

	const rulesetArgs = config.rulesets?.map(r => `--ruleset ${r}`).join(' ') || '';
	const cmd = `spectral lint "${filePath}" --format json ${rulesetArgs}`;
	const { stdout, stderr } = await execAsync(cmd, {
	cwd: repoPath,
	maxBuffer: 10 * 1024 * 1024, // 10MB buffer
	timeout: 60000 // 1 minute timeout
	});
	// Parse JSON output
	const results: SpectralResult[] = JSON.parse(stdout || '[]');
	const schemaType = await getSchemaType(filePath);
	const relativePath = path.relative(repoPath, filePath);
	for (const result of results) {
	issues.push({
	tool: 'spectral',
	file: relativePath,
	line: result.range.start.line + 1, // Spectral uses 0-based lines
	const execFileAsync = promisify(require('child_process').execFile);
	const unsafePattern = /[;&|`$<>()\n\r]/;
	if (typeof filePath !== 'string' || unsafePattern.test(filePath)) {
	throw new Error('Invalid filePath provided for spectral lint');
	}
	const rulesetArgsArray: string[] = [];
	if (Array.isArray(config.rulesets)) {
	for (const r of config.rulesets) {
	if (typeof r !== 'string' || unsafePattern.test(r) || /\s/.test(r)) {
	throw new Error('Invalid ruleset value provided for spectral lint');
	}
	rulesetArgsArray.push('--ruleset', r);
	}
	}
	const args = ['lint', filePath, '--format', 'json', ...rulesetArgsArray];
	const { stdout, stderr } = await execFileAsync('spectral', args, {
	cwd: repoPath,

[corgea]

OS Command Injection ( 🔴 Critical ) - The code runs an OS command using user input without properly cleaning it, allowing attackers to run harmful commands on the system. View in Corgea ↗

More Details

🎟️Issue Explanation: The code runs an OS command using user input without properly cleaning it, allowing attackers to run harmful commands on the system.

- "request.repositoryUrl" is used directly in "execSync", which runs shell commands; this input can include special characters.
- An attacker can add extra commands like `; rm -rf /` to "repositoryUrl", causing dangerous system actions when executed.
- This risks system integrity since the code handles repository cloning, a sensitive operation, but doesn't sanitize its input properly.

🪄Fix Explanation: The fix mitigates OS command injection by validating "repositoryUrl" against a strict regex and switching from unsafe "execSync" with shell interpolation to "execFileSync" which runs commands without a shell, preventing injection via special characters.
- Validates "repositoryUrl" ensuring it only matches trusted HTTPS or SSH Git URL formats with safe characters using "/^(https:\/\/|git@)[A-Za-z0-9._\/:-]+$/".
- Converts "repositoryUrl" to string safely and throws an error if validation fails, stopping execution before running any OS command.
- Replaces "execSync(`git clone ${repoUrl} ${repoPath}`)" with "execFileSync('git', ['clone', repoUrl, repoPath])" to avoid shell interpretation of input.
- Adds a "timeout" option (60000ms) to "execFileSync" calls to prevent potential hangs during "git clone" or "git fetch".
- Applies the same secure "execFileSync" approach for "git fetch --all" with controlled arguments and working directory to avoid shell injection.

💡Important Instructions: Ensure input sources populating request.repositoryUrl remain sanitized and validated prior to this service call, and consider adding logging for rejection cases to monitor potential attack attempts.

Suggested change

	execSync(`git clone ${request.repositoryUrl} ${repoPath}`, { stdio: 'inherit' });
	} else if (!request.skipCache) {
	console.log(` Using cached repository, fetching updates...`);
	execSync('git fetch --all', { cwd: repoPath, stdio: 'ignore' });
	}
	// Detect branches
	const baseBranch = request.baseBranch || detectDefaultBranch(repoPath);
	const prBranch = request.prBranch || `pr-${request.prNumber}`;
	// Validate repositoryUrl to prevent command injection and enforce safe patterns
	const repoUrl = typeof request.repositoryUrl === 'string' ? request.repositoryUrl : String(request.repositoryUrl ?? '');
	if (!repoUrl || !/^(https:\/\/|git@)[A-Za-z0-9._\/:-]+$/.test(repoUrl)) {
	throw new Error('Invalid repositoryUrl: only https or git@ URLs with safe characters are allowed');
	}
	// Execute git safely without spawning a shell
	require('child_process').execFileSync('git', ['clone', repoUrl, repoPath], { stdio: 'inherit', timeout: 60000 });
	} else if (!request.skipCache) {

[corgea]

OS Command Injection ( 🔴 Critical ) - The code builds an OS command string using external input without properly handling special characters, risking unintended command execution. View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds an OS command string using external input without properly handling special characters, risking unintended command execution.

- "repoPath" comes from outside and is inserted directly into the command without sanitization, allowing injection.
- An attacker could craft "repoPath" with extra shell commands (e.g., using `;` or `&&`) to run arbitrary code.
- This affects critical operations since the code runs "bundle exec packwerk check" inside the repository's directory, risking data or system compromise.

🪄Fix Explanation: The fix removes shell command concatenation with untrusted input by setting the working directory via "cwd" instead of embedding "repoPath" in a shell command, eliminating command injection risk.
- Replaces "cd "${repoPath}" && bundle exec packwerk check 2>&1" with just "bundle exec packwerk check", removing unsafe string interpolation.
- Sets "cwd: repoPath" in "execAsync" options, safely changing directory without shell injection.
- Removes shell redirection "2>&1", relying on Node.js to capture stdout and stderr separately, preventing shell command misuse.
- Maintains original timeout and buffer settings ensuring consistent execution limits and output size.

💡Important Instructions: Verify that all callers of this code correctly handle stdout and stderr separately, as shell redirection is removed; test command execution in this new context.

Suggested change

	const command = `cd "${repoPath}" && bundle exec packwerk check 2>&1`;
	let rawOutput = '';
	const issues: RawIssue[] = [];
	try {
	const { stdout, stderr } = await execAsync(command, {
	const command = 'bundle exec packwerk check';
	let rawOutput = '';
	const issues: RawIssue[] = [];
	try {

[corgea]

OS Command Injection ( 🔴 Critical ) - The code builds a system command using user input directly, risking attackers injecting malicious commands that run on the OS unexpectedly. View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds a system command using user input directly, risking attackers injecting malicious commands that run on the OS unexpectedly.

- "cmd" includes "repoPath" from outside input without filtering special characters.
- An attacker could add shell commands in "repoPath", e.g., "; rm -rf /", altering "cmd" behavior.
- This risk is called OS Command Injection: attackers break or add commands, potentially harming the system or stealing data.

🪄Fix Explanation: The fix mitigates command injection by validating and sanitizing user inputs for repository and exclude paths, then using "execFile" with argument arrays instead of string command execution, eliminating unsafe shell interpolation.
<ul>
-Sanitizes "repoPath" using "path.resolve" and validates with a regex ("/^[A-Za-z0-9_\-./\\ ]+$/") to prevent special characters that could alter shell commands.
-Replaces string command concatenation ("cmd") with an argument array "trivyArgs" for safer command execution via "execFile".
-Processes "config.excludePaths" by resolving each path and filtering them with the same regex, ensuring only safe directory names are passed as arguments.
-Uses Node.js "child_process.execFile" (promisified) to execute the command, avoiding shell interpretation of input strings, which prevents injection.
</ul>

💡Important Instructions: Ensure any upstream inputs to repoPath and config.excludePaths are similarly validated and sanitized before reaching this function to maintain integrity and security.

Suggested change

	let cmd = `trivy config "${repoPath}" --format json`;
	// Add exclusions (trivy uses .trivyignore or --skip-dirs)
	if (config.excludePaths && config.excludePaths.length > 0) {
	cmd += ` --skip-dirs ${config.excludePaths.join(',')}`;
	}
	// Run trivy
	const { stdout } = await execAsync(cmd, {
	maxBuffer: 100 * 1024 * 1024,
	timeout: 600000 // 10 minute timeout
	});
	const safeRepoPath = path.resolve(repoPath);
	if (!/^[A-Za-z0-9_\-./\\ ]+$/.test(safeRepoPath)) {
	throw new Error('Invalid repository path provided.');
	}
	const execFileAsync = promisify(require('child_process').execFile);
	const trivyArgs: string[] = ['config', safeRepoPath, '--format', 'json'];
	// Add exclusions (trivy uses .trivyignore or --skip-dirs)
	const rawExcludes = Array.isArray(config.excludePaths) ? config.excludePaths : [];
	const safeExcludes = rawExcludes.map(p => path.resolve(p)).filter(p => /^[A-Za-z0-9_\-./\\ ]+$/.test(p));
	if (safeExcludes.length > 0) {
	trivyArgs.push('--skip-dirs', safeExcludes.join(','));

[corgea]

OS Command Injection ( 🔴 Critical ) - This code runs a system command using user input without proper checks, allowing attackers to run unintended commands on the OS. This can be dangerous if input is malicious. View in Corgea ↗

More Details

🎟️Issue Explanation: This code runs a system command using user input without proper checks, allowing attackers to run unintended commands on the OS. This can be dangerous if input is malicious.

- The "execSync" runs "git checkout ${prBranch}", where "prBranch" comes from outside and can include harmful data.
- If "prBranch" is "branchName; rm -rf /", it runs "git checkout branchName" then deletes files, showing injection risk.
- Because it handles OS commands, this can lead to unauthorized code execution or data loss if exploited.

🪄Fix Explanation: The fix validates branch names against a strict whitelist pattern and rejects unsafe inputs to prevent command injection via "execSync". It also adds "--" to safely separate options from arguments, and a timeout to limit execution time.
- Introduces a regex "safeBranchPattern = /^[A-Za-z0-9._\/-]+$/" to only allow alphanumeric characters, dots, slashes, underscores, and dashes in branch names.
- Adds a check to reject branch names starting with "-" to prevent option injection in the shell command.
- Changes "execSync" calls from "git checkout ${branch}" to "git checkout -- ${branch}", ensuring the branch is interpreted as a positional argument, not an option.
- Adds a "timeout: 15000" option to limit execution to 15 seconds, mitigating hanging or slow command risks.

💡Important Instructions: Ensure inputs provided to prBranch and baseBranch are always validated before this function is called to avoid unnecessary errors. No further changes in other code areas are needed for this fix.

diff --git a/packages/agents/src/two-branch/api/v9-analysis-service.ts b/packages/agents/src/two-branch/api/v9-analysis-service.ts
index 430923e..2e45e47 100644
--- a/packages/agents/src/two-branch/api/v9-analysis-service.ts
+++ b/packages/agents/src/two-branch/api/v9-analysis-service.ts
@@ -437,7 +437,11 @@ export class V9AnalysisService {
 
     // Analyze PR branch
     console.log(`   Analyzing PR branch...`);
-    execSync(`git checkout ${prBranch}`, { cwd: repoPath, stdio: 'ignore' });
+    const safeBranchPattern = /^[A-Za-z0-9._\/-]+$/;
+    if (!safeBranchPattern.test(prBranch) || prBranch.startsWith('-')) {
+      throw new Error('Invalid branch name for checkout');
+    }
+    execSync(`git checkout -- ${prBranch}`, { cwd: repoPath, stdio: 'ignore', timeout: 15000 });
     const prResult = await orchestrator.orchestrate(repoPath, 'pr', undefined, {
       includeAllSeverities: mode === 'complete',
       analysisMode: mode
@@ -447,7 +451,10 @@ export class V9AnalysisService {
 
     // Analyze base branch
     console.log(`   Analyzing base branch...`);
-    execSync(`git checkout ${baseBranch}`, { cwd: repoPath, stdio: 'ignore' });
+    if (!safeBranchPattern.test(baseBranch) || baseBranch.startsWith('-')) {
+      throw new Error('Invalid base branch name for checkout');
+    }
+    execSync(`git checkout -- ${baseBranch}`, { cwd: repoPath, stdio: 'ignore', timeout: 15000 });
     const baseResult = await orchestrator.orchestrate(repoPath, 'base', undefined, {
       includeAllSeverities: mode === 'complete',
       analysisMode: mode

To apply the fix, Download .patch.

[corgea]

OS Command Injection ( 🔴 Critical ) - This code runs an OS command using user input without sanitizing it, letting attackers change the intended command. This is called OS command injection. View in Corgea ↗

More Details

🎟️Issue Explanation: This code runs an OS command using user input without sanitizing it, letting attackers change the intended command. This is called OS command injection.

- The code uses "execSync(`git checkout ${baseBranch}`)" where "baseBranch" can come from user input.
- Special characters in "baseBranch" (like ; or &&) can add extra commands, changing what runs on the system.
- An attacker could inject commands to run malicious code on the server, risking sensitive data or control over the system.

🪄Fix Explanation: The fix mitigates command injection by sanitizing the branch name input, validating against a strict regex, disallowing dangerous patterns, and switching from execSync with string commands to execFileSync with argument arrays, preventing shell interpretation.
"const sanitizedBaseBranch = baseBranch?.trim();" trims input to remove unwanted whitespace for accurate validation.

💡Important Instructions: Ensure all other branch inputs that reach OS commands undergo similar sanitization before use. Also, validate that error handling for invalid branches properly prevents further execution.

Suggested change

	execSync(`git checkout ${baseBranch}`, { cwd: repoPath, stdio: 'ignore' });
	const baseResult = await orchestrator.orchestrate(repoPath, 'base', undefined, {
	includeAllSeverities: mode === 'complete',
	analysisMode: mode
	});
	const baseIssues = baseResult.toolResults.flatMap(t => t.issues);
	const sanitizedBaseBranch = baseBranch?.trim();
	if (!sanitizedBaseBranch || !/^[A-Za-z0-9._\/-]+$/.test(sanitizedBaseBranch) || sanitizedBaseBranch.startsWith('-') || sanitizedBaseBranch.includes('..') || sanitizedBaseBranch.includes('@{')) {
	throw new Error('Invalid base branch name');
	}
	(require('node:child_process').execFileSync)('git', ['checkout', sanitizedBaseBranch], { cwd: repoPath, stdio: 'ignore', timeout: 15000 });

[corgea]

OS Command Injection ( 🔴 Critical ) - The code runs a Git command using external input without cleaning it, risking attackers changing the command to harm the system. View in Corgea ↗

More Details

🎟️Issue Explanation: The code runs a Git command using external input without cleaning it, risking attackers changing the command to harm the system.

- Uses "execSync" with "request.prNumber" directly in a shell command, allowing injection of malicious input.
- An attacker could supply a value like "1; rm -rf /" to run harmful commands after "git fetch".
- Since the command runs in "repoPath", it could delete or change important project files, causing damage.

🪄Fix Explanation: The fix prevents command injection by validating that the PR number contains only digits and switches from shell command execution to a safer execFileSync call with arguments array, eliminating shell interpretation risks.
- Validates "request.prNumber" with "/^\d+$/" to ensure only numeric input is accepted, blocking malicious characters.
- Converts "prNumber" to string and trims it to avoid extra whitespace that could affect validation.
- Constructs the fetch reference string with the sanitized "prNumberStr" ensuring no injection vectors remain.
- Replaces "execSync" with "execFileSync" passing command and arguments as separate parameters, preventing shell interpolation.
- Maintains the same functionality by fetching the PR branch from origin with secure parameter handling.

💡Important Instructions: Ensure that all inputs to request.prNumber are properly set and that no other parts of this method depend on unvalidated PR numbers.

Suggested change

	execSync(`git fetch origin pull/${request.prNumber}/head:${prBranch}`, { cwd: repoPath, stdio: 'inherit' });
	}
	// Get modified files
	execSync(`git checkout ${prBranch}`, { cwd: repoPath, stdio: 'ignore' });
	const prNumberStr = String(request.prNumber ?? '').trim();
	if (!/^\d+$/.test(prNumberStr)) { throw new Error('Invalid PR number format'); }
	const fetchRef = `pull/${prNumberStr}/head:${prBranch}`;
	require('child_process').execFileSync('git', ['fetch', 'origin', fetchRef], { cwd: repoPath, stdio: 'inherit' });

[corgea]

OS Command Injection ( 🔴 Critical ) - The code runs a system command using external input without sanitizing it, risking command injection where attackers can run harmful commands. View in Corgea ↗

More Details

🎟️Issue Explanation: The code runs a system command using external input without sanitizing it, risking command injection where attackers can run harmful commands.

- The command "execSync(`git rev-parse --verify ${prBranch}`)" uses "prBranch" directly, which can include malicious characters.
- Attackers can inject extra shell commands by appending something like "; rm -rf /" to "prBranch", causing destructive behavior.
- This is risky because the code runs OS commands and "prBranch" is externally influenced, meaning user input or upstream sources can exploit it.

🪄Fix Explanation: The fix replaces unsafe command interpolation with validated inputs and uses "spawnSync" to avoid shell injection, ensuring only numeric PR numbers and safe branch names are accepted before running git commands.
- Replaces "execSync" with "spawnSync" and arguments array to prevent shell injection via string interpolation.
- Validates "request.prNumber" using regex "/^\d+$/", allowing only numeric PR numbers.
- Validates "prBranch" with "/^[A-Za-z0-9._/-]+$/" to allow only safe branch name characters.
- Uses explicit error throwing for invalid inputs or failed git operations to stop unsafe command execution.
- Adds timeout and disables shell usage ("shell: false") in "spawnSync" for additional security.

💡Important Instructions: Ensure calling code only provides request.prNumber and prBranch values consistent with the new validation rules to avoid runtime errors.

Suggested change

	execSync(`git rev-parse --verify ${prBranch}`, { cwd: repoPath, stdio: 'ignore' });
	} catch {
	console.log(` Fetching PR branch...`);
	execSync(`git fetch origin pull/${request.prNumber}/head:${prBranch}`, { cwd: repoPath, stdio: 'inherit' });
	}
	// Get modified files
	execSync(`git checkout ${prBranch}`, { cwd: repoPath, stdio: 'ignore' });
	const modifiedFiles = new Set(getModifiedFilesBetweenBranches(repoPath, baseBranch, prBranch));
	console.log(` ✅ Repository ready`);
	console.log(` Base branch: ${baseBranch}`);
	console.log(` PR branch: ${prBranch}`);
	console.log(` Modified files: ${modifiedFiles.size}\n`);
	return { repoPath, baseBranch, prBranch, modifiedFiles };
	const prNumberStr = String(request.prNumber);
	if (!/^\d+$/.test(prNumberStr)) { throw new Error('Invalid PR number'); }
	const safePrBranch = String(prBranch);
	if (!/^[A-Za-z0-9._/-]+$/.test(safePrBranch)) { throw new Error('Invalid branch name'); }
	const { spawnSync } = require('child_process');
	const verifyResult = spawnSync('git', ['rev-parse', '--verify', safePrBranch], { cwd: repoPath, stdio: 'ignore', shell: false, timeout: 15000 });
	if (verifyResult.status !== 0) { throw new Error('PR branch not verified'); }
	} catch {
	console.log(` Fetching PR branch...`);
	const prNumberStr2 = String(request.prNumber);
	if (!/^\d+$/.test(prNumberStr2)) { throw new Error('Invalid PR number'); }
	const safePrBranch2 = String(prBranch);
	if (!/^[A-Za-z0-9._/-]+$/.test(safePrBranch2)) { throw new Error('Invalid branch name'); }
	const { spawnSync } = require('child_process');
	const fetchRef = `pull/${prNumberStr2}/head:${safePrBranch2}`;

[corgea]

OS Command Injection ( 🔴 Critical ) - The code builds a command using user input without cleaning it, risking malicious commands running unexpectedly. This can let attackers run harmful OS commands. View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds a command using user input without cleaning it, risking malicious commands running unexpectedly. This can let attackers run harmful OS commands.

- The command string "cmd" includes input like "repoPath" and "excludePaths" directly, allowing injection of shell metacharacters.

- An attacker can manipulate "excludePaths" to inject extra commands, e.g., adding ""; rm -rf /"" to run harmful OS commands.

- This affects sensitive operations since "trufflehog" scans repos for secrets, risking unauthorized access or system damage.

🪄Fix Explanation: The fix prevents OS command injection by validating inputs used to construct the command string, ensuring only safe characters are allowed in "repoPath" and "excludePaths". It also sanitizes "maxDepth" by enforcing numeric bounds.
- Added a regex "SAFE_PATH = /^[A-Za-z0-9._\s\/\\:-]+$/" to validate "repoPath", returning an error if invalid, thus blocking malicious input.
- Sanitized "maxDepth": converted to number, checked for integer and limited to 1-100000 to prevent injection through non-numeric or extreme values.
- Added validation for each "exclude" path, only appending it to the command if it passes the "SAFE_PATH" test and is a string.
- These checks ensure no special shell characters or unexpected inputs can break out of intended command parameters.

💡Important Instructions: Ensure that all inputs arriving at this function are pre-validated or sanitized similarly, and monitor logs for any rejected inputs indicating potential attacks or misconfigurations.

diff --git a/packages/agents/src/two-branch/tools/universal/secret-scanner.ts b/packages/agents/src/two-branch/tools/universal/secret-scanner.ts
index df4e957..b5d9159 100644
--- a/packages/agents/src/two-branch/tools/universal/secret-scanner.ts
+++ b/packages/agents/src/two-branch/tools/universal/secret-scanner.ts
@@ -261,12 +261,25 @@ async function runTruffleHog(
     }
 
     // Build command
+    // Validate inputs to prevent OS command injection
+    const SAFE_PATH = /^[A-Za-z0-9._\s\/\\:-]+$/;
+    if (!SAFE_PATH.test(repoPath)) {
+      return {
+        tool: 'trufflehog',
+        issues: [],
+        scanDuration: Date.now() - startTime,
+        error: 'Invalid repo path'
+      };
+    }
     let cmd = `trufflehog filesystem "${repoPath}" --json`;
 
     if (config.scanHistory) {
       cmd = `trufflehog git "file://${repoPath}" --json`;
       if (config.maxDepth) {
-        cmd += ` --max-depth=${config.maxDepth}`;
+        const depth = Number(config.maxDepth);
+        if (Number.isInteger(depth) && depth > 0 && depth <= 100000) {
+          cmd += ` --max-depth=${depth}`;
+        }
       }
     }
 
@@ -277,7 +290,9 @@ async function runTruffleHog(
     // Add exclusions
     if (config.excludePaths && config.excludePaths.length > 0) {
       for (const exclude of config.excludePaths) {
-        cmd += ` --exclude-paths="${exclude}"`;
+        if (typeof exclude === 'string' && SAFE_PATH.test(exclude)) {
+          cmd += ` --exclude-paths="${exclude}"`;
+        }
       }
     }
 

To apply the fix, Download .patch.

[corgea]

OS Command Injection ( 🔴 Critical ) - The code uses external input to build a file path without validating it, risking unintended OS command execution via manipulated input. View in Corgea ↗

More Details

🎟️Issue Explanation: The code uses external input to build a file path without validating it, risking unintended OS command execution via manipulated input.

- The code builds "gemfilePath" using "repoPath", which may come from outside the program and can be manipulated.
- If "repoPath" contains special characters or commands, it could change how OS commands run later with "gemfilePath".
- Attackers can exploit this by injecting crafted input to execute malicious commands when the path is used in shell operations, risking security.

We could not generate a fix for this.

[corgea]

OS Command Injection ( 🔴 High ) - The code builds a command string using user input but doesn’t safely handle special characters, risking unintended OS commands running. This is OS Command Injection. View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds a command string using user input but doesn’t safely handle special characters, risking unintended OS commands running. This is OS Command Injection.

- The command string "cmd" is built with unchecked inputs like "repoPath", "config.frameworks", "config.excludePaths", and "config.skipChecks".
- If an attacker includes special symbols like ";" or "&" in inputs, they can inject extra commands, e.g., "repoPath="path; rm -rf /"".
- The code runs the constructed command with "execAsync(cmd)", executing injected commands on the OS, posing risks especially since it processes sensitive infrastructure configs.

🪄Fix Explanation: The fix prevents command injection by validating inputs with strict regex patterns and switching from shell string commands to "execFile" with argument arrays, avoiding shell interpretation of special characters.
- Input validation is added using regexes: "isSafePath" ensures "repoPath" and "excludePaths" contain only safe characters; "isSafeId" validates "skipChecks".
- Unsafe inputs cause immediate errors via "throw new Error", preventing execution with malicious data.
- Command construction switches from a concatenated string "cmd" to an argument array "args", passed to "execFileAsync", which executes commands without a shell.
- Loops explicitly push safe paths and checks into "args" to avoid string interpolation vulnerabilities seen in previous string concatenation.
- This approach blocks injection of shell metacharacters like ";&|`$<>\n()" effectively mitigating the OS command injection risk.

💡Important Instructions: Ensure that any upstream or downstream usage of repoPath, excludePaths, or skipChecks respects the validated safe character constraints to maintain overall security.

diff --git a/packages/agents/src/two-branch/tools/universal/iac-scanner.ts b/packages/agents/src/two-branch/tools/universal/iac-scanner.ts
index f60cc6d..b8cf2ac 100644
--- a/packages/agents/src/two-branch/tools/universal/iac-scanner.ts
+++ b/packages/agents/src/two-branch/tools/universal/iac-scanner.ts
@@ -126,8 +126,26 @@ async function runCheckov(
       };
     }
 
+    const execFileAsync = promisify(require('child_process').execFile);
+    const invalidChars = /[;&|`$<>\n()]/;
+    const pathAllowed = /^[a-zA-Z0-9._\-\/\\]+$/;
+    const idAllowed = /^[A-Za-z0-9_\-]+$/;
+    const isSafePath = (p: string) => typeof p === 'string' && !invalidChars.test(p) && pathAllowed.test(p);
+    const isSafeId   = (s: string) => typeof s === 'string' && !invalidChars.test(s) && idAllowed.test(s);
+    const repoPathSafe = repoPath;
+    if (!isSafePath(repoPathSafe)) {
+      throw new Error('Invalid repoPath: contains forbidden characters');
+    }
+    const safeExclude = (config.excludePaths || []).filter(isSafePath);
+    if (config.excludePaths && safeExclude.length !== config.excludePaths.length) {
+      throw new Error('Invalid excludePaths: contains forbidden characters');
+    }
+    const safeSkip = (config.skipChecks || []).filter(isSafeId);
+    if (config.skipChecks && safeSkip.length !== config.skipChecks.length) {
+      throw new Error('Invalid skipChecks: contains forbidden characters');
+    }
     // Build command
-    let cmd = `checkov -d "${repoPath}" -o json --compact`;
+    const args: string[] = ['-d', repoPathSafe, '-o', 'json', '--compact'];
 
     // Add framework filters
     if (config.frameworks && config.frameworks.length > 0) {
@@ -147,24 +165,24 @@ async function runCheckov(
         .map(f => frameworkMap[f])
         .filter(f => f);
       if (frameworks.length > 0) {
-        cmd += ` --framework ${frameworks.join(',')}`;
+        args.push('--framework', frameworks.join(','));
       }
     }
 
     // Add exclusions
     if (config.excludePaths && config.excludePaths.length > 0) {
-      cmd += ` --skip-path ${config.excludePaths.join(' --skip-path ')}`;
+      for (const p of safeExclude) { args.push('--skip-path', p); }
     }
 
     // Add skip checks
     if (config.skipChecks && config.skipChecks.length > 0) {
-      cmd += ` --skip-check ${config.skipChecks.join(',')}`;
+      args.push('--skip-check', safeSkip.join(','));
     }
 
     // Run checkov
     let stdout = '';
     try {
-      const result = await execAsync(cmd, {
+      const result = await execFileAsync('checkov', args, {
         maxBuffer: 100 * 1024 * 1024,
         timeout: 600000  // 10 minute timeout
       });

To apply the fix, Download .patch.

[corgea]

OS Command Injection ( 🔴 Critical ) - The code runs an OS command using a path from outside input without cleaning it, risking command injection. Attackers can alter the command to run unintended actions. View in Corgea ↗

More Details

🎟️Issue Explanation: The code runs an OS command using a path from outside input without cleaning it, risking command injection. Attackers can alter the command to run unintended actions.

- The command runs "trufflehog filesystem "${repoPath}" --json", using "repoPath" without filtering dangerous characters.
- If "repoPath" contains shell metacharacters (e.g., ;, &, |), an attacker could insert extra commands.
- This can lead to running harmful commands, risking data or system integrity, since "trufflehog" handles sensitive info from repositories.

We could not generate a fix for this.

[corgea]

OS Command Injection ( 🔴 Critical ) - The code builds an OS command with user input without proper checks, letting attackers alter or add commands unexpectedly. View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds an OS command with user input without proper checks, letting attackers alter or add commands unexpectedly.

- It adds "exclude" values from "config.excludePaths" directly into "cmd" without filtering special chars.
- Malicious input like ""; rm -rf / #"" could execute harmful commands by breaking out of the intended argument.
- This happens because the input isn't sanitized, allowing command injection and risking critical system control or data loss.

🪄Fix Explanation: The fix sanitizes and validates user-provided exclude paths by rejecting those with dangerous shell metacharacters and ensuring paths remain within the repository root, preventing command injection and directory traversal in the OS command construction.
- Added a regex "metacharRegex = /[;&|`$<>\n()]/" to detect dangerous shell metacharacters in each exclude path.
- Skips any exclude path containing metacharacters or if not a string, preventing shell command injection via malformed input.
- Converts exclude paths to absolute paths relative to the repo root using "path.resolve(repoRoot, exclude)" for normalization.
- Ensures each absolute exclude path is inside the repository root by checking "excludeAbs.startsWith(repoRoot + path.sep)", avoiding directory traversal.
- Only sanitized, validated, and safe absolute paths are appended to the command with "--exclude-paths="${excludeAbs}"", neutralizing unsafe input.

💡Important Instructions: Verify no downstream components modify or reuse excludePaths assuming original unsanitized input; ensure any logging avoids exposing raw inputs to prevent info leaks.

Suggested change

	if (config.excludePaths && config.excludePaths.length > 0) {
	for (const exclude of config.excludePaths) {
	cmd += ` --exclude-paths="${exclude}"`;
	}
	}
	// Run trufflehog
	const { stdout } = await execAsync(cmd, {
	maxBuffer: 100 * 1024 * 1024,
	timeout: 300000 // 5 minute timeout
	});
	// Parse JSONL output (one JSON object per line)
	if (stdout.trim()) {
	const lines = stdout.trim().split('\n');
	for (const line of lines) {
	if (config.excludePaths && config.excludePaths.length > 0) {
	const repoRoot = path.resolve(repoPath);
	const metacharRegex = /[;&|`$<>\n()]/;
	for (const exclude of config.excludePaths) {
	if (typeof exclude !== 'string' || metacharRegex.test(exclude)) {
	continue;
	}
	const excludeAbs = path.resolve(repoRoot, exclude);
	// Ensure exclude path is within the repository root to prevent traversal
	if (excludeAbs !== repoRoot && !excludeAbs.startsWith(repoRoot + path.sep)) {
	continue;
	}
	cmd += ` --exclude-paths="${excludeAbs}"`;
	}
	}

[corgea]

OS Command Injection ( 🔴 Critical ) - The code builds a system command using user input without properly cleaning it, risking attackers changing the command and causing unexpected actions. View in Corgea ↗

More Details

🎟️Issue Explanation: The code builds a system command using user input without properly cleaning it, risking attackers changing the command and causing unexpected actions.

- The variable "cmd" is constructed by inserting values from "repoPath" and "config.excludePaths" directly into the command string.
- An attacker could add special characters like semicolons or backticks in "repoPath" to run extra commands when "cmd" is executed.
- Since the command scans repositories, this exploit could lead to unauthorized access or disclosure of sensitive project files.

We could not generate a fix for this.