From c986efab30a9bbe2e161a679f10b2a53b6b04fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 27 Aug 2026 12:18:34 +0200 Subject: [PATCH] Correct operator runbooks against the current code - attempt-number-semantics: name the real bucket column (consecutiveFailuresByKind) and drop a drifted line citation - backup-restore: show the smoke-test response inside the standard { data, error } envelope - deploy: the manual migration step now uses the bundled Prisma CLI via node (the image strips pnpm); WHOOP vars are already on the shipped compose whitelist - encryption-key-rotation: point at the encrypted-column registry instead of a five-model list several releases behind - env-check: the CI gate shipped (env-check.yml); the partial-APNs example now shows the synthetic all-or-none row and the non-zero exit - password-reset: the admin API reset route exists; only the self-service route does not - tls-cert-pin: stop asserting client-repo pin-set details that cannot be audited from this repository - drop v141-followup-issues.md: a v1.4.1 punch list whose items are fixed, removed, or superseded; keeping it under docs/ops invites re-fixing shipped work --- docs/ops/attempt-number-semantics.md | 6 +- docs/ops/backup-restore.md | 15 +- docs/ops/deploy.md | 11 +- docs/ops/encryption-key-rotation.md | 9 +- docs/ops/env-check.md | 31 ++-- docs/ops/password-reset.md | 10 +- docs/ops/tls-cert-pin.md | 21 ++- docs/ops/v141-followup-issues.md | 260 --------------------------- 8 files changed, 63 insertions(+), 300 deletions(-) delete mode 100644 docs/ops/v141-followup-issues.md diff --git a/docs/ops/attempt-number-semantics.md b/docs/ops/attempt-number-semantics.md index 59f878944..b7b9ff5f8 100644 --- a/docs/ops/attempt-number-semantics.md +++ b/docs/ops/attempt-number-semantics.md @@ -17,7 +17,8 @@ that captures how many failures the bucket ledger has accumulated since the last success. The persistent-failure alert dispatcher in -`src/lib/integrations/status.ts:453` uses `Math.max(...buckets)` — the +`src/lib/integrations/status.ts` (the v1.4.43 W14 block) uses +`Math.max(...Object.values(buckets))` — the **max** across the three buckets. A 3-deep persistent streak pages even when the transient bucket is empty, and a flapping integration that ping-pongs between transient and reauth_required never trips a @@ -44,7 +45,8 @@ we want to wake someone up for. If you see an audit row with `attemptNumber >= threshold` but no alert was dispatched, check the per-bucket counts on -`IntegrationStatus.failureBuckets` for that user/integration pair. The +`IntegrationStatus.consecutiveFailuresByKind` (DB column +`consecutive_failures_by_kind`) for that user/integration pair. The sum may be high while every individual bucket sits below the threshold. This is working as designed; manually investigate only if a single bucket is at or above the threshold and `alertedAt` is unset (broken diff --git a/docs/ops/backup-restore.md b/docs/ops/backup-restore.md index 33fddad56..b465f2b5d 100644 --- a/docs/ops/backup-restore.md +++ b/docs/ops/backup-restore.md @@ -54,12 +54,15 @@ performs a 1-byte PUT + GET round-trip and returns: ```json { - "endpoint": "https://...r2.cloudflarestorage.com", - "bucket": "healthlog-backups", - "region": "auto", - "putLatencyMs": 142, - "getLatencyMs": 38, - "ok": true + "data": { + "endpoint": "https://...r2.cloudflarestorage.com", + "bucket": "healthlog-backups", + "region": "auto", + "putLatencyMs": 142, + "getLatencyMs": 38, + "ok": true + }, + "error": null } ``` diff --git a/docs/ops/deploy.md b/docs/ops/deploy.md index 57c3fd31e..f52e3c927 100644 --- a/docs/ops/deploy.md +++ b/docs/ops/deploy.md @@ -16,10 +16,12 @@ canonical command sequence on a single-instance host: # 1. Pull the new image. docker pull ghcr.io/mbombeck/healthlog:vX.Y.Z -# 2. Run migrations against the live DB using the new image. +# 2. Run migrations against the live DB using the new image. The +# production image strips pnpm/npm; use the entrypoint's node +# invocation of the bundled Prisma CLI instead. docker run --rm --env-file .env.production \ ghcr.io/mbombeck/healthlog:vX.Y.Z \ - pnpm prisma migrate deploy + sh -c 'NODE_PATH=/opt/prisma-cli/node_modules node /opt/prisma-cli/node_modules/prisma/build/index.js migrate deploy' # 3. Recreate the app container so it picks up the new image. docker compose up -d app @@ -40,8 +42,9 @@ them in order on boot like any other. WHOOP introduces two optional instance env vars, `WHOOP_REDIRECT_URI` and `WHOOP_WEBHOOK_SECRET` (HMAC secret for WHOOP webhook signature verification). Per-user WHOOP client id/secret live in Settings, not -in the environment. See `docs/integrations/whoop.md`. Add the two vars -to the compose `environment:` whitelist if you set them — vars not on +in the environment. See `docs/integrations/whoop.md`. The shipped +`docker-compose.yml` already whitelists both under `environment:`; only +a forked or older compose file needs them added by hand — vars not on the whitelist never reach the container. ### What goes wrong if you reverse it diff --git a/docs/ops/encryption-key-rotation.md b/docs/ops/encryption-key-rotation.md index bb236e8d3..7a06fe83f 100644 --- a/docs/ops/encryption-key-rotation.md +++ b/docs/ops/encryption-key-rotation.md @@ -47,8 +47,13 @@ under `v1` (the synthetic id assigned to the existing `ENCRYPTION_KEY`). pnpm dlx tsx scripts/rotate-encryption-key.ts v2 ``` The script is idempotent — running it again is a no-op for rows already - prefixed with `v2.`. It rotates User, WithingsConnection, AppSettings, - NotificationChannel, and PushSubscription rows. The summary line at the + prefixed with `v2.`. It rotates every registered encrypted column across + all models — the canonical registry lives in + `src/lib/crypto/encrypted-columns.ts`, and a guard test fails CI if any + `*Encrypted` column in the schema is missing from the registry or not + referenced by the script — plus the two non-`*Encrypted` members: + `IntegrationStatus.lastError` and `CoachMessage.encryptedContent` + (Bytes). The summary line at the end shows `scanned`, `rotated`, and `errors` per table+field; treat `errors > 0` as a hard failure and re-run after fixing the cause. 4. **Drop the old key (optional, recommended).** Once the script has run diff --git a/docs/ops/env-check.md b/docs/ops/env-check.md index 2e186ca46..af9b3e81a 100644 --- a/docs/ops/env-check.md +++ b/docs/ops/env-check.md @@ -46,8 +46,10 @@ states: | `[missing-optional]` | Variable is optional. The script reports it but does not fail. | The grep-friendly format lets a CI pipeline do `grep -q -'\[MISSING-REQUIRED\]'` to gate a deploy. The CI integration itself -is deferred to v1.4.43 — this release ships the CLI tool only. +'\[MISSING-REQUIRED\]'` to gate a deploy. CI already does: the +`env-check.yml` workflow runs `pnpm check-env --file +.env.production.example` on every PR and fails on missing required +vars. ## All-or-none groups @@ -65,11 +67,15 @@ Example output for a partial APNs config: [OK] APNS_TEAM_ID [OK] APNS_BUNDLE_ID [missing-optional] APNS_KEY — Satisfied by any of: APNS_KEY, APNS_KEY_FILE + [MISSING-REQUIRED] — Group is all-or-none but only 3/4 variables are set. Either set all or unset all. ``` -A reviewer immediately sees that `APNS_KEY` (or its `APNS_KEY_FILE` -alternative) is missing while the other three APNs vars are set — the -exact configuration that silently disables iOS push. +The APNs group is `allOrNone: true`, so the partial config also emits +the synthetic `` row at REQUIRED severity and the run +exits 1. A reviewer immediately sees that `APNS_KEY` (or its +`APNS_KEY_FILE` alternative) is missing while the other three APNs +vars are set — the exact configuration that silently disables iOS +push. ## Editing the manifest @@ -88,12 +94,13 @@ so changes go through a PR. To add a new variable: if the manifest change introduces a new classification branch (anyOf, allOrNone, …). -## Future: CI gate +## CI gate -Deferred to v1.4.43: +The GitHub Actions workflow `.github/workflows/env-check.yml` runs +`pnpm check-env --file .env.production.example` on every PR, blocking +changes that introduce new required vars without updating the example +file. -- GitHub Actions workflow that runs `pnpm check-env --file -.env.production.example` on every PR, blocking changes that - introduce new required vars without updating the example file. -- Coolify pre-deploy hook that runs `pnpm check-env` inside the - target container and aborts the deploy on exit code 1. +Still an idea only: a Coolify pre-deploy hook that runs +`pnpm check-env` inside the target container and aborts the deploy on +exit code 1. diff --git a/docs/ops/password-reset.md b/docs/ops/password-reset.md index 8f6f29f72..4ad1eace2 100644 --- a/docs/ops/password-reset.md +++ b/docs/ops/password-reset.md @@ -2,9 +2,13 @@ A self-hosted operator can reset a single user's password from inside the running container. This is an **operator-only** maintenance path — there is no -user-facing or API route for it, and it bypasses the normal account-recovery -flow. Use it only for an account you administer (for example, after a forgotten -password on a single-user instance). +self-service reset route for users, and it bypasses the normal +account-recovery flow. Admins do have an in-app alternative: +`POST /api/admin/users/{id}/reset-password` (cookie-admin only) sets a new +password and destroys the user's sessions, tokens, and trusted devices. The +CLI remains the path when no admin can log in. Use it only for an account you +administer (for example, after a forgotten password on a single-user +instance). > **Operator-only.** The script writes directly to the `users` table and is > intended to be run by the host operator with database access. Treat the new diff --git a/docs/ops/tls-cert-pin.md b/docs/ops/tls-cert-pin.md index fdb9cf293..f521c8cac 100644 --- a/docs/ops/tls-cert-pin.md +++ b/docs/ops/tls-cert-pin.md @@ -1,11 +1,10 @@ # TLS SPKI pinning — coupling, alarm, and re-pin runbook -The native client SPKI-pins the server's TLS chain at the **CA level**. It -ships a small pin set — currently the issuing intermediate plus the root — -and accepts a connection only when the served chain passes baseline X.509 -evaluation AND at least one certificate anywhere in the chain matches a pin -(any-match). The leaf is deliberately **not** pinned, so routine leaf -renewals are a client no-op. The outage risk is a **chain change**: the +The native client SPKI-pins the server's TLS chain at the **CA level**: it +ships a small pin set and rejects a connection whose validated chain +contains no pinned key. The leaf is deliberately **not** pinned, so routine +leaf renewals are a client no-op. The pin set itself and its enforcement +rules live in the client repository, not here. The outage risk is a **chain change**: the served chain stops terminating in the pinned CA keys — the server moves to a different CA, or the CA rotates its intermediate / root keys. This page explains the coupling, the server-side alarm that watches the served leaf, @@ -15,11 +14,11 @@ and the operator / release-owner steps when it fires. A SPKI pin is `base64(sha256(DER subjectPublicKeyInfo))` — a hash of a certificate's public key, not of the whole certificate. The native client -carries a set of these pins (build-time config, ≥ 2 well-formed pins -enforced for release builds) and rejects any TLS connection whose validated -chain contains no pinned key. Hosts outside the client's pinned-host list -fall back to plain system-trust validation, which is the documented model -for self-hosted instances that ship their own chain. +carries a set of these pins in its build-time config and rejects any TLS +connection whose validated chain contains no pinned key; the exact pin-set +rules are the client repository's contract. Self-hosted instances that ship +their own chain sit outside the pinned-host list and validate against +system trust. The app host's certificate auto-renews roughly every 90 days and each renewal mints a new leaf keypair. Because the client pins the CA keys rather diff --git a/docs/ops/v141-followup-issues.md b/docs/ops/v141-followup-issues.md deleted file mode 100644 index ec5d22eb6..000000000 --- a/docs/ops/v141-followup-issues.md +++ /dev/null @@ -1,260 +0,0 @@ -# v1.4.x follow-up issues - -Items identified by the v1.4.1 audit pass that warrant deeper work -than a hardening release should land. Each one carries enough context -that a future contributor can pick it up without re-running the audit. - -## Security — deferred - -### S-FOLLOW-1 — Idempotency-Key concurrent duplicate side-effects (HIGH) - -Audit reference: `~/infra/reports/v14-security-audit.md` H1. - -The `withIdempotency()` wrapper currently does -`findCached → handler → persistCached`. Two requests with the same -`Idempotency-Key` arriving in parallel both miss the SELECT, both -execute the handler (duplicate measurements / intake events), and -one INSERT loses on the unique constraint silently. The repo-doc -contract ("retries with the same Idempotency-Key … replay the -original response — no second side-effect") is therefore violated -whenever a flaky mobile client double-sends. - -**Fix shape**: acquire the slot before the handler runs. -`INSERT … ON CONFLICT DO NOTHING RETURNING id` reserving the -`(userId, key, method, path)` row with a sentinel -`responseStatus = 0`, then UPDATE on completion; concurrent losers -SELECT-loop with backoff for the row to be filled. Or wrap the -whole request in a Postgres advisory lock keyed by hash(userId, key) -via `pg_advisory_xact_lock`. - -Why deferred: needs a careful integration test against the new -testcontainers suite (two concurrent inserts → exactly one handler -invocation) and a public-API contract check that the -sentinel-status row never bleeds into a 2xx replay. Fits a v1.4.2 -PR scope on its own. - ---- - -### S-FOLLOW-2 — Encryption key fallback NODE_ENV gate (MEDIUM) - -Audit reference: `~/infra/reports/v14-security-audit.md` M1. - -`src/lib/crypto.ts:61-74` only requires the 64-hex production key -when `process.env.NODE_ENV === "production"`. A fresh Docker run -that forgets to set `NODE_ENV=production` (raw `node …` invocations, -some operator setups) silently accepts a 32-character key and pads -it deterministically with a SHA-256 of itself, halving the entropy -and making the resulting key recoverable from any leaked partial. - -**Fix shape**: invert the gate to `NODE_ENV !== "development" && -NODE_ENV !== "test"` — fail-closed by default — or require an -explicit `ALLOW_WEAK_DEV_KEYS=1` opt-in. - -Why deferred: needs a migration note for self-hosters whose `.env` -files lack `NODE_ENV` (default Coolify deploys do set it; bespoke -setups may not). Worth a release note + .env.example call-out. - ---- - -### S-FOLLOW-3 — Refresh-token reuse-detection serialisation (MEDIUM) - -Audit reference: `~/infra/reports/v14-security-audit.md` M2. - -`src/lib/auth/refresh-token.ts` reuse-detection is a `findUnique` -followed by a non-transactional `findMany + updateMany`. Concurrent -first-time refreshes can briefly have both clients holding -unrevoked access tokens before the loser's are revoked, and a -parallel reuse attempt doubles the audit-log noise. - -**Fix shape**: wrap the rotation in a single `prisma.$transaction` -with `Serializable` isolation, OR reorder to "claim first, mint -second": `prisma.refreshToken.update({ where: { id, usedAt: null } })` -fails atomically if the row was already consumed; only after the -update succeeds, call `issueAccessAndRefresh`. - -Why deferred: needs an integration test that fires N parallel -refreshes for the same token and asserts exactly one new pair is -issued. The testcontainers suite is the right home; v1.4.2 PR. - ---- - -### S-FOLLOW-4 — moodLog webhook HMAC lookup column (MEDIUM) - -Audit reference: `~/infra/reports/v14-security-audit.md` M3. - -`src/app/api/integrations/moodlog/webhook/route.ts` performs an -O(n) AES-GCM decrypt sweep across all enabled users for every -webhook hit. Side-channels: timing oracle for ordering, DoS -amplification (30/min IP rate-limit × n decrypts). - -**Fix shape**: add a `moodLogWebhookSecretLookupHash` column on -`User` (HMAC-SHA-256 keyed by `API_TOKEN_HMAC_KEY`, populated -alongside the encrypted secret on every write). Webhook does a -single indexed `findUnique` on the lookup hash, bypassing the -candidate iteration. The encryption stays as defence-in-depth. - -Why deferred: schema migration required. Backfill needs a one-off -script that decrypts every existing row and writes the lookup -hash. Owners need to approve the migration window. v1.4.2 or -v1.5. - ---- - -### S-FOLLOW-5 — moodLog `readMoodLogSecret` legacy fallback (MEDIUM) - -Audit reference: `~/infra/reports/v14-security-audit.md` M4. - -If `decrypt()` throws inside `src/lib/moodlog-secret.ts:27-37`, the -function returns the stored value as-is. Intent: legacy plaintext -rows keep working. Side effect: an attacker with read access to -the DB obtains the ciphertext blob and can submit it as the literal -webhook secret. - -**Fix shape**: detect "looks like a v1.4 envelope" -(`/^[A-Za-z0-9_-]{1,32}\..+$/`) and refuse the legacy fallback for -envelope-shaped values. Log a metric so the rotation-on-write -contract can be verified to drain. - -Why deferred: needs a small data-audit run to confirm no production -row has a non-envelope value before the strict check goes live. - ---- - -### S-FOLLOW-6 — Restore script writes decrypted JSON with default permissions (MEDIUM) - -Audit reference: `~/infra/reports/v14-security-audit.md` M5. - -`scripts/restore-backup.ts:73-122` writes the decrypted JSON via -`writeFileSync(out, plaintext, "utf8")` with default mode (0644 -typical). The file contains decrypted PHI for the restored user. - -**Fix shape**: -`writeFileSync(out, plaintext, { encoding: "utf8", mode: 0o600 })`. -Optional: warn if `out` is in a world-readable directory. - -Why deferred: tiny one-line fix; bundling with the next ops-script -update keeps the PR coherent. - ---- - -## Performance — deferred - -### P-FOLLOW-1 — Recharts top-level import on `/insights` (P0) - -Audit reference: `~/infra/reports/v14-performance-audit.md` P0. - -`src/app/insights/page.tsx:42-50` static-imports -`{ ScatterChart, Scatter, XAxis, … } from "recharts"` even though -`HealthChart` and `MoodChart` 8 lines above are already wrapped in -`dynamic(…, { ssr: false })`. The static import pulls the whole -recharts bundle into the route's first JS payload, defeating the -dynamic split. - -**Fix shape**: extract the BP-mood scatter into -`src/components/charts/bp-mood-scatter.tsx` and import it via -`dynamic(…, { ssr: false })` like the others. - -Why deferred: trivial mechanical change but needs a Lighthouse -before/after to quantify the bundle-size win for the v1.4.2 -release notes. - ---- - -### P-FOLLOW-2 — `/api/insights/targets` unbounded glucose history scan (P0) - -Audit reference: `~/infra/reports/v14-performance-audit.md` P0. - -`src/app/api/insights/targets/route.ts:600-604` pulls every glucose -measurement the user ever logged (no `gte: thirtyDaysAgo`) and -filters in JS. Diabetic users with multi-year history will -materialise tens of thousands of rows on every targets fetch. - -**Fix shape**: split into "latest per context" -(`distinct: ["glucoseContext"], orderBy: { measuredAt: "desc" }`) -and a `measuredAt: { gte: thirtyDaysAgo }` window query, mirroring -the pattern at lines 102-121 for `latestEverByType`. - -Why deferred: needs a per-context test asserting the new query -returns the same payload shape. Pairs nicely with the MoodEntry -index migration (P-FOLLOW-3). - ---- - -### P-FOLLOW-3 — `MoodEntry @@index` is on `(userId, date)` not `(userId, moodLoggedAt)` (P0) - -Audit reference: `~/infra/reports/v14-performance-audit.md` P0. - -`prisma/schema.prisma:438` declares `@@index([userId, date])`, but -active queries sort/filter by `moodLoggedAt`: - -- `src/app/api/insights/targets/route.ts:495` -- `src/app/api/insights/comprehensive/route.ts:52` -- `src/app/api/export/route.ts:87` - -The unique constraint `@@unique([userId, date, moodLoggedAt])` is -not selectable by Postgres for windowed `moodLoggedAt`-range reads. - -**Fix shape**: add `@@index([userId, moodLoggedAt])` (do NOT drop -the existing index — the unique still serves the moodlog webhook -upsert path). New migration `0026_moodentry_moodloggedat_index`. - -Why deferred: schema migration. Owner approval for the migration -window. - ---- - -### P-FOLLOW-4 to -7 — P1 / P2 perf items - -- N+1 count() per medication on admin reminder-check - (`src/app/api/admin/notifications/reminder-check/route.ts:84`). - Replace with `groupBy` on `medicationIntakeEvent`. -- `/api/analytics` 8-parallel full-history scan - (`src/app/api/analytics/route.ts:20-40`). Single `findMany` with - `type: { in: types }` and JS-side group. -- `/api/import` sequential `prisma.measurement.create` per row - (`src/app/api/import/route.ts:77-96`). Replace with - `createMany({ skipDuplicates: true })`. -- `pairByTimestamp` not used in `/api/analytics` BP correlation - (`src/app/api/analytics/route.ts:80-86`). Replace the quadratic - loop with the helper from `@/lib/analytics/correlations`. - ---- - -## Test-quality polish (5 minor items) - -Audit reference: `~/infra/reports/v14-test-theatre-audit.md`. - -- `src/app/api/gamification/__tests__/ios-format.test.ts` L111-112: - upgrade `toBeDefined()` to shape predicates. -- `src/lib/analytics/__tests__/classifications.test.ts` L169-170: - add `toMatch(/60/)` body-content assertion to compliance-alert tests. -- `src/lib/__tests__/idempotency.test.ts` L145: upgrade bare - `toHaveBeenCalled()` to `toHaveBeenCalledWith({ where: …key… })`. -- `src/app/api/telegram/webhook/__tests__/route.test.ts` L457: same - shape — upgrade or drop. - ---- - -## i18n hardcoded strings - -Audit reference: `~/infra/reports/v14-i18n-drift-audit.md`. - -35+ hardcoded strings in `src/components/settings/ai-section.tsx` -need to be moved into `messages/{en,de}.json` keys. The settings -section was extracted in #143 with strings inline as a known -follow-up; this is the cleanup PR. - -Why deferred: low blast-radius (only the AI provider settings page -shows English to a German user; the dashboard, dialogs, and -critical UX paths all flow through `t(…)`). v1.4.x or v1.5. - ---- - -## Notes - -These are the items the v1.4.1 audit identified that the hardening -release deliberately did NOT take. The audit also found 0 hallucinated -medical claims, 0 i18n key-parity gaps, and the test theatre rate -across the entire codebase was low (5 minor patterns). The codebase -is in healthy shape; this list is the punch-list for the next -proactive iteration, not a backlog of bugs we know about and ignore.