From bb7ade91ab16b7790692ca022b2abbe74034adc4 Mon Sep 17 00:00:00 2001 From: Tim Strazzere Date: Tue, 24 Feb 2026 13:42:29 -0800 Subject: [PATCH] fix: ensure exec usage does not allow poisoning Previous usage was vulnerable to "poisoned" tags which could enduce commands to be run when a `detectChanges` command was hit. This was primarily fixed in `local-backend.ts` however I changes the `execSync` usages where any injection was potentially able to be performed (e.g. staleness). Skipped touching wiki and generator as those use static input, though these should potentially be changed over in the future. --- gitnexus/src/cli/wiki.ts | 11 +++++---- gitnexus/src/core/wiki/generator.ts | 6 ++--- gitnexus/src/mcp/local/local-backend.ts | 32 +++++++++++++++---------- gitnexus/src/mcp/staleness.ts | 6 ++--- 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/gitnexus/src/cli/wiki.ts b/gitnexus/src/cli/wiki.ts index ac626fe809..70ab00785b 100644 --- a/gitnexus/src/cli/wiki.ts +++ b/gitnexus/src/cli/wiki.ts @@ -7,7 +7,7 @@ import path from 'path'; import readline from 'readline'; -import { execSync } from 'child_process'; +import { execSync, execFileSync } from 'child_process'; import cliProgress from 'cli-progress'; import { getGitRoot, isGitRepo } from '../storage/git.js'; import { getStoragePaths, loadMeta, loadCLIConfig, saveCLIConfig } from '../storage/repo-manager.js'; @@ -343,10 +343,11 @@ function hasGhCLI(): boolean { function publishGist(htmlPath: string): { url: string; rawUrl: string } | null { try { - const output = execSync( - `gh gist create "${htmlPath}" --desc "Repository Wiki — generated by GitNexus" --public`, - { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }, - ).trim(); + const output = execFileSync('gh', [ + 'gist', 'create', htmlPath, + '--desc', 'Repository Wiki — generated by GitNexus', + '--public', + ], { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }).trim(); // gh gist create prints the gist URL as the last line const lines = output.split('\n'); diff --git a/gitnexus/src/core/wiki/generator.ts b/gitnexus/src/core/wiki/generator.ts index 29a16a5419..666dc6e463 100644 --- a/gitnexus/src/core/wiki/generator.ts +++ b/gitnexus/src/core/wiki/generator.ts @@ -12,7 +12,7 @@ import fs from 'fs/promises'; import path from 'path'; -import { execSync } from 'child_process'; +import { execSync, execFileSync } from 'child_process'; import { initWikiDb, @@ -712,8 +712,8 @@ export class WikiGenerator { private getChangedFiles(fromCommit: string, toCommit: string): string[] { try { - const output = execSync( - `git diff ${fromCommit}..${toCommit} --name-only`, + const output = execFileSync( + 'git', ['diff', `${fromCommit}..${toCommit}`, '--name-only'], { cwd: this.repoPath }, ).toString().trim(); return output ? output.split('\n').filter(Boolean) : []; diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 9ae45ee6ad..4f7bbfd8c7 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -988,30 +988,30 @@ export class LocalBackend { await this.ensureInitialized(repo.id); const scope = params.scope || 'unstaged'; - const { execSync } = await import('child_process'); - - // Build git diff command based on scope - let diffCmd: string; + const { execFileSync } = await import('child_process'); + + // Build git diff args based on scope (using execFileSync to avoid shell injection) + let diffArgs: string[]; switch (scope) { case 'staged': - diffCmd = 'git diff --staged --name-only'; + diffArgs = ['diff', '--staged', '--name-only']; break; case 'all': - diffCmd = 'git diff HEAD --name-only'; + diffArgs = ['diff', 'HEAD', '--name-only']; break; case 'compare': if (!params.base_ref) return { error: 'base_ref is required for "compare" scope' }; - diffCmd = `git diff ${params.base_ref} --name-only`; + diffArgs = ['diff', params.base_ref, '--name-only']; break; case 'unstaged': default: - diffCmd = 'git diff --name-only'; + diffArgs = ['diff', '--name-only']; break; } - + let changedFiles: string[]; try { - const output = execSync(diffCmd, { cwd: repo.repoPath, encoding: 'utf-8' }); + const output = execFileSync('git', diffArgs, { cwd: repo.repoPath, encoding: 'utf-8' }); changedFiles = output.trim().split('\n').filter(f => f.length > 0); } catch (err: any) { return { error: `Git diff failed: ${err.message}` }; @@ -1185,9 +1185,15 @@ export class LocalBackend { // Simple text search across the repo for the old name (in files not already covered by graph) try { - const { execSync } = await import('child_process'); - const rgCmd = `rg -l --type-add "code:*.{ts,tsx,js,jsx,py,go,rs,java}" -t code "\\b${oldName}\\b" .`; - const output = execSync(rgCmd, { cwd: repo.repoPath, encoding: 'utf-8', timeout: 5000 }); + const { execFileSync } = await import('child_process'); + const rgArgs = [ + '-l', + '--type-add', 'code:*.{ts,tsx,js,jsx,py,go,rs,java}', + '-t', 'code', + `\\b${oldName}\\b`, + '.', + ]; + const output = execFileSync('rg', rgArgs, { cwd: repo.repoPath, encoding: 'utf-8', timeout: 5000 }); const files = output.trim().split('\n').filter(f => f.length > 0); for (const file of files) { diff --git a/gitnexus/src/mcp/staleness.ts b/gitnexus/src/mcp/staleness.ts index 0b7cf2a15e..8c044e61dd 100644 --- a/gitnexus/src/mcp/staleness.ts +++ b/gitnexus/src/mcp/staleness.ts @@ -5,7 +5,7 @@ * Returns a hint for the LLM to call analyze if stale. */ -import { execSync } from 'child_process'; +import { execFileSync } from 'child_process'; import path from 'path'; export interface StalenessInfo { @@ -20,8 +20,8 @@ export interface StalenessInfo { export function checkStaleness(repoPath: string, lastCommit: string): StalenessInfo { try { // Get count of commits between lastCommit and HEAD - const result = execSync( - `git rev-list --count ${lastCommit}..HEAD`, + const result = execFileSync( + 'git', ['rev-list', '--count', `${lastCommit}..HEAD`], { cwd: repoPath, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] } ).trim();