Skip to content

Commit 8a15720

Browse files
committed
fix(compare): exclude git-ignored files and improve git command security
- Fix false positives where git-ignored files appeared as "added" components - Filter git-ignored files from comparison results and recalculate counts - Add isGitIgnored() and filterGitIgnoredFiles() utilities Security: Refactor execGit() to use spawn instead of exec to prevent command injection. spawn executes without shell interpretation and properly handles file paths with spaces/special characters.
1 parent 378ab58 commit 8a15720

File tree

4 files changed

+575
-58
lines changed

4 files changed

+575
-58
lines changed

src/cli/handlers/compareHandler.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
removeWorktree,
2424
createBaselinePaths,
2525
cleanupBaselinePaths,
26+
filterGitIgnoredFiles,
2627
type GitBaselinePaths,
2728
} from '../../utils/git.js';
2829

@@ -461,7 +462,70 @@ async function handleGitBaselineCompare(options: {
461462
gitBaseline: true, // Enable path normalization for git baseline comparisons
462463
};
463464

464-
const result = await multiFileCompare(multiCompareOptions);
465+
let result = await multiFileCompare(multiCompareOptions);
466+
467+
// Filter out git-ignored files from comparison results
468+
// This prevents false positives where git-ignored files (like next-env.d.ts)
469+
// exist in working directory but not in git worktree
470+
const projectRoot = process.cwd();
471+
for (const folder of result.folders) {
472+
if (folder.componentResult) {
473+
const cr = folder.componentResult;
474+
// Filter added components that are git-ignored
475+
cr.added = await filterGitIgnoredFiles(cr.added, projectRoot);
476+
// Filter removed components that are git-ignored
477+
cr.removed = await filterGitIgnoredFiles(cr.removed, projectRoot);
478+
// Filter changed components that are git-ignored
479+
cr.changed = (await Promise.all(
480+
cr.changed.map(async ({ id, deltas }) => {
481+
const filtered = await filterGitIgnoredFiles([id], projectRoot);
482+
return filtered.length > 0 ? { id, deltas } : null;
483+
})
484+
)).filter((item): item is { id: string; deltas: any[] } => item !== null);
485+
486+
// Recalculate status if all changes were filtered out
487+
if (cr.added.length === 0 && cr.removed.length === 0 && cr.changed.length === 0) {
488+
folder.status = 'PASS';
489+
// Update component result status as well
490+
cr.status = 'PASS';
491+
}
492+
}
493+
}
494+
495+
// Recalculate summary counts after filtering
496+
const addedFolders = result.folders.filter(f => f.status === 'ADDED').length;
497+
const orphanedFolders = result.folders.filter(f => f.status === 'ORPHANED').length;
498+
const driftFolders = result.folders.filter(f => f.status === 'DRIFT').length;
499+
const passFolders = result.folders.filter(f => f.status === 'PASS').length;
500+
501+
// Recalculate component counts from filtered results
502+
let totalComponentsAdded = 0;
503+
let totalComponentsRemoved = 0;
504+
let totalComponentsChanged = 0;
505+
506+
for (const folder of result.folders) {
507+
if (folder.componentResult && folder.status === 'DRIFT') {
508+
totalComponentsAdded += folder.componentResult.added.length;
509+
totalComponentsRemoved += folder.componentResult.removed.length;
510+
totalComponentsChanged += folder.componentResult.changed.length;
511+
}
512+
}
513+
514+
// Update summary
515+
result.summary = {
516+
totalFolders: result.folders.length,
517+
addedFolders,
518+
orphanedFolders,
519+
driftFolders,
520+
passFolders,
521+
totalComponentsAdded,
522+
totalComponentsRemoved,
523+
totalComponentsChanged,
524+
};
525+
526+
// Recalculate overall status
527+
result.status = addedFolders > 0 || orphanedFolders > 0 || driftFolders > 0 ? 'DRIFT' : 'PASS';
528+
465529
displayMultiFileCompareResult(result, stats, quiet);
466530

467531
// Step 5: Clean up

src/utils/git.ts

Lines changed: 128 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,12 @@
33
* Handles worktree creation, ref validation, and cleanup
44
*/
55

6-
import { exec } from 'node:child_process';
7-
import { promisify } from 'node:util';
6+
import { spawn } from 'node:child_process';
87
import { join } from 'node:path';
98
import { access, rm, mkdir } from 'node:fs/promises';
109
import { tmpdir } from 'node:os';
1110
import { debugLog, debugError } from './debug.js';
1211

13-
const execAsync = promisify(exec);
14-
1512
/**
1613
* Result from creating a git worktree
1714
*/
@@ -36,6 +33,7 @@ export interface GitOptions {
3633

3734
/**
3835
* Execute a git command and return stdout
36+
* Uses spawn instead of exec for better security (no shell interpretation)
3937
* @throws Error if git command fails
4038
*/
4139
async function execGit(
@@ -48,33 +46,57 @@ async function execGit(
4846

4947
debugLog('git', `Executing: ${command}`, { cwd });
5048

51-
try {
52-
const { stdout, stderr } = await execAsync(command, {
49+
return new Promise((resolve, reject) => {
50+
const child = spawn('git', args, {
5351
cwd,
5452
timeout,
55-
maxBuffer: 10 * 1024 * 1024, // 10MB buffer for large repos
53+
stdio: ['ignore', 'pipe', 'pipe'],
5654
});
5755

58-
if (stderr && !stderr.includes('Preparing worktree')) {
59-
// Some git commands output to stderr even on success
60-
debugLog('git', `stderr: ${stderr.trim()}`);
56+
let stdout = '';
57+
let stderr = '';
58+
59+
// TypeScript needs explicit type assertion for stdio pipes
60+
if (child.stdout) {
61+
child.stdout.on('data', (data: Buffer) => {
62+
stdout += data.toString();
63+
});
6164
}
6265

63-
return stdout.trim();
64-
} catch (error) {
65-
const err = error as Error & { stderr?: string; code?: number };
66-
debugError('git', 'execGit', {
67-
command,
68-
cwd,
69-
message: err.message,
70-
stderr: err.stderr,
71-
code: err.code,
66+
if (child.stderr) {
67+
child.stderr.on('data', (data: Buffer) => {
68+
stderr += data.toString();
69+
});
70+
}
71+
72+
child.on('error', (error: Error) => {
73+
debugError('git', 'execGit spawn error', {
74+
command,
75+
cwd,
76+
message: error.message,
77+
});
78+
reject(new Error(`Git command failed: ${error.message}`));
7279
});
7380

74-
// Extract meaningful error message from stderr
75-
const errorMessage = err.stderr?.trim() || err.message;
76-
throw new Error(`Git command failed: ${errorMessage}`);
77-
}
81+
child.on('close', (code: number | null) => {
82+
if (code === 0) {
83+
if (stderr && !stderr.includes('Preparing worktree')) {
84+
// Some git commands output to stderr even on success
85+
debugLog('git', `stderr: ${stderr.trim()}`);
86+
}
87+
resolve(stdout.trim());
88+
} else {
89+
debugError('git', 'execGit', {
90+
command,
91+
cwd,
92+
code,
93+
stderr: stderr.trim(),
94+
});
95+
const errorMessage = stderr.trim() || `Command failed with exit code ${code}`;
96+
reject(new Error(`Git command failed: ${errorMessage}`));
97+
}
98+
});
99+
});
78100
}
79101

80102
/**
@@ -362,6 +384,89 @@ export async function createBaselinePaths(
362384
};
363385
}
364386

387+
/**
388+
* Check if a file is git-ignored
389+
* @param filePath - File path relative to git root or absolute path
390+
* @param options - Git options
391+
* @returns true if the file is git-ignored, false otherwise
392+
*/
393+
export async function isGitIgnored(
394+
filePath: string,
395+
options: GitOptions = {}
396+
): Promise<boolean> {
397+
try {
398+
// git check-ignore --quiet returns exit code 0 if file is ignored, 1 if not ignored
399+
// execGit throws on non-zero exit codes, so if it succeeds, file is ignored
400+
await execGit(['check-ignore', '--quiet', filePath], options);
401+
return true; // Command succeeded, file is ignored
402+
} catch {
403+
// Command failed (exit code 1), file is not ignored
404+
return false;
405+
}
406+
}
407+
408+
/**
409+
* Filter out git-ignored files from an array of file paths
410+
* @param filePaths - Array of file paths to filter (may be normalized basenames in git baseline mode)
411+
* @param projectRoot - Project root directory (for resolving relative paths)
412+
* @param options - Git options
413+
* @returns Array of file paths that are NOT git-ignored
414+
*/
415+
export async function filterGitIgnoredFiles(
416+
filePaths: string[],
417+
projectRoot: string,
418+
options: GitOptions = {}
419+
): Promise<string[]> {
420+
const { relative } = await import('node:path');
421+
422+
const filtered: string[] = [];
423+
424+
for (const filePath of filePaths) {
425+
let isIgnored = false;
426+
427+
// If the path contains a slash, it's a full relative path
428+
if (filePath.includes('/')) {
429+
// Convert to relative path from project root if needed
430+
let relativePath: string;
431+
if (filePath.startsWith('/') || filePath.match(/^[A-Z]:/)) {
432+
// Absolute path - convert to relative
433+
relativePath = relative(projectRoot, filePath).replace(/\\/g, '/');
434+
} else {
435+
relativePath = filePath;
436+
}
437+
438+
// Check if file is git-ignored
439+
isIgnored = await isGitIgnored(relativePath, { ...options, cwd: projectRoot });
440+
} else {
441+
// It's just a basename (normalized in git baseline mode)
442+
// Check if the basename itself is git-ignored (works for root-level files like next-env.d.ts)
443+
isIgnored = await isGitIgnored(filePath, { ...options, cwd: projectRoot });
444+
445+
// Also check common patterns that might match this basename
446+
if (!isIgnored) {
447+
// Check common git-ignore patterns that might match
448+
const patternsToCheck = [
449+
`**/${filePath}`,
450+
`*/${filePath}`,
451+
];
452+
453+
for (const pattern of patternsToCheck) {
454+
if (await isGitIgnored(pattern, { ...options, cwd: projectRoot })) {
455+
isIgnored = true;
456+
break;
457+
}
458+
}
459+
}
460+
}
461+
462+
if (!isIgnored) {
463+
filtered.push(filePath);
464+
}
465+
}
466+
467+
return filtered;
468+
}
469+
365470
/**
366471
* Clean up git baseline comparison directories
367472
*/

0 commit comments

Comments
 (0)