fix(deploy): prevent command injection from angular.json in ng deploy (#3738)

The SSR deploy builder shelled out with values read straight from
angular.json: the package manager and each server externalDependencies
entry reached `execSync` as a command string, as did the functions output
path. A malicious or cloned workspace could run arbitrary commands the
moment a developer ran `ng deploy`.

Both calls now go through a single runner built on cross-spawn, which
passes arguments as argv entries and launches the Windows .cmd shims that
child_process.execFile cannot, so the deploy keeps working cross-platform
without a shell. The package manager is checked against a supported set
and dependency names are rejected unless they are plain specifiers.

Tests cover the validators and assert the call sites route through the
shell-free runner, so reverting to a shell or dropping a validator fails
the suite.

(cherry picked from commit 0e8ed0c)

Author: herdiyana256

package-lock.json

@@ -87,6 +87,7 @@
     "@angular/cli": "^20.0.0",
     "@angular/compiler-cli": "^20.0.0",
     "@angular/platform-server": "^20.0.0",
+    "@types/cross-spawn": "^6.0.6",
     "@types/fs-extra": "^7.0.0",
     "@types/gzip-size": "^5.1.1",
     "@types/inquirer": "^0.0.44",

package.json

@@ -3,7 +3,7 @@ import { join } from 'path';
 import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect';
 import { JsonObject, logging } from '@angular-devkit/core';
 import { BuildTarget, FSHost, FirebaseDeployConfig, FirebaseTools } from '../interfaces';
-import deploy, { deployToFunction } from './actions'
+import deploy, { assertSafeDependencyName, assertSupportedPackageManager, deployToFunction, findPackageVersion, processHost } from './actions'
 import 'jasmine';
 
 let context: BuilderContext;
@@ -300,3 +300,91 @@ describe('universal deployment', () => {
     expect(spy).not.toHaveBeenCalled();
   });*/
 });
+
+describe('deploy input validation (command-injection hardening)', () => {
+  describe('assertSupportedPackageManager', () => {
+    ['npm', 'yarn', 'pnpm', 'cnpm', 'bun'].forEach((pm) => {
+      it(`allows the supported package manager "${pm}"`, () => {
+        expect(assertSupportedPackageManager(pm)).toBe(pm);
+      });
+    });
+
+    it('rejects a package manager carrying a shell payload', () => {
+      expect(() => assertSupportedPackageManager('npm; touch /tmp/pwned #'))
+        .toThrowError(/Unsupported package manager/);
+    });
+
+    it('rejects an arbitrary executable path', () => {
+      expect(() => assertSupportedPackageManager('/tmp/evil')).toThrowError(/Unsupported package manager/);
+    });
+  });
+
+  describe('assertSafeDependencyName', () => {
+    ['rxjs', '@angular/core', '@angular/*', 'some-pkg', 'a.b_c'].forEach((name) => {
+      it(`allows the valid dependency name "${name}"`, () => {
+        expect(assertSafeDependencyName(name)).toBe(name);
+      });
+    });
+
+    ['evil; touch /tmp/pwned #', 'a b', '$(id)', '`id`', 'a|b', 'a&b', '-rf', '', 'a>b'].forEach((name) => {
+      it(`rejects the unsafe dependency name ${JSON.stringify(name)}`, () => {
+        expect(() => assertSafeDependencyName(name)).toThrowError(/Invalid dependency name/);
+      });
+    });
+  });
+
+  // These guard the fix at its call sites: the validators above are only useful
+  // if the deploy code keeps routing every command through the shell-free runner.
+  // A regression to execSync/`shell: true`, or a dropped validator call, fails here.
+  describe('call sites route through the shell-free runner', () => {
+    beforeEach(() => initMocks());
+
+    it('installs functions dependencies via the runner with an argv array and no shell', async () => {
+      // The install branch only runs when the generated package.json exists.
+      spyOn(fsHost, 'existsSync').and.returnValue(true);
+      const runSpy = spyOn(processHost, 'runPackageBin').and.returnValue(Buffer.from(''));
+
+      await deployToFunction(
+        firebaseMock,
+        context,
+        workspaceRoot,
+        STATIC_BUILD_TARGET,
+        SERVER_BUILD_TARGET,
+        { preview: false },
+        undefined,
+        fsHost
+      );
+
+      expect(runSpy).toHaveBeenCalledTimes(1);
+      const [command, args, options] = runSpy.calls.mostRecent().args;
+      expect(command).toBe('npm');
+      expect(args).toEqual(['--prefix', join(workspaceRoot, 'dist'), 'install']);
+      // No shell: a `shell` option would reopen the injection this PR closes.
+      expect((options as any)?.shell).toBeFalsy();
+    });
+
+    it('runs the package manager through the runner with a validated argv array', () => {
+      const runSpy = spyOn(processHost, 'runPackageBin').and.returnValue(Buffer.from(''));
+
+      findPackageVersion('npm', 'rxjs');
+
+      expect(runSpy).toHaveBeenCalledWith('npm', ['list', 'rxjs']);
+    });
+
+    it('rejects an unsupported package manager before spawning anything', () => {
+      const runSpy = spyOn(processHost, 'runPackageBin');
+
+      expect(() => findPackageVersion('npm; touch /tmp/pwned #', 'rxjs'))
+        .toThrowError(/Unsupported package manager/);
+      expect(runSpy).not.toHaveBeenCalled();
+    });
+
+    it('rejects an unsafe dependency name before spawning anything', () => {
+      const runSpy = spyOn(processHost, 'runPackageBin');
+
+      expect(() => findPackageVersion('npm', 'evil; touch /tmp/pwned #'))
+        .toThrowError(/Invalid dependency name/);
+      expect(runSpy).not.toHaveBeenCalled();
+    });
+  });
+});

src/schematics/deploy/actions.jasmine.ts

@@ -1,8 +1,9 @@
-import { SpawnOptionsWithoutStdio, execSync, spawn } from 'child_process';
+import { SpawnOptionsWithoutStdio, SpawnSyncOptions, spawn } from 'child_process';
 import { existsSync, readFileSync, renameSync, writeFileSync } from 'fs';
 import { dirname, join } from 'path';
 import { BuilderContext, targetFromTargetString } from '@angular-devkit/architect';
 import { SchematicsException } from '@angular-devkit/schematics';
+import crossSpawn from 'cross-spawn';
 import { copySync, removeSync } from 'fs-extra';
 import * as inquirer from 'inquirer';
 import open from 'open';
@@ -115,8 +116,78 @@ const defaultFsHost: FSHost = {
   existsSync,
 };
 
-const findPackageVersion = (packageManager: string, name: string) => {
-  const match = execSync(`${packageManager} list ${name}`).toString().match(`[^|s]${escapeRegExp(name)}[@| ][^s]+(s.+)?$`);
+// Package managers AngularFire is willing to shell out to when resolving
+// dependency versions. Kept in sync with the Angular CLI's own list. The value
+// comes from `cli.packageManager` in angular.json, which this builder reads with
+// a raw `JSON.parse`, i.e. it is NOT run through the Angular CLI's own schema
+// validation, so it must be checked here before it is ever used as an argv0.
+export const SUPPORTED_PACKAGE_MANAGERS = ['npm', 'yarn', 'pnpm', 'cnpm', 'bun'];
+
+export const assertSupportedPackageManager = (packageManager: string): string => {
+  if (!SUPPORTED_PACKAGE_MANAGERS.includes(packageManager)) {
+    throw new SchematicsException(
+      `Unsupported package manager "${packageManager}" in angular.json (cli.packageManager). ` +
+      `Expected one of: ${SUPPORTED_PACKAGE_MANAGERS.join(', ')}.`
+    );
+  }
+  return packageManager;
+};
+
+// A dependency name comes from `architect.<project>.server.options.externalDependencies`
+// in angular.json. Reject anything that is not a plain package specifier so it can
+// neither inject shell metacharacters (defence in depth alongside execFileSync) nor
+// be parsed as a CLI flag by the package manager (argument injection).
+export const assertSafeDependencyName = (name: string): string => {
+  // Valid npm package names / esbuild external globs never contain whitespace or
+  // shell metacharacters, and never start with a dash. Reject anything else so the
+  // value can neither inject a shell command (defence in depth alongside
+  // execFileSync) nor be parsed as a package-manager flag (argument injection).
+  if (typeof name !== 'string' || name.length === 0 || name.startsWith('-') ||
+      /[\s;&|$`(){}<>!\\'"]/.test(name)) {
+    throw new SchematicsException(
+      `Invalid dependency name ${JSON.stringify(name)} in angular.json (server externalDependencies).`
+    );
+  }
+  return name;
+};
+
+// All shelling out from the deploy builder funnels through this single runner.
+// cross-spawn (v7) resolves the platform-appropriate executable and escapes each
+// argument, so a value taken from angular.json is passed as an argv entry and can
+// never be parsed as shell syntax. Unlike child_process.execFile it can launch a
+// Windows `.cmd`/`.bat` shim (npm, yarn, pnpm and cnpm all ship as `.cmd` shims
+// there), so `ng deploy` keeps working cross-platform. We deliberately avoid
+// `shell: true`, whose args-array form Node runtime-deprecates (DEP0190) because
+// it concatenates the arguments without escaping, reopening the injection.
+// It is exported as an object so tests can assert the deploy code shells out only
+// through here and never regresses to execSync or `shell: true`.
+export const processHost = {
+  runPackageBin(
+    command: string,
+    args: string[],
+    options: SpawnSyncOptions = {},
+  ): Buffer {
+    const result = crossSpawn.sync(command, args, options);
+    if (result.error) { throw result.error; }
+    if (result.status !== 0) {
+      throw new SchematicsException(
+        `Command "${command}" exited with ${result.signal ? `signal ${result.signal}` : `code ${result.status}`}.`
+      );
+    }
+    return result.stdout;
+  },
+};
+
+export const findPackageVersion = (packageManager: string, name: string) => {
+  // Run the package manager without a shell (argument array, no `shell` option)
+  // so a dependency name or package-manager value taken from angular.json cannot
+  // be interpreted as a shell command. Both values are validated first, so an
+  // unsupported manager or unsafe name throws before anything is ever spawned.
+  const output = processHost.runPackageBin(assertSupportedPackageManager(packageManager), [
+    'list',
+    assertSafeDependencyName(name),
+  ]).toString();
+  const match = output.match(`[^|s]${escapeRegExp(name)}[@| ][^s]+(s.+)?$`);
   return match ? match[0].split(new RegExp(`${escapeRegExp(name)}[@| ]`))[1].split(/\s/)[0] : null;
 };
 
@@ -238,7 +309,12 @@ export const deployToFunction = async (
   const siteTarget = options.target ?? context.target!.project;
 
   if (fsHost.existsSync(functionsPackageJsonPath)) {
-    execSync(`npm --prefix ${functionsOut} install`);
+    // Pass the output directory as an argv entry rather than interpolating it
+    // into a shell string; `functionsOut` derives from the `outputPath` deploy
+    // option (angular.json) and must not be able to inject shell commands. npm is
+    // a `.cmd` shim on Windows, which child_process.execFile cannot launch, so
+    // this goes through the shell-free cross-spawn runner instead.
+    processHost.runPackageBin('npm', ['--prefix', functionsOut, 'install'], { stdio: 'inherit' });
   } else {
     console.error(`No package.json exists at ${functionsOut}`);
   }