mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
chore(release): v1.28.48 — security follow-ups
From the campaign's security re-verification pass (posture otherwise held
across the 10-release week — no Critical/High):
- MEDIUM: the OIDC auto-provision branch re-introduced the first-admin race
the password-register route fixed in v1.28.42 — count-then-create with no
lock, so two concurrent OIDC first-logins (or an OIDC first-login racing a
password first-registration) in the empty-DB window could both mint ADMIN.
Now wraps count+role+create in a $transaction taking the SAME advisory lock
key ('register:first-admin') and re-counts inside, so the two paths
serialise; every provisioned field preserved.
- LOW: the nutrients batch catch echoed the raw exception message into the
client-facing reason (violating its closed reason-set). Now returns the
fixed 'upsert_failed' member and logs the detail server-side only.
No migration (advisory lock is runtime).
This commit is contained in:
1 parent
f3a3b8f3a7
commit
a9a318d4a4
8 files changed
+163
-20
No files matched your search
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
+1
-1
@@ -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/*`
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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", {
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -208,8 +208,12 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
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 });
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user