use standardized uri format

Author: Berlioz

src/deploy/functions/build.spec.ts

@@ -390,7 +390,7 @@ describe("envWithType", () => {
 });
 
 describe("applyEnvSecretOverrides", () => {
-  it("should add the default secret for the keyname to secretEnvironmentVariables if not present", () => {
+  it("throws an error if the secret is in an unknown format", () => {
     const testBuild: build.Build = {
       endpoints: {
         func: {
@@ -405,15 +405,33 @@ describe("applyEnvSecretOverrides", () => {
       params: [],
       requiredAPIs: [],
     };
-    const testSecretRefs: Record<string, string> = { foo: "" };
-    build.applyEnvSecretOverrides(testBuild, testSecretRefs);
-    expect(testBuild.endpoints["func"].secretEnvironmentVariables).to.deep.equal([
-      {
-        key: "foo",
-        secret: "foo",
-        projectId: "test-project",
+    const testSecretRefs: Record<string, string> = {
+      foo: "projects/1234/secrets/api_key/versions/2",
+    };
+    expect(() => build.applyEnvSecretOverrides(testBuild, testSecretRefs)).to.throw(
+      /not a valid Cloud Secret Manager reference/,
+    );
+  });
+
+  it("throws an error if the secret references an invalid Cloud resource id", () => {
+    const testBuild: build.Build = {
+      endpoints: {
+        func: {
+          region: "us-central1",
+          project: "test-project",
+          platform: "gcfv2",
+          runtime: "nodejs18",
+          entryPoint: "func1",
+          httpsTrigger: {},
+        },
       },
-    ]);
+      params: [],
+      requiredAPIs: [],
+    };
+    const testSecretRefs: Record<string, string> = { foo: "csm://NOCAPS/4" };
+    expect(() => build.applyEnvSecretOverrides(testBuild, testSecretRefs)).to.throw(
+      /not a valid Cloud Secret Manager reference/,
+    );
   });
 
   it("should add the referenced secret to secretEnvironmentVariables if not present", () => {
@@ -431,7 +449,7 @@ describe("applyEnvSecretOverrides", () => {
       params: [],
       requiredAPIs: [],
     };
-    const testSecretRefs: Record<string, string> = { foo: "bar" };
+    const testSecretRefs: Record<string, string> = { foo: "csm://bar" };
     build.applyEnvSecretOverrides(testBuild, testSecretRefs);
     expect(testBuild.endpoints["func"].secretEnvironmentVariables).to.deep.equal([
       {
@@ -457,7 +475,7 @@ describe("applyEnvSecretOverrides", () => {
       params: [],
       requiredAPIs: [],
     };
-    const testSecretRefs: Record<string, string> = { foo: "bar@4" };
+    const testSecretRefs: Record<string, string> = { foo: "csm://bar/4" };
     build.applyEnvSecretOverrides(testBuild, testSecretRefs);
     expect(testBuild.endpoints["func"].secretEnvironmentVariables).to.deep.equal([
       {
@@ -484,7 +502,7 @@ describe("applyEnvSecretOverrides", () => {
       params: [],
       requiredAPIs: [],
     };
-    const testSecretRefs: Record<string, string> = { foo: "bar@latest" };
+    const testSecretRefs: Record<string, string> = { foo: "csm://bar/latest" };
     build.applyEnvSecretOverrides(testBuild, testSecretRefs);
     expect(testBuild.endpoints["func"].secretEnvironmentVariables).to.deep.equal([
       {
@@ -518,7 +536,7 @@ describe("applyEnvSecretOverrides", () => {
       params: [],
       requiredAPIs: [],
     };
-    const testSecretRefs: Record<string, string> = { foo: "bar@4" };
+    const testSecretRefs: Record<string, string> = { foo: "csm://bar/4" };
     build.applyEnvSecretOverrides(testBuild, testSecretRefs);
     expect(testBuild.endpoints["func"].secretEnvironmentVariables).to.deep.equal([
       {

src/deploy/functions/build.ts

@@ -9,6 +9,15 @@ import { ExprParseError } from "./cel";
 import { defineSecret } from "firebase-functions/params";
 
 export const REGION_TBD = "REGION_TBD";
+export const SECRET_REF_PREFIX = "FIREBASE_SECRET_REF_";
+// prettier-ignore
+export const SECRET_REF_RE = new RegExp(
+  "^" +                                      // start of string
+  "csm:\/\/" +                               // scheme csm://
+  "([a-z](?:[-a-z0-9]{0,61}[a-z0-9])?)" +    // secret ID is a GCP resource ID
+  "(?:\/([-a-z0-9]+))?" +                    // capture an optional GCP label for version
+  "$"                                        // end of string
+);
 
 /* The union of a customer-controlled deployment and potentially deploy-time defined parameters */
 export interface Build {
@@ -737,15 +746,23 @@ export function applyPrefix(build: Build, prefix: string): void {
 
 /**
  * Applies overrides from the .env file binding Secrets to a different Cloud Secret Manager resource.
- * For example, "&API_KEY=other@2" binds the value of /projects/1234/secrets/other/versions/2 to env var API_KEY and the secret param named API_KEY.
+ * Secrets references are of the form csm://secretName/version, referencing a Secret in the same project as the Endpoint.
+ * /version can be omitted and will cause the secret to resolve to whatever the latest version was at time of deploy.
  *
  * For each binding imported from the .env file,
  * 1) TODO: Check if a conflicting SecretParam with the same name exists. If so, override the param so that the prompting flow will look in the right place when deciding whether or not to create a new Secret.
  * 2) Upsert the binding directly into the Build's SecretEnvVars, which will cause it to be actually available in process.ENV
  */
 export function applyEnvSecretOverrides(build: Build, envSecrets: Record<string, string>) {
   Object.entries(envSecrets).forEach(([key, secretRef]) => {
-    const [resourceId, versionRef] = secretRef.split("@");
+    const match = SECRET_REF_RE.exec(secretRef);
+    if (!match) {
+      throw new FirebaseError(
+        `Secret reference '${secretRef}' is not a valid Cloud Secret Manager reference.`,
+      );
+    }
+    const resourceId = match[1];
+    const versionRef = match[2];
 
     Object.entries(build.endpoints).forEach(([, endpoint]) => {
       let needEnvInsert = true;

src/deploy/functions/prepare.spec.ts

@@ -21,10 +21,10 @@ import { BEFORE_CREATE_EVENT, BEFORE_SIGN_IN_EVENT } from "../../functions/event
 import { latest } from "./runtimes/supported";
 
 describe("partition env helper", () => {
-  it("splits a Record into two based on which keys begin with &", () => {
+  it("splits a Record into two based on which keys begin with FIREBASE_SECRET_REF", () => {
     const input = {
       foo: "bar",
-      "&baz": "quux",
+      FIREBASE_SECRET_REF_baz: "quux",
     } as Record<string, string>;
     const { userEnvs: userEnvs, secretRefs: secretRefs } = prepare.partitionUserEnvs(input);
     expect(userEnvs).to.deep.equal({ foo: "bar" });

src/deploy/functions/prepare.ts

@@ -152,7 +152,7 @@ export async function prepare(
       .filter((e) => e.codebase === codebase || e.codebase === undefined);
     await resolveDefaultRegionsForBuild(wantBuild, backend.of(...relevantEndpoints));
 
-    if (experiments.isEnabled("functionsEnvParams")) {
+    if (experiments.isEnabled("secretEnvParams")) {
       build.applyEnvSecretOverrides(wantBuild, secretRefs);
     }
 
@@ -798,16 +798,18 @@ export async function ensureAllRequiredAPIsEnabled(
 }
 
 /**
- * Divides the environment variables between normal envs and references to a Secret resource
+ * Divides the environment variables between normal envs and references to a Secret resource.
+ * Secret references begin with FIREBASE_SECRET_REF_ and are not provided directly in process.env,
+ * but the Cloud Secret Manager resource they reference is loaded through SecretEnvVars.
  */
 export function partitionUserEnvs(allEnvs: Record<string, string>): {
   userEnvs: Record<string, string>;
   secretRefs: Record<string, string>;
 } {
   const [secretRefs, userEnvs] = Object.entries(allEnvs).reduce(
     (accumulatedSplit, [k, v]) => {
-      if (k.startsWith("&")) {
-        accumulatedSplit[0][k.slice(1)] = v;
+      if (k.startsWith(build.SECRET_REF_PREFIX)) {
+        accumulatedSplit[0][k.replace(new RegExp("^" + build.SECRET_REF_PREFIX), "")] = v;
       } else {
         accumulatedSplit[1][k] = v;
       }

src/experiments.ts

@@ -214,7 +214,7 @@ export const ALL_EXPERIMENTS = experiments({
     default: false,
     public: true,
   },
-  functionsEnvParams: {
+  secretEnvParams: {
     shortDescription:
       "Enable experimental support for storing references to Secrets in the .env for a Functions deploy.",
     default: false,

src/functions/env.spec.ts

@@ -16,11 +16,6 @@ describe("functions/env", () => {
         input: "FOO=foo        ",
         want: { FOO: "foo" },
       },
-      {
-        description: "should parse &key values with trailing spaces",
-        input: "&FOO=foo        ",
-        want: { "&FOO": "foo" },
-      },
       {
         description: "should parse exported values",
         input: "export FOO=foo",
@@ -45,15 +40,6 @@ BAR=bar
 `,
         want: { FOO: "foo1\nfoo2", BAR: "bar" },
       },
-      {
-        description: "should parse multi-line values without getting confused about &",
-        input: `
-&FOO="foo1
-&foo2"
-BAR=bar
-`,
-        want: { "&FOO": "foo1\n&foo2", BAR: "bar" },
-      },
       {
         description: "should parse many double quoted values",
         input: 'FOO="foo"\nBAR="bar"',
@@ -300,6 +286,10 @@ FOO=foo
       expect(() => {
         env.validateKey("EXT_INSTANCE_ID");
       }).to.throw("starts with a reserved prefix");
+
+      it("except when the key is an allowlisted form", () => {
+        env.validateKey("FIREBASE_SECRET_REF_API_KEY");
+      });
     });
   });