From bfd3441bf80691eddd06c277ca23954fb451a171 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 20 Oct 2026 14:55:08 +0100 Subject: [PATCH 1/8] fix(delete-user-data): reject invalid select and regex param values at deploy Values already in .env skip the CLI's prompt-time validation, so the kit deployed with values the extension refuses at install time and then quietly did less, e.g. FIRESTORE_DELETE_MODE=RECURSIVE ran as shallow and ENABLE_AUTO_DISCOVERY=true left auto-discovery off. Check every param's declared select options and validationRegex at module load, so discovery fails with the param, the bad value and the allowed values. Fixes #3258 --- kits/delete-user-data/CHANGELOG.md | 2 + kits/delete-user-data/README.md | 18 +++ kits/delete-user-data/src/config.ts | 46 +++++++ kits/delete-user-data/src/index.ts | 2 + .../tests/valid-params.test.ts | 124 ++++++++++++++++++ 5 files changed, 192 insertions(+) create mode 100644 kits/delete-user-data/tests/valid-params.test.ts diff --git a/kits/delete-user-data/CHANGELOG.md b/kits/delete-user-data/CHANGELOG.md index 0e40c6a6f..7462977be 100644 --- a/kits/delete-user-data/CHANGELOG.md +++ b/kits/delete-user-data/CHANGELOG.md @@ -1,3 +1,5 @@ +- fix: invalid values in `.env` now fail the deploy instead of deploying and quietly doing less. The Firebase CLI only checks values it prompts for, so `FIRESTORE_DELETE_MODE=RECURSIVE` deployed and ran as `shallow` (leaving subcollections behind), and `ENABLE_AUTO_DISCOVERY=true` deployed with auto-discovery off. The kit now rejects `FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` and `SELECTED_DATABASE_LOCATION` values that are not one of their options, and `SELECTED_DATABASE_INSTANCE` and `CLOUD_STORAGE_BUCKET` values that fail the extension's format checks, as the extension did at install time. Option labels such as `Recursive` and `Yes` are rejected too. + - fix: an Eventarc publish failure no longer fails the function. A channel that was deleted or is unreachable answers `PERMISSION_DENIED`, and the publish was awaited after the work, so the deletion completed but the invocation still failed. The publish failure is now logged as a warning and the invocation continues. Deploys also declare `eventarcpublishing.googleapis.com`, so the API is enabled on your project whether or not you use events. The failed invocation used to be redelivered, re-running a deletion that had already succeeded; it no longer is. - fix: `ENABLE_AUTO_DISCOVERY` accepts the extension's `yes` / `no` values again. The kit had declared it as a boolean param, which only counts the literal `true` as enabled, so a `.env` copied from an installed extension instance carried `yes` and silently left auto-discovery switched off. The param is now the extension's labeled `Yes` / `No` select and is read the same way the extension read it. diff --git a/kits/delete-user-data/README.md b/kits/delete-user-data/README.md index 465d7a7cd..c896db787 100644 --- a/kits/delete-user-data/README.md +++ b/kits/delete-user-data/README.md @@ -96,6 +96,15 @@ Deploy a single instance with `firebase deploy --only functions:`. Set these values in a `.env` (or `.env.`) file. The Firebase CLI loads them at deploy time and prompts for any required values that are missing. +The CLI does not check values that are already in `.env`, so the kit checks +them itself when the deploy loads it, as the extension did at install time. The +deploy fails with an error naming the setting and the allowed values if +`FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` or +`SELECTED_DATABASE_LOCATION` is not one of the values listed below, or if +`SELECTED_DATABASE_INSTANCE` or `CLOUD_STORAGE_BUCKET` is not in the format the +extension accepted (an instance id rather than a URL, and a bucket name without +`gs://` or capital letters). The values are case sensitive. + The instance id is not a setting: the CLI provides it to each instance as `FIREBASE_KIT_INSTANCE_ID`, set to that instance's key in the `instances` map. `FIREBASE_` is a reserved prefix in `.env` files, so it cannot be set or @@ -215,6 +224,15 @@ The Firebase CLI now sets it to the function that published the event so it differs between `clearData`, `handleSearch` and `handleDeletion`. If your event triggers filter on `source`, filter on the event `type` instead. +### Option labels are not accepted + +The extension's installer accepted an option's label (such as `Recursive` or +`Yes`) as well as its value, but then did not recognize it: `Recursive` deleted +shallowly and `Yes` left auto-discovery off. The kit accepts only the values +(`recursive`, `shallow`, `yes`, `no`, and the location ids), and fails the +deploy on a label. If you copied your `.env` from an extension instance that +was installed with a label, change it to the value. + ### Unchanged Event types and payloads are the same. The functions still publish diff --git a/kits/delete-user-data/src/config.ts b/kits/delete-user-data/src/config.ts index e172ae79e..d731a02a6 100644 --- a/kits/delete-user-data/src/config.ts +++ b/kits/delete-user-data/src/config.ts @@ -258,3 +258,49 @@ export function assertRequiredParams( ); } } + +/** + * Rejects param values the published extension refuses at install time: a + * select value that is not one of its option values, or a text value that + * fails its `validationRegex`. + * + * The CLI applies a param's `input` only when it prompts, so a value already + * in `.env` reaches the handlers unchecked, and they compare it with `===` + * (`FIRESTORE_DELETE_MODE=RECURSIVE` runs as shallow, for example). Option + * labels such as `Recursive` are rejected too, as the handlers only understand + * the values. Unset and empty values are skipped: defaults cover the first, + * and `assertRequiredParams` decides about the second. Called at module scope + * so deploy-time discovery fails before the function ships. + */ +export function assertValidParams(): void { + const invalid: string[] = []; + + for (const param of Object.values(params)) { + const raw = process.env[param.name]; + const input = param.options?.input; + if (raw === undefined || raw === "" || !input) { + continue; + } + + const setting = `${param.name}=${JSON.stringify(raw)}`; + if ("select" in input) { + const allowed = input.select.options.map(({ value }) => String(value)); + if (!allowed.includes(raw)) { + invalid.push(`${setting} must be one of: ${allowed.join(", ")}`); + } + } else if ("text" in input && input.text.validationRegex) { + const regex = new RegExp(input.text.validationRegex); + if (!regex.test(raw)) { + const reason = input.text.validationErrorMessage ?? "Invalid value"; + invalid.push(`${setting}: ${reason} (must match ${regex})`); + } + } + } + + if (invalid.length > 0) { + throw new Error( + `Invalid parameter values: ${invalid.join("; ")}. ` + + "Fix them in your .env file, or remove the entries to use their defaults." + ); + } +} diff --git a/kits/delete-user-data/src/index.ts b/kits/delete-user-data/src/index.ts index cca4a43d0..4ee6caf84 100644 --- a/kits/delete-user-data/src/index.ts +++ b/kits/delete-user-data/src/index.ts @@ -23,6 +23,7 @@ import { onUserDeleted } from "firebase-functions/v2/identity"; import { onMessagePublished } from "firebase-functions/v2/pubsub"; import { assertRequiredParams, + assertValidParams, CONFIG_EXPRESSIONS, configFromEnv, } from "./config"; @@ -37,6 +38,7 @@ import { import * as logs from "./logs"; assertRequiredParams(); +assertValidParams(); export * from "./lib"; const REQUIRED_ROLES: ReadonlyArray = [ diff --git a/kits/delete-user-data/tests/valid-params.test.ts b/kits/delete-user-data/tests/valid-params.test.ts new file mode 100644 index 000000000..e998a4ab9 --- /dev/null +++ b/kits/delete-user-data/tests/valid-params.test.ts @@ -0,0 +1,124 @@ +/** + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { afterEach, beforeAll, describe, expect, test, vi } from "vitest"; + +let assertValidParams: () => void; + +beforeAll(async () => { + process.env.FIREBASE_KIT_INSTANCE_ID ??= "test-instance"; + ({ assertValidParams } = await import("../src/config")); +}); + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +// Values the published extension's install API rejects (checked with +// `validateOnly` against firebase/delete-user-data@0.1.30). +const REJECTED: Array<[string, string, string]> = [ + ["FIRESTORE_DELETE_MODE", "RECURSIVE", "recursive, shallow"], + ["FIRESTORE_DELETE_MODE", "recursivex", "recursive, shallow"], + ["FIRESTORE_DELETE_MODE", "recursive ", "recursive, shallow"], + ["ENABLE_AUTO_DISCOVERY", "true", "yes, no"], + ["ENABLE_AUTO_DISCOVERY", "YES", "yes, no"], + [ + "SELECTED_DATABASE_LOCATION", + "us-east1", + "us-central1, europe-west1, asia-southeast1", + ], + [ + "SELECTED_DATABASE_LOCATION", + "US-CENTRAL1", + "us-central1, europe-west1, asia-southeast1", + ], + [ + "SELECTED_DATABASE_INSTANCE", + "https://x.firebaseio.com", + "Invalid database instance", + ], + ["CLOUD_STORAGE_BUCKET", "gs://x", "Invalid storage bucket"], + ["CLOUD_STORAGE_BUCKET", "Upper_Case", "Invalid storage bucket"], +]; + +const ACCEPTED: Array<[string, string]> = [ + ["FIRESTORE_DELETE_MODE", "recursive"], + ["FIRESTORE_DELETE_MODE", "shallow"], + ["ENABLE_AUTO_DISCOVERY", "yes"], + ["ENABLE_AUTO_DISCOVERY", "no"], + ["SELECTED_DATABASE_LOCATION", "us-central1"], + ["SELECTED_DATABASE_LOCATION", "europe-west1"], + ["SELECTED_DATABASE_LOCATION", "asia-southeast1"], + ["SELECTED_DATABASE_INSTANCE", "my-instance"], + ["CLOUD_STORAGE_BUCKET", "my-project-12345.appspot.com"], + ["CLOUD_STORAGE_BUCKET", "my_bucket-1.example"], +]; + +describe("assertValidParams", () => { + test("passes when no params have been supplied yet", () => { + expect(() => assertValidParams()).not.toThrow(); + }); + + test.each(REJECTED)("rejects %s=%j", (name, value, allowed) => { + vi.stubEnv(name, value); + + expect(() => assertValidParams()).toThrow( + `${name}=${JSON.stringify(value)}` + ); + expect(() => assertValidParams()).toThrow(allowed); + }); + + test.each(ACCEPTED)("accepts %s=%j", (name, value) => { + vi.stubEnv(name, value); + + expect(() => assertValidParams()).not.toThrow(); + }); + + // The extension accepts a select label, stores it verbatim, and its runtime + // then treats `Recursive` as shallow and `Yes` as off. Only values are safe. + test.each([ + ["FIRESTORE_DELETE_MODE", "Recursive"], + ["ENABLE_AUTO_DISCOVERY", "Yes"], + ["SELECTED_DATABASE_LOCATION", "Belgium"], + ])("rejects the option label %s=%j", (name, value) => { + vi.stubEnv(name, value); + + expect(() => assertValidParams()).toThrow(name); + }); + + // Blank values belong to assertRequiredParams; optional ones may be blank, + // as in the extension. + test.each([ + "FIRESTORE_DELETE_MODE", + "ENABLE_AUTO_DISCOVERY", + "SELECTED_DATABASE_LOCATION", + "SELECTED_DATABASE_INSTANCE", + "CLOUD_STORAGE_BUCKET", + ])("leaves an empty %s alone", (name) => { + vi.stubEnv(name, ""); + + expect(() => assertValidParams()).not.toThrow(); + }); + + test("names every invalid param in one error", () => { + vi.stubEnv("FIRESTORE_DELETE_MODE", "RECURSIVE"); + vi.stubEnv("ENABLE_AUTO_DISCOVERY", "true"); + + expect(() => assertValidParams()).toThrow( + /FIRESTORE_DELETE_MODE="RECURSIVE".*; ENABLE_AUTO_DISCOVERY="true"/ + ); + }); +}); From b13b5d5b8a6d677904ee3303953dac0ce5b5c02d Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 27 Oct 2026 11:21:53 +0000 Subject: [PATCH 2/8] fix(delete-user-data): accept the Shallow and No option labels The extension only checks for `recursive` and `yes`, so `Shallow` and `No` ran exactly like their values. Only reject the labels it misread: `Recursive`, `Yes` and the location labels. --- kits/delete-user-data/CHANGELOG.md | 2 +- kits/delete-user-data/README.md | 17 +++++++++-------- kits/delete-user-data/src/config.ts | 15 +++++++++++---- .../delete-user-data/tests/valid-params.test.ts | 13 ++++++++++++- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/kits/delete-user-data/CHANGELOG.md b/kits/delete-user-data/CHANGELOG.md index 7462977be..f93adb85e 100644 --- a/kits/delete-user-data/CHANGELOG.md +++ b/kits/delete-user-data/CHANGELOG.md @@ -1,4 +1,4 @@ -- fix: invalid values in `.env` now fail the deploy instead of deploying and quietly doing less. The Firebase CLI only checks values it prompts for, so `FIRESTORE_DELETE_MODE=RECURSIVE` deployed and ran as `shallow` (leaving subcollections behind), and `ENABLE_AUTO_DISCOVERY=true` deployed with auto-discovery off. The kit now rejects `FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` and `SELECTED_DATABASE_LOCATION` values that are not one of their options, and `SELECTED_DATABASE_INSTANCE` and `CLOUD_STORAGE_BUCKET` values that fail the extension's format checks, as the extension did at install time. Option labels such as `Recursive` and `Yes` are rejected too. +- fix: invalid values in `.env` now fail the deploy instead of deploying and quietly doing less. The Firebase CLI only checks values it prompts for, so `FIRESTORE_DELETE_MODE=RECURSIVE` deployed and ran as `shallow` (leaving subcollections behind), and `ENABLE_AUTO_DISCOVERY=true` deployed with auto-discovery off. The kit now rejects `FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` and `SELECTED_DATABASE_LOCATION` values that are not one of their options, and `SELECTED_DATABASE_INSTANCE` and `CLOUD_STORAGE_BUCKET` values that fail the extension's format checks, as the extension did at install time. The option labels the extension misread (`Recursive`, `Yes` and the location labels) are rejected too. - fix: an Eventarc publish failure no longer fails the function. A channel that was deleted or is unreachable answers `PERMISSION_DENIED`, and the publish was awaited after the work, so the deletion completed but the invocation still failed. The publish failure is now logged as a warning and the invocation continues. Deploys also declare `eventarcpublishing.googleapis.com`, so the API is enabled on your project whether or not you use events. The failed invocation used to be redelivered, re-running a deletion that had already succeeded; it no longer is. diff --git a/kits/delete-user-data/README.md b/kits/delete-user-data/README.md index c896db787..a7164b881 100644 --- a/kits/delete-user-data/README.md +++ b/kits/delete-user-data/README.md @@ -224,14 +224,15 @@ The Firebase CLI now sets it to the function that published the event so it differs between `clearData`, `handleSearch` and `handleDeletion`. If your event triggers filter on `source`, filter on the event `type` instead. -### Option labels are not accepted - -The extension's installer accepted an option's label (such as `Recursive` or -`Yes`) as well as its value, but then did not recognize it: `Recursive` deleted -shallowly and `Yes` left auto-discovery off. The kit accepts only the values -(`recursive`, `shallow`, `yes`, `no`, and the location ids), and fails the -deploy on a label. If you copied your `.env` from an extension instance that -was installed with a label, change it to the value. +### Some option labels are not accepted + +The extension's installer accepted an option's label as well as its value, but +then misread `Recursive` (it deleted shallowly), `Yes` (it left auto-discovery +off) and the location labels (`United States`, `Belgium`, `Singapore`). The kit +fails the deploy on these labels. If you copied your `.env` from an extension +instance that was installed with one, change it to the value (`recursive`, +`yes`, or the location id). `Shallow` and `No` worked in the extension and are +still accepted. ### Unchanged diff --git a/kits/delete-user-data/src/config.ts b/kits/delete-user-data/src/config.ts index d731a02a6..210d6f2e4 100644 --- a/kits/delete-user-data/src/config.ts +++ b/kits/delete-user-data/src/config.ts @@ -259,6 +259,12 @@ export function assertRequiredParams( } } +// The extension stored these labels as-is and ran them like their value. +const SAFE_LABELS: Readonly> = { + FIRESTORE_DELETE_MODE: ["Shallow"], + ENABLE_AUTO_DISCOVERY: ["No"], +}; + /** * Rejects param values the published extension refuses at install time: a * select value that is not one of its option values, or a text value that @@ -268,9 +274,10 @@ export function assertRequiredParams( * in `.env` reaches the handlers unchecked, and they compare it with `===` * (`FIRESTORE_DELETE_MODE=RECURSIVE` runs as shallow, for example). Option * labels such as `Recursive` are rejected too, as the handlers only understand - * the values. Unset and empty values are skipped: defaults cover the first, - * and `assertRequiredParams` decides about the second. Called at module scope - * so deploy-time discovery fails before the function ships. + * the values, except those in `SAFE_LABELS`. Unset and empty values are + * skipped: defaults cover the first, and `assertRequiredParams` decides about + * the second. Called at module scope so deploy-time discovery fails before the + * function ships. */ export function assertValidParams(): void { const invalid: string[] = []; @@ -285,7 +292,7 @@ export function assertValidParams(): void { const setting = `${param.name}=${JSON.stringify(raw)}`; if ("select" in input) { const allowed = input.select.options.map(({ value }) => String(value)); - if (!allowed.includes(raw)) { + if (!allowed.includes(raw) && !SAFE_LABELS[param.name]?.includes(raw)) { invalid.push(`${setting} must be one of: ${allowed.join(", ")}`); } } else if ("text" in input && input.text.validationRegex) { diff --git a/kits/delete-user-data/tests/valid-params.test.ts b/kits/delete-user-data/tests/valid-params.test.ts index e998a4ab9..d6b57db71 100644 --- a/kits/delete-user-data/tests/valid-params.test.ts +++ b/kits/delete-user-data/tests/valid-params.test.ts @@ -88,7 +88,7 @@ describe("assertValidParams", () => { }); // The extension accepts a select label, stores it verbatim, and its runtime - // then treats `Recursive` as shallow and `Yes` as off. Only values are safe. + // then treats `Recursive` as shallow and `Yes` as off. test.each([ ["FIRESTORE_DELETE_MODE", "Recursive"], ["ENABLE_AUTO_DISCOVERY", "Yes"], @@ -99,6 +99,17 @@ describe("assertValidParams", () => { expect(() => assertValidParams()).toThrow(name); }); + // The runtime only checks for `recursive` and `yes`, so these labels already + // ran like their values. + test.each([ + ["FIRESTORE_DELETE_MODE", "Shallow"], + ["ENABLE_AUTO_DISCOVERY", "No"], + ])("accepts the option label %s=%j", (name, value) => { + vi.stubEnv(name, value); + + expect(() => assertValidParams()).not.toThrow(); + }); + // Blank values belong to assertRequiredParams; optional ones may be blank, // as in the extension. test.each([ From dce498fb40c7b3b874075888e14cfbbb654b8ba7 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 27 Oct 2026 12:02:45 +0000 Subject: [PATCH 3/8] fix(delete-user-data): default a blank RTDB location to us-central1 --- kits/delete-user-data/src/export-config.ts | 5 +++-- kits/delete-user-data/tests/export-config.test.ts | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/kits/delete-user-data/src/export-config.ts b/kits/delete-user-data/src/export-config.ts index 240dbea0d..b36baff64 100644 --- a/kits/delete-user-data/src/export-config.ts +++ b/kits/delete-user-data/src/export-config.ts @@ -40,7 +40,7 @@ export interface ResolvedDeleteUserDataConfig { firestoreDatabaseId: string; firestoreDeleteMode: FirestoreDeleteMode; rtdbInstance?: string; - rtdbLocation?: string; + rtdbLocation: string; rtdbPaths?: string; storageBucket?: string; storagePaths?: string; @@ -58,6 +58,7 @@ const DEFAULT_FIRESTORE_DATABASE_ID = "(default)"; const DEFAULT_FIRESTORE_DELETE_MODE: FirestoreDeleteMode = "shallow"; const DEFAULT_SEARCH_DEPTH = 3; const DEFAULT_SEARCH_FIELDS = "id,uid,userId"; +const DEFAULT_RTDB_LOCATION = "us-central1"; function topicName( name: string | undefined, instanceId: string, @@ -77,7 +78,7 @@ export function resolveDeleteUserDataConfig( firestoreDeleteMode: config.firestoreDeleteMode ?? DEFAULT_FIRESTORE_DELETE_MODE, rtdbInstance: config.rtdbInstance, - rtdbLocation: config.rtdbLocation, + rtdbLocation: config.rtdbLocation ?? DEFAULT_RTDB_LOCATION, rtdbPaths: config.rtdbPaths, storageBucket: config.storageBucket, storagePaths: config.storagePaths, diff --git a/kits/delete-user-data/tests/export-config.test.ts b/kits/delete-user-data/tests/export-config.test.ts index 549d31d0f..1d6147ff5 100644 --- a/kits/delete-user-data/tests/export-config.test.ts +++ b/kits/delete-user-data/tests/export-config.test.ts @@ -57,6 +57,7 @@ describe("resolveDeleteUserDataConfig", () => { expect(config.firestoreDatabaseId).toBe("(default)"); expect(config.firestoreDeleteMode).toBe("shallow"); + expect(config.rtdbLocation).toBe("us-central1"); expect(config.enableAutoDiscovery).toBe(false); expect(config.searchDepth).toBe(3); expect(config.searchFields).toBe("id,uid,userId"); From d279addd0a5e4cf1ce79f4c7dc48ce812e273441 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 27 Oct 2026 12:37:45 +0000 Subject: [PATCH 4/8] fix(delete-user-data): tidy invalid-param errors, docs and tests --- kits/delete-user-data/README.md | 12 +++++++----- kits/delete-user-data/src/config.ts | 8 +++++--- kits/delete-user-data/tests/index.test.ts | 18 +++++++++++++++++- .../tests/valid-params.test.ts | 9 +++++++++ 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/kits/delete-user-data/README.md b/kits/delete-user-data/README.md index a7164b881..d1cb1fc6c 100644 --- a/kits/delete-user-data/README.md +++ b/kits/delete-user-data/README.md @@ -98,12 +98,14 @@ loads them at deploy time and prompts for any required values that are missing. The CLI does not check values that are already in `.env`, so the kit checks them itself when the deploy loads it, as the extension did at install time. The -deploy fails with an error naming the setting and the allowed values if -`FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` or -`SELECTED_DATABASE_LOCATION` is not one of the values listed below, or if +deploy fails with an error naming the setting if `FIRESTORE_DELETE_MODE`, +`ENABLE_AUTO_DISCOVERY` or `SELECTED_DATABASE_LOCATION` is not one of the +values listed below (the error lists the allowed values), or if `SELECTED_DATABASE_INSTANCE` or `CLOUD_STORAGE_BUCKET` is not in the format the -extension accepted (an instance id rather than a URL, and a bucket name without -`gs://` or capital letters). The values are case sensitive. +extension accepted: an instance id rather than a URL, and a bucket name without +`gs://` or capital letters (the error gives the extension's message). The values +are case sensitive. The emulator runs the same check, so a bad value stops that +instance's functions from loading there. The instance id is not a setting: the CLI provides it to each instance as `FIREBASE_KIT_INSTANCE_ID`, set to that instance's key in the `instances` map. diff --git a/kits/delete-user-data/src/config.ts b/kits/delete-user-data/src/config.ts index 210d6f2e4..3d80e88fb 100644 --- a/kits/delete-user-data/src/config.ts +++ b/kits/delete-user-data/src/config.ts @@ -298,8 +298,9 @@ export function assertValidParams(): void { } else if ("text" in input && input.text.validationRegex) { const regex = new RegExp(input.text.validationRegex); if (!regex.test(raw)) { - const reason = input.text.validationErrorMessage ?? "Invalid value"; - invalid.push(`${setting}: ${reason} (must match ${regex})`); + const reason = + input.text.validationErrorMessage ?? `must match ${regex}`; + invalid.push(`${setting}: ${reason}`); } } } @@ -307,7 +308,8 @@ export function assertValidParams(): void { if (invalid.length > 0) { throw new Error( `Invalid parameter values: ${invalid.join("; ")}. ` + - "Fix them in your .env file, or remove the entries to use their defaults." + "Fix them in this instance's .env or .env. file, or remove " + + "the entries to use their defaults." ); } } diff --git a/kits/delete-user-data/tests/index.test.ts b/kits/delete-user-data/tests/index.test.ts index b031ab638..36b50455c 100644 --- a/kits/delete-user-data/tests/index.test.ts +++ b/kits/delete-user-data/tests/index.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { beforeEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; vi.mock("../src/logs"); vi.mock("../src/handlers", async (importOriginal) => ({ @@ -68,3 +68,19 @@ describe("clearData", () => { expect(logs.deletionEventMissingUid).toHaveBeenCalledWith("event-id"); }); }); + +describe("module load", () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + + // Deploy-time discovery imports the kit, so this is what fails the deploy. + test("throws when a param value is invalid", async () => { + vi.resetModules(); + vi.stubEnv("FIRESTORE_DELETE_MODE", "RECURSIVE"); + + await expect(import("../src/index")).rejects.toThrow( + 'FIRESTORE_DELETE_MODE="RECURSIVE"' + ); + }); +}); diff --git a/kits/delete-user-data/tests/valid-params.test.ts b/kits/delete-user-data/tests/valid-params.test.ts index d6b57db71..8a716cab2 100644 --- a/kits/delete-user-data/tests/valid-params.test.ts +++ b/kits/delete-user-data/tests/valid-params.test.ts @@ -52,6 +52,13 @@ const REJECTED: Array<[string, string, string]> = [ ], ["CLOUD_STORAGE_BUCKET", "gs://x", "Invalid storage bucket"], ["CLOUD_STORAGE_BUCKET", "Upper_Case", "Invalid storage bucket"], + ["CLOUD_STORAGE_BUCKET", "abc\n", "Invalid storage bucket"], + ["SELECTED_DATABASE_INSTANCE", "abc\n", "Invalid database instance"], + [ + "SELECTED_DATABASE_LOCATION", + " ", + "us-central1, europe-west1, asia-southeast1", + ], ]; const ACCEPTED: Array<[string, string]> = [ @@ -63,6 +70,8 @@ const ACCEPTED: Array<[string, string]> = [ ["SELECTED_DATABASE_LOCATION", "europe-west1"], ["SELECTED_DATABASE_LOCATION", "asia-southeast1"], ["SELECTED_DATABASE_INSTANCE", "my-instance"], + ["SELECTED_DATABASE_INSTANCE", " my-instance"], + ["SELECTED_DATABASE_INSTANCE", "a b"], ["CLOUD_STORAGE_BUCKET", "my-project-12345.appspot.com"], ["CLOUD_STORAGE_BUCKET", "my_bucket-1.example"], ]; From 45e1bd16460a7012f0079d79122668c138347897 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 27 Oct 2026 15:46:35 +0000 Subject: [PATCH 5/8] fix(delete-user-data): keep a blank RTDB location unset Reverts the us-central1 default for a blank SELECTED_DATABASE_LOCATION. The extension skips RTDB paths when the location is blank, and the README documents that gate; defaulting it pointed EU instances at the US host. --- kits/delete-user-data/src/export-config.ts | 5 ++--- kits/delete-user-data/tests/export-config.test.ts | 9 ++++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/kits/delete-user-data/src/export-config.ts b/kits/delete-user-data/src/export-config.ts index b36baff64..240dbea0d 100644 --- a/kits/delete-user-data/src/export-config.ts +++ b/kits/delete-user-data/src/export-config.ts @@ -40,7 +40,7 @@ export interface ResolvedDeleteUserDataConfig { firestoreDatabaseId: string; firestoreDeleteMode: FirestoreDeleteMode; rtdbInstance?: string; - rtdbLocation: string; + rtdbLocation?: string; rtdbPaths?: string; storageBucket?: string; storagePaths?: string; @@ -58,7 +58,6 @@ const DEFAULT_FIRESTORE_DATABASE_ID = "(default)"; const DEFAULT_FIRESTORE_DELETE_MODE: FirestoreDeleteMode = "shallow"; const DEFAULT_SEARCH_DEPTH = 3; const DEFAULT_SEARCH_FIELDS = "id,uid,userId"; -const DEFAULT_RTDB_LOCATION = "us-central1"; function topicName( name: string | undefined, instanceId: string, @@ -78,7 +77,7 @@ export function resolveDeleteUserDataConfig( firestoreDeleteMode: config.firestoreDeleteMode ?? DEFAULT_FIRESTORE_DELETE_MODE, rtdbInstance: config.rtdbInstance, - rtdbLocation: config.rtdbLocation ?? DEFAULT_RTDB_LOCATION, + rtdbLocation: config.rtdbLocation, rtdbPaths: config.rtdbPaths, storageBucket: config.storageBucket, storagePaths: config.storagePaths, diff --git a/kits/delete-user-data/tests/export-config.test.ts b/kits/delete-user-data/tests/export-config.test.ts index 1d6147ff5..ca3f6248c 100644 --- a/kits/delete-user-data/tests/export-config.test.ts +++ b/kits/delete-user-data/tests/export-config.test.ts @@ -57,12 +57,19 @@ describe("resolveDeleteUserDataConfig", () => { expect(config.firestoreDatabaseId).toBe("(default)"); expect(config.firestoreDeleteMode).toBe("shallow"); - expect(config.rtdbLocation).toBe("us-central1"); expect(config.enableAutoDiscovery).toBe(false); expect(config.searchDepth).toBe(3); expect(config.searchFields).toBe("id,uid,userId"); }); + // The extension skips RTDB paths when the location is blank, rather than + // assuming us-central1, so a blank location must stay unset. + test("leaves a blank RTDB location unset", () => { + const config = resolveDeleteUserDataConfig({ instanceId: "my-instance" }); + + expect(config.rtdbLocation).toBeUndefined(); + }); + test("derives topic names from the instance id", () => { const config = resolveDeleteUserDataConfig({ instanceId: "my-instance" }); From f5c0a759b2f8a10ff52721c900cc03312f44db35 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 27 Oct 2026 15:57:11 +0000 Subject: [PATCH 6/8] fix(delete-user-data): report blank and invalid params in one error assertRequiredParams() threw before assertValidParams() ran, so a .env with a blank required param and an invalid value needed two deploys to see both. assertParams() runs both checks and throws once with both messages. --- kits/delete-user-data/README.md | 3 ++- kits/delete-user-data/src/config.ts | 21 +++++++++++++++++++++ kits/delete-user-data/src/index.ts | 10 ++-------- kits/delete-user-data/tests/index.test.ts | 11 +++++++++++ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/kits/delete-user-data/README.md b/kits/delete-user-data/README.md index d1cb1fc6c..d59b2bfda 100644 --- a/kits/delete-user-data/README.md +++ b/kits/delete-user-data/README.md @@ -103,7 +103,8 @@ deploy fails with an error naming the setting if `FIRESTORE_DELETE_MODE`, values listed below (the error lists the allowed values), or if `SELECTED_DATABASE_INSTANCE` or `CLOUD_STORAGE_BUCKET` is not in the format the extension accepted: an instance id rather than a URL, and a bucket name without -`gs://` or capital letters (the error gives the extension's message). The values +`gs://` or capital letters (the error gives the extension's message). One error +lists every bad setting, along with any required setting left blank. The values are case sensitive. The emulator runs the same check, so a bad value stops that instance's functions from loading there. diff --git a/kits/delete-user-data/src/config.ts b/kits/delete-user-data/src/config.ts index 3d80e88fb..0484c6978 100644 --- a/kits/delete-user-data/src/config.ts +++ b/kits/delete-user-data/src/config.ts @@ -313,3 +313,24 @@ export function assertValidParams(): void { ); } } + +/** + * Runs `assertRequiredParams` and `assertValidParams` and throws once with + * both messages, so a `.env` with a blank required param and an invalid value + * reports both in one deploy rather than one per redeploy. + */ +export function assertParams(): void { + const errors: string[] = []; + + for (const check of [assertRequiredParams, assertValidParams]) { + try { + check(); + } catch (err) { + errors.push((err as Error).message); + } + } + + if (errors.length > 0) { + throw new Error(errors.join("\n")); + } +} diff --git a/kits/delete-user-data/src/index.ts b/kits/delete-user-data/src/index.ts index 4ee6caf84..ea5d367c7 100644 --- a/kits/delete-user-data/src/index.ts +++ b/kits/delete-user-data/src/index.ts @@ -21,12 +21,7 @@ import type { Role } from "firebase-functions/v2"; import { requiresAPI, requiresRole } from "firebase-functions/v2"; import { onUserDeleted } from "firebase-functions/v2/identity"; import { onMessagePublished } from "firebase-functions/v2/pubsub"; -import { - assertRequiredParams, - assertValidParams, - CONFIG_EXPRESSIONS, - configFromEnv, -} from "./config"; +import { assertParams, CONFIG_EXPRESSIONS, configFromEnv } from "./config"; import * as events from "./events"; import { getDatabaseUrl, resolveDeleteUserDataConfig } from "./export-config"; import { @@ -37,8 +32,7 @@ import { } from "./handlers"; import * as logs from "./logs"; -assertRequiredParams(); -assertValidParams(); +assertParams(); export * from "./lib"; const REQUIRED_ROLES: ReadonlyArray = [ diff --git a/kits/delete-user-data/tests/index.test.ts b/kits/delete-user-data/tests/index.test.ts index 36b50455c..749f6f1fd 100644 --- a/kits/delete-user-data/tests/index.test.ts +++ b/kits/delete-user-data/tests/index.test.ts @@ -83,4 +83,15 @@ describe("module load", () => { 'FIRESTORE_DELETE_MODE="RECURSIVE"' ); }); + + test("reports blank required and invalid params in one error", async () => { + vi.resetModules(); + vi.stubEnv("CLOUD_STORAGE_BUCKET", ""); + vi.stubEnv("FIRESTORE_DELETE_MODE", "RECURSIVE"); + + const load = import("../src/index"); + + await expect(load).rejects.toThrow("CLOUD_STORAGE_BUCKET"); + await expect(load).rejects.toThrow('FIRESTORE_DELETE_MODE="RECURSIVE"'); + }); }); From 32a478fee11a8e452cc67f5069a1ef4b7265af0b Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 27 Oct 2026 16:15:07 +0000 Subject: [PATCH 7/8] fix(delete-user-data): accept option labels as the extension did The extension's installer accepted a select param's label as well as its value and ran it as-is, so Recursive deleted shallowly and Yes left auto-discovery off. The kit now accepts every label too and runs it the same way, rather than failing the deploy. The README warns what each label does. --- kits/delete-user-data/CHANGELOG.md | 2 +- kits/delete-user-data/README.md | 18 +++++++---------- kits/delete-user-data/src/config.ts | 20 ++++++++----------- .../tests/valid-params.test.ts | 20 +++++++------------ 4 files changed, 23 insertions(+), 37 deletions(-) diff --git a/kits/delete-user-data/CHANGELOG.md b/kits/delete-user-data/CHANGELOG.md index e074fb0e0..e804a1cd4 100644 --- a/kits/delete-user-data/CHANGELOG.md +++ b/kits/delete-user-data/CHANGELOG.md @@ -1,4 +1,4 @@ -- fix: invalid values in `.env` now fail the deploy instead of deploying and quietly doing less. The Firebase CLI only checks values it prompts for, so `FIRESTORE_DELETE_MODE=RECURSIVE` deployed and ran as `shallow` (leaving subcollections behind), and `ENABLE_AUTO_DISCOVERY=true` deployed with auto-discovery off. The kit now rejects `FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` and `SELECTED_DATABASE_LOCATION` values that are not one of their options, and `SELECTED_DATABASE_INSTANCE` and `CLOUD_STORAGE_BUCKET` values that fail the extension's format checks, as the extension did at install time. The option labels the extension misread (`Recursive`, `Yes` and the location labels) are rejected too. +- fix: invalid values in `.env` now fail the deploy instead of deploying and quietly doing less. The Firebase CLI only checks values it prompts for, so `FIRESTORE_DELETE_MODE=RECURSIVE` deployed and ran as `shallow` (leaving subcollections behind), and `ENABLE_AUTO_DISCOVERY=true` deployed with auto-discovery off. The kit now rejects `FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` and `SELECTED_DATABASE_LOCATION` values that are not one of their options, and `SELECTED_DATABASE_INSTANCE` and `CLOUD_STORAGE_BUCKET` values that fail the extension's format checks, as the extension did at install time. Option labels such as `Recursive` are still accepted, as the extension accepted them, and run as they did in the extension. - fix: deploy `clearData`, `handleSearch` and `handleDeletion` with the extension's runtime settings: `concurrency: 1`, `cpu: "gcf_gen1"` (0.1666 vCPU) and `ingressSettings: "ALLOW_INTERNAL_ONLY"`. `setGlobalOptions` can't change these. Before, the kit took the 2nd gen defaults of concurrency `80`, 1 vCPU and `ALLOW_ALL`. With auto-discovery on, one 256MiB `handleSearch` instance took many searches at once, ran out of memory, and dropped every search in flight on it without an error, so user data was left undeleted. Existing kit deployments switch on their next deploy. diff --git a/kits/delete-user-data/README.md b/kits/delete-user-data/README.md index 1a27a81e3..dc366228a 100644 --- a/kits/delete-user-data/README.md +++ b/kits/delete-user-data/README.md @@ -100,7 +100,7 @@ The CLI does not check values that are already in `.env`, so the kit checks them itself when the deploy loads it, as the extension did at install time. The deploy fails with an error naming the setting if `FIRESTORE_DELETE_MODE`, `ENABLE_AUTO_DISCOVERY` or `SELECTED_DATABASE_LOCATION` is not one of the -values listed below (the error lists the allowed values), or if +values listed below or their labels (the error lists the allowed values), or if `SELECTED_DATABASE_INSTANCE` or `CLOUD_STORAGE_BUCKET` is not in the format the extension accepted: an instance id rather than a URL, and a bucket name without `gs://` or capital letters (the error gives the extension's message). One error @@ -108,6 +108,12 @@ lists every bad setting, along with any required setting left blank. The values are case sensitive. The emulator runs the same check, so a bad value stops that instance's functions from loading there. +An option's label (such as `Recursive`) is accepted because the extension's +installer accepted it, but it runs as it did in the extension: `Recursive` +deletes shallowly, `Yes` leaves auto-discovery off, and a location label +(`United States`, `Belgium`, `Singapore`) gives a database URL that doesn't +exist. Use the values instead. + The instance id is not a setting: the CLI provides it to each instance as `FIREBASE_KIT_INSTANCE_ID`, set to that instance's key in the `instances` map. `FIREBASE_` is a reserved prefix in `.env` files, so it cannot be set or @@ -241,16 +247,6 @@ The Firebase CLI now sets it to the function that published the event so it differs between `clearData`, `handleSearch` and `handleDeletion`. If your event triggers filter on `source`, filter on the event `type` instead. -### Some option labels are not accepted - -The extension's installer accepted an option's label as well as its value, but -then misread `Recursive` (it deleted shallowly), `Yes` (it left auto-discovery -off) and the location labels (`United States`, `Belgium`, `Singapore`). The kit -fails the deploy on these labels. If you copied your `.env` from an extension -instance that was installed with one, change it to the value (`recursive`, -`yes`, or the location id). `Shallow` and `No` worked in the extension and are -still accepted. - ### Unchanged Event types and payloads are the same. The functions still publish diff --git a/kits/delete-user-data/src/config.ts b/kits/delete-user-data/src/config.ts index 0484c6978..64bf2d8ee 100644 --- a/kits/delete-user-data/src/config.ts +++ b/kits/delete-user-data/src/config.ts @@ -259,22 +259,16 @@ export function assertRequiredParams( } } -// The extension stored these labels as-is and ran them like their value. -const SAFE_LABELS: Readonly> = { - FIRESTORE_DELETE_MODE: ["Shallow"], - ENABLE_AUTO_DISCOVERY: ["No"], -}; - /** * Rejects param values the published extension refuses at install time: a - * select value that is not one of its option values, or a text value that - * fails its `validationRegex`. + * select value that is not one of its option values or labels, or a text value + * that fails its `validationRegex`. * * The CLI applies a param's `input` only when it prompts, so a value already * in `.env` reaches the handlers unchecked, and they compare it with `===` * (`FIRESTORE_DELETE_MODE=RECURSIVE` runs as shallow, for example). Option - * labels such as `Recursive` are rejected too, as the handlers only understand - * the values, except those in `SAFE_LABELS`. Unset and empty values are + * labels such as `Recursive` are accepted, as the extension's installer + * accepted them, and run as the extension ran them. Unset and empty values are * skipped: defaults cover the first, and `assertRequiredParams` decides about * the second. Called at module scope so deploy-time discovery fails before the * function ships. @@ -291,8 +285,10 @@ export function assertValidParams(): void { const setting = `${param.name}=${JSON.stringify(raw)}`; if ("select" in input) { - const allowed = input.select.options.map(({ value }) => String(value)); - if (!allowed.includes(raw) && !SAFE_LABELS[param.name]?.includes(raw)) { + const { options } = input.select; + const allowed = options.map(({ value }) => String(value)); + const labels = options.map(({ label }) => label); + if (!allowed.includes(raw) && !labels.includes(raw)) { invalid.push(`${setting} must be one of: ${allowed.join(", ")}`); } } else if ("text" in input && input.text.validationRegex) { diff --git a/kits/delete-user-data/tests/valid-params.test.ts b/kits/delete-user-data/tests/valid-params.test.ts index 8a716cab2..abf0f7948 100644 --- a/kits/delete-user-data/tests/valid-params.test.ts +++ b/kits/delete-user-data/tests/valid-params.test.ts @@ -96,23 +96,17 @@ describe("assertValidParams", () => { expect(() => assertValidParams()).not.toThrow(); }); - // The extension accepts a select label, stores it verbatim, and its runtime - // then treats `Recursive` as shallow and `Yes` as off. + // The extension's installer accepts a select label and stores it verbatim, + // so the kit accepts it too, even though `Recursive` then runs as shallow + // and `Yes` as off, as they did in the extension. test.each([ ["FIRESTORE_DELETE_MODE", "Recursive"], - ["ENABLE_AUTO_DISCOVERY", "Yes"], - ["SELECTED_DATABASE_LOCATION", "Belgium"], - ])("rejects the option label %s=%j", (name, value) => { - vi.stubEnv(name, value); - - expect(() => assertValidParams()).toThrow(name); - }); - - // The runtime only checks for `recursive` and `yes`, so these labels already - // ran like their values. - test.each([ ["FIRESTORE_DELETE_MODE", "Shallow"], + ["ENABLE_AUTO_DISCOVERY", "Yes"], ["ENABLE_AUTO_DISCOVERY", "No"], + ["SELECTED_DATABASE_LOCATION", "United States"], + ["SELECTED_DATABASE_LOCATION", "Belgium"], + ["SELECTED_DATABASE_LOCATION", "Singapore"], ])("accepts the option label %s=%j", (name, value) => { vi.stubEnv(name, value); From 78ccd898cd7754a32207de9ca4b0a4f892de8ae3 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 27 Oct 2026 16:19:35 +0000 Subject: [PATCH 8/8] fix(delete-user-data): fix the select label type error Destructuring the optional label failed to type-check against the union of select option arrays, which broke the kit build. --- kits/delete-user-data/src/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kits/delete-user-data/src/config.ts b/kits/delete-user-data/src/config.ts index 64bf2d8ee..954a7b9fc 100644 --- a/kits/delete-user-data/src/config.ts +++ b/kits/delete-user-data/src/config.ts @@ -287,7 +287,7 @@ export function assertValidParams(): void { if ("select" in input) { const { options } = input.select; const allowed = options.map(({ value }) => String(value)); - const labels = options.map(({ label }) => label); + const labels = options.map((option) => option.label); if (!allowed.includes(raw) && !labels.includes(raw)) { invalid.push(`${setting} must be one of: ${allowed.join(", ")}`); }