fix(@schematics/angular): escape module specifier in insertImport

[herdiyana256]

`insertImport` (`packages/schematics/angular/utility/ast-utils.ts`) builds an `import ... from ''` statement by interpolating `fileName` directly between single quotes, with no escaping. A `fileName` containing a single quote breaks out of the string literal, so a caller that passes untrusted data as the module specifier ends up inserting attacker-controlled source into the target file instead of a plain import path.

This is reachable through `addRootProvider`'s `external(symbolName, moduleName)` (`utility/standalone/code_block.ts`), which calls `insertImport` with `moduleName` as `fileName` — any `ng add` schematic that calls `external()` with a module name sourced from project config rather than a hardcoded string inherits this. Confirmed with the real, unmodified `insertImport` against a `fileName` of ``x'; console.log('INJECTED'); const _y = 'y``: the generated file contained the `console.log(...)` call as live top-level code rather than an unusual import path, and running the generated file executed it.

`fileName` is now escaped for backslashes and single quotes before being interpolated, matching the existing single-quote output format so the common case (a real module specifier, which never contains a quote or backslash) is byte-for-byte unchanged — verified against the existing `insertImport` describe block in `ast-utils_spec.ts` by hand, all of which assert exact single-quoted output.

Note on verification: this repo's test suite runs via Bazel (`bazel test //packages/...`), which wasn't feasible to run end-to-end in the environment this fix was developed in. The escaping logic was verified standalone against the existing test cases' expected output strings and against the malicious-input case described above; happy to have CI confirm on this PR.

[gemini-code-assist]

Code Review

This pull request updates the insertImport utility in ast-utils.ts to escape backslashes and single quotes in the import file name. The reviewer suggested extending this escaping logic to handle newline and carriage return characters to prevent syntax errors, and recommended adding unit tests to verify the escaping behavior.

[herdiyana256]

Both addressed in 02bec13:

• Now also escaping \n/\r in the module specifier, in addition to backslashes and single quotes.
• Added two test cases to ast-utils_spec.ts (insertImport describe block): one for quote/backslash escaping (matches the suggested case exactly), one for newline/carriage-return escaping.

[alan-agius4]

Thanks for this change.

However, because this is a private utility, external attackers cannot control the input. Furthermore, if an attacker already has the ability to execute a schematic via the command line, the underlying system is already compromised, indicating much broader and more critical security vulnerabilities.

[herdiyana256]

Fair points, but I don't think either holds for this specific path.

It's not a private utility. It's exported in @schematics/angular/package.json:

"./utility": "./utility/index.js",
"./utility/*": "./utility/*.js",

So insertImport is importable as @schematics/angular/utility/ast-utils, and addRootProvider / addRootImport are documented (@example in rules.ts) for third-party schematics to consume. AngularFire is one that does:

// @angular/fire - src/schematics/setup/index.ts
import { addRootProvider } from '@schematics/angular/utility';

// src/schematics/utils.ts
external("connectorConfig", config.package)

config.package there isn't a hardcoded literal. It comes from the project's own connector.yaml (generate.javascriptSdk.package, read in parseDataConnectConfig), with no trusted schema in front of it. That's the input a caller ends up passing as fileName.

And it doesn't need CLI access. The tainted value is a project data file, not the command. Someone runs ng add @angular/fire on a repo they cloned, a Data Connect schema pulled from the console, or a teammate's branch, which is a normal thing to do. A connector.yaml in that repo then decides what lands in app.config.ts. If reading project files during ng add counted as "already compromised", no schematic would ever need to validate its inputs.

The x'; ...; const _y = 'y specifier makes insertImport emit a Change whose toAdd breaks out of the from "..." literal into top-level code that then gets committed and runs at build. The file already uses JSON.stringify for other interpolated values, so encoding this one is consistent. It's also a one-line change that leaves the normal case byte-for-byte identical.

If you'd rather the check sit in the callers, I can move it there instead, but the shared utility is the place that removes the footgun for every consumer.