diff --git a/kits/firestore-send-email/CHANGELOG.md b/kits/firestore-send-email/CHANGELOG.md index 3fa1f9c9d..39f04be57 100644 --- a/kits/firestore-send-email/CHANGELOG.md +++ b/kits/firestore-send-email/CHANGELOG.md @@ -1,3 +1,5 @@ +- fix: invalid values in `.env` now fail the deploy instead of leaving mail unprocessed or quietly changing behaviour. The Firebase CLI only checks values it prompts for, so `TTL_EXPIRE_TYPE=days` deployed and left every document without a `delivery` field, `TTL_EXPIRE_VALUE=-1` set an `expireAt` in the past, and `AUTH_TYPE=oauth2` sent through the SMTP URI. The kit now rejects `DATABASE_REGION`, `AUTH_TYPE`, `OAUTH_SECURE` and `TTL_EXPIRE_TYPE` values that are not one of their options, and `SMTP_CONNECTION_URI`, `MAIL_COLLECTION`, `DEFAULT_FROM` and `TTL_EXPIRE_VALUE` values that fail the extension's format checks, as the extension did at install time. `DATABASE_REGION` is now case sensitive, as it was in the extension. Option labels such as `Day` are still accepted, as the extension accepted them, and run as they did in the extension. An empty `OAUTH_SECURE` now means `true`, as in the extension. + - fix: deploy `processQueue` with the extension's runtime settings: `concurrency: 1`, `cpu: "gcf_gen1"` (0.1666 vCPU at 256MiB) 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`. A burst of mail with attachments landed on one 256MiB instance, which ran out of memory and dropped every event in flight on it: the documents were left in `PENDING` or `PROCESSING`, or with no `delivery` field, and were never retried or marked as failed, so the mail was never sent. 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 before the work the function exists to do, so the mail was never sent. 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. diff --git a/kits/firestore-send-email/README.md b/kits/firestore-send-email/README.md index b54c65da9..040a6c228 100644 --- a/kits/firestore-send-email/README.md +++ b/kits/firestore-send-email/README.md @@ -112,6 +112,35 @@ the CLI connects them to the function at deploy time. | `ttlExpireValue` | `TTL_EXPIRE_VALUE` | no | `1` | TTL amount when expire type is set | | `tlsOptions` | `TLS_OPTIONS` | no | `{}` | JSON TLS options for the SMTP transport | +Single-quote JSON values such as `TLS_OPTIONS` in `.env`. In a double-quoted +value the CLI turns `\n` into a real newline, so a PEM certificate no longer +parses as JSON, and the kit falls back to `{"rejectUnauthorized":false}` +(certificate checks off) with only a warning: + +``` +TLS_OPTIONS='{"ca":"-----BEGIN CERTIFICATE-----\n...\n-----END CERTIFICATE-----\n"}' +``` + +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 `DATABASE_REGION`, +`AUTH_TYPE`, `OAUTH_SECURE` or `TTL_EXPIRE_TYPE` is not one of its option values +or their labels (the error lists the allowed values), or if +`SMTP_CONNECTION_URI`, `MAIL_COLLECTION`, `DEFAULT_FROM` or `TTL_EXPIRE_VALUE` +is not in the format the extension accepted (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 the +function from loading there. An empty `OAUTH_SECURE` means `true`, as it did in +the extension. + +An option's label (such as `Day`) is accepted because the extension's +installer accepted it, but it runs as it did in the extension: any +`TTL_EXPIRE_TYPE` label, `Never` included, leaves every document without a +`delivery` field, `OAUTH_SECURE=Yes` connects without TLS, and a +`DATABASE_REGION` label is not a region the function can deploy to. Use the +values instead. + ## Multiple instances To process several mail queues, add one entry per instance to the `instances` @@ -177,8 +206,8 @@ location setting. The kit deploys the function to the region derived from Firestore locations (`europe-west2`, `us-east1`, ...) are used as-is; the multi-region locations map to a Cloud Run region inside them - `nam5` and `nam7` to `us-central1`, `eur3` to `europe-west1` - because they are not Cloud -Run regions themselves and would fail the deploy. The value is matched -case-insensitively. The Firestore trigger always fires in the database's own +Run regions themselves and would fail the deploy. The value is case sensitive, +as in the extension, so `NAM5` fails the deploy. The Firestore trigger always fires in the database's own region, whatever region the function runs in. If you copied `DATABASE_REGION` into your `.env` from an extension install, it @@ -232,18 +261,11 @@ with `EXT_`, so `EVENTARC_CHANNEL` unset, nothing is published and the function is otherwise unaffected. -### Settings in `.env` are not validated at deploy time - -The extension rejected a malformed `DEFAULT_FROM`, a `MAIL_COLLECTION` that was -not a valid collection path, an `SMTP_CONNECTION_URI` that was not -`smtp(s)://...:port`, and a `TTL_EXPIRE_VALUE` that was not a positive integer, -before it would install. The kit declares the same checks, but they apply only to -a value entered at the `firebase deploy` prompt, not to one read from `.env`. A -bad from address or connection URI in `.env` deploys cleanly and every document -fails at send time with `delivery.state: ERROR` instead. `TTL_EXPIRE_VALUE: 0` is -silently treated as `1`, and a negative value produces a `delivery.expireAt` in -the past, which a TTL policy will act on immediately. Only a required setting -left empty in `.env`, such as `DEFAULT_FROM=`, fails the deploy. +### An empty required setting fails the deploy + +The kit checks `.env` values against the extension's install-time rules (see +[Configuration](#configuration)), with one difference: a required setting left +empty, such as `DEFAULT_FROM=`, fails the deploy instead of taking its default. ### The setup steps and the OAuth2 helper are not in the README diff --git a/kits/firestore-send-email/src/config.ts b/kits/firestore-send-email/src/config.ts index 97d4afbd0..bb968515b 100644 --- a/kits/firestore-send-email/src/config.ts +++ b/kits/firestore-send-email/src/config.ts @@ -362,7 +362,10 @@ export function configFromEnv(): SendEmailConfig { tlsOptions: params.tlsOptions.value(), host: params.host.value(), oauthPort: params.oauthPort.value(), - oauthSecure: params.oauthSecure.value(), + // The extension platform swapped a blank optional value for its default + // (`true`), but the boolean param reads "" as false. + oauthSecure: + process.env.OAUTH_SECURE === "" ? undefined : params.oauthSecure.value(), user: params.user.value(), clientId: authType === AuthenticatonType.OAuth2 @@ -433,3 +436,78 @@ 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 coerce it + * (`TTL_EXPIRE_TYPE=days` leaves every document without a `delivery` field, + * and `TTL_EXPIRE_VALUE=-1` sets an `expireAt` in the past, for example). + * Option labels such as `Day` 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]; + // Secrets declare no `input`. + const declared = param.options; + const input = declared && "input" in declared ? declared.input : undefined; + 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/firestore-send-email/src/index.ts b/kits/firestore-send-email/src/index.ts index a0319bab6..96f0f0b3a 100644 --- a/kits/firestore-send-email/src/index.ts +++ b/kits/firestore-send-email/src/index.ts @@ -20,7 +20,7 @@ import { onDocumentWritten } from "firebase-functions/firestore"; import type { Role } from "firebase-functions/v2"; import { requiresAPI, requiresRole } from "firebase-functions/v2"; import { - assertRequiredParams, + assertParams, configFromEnv, envDeployOptions, secretParams, @@ -32,7 +32,7 @@ import { transportLayer } from "./helpers"; import * as logs from "./logs"; import { Templates } from "./templates"; -assertRequiredParams(); +assertParams(); export * from "./lib"; const REQUIRED_ROLES: ReadonlyArray = [ diff --git a/kits/firestore-send-email/tests/config-runtime.test.ts b/kits/firestore-send-email/tests/config-runtime.test.ts index d1a5180a4..a80e2daef 100644 --- a/kits/firestore-send-email/tests/config-runtime.test.ts +++ b/kits/firestore-send-email/tests/config-runtime.test.ts @@ -29,6 +29,7 @@ import { declaredParams } from "firebase-functions/params"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { configFromEnv } from "../src/config"; +import { resolveConfig } from "../src/export-config"; function declaration(name: string) { const param = declaredParams.find((candidate) => candidate.name === name); @@ -85,4 +86,10 @@ describe("OAUTH_SECURE values inherited from the extension", () => { test("treats an unset variable as insecure, as the extension did", () => { expect(configFromEnv().oauthSecure).toBe(false); }); + + // The extension platform replaced a blank optional value with its default. + test("treats a blank variable as secure, as the extension did", () => { + process.env.OAUTH_SECURE = ""; + expect(resolveConfig(configFromEnv()).oauthSecure).toBe(true); + }); }); diff --git a/kits/firestore-send-email/tests/index.test.ts b/kits/firestore-send-email/tests/index.test.ts index 16b597241..9f8879e92 100644 --- a/kits/firestore-send-email/tests/index.test.ts +++ b/kits/firestore-send-email/tests/index.test.ts @@ -59,7 +59,6 @@ describe("processQueue options", () => { ["nam5", "us-central1"], ["nam7", "us-central1"], ["eur3", "europe-west1"], - ["NAM5", "us-central1"], ])( "multi-region DATABASE_REGION %s deploys the function to %s", async (databaseRegion, expectedRegion) => { @@ -99,3 +98,41 @@ describe("processQueue options", () => { expect(options.ingressSettings).toBe("ALLOW_INTERNAL_ONLY"); }); }); + +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("TTL_EXPIRE_TYPE", "days"); + + await expect(import("../src/index")).rejects.toThrow( + 'TTL_EXPIRE_TYPE="days"' + ); + }); + + // The extension's select is case sensitive, so a DATABASE_REGION it rejected + // no longer reaches the case-insensitive region lookup. + test("throws on a DATABASE_REGION in the wrong case", async () => { + vi.resetModules(); + vi.stubEnv("DATABASE_REGION", "NAM5"); + + await expect(import("../src/index")).rejects.toThrow( + 'DATABASE_REGION="NAM5"' + ); + }); + + test("reports blank required and invalid params in one error", async () => { + vi.resetModules(); + vi.stubEnv("DEFAULT_FROM", ""); + vi.stubEnv("TTL_EXPIRE_VALUE", "0"); + + const load = import("../src/index"); + + await expect(load).rejects.toThrow("DEFAULT_FROM"); + await expect(load).rejects.toThrow('TTL_EXPIRE_VALUE="0"'); + }); +}); diff --git a/kits/firestore-send-email/tests/valid-params.test.ts b/kits/firestore-send-email/tests/valid-params.test.ts new file mode 100644 index 000000000..7618ce1d9 --- /dev/null +++ b/kits/firestore-send-email/tests/valid-params.test.ts @@ -0,0 +1,175 @@ +/** + * 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, + beforeEach, + describe, + expect, + test, + vi, +} from "vitest"; + +let assertValidParams: () => void; + +beforeAll(async () => { + process.env.FIREBASE_KIT_INSTANCE_ID ??= "test-instance"; + ({ assertValidParams } = await import("../src/config")); +}); + +// AUTH_TYPE is a real param name that can collide with an ambient shell var, +// so start every test from a known environment. +beforeEach(() => { + for (const name of ["AUTH_TYPE", "SMTP_CONNECTION_URI"]) { + vi.stubEnv(name, undefined); + } +}); + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +const TTL_TYPES = "never, hour, day, week, month, year"; +const TTL_VALUE = "The value must be an integer value greater than zero."; +const SMTP_URI = "Invalid SMTP connection URI"; +const COLLECTION = "Must be a valid Cloud Firestore collection"; +const FROM = "Must be a valid email address"; + +// Values the published extension's install API rejects (`days`, `0`, `abc` +// and `oauth2` checked with `validateOnly` against +// firebase/firestore-send-email@0.2.10; the rest fail the same select options +// or validationRegex in its extension.yaml). +const REJECTED: Array<[string, string, string]> = [ + ["TTL_EXPIRE_TYPE", "days", TTL_TYPES], + ["TTL_EXPIRE_TYPE", "DAY", TTL_TYPES], + ["TTL_EXPIRE_TYPE", "day ", TTL_TYPES], + ["TTL_EXPIRE_VALUE", "0", TTL_VALUE], + ["TTL_EXPIRE_VALUE", "abc", TTL_VALUE], + ["TTL_EXPIRE_VALUE", "1.5", TTL_VALUE], + ["TTL_EXPIRE_VALUE", "-1", TTL_VALUE], + ["TTL_EXPIRE_VALUE", "01", TTL_VALUE], + ["TTL_EXPIRE_VALUE", " ", TTL_VALUE], + ["AUTH_TYPE", "oauth2", "UsernamePassword, OAuth2"], + ["AUTH_TYPE", "ApiKey", "UsernamePassword, OAuth2"], + ["OAUTH_SECURE", "yes", "true, false"], + ["OAUTH_SECURE", "TRUE", "true, false"], + ["OAUTH_SECURE", "1", "true, false"], + ["DATABASE_REGION", "NAM5", "eur3, nam5, nam7"], + ["DATABASE_REGION", "us-central2", "eur3, nam5, nam7"], + ["SMTP_CONNECTION_URI", "smtp.gmail.com:465", SMTP_URI], + ["SMTP_CONNECTION_URI", "smtps://smtp.gmail.com", SMTP_URI], + ["SMTP_CONNECTION_URI", " ", SMTP_URI], + ["MAIL_COLLECTION", "mail/doc", COLLECTION], + ["MAIL_COLLECTION", "/mail", COLLECTION], + ["DEFAULT_FROM", "not-an-email", FROM], + ["DEFAULT_FROM", "Name ", FROM], +]; + +const ACCEPTED: Array<[string, string]> = [ + ["TTL_EXPIRE_TYPE", "never"], + ["TTL_EXPIRE_TYPE", "hour"], + ["TTL_EXPIRE_TYPE", "day"], + ["TTL_EXPIRE_TYPE", "week"], + ["TTL_EXPIRE_TYPE", "month"], + ["TTL_EXPIRE_TYPE", "year"], + ["TTL_EXPIRE_VALUE", "1"], + ["TTL_EXPIRE_VALUE", "30"], + ["AUTH_TYPE", "UsernamePassword"], + ["AUTH_TYPE", "OAuth2"], + ["OAUTH_SECURE", "true"], + ["OAUTH_SECURE", "false"], + ["DATABASE_REGION", "nam5"], + ["DATABASE_REGION", "europe-west2"], + ["SMTP_CONNECTION_URI", "smtps://username@smtp.hostname.com:465"], + ["SMTP_CONNECTION_URI", "smtp://user:pass@smtp.hostname.com:587?pool=true"], + ["MAIL_COLLECTION", "mail"], + ["MAIL_COLLECTION", "users/uid/mail"], + ["DEFAULT_FROM", "foobar@example.com"], + ["DEFAULT_FROM", "Friendly Firebaser "], + // No select or validationRegex in the extension, so anything goes. + ["DATABASE", "my-db"], + ["TLS_OPTIONS", "not json"], +]; + +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 it then runs as it did in the + // extension: `Day` leaves every document unprocessed, `Yes` connects + // without TLS, and a location label is not a region. + test.each([ + ["TTL_EXPIRE_TYPE", "Never"], + ["TTL_EXPIRE_TYPE", "Hour"], + ["TTL_EXPIRE_TYPE", "Day"], + ["TTL_EXPIRE_TYPE", "Week"], + ["TTL_EXPIRE_TYPE", "Month"], + ["TTL_EXPIRE_TYPE", "Year"], + ["AUTH_TYPE", "Username & Password"], + ["OAUTH_SECURE", "Yes"], + ["OAUTH_SECURE", "No"], + ["DATABASE_REGION", "Multi-region (United States)"], + ["DATABASE_REGION", "Iowa (us-central1)"], + ])("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([ + "DATABASE", + "DATABASE_REGION", + "AUTH_TYPE", + "OAUTH_SECURE", + "MAIL_COLLECTION", + "DEFAULT_FROM", + "TTL_EXPIRE_TYPE", + "TTL_EXPIRE_VALUE", + ])("leaves an empty %s alone", (name) => { + vi.stubEnv(name, ""); + + expect(() => assertValidParams()).not.toThrow(); + }); + + test("names every invalid param in one error", () => { + vi.stubEnv("TTL_EXPIRE_TYPE", "days"); + vi.stubEnv("TTL_EXPIRE_VALUE", "0"); + + expect(() => assertValidParams()).toThrow( + /TTL_EXPIRE_TYPE="days".*; TTL_EXPIRE_VALUE="0"/ + ); + }); +});