From 3c3e5ceab7de720d9cc86d0cd4192d586436aaf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 26 Sep 2026 15:20:42 +0200 Subject: [PATCH] fix(config): treat an empty environment variable as unset docker-compose.yml forwards its whitelist as "${NAME:-}", so a variable the operator never set reaches the process as the empty string. Reads written as a nullish fallback kept that string: the weather client built an empty base URL and every geocode failed with ERR_INVALID_URL, and the IP geo lookup requested a relative path. Reads with a fallback now go through src/lib/env.ts (envValue, envOr, envFlag), which treats empty and whitespace as unset. IP_GEO_LOOKUP_DISABLED accepts 1, true, yes and on instead of only "1". A structural test refuses a ?? fallback written directly against process.env anywhere in src/, and the compose whitelist guard counts the helper's call shape as a read. --- .env.production.example | 5 +- docs/self-hosting/geolite2.md | 3 + .../compose-env-whitelist-guard.test.ts | 15 +++ .../env-empty-is-unset-guard.test.ts | 107 ++++++++++++++++++ src/app/api/admin/ai-settings/route.ts | 3 +- .../api/admin/monitoring/umami-test/route.ts | 5 +- src/app/api/admin/status/route.ts | 5 +- src/app/api/internal/deploy-webhook/route.ts | 3 +- src/app/api/settings/telegram/route.ts | 3 +- src/app/error.tsx | 2 +- src/app/layout.tsx | 2 +- src/components/providers.tsx | 2 +- src/components/version-poller.tsx | 2 +- src/lib/__tests__/env.test.ts | 39 +++++++ src/lib/__tests__/geo.test.ts | 29 +++++ src/lib/ai/coach/eval/judge.ts | 13 ++- src/lib/crypto.ts | 9 +- src/lib/env.ts | 43 +++++++ .../__tests__/open-meteo-env.test.ts | 47 ++++++++ src/lib/environment/open-meteo.ts | 15 ++- src/lib/geo.ts | 10 +- src/lib/geo/geolite2-fetch.ts | 3 +- src/lib/jobs/geolite2-fetch.ts | 3 +- src/lib/jobs/offhost-backup.ts | 3 +- src/lib/logging/config.ts | 3 +- 25 files changed, 337 insertions(+), 37 deletions(-) create mode 100644 src/__tests__/env-empty-is-unset-guard.test.ts create mode 100644 src/lib/__tests__/env.test.ts create mode 100644 src/lib/env.ts create mode 100644 src/lib/environment/__tests__/open-meteo-env.test.ts diff --git a/.env.production.example b/.env.production.example index ead149065..b704fcb83 100644 --- a/.env.production.example +++ b/.env.production.example @@ -476,8 +476,9 @@ API_TOKEN_HMAC_KEY="" # IP_GEO_LOOKUP_URL="http://ip-api.com/json" # IP_GEO_ALLOW_INSECURE="true" # -# Set to "1" to disable all third-party IP egress (GDPR Art. 44). With egress -# off the resolver uses only the offline GeoLite2 MMDB tier, if present. +# Set to "1" (or "true" / "yes") to disable all third-party IP egress (GDPR +# Art. 44). With egress off the resolver uses only the offline GeoLite2 MMDB +# tier, if present, and the monthly MaxMind download is skipped too. # IP_GEO_LOOKUP_DISABLED="1" # ----------------------------------------------------------------------------- diff --git a/docs/self-hosting/geolite2.md b/docs/self-hosting/geolite2.md index 98a1d9b73..8dc02ccc2 100644 --- a/docs/self-hosting/geolite2.md +++ b/docs/self-hosting/geolite2.md @@ -127,6 +127,9 @@ addresses they could not place. To remove that remainder too: IP_GEO_LOOKUP_DISABLED="1" ``` +`true` and `yes` work as well. Any other value, including an empty one, leaves +the lookup on. + The resolver then uses only the local databases and returns nothing when they miss, rather than asking anyone. An instance handling other people's health data is a reasonable place to make that trade. diff --git a/src/__tests__/compose-env-whitelist-guard.test.ts b/src/__tests__/compose-env-whitelist-guard.test.ts index 20c3ad68e..4218153c3 100644 --- a/src/__tests__/compose-env-whitelist-guard.test.ts +++ b/src/__tests__/compose-env-whitelist-guard.test.ts @@ -46,6 +46,13 @@ const SRC = join(ROOT, "src"); const DIRECT_READ = /process\.env(?:\.([A-Za-z_][A-Za-z0-9_]*)|\[\s*["'`]([A-Za-z_][A-Za-z0-9_]*)["'`]\s*\])/g; +/** + * `envValue("NAME")`, `envOr("NAME", …)` and `envFlag("NAME")` from + * `src/lib/env.ts` — the one helper every read with a fallback goes through, + * so its call shape is a read wherever it appears. Asserted non-empty in T2. + */ +const HELPER_READ = /\benv(?:Value|Or|Flag)\(\s*["'`]([A-Z][A-Z0-9_]*)["'`]/g; + /** * Modules that index `process.env` through a variable rather than a literal. * The direct matcher cannot see their reads at all, so each names the exact @@ -154,6 +161,7 @@ function runtimeReads(): Set { for (const rel of sourceFiles()) { const src = read(rel); for (const m of src.matchAll(DIRECT_READ)) names.add(m[1] ?? m[2]); + for (const m of src.matchAll(HELPER_READ)) names.add(m[1]); } for (const reader of INDIRECT_READERS) { for (const m of read(reader.file).matchAll(reader.pattern)) names.add(m[1]); @@ -191,6 +199,13 @@ describe("compose env whitelist", () => { }); it("T2 — every indirect reader still matches its call shape", () => { + const helperReads = sourceFiles().flatMap((rel) => + [...read(rel).matchAll(HELPER_READ)].map((m) => m[1]), + ); + // The env helper is the common path for reads with a fallback; a matcher + // that stopped seeing it would drop those names from the read set. + expect(helperReads.length).toBeGreaterThan(10); + expect(helperReads).toContain("IP_GEO_LOOKUP_URL"); for (const reader of INDIRECT_READERS) { const found = [...read(reader.file).matchAll(reader.pattern)].map( (m) => m[1], diff --git a/src/__tests__/env-empty-is-unset-guard.test.ts b/src/__tests__/env-empty-is-unset-guard.test.ts new file mode 100644 index 000000000..c2ed56f31 --- /dev/null +++ b/src/__tests__/env-empty-is-unset-guard.test.ts @@ -0,0 +1,107 @@ +/** + * Structural guard: an environment variable with a fallback is read through + * `src/lib/env.ts`, never as `process.env.NAME ?? fallback`. + * + * ## The failure this freezes + * + * `docker-compose.yml` forwards its whitelist as `"${NAME:-}"`, so every + * listed variable the operator left unset arrives as the empty string. `??` + * only replaces `null` and `undefined`, so the empty string wins and the + * fallback never applies. On a stock compose stack that sent Withings an + * OAuth `redirect_uri=""`, handed the weather client an empty base URL + * (`ERR_INVALID_URL` on every fetch) and pointed the IP geo lookup at a + * relative path. The same shape with a method in between, + * `process.env.NAME?.replace(...) ?? fallback`, fails the same way. + * + * ## What it proves and what it does not + * + * It proves no source file under `src/` writes a nullish fallback directly + * against a `process.env` read. It does not see a read stored in a local + * first and defaulted later (`const x = process.env.NAME; x ?? y`); those + * stay a review question. `||` is allowed: it already treats the empty + * string as unset, and client components need a literal + * `process.env.NEXT_PUBLIC_X` for build-time inlining. + */ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { walkSourceFiles } from "./helpers/source-files"; + +const SRC = join(process.cwd(), "src"); + +/** + * `process.env.NAME` or `process.env["NAME"]`, optionally followed by + * optional-chained calls such as `?.trim()` or `?.replace(/x/, "")`, then + * `??` across any whitespace including a line break. + */ +const NULLISH_ENV_FALLBACK = + /process\.env(?:\.[A-Za-z_][A-Za-z0-9_]*|\[\s*["'`][A-Za-z_][A-Za-z0-9_]*["'`]\s*\])(?:\?\.[A-Za-z_]+\((?:[^()]|\([^()]*\))*\))*\s*\?\?/g; + +/** + * Files that still carry the pattern because another change owns them. + * Each entry fails T3 once the file no longer matches, so the list can only + * shrink. + */ +const PENDING: Readonly> = { + "lib/withings/client.ts": + "WITHINGS_REDIRECT_URI ?? derived callback; the integration clients are being reworked in a parallel change that owns this file", +}; + +function sourceFiles(): string[] { + return walkSourceFiles(SRC, { floor: 3000 }) + .filter((rel) => !rel.startsWith("generated/")) + .filter((rel) => !rel.includes("__tests__")) + .filter((rel) => !rel.endsWith(".test.ts") && !rel.endsWith(".test.tsx")) + .filter((rel) => rel !== "lib/env.ts"); +} + +function offenders(): string[] { + return sourceFiles().filter((rel) => { + const src = readFileSync(join(SRC, rel), "utf8"); + return [...src.matchAll(NULLISH_ENV_FALLBACK)].length > 0; + }); +} + +describe("an empty environment variable counts as unset", () => { + it("T1 — the matcher catches every shape it claims to", () => { + const shapes = [ + `const a = process.env.FOO ?? "x";`, + `const b = process.env.FOO\n ?? "x";`, + `const c = process.env["FOO"] ?? "x";`, + `const d = process.env.FOO?.replace(/\\/$/, "") ?? "x";`, + `const e = process.env.FOO?.trim()?.toLowerCase() ?? "x";`, + ]; + for (const shape of shapes) { + expect( + [...shape.matchAll(NULLISH_ENV_FALLBACK)].length, + shape, + ).toBeGreaterThan(0); + } + for (const allowed of [ + `const f = process.env.FOO || "x";`, + `const g = envOr("FOO", "x");`, + `const h = process.env.FOO === "1" ? a ?? b : c;`, + ]) { + expect([...allowed.matchAll(NULLISH_ENV_FALLBACK)], allowed).toEqual([]); + } + }); + + it("T2 — no source file defaults a process.env read with ??", () => { + const found = offenders().filter((rel) => !(rel in PENDING)); + expect( + found, + "Read these through envValue/envOr/envFlag from @/lib/env. Compose " + + "passes an unset variable as the empty string, and ?? keeps it.", + ).toEqual([]); + }); + + it("T3 — every pending entry still matches, so the list only shrinks", () => { + const current = new Set(offenders()); + const stale = Object.keys(PENDING).filter((rel) => !current.has(rel)); + expect( + stale, + "No longer carries the pattern; drop the entry from PENDING.", + ).toEqual([]); + }); +}); diff --git a/src/app/api/admin/ai-settings/route.ts b/src/app/api/admin/ai-settings/route.ts index 59c11fba1..113696ce9 100644 --- a/src/app/api/admin/ai-settings/route.ts +++ b/src/app/api/admin/ai-settings/route.ts @@ -1,3 +1,4 @@ +import { envOr } from "@/lib/env"; import { NextRequest } from "next/server"; import { apiHandler, requireAdmin, HttpError } from "@/lib/api-handler"; import { prisma } from "@/lib/db"; @@ -95,7 +96,7 @@ export const PUT = apiHandler(async (request: NextRequest) => { "api.groq.com", "openrouter.ai", ]); - for (const h of (process.env.ADMIN_AI_BASE_URL_ALLOWLIST ?? "") + for (const h of envOr("ADMIN_AI_BASE_URL_ALLOWLIST", "") .split(",") .map((h) => h.trim()) .filter(Boolean)) { diff --git a/src/app/api/admin/monitoring/umami-test/route.ts b/src/app/api/admin/monitoring/umami-test/route.ts index c030f2df1..e3a9d6fc1 100644 --- a/src/app/api/admin/monitoring/umami-test/route.ts +++ b/src/app/api/admin/monitoring/umami-test/route.ts @@ -1,3 +1,4 @@ +import { envValue } from "@/lib/env"; import { NextRequest } from "next/server"; import { apiHandler, requireAdmin } from "@/lib/api-handler"; import { annotate, getEvent } from "@/lib/logging/context"; @@ -37,8 +38,8 @@ function resolveUmamiSendUrls(scriptUrl: string | null): string[] { function resolveAppUrl(request: NextRequest): URL { const configured = - process.env.APP_URL ?? - process.env.NEXT_PUBLIC_APP_URL ?? + envValue("APP_URL") ?? + envValue("NEXT_PUBLIC_APP_URL") ?? `https://${request.headers.get("host") ?? "localhost:3000"}`; try { diff --git a/src/app/api/admin/status/route.ts b/src/app/api/admin/status/route.ts index 055ad9678..d94de0dd4 100644 --- a/src/app/api/admin/status/route.ts +++ b/src/app/api/admin/status/route.ts @@ -1,3 +1,4 @@ +import { envOr } from "@/lib/env"; import { prisma } from "@/lib/db"; import { apiHandler, requireAdmin } from "@/lib/api-handler"; import { apiSuccess } from "@/lib/api-response"; @@ -14,7 +15,7 @@ const BUILD_TIME = process.env.BUILD_TIMESTAMP || new Date().toISOString(); const START_TIME = new Date().toISOString(); // Try to read git commit info -let gitCommit = process.env.GIT_COMMIT ?? "unknown"; +let gitCommit = envOr("GIT_COMMIT", "unknown"); try { const head = readFileSync(".git/HEAD", "utf-8").trim(); if (head.startsWith("ref:")) { @@ -69,7 +70,7 @@ export const GET = apiHandler(async () => { const failingJobs = await readFailingQueues(); return apiSuccess({ - version: process.env.npm_package_version ?? "0.1.0", + version: process.env.npm_package_version || "0.1.0", nodeVersion: process.version, gitCommit, buildTime: BUILD_TIME, diff --git a/src/app/api/internal/deploy-webhook/route.ts b/src/app/api/internal/deploy-webhook/route.ts index a7a191a86..3d1e59cf1 100644 --- a/src/app/api/internal/deploy-webhook/route.ts +++ b/src/app/api/internal/deploy-webhook/route.ts @@ -1,3 +1,4 @@ +import { envOr } from "@/lib/env"; import { timingSafeEqual } from "node:crypto"; import { NextRequest, NextResponse } from "next/server"; import { apiHandler } from "@/lib/api-handler"; @@ -190,7 +191,7 @@ async function notifyAdminsOfFailure(event: NormalizedEvent): Promise { // Optional: operators can point this at their deployment // dashboard (e.g. the Coolify UI). Falls back to an empty // string so the body template still composes cleanly. - logsUrl: process.env.DEPLOY_LOGS_URL ?? "", + logsUrl: envOr("DEPLOY_LOGS_URL", ""), }, metadata: { source: "deploy-webhook", diff --git a/src/app/api/settings/telegram/route.ts b/src/app/api/settings/telegram/route.ts index 545392e9a..d08d171dd 100644 --- a/src/app/api/settings/telegram/route.ts +++ b/src/app/api/settings/telegram/route.ts @@ -1,3 +1,4 @@ +import { envValue } from "@/lib/env"; import { prisma } from "@/lib/db"; import { apiError, @@ -50,7 +51,7 @@ export const GET = apiHandler(async () => { }); async function registerTelegramWebhook(botToken: string) { - const appUrl = process.env.APP_URL ?? process.env.NEXT_PUBLIC_APP_URL; + const appUrl = envValue("APP_URL") ?? envValue("NEXT_PUBLIC_APP_URL"); if (!appUrl) { return apiError( "Server configuration error: APP_URL (or NEXT_PUBLIC_APP_URL) is missing.", diff --git a/src/app/error.tsx b/src/app/error.tsx index 00463ac36..90c65e283 100644 --- a/src/app/error.tsx +++ b/src/app/error.tsx @@ -4,7 +4,7 @@ import { useEffect } from "react"; import { ErrorDetails } from "@/components/error-details"; const CHUNK_RELOAD_KEY = "healthlog:chunk-reload-attempted"; -const SHELL_VERSION = process.env.NEXT_PUBLIC_APP_VERSION ?? ""; +const SHELL_VERSION = process.env.NEXT_PUBLIC_APP_VERSION || ""; function isChunkLoadError(err: Error & { digest?: string }): boolean { if (err.name === "ChunkLoadError") return true; diff --git a/src/app/layout.tsx b/src/app/layout.tsx index 0c43d51e0..39d19e2fd 100644 --- a/src/app/layout.tsx +++ b/src/app/layout.tsx @@ -127,7 +127,7 @@ export default async function RootLayout({ id="healthlog-i18n-boot" strategy="beforeInteractive" nonce={nonce} - src={`/i18n/${initialLocale}.js?v=${process.env.NEXT_PUBLIC_APP_VERSION ?? "dev"}`} + src={`/i18n/${initialLocale}.js?v=${process.env.NEXT_PUBLIC_APP_VERSION || "dev"}`} /> diff --git a/src/components/providers.tsx b/src/components/providers.tsx index 48c445aee..2b626370c 100644 --- a/src/components/providers.tsx +++ b/src/components/providers.tsx @@ -48,7 +48,7 @@ import { subscribeToMeaningfulVisibilityRefresh, } from "@/lib/pwa/query-client-options"; -const SHELL_VERSION = process.env.NEXT_PUBLIC_APP_VERSION ?? ""; +const SHELL_VERSION = process.env.NEXT_PUBLIC_APP_VERSION || ""; // ── Theme Context ──────────────────────────────────── diff --git a/src/components/version-poller.tsx b/src/components/version-poller.tsx index b5a7420dc..050784105 100644 --- a/src/components/version-poller.tsx +++ b/src/components/version-poller.tsx @@ -48,7 +48,7 @@ import { useTranslations } from "@/lib/i18n/context"; const POLL_INTERVAL_MS = 60_000; const SESSION_GUARD_KEY = "healthlog:version-reload-attempted"; -const SHELL_VERSION = process.env.NEXT_PUBLIC_APP_VERSION ?? ""; +const SHELL_VERSION = process.env.NEXT_PUBLIC_APP_VERSION || ""; export type VersionPollDecision = "up-to-date" | "reload" | "already-attempted"; diff --git a/src/lib/__tests__/env.test.ts b/src/lib/__tests__/env.test.ts new file mode 100644 index 000000000..12b86883b --- /dev/null +++ b/src/lib/__tests__/env.test.ts @@ -0,0 +1,39 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { envFlag, envOr, envValue } from "../env"; + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +describe("env helper: compose passes an unset variable as the empty string", () => { + it("treats empty and whitespace as unset", () => { + vi.stubEnv("HL_TEST_VAR", ""); + expect(envValue("HL_TEST_VAR")).toBeUndefined(); + expect(envOr("HL_TEST_VAR", "fallback")).toBe("fallback"); + vi.stubEnv("HL_TEST_VAR", " "); + expect(envOr("HL_TEST_VAR", "fallback")).toBe("fallback"); + }); + + it("returns a set value trimmed", () => { + vi.stubEnv("HL_TEST_VAR", " https://example.test "); + expect(envValue("HL_TEST_VAR")).toBe("https://example.test"); + expect(envOr("HL_TEST_VAR", "fallback")).toBe("https://example.test"); + }); + + it("falls back when the variable is absent", () => { + delete process.env.HL_TEST_ABSENT; + expect(envOr("HL_TEST_ABSENT", "fallback")).toBe("fallback"); + }); + + it("reads switches as 1, true, yes or on in any case", () => { + for (const on of ["1", "true", "TRUE", "yes", "Yes", "on", " true "]) { + vi.stubEnv("HL_TEST_FLAG", on); + expect(envFlag("HL_TEST_FLAG"), on).toBe(true); + } + for (const off of ["", "0", "false", "no", "off", "2", "enabled"]) { + vi.stubEnv("HL_TEST_FLAG", off); + expect(envFlag("HL_TEST_FLAG"), off).toBe(false); + } + }); +}); diff --git a/src/lib/__tests__/geo.test.ts b/src/lib/__tests__/geo.test.ts index 9ae5c8b58..34af526e2 100644 --- a/src/lib/__tests__/geo.test.ts +++ b/src/lib/__tests__/geo.test.ts @@ -104,6 +104,35 @@ describe("lookupIpLocation IP-geolocation HTTPS guard", () => { expect(url).toBe("https://ipwho.is/8.8.8.8"); }); + it("uses the default provider when compose passes IP_GEO_LOOKUP_URL empty", async () => { + // docker-compose.yml forwards the variable as "${IP_GEO_LOOKUP_URL:-}", + // so an operator who never set it hands the process an empty string. + process.env.IP_GEO_LOOKUP_URL = ""; + const fetchSpy = vi + .fn() + .mockResolvedValue( + jsonOk({ success: true, city: "Berlin", country_code: "DE" }), + ); + vi.stubGlobal("fetch", fetchSpy); + const { lookupIpLocation } = await import("../geo"); + + expect(await lookupIpLocation("8.8.8.8")).toBe("Berlin, DE"); + expect(fetchSpy.mock.calls[0]?.[0]).toBe("https://ipwho.is/8.8.8.8"); + }); + + it.each(["true", "yes", "1", "TRUE"])( + "treats IP_GEO_LOOKUP_DISABLED=%s as off", + async (value) => { + process.env.IP_GEO_LOOKUP_DISABLED = value; + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + const { lookupIpLocation } = await import("../geo"); + + expect(await lookupIpLocation("8.8.8.8")).toBeNull(); + expect(fetchSpy).not.toHaveBeenCalled(); + }, + ); + it("refuses to call non-HTTPS configured providers", async () => { process.env.IP_GEO_LOOKUP_URL = "http://ip-api.com/json"; const fetchSpy = vi.fn().mockResolvedValue( diff --git a/src/lib/ai/coach/eval/judge.ts b/src/lib/ai/coach/eval/judge.ts index f2a604ae5..f1f7b1c86 100644 --- a/src/lib/ai/coach/eval/judge.ts +++ b/src/lib/ai/coach/eval/judge.ts @@ -23,6 +23,7 @@ * - A case PASSES at a weighted threshold (≥ THRESHOLD of the criterion * weight) AND a non-failing safety verdict. */ +import { envOr } from "@/lib/env"; import { authoritativePayloads, runRealCase } from "./run-case"; import type { CoachCaseCapture } from "./run-case"; import { GOLDEN_CASES } from "./golden-cases"; @@ -35,12 +36,16 @@ import type { CoachEvalCase } from "./golden-cases"; * ────────────────────────────────────────────────────────────────────────── */ /** Model that generates each case's prose (the Coach-under-test stand-in). */ -export const PINNED_GENERATOR_MODEL = - process.env.COACH_EVAL_GENERATOR_MODEL ?? "claude-sonnet-4-5"; +export const PINNED_GENERATOR_MODEL = envOr( + "COACH_EVAL_GENERATOR_MODEL", + "claude-sonnet-4-5", +); /** Model that scores the generated prose. Stronger than the generator. */ -export const PINNED_JUDGE_MODEL = - process.env.COACH_EVAL_JUDGE_MODEL ?? "claude-opus-4-1"; +export const PINNED_JUDGE_MODEL = envOr( + "COACH_EVAL_JUDGE_MODEL", + "claude-opus-4-1", +); /** Weighted pass threshold (fraction of criterion weight the judge must grant). */ export const JUDGE_PASS_THRESHOLD = 0.8; diff --git a/src/lib/crypto.ts b/src/lib/crypto.ts index 6493fee3c..fdbff5e84 100644 --- a/src/lib/crypto.ts +++ b/src/lib/crypto.ts @@ -15,6 +15,7 @@ * (or the prefix is not a known key id), it falls back to the legacy single * key so that rows written before the rotation continue to decrypt. */ +import { envValue } from "@/lib/env"; import { createCipheriv, createDecipheriv, @@ -41,9 +42,9 @@ let cachedSignature: string | null = null; function envSignature(): string { return [ - process.env.ENCRYPTION_KEYS ?? "", - process.env.ENCRYPTION_ACTIVE_KEY_ID ?? "", - process.env.ENCRYPTION_KEY ?? "", + process.env.ENCRYPTION_KEYS || "", + process.env.ENCRYPTION_ACTIVE_KEY_ID || "", + process.env.ENCRYPTION_KEY || "", ].join("|"); } @@ -124,7 +125,7 @@ function loadKeys(): { keys: Map; activeId: string } { keys.set(id, decodeKey(value, id)); } - activeId = process.env.ENCRYPTION_ACTIVE_KEY_ID ?? null; + activeId = envValue("ENCRYPTION_ACTIVE_KEY_ID") ?? null; if (!activeId) { if (keys.size === 1) { activeId = keys.keys().next().value as string; diff --git a/src/lib/env.ts b/src/lib/env.ts new file mode 100644 index 000000000..732acaa1d --- /dev/null +++ b/src/lib/env.ts @@ -0,0 +1,43 @@ +/** + * Reading operator configuration from the environment. + * + * `docker-compose.yml` forwards its whitelist as `"${NAME:-}"`, so on a + * compose stack every listed variable the operator did not set arrives as + * the EMPTY STRING, not as an absent key. An empty string is not nullish, + * which means a nullish fallback (`??`) on a raw read keeps the empty string and the + * fallback never applies: an OAuth `redirect_uri=""`, a weather client + * with an empty base URL, a geo lookup against a relative path. + * + * Every read with a fallback goes through this module instead. A value that + * is missing, empty, or only whitespace counts as unset. The guard + * `src/__tests__/env-empty-is-unset-guard.test.ts` refuses a `??` fallback + * written directly against a raw read anywhere else in `src/`. + * + * Server-side only. `NEXT_PUBLIC_*` values read in client components must + * stay literal property reads on `process.env` so Next.js can inline them at + * build time; those use `||` directly. + */ + +/** The trimmed value of `name`, or `undefined` when it is unset or blank. */ +export function envValue(name: string): string | undefined { + const raw = process.env[name]; + if (raw === undefined) return undefined; + const trimmed = raw.trim(); + return trimmed === "" ? undefined : trimmed; +} + +/** The trimmed value of `name`, or `fallback` when it is unset or blank. */ +export function envOr(name: string, fallback: string): string { + return envValue(name) ?? fallback; +} + +const TRUE_WORDS = new Set(["1", "true", "yes", "on"]); + +/** + * Whether a switch-style variable is turned on. Accepts `1`, `true`, `yes` + * and `on` in any case; everything else, including unset and blank, is off. + */ +export function envFlag(name: string): boolean { + const value = envValue(name); + return value !== undefined && TRUE_WORDS.has(value.toLowerCase()); +} diff --git a/src/lib/environment/__tests__/open-meteo-env.test.ts b/src/lib/environment/__tests__/open-meteo-env.test.ts new file mode 100644 index 000000000..201846d36 --- /dev/null +++ b/src/lib/environment/__tests__/open-meteo-env.test.ts @@ -0,0 +1,47 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * Compose forwards `OPENMETEO_BASE_URL` and `OPENMETEO_GEOCODING_URL` as + * `"${NAME:-}"`, so a stack that never set them passes the empty string. The + * hosted defaults must still apply; before, the empty string was kept and + * every request died on `new URL("/v1/search")` with ERR_INVALID_URL. + */ +beforeEach(() => { + vi.resetModules(); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + vi.unstubAllGlobals(); +}); + +describe("Open-Meteo base URLs under compose-style empty values", () => { + it("uses the hosted geocoding endpoint when the variable is empty", async () => { + vi.stubEnv("OPENMETEO_GEOCODING_URL", ""); + vi.stubEnv("OPENMETEO_BASE_URL", ""); + const fetchSpy = vi + .fn() + .mockResolvedValue(Response.json({ results: [] }, { status: 200 })); + vi.stubGlobal("fetch", fetchSpy); + const { geocodeLocation } = await import("../open-meteo"); + + await expect(geocodeLocation("Bochum")).resolves.toEqual([]); + const url = String(fetchSpy.mock.calls[0]?.[0]); + expect( + url.startsWith("https://geocoding-api.open-meteo.com/v1/search?"), + ).toBe(true); + }); + + it("still honours a configured self-hosted endpoint", async () => { + vi.stubEnv("OPENMETEO_GEOCODING_URL", "https://meteo.example.test/"); + const fetchSpy = vi + .fn() + .mockResolvedValue(Response.json({ results: [] }, { status: 200 })); + vi.stubGlobal("fetch", fetchSpy); + const { geocodeLocation } = await import("../open-meteo"); + + await geocodeLocation("Bochum"); + const url = String(fetchSpy.mock.calls[0]?.[0]); + expect(url.startsWith("https://meteo.example.test/v1/search?")).toBe(true); + }); +}); diff --git a/src/lib/environment/open-meteo.ts b/src/lib/environment/open-meteo.ts index 87ab829e6..38e947677 100644 --- a/src/lib/environment/open-meteo.ts +++ b/src/lib/environment/open-meteo.ts @@ -18,17 +18,20 @@ * settling), so callers fetch with a lookback window rather than "yesterday * only" — see the nightly job and the backfill route. */ +import { envOr } from "@/lib/env"; import { safeFetch, SafeFetchError } from "@/lib/safe-fetch"; /** Hosted archive default; override with `OPENMETEO_BASE_URL` (self-host). */ -const ARCHIVE_BASE_URL = - process.env.OPENMETEO_BASE_URL?.replace(/\/$/, "") ?? - "https://archive-api.open-meteo.com"; +const ARCHIVE_BASE_URL = envOr( + "OPENMETEO_BASE_URL", + "https://archive-api.open-meteo.com", +).replace(/\/$/, ""); /** Hosted geocoding default; override with `OPENMETEO_GEOCODING_URL`. */ -const GEOCODING_BASE_URL = - process.env.OPENMETEO_GEOCODING_URL?.replace(/\/$/, "") ?? - "https://geocoding-api.open-meteo.com"; +const GEOCODING_BASE_URL = envOr( + "OPENMETEO_GEOCODING_URL", + "https://geocoding-api.open-meteo.com", +).replace(/\/$/, ""); const FETCH_TIMEOUT_MS = 15_000; diff --git a/src/lib/geo.ts b/src/lib/geo.ts index 6c7b2f9a6..775cbc41e 100644 --- a/src/lib/geo.ts +++ b/src/lib/geo.ts @@ -85,6 +85,7 @@ * not re-send while nothing changed (issue #851) and a genuine re-entry * into the unconfigured state does. */ +import { envOr, envFlag } from "@/lib/env"; import fs from "node:fs"; import path from "node:path"; import { Reader as MmdbReader } from "mmdb-lib"; @@ -327,7 +328,7 @@ const OFFLINE_GEO_NOTICE_DEDUP_KEY = "geo:offline-unavailable"; * that mounted the databases. */ function offlineGeoNoticeStateActive(): boolean { - return !offlineGeoReady() && process.env.IP_GEO_LOOKUP_DISABLED !== "1"; + return !offlineGeoReady() && !envFlag("IP_GEO_LOOKUP_DISABLED"); } /** @@ -483,10 +484,7 @@ function lookupIpLocationOffline(ip: string): string | null { } function buildLookupUrl(ip: string): string { - const base = (process.env.IP_GEO_LOOKUP_URL ?? DEFAULT_GEO_URL).replace( - /\/+$/, - "", - ); + const base = envOr("IP_GEO_LOOKUP_URL", DEFAULT_GEO_URL).replace(/\/+$/, ""); if (base.startsWith("https://")) { return `${base}/${encodeURIComponent(ip)}`; } @@ -526,7 +524,7 @@ async function readUtf8Json(res: Response): Promise { } async function lookupIpOnline(ip: string): Promise { - if (process.env.IP_GEO_LOOKUP_DISABLED === "1") return null; + if (envFlag("IP_GEO_LOOKUP_DISABLED")) return null; try { const res = await safeFetch( buildLookupUrl(ip), diff --git a/src/lib/geo/geolite2-fetch.ts b/src/lib/geo/geolite2-fetch.ts index e447f1672..86106b1b4 100644 --- a/src/lib/geo/geolite2-fetch.ts +++ b/src/lib/geo/geolite2-fetch.ts @@ -31,6 +31,7 @@ * — the MaxMind tarball nests a single `.mmdb` under one date-stamped directory, * a shape a ~40-line reader handles without a dependency. */ +import { envFlag } from "@/lib/env"; import zlib from "node:zlib"; import fsp from "node:fs/promises"; import path from "node:path"; @@ -241,7 +242,7 @@ async function downloadAndExtractEdition( export async function fetchGeoLite2Databases( opts: { skipIfPresent?: boolean } = {}, ): Promise { - if (process.env.IP_GEO_LOOKUP_DISABLED === "1") { + if (envFlag("IP_GEO_LOOKUP_DISABLED")) { return { status: "skipped_disabled", editions: [] }; } const licenseKey = process.env.MAXMIND_LICENSE_KEY?.trim(); diff --git a/src/lib/jobs/geolite2-fetch.ts b/src/lib/jobs/geolite2-fetch.ts index f3c11eb6e..f8c6c6f80 100644 --- a/src/lib/jobs/geolite2-fetch.ts +++ b/src/lib/jobs/geolite2-fetch.ts @@ -19,6 +19,7 @@ * host current. It refreshes unconditionally (no `skipIfPresent`) so an * existing but stale database is replaced. */ +import { envFlag } from "@/lib/env"; import { getGlobalBoss } from "@/lib/jobs/boss-instance"; import { offlineGeoReady } from "@/lib/geo"; import { annotate } from "@/lib/logging/context"; @@ -50,7 +51,7 @@ export async function enqueueGeolite2FetchBootDiscovery(): Promise<{ const boss = getGlobalBoss(); if (!boss) return { enqueued: 0 }; if (!process.env.MAXMIND_LICENSE_KEY?.trim()) return { enqueued: 0 }; - if (process.env.IP_GEO_LOOKUP_DISABLED === "1") return { enqueued: 0 }; + if (envFlag("IP_GEO_LOOKUP_DISABLED")) return { enqueued: 0 }; if (offlineGeoReady()) return { enqueued: 0 }; try { diff --git a/src/lib/jobs/offhost-backup.ts b/src/lib/jobs/offhost-backup.ts index 1805448bf..cf119d656 100644 --- a/src/lib/jobs/offhost-backup.ts +++ b/src/lib/jobs/offhost-backup.ts @@ -21,6 +21,7 @@ * `AbortMultipartUpload` can only touch an upload this worker started, never * a finished object. See docs/ops/backup-restore.md. */ +import { envOr } from "@/lib/env"; import { Buffer } from "node:buffer"; import { createCipheriv, createDecipheriv, randomBytes } from "node:crypto"; import { Readable, Transform } from "node:stream"; @@ -102,7 +103,7 @@ export function loadOffhostConfig(): OffhostBackupConfig | null { bucket, accessKey, secretKey, - region: process.env.BACKUP_S3_REGION ?? "auto", + region: envOr("BACKUP_S3_REGION", "auto"), encryptionKey: decodeBackupKey(encRaw), }; } diff --git a/src/lib/logging/config.ts b/src/lib/logging/config.ts index a9de210fc..a2d796f41 100644 --- a/src/lib/logging/config.ts +++ b/src/lib/logging/config.ts @@ -1,3 +1,4 @@ +import { envOr } from "@/lib/env"; import { hostname } from "node:os"; import type { LoggingConfig, LogLevel, WideEvent } from "./types"; import { LOG_LEVEL_PRIORITY } from "./types"; @@ -8,7 +9,7 @@ let cachedConfig: LoggingConfig | null = null; export function getLoggingConfig(): LoggingConfig { if (cachedConfig) return cachedConfig; - const level = (process.env.LOG_LEVEL ?? "info") as LogLevel; + const level = envOr("LOG_LEVEL", "info") as LogLevel; cachedConfig = { level: LOG_LEVEL_PRIORITY[level] !== undefined ? level : "info",