diff --git a/kits/delete-user-data/CHANGELOG.md b/kits/delete-user-data/CHANGELOG.md index a69845f22..e804a1cd4 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` 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. - 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 54d7d5247..dc366228a 100644 --- a/kits/delete-user-data/README.md +++ b/kits/delete-user-data/README.md @@ -96,6 +96,24 @@ 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 if `FIRESTORE_DELETE_MODE`, +`ENABLE_AUTO_DISCOVERY` or `SELECTED_DATABASE_LOCATION` is not one of the +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 +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 diff --git a/kits/delete-user-data/src/config.ts b/kits/delete-user-data/src/config.ts index e172ae79e..954a7b9fc 100644 --- a/kits/delete-user-data/src/config.ts +++ b/kits/delete-user-data/src/config.ts @@ -258,3 +258,75 @@ 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 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 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. + */ +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 { options } = input.select; + const allowed = options.map(({ value }) => String(value)); + const labels = options.map((option) => option.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) { + const regex = new RegExp(input.text.validationRegex); + if (!regex.test(raw)) { + const reason = + input.text.validationErrorMessage ?? `must match ${regex}`; + invalid.push(`${setting}: ${reason}`); + } + } + } + + if (invalid.length > 0) { + throw new Error( + `Invalid parameter values: ${invalid.join("; ")}. ` + + "Fix them in this instance's .env or .env. file, or remove " + + "the entries to use their defaults." + ); + } +} + +/** + * 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 2d981b536..150951bd8 100644 --- a/kits/delete-user-data/src/index.ts +++ b/kits/delete-user-data/src/index.ts @@ -21,11 +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, - CONFIG_EXPRESSIONS, - configFromEnv, -} from "./config"; +import { assertParams, CONFIG_EXPRESSIONS, configFromEnv } from "./config"; import * as events from "./events"; import { getDatabaseUrl, resolveDeleteUserDataConfig } from "./export-config"; import { @@ -36,7 +32,7 @@ import { } from "./handlers"; import * as logs from "./logs"; -assertRequiredParams(); +assertParams(); export * from "./lib"; const REQUIRED_ROLES: ReadonlyArray = [ diff --git a/kits/delete-user-data/tests/export-config.test.ts b/kits/delete-user-data/tests/export-config.test.ts index 549d31d0f..ca3f6248c 100644 --- a/kits/delete-user-data/tests/export-config.test.ts +++ b/kits/delete-user-data/tests/export-config.test.ts @@ -62,6 +62,14 @@ describe("resolveDeleteUserDataConfig", () => { 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" }); diff --git a/kits/delete-user-data/tests/index.test.ts b/kits/delete-user-data/tests/index.test.ts index c8709200e..95e9c833d 100644 --- a/kits/delete-user-data/tests/index.test.ts +++ b/kits/delete-user-data/tests/index.test.ts @@ -70,6 +70,33 @@ describe("clearData", () => { }); }); +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"' + ); + }); + + 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"'); + }); +}); + describe("runtime options", () => { // Concurrency 80 let auto-discovery fan many searches onto one 256MiB // instance, which ran out of memory and dropped them. 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..abf0f7948 --- /dev/null +++ b/kits/delete-user-data/tests/valid-params.test.ts @@ -0,0 +1,138 @@ +/** + * 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"], + ["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]> = [ + ["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"], + ["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"], +]; + +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'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"], + ["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); + + expect(() => assertValidParams()).not.toThrow(); + }); + + // 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"/ + ); + }); +});