From 7ecaef7fc0d3237c827cda34e735c0927edf570b Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 15 Dec 2025 19:12:11 -0500 Subject: [PATCH] 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. --- .../cli/src/commands/update/utilities/git.ts | 85 +++++++++++++------ 1 file changed, 59 insertions(+), 26 deletions(-) diff --git a/packages/angular/cli/src/commands/update/utilities/git.ts b/packages/angular/cli/src/commands/update/utilities/git.ts index 631e5b9bb99e..0998c9c61fff 100644 --- a/packages/angular/cli/src/commands/update/utilities/git.ts +++ b/packages/angular/cli/src/commands/update/utilities/git.ts @@ -6,33 +6,57 @@ * found in the LICENSE file at https://angular.dev/license */ -import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; import * as path from 'node:path'; +/** + * Execute a git command. + * @param args Arguments to pass to the git command. + * @param input Optional input to pass to the command via stdin. + * @returns The output of the command. + */ +function execGit(args: string[], input?: string): string { + return execFileSync('git', args, { encoding: 'utf8', stdio: 'pipe', input }); +} + /** * Checks if the git repository is clean. - * @param root The root directory of the project. - * @returns True if the repository is clean, false otherwise. + * This function only checks for changes that are within the specified root directory. + * Changes outside the root directory are ignored. + * @param root The root directory of the project to check. + * @returns True if the repository is clean within the root, false otherwise. */ export function checkCleanGit(root: string): boolean { try { - const topLevel = execSync('git rev-parse --show-toplevel', { - encoding: 'utf8', - stdio: 'pipe', - }); - const result = execSync('git status --porcelain', { encoding: 'utf8', stdio: 'pipe' }); - if (result.trim().length === 0) { + const topLevel = execGit(['rev-parse', '--show-toplevel']); + const result = execGit(['status', '--porcelain', '-z']); + if (result.length === 0) { return true; } - // Only files inside the workspace root are relevant - for (const entry of result.split('\n')) { - const relativeEntry = path.relative( - path.resolve(root), - path.resolve(topLevel.trim(), entry.slice(3).trim()), - ); + const entries = result.split('\0'); + for (let i = 0; i < entries.length; i++) { + const line = entries[i]; + if (!line) { + continue; + } + + // Status is the first 2 characters. + // If the status is a rename ('R'), the next entry in the split array is the target path. + let filePath = line.slice(3); + const status = line.slice(0, 2); + if (status[0] === 'R') { + // Check the source path (filePath) + if (isPathInsideRoot(filePath, root, topLevel.trim())) { + return false; + } + + // The next entry is the target path of the rename. + i++; + filePath = entries[i]; + } - if (!relativeEntry.startsWith('..') && !path.isAbsolute(relativeEntry)) { + if (isPathInsideRoot(filePath, root, topLevel.trim())) { return false; } } @@ -41,15 +65,24 @@ export function checkCleanGit(root: string): boolean { return true; } +function isPathInsideRoot(filePath: string, root: string, topLevel: string): boolean { + const relativeEntry = path.relative(path.resolve(root), path.resolve(topLevel, filePath)); + + return !relativeEntry.startsWith('..') && !path.isAbsolute(relativeEntry); +} + /** * Checks if the working directory has pending changes to commit. - * @returns Whether or not the working directory has Git changes to commit. + * @returns Whether or not the working directory has Git changes to commit. Returns false if not in a Git repository. */ export function hasChangesToCommit(): boolean { - // List all modified files not covered by .gitignore. - // If any files are returned, then there must be something to commit. - - return execSync('git ls-files -m -d -o --exclude-standard').toString() !== ''; + try { + // List all modified files not covered by .gitignore. + // If any files are returned, then there must be something to commit. + return execGit(['ls-files', '-m', '-d', '-o', '--exclude-standard']).trim() !== ''; + } catch { + return false; + } } /** @@ -58,19 +91,19 @@ export function hasChangesToCommit(): boolean { */ export function createCommit(message: string) { // Stage entire working tree for commit. - execSync('git add -A', { encoding: 'utf8', stdio: 'pipe' }); + execGit(['add', '-A']); // Commit with the message passed via stdin to avoid bash escaping issues. - execSync('git commit --no-verify -F -', { encoding: 'utf8', stdio: 'pipe', input: message }); + execGit(['commit', '--no-verify', '-F', '-'], message); } /** - * Finds the Git SHA hash of the HEAD commit. - * @returns The Git SHA hash of the HEAD commit. Returns null if unable to retrieve the hash. + * Finds the full Git SHA hash of the HEAD commit. + * @returns The full Git SHA hash of the HEAD commit. Returns null if unable to retrieve the hash. */ export function findCurrentGitSha(): string | null { try { - return execSync('git rev-parse HEAD', { encoding: 'utf8', stdio: 'pipe' }).trim(); + return execGit(['rev-parse', 'HEAD']).trim(); } catch { return null; }