feat(schematics): warn during ng add when firebase is installed at more than one version

src/schematics/deploy/actions.jasmine.ts

@@ -364,15 +364,23 @@ describe('deploy input validation (command-injection hardening)', () => {
   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);
+        expect(assertSafeDependencyName(name, 'in a test')).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/);
+        expect(() => assertSafeDependencyName(name, 'in a test')).toThrowError(/Invalid dependency name/);
       });
     });
+
+    it('names where the value came from, so the user knows what to go and edit', () => {
+      /* The context used to be part of the message unconditionally. Now that it is an argument,
+       * nothing but this asserts that the deploy call site still passes it, and a message reading
+       * only `Invalid dependency name "--registry=..."` says nothing about angular.json. */
+      expect(() => findPackageVersion('npm', '--registry=http://example.test'))
+        .toThrowError(/in angular\.json \(server externalDependencies\)/);
+    });
   });
 
   // These guard the fix at its call sites: the validators above are only useful

src/schematics/deploy/actions.ts

@@ -12,6 +12,7 @@ import { satisfies } from 'semver';
 import tripleBeam from 'triple-beam';
 import * as winston from 'winston';
 import { BuildTarget, CloudRunOptions, DeployBuilderSchema, FSHost, FirebaseTools } from '../interfaces';
+import { assertSafeDependencyName } from '../workspace.js';
 import { DEFAULT_FUNCTION_NAME, defaultFunction, defaultPackage, dockerfile, functionGen2 } from './functions-templates.js';
 
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
@@ -191,23 +192,10 @@ export const assertSupportedPackageManager = (packageManager: string): string =>
   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;
-};
+/* Rejects a dependency name that is not a plain package specifier. Here the names come
+ * from `architect.<project>.server.options.externalDependencies` in angular.json.
+ * Re-exported so this file's existing importers and specs keep working. */
+export { assertSafeDependencyName };
 
 // All shelling out from the deploy builder funnels through this single runner.
 // cross-spawn (v7) resolves the platform-appropriate executable and escapes each
@@ -243,7 +231,7 @@ export const findPackageVersion = (packageManager: string, name: string) => {
   // unsupported manager or unsafe name throws before anything is ever spawned.
   const output = processHost.runPackageBin(assertSupportedPackageManager(packageManager), [
     'list',
-    assertSafeDependencyName(name),
+    assertSafeDependencyName(name, 'in angular.json (server externalDependencies)'),
   ]).toString();
   const match = output.match(`[^|s]${escapeRegExp(name)}[@| ][^s]+(s.+)?$`);
   return match ? match[0].split(new RegExp(`${escapeRegExp(name)}[@| ]`))[1].split(/\s/)[0] : null;

src/schematics/duplicatePackages/format.ts

@@ -0,0 +1,75 @@
+/* Turning a report into text. Nothing here decides whether the result is good or bad. */
+
+import { compareBuild as semverCompareBuild, valid as semverValid } from 'semver';
+import { executableFor } from './queries.js';
+import { maxPrintedEntries } from './types.js';
+import type { InstalledCopyReport, InstalledEntry } from './types.js';
+
+/** Orders versions. Anything semver rejects falls back to a string compare. */
+const compareVersions = (left: string, right: string): number => {
+  const leftValid = semverValid(left) !== null;
+  const rightValid = semverValid(right) !== null;
+  // Semver-comparable versions must be grouped ahead of the rest before either comparison runs.
+  if (leftValid !== rightValid) { return leftValid ? -1 : 1; }
+  if (leftValid) { return semverCompareBuild(left, right); }
+  // Compare by code unit because localeCompare ordering depends on runtime locale and ICU data.
+  return left < right ? -1 : left > right ? 1 : 0;
+};
+
+/** The distinct versions among a set of entries, ordered numerically. */
+export const distinctVersions = (entries: InstalledEntry[]): string[] =>
+  [...new Set(entries.map(entry => entry.version))].sort(compareVersions);
+
+/**
+ * Which entries to print when there are more than `maxPrintedEntries` of them.
+ *
+ * Keep one entry per distinct version first: cutting the list where it happens to end can leave
+ * twenty lines all showing the same version and drop the duplicate the reader came for.
+ */
+const entriesToPrint = (report: InstalledCopyReport): InstalledEntry[] => {
+  if (report.entries.length <= maxPrintedEntries) { return report.entries; }
+  const chosen = new Set<InstalledEntry>();
+  for (const version of report.versions) {
+    if (chosen.size >= maxPrintedEntries) { break; }
+    const representative = report.entries.find(entry => entry.version === version);
+    if (representative) { chosen.add(representative); }
+  }
+  for (const entry of report.entries) {
+    if (chosen.size >= maxPrintedEntries) { break; }
+    chosen.add(entry);
+  }
+  // Printed in the order the manager reported them, not the order they were chosen in.
+  return report.entries.filter(entry => chosen.has(entry));
+};
+
+/**
+ * Turns a report into printable lines, stating what was found and nothing more. No claim that an
+ * install is safe or broken, and no advice: a caller decides what, if anything, to say.
+ */
+export const formatInstalledCopies = (report: InstalledCopyReport): string[] => {
+  const versionCount = report.versions.length;
+  /* Don't add a count line when nothing was found. Also, multiple entries aren't necessarily
+   * multiple installed copies, so don't phrase them as copies. */
+  const lines = report.entries.length === 0 ? [] : [
+    `${report.packageName}: ${versionCount} distinct ${versionCount === 1 ? 'version' : 'versions'}, ` +
+    `reached by ${report.entries.length} dependent ` +
+    `${report.entries.length === 1 ? 'package' : 'packages'}` +
+    (report.packageManager ? ` (according to ${executableFor(report.packageManager)})` : ''),
+  ];
+  const printed = entriesToPrint(report);
+  for (const entry of printed) {
+    const via = entry.dependencyPath.length ? entry.dependencyPath.join(' > ') : 'the workspace root';
+    lines.push(`  ${entry.version} via ${via}`);
+  }
+  if (printed.length < report.entries.length) {
+    // With more distinct versions than lines allowed, some versions get no line of their own.
+    const shown = new Set(printed.map(entry => entry.version));
+    const unnamed = report.versions.filter(version => !shown.has(version)).length;
+    lines.push(
+      `  ... and ${report.entries.length - printed.length} more` +
+      (unnamed ? `, among them ${unnamed} further ${unnamed === 1 ? 'version' : 'versions'} not listed here` : '')
+    );
+  }
+  for (const problem of report.problems) { lines.push(`  problem: ${problem}`); }
+  return lines;
+};

src/schematics/duplicatePackages/index.ts

@@ -0,0 +1,232 @@
+/*
+ * Asks the workspace's own package manager what it installed for a package, and reports every
+ * entry it named plus the distinct versions among them.
+ *
+ * "Entry" rather than "copy", because a manager lists each place the package was reached from, and
+ * two entries at the same version may be one directory on disk or two. A package installed at two
+ * versions is two module instances, and they reject each other's objects at runtime with errors
+ * naming the caller's code rather than the duplication.
+ *
+ * This file owns identifying which manager the workspace uses and running it. Reading its output
+ * lives in one file per manager beside this one; turning a result into text lives in `format.ts`.
+ *
+ * Nothing here renders a verdict. It reports what each manager said. A caller that wants to warn
+ * writes that rule itself, where it can be read.
+ *
+ * Known limit: in a monorepo the question is answered for the whole workspace while `ng add` was
+ * pointed at one project inside it, so a project resolving exactly one version can be warned about
+ * a sibling's. Scoping it is per-manager work and neither yarn scopes by directory at all.
+ */
+
+import { existsSync, readFileSync } from 'fs';
+import { join } from 'path';
+import crossSpawn from 'cross-spawn';
+import { assertSafeDependencyName, lockfiles, readJson, stringAt, workspaceRootFor } from '../workspace.js';
+import { distinctVersions } from './format.js';
+import { executableFor, queries } from './queries.js';
+import { defaultTimeoutMs, yarnProbeTimeoutMs } from './types.js';
+import type { DeclaredManager, InstalledCopyReport, InstalledEntry, PackageManager, QueryOptions, SpawnOutcome } from './types.js';
+
+export type { InstalledCopyReport, InstalledEntry, PackageManager, QueryOptions, SpawnOutcome } from './types.js';
+export { distinctVersions, formatInstalledCopies } from './format.js';
+export { investigationCommandFor, parseInstalledEntries, queryArgsFor } from './queries.js';
+
+/**
+ * Decides from `yarn --version` report whether this is yarn 1.x or yarn 2+. Anything unrecognized
+ * falls back to yarn 2+, whose parser rejects unfamiliar input rather than mis-reading it.
+ */
+export const yarnFromVersion = (reportedVersion: string): PackageManager => {
+  /* Handles both `yarn --version` output (always a full version) and a declared specifier, which
+   * may be `1`, `^1.22.22` or `1.x`. Only the leading major matters, and "10" must not match. */
+  const major = /^[^\d]*(\d+)/.exec(reportedVersion.trim())?.[1];
+  return major === '1' ? 'yarn-classic' : 'yarn';
+};
+
+/**
+ * Reads what a project declares about its package manager. The caller only falls back to looking
+ * for lockfiles when this finds nothing. A declaration is more certain than looking for lockfiles.
+ *
+ * Directories must be passed nearest first. An Angular workspace nested inside a monorepo often
+ * states its manager in its own `angular.json` while the monorepo root never mentions one, so
+ * reading only the monorepo root would throw away the more specific statement.
+ */
+const declaredManager = (directories: string[]): DeclaredManager => {
+  const declarations: string[] = [];
+  for (const directory of directories) {
+    declarations.push(stringAt(readJson(join(directory, 'package.json')), 'packageManager'));
+    declarations.push(stringAt(readJson(join(directory, 'angular.json')), 'cli', 'packageManager'));
+  }
+  for (const declaration of declarations) {
+    // corepack spells it `name@version`, angular.json spells it `name`.
+    const name = declaration.split('@')[0];
+    if (!name) { continue; }
+    if (name === 'npm' || name === 'pnpm') { return { manager: name }; }
+    if (name === 'yarn') {
+      const version = declaration.includes('@') ? declaration.split('@')[1] : '';
+      return version ? { manager: yarnFromVersion(version) } : { yarnOfUnknownVersion: true };
+    }
+    // `cli.packageManager` also accepts bun and cnpm, which this module has no query for.
+    return { unqueryable: name };
+  }
+  return {};
+};
+
+/** Every spawn this module makes funnels through this one runner. Exported as object for specs. */
+export const commandHost = {
+  /** Runs a command without a shell and returns its output, whether or not it exited cleanly. */
+  run(command: string, args: string[], cwd: string, timeoutMs: number): SpawnOutcome {
+    const result = crossSpawn.sync(command, args, {
+      cwd,
+      encoding: 'utf8',
+      // Raised to at least 1: Node reads `timeout: 0` as no timeout at all.
+      timeout: Math.max(1, timeoutMs),
+      // The default 1 MiB truncates a large monorepo's listing into an ENOBUFS failure.
+      maxBuffer: 64 * 1024 * 1024,
+      // This runs unattended inside `ng add`, where a console window would be a surprise.
+      windowsHide: true,
+    });
+    return {
+      stdout: result.stdout ?? '',
+      status: result.status,
+      failure: result.error?.message,
+    };
+  },
+};
+
+/** Checks the workspace's declaration first, then its lockfile, and for yarn a version probe. */
+export const detectPackageManager = (
+  workspaceRoot: string,
+  problems: string[] = [],
+): PackageManager | undefined =>
+  detectFrom(workspaceRoot, workspaceRootFor(workspaceRoot), problems);
+
+/** Detection for a caller that already found the workspace root. */
+const detectFrom = (
+  startDirectory: string,
+  root: string,
+  problems: string[],
+): PackageManager | undefined => {
+  /* Declarations are read from the caller's own directory as well as `root`, because a nested
+   * Angular workspace can name a manager its monorepo root does not. The query itself always
+   * runs in `root`, where the lockfile and node_modules live. */
+  const declared = declaredManager(root === startDirectory ? [root] : [startDirectory, root]);
+  if (declared.manager) { return declared.manager; }
+  if (declared.unqueryable) {
+    problems.push(
+      `the workspace declares ${declared.unqueryable}, and this check was not designed to ` +
+      `handle ${declared.unqueryable}`
+    );
+    return undefined;
+  }
+  // A declaration of plain `yarn` names the manager already, leaving only its version to find.
+  const detected = declared.yarnOfUnknownVersion
+    ? 'yarn'
+    : lockfiles.find(([, lockfile]) => existsSync(join(root, lockfile)))?.[0];
+  if (detected !== 'yarn') { return detected; }
+
+  /* The lockfile names its own generation: yarn 1 writes "# yarn lockfile v1" in its header,
+   * yarn 2+ writes an "__metadata:" block. Reading it beats probing the yarn on PATH, which can
+   * be a different generation than the one that wrote this project. */
+  try {
+    const lockfileHead = readFileSync(join(root, 'yarn.lock'), 'utf8').slice(0, 500);
+    if (lockfileHead.includes('yarn lockfile v1')) { return 'yarn-classic'; }
+    if (lockfileHead.includes('__metadata:')) { return 'yarn'; }
+  } catch { /* No readable lockfile, e.g. a bare `yarn` declaration before install: probe. */ }
+
+  const assumedYarn2Problem =
+    'so yarn 2+ was assumed. If this project uses yarn 1.x, nothing would have been found';
+  const probe = commandHost.run('yarn', ['--version'], root, yarnProbeTimeoutMs);
+  const reported = probe.stdout.trim();
+  // Every yarn version contains a digit. Anything else is unreadable however the process exited.
+  if (!/\d/.test(reported)) {
+    const cause = probe.failure ?? (probe.status === 0 ? 'it printed no version' : `exit status ${probe.status}`);
+    problems.push(`could not determine the yarn version (${cause}), ${assumedYarn2Problem}`);
+    return 'yarn';
+  }
+  const whichYarn = yarnFromVersion(reported);
+  if (probe.failure || probe.status !== 0) {
+    // Name the yarn actually chosen. A blanket "yarn 2+ was assumed" could contradict it.
+    problems.push(
+      `yarn printed version ${reported} but exited with ` +
+      `${probe.failure ?? `status ${probe.status}`}, so ${whichYarn} was used`
+    );
+  }
+  return whichYarn;
+};
+
+/**
+ * Asks the workspace's package manager where `packageName` is installed and at which versions.
+ * @param packageName the package to ask about, for example `'firebase'` or `'rxfire'`
+ * @param workspaceRoot the directory holding the lockfile and `node_modules`
+ */
+export const findInstalledCopies = (
+  packageName: string,
+  workspaceRoot: string,
+  options: QueryOptions = {},
+): InstalledCopyReport => {
+  const problems: string[] = [];
+  const timeoutMs = options.timeoutMs ?? defaultTimeoutMs;
+  assertSafeDependencyName(packageName, 'as the package to report installed copies of');
+
+  const root = workspaceRootFor(workspaceRoot);
+  /* Both directories: a nested Angular workspace can name a package manager its monorepo root
+   * does not. `root` is passed in because it has already been found. */
+  const packageManager = options.packageManager
+    ?? detectFrom(workspaceRoot, root, problems);
+  if (!packageManager) {
+    if (problems.length === 0) {
+      problems.push('no package manager could be identified, so nothing was queried');
+    }
+    return { packageName, packageManager: undefined, entries: [], versions: [], problems };
+  }
+
+  // run the package manager command to check for duplicate installs
+  const query = queries[packageManager];
+  const outcome = commandHost.run(
+    executableFor(packageManager), query.args(packageName), root, timeoutMs);
+
+  if (outcome.failure) {
+    // A kill on timeout arrives here as an error, not a status.
+    const timedOut = outcome.failure.includes('ETIMEDOUT');
+    problems.push(timedOut
+      ? `${executableFor(packageManager)} took longer than the ${timeoutMs / 1000} second timeout`
+      : `could not run ${executableFor(packageManager)} (${outcome.failure})`);
+    return { packageName, packageManager, entries: [], versions: [], problems };
+  }
+
+  // The name the user can type. `yarn-classic` is this module's word, not a program.
+  const executable = executableFor(packageManager);
+  let entries: InstalledEntry[];
+  try {
+    // Parse even on a non-zero exit: these commands report unrelated problems and still answer.
+    entries = query.parse(outcome.stdout, packageName, problems);
+  } catch (error) {
+    const reason = error instanceof Error ? error.message : String(error);
+    problems.push(`${executable} output could not be parsed (${reason})`);
+    if (outcome.status !== 0) { problems.push(`${executable} exited with status ${outcome.status}`); }
+    return { packageName, packageManager, entries: [], versions: [], problems };
+  }
+
+  // Finding nothing is not the same as there being nothing, and silence would imply the latter.
+  if (entries.length === 0) {
+    /* The exit status is only reported when it corroborates an empty answer. These commands exit
+     * non-zero for reasons unrelated to the question, such as an unmet peer elsewhere, and a
+     * complete answer next to "exited with status 1" reads as a failed check. */
+    if (outcome.status !== 0) {
+      problems.push(`${executable} exited with status ${outcome.status}`);
+    }
+    if (outcome.stdout.trim().length > 0) {
+      problems.push(
+        `${packageName} was not mentioned in ${executable}'s output, so either it is not ` +
+        'installed or the output is in a shape this cannot read');
+    } else if (outcome.status !== 0) {
+      problems.push(
+        `${executable} exited without printing anything, so nothing could be read about ` +
+        `${packageName}`);
+    }
+    /* Exit 0 with nothing printed is yarn 2+'s well-formed answer for "nothing depends on this
+     * package". No copies means no duplicate, so nothing is recorded and the caller stays silent. */
+  }
+
+  return { packageName, packageManager, entries, versions: distinctVersions(entries), problems };
+};