Skip to content

Commit 0578dfe

Browse files
committed
refactor(@angular/cli): standardize update command git utility execution
All `git` commands now use `execFileSync` instead of `execSync` to prevent shell injection vulnerabilities and provide more predictable execution. `checkCleanGit` now utilizes `git status --porcelain -z` for NUL-terminated output, ensuring correct handling of filenames with spaces or special characters, and preventing potential path trimming bugs. An `execGit` helper function was introduced to reduce code duplication and standardize `git` command execution options. `hasChangesToCommit` now gracefully handles non-Git repositories by returning `false` instead of throwing.
1 parent eb48d71 commit 0578dfe

File tree

1 file changed

+62
-26
lines changed
  • packages/angular/cli/src/commands/update/utilities

1 file changed

+62
-26
lines changed

packages/angular/cli/src/commands/update/utilities/git.ts

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,33 +6,57 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import { execSync } from 'node:child_process';
9+
import { execFileSync } from 'node:child_process';
1010
import * as path from 'node:path';
1111

12+
/**
13+
* Execute a git command.
14+
* @param args Arguments to pass to the git command.
15+
* @param input Optional input to pass to the command via stdin.
16+
* @returns The output of the command.
17+
*/
18+
function execGit(args: string[], input?: string): string {
19+
return execFileSync('git', args, { encoding: 'utf8', stdio: 'pipe', input });
20+
}
21+
1222
/**
1323
* Checks if the git repository is clean.
14-
* @param root The root directory of the project.
15-
* @returns True if the repository is clean, false otherwise.
24+
* This function only checks for changes that are within the specified root directory.
25+
* Changes outside the root directory are ignored.
26+
* @param root The root directory of the project to check.
27+
* @returns True if the repository is clean within the root, false otherwise.
1628
*/
1729
export function checkCleanGit(root: string): boolean {
1830
try {
19-
const topLevel = execSync('git rev-parse --show-toplevel', {
20-
encoding: 'utf8',
21-
stdio: 'pipe',
22-
});
23-
const result = execSync('git status --porcelain', { encoding: 'utf8', stdio: 'pipe' });
24-
if (result.trim().length === 0) {
31+
const topLevel = execGit(['rev-parse', '--show-toplevel']);
32+
const result = execGit(['status', '--porcelain', '-z']);
33+
if (result.length === 0) {
2534
return true;
2635
}
2736

28-
// Only files inside the workspace root are relevant
29-
for (const entry of result.split('\n')) {
30-
const relativeEntry = path.relative(
31-
path.resolve(root),
32-
path.resolve(topLevel.trim(), entry.slice(3).trim()),
33-
);
37+
const entries = result.split('\0');
38+
for (let i = 0; i < entries.length; i++) {
39+
const line = entries[i];
40+
if (!line) {
41+
continue;
42+
}
43+
44+
// Status is the first 2 characters.
45+
// If the status is a rename ('R'), the next entry in the split array is the target path.
46+
let filePath = line.slice(3);
47+
const status = line.slice(0, 2);
48+
if (status[0] === 'R') {
49+
// Check the source path (filePath)
50+
if (isPathInsideRoot(filePath, root, topLevel.trim())) {
51+
return false;
52+
}
53+
54+
// The next entry is the target path of the rename.
55+
i++;
56+
filePath = entries[i];
57+
}
3458

35-
if (!relativeEntry.startsWith('..') && !path.isAbsolute(relativeEntry)) {
59+
if (isPathInsideRoot(filePath, root, topLevel.trim())) {
3660
return false;
3761
}
3862
}
@@ -41,15 +65,27 @@ export function checkCleanGit(root: string): boolean {
4165
return true;
4266
}
4367

68+
function isPathInsideRoot(filePath: string, root: string, topLevel: string): boolean {
69+
const relativeEntry = path.relative(
70+
path.resolve(root),
71+
path.resolve(topLevel, filePath),
72+
);
73+
74+
return !relativeEntry.startsWith('..') && !path.isAbsolute(relativeEntry);
75+
}
76+
4477
/**
4578
* Checks if the working directory has pending changes to commit.
46-
* @returns Whether or not the working directory has Git changes to commit.
79+
* @returns Whether or not the working directory has Git changes to commit. Returns false if not in a Git repository.
4780
*/
4881
export function hasChangesToCommit(): boolean {
49-
// List all modified files not covered by .gitignore.
50-
// If any files are returned, then there must be something to commit.
51-
52-
return execSync('git ls-files -m -d -o --exclude-standard').toString() !== '';
82+
try {
83+
// List all modified files not covered by .gitignore.
84+
// If any files are returned, then there must be something to commit.
85+
return execGit(['ls-files', '-m', '-d', '-o', '--exclude-standard']).trim() !== '';
86+
} catch {
87+
return false;
88+
}
5389
}
5490

5591
/**
@@ -58,19 +94,19 @@ export function hasChangesToCommit(): boolean {
5894
*/
5995
export function createCommit(message: string) {
6096
// Stage entire working tree for commit.
61-
execSync('git add -A', { encoding: 'utf8', stdio: 'pipe' });
97+
execGit(['add', '-A']);
6298

6399
// Commit with the message passed via stdin to avoid bash escaping issues.
64-
execSync('git commit --no-verify -F -', { encoding: 'utf8', stdio: 'pipe', input: message });
100+
execGit(['commit', '--no-verify', '-F', '-'], message);
65101
}
66102

67103
/**
68-
* Finds the Git SHA hash of the HEAD commit.
69-
* @returns The Git SHA hash of the HEAD commit. Returns null if unable to retrieve the hash.
104+
* Finds the full Git SHA hash of the HEAD commit.
105+
* @returns The full Git SHA hash of the HEAD commit. Returns null if unable to retrieve the hash.
70106
*/
71107
export function findCurrentGitSha(): string | null {
72108
try {
73-
return execSync('git rev-parse HEAD', { encoding: 'utf8', stdio: 'pipe' }).trim();
109+
return execGit(['rev-parse', 'HEAD']).trim();
74110
} catch {
75111
return null;
76112
}

0 commit comments

Comments
 (0)