fix(ng-dev): redact URL credentials from child process logs and errors

ng-dev/utils/child-process.ts

@@ -70,7 +70,7 @@ export abstract class ChildProcess {
   ) {
     return new Promise<void>((resolve, reject) => {
       const commandText = `${command} ${args.join(' ')}`;
-      Log.debug(`Executing command: ${commandText}`);
+      Log.debug(`Executing command: ${sanitize(commandText)}`);
       const childProcess = _spawn(command, args, {...options, stdio: 'inherit'});
       // The `close` event is used because the process is guaranteed to have completed writing to
       // stdout and stderr, using the `exit` event can cause inconsistent information in stdout and
@@ -92,7 +92,7 @@ export abstract class ChildProcess {
     const commandText = `${command} ${args.join(' ')}`;
     const env = getEnvironmentForNonInteractiveCommand(options.env);
 
-    Log.debug(`Executing command: ${commandText}`);
+    Log.debug(`Executing command: ${sanitize(commandText)}`);
 
     const {
       status: exitCode,
@@ -108,7 +108,7 @@ export abstract class ChildProcess {
       return {status, stdout, stderr};
     }
 
-    throw new Error(stderr);
+    throw new Error(sanitize(stderr));
   }
 
   /**
@@ -190,7 +190,11 @@ function processAsyncCmd(
     let stdout = '';
     let stderr = '';
 
-    Log.debug(`Executing command: ${command}`);
+    Log.debug(`Executing command: ${sanitize(command)}`);
+
+    childProcess.on('error', (err) => {
+      reject(err);
+    });
 
     // If provided, write `input` text to the process `stdin`.
     if (options.input !== undefined) {
@@ -210,7 +214,7 @@ function processAsyncCmd(
       // If console output is enabled, print the message directly to the stderr. Note that
       // we intentionally print all output to stderr as stdout should not be polluted.
       if (options.mode === undefined || options.mode === 'enabled') {
-        process.stderr.write(message);
+        process.stderr.write(sanitize(String(message)));
       }
     });
 
@@ -220,7 +224,7 @@ function processAsyncCmd(
       // If console output is enabled, print the message directly to the stderr. Note that
       // we intentionally print all output to stderr as stdout should not be polluted.
       if (options.mode === undefined || options.mode === 'enabled') {
-        process.stderr.write(message);
+        process.stderr.write(sanitize(String(message)));
       }
     });
 
@@ -231,8 +235,8 @@ function processAsyncCmd(
       const exitDescription = exitCode !== null ? `exit code "${exitCode}"` : `signal "${signal}"`;
       const status = statusFromExitCodeAndSignal(exitCode, signal);
       const printFn = status !== 0 && options.mode === 'on-error' ? Log.error : Log.debug;
-      printFn(`Command "${command}" completed with ${exitDescription}.`);
-      printFn(`Process output: \n${logOutput}`);
+      printFn(`Command "${sanitize(command)}" completed with ${exitDescription}.`);
+      printFn(`Process output: \n${sanitize(logOutput)}`);
 
       // On success, resolve the promise. Otherwise reject with the captured stderr
       // and stdout log output if the output mode was set to `silent`.
@@ -244,3 +248,11 @@ function processAsyncCmd(
     });
   });
 }
+
+/** Sanitizes a string by redacting URL credentials. */
+function sanitize(value: string | null | undefined): string {
+  if (!value) {
+    return '';
+  }
+  return value.replace(/(https?:\/\/)([^@:/]*)(:[^@/]+)?@/g, '$1<TOKEN>@');
+}

ng-dev/utils/test/child-process.spec.ts

@@ -0,0 +1,113 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {ChildProcess} from '../child-process.js';
+import {Log} from '../logging.js';
+
+describe('ChildProcess sanitization', () => {
+  let debugSpy: jasmine.Spy;
+  let errorSpy: jasmine.Spy;
+
+  beforeEach(() => {
+    debugSpy = spyOn(Log, 'debug');
+    errorSpy = spyOn(Log, 'error');
+  });
+
+  describe('spawnInteractive', () => {
+    it('should sanitize command logs', async () => {
+      // We don't want to actually run an interactive process if we can avoid it,
+      // but spawnInteractive returns a promise that resolves when the process exits.
+      // We can run a quick command.
+      await ChildProcess.spawnInteractive('node', ['-e', '', 'https://user:password@github.com']);
+      expect(debugSpy).toHaveBeenCalledWith(
+        'Executing command: node -e  https://<TOKEN>@github.com',
+      );
+    });
+  });
+
+  describe('spawnSync', () => {
+    it('should sanitize command logs', () => {
+      ChildProcess.spawnSync('node', ['-e', '', 'https://user:password@github.com']);
+      expect(debugSpy).toHaveBeenCalledWith(
+        'Executing command: node -e  https://<TOKEN>@github.com',
+      );
+    });
+
+    it('should not corrupt scoped package URLs', () => {
+      ChildProcess.spawnSync('node', ['-e', '', 'https://npm.pkg.github.com/@angular/cli']);
+      expect(debugSpy).toHaveBeenCalledWith(
+        'Executing command: node -e  https://npm.pkg.github.com/@angular/cli',
+      );
+    });
+
+    it('should sanitize URLs with empty username', () => {
+      ChildProcess.spawnSync('node', ['-e', '', 'https://:password@github.com']);
+      expect(debugSpy).toHaveBeenCalledWith(
+        'Executing command: node -e  https://<TOKEN>@github.com',
+      );
+    });
+
+    it('should sanitize thrown error message', () => {
+      expect(() => {
+        // Run a node script that writes to stderr and exits with 1
+        ChildProcess.spawnSync('node', [
+          '-e',
+          'console.error("failed: https://token@github.com"); process.exit(1)',
+        ]);
+      }).toThrowError(/failed: https:\/\/<TOKEN>@github.com/);
+    });
+
+    it('should handle spawn failures (e.g. command not found) without throwing TypeError', () => {
+      expect(() => {
+        ChildProcess.spawnSync('non-existent-command-12345', []);
+      }).toThrowError();
+    });
+  });
+
+  describe('spawn', () => {
+    it('should sanitize command logs and output', async () => {
+      await ChildProcess.spawn('node', [
+        '-e',
+        'console.log("output: https://token@github.com"); console.error("error: https://user:pass@github.com")',
+      ]);
+
+      expect(debugSpy).toHaveBeenCalledWith(jasmine.stringContaining('Executing command: node -e'));
+      // Verify command is sanitized in log
+      expect(debugSpy).toHaveBeenCalledWith(
+        jasmine.stringContaining(
+          'node -e console.log("output: https://<TOKEN>@github.com"); console.error("error: https://<TOKEN>@github.com")',
+        ),
+      );
+
+      // Verify output is sanitized in log
+      expect(debugSpy).toHaveBeenCalledWith(jasmine.stringContaining('Process output:'));
+      expect(debugSpy).toHaveBeenCalledWith(
+        jasmine.stringContaining('output: https://<TOKEN>@github.com'),
+      );
+      expect(debugSpy).toHaveBeenCalledWith(
+        jasmine.stringContaining('error: https://<TOKEN>@github.com'),
+      );
+    });
+  });
+
+  describe('exec', () => {
+    it('should sanitize command logs and output', async () => {
+      // For exec, the command is a single string
+      const command = 'node -e "console.log(\'output: https://token@github.com\')"';
+      await ChildProcess.exec(command);
+
+      expect(debugSpy).toHaveBeenCalledWith(
+        'Executing command: node -e "console.log(\'output: https://<TOKEN>@github.com\')"',
+      );
+      expect(debugSpy).toHaveBeenCalledWith(jasmine.stringContaining('Process output:'));
+      expect(debugSpy).toHaveBeenCalledWith(
+        jasmine.stringContaining('output: https://<TOKEN>@github.com'),
+      );
+    });
+  });
+});