From 93168ae360a835b018a882183019f58fa53a16a1 Mon Sep 17 00:00:00 2001 From: Trent Larson Date: Mon, 14 Sep 2026 11:43:22 -0600 Subject: [PATCH] make the boolean checks consistent, and guard against mistakes --- .env.example | 1 + CHANGELOG.md | 7 +++++- README.md | 4 +++- src/env.ts | 19 ++++++++------- test/env.test.ts | 60 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 12 deletions(-) create mode 100644 test/env.test.ts diff --git a/.env.example b/.env.example index 3451c14..696ad05 100644 --- a/.env.example +++ b/.env.example @@ -41,6 +41,7 @@ # one of the two "from" values are all present. # TWILIO_ACCOUNT_SID= # TWILIO_AUTH_TOKEN= +# Identifier for sender whom this is from. # Prefer the Messaging Service once an A2P 10DLC campaign is approved: the # campaign lives on it, and it wins when both are set. # TWILIO_MESSAGING_SERVICE_SID= diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e0e17b..37be0b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.2.0] - 2026.09.05 +## [Unreleased] +### Changed +- `SMS_ENABLED`, `SMS_REQUIRE_ACTION_CLAIM`, and `SMS_DEV_ECHO_CODE` parse like `DEBUG_ENDPOINT`: trimmed, case-insensitive, accepting `true`/`1`/`yes`/`on` and `false`/`0`/`no`/`off`. (A trailing space from `docker run --env-file` no longer silently turns SMS off, and an unrecognized `SMS_REQUIRE_ACTION_CLAIM` keeps the claim required instead of disabling it.) + + +## [0.2.1] - 2026.09.09 ### Added - `notifyHourUtc` (0-23) and `notifyMinuteUtc` (0-59) are **required** on an alert-authorization batch, both integers and both UTC — the field names carry the frame, so no offset or zone travels with them. Stored zero-padded as one `HH:MM` value in `notify_hour_min_utc`. Required rather than optional because the alternative default is not "no gate" but "the first tick after midnight UTC", which puts every user in one five-minute window. The scheduler holds a user's whole daily run, search included, until that instant, and reports the held users as `deferred`. A stored UTC time does not follow the user through a daylight-saving change; a fresh batch carrying the new offset corrects it - Both alertSearch schedulers pick their users with one query per pass (`listPendingForDay`) instead of one per user: it returns everyone holding an unused JWT for the current UTC day, flagged by whether their `notify_hour_min_utc` has arrived. A user who has already run that day no longer appears, so an idle tick over 2000 users costs 0 queries and ~1ms, down from 3 queries per user and ~141ms diff --git a/README.md b/README.md index 567037e..6c1c3d5 100644 --- a/README.md +++ b/README.md @@ -748,13 +748,15 @@ Set `NOTIFY_DATA_DIR` to a durable directory, or keep the Docker default `/app/d | `SMS_ACTION_JWT_MAX_AGE_SEC` | Optional | `300` | | `SMS_MAX_DIDS_PER_PHONE` | Optional | `5` | | `SMS_ALERT_SEARCH_INTERVAL_MS` | Optional | `300000` | -| `SMS_REQUIRE_ACTION_CLAIM` | Must **not** be `false` in production | `true` | +| `SMS_REQUIRE_ACTION_CLAIM` | Must **not** be a false value in production | `true` | | `SMS_ALLOWED_RECIPIENT_DIDS` | Leave **unset** in production; on a test server, set it to the DIDs that server may text | Unset (no restriction) | | `SMS_BLOCKED_NUMBERS` | Optional; numbers blocked by configuration, on top of the `sms_blocked_numbers` table | Empty | | `SMS_DEV_ECHO_CODE` | Must be unset or `false`; honored only under `NODE_ENV=test-local` | `false` | | Replicas | **One** process | Not enforced in code | | Persistent volume | **Required** for Docker so SQLite survives replace | None unless you pass `-v` | +Boolean flags (`SMS_ENABLED`, `SMS_REQUIRE_ACTION_CLAIM`, `SMS_DEV_ECHO_CODE`, `DEBUG_ENDPOINT`) accept `true`/`1`/`yes`/`on` and `false`/`0`/`no`/`off`, case-insensitive, with surrounding whitespace ignored. Any other value, including a quoted one like `"true"` (which `docker run --env-file` passes through with its quotes), leaves the flag at its default. + Rotating `SMS_CODE_SECRET` invalidates every pending verification code and orphans every stored `phone_hash`. Rotate between deploys, not casually. diff --git a/src/env.ts b/src/env.ts index 1897c8f..00a58c4 100644 --- a/src/env.ts +++ b/src/env.ts @@ -23,7 +23,12 @@ export const PARTNER_URL = /** NODE_ENV value that unlocks developer conveniences. Never set in production. */ export const TEST_LOCAL_ENV = "test-local"; -/** Truthy env values for boolean flags: "true"/"1"/"yes"/"on" (case-insensitive). */ +/** + * Every boolean env flag goes through here. Accepts "true"/"1"/"yes"/"on" and + * "false"/"0"/"no"/"off", case-insensitive and trimmed, because `docker run + * --env-file` keeps trailing whitespace. Anything else yields the fallback, so + * a typo leaves a flag at its documented default rather than flipping it. + */ function envFlag(value: string | undefined, fallback: boolean): boolean { if (value === undefined || value.trim().length === 0) return fallback; const normalized = value.trim().toLowerCase(); @@ -35,12 +40,6 @@ function envFlag(value: string | undefined, fallback: boolean): boolean { /** Mounts /debug when true. Off unless explicitly enabled. */ export const DEBUG_ENDPOINT = envFlag(process.env.DEBUG_ENDPOINT, false); -function booleanEnv(name: string, fallback: boolean): boolean { - const raw = process.env[name]; - if (raw === undefined || raw.length === 0) return fallback; - return raw.toLowerCase() === "true" || raw === "1"; -} - function intEnv(name: string, fallback: number): number { const raw = process.env[name]; if (raw === undefined || raw.length === 0) return fallback; @@ -103,7 +102,7 @@ export type SmsConfig = { export function smsConfig(): SmsConfig { const isTestLocal = process.env.NODE_ENV === TEST_LOCAL_ENV; return { - enabled: booleanEnv("SMS_ENABLED", false), + enabled: envFlag(process.env.SMS_ENABLED, false), codeSecret: stringEnv("SMS_CODE_SECRET"), twilioAccountSid: stringEnv("TWILIO_ACCOUNT_SID"), twilioAuthToken: stringEnv("TWILIO_AUTH_TOKEN"), @@ -115,10 +114,10 @@ export function smsConfig(): SmsConfig { actionJwtMaxAgeSec: intEnv("SMS_ACTION_JWT_MAX_AGE_SEC", 300), maxDidsPerPhone: intEnv("SMS_MAX_DIDS_PER_PHONE", 5), alertSearchIntervalMs: intEnv("SMS_ALERT_SEARCH_INTERVAL_MS", 300000), - requireActionClaim: booleanEnv("SMS_REQUIRE_ACTION_CLAIM", true), + requireActionClaim: envFlag(process.env.SMS_REQUIRE_ACTION_CLAIM, true), // NODE_ENV is checked first: a production process with the flag set by // accident echoes nothing, because its NODE_ENV is not test-local. - devEchoCode: isTestLocal && booleanEnv("SMS_DEV_ECHO_CODE", false), + devEchoCode: isTestLocal && envFlag(process.env.SMS_DEV_ECHO_CODE, false), allowedRecipientDids: didListEnv("SMS_ALLOWED_RECIPIENT_DIDS"), blockedNumbers: (process.env.SMS_BLOCKED_NUMBERS ?? "") .split(",") diff --git a/test/env.test.ts b/test/env.test.ts new file mode 100644 index 0000000..94a2083 --- /dev/null +++ b/test/env.test.ts @@ -0,0 +1,60 @@ +import assert from "node:assert/strict"; +import { afterEach, beforeEach, describe, it } from "node:test"; +import { smsConfig } from "../src/env.js"; + +const KEYS = ["SMS_ENABLED", "SMS_REQUIRE_ACTION_CLAIM"] as const; + +const saved: Record = {}; + +beforeEach(() => { + for (const key of KEYS) { + saved[key] = process.env[key]; + delete process.env[key]; + } +}); + +afterEach(() => { + for (const key of KEYS) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + } +}); + +describe("smsConfig boolean flags", () => { + it("uses the defaults when unset or empty", () => { + assert.equal(smsConfig().enabled, false); + assert.equal(smsConfig().requireActionClaim, true); + process.env.SMS_ENABLED = ""; + process.env.SMS_REQUIRE_ACTION_CLAIM = ""; + assert.equal(smsConfig().enabled, false); + assert.equal(smsConfig().requireActionClaim, true); + }); + + it("accepts every truthy spelling", () => { + for (const value of ["true", "TRUE", "1", "yes", "On"]) { + process.env.SMS_ENABLED = value; + assert.equal(smsConfig().enabled, true, value); + } + }); + + it("accepts every falsy spelling", () => { + for (const value of ["false", "FALSE", "0", "no", "Off"]) { + process.env.SMS_REQUIRE_ACTION_CLAIM = value; + assert.equal(smsConfig().requireActionClaim, false, value); + } + }); + + it("ignores surrounding whitespace, as docker --env-file leaves it", () => { + process.env.SMS_ENABLED = "true "; + assert.equal(smsConfig().enabled, true); + process.env.SMS_REQUIRE_ACTION_CLAIM = " false\t"; + assert.equal(smsConfig().requireActionClaim, false); + }); + + it("falls back to the default on an unrecognized value", () => { + process.env.SMS_ENABLED = '"true"'; + assert.equal(smsConfig().enabled, false); + process.env.SMS_REQUIRE_ACTION_CLAIM = "nope"; + assert.equal(smsConfig().requireActionClaim, true); + }); +});