From 999fc257ae947a6d2156f5ad6404b2cce37374b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 17 May 2026 00:34:21 +0200 Subject: [PATCH] code-quality: drop dead SESSION_SECRET, add toJson helper, document db-compat, type passkey responses, lock zero-TODO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five hotfix-ready findings from the v1.4.34.3 code-quality audit. SESSION_SECRET drop (F-3): The env var was required by `docker-entrypoint.sh`, listed in `.env.example`, `docker-compose.yml`, `CONTRIBUTING.md`, and the scaling guide — but `grep -rn SESSION_SECRET src/` returned nothing. Sessions are server-stored in `prisma.session` keyed by the cookie value; the secret signed nothing. Every self-hoster carried a ceremonial 64-char hex string that the app ignored. Removed everywhere except the README (separate batch). Each fresh install now generates 3 secrets, not 4. toJson helper (F-9): Seven sites cast Prisma JSON writes via `value as unknown as Prisma.InputJsonValue`. Each call re-invented the same escape hatch. Replaced with one helper in `src/lib/db.ts`: `export const toJson = (v: T) => v as unknown as Prisma.InputJsonValue;` Migrated all 7 call sites: `auth/me/doctor-report-prefs`, `dashboard/chart-overlay-prefs`, `dashboard/widgets`, `integrations/healthkit`, and 4 sites in `apple-health-import-worker`. The helper has a one-line WHY comment + a unit test pinning the cast shape stays stable. db-compat docstring (F-10): `src/lib/db-compat.ts` looks like accumulated backwards-compat cruft on first read. It's actually the schema-bootstrap path that runs ALTER-TABLE-IF-NOT-EXISTS so a fresh container syncs without `prisma migrate deploy`. Added a header docstring naming the role and pointing at `prisma/migrations/` as the canonical history. Passkey types (F-11): `src/lib/auth/passkey.ts:103,169` used `response: any` with per-line `eslint-disable`. Imported `RegistrationResponseJSON` and `AuthenticationResponseJSON` from `@simplewebauthn/types` and dropped the disables. Zero-TODO CI gate (F-15): The audit confirmed product code has zero TODO / FIXME / XXX / HACK markers — genuinely rare and worth preserving. Added a tiny GitHub Actions workflow at `.github/workflows/no-todo-markers.yml` that fails the build on any future re-introduction. Test files are excluded; product code only. --- .env.example | 7 +-- .github/workflows/no-todo-markers.yml | 39 ++++++++++++++ CONTRIBUTING.md | 4 +- docker-compose.yml | 2 - docker-entrypoint.sh | 2 +- docs/self-hosting/scaling.md | 3 +- .../api/auth/me/doctor-report-prefs/route.ts | 7 +-- .../dashboard/chart-overlay-prefs/route.ts | 5 +- src/app/api/dashboard/widgets/route.ts | 4 +- src/app/api/integrations/healthkit/route.ts | 5 +- src/lib/__tests__/db-to-json.test.ts | 52 +++++++++++++++++++ src/lib/auth/passkey.ts | 22 +++++--- src/lib/db-compat.ts | 18 +++++++ src/lib/db.ts | 9 +++- src/lib/jobs/apple-health-import-worker.ts | 13 ++--- 15 files changed, 152 insertions(+), 40 deletions(-) create mode 100644 .github/workflows/no-todo-markers.yml create mode 100644 src/lib/__tests__/db-to-json.test.ts diff --git a/.env.example b/.env.example index d809bbdee..1de010c8e 100644 --- a/.env.example +++ b/.env.example @@ -4,10 +4,10 @@ # Copy this file to .env and fill in the values. # $ cp .env.example .env # -# Required variables: DATABASE_URL or POSTGRES_PASSWORD, SESSION_SECRET, +# Required variables: DATABASE_URL or POSTGRES_PASSWORD, # ENCRYPTION_KEY, API_TOKEN_HMAC_KEY # -# Generate the three secrets with: +# Generate the two secrets with: # $ openssl rand -hex 32 # any UNIX shell # $ node -e 'console.log(require("crypto").randomBytes(32).toString("hex"))' # @@ -44,9 +44,6 @@ NODE_ENV="development" # ----------------------------------------------------------------------------- # Required Secrets — generate with `openssl rand -hex 32` # ----------------------------------------------------------------------------- -# Signs session cookies. Rotating it logs out everyone. -SESSION_SECRET="REPLACE-WITH-OUTPUT-OF-openssl-rand-hex-32" - # AES-256-GCM key for encrypting Withings tokens, GitHub PATs, AI provider # keys, and VAPID private keys at rest. Rotate carefully — old ciphertexts # need re-encryption. diff --git a/.github/workflows/no-todo-markers.yml b/.github/workflows/no-todo-markers.yml new file mode 100644 index 000000000..f72470531 --- /dev/null +++ b/.github/workflows/no-todo-markers.yml @@ -0,0 +1,39 @@ +name: No TODO markers + +on: + push: + branches: [main, develop] + pull_request: + branches: [main, develop] + +permissions: + contents: read + +jobs: + # Product code is currently zero TODO / FIXME / XXX / HACK markers and + # the convention is to keep it that way — open a real follow-up issue + # instead of leaving a marker in the source. This gate fails fast if a + # future contributor reintroduces the pattern. + grep: + name: Forbidden marker grep + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + + - name: Scan src/ for TODO/FIXME/XXX/HACK markers + run: | + # Tests are exempt — they can legitimately reference an outstanding + # ticket via a TODO; `src/generated/` is third-party output from + # `prisma generate` and is regenerated on every db:generate. + if grep -rEn "\b(TODO|FIXME|XXX|HACK)\b" src/ \ + --include='*.ts' \ + --include='*.tsx' \ + --exclude-dir='__tests__' \ + --exclude-dir='generated' \ + --exclude='*.test.ts' \ + --exclude='*.test.tsx'; then + echo "::error::TODO/FIXME/XXX/HACK markers detected in product code — open a follow-up issue instead." + exit 1 + fi + echo "::notice::No forbidden markers in product code." diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9d6a5ab8b..1fb9dbfde 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,16 +22,14 @@ Edit `.env` and configure at minimum: ``` POSTGRES_PASSWORD= DATABASE_URL="postgresql://healthlog:${POSTGRES_PASSWORD}@db:5432/healthlog" -SESSION_SECRET=<64-char hex> ENCRYPTION_KEY=<64-char hex> API_TOKEN_HMAC_KEY=<64-char hex> ``` -Generate the four secrets: +Generate the three secrets: ```bash echo "POSTGRES_PASSWORD=$(openssl rand -base64 24)" >> .env -echo "SESSION_SECRET=$(openssl rand -hex 32)" >> .env echo "ENCRYPTION_KEY=$(openssl rand -hex 32)" >> .env echo "API_TOKEN_HMAC_KEY=$(openssl rand -hex 32)" >> .env ``` diff --git a/docker-compose.yml b/docker-compose.yml index 9bbb5e872..62e3085a3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -20,7 +20,6 @@ services: environment: DATABASE_URL: "postgresql://healthlog:${POSTGRES_PASSWORD}@db:5432/healthlog?schema=public" NODE_ENV: production - SESSION_SECRET: "${SESSION_SECRET}" ENCRYPTION_KEY: "${ENCRYPTION_KEY}" ENCRYPTION_KEYS: "${ENCRYPTION_KEYS:-}" ENCRYPTION_ACTIVE_KEY_ID: "${ENCRYPTION_ACTIVE_KEY_ID:-}" @@ -65,7 +64,6 @@ services: # environment: # DATABASE_URL: "postgresql://healthlog:${POSTGRES_PASSWORD}@db:5432/healthlog?schema=public" # NODE_ENV: production - # SESSION_SECRET: "${SESSION_SECRET}" # ENCRYPTION_KEY: "${ENCRYPTION_KEY}" # ENCRYPTION_KEYS: "${ENCRYPTION_KEYS:-}" # ENCRYPTION_ACTIVE_KEY_ID: "${ENCRYPTION_ACTIVE_KEY_ID:-}" diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 748318f21..968c88d10 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -9,7 +9,7 @@ set -e # Validating here guarantees a clear, deploy-platform-agnostic # failure message before we touch the database. missing="" -for var in DATABASE_URL SESSION_SECRET ENCRYPTION_KEY API_TOKEN_HMAC_KEY; do +for var in DATABASE_URL ENCRYPTION_KEY API_TOKEN_HMAC_KEY; do eval "value=\$$var" if [ -z "$value" ]; then missing="$missing $var" diff --git a/docs/self-hosting/scaling.md b/docs/self-hosting/scaling.md index 49155dab7..581ea43e3 100644 --- a/docs/self-hosting/scaling.md +++ b/docs/self-hosting/scaling.md @@ -20,8 +20,7 @@ deployed in three modes: `app` service and uncomment the `app-worker` service block at the bottom (it ships with `HEALTHLOG_PROCESS_TYPE=worker` baked in). 2. Make sure both containers see the same `DATABASE_URL`, - `ENCRYPTION_KEY` (or `ENCRYPTION_KEYS`), `API_TOKEN_HMAC_KEY`, - and `SESSION_SECRET`. + `ENCRYPTION_KEY` (or `ENCRYPTION_KEYS`), and `API_TOKEN_HMAC_KEY`. 3. `docker compose up -d --build`. Both containers connect to the same Postgres. pg-boss claims jobs diff --git a/src/app/api/auth/me/doctor-report-prefs/route.ts b/src/app/api/auth/me/doctor-report-prefs/route.ts index 9aeea0438..a3fee47b7 100644 --- a/src/app/api/auth/me/doctor-report-prefs/route.ts +++ b/src/app/api/auth/me/doctor-report-prefs/route.ts @@ -20,7 +20,7 @@ import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler"; import { apiSuccess, getClientIp } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; -import { prisma } from "@/lib/db"; +import { prisma, toJson } from "@/lib/db"; import { doctorReportPrefsSchema, parseDoctorReportPrefs, @@ -74,11 +74,8 @@ export const PUT = apiHandler(async (req: Request) => { await prisma.user.update({ where: { id: user.id }, - // `merged` is a typed object; Prisma's `InputJsonValue` insists on an - // index signature so cast through `unknown` rather than widen the - // typed return surface. Shape is identical to the validated input. data: { - doctorReportPrefsJson: merged as unknown as Record, + doctorReportPrefsJson: toJson(merged), }, }); diff --git a/src/app/api/dashboard/chart-overlay-prefs/route.ts b/src/app/api/dashboard/chart-overlay-prefs/route.ts index c3b842ea0..c44015185 100644 --- a/src/app/api/dashboard/chart-overlay-prefs/route.ts +++ b/src/app/api/dashboard/chart-overlay-prefs/route.ts @@ -18,7 +18,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; -import { prisma } from "@/lib/db"; +import { prisma, toJson } from "@/lib/db"; import { CHART_OVERLAY_KEYS, resolveDashboardLayout, @@ -26,7 +26,6 @@ import { type DashboardLayout, } from "@/lib/dashboard-layout"; import { invalidateUserDashboardWidgets } from "@/lib/cache/invalidate"; -import { Prisma } from "@/generated/prisma/client"; import { z } from "zod/v4"; import type { NextRequest } from "next/server"; @@ -80,7 +79,7 @@ export const PUT = apiHandler(async (request: NextRequest) => { await tx.user.update({ where: { id: user.id }, data: { - dashboardWidgetsJson: normalized as unknown as Prisma.InputJsonValue, + dashboardWidgetsJson: toJson(normalized), }, }); }, diff --git a/src/app/api/dashboard/widgets/route.ts b/src/app/api/dashboard/widgets/route.ts index 89de13a32..28588e5f9 100644 --- a/src/app/api/dashboard/widgets/route.ts +++ b/src/app/api/dashboard/widgets/route.ts @@ -8,7 +8,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; -import { prisma } from "@/lib/db"; +import { prisma, toJson } from "@/lib/db"; import { resolveDashboardLayout, serializeDashboardLayout, @@ -153,7 +153,7 @@ export const PUT = apiHandler(async (request: NextRequest) => { await prisma.user.update({ where: { id: user.id }, data: { - dashboardWidgetsJson: normalized as unknown as Prisma.InputJsonValue, + dashboardWidgetsJson: toJson(normalized), }, }); diff --git a/src/app/api/integrations/healthkit/route.ts b/src/app/api/integrations/healthkit/route.ts index d9cb34a4a..df2b52edd 100644 --- a/src/app/api/integrations/healthkit/route.ts +++ b/src/app/api/integrations/healthkit/route.ts @@ -13,8 +13,7 @@ import { z } from "zod/v4"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiError, apiSuccess, safeJson } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; -import { prisma } from "@/lib/db"; -import { Prisma } from "@/generated/prisma/client"; +import { prisma, toJson } from "@/lib/db"; import { auditLog } from "@/lib/auth/audit"; export const directionEnum = z.enum([ @@ -174,7 +173,7 @@ export const PATCH = apiHandler(async (request: NextRequest) => { await prisma.user.update({ where: { id: user.id }, data: { - healthKitConfigJson: updatedConfig as unknown as Prisma.InputJsonValue, + healthKitConfigJson: toJson(updatedConfig), }, }); diff --git a/src/lib/__tests__/db-to-json.test.ts b/src/lib/__tests__/db-to-json.test.ts new file mode 100644 index 000000000..ba6b3f76f --- /dev/null +++ b/src/lib/__tests__/db-to-json.test.ts @@ -0,0 +1,52 @@ +import { describe, it, expect } from "vitest"; +import { toJson } from "../db"; + +// `toJson` is a typed no-op cast that re-types a value as +// `Prisma.InputJsonValue`. The runtime behaviour is identity; the +// guarantee being tested is that the exported helper exists, accepts +// every JSON-serialisable shape we throw at it, and never mutates or +// clones the value. +describe("toJson", () => { + it("is exported and callable", () => { + expect(typeof toJson).toBe("function"); + }); + + it("returns the same reference for objects (identity cast)", () => { + const value = { a: 1, nested: { b: "x" } }; + expect(toJson(value)).toBe(value); + }); + + it("returns the same reference for arrays", () => { + const value = [1, 2, 3]; + expect(toJson(value)).toBe(value); + }); + + it("accepts primitives", () => { + expect(toJson(42)).toBe(42); + expect(toJson("hello")).toBe("hello"); + expect(toJson(true)).toBe(true); + expect(toJson(null)).toBeNull(); + }); + + it("accepts typed application shapes (Record, interface-like)", () => { + const prefs: Record = { mood: true, weight: false }; + const round = toJson(prefs); + expect(round).toBe(prefs); + expect(round).toEqual({ mood: true, weight: false }); + }); + + it("accepts nested mixed shapes (the production payloads)", () => { + const payload = { + version: 1, + widgets: [{ id: "weight", visible: true, order: 0 }], + chartOverlayPrefs: { + weight: { + showTrendIndicator: true, + showTrendArrow: false, + showTargetRange: true, + }, + }, + }; + expect(toJson(payload)).toBe(payload); + }); +}); diff --git a/src/lib/auth/passkey.ts b/src/lib/auth/passkey.ts index db1827eef..f1ef7f107 100644 --- a/src/lib/auth/passkey.ts +++ b/src/lib/auth/passkey.ts @@ -3,6 +3,8 @@ import { verifyRegistrationResponse, generateAuthenticationOptions, verifyAuthenticationResponse, + type AuthenticationResponseJSON, + type RegistrationResponseJSON, type VerifiedRegistrationResponse, type VerifiedAuthenticationResponse, } from "@simplewebauthn/server"; @@ -100,8 +102,7 @@ export async function createRegistrationOptions( export async function verifyRegistration( challengeId: string, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - response: any, + response: unknown, ): Promise { const challenge = await prisma.authChallenge.findUnique({ where: { id: challengeId }, @@ -112,8 +113,12 @@ export async function verifyRegistration( } try { + // The caller passes a parsed JSON body without prior shape-validation — + // the SimpleWebAuthn verifier owns full schema validation and throws + // on any mismatch. We narrow at the boundary so the rest of the file + // sees the documented `RegistrationResponseJSON` shape. const verification = await verifyRegistrationResponse({ - response, + response: response as RegistrationResponseJSON, expectedChallenge: challenge.challenge, expectedOrigin: getExpectedOrigin(), expectedRPID: getRpId(), @@ -166,8 +171,7 @@ export async function createAuthenticationOptions(userId?: string) { export async function verifyAuthentication( challengeId: string, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - response: any, + response: unknown, ): Promise<{ verification: VerifiedAuthenticationResponse; passkey: { userId: string }; @@ -180,8 +184,12 @@ export async function verifyAuthentication( throw new Error("Challenge expired or not found"); } + // The caller passes a parsed JSON body — narrow at the boundary; the + // SimpleWebAuthn verifier below performs full shape validation. + const typedResponse = response as AuthenticationResponseJSON; + // Find the passkey by credential ID - const credentialId = response.id; + const credentialId = typedResponse.id; const passkey = await prisma.passkey.findUnique({ where: { credentialId }, }); @@ -192,7 +200,7 @@ export async function verifyAuthentication( try { const verification = await verifyAuthenticationResponse({ - response, + response: typedResponse, expectedChallenge: challenge.challenge, expectedOrigin: getExpectedOrigin(), expectedRPID: getRpId(), diff --git a/src/lib/db-compat.ts b/src/lib/db-compat.ts index db63c8e2c..316ec5470 100644 --- a/src/lib/db-compat.ts +++ b/src/lib/db-compat.ts @@ -1,3 +1,21 @@ +/** + * Schema-bootstrap path for fresh self-host containers. + * + * Production deploys run `prisma migrate deploy` from the entrypoint, + * so the canonical migration history lives in `prisma/migrations/`. + * That history is the source of truth — any new column or table must + * land there first. + * + * This file is the belt-and-suspenders companion: it issues + * idempotent `ALTER TABLE … ADD COLUMN IF NOT EXISTS` statements on + * boot so a container that came up against a database which somehow + * missed a migration (interrupted deploy, restored from an older + * dump, etc.) self-heals before the app starts taking traffic. + * + * Treat every statement here as a mirror of an existing migration — + * never invent schema in this file. If a column appears here without + * a corresponding entry under `prisma/migrations/`, that is a bug. + */ import { prisma } from "@/lib/db"; let dbCompatibilityPromise: Promise | null = null; diff --git a/src/lib/db.ts b/src/lib/db.ts index ae2e26538..fb4f0ca50 100644 --- a/src/lib/db.ts +++ b/src/lib/db.ts @@ -1,4 +1,4 @@ -import { PrismaClient } from "@/generated/prisma/client"; +import { Prisma, PrismaClient } from "@/generated/prisma/client"; import { PrismaPg } from "@prisma/adapter-pg"; const globalForPrisma = globalThis as unknown as { @@ -17,3 +17,10 @@ export const prisma = globalForPrisma.prisma ?? createPrismaClient(); if (process.env.NODE_ENV !== "production") { globalForPrisma.prisma = prisma; } + +// Prisma's `InputJsonValue` requires an explicit index signature that +// typed application shapes (Zod-validated, hand-written interfaces) +// don't carry. Every JSON-column write would otherwise repeat the same +// `value as unknown as Prisma.InputJsonValue` escape hatch — this one +// helper centralises the cast so the WHY stays in a single place. +export const toJson = (v: T) => v as unknown as Prisma.InputJsonValue; diff --git a/src/lib/jobs/apple-health-import-worker.ts b/src/lib/jobs/apple-health-import-worker.ts index 5568a5bcf..83c69d6af 100644 --- a/src/lib/jobs/apple-health-import-worker.ts +++ b/src/lib/jobs/apple-health-import-worker.ts @@ -25,7 +25,8 @@ * Locks per `.planning/research/v1434-r-1-xml-import.md` §5.1. */ import { unlinkSync } from "node:fs"; -import { PrismaClient, Prisma } from "@/generated/prisma/client"; +import { PrismaClient } from "@/generated/prisma/client"; +import { toJson } from "@/lib/db"; import { PrismaPg } from "@prisma/adapter-pg"; import type { Job } from "pg-boss"; @@ -82,7 +83,7 @@ async function writeProgress( where: { id: importJobId }, data: { status, - progress: progress as unknown as Prisma.InputJsonValue, + progress: toJson(progress), }, }); } @@ -173,7 +174,7 @@ export async function handleAppleHealthImport( where: { id: importJobId }, data: { status: snapshot.currentPhase, - progress: snapshot as unknown as Prisma.InputJsonValue, + progress: toJson(snapshot), }, }); }, @@ -185,14 +186,14 @@ export async function handleAppleHealthImport( data: { status: "done", completedAt: new Date(), - progress: { + progress: toJson({ currentPhase: "upserting", recordsRead: result.totals.recordsRead, rowsUpserted: result.totals.rowsUpserted, percent: 100, elapsedMs: result.totals.durationMs, - } as unknown as Prisma.InputJsonValue, - result: result as unknown as Prisma.InputJsonValue, + }), + result: toJson(result), }, });