diff --git a/src/__tests__/baselines/import-cycles.json b/src/__tests__/baselines/import-cycles.json index d92e0b7a9..ba40e52d7 100644 --- a/src/__tests__/baselines/import-cycles.json +++ b/src/__tests__/baselines/import-cycles.json @@ -1,6 +1,6 @@ { "$comment": "Runtime import cycles under src/: the back-edge count and the strongly connected components. Maintained by src/__tests__/import-cycle-guard.test.ts.", - "cycles": 13, + "cycles": 7, "components": [ [ "components/command-palette/command-palette.tsx", @@ -19,15 +19,8 @@ "lib/medications/scheduling/schedule-eras.ts" ], [ - "lib/auth/audit.ts", - "lib/geo.ts", "lib/integrations/oauth-kit/connect-target.ts", - "lib/jobs/offhost-purge.ts", "lib/managed-profiles/lifecycle.ts", - "lib/notifications/channel-state.ts", - "lib/notifications/delivery-identity.ts", - "lib/notifications/dispatch-localised.ts", - "lib/notifications/dispatcher.ts", "lib/sharing/grants.ts" ], [ diff --git a/src/instrumentation.ts b/src/instrumentation.ts index d75f9f8e5..0aea0b2fe 100644 --- a/src/instrumentation.ts +++ b/src/instrumentation.ts @@ -35,6 +35,17 @@ export async function register() { // A readiness summary must never break boot. } + // The admin notice for a host whose login-IP lookups leave it. The geo + // resolver evaluates it on its first public-IP lookup; it holds a slot + // rather than importing the notification layer (see the notice module). + try { + const { registerOfflineGeoNotice } = + await import("@/lib/notifications/offline-geo-notice"); + registerOfflineGeoNotice(); + } catch { + // Without it the lookups still resolve; only the notice is missing. + } + // Does the configured encryption key open this database? A process whose // key does not refuses every API request with one clear 503 and starts // neither the producer nor the worker (a wrong key must never write). diff --git a/src/lib/__tests__/geo-offline-detection.test.ts b/src/lib/__tests__/geo-offline-detection.test.ts index 0205a83b0..aeea09d89 100644 --- a/src/lib/__tests__/geo-offline-detection.test.ts +++ b/src/lib/__tests__/geo-offline-detection.test.ts @@ -9,6 +9,8 @@ import { import { tmpdir } from "node:os"; import { join } from "node:path"; +import { readSource } from "@/__tests__/helpers/source-files"; + /** * Runtime detection + once-per-state admin notification for the offline * GeoLite2 tier. @@ -17,7 +19,8 @@ import { join } from "node:path"; * whenever `MAXMIND_LICENSE_KEY` is unset, so the Docker COPY still * lands a non-empty directory in `/opt/geolite2/`. The runtime resolver * detects the marker on first fallback and fires a localised - * notification to every admin. + * notification to every admin, through the notice module registered in + * its slot at boot (`src/lib/notifications/offline-geo-notice.ts`). * * Issue #851: the sent-latch used to be a module-level boolean, so every * worker boot re-sent the same notice while the configuration never @@ -141,6 +144,18 @@ async function flushMicrotasks(): Promise { } } +/** + * The resolver plus the notice in its slot, the way a booted process has + * them: `src/instrumentation.ts` registers the notice before any request. + */ +async function loadGeo(): Promise { + const geo = await import("../geo"); + const { registerOfflineGeoNotice } = + await import("@/lib/notifications/offline-geo-notice"); + registerOfflineGeoNotice(); + return geo; +} + beforeEach(() => { process.env = { ...ORIGINAL_ENV }; delete process.env.IP_GEO_LOOKUP_DISABLED; @@ -161,14 +176,41 @@ afterEach(() => { }); describe("offline-geo runtime detection", () => { + it("evaluates nothing before the notice is registered, and runs on the first lookup after", async () => { + writeFileSync(join(tmpRoot, ".empty"), ""); + const geo = await import("../geo"); + + await geo.lookupIpLocation("8.8.8.8"); + await flushMicrotasks(); + expect(findManySpy).not.toHaveBeenCalled(); + expect(dispatchSpy).not.toHaveBeenCalled(); + + const { registerOfflineGeoNotice } = + await import("@/lib/notifications/offline-geo-notice"); + registerOfflineGeoNotice(); + await geo.lookupIpLocation("1.1.1.1"); + await flushMicrotasks(); + expect(dispatchSpy).toHaveBeenCalledTimes(1); + }); + + it("is registered at boot", () => { + const boot = readSource( + "src/instrumentation.ts", + "registerOfflineGeoNotice", + ); + expect(boot).toMatch( + /import\("@\/lib\/notifications\/offline-geo-notice"\)[\s\S]*registerOfflineGeoNotice\(\)/, + ); + }); + it("offlineGeoReady() is false when the .empty marker is present", async () => { writeFileSync(join(tmpRoot, ".empty"), ""); - const { offlineGeoReady } = await import("../geo"); + const { offlineGeoReady } = await loadGeo(); expect(offlineGeoReady()).toBe(false); }); it("offlineGeoReady() is false when the City MMDB is absent", async () => { - const { offlineGeoReady } = await import("../geo"); + const { offlineGeoReady } = await loadGeo(); expect(offlineGeoReady()).toBe(false); }); @@ -184,7 +226,7 @@ describe("offline-geo runtime detection", () => { process.chdir(tmpRoot); try { process.env.GEOLITE2_DIR = ""; - const { offlineGeoReady } = await import("../geo"); + const { offlineGeoReady } = await loadGeo(); expect(offlineGeoReady()).toBe(false); } finally { process.chdir(originalCwd); @@ -196,13 +238,13 @@ describe("offline-geo runtime detection", () => { join(tmpRoot, "GeoLite2-City.mmdb"), Buffer.from("city-stub"), ); - const { offlineGeoReady } = await import("../geo"); + const { offlineGeoReady } = await loadGeo(); expect(offlineGeoReady()).toBe(true); }); it("fires the admin notification when the marker is present and lookupIpLocation falls back", async () => { writeFileSync(join(tmpRoot, ".empty"), ""); - const { lookupIpLocation } = await import("../geo"); + const { lookupIpLocation } = await loadGeo(); await lookupIpLocation("8.8.8.8"); await flushMicrotasks(); @@ -226,7 +268,7 @@ describe("offline-geo runtime detection", () => { it("names the real online provider in the notification params", async () => { writeFileSync(join(tmpRoot, ".empty"), ""); process.env.IP_GEO_LOOKUP_URL = "https://ip-api.example/json"; - const { lookupIpLocation } = await import("../geo"); + const { lookupIpLocation } = await loadGeo(); await lookupIpLocation("8.8.8.8"); await flushMicrotasks(); @@ -286,7 +328,7 @@ describe("offline-geo runtime detection", () => { // The stubbed reader returns null for every IP, which is fine — the // lookup will still go through the online path, but the offline // check sees a healthy directory so the notification stays silent. - const { lookupIpLocation } = await import("../geo"); + const { lookupIpLocation } = await loadGeo(); await lookupIpLocation("8.8.8.8"); await flushMicrotasks(); @@ -301,7 +343,7 @@ describe("offline-geo runtime detection", () => { // left the state exactly like one that mounted the databases. writeFileSync(join(tmpRoot, ".empty"), ""); process.env.IP_GEO_LOOKUP_DISABLED = "1"; - const { lookupIpLocation } = await import("../geo"); + const { lookupIpLocation } = await loadGeo(); await lookupIpLocation("8.8.8.8"); await flushMicrotasks(); @@ -312,7 +354,7 @@ describe("offline-geo runtime detection", () => { it("fires the notification exactly once across repeated fallbacks in one process", async () => { writeFileSync(join(tmpRoot, ".empty"), ""); - const { lookupIpLocation, lookupIpAsn } = await import("../geo"); + const { lookupIpLocation, lookupIpAsn } = await loadGeo(); await lookupIpLocation("8.8.8.8"); await flushMicrotasks(); @@ -328,7 +370,7 @@ describe("offline-geo runtime detection", () => { it("falls back silently when no admin user is configured", async () => { writeFileSync(join(tmpRoot, ".empty"), ""); findManySpy.mockResolvedValueOnce([]); - const { lookupIpLocation } = await import("../geo"); + const { lookupIpLocation } = await loadGeo(); await lookupIpLocation("8.8.8.8"); await flushMicrotasks(); @@ -339,7 +381,7 @@ describe("offline-geo runtime detection", () => { it("lookupIpAsn fires the notification when the ASN reader is missing and the marker is set", async () => { writeFileSync(join(tmpRoot, ".empty"), ""); - const { lookupIpAsn } = await import("../geo"); + const { lookupIpAsn } = await loadGeo(); const result = lookupIpAsn("8.8.8.8"); await flushMicrotasks(); @@ -353,7 +395,7 @@ describe("offline-geo runtime detection", () => { // ── Boot 1: unconfigured → the notice goes out and anchors durably. writeFileSync(join(tmpRoot, ".empty"), ""); - let geo = await import("../geo"); + let geo = await loadGeo(); await geo.lookupIpLocation("8.8.8.8"); await flushMicrotasks(); expect(dispatchSpy).toHaveBeenCalledTimes(1); @@ -362,7 +404,7 @@ describe("offline-geo runtime detection", () => { // ── Boot 2: worker restart, state unchanged → the persisted anchor // keeps it silent. This is the #851 repeat, pinned closed. vi.resetModules(); - geo = await import("../geo"); + geo = await loadGeo(); await geo.lookupIpLocation("8.8.8.8"); await flushMicrotasks(); expect(dispatchSpy).toHaveBeenCalledTimes(1); @@ -373,7 +415,7 @@ describe("offline-geo runtime detection", () => { unlinkSync(join(tmpRoot, ".empty")); writeFileSync(cityDb, Buffer.from("city-stub")); vi.resetModules(); - geo = await import("../geo"); + geo = await loadGeo(); await geo.lookupIpLocation("8.8.8.8"); await flushMicrotasks(); expect(dispatchSpy).toHaveBeenCalledTimes(1); @@ -384,7 +426,7 @@ describe("offline-geo runtime detection", () => { unlinkSync(cityDb); writeFileSync(join(tmpRoot, ".empty"), ""); vi.resetModules(); - geo = await import("../geo"); + geo = await loadGeo(); await geo.lookupIpLocation("8.8.8.8"); await flushMicrotasks(); expect(dispatchSpy).toHaveBeenCalledTimes(2); @@ -396,7 +438,7 @@ describe("offline-geo runtime detection", () => { // placing fresh databases. The next lookup must observe the healthy // state and release the anchor without waiting for a restart. writeFileSync(join(tmpRoot, ".empty"), ""); - const geo = await import("../geo"); + const geo = await loadGeo(); await geo.lookupIpLocation("8.8.8.8"); await flushMicrotasks(); diff --git a/src/lib/geo.ts b/src/lib/geo.ts index 86707f29c..baebbf1ea 100644 --- a/src/lib/geo.ts +++ b/src/lib/geo.ts @@ -91,11 +91,6 @@ import path from "node:path"; import { Reader as MmdbReader } from "mmdb-lib"; import type { AsnResponse, CityResponse } from "mmdb-lib/lib/reader/response"; import { getEvent } from "@/lib/logging/context"; -import { - claimNotificationEvent, - releaseNotificationEvent, - CONFIG_NOTICE_EVENT_TYPE, -} from "@/lib/notifications/reminder-dedup"; import { safeFetch } from "@/lib/safe-fetch"; interface IpwhoIsResponse { @@ -309,7 +304,7 @@ export function __resetGeoLite2CacheForTests(): void { GEO_CACHE.clear(); } -// ── Offline readiness + once-per-state admin notification ──────────── +// ── Offline readiness + the once-per-state admin notice ────────────── // // The CI workflow drops an `.empty` marker into the geo asset directory // when the maintainer has not configured `MAXMIND_LICENSE_KEY`, in @@ -318,45 +313,27 @@ export function __resetGeoLite2CacheForTests(): void { // `/api/version`, the admin status surface, and the resolver's own // offline-first branch, so the three cannot disagree. // -// The notification is once per CONFIGURATION STATE. A module-level latch -// alone re-sent it on every worker boot while the state never changed -// (issue #851: same notice again and again on a host that has simply not -// mounted the databases). The durable anchor is a per-admin -// `notification_events` claim — the same ledger every other repeat-prone -// notice anchors in — released when the state exits, i.e. when the -// databases are configured or the online lookup is disabled. Entering the -// unconfigured state sends once; staying in it is silent across restarts; -// leaving and re-entering sends again. -// -// `noticeEvaluatedThisProcess` remains as a cheap gate so the ledger is -// consulted once per worker boot (and again after a runtime database -// fetch resets it), not on every lookup. Test-only reset is folded into -// the cache reset above. +// The admin notice about that state lives in +// `src/lib/notifications/offline-geo-notice.ts`, which fills the slot below +// at boot. This module only decides WHEN it is evaluated: on the first +// public-IP lookup of a process, and again after a runtime database fetch +// resets `noticeEvaluatedThisProcess`. It imports nothing from the +// notification layer: `auditLog` resolves locations through here, and the +// dispatcher's own collaborators write audit rows. let noticeEvaluatedThisProcess = false; -/** Durable anchor identity for the offline-geo notice in `notification_events`. */ -const OFFLINE_GEO_NOTICE_DEDUP_KEY = "geo:offline-unavailable"; +/** The offline-geo notice, once `setOfflineGeoNotice` has filled the slot. */ +let offlineGeoNotice: (() => Promise) | null = null; /** - * Whether the host is in the state the notice describes: the offline tier - * is not configured AND lookups still egress to the online provider. With - * `IP_GEO_LOOKUP_DISABLED=1` there is no egress to warn about, so the - * disabled host counts as having left the state — exactly like a host - * that mounted the databases. + * Fill the slot the lookups evaluate the offline-geo notice through. Called + * once at boot by `registerOfflineGeoNotice`; a later call replaces it. */ -function offlineGeoNoticeStateActive(): boolean { - return !offlineGeoReady() && !envFlag("IP_GEO_LOOKUP_DISABLED"); +export function setOfflineGeoNotice(notice: () => Promise): void { + offlineGeoNotice = notice; } -/** - * Where the admin notification sends someone who wants the offline tier. - * The in-repo path is the durable one — the runbook ships with the source - * that emits this link, so the two cannot drift apart. - */ -const GEOLITE2_DOCS_URL = - "https://github.com/MBombeck/HealthLog/blob/main/docs/self-hosting/geolite2.md"; - export function offlineGeoReady(): boolean { try { const dir = geoLiteDir(); @@ -367,79 +344,15 @@ export function offlineGeoReady(): boolean { } } +/** + * Evaluate the notice once per process (and once more after each reset). + * A process whose slot is still empty evaluates nothing and keeps the latch + * open, so the first lookup after the slot is filled still runs it. + */ async function evaluateOfflineGeoNotice(): Promise { - if (noticeEvaluatedThisProcess) return; + if (noticeEvaluatedThisProcess || !offlineGeoNotice) return; noticeEvaluatedThisProcess = true; - // Defer the resolve so we don't pay the cost of pulling the - // Prisma client into the import graph until the first evaluation — - // most calls are cache hits or private-IP short-circuits. - try { - const { prisma } = await import("@/lib/db"); - - if (!offlineGeoNoticeStateActive()) { - // The state the notice describes does not hold. Release any - // persisted sent-anchor so a future re-entry into the unconfigured - // state notifies again — this is the "state exit" edge. - await releaseNotificationEvent(prisma, { - eventType: CONFIG_NOTICE_EVENT_TYPE, - dedupKey: OFFLINE_GEO_NOTICE_DEDUP_KEY, - }); - return; - } - - const { dispatchLocalisedNotification } = - await import("@/lib/notifications/dispatch-localised"); - const admins = await prisma.user.findMany({ - where: { role: "ADMIN" }, - select: { id: true }, - }); - if (admins.length === 0) { - getEvent()?.addWarning( - "geo: offline databases unavailable, no admin user configured to notify", - ); - return; - } - for (const admin of admins) { - // The anchor is unwindowed (`since` at epoch): it holds until the - // state exit above releases it, however many restarts happen in - // between. An admin who already has the anchor claimed nothing and - // hears nothing. - const claimed = await claimNotificationEvent(prisma, { - recordUserId: admin.id, - eventType: CONFIG_NOTICE_EVENT_TYPE, - dedupKey: OFFLINE_GEO_NOTICE_DEDUP_KEY, - since: new Date(0), - }); - if (!claimed) continue; - getEvent()?.addWarning( - `geo: offline databases unavailable, every lookup goes online to ${resolveGeoProviderHost()} — notifying admin`, - ); - await dispatchLocalisedNotification({ - userId: admin.id, - titleKey: "notifications.admin.offlineGeoUnavailableTitle", - messageKey: "notifications.admin.offlineGeoUnavailableBody", - // The message names what the person reading it can actually do. - // It used to point at a licence-key secret in this repository's - // Actions settings, which nobody running the published image can - // set (issue #659) — the reachable path is mounting your own - // databases and pointing GEOLITE2_DIR at them. - params: { - host: resolveGeoProviderHost(), - docsUrl: GEOLITE2_DOCS_URL, - }, - metadata: { - source: "geo-offline-detection", - url: "/admin/system-status", - }, - }); - } - } catch (err) { - // Never let a notification failure propagate — the auth-audit - // caller is fire-and-forget. - getEvent()?.addWarning( - `geo: offline-geo notification failed: ${err instanceof Error ? err.message : String(err)}`, - ); - } + await offlineGeoNotice(); } /** diff --git a/src/lib/notifications/offline-geo-notice.ts b/src/lib/notifications/offline-geo-notice.ts new file mode 100644 index 000000000..5f1e87ebc --- /dev/null +++ b/src/lib/notifications/offline-geo-notice.ts @@ -0,0 +1,139 @@ +/** + * The admin notice for a host whose login-IP lookups leave it: the offline + * GeoLite2 tier is not configured and the online provider is still on. + * + * The notice is once per CONFIGURATION STATE. A module-level latch alone + * re-sent it on every worker boot while the state never changed (issue #851: + * same notice again and again on a host that has simply not mounted the + * databases). The durable anchor is a per-admin `notification_events` claim, + * the same ledger every other repeat-prone notice anchors in, released when + * the state exits, i.e. when the databases are configured or the online + * lookup is disabled. Entering the unconfigured state sends once; staying in + * it is silent across restarts; leaving and re-entering sends again. + * + * When it runs is decided by the resolver in `src/lib/geo.ts`: on the first + * public-IP lookup of a process, and again after a runtime database fetch. + * The resolver does not import this module. It holds a slot that this module + * fills (`registerOfflineGeoNotice`, called once at boot from + * `src/instrumentation.ts`), because the lookup sits under `auditLog`, and an + * import from it into the notification dispatcher closed a loop back to the + * audit log through the dispatcher's own collaborators. + */ +import { + offlineGeoReady, + resolveGeoProviderHost, + setOfflineGeoNotice, +} from "@/lib/geo"; +import { envFlag } from "@/lib/env"; +import { getEvent } from "@/lib/logging/context"; +import { + claimNotificationEvent, + releaseNotificationEvent, + CONFIG_NOTICE_EVENT_TYPE, +} from "@/lib/notifications/reminder-dedup"; + +/** Durable anchor identity for the offline-geo notice in `notification_events`. */ +const OFFLINE_GEO_NOTICE_DEDUP_KEY = "geo:offline-unavailable"; + +/** + * Where the admin notification sends someone who wants the offline tier. + * The in-repo path is the durable one — the runbook ships with the source + * that emits this link, so the two cannot drift apart. + */ +const GEOLITE2_DOCS_URL = + "https://github.com/MBombeck/HealthLog/blob/main/docs/self-hosting/geolite2.md"; + +/** + * Whether the host is in the state the notice describes: the offline tier + * is not configured AND lookups still egress to the online provider. With + * `IP_GEO_LOOKUP_DISABLED=1` there is no egress to warn about, so the + * disabled host counts as having left the state — exactly like a host + * that mounted the databases. + */ +function offlineGeoNoticeStateActive(): boolean { + return !offlineGeoReady() && !envFlag("IP_GEO_LOOKUP_DISABLED"); +} + +/** + * Evaluate the configuration state once: release the anchor when the state + * does not hold, otherwise claim it per admin and send to each admin who did + * not hold it yet. Never throws; the lookup that triggers it sits under a + * fire-and-forget audit write. + */ +export async function evaluateOfflineGeoNotice(): Promise { + // Defer the resolve so we don't pay the cost of pulling the + // Prisma client into the import graph until the first evaluation — + // most calls are cache hits or private-IP short-circuits. + try { + const { prisma } = await import("@/lib/db"); + + if (!offlineGeoNoticeStateActive()) { + // The state the notice describes does not hold. Release any + // persisted sent-anchor so a future re-entry into the unconfigured + // state notifies again — this is the "state exit" edge. + await releaseNotificationEvent(prisma, { + eventType: CONFIG_NOTICE_EVENT_TYPE, + dedupKey: OFFLINE_GEO_NOTICE_DEDUP_KEY, + }); + return; + } + + const { dispatchLocalisedNotification } = + await import("@/lib/notifications/dispatch-localised"); + const admins = await prisma.user.findMany({ + where: { role: "ADMIN" }, + select: { id: true }, + }); + if (admins.length === 0) { + getEvent()?.addWarning( + "geo: offline databases unavailable, no admin user configured to notify", + ); + return; + } + for (const admin of admins) { + // The anchor is unwindowed (`since` at epoch): it holds until the + // state exit above releases it, however many restarts happen in + // between. An admin who already has the anchor claimed nothing and + // hears nothing. + const claimed = await claimNotificationEvent(prisma, { + recordUserId: admin.id, + eventType: CONFIG_NOTICE_EVENT_TYPE, + dedupKey: OFFLINE_GEO_NOTICE_DEDUP_KEY, + since: new Date(0), + }); + if (!claimed) continue; + getEvent()?.addWarning( + `geo: offline databases unavailable, every lookup goes online to ${resolveGeoProviderHost()} — notifying admin`, + ); + await dispatchLocalisedNotification({ + userId: admin.id, + titleKey: "notifications.admin.offlineGeoUnavailableTitle", + messageKey: "notifications.admin.offlineGeoUnavailableBody", + // The message names what the person reading it can actually do. + // It used to point at a licence-key secret in this repository's + // Actions settings, which nobody running the published image can + // set (issue #659) — the reachable path is mounting your own + // databases and pointing GEOLITE2_DIR at them. + params: { + host: resolveGeoProviderHost(), + docsUrl: GEOLITE2_DOCS_URL, + }, + metadata: { + source: "geo-offline-detection", + url: "/admin/system-status", + }, + }); + } + } catch (err) { + // Never let a notification failure propagate — the auth-audit + // caller is fire-and-forget. + getEvent()?.addWarning( + `geo: offline-geo notification failed: ${err instanceof Error ? err.message : String(err)}`, + ); + } +} + +/** Fill the resolver's slot, so its first public-IP lookup evaluates the notice. */ +export function registerOfflineGeoNotice(): void { + setOfflineGeoNotice(evaluateOfflineGeoNotice); +}