mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 05:42:19 +00:00
code-quality: drop dead SESSION_SECRET, add toJson helper, document db-compat, type passkey responses, lock zero-TODO
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 = <T>(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.
This commit is contained in:
1 parent
1768b1d971
commit
999fc257ae
15 files changed
+152
-40
No files matched your search
+2
-5
@@ -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.
|
||||
|
||||
@@ -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."
|
||||
+1
-3
@@ -22,16 +22,14 @@ Edit `.env` and configure at minimum:
|
||||
```
|
||||
POSTGRES_PASSWORD=<base64, 24 bytes>
|
||||
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
|
||||
```
|
||||
|
||||
@@ -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:-}"
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, boolean>,
|
||||
doctorReportPrefsJson: toJson(merged),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -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),
|
||||
},
|
||||
});
|
||||
},
|
||||
|
||||
@@ -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),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -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),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -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<string, boolean> = { 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);
|
||||
});
|
||||
});
|
||||
+15
-7
@@ -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<VerifiedRegistrationResponse> {
|
||||
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(),
|
||||
|
||||
@@ -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<void> | null = null;
|
||||
|
||||
+8
-1
@@ -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 = <T>(v: T) => v as unknown as Prisma.InputJsonValue;
|
||||
@@ -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),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user