ext:export improvements (smarter choice of where to write file to, automatic secret ejection) by Berlioz · Pull Request #10932 · firebase/firebase-tools

Berlioz · 2026-08-12T23:40:00Z

No description provided.

wiz-9635d3485b · 2026-08-12T23:40:32Z

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	-
Sensitive Data	-
Secrets	-
IaC Misconfigurations	-
SAST Findings	3
Software Management Findings	-

Total	3

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio.

gemini-code-assist

Code Review

This pull request updates the ext:export command to support exporting Extension instances to Function Kits, introducing new options (--kit and --outputDir) and logic to transfer control of secrets from Extensions to Kits. The review identified a few critical issues: a potential TypeError in ext-export.ts when functions is not defined in firebase.json, another potential TypeError in export.ts when an optional secret parameter is missing from liveParams, and a style guide violation where a user-facing error is logged instead of throwing a FirebaseError.

gemini-code-assist · 2026-08-12T23:40:55Z

+    const functionsConfig = firebaseConfig.get("functions");
+    const functions = Array.isArray(functionsConfig) ? functionsConfig : [functionsConfig];
+    for (const fn of functions) {
+      if (typeof fn.kit === "undefined" || typeof fn.instances === "undefined") {
+        continue;
+      }


If functionsConfig is undefined (e.g., when functions is not defined in firebase.json), functions will be initialized as [undefined]. When iterating, fn will be undefined, causing a TypeError: Cannot read properties of undefined (reading 'kit') when evaluating typeof fn.kit. We should handle the case where functionsConfig is falsy or filter out undefined values.

const functionsConfig = firebaseConfig.get("functions"); const functions = Array.isArray(functionsConfig) ? functionsConfig : functionsConfig ? [functionsConfig] : []; for (const fn of functions) { if (!fn || typeof fn.kit === "undefined" || typeof fn.instances === "undefined") { continue; }

the config loader coerces the config to always contain a default functions config if one isn't present

gemini-code-assist · 2026-08-12T23:40:55Z

+    const secretName = specParam.param;
+    const resourceName = liveParams[secretName];
+    const match = resourceName.match(SECRET_VERSION_NAME_REGEX);


If a secret parameter is optional or not set in liveParams, resourceName will be undefined. Calling resourceName.match(...) will result in a runtime crash (TypeError: Cannot read properties of undefined (reading 'match')). We should add a defensive check to ensure resourceName is defined before attempting to match it.

Suggested change

const secretName = specParam.param;

const resourceName = liveParams[secretName];

const match = resourceName.match(SECRET_VERSION_NAME_REGEX);

const secretName = specParam.param;

const resourceName = liveParams[secretName];

if (!resourceName) {

continue;

}

const match = resourceName.match(SECRET_VERSION_NAME_REGEX);

pretty sure that can't happen in an ACTIVE extension since there's no way to define a secret param as optional, but it's true that this could be a problem if run on an ERRORED extension for some reason.

gemini-code-assist · 2026-08-12T23:40:55Z

+  if (instance.state !== "ACTIVE" && !options.force) {
+    logger.error(
+      `Extension ${options.instance} is in state ${instance.state}. To export a non-ACTIVE extension, use the --force option.`,
+    );
+    return;
+  }


According to the repository style guide, expected user-facing errors should throw a FirebaseError instead of logging an error and returning. This ensures the CLI exits with a non-zero status code and handles the error consistently.

if (instance.state !== "ACTIVE" && !options.force) { throw new FirebaseError( `Extension ${options.instance} is in state ${instance.state}. To export a non-ACTIVE extension, use the --force option.`, ); }

References

Throw FirebaseError for expected, user-facing errors. ^(link)

ajperel

A need to review more closely, but here's some initial feedback.

ajperel · 2026-08-13T01:20:08Z

+  if (secretCount > 0) {
+    if (
+      !(await confirm({
+        message: `${secretCount} Cloud Secret Manager resources found in export. Transfer control of secrets from Extensions to Kits?`,


Nit: Kits aren't going to manage secrets. This sounds like something else is going to take it up for auto managing for them.

They must self-manage secrets after this.

Also doesn't help a user understand why this is important. If we're going to prompt we should probably explain why.

?.. in export. Stop Firebase Extensions management of secrets? This is necessary to prevent extension uninstall from deleting potentially migrated secrets."

I don't love this.

I still wonder if it should be a flag rather than a prompt. Maybe worth a quick discussion with @inlined

ajperel · 2026-08-13T01:22:04Z


+/**
+ * PATCHes a Secret resource by removing any "firebase-extensions-managed" labels
+ * and replacing them with "firebase-managed": "functions"


ohh this is new to me. What's the implication of firebase-managed: functions? Do we do that for other secrets we right from the command line?

Yeah, we actually have a small collection of bugs because it used to be firebase-managed: true but now it's been split into firebase-managed: functions and firebase-managed: apphosting.

ajperel

Some mostly nitty comments the substantive thing being we need to make sure we handle secrets not being disassociated from Extensions well.

Also you should add a CHANGELOG and maybe some tests?

ajperel · 2026-08-13T23:45:43Z

  )
  .option(
-    `--instance <instanceId>`,
+    `-e`,


As nice as this is, it's a breaking change and I don't want to break anything that currently scripts usage of --instance.

I'd just keep it as is, though you could alias and deprecate if we really wanted.

ajperel · 2026-08-13T23:46:35Z

-    if (experiments.isEnabled("extMigrationFeatures") && options.mode === "functions") {
+    if (
+      experiments.isEnabled("extMigrationFeatures") &&
+      (options.mode === "functions" || options.mode === "kits")


I think you took previous feedback and we no longer have "kits"?

ajperel · 2026-08-13T23:50:41Z

+      if (typeof fn.kit === "undefined" || typeof fn.instances === "undefined") {
+        continue;
+      }
+      for (const [kitId, kitPath] of Object.entries(fn.instances)) {


nit: probably better as instanceId, instanceConfigPath or something like that.

ajperel · 2026-08-13T23:54:24Z

+    }
+    const projectId = match.groups.project;
+    const secretId = match.groups.secret;
+    await transferSecretToKits(projectId, secretId);


Somewhere around this we should have some really good error messaging to users. I really want to avoid the case where users successfully get a .env file but fail to unmanage secrets. In that case if they move forward with the migration and the secrets are deleted they're just totally lost.

We should at the very least warn LOUDLY that secrets will be deleted and they should re-export to try and fix. But maybe we should go so far as to fail writing the .env file so they have to repeat.

ajperel · 2026-08-13T23:55:16Z

+        newLabels[labelKey] = labelValue;
+      }
+    }
+  } catch (err: any) {


Joe's review bot says you should really use unknown instead of any.

inlined

I think it LGTM with the nit that I'm going to need to be able to call some of this code without it going to the file system (because I'll pass it to init instead)

inlined · 2026-08-14T01:15:50Z

-  // Write to firebase root if inside a firebase project dir, otherwise <currentDir>/<instanceId>
-  let writeLocationOpts: UserEnvsOpts;
+  const writeLocation: UserEnvsOpts = kitExportTarget(instanceId, projectId, options);
+  if (hasUserEnvs(writeLocation)) {


For me to use this in ext:migrate I'm going to need you to separate the code that generates environment variables from the code that saves them to disk. This is generally a good thing for testability anyway.

If I'm understanding you correctly, this is already implemented in the functionsEnvFromInstance helper defined in src/extensions/export.ts

inlined · 2026-08-14T01:16:56Z

+    writeUserEnvs(convertedEnv, writeLocation);
+  }
+
+  if (secretCount > 0) {


nit: I usually prefer either testing the opposite and returning early or having a helper alone. Helps you avoid pyramid code.

inlined · 2026-08-14T01:18:21Z

+    const functionsConfig = firebaseConfig.get("functions");
+    const functions = Array.isArray(functionsConfig) ? functionsConfig : [functionsConfig];
+    for (const fn of functions) {
+      if (typeof fn.kit === "undefined" || typeof fn.instances === "undefined") {


Is it not valid to use this command to create a new/first instance?

It should be. The if() on l:247 is more of a type narrowing clause, since the Config made available by the CLI doesn't validate the format by default. It looks like the .src getter on it does validate though, so I'm moving the code to rely on that for readability.

inlined · 2026-08-14T01:21:12Z

+    const resourceName = liveParams[secretName];
+    if (!resourceName) {
+      throw new FirebaseError(
+        `Secret ${secretName} was defined in the extension spec, but is missing in live deployed secrets.`,


This seems like something that should never happen. Should it have a different exit code?

I'd forgotten about that. Good catch.

ext:export understands kits in firebase.json and ejects secrets

f103e23

gemini-code-assist Bot reviewed Aug 12, 2026

View reviewed changes

slop

4b14150

Berlioz requested a review from ajperel August 12, 2026 23:52

Berlioz assigned ajperel Aug 12, 2026

Berlioz changed the title ~~WIP: ext:export improvements~~ ext:export improvements (smarter choice of where to write file to, automatic secret ejection) Aug 12, 2026

ajperel reviewed Aug 13, 2026

View reviewed changes

Berlioz added 2 commits August 13, 2026 14:57

first review pass

9aabc10

lint

552ae2f

ajperel requested a review from inlined August 13, 2026 23:37

ajperel assigned inlined Aug 13, 2026

ajperel reviewed Aug 13, 2026

View reviewed changes

inlined approved these changes Aug 14, 2026

View reviewed changes

Berlioz added 2 commits August 17, 2026 16:04

Merge branch 'main' into vsfan_eject_secrets

062aad7

review

e164b47

Uh oh!

Conversation

Berlioz commented Aug 12, 2026

Uh oh!

wiz-9635d3485b Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Wiz Scan Summary

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 12, 2026

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inlined 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wiz-9635d3485b Bot commented Aug 12, 2026 •

edited

Loading