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

* feat(schematics): shared workspace readers, and name the source of a rejected dependency

workspace.ts holds what more than one schematic needs to know about a user's
workspace on disk: the lockfile-to-manager table, tolerant JSON readers, the
upward walk that finds the directory owning an install, and
assertSafeDependencyName, moved from deploy/actions.ts unchanged.

Files are parsed with jsonc-parser, as utils.ts already does: Angular
tolerates comments in angular.json, and strict parsing silently dropped a
commented file's cli.packageManager declaration. The upward walk also stops
at bun.lock, bun.lockb and deno.lock, which mark the directory owning an
install even though nothing here can query those managers.

assertSafeDependencyName gains a required source parameter naming where the
value came from. A rejected name is useless to a user who is not told which
file to go and edit, and the deploy spec pins that its error still points at
angular.json.

* feat(schematics): report every installed copy of a package by asking its own package manager

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. The
reports that reach this repo are diagnosed by telling the reporter to run
npm ls firebase. This module runs that question itself.

One file per manager: npm, pnpm, yarn 2+ and yarn 1.x each get their command
and their reader, since the four output formats share nothing. index.ts
identifies the workspace's manager from its own declarations before its
lockfiles, and tells the two yarns apart by the lockfile's own header
('# yarn lockfile v1' against an __metadata: block), probing yarn --version
only when no lockfile is readable: the binary on PATH and the project
disagree in corepack's default state. The query runs through one spawn
wrapper (cross-spawn, argument array, no shell) and reports entries,
distinct versions and problems without rendering any verdict.

Finding nothing is ambiguous, so every path that cannot reach an answer
records a problem: silence downstream has to mean checked and fine.

Parsing specs run against output captured verbatim from real installs of all
four managers. One spec launches npm for real, which is the part captured
output cannot cover and the part that fails first on Windows.

Known limit, deliberate: in a monorepo the question is answered for the whole
workspace while the caller was pointed at one project inside it, so a project
resolving one version can be warned about a sibling's.

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

Fixes #3754. ng add runs the check after the install task and before the
feature prompt, so node_modules is on disk to be asked about and the warning
appears whatever the user selects.

duplicateWarning.ts owns the verdict the reporter refuses to render. It says
one of three things: more than one version found, with each version's
dependency chain named so the user can see what pulled the second copy in; the
check could not be completed, with the reasons; or nothing, which a reader may
take as checked and fine. The whole body is inside one try, because this is a
courtesy and an exception here must not abort ng add.

Verified end to end against the built tarball: a fresh Angular 21 app with a
planted second firebase warns between the install and the prompt, and the same
app without the duplicate prints nothing.

Author: armando-navarro

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.jasmine.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/format.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 };
+};