unset secret param handling by Berlioz · Pull Request #11087 · firebase/firebase-tools

Berlioz · 2026-09-15T00:28:55Z

Support the optional field on SecretParams, indicating that the user can decline to bind the SecretParam to a Cloud Secret Manager resource. In this case, the runtime value of the Secret is undefined. This is backed by a new binding type in .env files: it is now legal to bind optional param SECRET_REFs to the empty string.

src/extensions/export.ts: Writes "" to .env for secrets defined in the spec but not present in live. Also skips the unejected secret checks for thos esecrets.

src/deploy/functions/build.ts: Introduces a new unset field on ParsedSecretRef and modifies secret/param binding behavior if this field is true. I experimented with using typescript unions to ensure that unset and secretId can't both be set at the same time and it worked but honestly it was really ugly.

src/deploy/functions/params.ts: Skips the ensureSecret() flow entirely if the unset field has propagated from ParsedSecretRef. Allows users to choose not to create a Secret resource for optional params, which results in the empty string being written to .env.

src/deploy/functions/prepare.ts and src/deploy/functions/backend.ts: If a secret came back from param resolution with an empty string binding, redact it from the Backend's SecretEnvironmentVariables, which control which Secret resources are attached to the function's backend.

UX example:

GOOGLE_API_KEY_FOO (Optional): The value for this secret will be stored in Cloud Secret Manager (https://cloud.google.com/secret-manager/pricing) as GOOGLE_API_KEY_FOO.
✔ What resource ID do you want to use for the backing Secret resource for secret param GOOGLE_API_KEY_FOO? GOOGLE_API_KEY_FOO
✔ Enter a value for GOOGLE_API_KEY_FOO; enter nothing to skip:

gemini-code-assist

Code Review

This pull request introduces support for deliberately unset secrets (such as optional secrets in Extensions) during deployment and export. It updates secret parsing, environment binding, and parameter handling to skip prompting and prevent deployment errors for these unset secrets. The review feedback highlights an issue in applyEnvSecretBindings where a non-array secretEnvironmentVariables could bypass the unset check and trigger a spurious warning; restructuring the condition to always perform an early continue when unset is true is recommended.

Berlioz · 2026-09-15T05:44:43Z

-  force?: boolean,
+  nonInteractive: boolean,
+  force: boolean,
+  recursive = false,


So this is deranged, and probably a sign that this function needs some serious refactoring, but I don't see an immediately better way to implement the new prompting UX we agreed on.

The issue is that we recursively call this function in the case where the resourceId is blank and we prompt the user to enter one (currently that's only when secretEnvParams experiment is on, but presumably that will be the default soon). We do this because if the user enters the name of a secret that already exists, we want to take advantage of all the does-the-secret-exist/does-the-version-exist/is-the-secret-in-a-bad-state logic this function gives us.

This means that the the value for this secret will be stored in Cloud Secret Manager (https://cloud.google.com/secret-manager/pricing)... string will be printed twice in that case, because of the way we reordered the messages being printed.

kmcnellis · 2026-09-15T21:16:40Z

+  if (secretParam.unset) {
+    return "";
+  }
+  secretParam.optional = true;


Is this intentional? the bot thinks its a bug:

⚠️ Critical Finding / Bug
Accidental forced optionality in ensureSecret() (src/deploy/functions/params.ts:294):
secretParam.optional = true; // ⚠️ BUG: Overwrites every secret to optional!

Impact: Line 294 unconditionally mutates secretParam.optional = true for all secrets passing through ensureSecret(). This causes every standard required secret to display (Optional) and ; enter nothing to skip, allowing developers to inadvertently skip required production secrets during firebase deploy.

It's a (communicated to me at least) hack for local testing until Victor makes the change to firebase-functions that enables us to set real optional values. Definitely a bad bug if it was something we were checking in, but it will be removed before we merge.

The real order will be update Functions SDK. Revert this hack, and instead use that to have real optional secrets while doing manual testing for this PR. And then land (after addressing any other substantive comments)

Yeah sorry I mentioned I was doing this in the CF3 chat room, should have added a comment or something to the code

ajperel

AI had a point that if "secretEnvParams" experiment is off optional secrets will break cause we'll never write REF= to the .env file. We should probably force all secrets to be required if secretEnvParams is off and like log a warning to turn on the experiment if we encounter an optional param while it's off.

That said we'll turn the experiment on by default in the next release and probably not turn it off... but we can't control what users do so as long as we have the experiment we should do that defensively. Otherwise folks could get broken. I could see someone turning it off if they just want the old behavior and not to be prompted for secret resource locations.

I will do another review tomorrow morning to try to understand this more deeply myself, this pass was heavily aided by AI.

ajperel · 2026-09-15T21:40:12Z

    if (secretParam.format === "json") {
      validateJsonSecret(secretParam.name, secretValue);
    }
+    if (secretValue === "" && secretParam.optional) {


We should probably do this before we validateJsonSecret right? Because if it's empty it won't be valid JSON.

And we don't need to handle undefined here for JSON cause in this instance prompting returns the empty string and we write and empty ref... it's in functions getter that we do undefined?

ajperel · 2026-09-15T21:50:51Z

+  resolvedSecretRefs: Record<string, string>,
+) {
+  const missingSecrets: string[] = [];
+  for (const [secretName, secretBinding] of Object.entries(resolvedSecretRefs)) {


AI says secretName might be lowercase but secretEnvVar might be uppercase and we could fail to unbind below if cases differ. should we normalize everything to uppcase case just to be safe.

but in at least one other place this didn't seem to be a real thing. Can we hit a problem if users do:

defineSecret("my_secret") instead of defineSecret("MY_SECRET") or something?

ajperel · 2026-09-15T21:51:11Z

+ * Removes any Secret Params with a resolved reference of "", corresponding to an Optional secret that the user has declined to create,
+ * from the SecretEnvironmentVariables that the Functions backend will be provided.
+ */
+export function unbindMissingOptionalSecrets(


Please fix the warning for missing return type since this is a new function.

ajperel · 2026-09-16T00:37:45Z

    }
+    let label = secretParam.label || secretParam.name;
+    if (!recursive) {
+      if (secretParam.optional) {


AI says there is a small bug in and extra (optional) in the value prompt in case A.

Case A: Default (secretEnvParams is OFF)

GOOGLE_API_KEY_FOO (Optional): The value for this secret will be stored in Cloud Secret Manager (https://cloud.google.com/secret-manager/pricing) as GOOGLE_API_KEY_FOO. ? Enter a value for GOOGLE_API_KEY_FOO (Optional); enter nothing to skip:

Case B: Experimental (secretEnvParams is ON)

GOOGLE_API_KEY_FOO (Optional): The value for this secret will be stored in Cloud Secret Manager (https://cloud.google.com/secret-manager/pricing) as GOOGLE_API_KEY_FOO. ✔ What resource ID do you want to use for the backing Secret resource for secret param GOOGLE_API_KEY_FOO? GOOGLE_API_KEY_FOO ? Enter a value for GOOGLE_API_KEY_FOO; enter nothing to skip:

And that the fix is to do:

if (!recursive) { const headerLabel = secretParam.optional ? `${label} (Optional)` : label; const notice = `The value for this secret will be stored in Cloud Secret Manager (https://cloud.google.com/secret-manager/pricing) as ${resourceId}.`; const desc = secretParam.description ? `${secretParam.description} ${notice}` : notice; logger.info(`\n${clc.bold(headerLabel)}: ${(await marked(desc)).trim()}`); }

This is tiny, And we'll probably tweak this again if we pull out prompting so I don't feel strongly about it.

unset secret param handling

1e12de5

Berlioz requested a review from ajperel September 15, 2026 00:28

gemini-code-assist Bot reviewed Sep 15, 2026

View reviewed changes

Comment thread src/deploy/functions/build.ts

Berlioz added 2 commits September 14, 2026 22:37

fully support secret param optionals

00f2cd8

lint

582156c

Berlioz commented Sep 15, 2026

View reviewed changes

Berlioz added 2 commits September 14, 2026 22:46

remove dead code

56f954f

test readability

8c20fa6

kmcnellis reviewed Sep 15, 2026

View reviewed changes

Berlioz added 3 commits September 15, 2026 15:19

remove testing code

996b10f

test

b127603

test but working

7643b4b

ajperel reviewed Sep 16, 2026

View reviewed changes

Navigation Menu

Uh oh!

unset secret param handling - #11087

unset secret param handling#11087
Berlioz wants to merge 8 commits into
mainfrom
vsfan_optional_secrets

Berlioz commented Sep 15, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Berlioz Sep 15, 2026

Uh oh!

kmcnellis Sep 15, 2026

Uh oh!

kmcnellis Sep 15, 2026

Uh oh!

ajperel Sep 15, 2026

Uh oh!

Berlioz Sep 15, 2026

Uh oh!

ajperel left a comment

Uh oh!

ajperel Sep 15, 2026

Uh oh!

ajperel Sep 15, 2026

Uh oh!

ajperel Sep 15, 2026

Uh oh!

Uh oh!

ajperel Sep 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Berlioz commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajperel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Berlioz commented Sep 15, 2026 •

edited

Loading