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.
armando-navarro · Sep 2, 2026 · Sep 1, 2026 · Sep 1, 2026 · Sep 1, 2026 · Sep 2, 2026
commit 0459c42abfa3423efad3704f9ccf881a2ae8c5f4
diff --git a/src/schematics/deploy/actions.jasmine.ts b/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

diff --git a/src/schematics/deploy/actions.ts b/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;

diff --git a/src/schematics/workspace.ts b/src/schematics/workspace.ts
@@ -0,0 +1,108 @@
+/*
+ * What more than one schematic needs to know about a user's workspace as it exists on disk, plus
+ * the rules for handling what it finds there.
+ *
+ * `common.ts` works against the schematic `Tree`, which is the pending state of a change. The
+ * readers here go to the real filesystem, which is what a schematic needs when it wants to know
+ * what is actually installed rather than what is about to be written.
+ *
+ * `assertSafeDependencyName` reads nothing. It lives here because it guards values taken from
+ * these same files before they reach a process argv, and both callers of it are callers of the
+ * readers above.
+ */
+
+import { existsSync, readFileSync } from 'fs';
+import { dirname, join } from 'path';
+import { SchematicsException } from '@angular-devkit/schematics';
+import { parse as parseJsonWithComments } from 'jsonc-parser';
+
+/**
+ * `yarn` means yarn 2 and later. `yarn-classic` is yarn 1.x, which is still what
+ * `npm i -g yarn` installs and which reports dependencies in an unrelated format.
+ */
+export type PackageManager = 'npm' | 'pnpm' | 'yarn' | 'yarn-classic';
+
+/**
+ * Lockfile names, and the manager each one identifies, in the order they are checked.
+ *
+ * Both yarns write `yarn.lock`, so it maps to yarn 2+ here and whoever needs to tell the two
+ * apart settles that separately.
+ */
+export const lockfiles: [PackageManager, string][] = [
+  ['pnpm', 'pnpm-lock.yaml'],
+  ['yarn', 'yarn.lock'],
+  ['npm', 'package-lock.json'],
+];
+
+/**
+ * Reads and parses a JSON file, returning undefined rather than throwing when it cannot be used.
+ *
+ * A missing, unreadable or malformed file is an ordinary shape for the things this is pointed at
+ * (a workspace `package.json`, an `angular.json`), so callers branch on the result instead of
+ * wrapping every call.
+ */
+export const readJson = (path: string): unknown => {
+  try {
+    /* jsonc, not JSON.parse: Angular tolerates comments in angular.json and this repo already
+     * reads it that way (`utils.ts`). Strict parsing silently dropped a commented file's
+     * `cli.packageManager` declaration. */
+    return parseJsonWithComments(readFileSync(path, 'utf8'));
+  } catch { return undefined; }
+};
+
+/**
+ * Reads a nested string field out of a parsed JSON file, returning '' for any other shape.
+ *
+ * These files are the user's to write, so every level may be missing or hold a type the schema
+ * does not allow, and none of that is worth an exception.
+ */
+export const stringAt = (source: unknown, ...path: string[]): string => {
+  let value: unknown = source;
+  for (const key of path) {
+    if (typeof value !== 'object' || value === null) { return ''; }
+    value = Reflect.get(value, key);
+  }
+  return typeof value === 'string' ? value : '';
+};
+
+/**
+ * How far above the starting directory a monorepo root is looked for. Without a limit the walk
+ * goes all the way to the filesystem root.
+ */
+export const maxWorkspaceWalkDepth = 8;
+
+/**
+ * Finds the directory that owns the install, by walking up from `startDirectory` until a lockfile
+ * or a `packageManager` declaration appears.
+ */
+export const workspaceRootFor = (startDirectory: string): string => {
+  let directory = startDirectory;
+  for (let depth = 0; depth <= maxWorkspaceWalkDepth; depth++) {
+    /* Lockfiles this cannot query still mark the directory that owns the install. Without them
+     * a bun or deno project walks past its own root and the question is answered wherever an
+     * unrelated ancestor left a lockfile. */
+    const ownershipMarkers = [...lockfiles.map(([, lockfile]) => lockfile),
+      'bun.lock', 'bun.lockb', 'deno.lock'];
+    const owns = ownershipMarkers.some(marker => existsSync(join(directory, marker)))
+      || stringAt(readJson(join(directory, 'package.json')), 'packageManager') !== '';
+    if (owns) { return directory; }
+    const parent = dirname(directory);
+    if (parent === directory) { break; }
+    directory = parent;
+  }
+  return startDirectory;
+};
+
+/**
+ * Rejects a dependency name that could be read as a shell command or as a package-manager flag.
+ * @param name the dependency name to check
+ * @param source names where the value came from, so a rejected name tells the user which file
+ *   to go and edit.
+ */
+export const assertSafeDependencyName = (name: string, source: string): string => {
+  if (typeof name !== 'string' || name.length === 0 || name.startsWith('-') ||
+      /[\s;&|$`(){}<>!\\'"]/.test(name)) {
+    throw new SchematicsException(`Invalid dependency name ${JSON.stringify(name)} ${source}.`);
+  }
+  return name;
+};