From c9bb241df540bd6cef1f11530d7714e9d0278572 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Tue, 8 Sep 2026 16:27:03 -0700 Subject: [PATCH 1/3] prompt correctly for namespaced prefixes --- src/deploy/functions/build.spec.ts | 11 ++- src/deploy/functions/build.ts | 36 +++++--- src/deploy/functions/params.spec.ts | 123 +++++++++++++++++++++++----- src/deploy/functions/params.ts | 27 ++++-- src/functions/kits/install.spec.ts | 32 ++++---- src/functions/kits/install.ts | 16 ++-- src/functions/secrets.ts | 2 +- 7 files changed, 181 insertions(+), 66 deletions(-) diff --git a/src/deploy/functions/build.spec.ts b/src/deploy/functions/build.spec.ts index 59392210200..f67a3d7c172 100644 --- a/src/deploy/functions/build.spec.ts +++ b/src/deploy/functions/build.spec.ts @@ -728,8 +728,12 @@ describe("applyPrefix", () => { expect(Object.keys(testBuild.endpoints).sort()).to.deep.equal(["func1", "func2"]); }); - it("should prefix secret names in secretEnvironmentVariables", () => { + it("should prefix secret names in secretEnvironmentVariables and update param references to that secret", () => { const testBuild: build.Build = { + params: [ + { type: "secret", name: "API_KEY" }, + { type: "secret", name: "DB_PASSWORD" }, + ], endpoints: { func1: { region: "us-central1", @@ -755,7 +759,6 @@ describe("applyPrefix", () => { ], }, }, - params: [], requiredAPIs: [], }; @@ -772,6 +775,10 @@ describe("applyPrefix", () => { expect(testBuild.endpoints["staging-func2"].secretEnvironmentVariables).to.deep.equal([ { key: "SERVICE_TOKEN", secret: "staging-service-secret", projectId: "test-project" }, ]); + expect(testBuild.params).to.deep.equal([ + { type: "secret", name: "API_KEY", resourceId: "STAGING_API_KEY" }, + { type: "secret", name: "DB_PASSWORD", resourceId: "STAGING_DB_PASSWORD" }, + ]); }); it("throws if combined function id exceeds 63 characters", () => { diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index 87bdcf70046..48053721481 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -8,6 +8,7 @@ import { FirebaseConfig } from "./args"; import { Runtime } from "./runtimes/supported"; import { ExprParseError } from "./cel"; import { defineSecret } from "firebase-functions/params"; +import { toUpperSnakeCase } from "../../functions/secrets"; export const REGION_TBD = "REGION_TBD"; export const SECRET_REF_PREFIX = "FIREBASE_SECRET_REF_"; @@ -354,15 +355,15 @@ export async function resolveBackend(opts: ResolveBackendOpts): Promise<{ envs: Record; secretRefs: Record; }> { - const { paramValues: paramValues, secretRefs: secretRefs } = await params.resolveParams( - opts.build.params, - opts.firebaseConfig, - envWithTypes(opts.build.params, opts.userEnvs), - opts.codebase, - opts.nonInteractive, - opts.force, - opts.isEmulator, - ); + const { paramValues: paramValues, secretRefs: secretRefs } = await params.resolveParams({ + params: opts.build.params, + firebaseConfig: opts.firebaseConfig, + userEnvs: envWithTypes(opts.build.params, opts.userEnvs), + codebase: opts.codebase, + nonInteractive: opts.nonInteractive, + force: opts.force, + isEmulator: opts.isEmulator, + }); return { backend: toBackend(opts.build, paramValues), envs: paramValues, secretRefs: secretRefs }; } @@ -740,6 +741,14 @@ function discoverTrigger(endpoint: Endpoint, region: string, r: Resolver): backe * Prefixes all endpoint IDs and secret names in a build with a given prefix. * This ensures that functions and their associated secrets from different codebases * remain isolated and don't conflict when deployed to the same project. + * + * Secret params in a build are rewritten to point to resource names respecting + * the same prefixing, so that the interactive secret creation flow still works + * and the non-interactive message prints the correct secret to create. + * + * When deploying a function which already has secret bindings in its .env files, + * applyEnvSecretBindings will run after this and overwrite both the updated + * secret params and SecretEnvVars to reflect deployed reality. */ export function applyPrefix(build: Build, prefix: string): void { if (!prefix) { @@ -773,6 +782,13 @@ export function applyPrefix(build: Build, prefix: string): void { } build.endpoints = newEndpoints; + for (const param of build.params) { + if (param.type !== "secret") { + continue; + } + param.resourceId = toUpperSnakeCase(`${prefix}-${param.resourceId || param.name}`); + } + if (build.lifecycleHooks) { for (const hook of Object.values(build.lifecycleHooks)) { if ("task" in hook) { @@ -806,7 +822,7 @@ export interface ParsedSecretRef { * /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. + * 1) 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 applyEnvSecretBindings( diff --git a/src/deploy/functions/params.spec.ts b/src/deploy/functions/params.spec.ts index 66b0ab53bcc..08f408d8585 100644 --- a/src/deploy/functions/params.spec.ts +++ b/src/deploy/functions/params.spec.ts @@ -100,7 +100,14 @@ describe("resolveParams", () => { const paramsToResolve: params.Param[] = []; const userEnv: Record = {}; expect( - (await params.resolveParams(paramsToResolve, fakeConfig, userEnv, "default")).paramValues, + ( + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: userEnv, + codebase: "default", + }) + ).paramValues, ).to.deep.equal(expectedInternalParams); }); @@ -121,7 +128,14 @@ describe("resolveParams", () => { baz: new params.ParamValue("true", false, { string: false, number: false, boolean: true }), }; expect( - (await params.resolveParams(paramsToResolve, fakeConfig, userEnv, "default")).paramValues, + ( + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: userEnv, + codebase: "default", + }) + ).paramValues, ).to.deep.equal( Object.assign( { @@ -148,7 +162,14 @@ describe("resolveParams", () => { }), }; expect( - (await params.resolveParams(paramsToResolve, fakeConfig, userEnv, "default")).paramValues, + ( + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: userEnv, + codebase: "default", + }) + ).paramValues, ).to.deep.equal({ DATABASE_URL: new params.ParamValue(fakeConfig.databaseURL, true, { string: true, @@ -178,12 +199,12 @@ describe("resolveParams", () => { const userEnv: Record = {}; expect( ( - await params.resolveParams( - paramsToResolve, - { locationId: "", projectId: "foo", storageBucket: "", databaseURL: "" }, - userEnv, - "default", - ) + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: { locationId: "", projectId: "foo", storageBucket: "", databaseURL: "" }, + userEnvs: userEnv, + codebase: "default", + }) ).paramValues, ).to.deep.equal({ GCLOUD_PROJECT: expectedInternalParams.GCLOUD_PROJECT, @@ -202,7 +223,14 @@ describe("resolveParams", () => { ]; input.resolves("bar"); expect( - (await params.resolveParams(paramsToResolve, fakeConfig, {}, "default")).paramValues, + ( + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + }) + ).paramValues, ).to.deep.equal( Object.assign( { @@ -229,7 +257,12 @@ describe("resolveParams", () => { }, ]; input.resolves("baz"); - await params.resolveParams(paramsToResolve, fakeConfig, {}, "default"); + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + }); expect(input.getCall(1).args[0].default).to.eq("baz"); }); @@ -249,7 +282,12 @@ describe("resolveParams", () => { }, ]; input.resolves("baz"); - await params.resolveParams(paramsToResolve, fakeConfig, {}, "default"); + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + }); expect(input.getCall(1).args[0].default).to.eq("baz/quox"); }); @@ -275,7 +313,12 @@ describe("resolveParams", () => { }, ]; input.resolves("baz"); - await params.resolveParams(paramsToResolve, fakeConfig, {}, "default"); + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + }); expect(input.getCall(0).args[0].default).to.eq("https://foo.firebaseio.com/quox"); expect(input.getCall(1).args[0].default).to.eq("projectID: foo"); expect(input.getCall(2).args[0].default).to.eq( @@ -293,8 +336,14 @@ describe("resolveParams", () => { }, ]; input.resolves(""); - await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, "default")).to.eventually.be - .rejected; + await expect( + params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + }), + ).to.eventually.be.rejected; }); it("errors when the default is a CEL expression that resolves to the wrong type", async () => { @@ -313,8 +362,14 @@ describe("resolveParams", () => { }, ]; input.resolves("22"); - await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, "default")).to.eventually.be - .rejected; + await expect( + params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + }), + ).to.eventually.be.rejected; }); it("does not throw in non-interactive mode if secret exists in cloud", async () => { @@ -324,8 +379,15 @@ describe("resolveParams", () => { secretVersion: { versionId: "1", state: "ENABLED", secret: {} as any }, }); - await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, "default", true)).to.be - .fulfilled; + await expect( + params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + nonInteractive: true, + }), + ).to.be.fulfilled; getSecretMetadataStub.restore(); }); @@ -337,7 +399,13 @@ describe("resolveParams", () => { }); await expect( - params.resolveParams(paramsToResolve, fakeConfig, {}, "default", true), + params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + nonInteractive: true, + }), ).to.be.rejectedWith(FirebaseError, /In non-interactive mode but have no value for the secret/); getSecretMetadataStub.restore(); @@ -347,7 +415,13 @@ describe("resolveParams", () => { const paramsToResolve: params.Param[] = [{ name: "MY_SECRET", type: "secret" }]; const getSecretMetadataSpy = sinon.spy(secretManager, "getSecretMetadata"); - await params.resolveParams(paramsToResolve, fakeConfig, {}, "default", false, false, true); + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "default", + isEmulator: true, + }); expect(getSecretMetadataSpy.called).to.be.false; getSecretMetadataSpy.restore(); @@ -362,7 +436,12 @@ describe("resolveParams", () => { }, ]; input.resolves("bar"); - await params.resolveParams(paramsToResolve, fakeConfig, {}, "my-codebase"); + await params.resolveParams({ + params: paramsToResolve, + firebaseConfig: fakeConfig, + userEnvs: {}, + codebase: "my-codebase", + }); expect( loggerInfoStub.calledWith(sinon.match(/Prompting for parameters for codebase.*my-codebase/)), ).to.be.true; diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 82f4384b128..00c74f23b5e 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -383,6 +383,16 @@ function canSatisfyParam(param: Param, value: RawParamValue): boolean { assertExhaustive(param); } +interface ResolveParamOpts { + params: Param[]; + firebaseConfig: FirebaseConfig; + userEnvs: Record; + codebase: string; + nonInteractive?: boolean; + force?: boolean; + isEmulator?: boolean; +} + /** * A param defined by the SDK may resolve to: * - a reference to a secret in Cloud Secret Manager, which we validate the existence of and prompt for if missing @@ -395,14 +405,17 @@ function canSatisfyParam(param: Param, value: RawParamValue): boolean { * - after prompting, the resolved value of the param is written to the most specific .env file available */ export async function resolveParams( - params: Param[], - firebaseConfig: FirebaseConfig, - userEnvs: Record, - codebase: string, - nonInteractive?: boolean, - force?: boolean, - isEmulator = false, + opts: ResolveParamOpts, ): Promise<{ paramValues: Record; secretRefs: Record }> { + const { + params, + firebaseConfig, + userEnvs, + codebase, + nonInteractive = false, + force = false, + isEmulator = false, + } = opts; const paramValues: Record = populateDefaultParams(firebaseConfig); const secretRefs: Record = {}; diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 250afefd8b5..0a072bb8458 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -2228,14 +2228,14 @@ describe("functions/kits/install", () => { projectAlias: "staging", }); - expect(resolveParamsStub).to.have.been.calledWith( - paramList, - { projectId: "my-project" }, - sinon.match.object, - "inst", - false, - false, - ); + expect(resolveParamsStub).to.have.been.calledWith({ + params: paramList, + firebaseConfig: { projectId: "my-project" }, + userEnvs: sinon.match.object, + codebase: "inst", + nonInteractive: false, + force: false, + }); expect(writeResolvedParamsStub).to.have.been.calledWith( resolvedParamValues, @@ -2290,14 +2290,14 @@ describe("functions/kits/install", () => { params: paramList, }); - expect(resolveParamsStub).to.have.been.calledWith( - paramList, - { projectId: "my-project" }, - sinon.match.object, - "inst", - undefined, - undefined, - ); + expect(resolveParamsStub).to.have.been.calledWith({ + params: paramList, + firebaseConfig: { projectId: "my-project" }, + userEnvs: sinon.match.object, + codebase: "inst", + nonInteractive: undefined, + force: undefined, + }); }); it("should propagate errors thrown by resolveParams", async () => { diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index eee0b37aa82..be6e936698c 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -1053,14 +1053,14 @@ export async function promptAndWriteKitParams( } const typedUserEnvs = build.envWithTypes(options.params, userEnvs); - const { paramValues: resolvedEnvs, secretRefs: resolvedSecretRefs } = await params.resolveParams( - options.params, - firebaseConfig, - typedUserEnvs, - options.instanceId, - options.nonInteractive, - options.force, - ); + const { paramValues: resolvedEnvs, secretRefs: resolvedSecretRefs } = await params.resolveParams({ + params: options.params, + firebaseConfig: firebaseConfig, + userEnvs: typedUserEnvs, + codebase: options.instanceId, + nonInteractive: options.nonInteractive, + force: options.force, + }); functionsEnv.writeResolvedParams(resolvedEnvs, userEnvs, userEnvOpt); if (experiments.isEnabled("secretEnvParams")) { diff --git a/src/functions/secrets.ts b/src/functions/secrets.ts index aa95be09a5c..1f776e0f527 100644 --- a/src/functions/secrets.ts +++ b/src/functions/secrets.ts @@ -52,7 +52,7 @@ type ProjectInfo = { projectNumber: string; }; -function toUpperSnakeCase(key: string): string { +export function toUpperSnakeCase(key: string): string { return key .replace(/[.-]/g, "_") .replace(/([a-z])([A-Z])/g, "$1_$2") From 64be00bad2e249a434bc33d3306258978c5bcb59 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Tue, 8 Sep 2026 16:40:08 -0700 Subject: [PATCH 2/3] bugfix --- src/deploy/functions/build.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index 48053721481..04ca486796a 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -776,7 +776,7 @@ export function applyPrefix(build: Build, prefix: string): void { if (endpoint.secretEnvironmentVariables) { endpoint.secretEnvironmentVariables = endpoint.secretEnvironmentVariables.map((secret) => ({ ...secret, - secret: `${prefix}-${secret.secret}`, + secret: toUpperSnakeCase(`${prefix}-${secret.secret}`), })); } } From 38c85c11b7abde5f0c223bea967a10d99c0106fb Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 9 Sep 2026 16:13:04 -0700 Subject: [PATCH 3/3] just rip out secret prefixing --- src/deploy/functions/build.spec.ts | 53 ------------------------------ src/deploy/functions/build.ts | 29 ++-------------- src/functions/secrets.ts | 2 +- 3 files changed, 4 insertions(+), 80 deletions(-) diff --git a/src/deploy/functions/build.spec.ts b/src/deploy/functions/build.spec.ts index f67a3d7c172..e362b3e4ee7 100644 --- a/src/deploy/functions/build.spec.ts +++ b/src/deploy/functions/build.spec.ts @@ -728,59 +728,6 @@ describe("applyPrefix", () => { expect(Object.keys(testBuild.endpoints).sort()).to.deep.equal(["func1", "func2"]); }); - it("should prefix secret names in secretEnvironmentVariables and update param references to that secret", () => { - const testBuild: build.Build = { - params: [ - { type: "secret", name: "API_KEY" }, - { type: "secret", name: "DB_PASSWORD" }, - ], - endpoints: { - func1: { - region: "us-central1", - project: "test-project", - platform: "gcfv2", - runtime: "nodejs18", - entryPoint: "func1", - httpsTrigger: {}, - secretEnvironmentVariables: [ - { key: "API_KEY", secret: "api-secret", projectId: "test-project" }, - { key: "DB_PASSWORD", secret: "db-secret", projectId: "test-project" }, - ], - }, - func2: { - region: "us-west1", - project: "test-project", - platform: "gcfv1", - runtime: "nodejs16", - entryPoint: "func2", - httpsTrigger: {}, - secretEnvironmentVariables: [ - { key: "SERVICE_TOKEN", secret: "service-secret", projectId: "test-project" }, - ], - }, - }, - requiredAPIs: [], - }; - - build.applyPrefix(testBuild, "staging"); - - expect(Object.keys(testBuild.endpoints).sort()).to.deep.equal([ - "staging-func1", - "staging-func2", - ]); - expect(testBuild.endpoints["staging-func1"].secretEnvironmentVariables).to.deep.equal([ - { key: "API_KEY", secret: "staging-api-secret", projectId: "test-project" }, - { key: "DB_PASSWORD", secret: "staging-db-secret", projectId: "test-project" }, - ]); - expect(testBuild.endpoints["staging-func2"].secretEnvironmentVariables).to.deep.equal([ - { key: "SERVICE_TOKEN", secret: "staging-service-secret", projectId: "test-project" }, - ]); - expect(testBuild.params).to.deep.equal([ - { type: "secret", name: "API_KEY", resourceId: "STAGING_API_KEY" }, - { type: "secret", name: "DB_PASSWORD", resourceId: "STAGING_DB_PASSWORD" }, - ]); - }); - it("throws if combined function id exceeds 63 characters", () => { const longId = "a".repeat(34); // with 30-char prefix + dash = 65 total const testBuild: build.Build = build.of({ diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index 04ca486796a..498d485fc37 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -8,7 +8,6 @@ import { FirebaseConfig } from "./args"; import { Runtime } from "./runtimes/supported"; import { ExprParseError } from "./cel"; import { defineSecret } from "firebase-functions/params"; -import { toUpperSnakeCase } from "../../functions/secrets"; export const REGION_TBD = "REGION_TBD"; export const SECRET_REF_PREFIX = "FIREBASE_SECRET_REF_"; @@ -738,17 +737,9 @@ function discoverTrigger(endpoint: Endpoint, region: string, r: Resolver): backe } /** - * Prefixes all endpoint IDs and secret names in a build with a given prefix. - * This ensures that functions and their associated secrets from different codebases - * remain isolated and don't conflict when deployed to the same project. - * - * Secret params in a build are rewritten to point to resource names respecting - * the same prefixing, so that the interactive secret creation flow still works - * and the non-interactive message prints the correct secret to create. - * - * When deploying a function which already has secret bindings in its .env files, - * applyEnvSecretBindings will run after this and overwrite both the updated - * secret params and SecretEnvVars to reflect deployed reality. + * Prefixes all endpoint IDs in a build with a given prefix. + * This ensures that functions from different codebases or Kits instances + * don't conflict when deployed to the same project. */ export function applyPrefix(build: Build, prefix: string): void { if (!prefix) { @@ -772,23 +763,9 @@ export function applyPrefix(build: Build, prefix: string): void { } newEndpoints[newId] = endpoint; - - if (endpoint.secretEnvironmentVariables) { - endpoint.secretEnvironmentVariables = endpoint.secretEnvironmentVariables.map((secret) => ({ - ...secret, - secret: toUpperSnakeCase(`${prefix}-${secret.secret}`), - })); - } } build.endpoints = newEndpoints; - for (const param of build.params) { - if (param.type !== "secret") { - continue; - } - param.resourceId = toUpperSnakeCase(`${prefix}-${param.resourceId || param.name}`); - } - if (build.lifecycleHooks) { for (const hook of Object.values(build.lifecycleHooks)) { if ("task" in hook) { diff --git a/src/functions/secrets.ts b/src/functions/secrets.ts index 1f776e0f527..aa95be09a5c 100644 --- a/src/functions/secrets.ts +++ b/src/functions/secrets.ts @@ -52,7 +52,7 @@ type ProjectInfo = { projectNumber: string; }; -export function toUpperSnakeCase(key: string): string { +function toUpperSnakeCase(key: string): string { return key .replace(/[.-]/g, "_") .replace(/([a-z])([A-Z])/g, "$1_$2")