diff --git a/CHANGELOG.md b/CHANGELOG.md index 94c58a823..3a003e116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## [Unreleased] +## [1.28.48] — 2026-07-16 — Security follow-ups + +Two fixes from the campaign's security re-verification: + +- On a brand-new instance with single-sign-on configured, two people signing in + for the very first time at the same moment could both be made admin. The + first-admin selection now takes the same lock as password registration, so + exactly one admin is created no matter which sign-in path or how concurrent. +- A background nutrient-import error could echo a raw database message back to + the client; it now returns a fixed status and logs the detail server-side only. + ## [1.28.47] — 2026-07-16 — Job queue engine update (pg-boss 12.26) Updates the background-job engine (pg-boss) from 12.20 to 12.26, which adds diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 94c971160..e0a47af35 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: HealthLog API - version: 1.28.47 + version: 1.28.48 description: >- Self-hosted personal-health-tracking PWA — public API surface for the iOS native client and external ingest. diff --git a/package.json b/package.json index 32751e6aa..8bd4cbbee 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.28.47", + "version": "1.28.48", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "PolyForm-Noncommercial-1.0.0", "homepage": "https://healthlog.dev", diff --git a/public/sw.js b/public/sw.js index b9a150cbe..61291c47b 100644 --- a/public/sw.js +++ b/public/sw.js @@ -36,7 +36,7 @@ try { // v1.4.38.4 → v1.4.42. Do not hand-edit; bump `package.json` and rebuild. const CACHE_VERSION = (typeof self !== "undefined" && self.__APP_VERSION__) || - /* @sw-version-fallback */ "v1.28.47"; + /* @sw-version-fallback */ "v1.28.48"; const STATIC_CACHE = `healthlog-static-${CACHE_VERSION}`; const PAGE_CACHE = `healthlog-pages-${CACHE_VERSION}`; // v1.18.6 — read-only data cache for a curated allowlist of safe GET `/api/*` diff --git a/src/app/api/auth/oidc/callback/__tests__/route.test.ts b/src/app/api/auth/oidc/callback/__tests__/route.test.ts index fa4dec016..b221295ce 100644 --- a/src/app/api/auth/oidc/callback/__tests__/route.test.ts +++ b/src/app/api/auth/oidc/callback/__tests__/route.test.ts @@ -29,6 +29,11 @@ vi.mock("@/lib/db", () => ({ }, appSettings: { findUnique: vi.fn() }, webauthnMfaCredential: { count: vi.fn() }, + // v1.28.48 — the first-admin provision count+insert now runs inside a + // transaction-scoped advisory lock. Default `$transaction` to run the + // callback with the same prisma mock as its `tx` handle. + $transaction: vi.fn(), + $queryRaw: vi.fn(), }, })); @@ -179,6 +184,12 @@ beforeEach(() => { ticket: "mfa-ticket-1", expiresAt: new Date(Date.now() + 5 * 60 * 1000), }); + // Run the provision transaction callback against the same prisma mock as + // its `tx` handle, and let the advisory-lock `$queryRaw` resolve. + vi.mocked(prisma.$transaction).mockImplementation((( + fn: (tx: typeof prisma) => unknown, + ) => fn(prisma)) as never); + vi.mocked(prisma.$queryRaw).mockResolvedValue([{ locked: 1 }] as never); }); describe("GET /api/auth/oidc/callback", () => { @@ -370,6 +381,90 @@ describe("GET /api/auth/oidc/callback", () => { expect(res.status).toBe(307); }); + it("mints ADMIN for a genuine first OIDC user (in-lock count 0)", async () => { + mockIdentity({ + sub: "sub-first", + email: "first@example.com", + emailVerified: true, + }); + mockUserLookups({ byIdentity: null, byEmail: null }); + // Both the outer registration gate and the in-lock re-count observe an + // empty DB → the provisioned account is the operator's first ADMIN. + vi.mocked(prisma.user.count).mockResolvedValue(0); + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + registrationEnabled: true, + } as never); + vi.mocked(prisma.user.create).mockResolvedValue( + userRow({ id: "user-new", onboardingCompletedAt: null }) as never, + ); + + const res = await GET(validRequest()); + + const createArgs = vi.mocked(prisma.user.create).mock.calls[0]?.[0] as { + data: { role: string }; + }; + expect(createArgs.data.role).toBe("ADMIN"); + expect(createSession).toHaveBeenCalled(); + expect(res.status).toBe(307); + }); + + it("mints USER when a concurrent first user committed first (stale outer read 0, in-lock re-count 1)", async () => { + // The outer read observed the empty-DB window (0), but by the time the + // advisory-locked transaction re-counts, the racing first provision has + // committed → 1. Without the in-lock re-count this second user would also + // be minted ADMIN (the double-admin bug); the fresh count closes it. The + // shared lock key also serialises this against a racing password register. + mockIdentity({ + sub: "sub-second", + email: "second@example.com", + emailVerified: true, + }); + mockUserLookups({ byIdentity: null, byEmail: null }); + vi.mocked(prisma.user.count) + .mockResolvedValueOnce(0) // outer registrationEnabled gate + .mockResolvedValueOnce(1); // inside the advisory-locked transaction + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + registrationEnabled: true, + } as never); + vi.mocked(prisma.user.create).mockResolvedValue( + userRow({ id: "user-new", onboardingCompletedAt: null }) as never, + ); + + const res = await GET(validRequest()); + + const createArgs = vi.mocked(prisma.user.create).mock.calls[0]?.[0] as { + data: { role: string }; + }; + expect(createArgs.data.role).toBe("USER"); + expect(res.status).toBe(307); + }); + + it("acquires the transaction-scoped advisory lock before provisioning", async () => { + mockIdentity({ + sub: "sub-lock", + email: "lock@example.com", + emailVerified: true, + }); + mockUserLookups({ byIdentity: null, byEmail: null }); + vi.mocked(prisma.user.count).mockResolvedValue(0); + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + registrationEnabled: true, + } as never); + vi.mocked(prisma.user.create).mockResolvedValue( + userRow({ id: "user-new", onboardingCompletedAt: null }) as never, + ); + + await GET(validRequest()); + + // The lock (`$queryRaw` on `pg_advisory_xact_lock`) runs inside the + // `$transaction`, and the create is gated behind both. + expect(prisma.$queryRaw).toHaveBeenCalled(); + expect(prisma.$transaction).toHaveBeenCalled(); + expect( + vi.mocked(prisma.$queryRaw).mock.invocationCallOrder[0], + ).toBeLessThan(vi.mocked(prisma.user.create).mock.invocationCallOrder[0]); + }); + it("refuses to auto-provision when registration is closed", async () => { mockIdentity({ sub: "sub-3", email: "blocked@example.com" }); mockUserLookups({ byIdentity: null, byEmail: null }); diff --git a/src/app/api/auth/oidc/callback/route.ts b/src/app/api/auth/oidc/callback/route.ts index 0311a179e..9e125e173 100644 --- a/src/app/api/auth/oidc/callback/route.ts +++ b/src/app/api/auth/oidc/callback/route.ts @@ -285,19 +285,43 @@ export const GET = apiHandler(async (req: NextRequest) => { .then((u: unknown) => u !== null), ); - user = await prisma.user.create({ - data: { - email: email.toLowerCase(), - username, - passwordHash: null, - role: userCount === 0 ? "ADMIN" : "USER", - // Same server-default resolution the register route uses — an - // SSO-provisioned account has no registration form to carry the - // browser-detected timezone. - timezone: await resolveServerDefaultTimezone(), - oidcIssuer: metadata.issuer, - oidcSub: identity.sub, - }, + // Same server-default resolution the register route uses — an + // SSO-provisioned account has no registration form to carry the + // browser-detected timezone. + const timezone = await resolveServerDefaultTimezone(); + + // v1.28.48 — the first provisioned user becomes ADMIN. Deriving that + // role from the early `userCount` (read above, before username + // derivation) and then creating without coordination is the same + // check-then-act race the password-register route closed in v1.28.42: + // two concurrent OIDC first-logins racing the empty-DB window would + // both observe `0` and both be minted ADMIN. Serialise the + // count+insert behind a transaction-scoped advisory lock (released on + // commit/rollback) and re-count *inside* the lock so the second + // provision observes the first's committed row and is minted USER. + // The lock key STRING matches the register route's — an OIDC + // first-login racing a password first-registration is serialised + // against the same lock, so only one ADMIN can ever be minted. + user = await prisma.$transaction(async (tx) => { + // `pg_advisory_xact_lock` returns void, which the client cannot + // deserialize as a column — selecting FROM it yields a plain int row. + await tx.$queryRaw` + SELECT 1 AS locked + FROM pg_advisory_xact_lock(hashtextextended('register:first-admin', 0)) + `; + const priorUsers = await tx.user.count(); + const role = priorUsers === 0 ? "ADMIN" : "USER"; + return tx.user.create({ + data: { + email: email.toLowerCase(), + username, + passwordHash: null, + role, + timezone, + oidcIssuer: metadata.issuer, + oidcSub: identity.sub, + }, + }); }); await auditLog("auth.oidc.provisioned", { diff --git a/src/app/api/nutrients/batch/__tests__/route.test.ts b/src/app/api/nutrients/batch/__tests__/route.test.ts index e1803183f..140ef2529 100644 --- a/src/app/api/nutrients/batch/__tests__/route.test.ts +++ b/src/app/api/nutrients/batch/__tests__/route.test.ts @@ -333,9 +333,12 @@ describe("POST /api/nutrients/batch — upsert semantics", () => { ); }); - it("a thrown upsert marks only that entry skipped upsert_failed", async () => { + it("a thrown upsert marks only that entry skipped upsert_failed without leaking the raw error", async () => { + // The exception message must never reach the client — the per-entry + // `reason` is a closed set. A raw-message echo was a v1.28.48 fix. + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); vi.mocked(prisma.nutrientIntakeDay.upsert) - .mockRejectedValueOnce(new Error("boom")) + .mockRejectedValueOnce(new Error("boom: sensitive db detail")) .mockResolvedValueOnce({} as never); const res = await POST( @@ -348,7 +351,13 @@ describe("POST /api/nutrients/batch — upsert semantics", () => { ); const body = (await res.json()) as BatchResponse; expect(body.data.entries[0].status).toBe("skipped"); + expect(body.data.entries[0].reason).toBe("upsert_failed"); + expect(body.data.skipped[0].reason).toBe("upsert_failed"); + // The raw exception text is logged server-side only, never echoed. + expect(JSON.stringify(body)).not.toContain("sensitive db detail"); + expect(spy).toHaveBeenCalled(); expect(body.data.entries[1].status).toBe("inserted"); expect(body.data.inserted).toBe(1); + spy.mockRestore(); }); }); diff --git a/src/app/api/nutrients/batch/route.ts b/src/app/api/nutrients/batch/route.ts index c952fa911..ae78c001a 100644 --- a/src/app/api/nutrients/batch/route.ts +++ b/src/app/api/nutrients/batch/route.ts @@ -208,8 +208,12 @@ async function postBatch(request: NextRequest): Promise { results.push({ index: i, status: "inserted" }); } } catch (err: unknown) { - const reason = - err instanceof Error ? err.message.slice(0, 120) : "upsert_failed"; + // The client-facing `reason` is a closed set (see the file header) — + // never echo raw exception text into it. Log the real error + // server-side only (SWC keeps `console.error` in prod) and return the + // fixed `upsert_failed` member. + console.error("[nutrient-batch] upsert failed", err); + const reason = "upsert_failed"; skipped.push({ index: i, reason }); results.push({ index: i, status: "skipped", reason }); }