From 9bc95713cbece88f05528c9a9e157e10cb1e411f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 06:49:17 +0200 Subject: [PATCH] fix(client): keep server modules out of the browser bundle The side-effect taxonomy imported its enums from the Prisma client entry, and the recurrence engine the medication wizard previews schedules with imported the logging context, which brings node:async_hooks, node:crypto and node:os along. The taxonomy reads the enums entry now, and a rule that does not parse is reported through an observer the server registers, the pattern the source-priority parser already uses. A guard walks the import graph from every client component and fails on a server-only module. --- .../client-bundle-server-import-guard.test.ts | 135 ++++++++++++++++++ src/lib/logging/context.ts | 10 ++ .../recurrence-parse-error-observer.test.ts | 53 +++++++ src/lib/medications/scheduling/recurrence.ts | 25 +++- src/lib/medications/side-effects/taxonomy.ts | 4 +- 5 files changed, 221 insertions(+), 6 deletions(-) create mode 100644 src/__tests__/client-bundle-server-import-guard.test.ts create mode 100644 src/lib/medications/scheduling/__tests__/recurrence-parse-error-observer.test.ts diff --git a/src/__tests__/client-bundle-server-import-guard.test.ts b/src/__tests__/client-bundle-server-import-guard.test.ts new file mode 100644 index 000000000..26c431a99 --- /dev/null +++ b/src/__tests__/client-bundle-server-import-guard.test.ts @@ -0,0 +1,135 @@ +/** + * No client component reaches a server-only module. + * + * A `"use client"` file ships to the browser together with everything it + * imports, transitively. Two modules did reach the server side that way: the + * side-effect taxonomy imported its enums from the Prisma CLIENT entry, and + * the recurrence engine the medication wizard previews schedules with + * imported the logging context, which brings `node:async_hooks`, `node:crypto` + * and `node:os` with it. The build tolerated both; neither belongs in a + * browser bundle, and the next such import could be the database client. + * + * The project has no `server-only` package to mark modules with, so this + * walks the graph instead: from every `"use client"` file, through every + * value import (`import type` is erased and not followed), and fails on any + * reachable import of the database client, the Prisma client entry (the + * `enums` entry is browser-safe and allowed), `next/headers`, the job queue, + * or a Node built-in. The failure prints the chain, so the fix is obvious. + * + * Limits, stated: specifiers are resolved the way the app resolves `@/…` and + * relative paths; a re-export through a package outside `src` is not + * followed. The floors below fail the guard if the walk finds too little to + * mean anything. + */ +import { existsSync, readFileSync, statSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { walkSourceFiles } from "./helpers/source-files"; + +const SRC = join(process.cwd(), "src"); + +const NODE_BUILTIN = + /^(node:|(fs|path|crypto|child_process|net|tls|zlib|stream|os|async_hooks|worker_threads|http|https|dns)(\/|$))/; + +function isServerOnly(spec: string, file: string | null): boolean { + if (file) { + const rel = file.slice(SRC.length + 1); + return rel === "lib/db.ts" || rel.startsWith("generated/prisma/client"); + } + return ( + NODE_BUILTIN.test(spec) || + spec === "next/headers" || + spec === "pg-boss" || + spec === "pg" || + spec.startsWith("@prisma/") + ); +} + +function resolveSpec(spec: string, from: string): string | null { + let base: string; + if (spec.startsWith("@/")) base = join(SRC, spec.slice(2)); + else if (spec.startsWith(".")) base = resolve(dirname(from), spec); + else return null; + for (const c of [ + `${base}.ts`, + `${base}.tsx`, + join(base, "index.ts"), + join(base, "index.tsx"), + ]) { + if (existsSync(c) && statSync(c).isFile()) return c; + } + return null; +} + +/** Value imports only: `import type` and all-`type` named lists are erased. */ +function valueImports(source: string): string[] { + const out: string[] = []; + const re = + /(?:^|\n)\s*(?:import|export)\s+(type\s+)?([\s\S]{0,600}?)\s*from\s*["']([^"']+)["']/g; + for (const m of source.matchAll(re)) { + if (m[1]) continue; + const clause = m[2].trim(); + if (/^\{[^}]*\}$/.test(clause)) { + const names = clause + .replace(/[{}\s]/g, " ") + .split(",") + .map((n) => n.trim()) + .filter(Boolean); + if (names.length > 0 && names.every((n) => n.startsWith("type "))) + continue; + } + out.push(m[3]); + } + for (const m of source.matchAll(/(?:^|\n)\s*import\s+["']([^"']+)["']/g)) + out.push(m[1]); + for (const m of source.matchAll(/import\(\s*["']([^"']+)["']\s*\)/g)) + out.push(m[1]); + return out; +} + +function walkFromClients() { + const files = walkSourceFiles(SRC, { floor: 2000 }) + .filter((f) => !f.startsWith("generated/") && !f.includes("__tests__/")) + .filter((f) => !/\.test\.tsx?$/.test(f)) + .map((f) => join(SRC, f)); + const clients = files.filter((f) => + /^\s*["']use client["']/.test(readFileSync(f, "utf8")), + ); + const parent = new Map(clients.map((c) => [c, null])); + const queue = [...clients]; + const violations: string[] = []; + for (let i = 0; i < queue.length; i++) { + const file = queue[i]; + for (const spec of valueImports(readFileSync(file, "utf8"))) { + const target = resolveSpec(spec, file); + if (isServerOnly(spec, target)) { + const chain = [spec]; + for (let at: string | null = file; at; at = parent.get(at) ?? null) { + chain.unshift(at.slice(SRC.length + 1)); + } + violations.push(chain.join(" -> ")); + continue; + } + if (target && !parent.has(target)) { + parent.set(target, file); + queue.push(target); + } + } + } + return { clients: clients.length, reachable: parent.size, violations }; +} + +describe("client bundle", () => { + const result = walkFromClients(); + + it("walks enough of the tree to mean something", () => { + expect(result.clients).toBeGreaterThan(500); + expect(result.reachable).toBeGreaterThan(900); + }); + + it("reaches no server-only module from a client component", () => { + expect(result.violations).toEqual([]); + }); +}); diff --git a/src/lib/logging/context.ts b/src/lib/logging/context.ts index 13e550918..28fc983fa 100644 --- a/src/lib/logging/context.ts +++ b/src/lib/logging/context.ts @@ -2,6 +2,7 @@ import { AsyncLocalStorage } from "node:async_hooks"; import type { WideEvent } from "./types"; import { WideEventBuilder } from "./event-builder"; import { registerSourcePriorityParseObserver } from "@/lib/validations/source-priority"; +import { registerRecurrenceParseErrorObserver } from "@/lib/medications/scheduling/recurrence"; import { DATETIME_NO_SECONDS_COUNTER, registerDatetimeNoSecondsObserver, @@ -42,6 +43,15 @@ registerSourcePriorityParseObserver((fields) => { annotate(fields); }); +// A medication recurrence rule that does not parse. Same bundle-safe +// registration: the scheduling module also runs in the browser. +registerRecurrenceParseErrorObserver((rrule) => { + annotate({ + action: { name: "medication.recurrence.parse_error" }, + meta: { rrule }, + }); +}); + // v1.43 — count every request instant refused for lacking seconds, per wide // event, so a client still on a minute-precision encoder is visible in the // logs (`meta["validation.datetime.no_seconds"]`) before its users report diff --git a/src/lib/medications/scheduling/__tests__/recurrence-parse-error-observer.test.ts b/src/lib/medications/scheduling/__tests__/recurrence-parse-error-observer.test.ts new file mode 100644 index 000000000..83e1667d4 --- /dev/null +++ b/src/lib/medications/scheduling/__tests__/recurrence-parse-error-observer.test.ts @@ -0,0 +1,53 @@ +/** + * A recurrence rule that does not parse still reaches the wide event on the + * server, now that the scheduling module no longer imports the logging + * context (it runs in the browser too). + */ +import { describe, expect, it } from "vitest"; + +import { WideEventBuilder } from "@/lib/logging/event-builder"; +import { eventStorage } from "@/lib/logging/context"; +import { occurrencesBetween } from "../recurrence"; + +describe("recurrence parse error", () => { + it("is annotated on the server's wide event", () => { + const evt = new WideEventBuilder("http"); + const slots = eventStorage.run(evt, () => + occurrencesBetween( + { + id: "sched-1", + rrule: "FREQ=NOPE;BYDAY=XX", + rollingIntervalDays: null, + timesOfDay: ["08:00"], + daysOfWeek: null, + windowStart: "08:00", + windowEnd: "09:00", + reminderGraceMinutes: null, + scheduleType: "SCHEDULED", + cyclicOnWeeks: null, + cyclicOffWeeks: null, + }, + new Date("2026-06-01T00:00:00Z"), + new Date("2026-06-07T23:59:59Z"), + { + timeZone: "Europe/Berlin", + lastIntakeAt: null, + medication: { + id: "med-1", + startsOn: new Date("2026-06-01T00:00:00Z"), + endsOn: null, + oneShot: false, + createdAt: new Date("2026-01-01T00:00:00Z"), + }, + }, + ), + ); + expect(slots).toEqual([]); + const json = evt.toJSON() as { + action?: { name: string }; + meta?: Record; + }; + expect(json.action?.name).toBe("medication.recurrence.parse_error"); + expect(json.meta?.rrule).toBe("FREQ=NOPE;BYDAY=XX"); + }); +}); diff --git a/src/lib/medications/scheduling/recurrence.ts b/src/lib/medications/scheduling/recurrence.ts index 568336b39..091ae2e4c 100644 --- a/src/lib/medications/scheduling/recurrence.ts +++ b/src/lib/medications/scheduling/recurrence.ts @@ -55,7 +55,6 @@ import { RRule } from "rrule"; -import { annotate } from "@/lib/logging/context"; import { parseScheduleRecurrence } from "@/lib/medication-schedule"; import { wallClockInTz } from "@/lib/tz/wall-clock"; import { startOfLocalDayInTz } from "@/lib/tz/local-day"; @@ -63,6 +62,25 @@ import { tzOffsetMinutes } from "@/lib/tz/format"; import { startOfUtcDay } from "@/lib/tz/start-of-utc-day"; import { hhmmToMinutesOrNull } from "@/lib/medications/scheduling/hhmm"; +/** + * Where a recurrence rule that does not parse is reported. + * + * This module runs in the browser too (the medication wizard previews a + * schedule with it), so it must not import the logging context: that pulls + * `node:async_hooks` and the wide-event builder into the client bundle. The + * server wires the observer when the logging context loads, the same + * bundle-safe registration the source-priority parser uses; the browser + * leaves it a no-op. + */ +type RecurrenceParseErrorObserver = (rrule: string) => void; +let recurrenceParseErrorObserver: RecurrenceParseErrorObserver = () => {}; + +export function registerRecurrenceParseErrorObserver( + observer: RecurrenceParseErrorObserver, +): void { + recurrenceParseErrorObserver = observer; +} + /** * v1.7.0 — schedule-type discriminator on the canonical schedule. * SCHEDULED keeps the rrule / rolling / legacy dispatch unchanged. PRN @@ -794,10 +812,7 @@ function expandRrule( try { rule = RRule.fromString(full); } catch { - annotate({ - action: { name: "medication.recurrence.parse_error" }, - meta: { rrule: rruleStr }, - }); + recurrenceParseErrorObserver(rruleStr); return []; } diff --git a/src/lib/medications/side-effects/taxonomy.ts b/src/lib/medications/side-effects/taxonomy.ts index be79f39f5..b50183d22 100644 --- a/src/lib/medications/side-effects/taxonomy.ts +++ b/src/lib/medications/side-effects/taxonomy.ts @@ -28,10 +28,12 @@ * key to a translated label. */ +// The enums entry, not the client: the side-effects form imports this module, +// and the client entry would pull the Prisma runtime into the browser bundle. import { MedicationSideEffectCategory, MedicationSideEffectEntry, -} from "@/generated/prisma/client"; +} from "@/generated/prisma/enums"; export const SIDE_EFFECT_CATEGORIES: Record< MedicationSideEffectEntry,