From 267f6aa06860b5430a84b3663da3caf94dd15119 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 10 Nov 2026 14:32:10 +0000 Subject: [PATCH 1/2] fix(firestore-send-email): reject invalid select and regex param values at deploy Fixes #3295. --- kits/firestore-send-email/CHANGELOG.md | 2 + kits/firestore-send-email/README.md | 53 +++-- kits/firestore-send-email/src/config.ts | 97 +++++++- kits/firestore-send-email/src/index.ts | 4 +- .../tests/config-runtime.test.ts | 7 + kits/firestore-send-email/tests/index.test.ts | 39 +++- .../tests/valid-params.test.ts | 214 ++++++++++++++++++ 7 files changed, 398 insertions(+), 18 deletions(-) create mode 100644 kits/firestore-send-email/tests/valid-params.test.ts diff --git a/kits/firestore-send-email/CHANGELOG.md b/kits/firestore-send-email/CHANGELOG.md index 27cdd5bb5..a36b440ac 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 `SMTP_CONNECTION_URI` also fails the deploy unless `AUTH_TYPE` is `OAuth2`, and an empty `OAUTH_SECURE` now means `true`, as in the extension. + - 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. - docs: the no-region fallback needs an explicit empty `DATABASE_REGION=` line in `.env`. Omitting the key entirely fails a non-interactive deploy, because the CLI resolves parameters before it reads their defaults. Corrects the note on the `DATABASE_REGION` mapping fix below; no behavior change. diff --git a/kits/firestore-send-email/README.md b/kits/firestore-send-email/README.md index 3bc671501..7343b5311 100644 --- a/kits/firestore-send-email/README.md +++ b/kits/firestore-send-email/README.md @@ -112,6 +112,36 @@ 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). It also fails if `SMTP_CONNECTION_URI` is set to an empty value and +`AUTH_TYPE` is not `OAuth2`, as no mail can be sent that way. 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 +207,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 +262,13 @@ 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 `SMTP_CONNECTION_URI` fails the deploy + +The kit checks `.env` values against the extension's install-time rules (see +[Configuration](#configuration)), with two differences. A required setting left +empty, such as `DEFAULT_FROM=`, fails the deploy instead of taking its default. +And `SMTP_CONNECTION_URI=` fails the deploy unless `AUTH_TYPE` is `OAuth2`. The +extension installed with it, but could never send mail with it. ### 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..cf8203734 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,95 @@ 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}`); + } + } + } + + // Not an install-time check in the extension, which accepted a blank URI but + // could never send mail with it. The handlers send through the URI under any + // auth type but OAuth2, so a blank one leaves every document without a + // `delivery` field. The TESTING transport ignores the URI, and an invalid + // AUTH_TYPE is reported above. + const authType = process.env.AUTH_TYPE || AuthenticatonType.UsernamePassword; + if ( + process.env.SMTP_CONNECTION_URI === "" && + authType !== AuthenticatonType.OAuth2 && + !invalid.some((message) => message.startsWith("AUTH_TYPE=")) && + process.env.TESTING !== "true" + ) { + invalid.push( + `SMTP_CONNECTION_URI="" must be set unless AUTH_TYPE is ${AuthenticatonType.OAuth2}` + ); + } + + 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 e16a5f5ca..963c24eb9 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 ee37425b5..48c415326 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) => { @@ -90,3 +89,41 @@ describe("processQueue options", () => { expect(document.toCEL()).toContain("params.MAIL_COLLECTION"); }); }); + +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..1b7d3e87a --- /dev/null +++ b/kits/firestore-send-email/tests/valid-params.test.ts @@ -0,0 +1,214 @@ +/** + * 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", "TESTING"]) { + 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"/ + ); + }); +}); + +// A blank URI under username/password auth leaves every document without a +// `delivery` field, so the kit rejects it even though the extension's +// installer allowed it. +describe("assertValidParams with a blank SMTP_CONNECTION_URI", () => { + test.each([undefined, "UsernamePassword", "Username & Password"])( + "rejects it when AUTH_TYPE is %j", + (authType) => { + vi.stubEnv("AUTH_TYPE", authType); + vi.stubEnv("SMTP_CONNECTION_URI", ""); + + expect(() => assertValidParams()).toThrow( + 'SMTP_CONNECTION_URI="" must be set unless AUTH_TYPE is OAuth2' + ); + } + ); + + test("accepts it under OAuth2, which does not use the URI", () => { + vi.stubEnv("AUTH_TYPE", "OAuth2"); + vi.stubEnv("SMTP_CONNECTION_URI", ""); + + expect(() => assertValidParams()).not.toThrow(); + }); + + test("accepts it in TESTING mode, whose transport ignores the URI", () => { + vi.stubEnv("TESTING", "true"); + vi.stubEnv("SMTP_CONNECTION_URI", ""); + + expect(() => assertValidParams()).not.toThrow(); + }); + + test("reports only AUTH_TYPE when that is invalid too", () => { + vi.stubEnv("AUTH_TYPE", "oauth2"); + vi.stubEnv("SMTP_CONNECTION_URI", ""); + + expect(() => assertValidParams()).toThrow('AUTH_TYPE="oauth2"'); + expect(() => assertValidParams()).not.toThrow(/SMTP_CONNECTION_URI/); + }); +}); From 5240861cdd5814028494aca8ec8484658c67551f Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 10 Nov 2026 14:34:50 +0000 Subject: [PATCH 2/2] fix(firestore-send-email): accept a blank SMTP_CONNECTION_URI, as the extension did --- kits/firestore-send-email/CHANGELOG.md | 2 +- kits/firestore-send-email/README.md | 9 ++-- kits/firestore-send-email/src/config.ts | 17 -------- .../tests/valid-params.test.ts | 41 +------------------ 4 files changed, 5 insertions(+), 64 deletions(-) diff --git a/kits/firestore-send-email/CHANGELOG.md b/kits/firestore-send-email/CHANGELOG.md index a36b440ac..2934ca18b 100644 --- a/kits/firestore-send-email/CHANGELOG.md +++ b/kits/firestore-send-email/CHANGELOG.md @@ -1,4 +1,4 @@ -- 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 `SMTP_CONNECTION_URI` also fails the deploy unless `AUTH_TYPE` is `OAuth2`, and an empty `OAUTH_SECURE` now means `true`, as in the extension. +- 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: 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 7343b5311..05b64b133 100644 --- a/kits/firestore-send-email/README.md +++ b/kits/firestore-send-email/README.md @@ -128,8 +128,7 @@ deploy fails with an error naming the setting if `DATABASE_REGION`, 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). It also fails if `SMTP_CONNECTION_URI` is set to an empty value and -`AUTH_TYPE` is not `OAuth2`, as no mail can be sent that way. One error lists +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 @@ -262,13 +261,11 @@ with `EXT_`, so `EVENTARC_CHANNEL` unset, nothing is published and the function is otherwise unaffected. -### An empty `SMTP_CONNECTION_URI` 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 two differences. A required setting left +[Configuration](#configuration)), with one difference: a required setting left empty, such as `DEFAULT_FROM=`, fails the deploy instead of taking its default. -And `SMTP_CONNECTION_URI=` fails the deploy unless `AUTH_TYPE` is `OAuth2`. The -extension installed with it, but could never send mail with it. ### 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 cf8203734..bb968515b 100644 --- a/kits/firestore-send-email/src/config.ts +++ b/kits/firestore-send-email/src/config.ts @@ -482,23 +482,6 @@ export function assertValidParams(): void { } } - // Not an install-time check in the extension, which accepted a blank URI but - // could never send mail with it. The handlers send through the URI under any - // auth type but OAuth2, so a blank one leaves every document without a - // `delivery` field. The TESTING transport ignores the URI, and an invalid - // AUTH_TYPE is reported above. - const authType = process.env.AUTH_TYPE || AuthenticatonType.UsernamePassword; - if ( - process.env.SMTP_CONNECTION_URI === "" && - authType !== AuthenticatonType.OAuth2 && - !invalid.some((message) => message.startsWith("AUTH_TYPE=")) && - process.env.TESTING !== "true" - ) { - invalid.push( - `SMTP_CONNECTION_URI="" must be set unless AUTH_TYPE is ${AuthenticatonType.OAuth2}` - ); - } - if (invalid.length > 0) { throw new Error( `Invalid parameter values: ${invalid.join("; ")}. ` + diff --git a/kits/firestore-send-email/tests/valid-params.test.ts b/kits/firestore-send-email/tests/valid-params.test.ts index 1b7d3e87a..7618ce1d9 100644 --- a/kits/firestore-send-email/tests/valid-params.test.ts +++ b/kits/firestore-send-email/tests/valid-params.test.ts @@ -34,7 +34,7 @@ beforeAll(async () => { // 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", "TESTING"]) { + for (const name of ["AUTH_TYPE", "SMTP_CONNECTION_URI"]) { vi.stubEnv(name, undefined); } }); @@ -173,42 +173,3 @@ describe("assertValidParams", () => { ); }); }); - -// A blank URI under username/password auth leaves every document without a -// `delivery` field, so the kit rejects it even though the extension's -// installer allowed it. -describe("assertValidParams with a blank SMTP_CONNECTION_URI", () => { - test.each([undefined, "UsernamePassword", "Username & Password"])( - "rejects it when AUTH_TYPE is %j", - (authType) => { - vi.stubEnv("AUTH_TYPE", authType); - vi.stubEnv("SMTP_CONNECTION_URI", ""); - - expect(() => assertValidParams()).toThrow( - 'SMTP_CONNECTION_URI="" must be set unless AUTH_TYPE is OAuth2' - ); - } - ); - - test("accepts it under OAuth2, which does not use the URI", () => { - vi.stubEnv("AUTH_TYPE", "OAuth2"); - vi.stubEnv("SMTP_CONNECTION_URI", ""); - - expect(() => assertValidParams()).not.toThrow(); - }); - - test("accepts it in TESTING mode, whose transport ignores the URI", () => { - vi.stubEnv("TESTING", "true"); - vi.stubEnv("SMTP_CONNECTION_URI", ""); - - expect(() => assertValidParams()).not.toThrow(); - }); - - test("reports only AUTH_TYPE when that is invalid too", () => { - vi.stubEnv("AUTH_TYPE", "oauth2"); - vi.stubEnv("SMTP_CONNECTION_URI", ""); - - expect(() => assertValidParams()).toThrow('AUTH_TYPE="oauth2"'); - expect(() => assertValidParams()).not.toThrow(/SMTP_CONNECTION_URI/); - }); -});