refactor(@angular/cli): harden git utilities and improve JSDoc

This commit refactors the git utility functions in `@angular/cli` to enhance robustness, security, and code clarity.

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.

Author: clydin

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,27 @@ 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 +94,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;
   }

packages/angular_devkit/architect_cli/BUILD.bazel

@@ -19,7 +19,6 @@ ts_project(
     deps = [
         ":node_modules/@angular-devkit/architect",
         ":node_modules/@angular-devkit/core",
-        ":node_modules/ansi-colors",
         ":node_modules/yargs-parser",
         "//:node_modules/@types/node",
         "//:node_modules/@types/yargs-parser",

packages/angular_devkit/architect_cli/bin/architect.ts

@@ -11,9 +11,9 @@ import { Architect } from '@angular-devkit/architect';
 import { WorkspaceNodeModulesArchitectHost } from '@angular-devkit/architect/node';
 import { JsonValue, json, logging, schema, tags, workspaces } from '@angular-devkit/core';
 import { NodeJsSyncHost, createConsoleLogger } from '@angular-devkit/core/node';
-import * as ansiColors from 'ansi-colors';
 import { existsSync } from 'node:fs';
 import * as path from 'node:path';
+import { styleText } from 'node:util';
 import yargsParser, { camelCase, decamelize } from 'yargs-parser';
 
 function findUp(names: string | string[], from: string) {
@@ -58,9 +58,6 @@ function usage(logger: logging.Logger, exitCode = 0): never {
   return process.exit(exitCode);
 }
 
-// Create a separate instance to prevent unintended global changes to the color configuration
-const colors = ansiColors.create();
-
 async function _executeTarget(
   parentLogger: logging.Logger,
   workspace: workspaces.WorkspaceDefinition,
@@ -100,9 +97,9 @@ async function _executeTarget(
   try {
     const result = await run.lastOutput;
     if (result.success) {
-      parentLogger.info(colors.green('SUCCESS'));
+      parentLogger.info(styleText(['green'], 'SUCCESS'));
     } else {
-      parentLogger.info(colors.red('FAILURE'));
+      parentLogger.info(styleText(['red'], 'FAILURE'));
     }
     parentLogger.info('Result: ' + JSON.stringify({ ...result, info: undefined }, null, 4));
 
@@ -114,7 +111,7 @@ async function _executeTarget(
 
     return result.success ? 0 : 1;
   } catch (err) {
-    parentLogger.info(colors.red('ERROR'));
+    parentLogger.info(styleText(['red'], 'ERROR'));
     parentLogger.info('\nLogs:');
     logs.forEach((l) => parentLogger.next(l));
 
@@ -141,9 +138,9 @@ async function main(args: string[]): Promise<number> {
   const logger = createConsoleLogger(argv['verbose'], process.stdout, process.stderr, {
     info: (s) => s,
     debug: (s) => s,
-    warn: (s) => colors.bold.yellow(s),
-    error: (s) => colors.bold.red(s),
-    fatal: (s) => colors.bold.red(s),
+    warn: (s) => styleText(['yellow', 'bold'], s),
+    error: (s) => styleText(['red', 'bold'], s),
+    fatal: (s) => styleText(['red', 'bold'], s),
   });
 
   // Check the target.

packages/angular_devkit/architect_cli/package.json

@@ -16,7 +16,6 @@
   "dependencies": {
     "@angular-devkit/architect": "workspace:0.0.0-EXPERIMENTAL-PLACEHOLDER",
     "@angular-devkit/core": "workspace:0.0.0-PLACEHOLDER",
-    "ansi-colors": "4.1.3",
     "yargs-parser": "22.0.0"
   }
 }