mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(api): narrow swallowed database errors and redact the incident URL
The incident forwarder stripped the query string but not path-segment secrets from the reported URL; route it through the same redaction the message and stack already use. Several delete-and-swallow paths caught every error as the expected unique/absent-row case, masking real failures on security-notification, logout, and integration-disconnect paths and the Telegram reply-consume guard; narrow each to the specific Prisma code and let real failures surface.
This commit is contained in:
1 parent
9aa0a3597a
commit
336ce5aef3
10 files changed
+136
-22
No files matched your search
@@ -1,5 +1,6 @@
|
||||
import { prisma } from "@/lib/db";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { isP2025 } from "@/lib/prisma-errors";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { apiSuccess, apiError, safeJson } from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
@@ -71,9 +72,15 @@ export const DELETE = apiHandler(async () => {
|
||||
const { user } = await requireAuth();
|
||||
annotate({ action: { name: "fitbit.credentials.delete" } });
|
||||
|
||||
// A missing connection row (P2025) is a benign "already disconnected"
|
||||
// no-op; any other failure must propagate rather than be swallowed —
|
||||
// otherwise the encrypted OAuth tokens can be left orphaned while the user
|
||||
// is told the integration was disconnected.
|
||||
await prisma.fitbitConnection
|
||||
.delete({ where: { userId: user.id } })
|
||||
.catch(() => {});
|
||||
.catch((err) => {
|
||||
if (!isP2025(err)) throw err;
|
||||
});
|
||||
|
||||
await prisma.user.update({
|
||||
where: { id: user.id },
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { timingSafeEqual } from "node:crypto";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { isP2025 } from "@/lib/prisma-errors";
|
||||
import { decrypt } from "@/lib/crypto";
|
||||
import {
|
||||
answerTelegramCallbackQuery,
|
||||
@@ -945,11 +946,19 @@ async function handleTextMessage(update: TelegramUpdate) {
|
||||
context.userId === user.id &&
|
||||
(context.kind === "mood_note" || context.kind === "measure_value")
|
||||
) {
|
||||
// Consume the context row regardless of outcome so a reply can't be
|
||||
// replayed onto the entry twice.
|
||||
await prisma.telegramPromptContext
|
||||
.delete({ where: { id: context.id } })
|
||||
.catch(() => {});
|
||||
// The delete IS the replay guard: consuming the context row is what
|
||||
// stops a redelivered reply from being applied to the entry twice. Make
|
||||
// it authoritative — if the delete fails for any reason OTHER than the
|
||||
// row already being gone (P2025), do NOT apply the reply; abort and let
|
||||
// Telegram redeliver against a still-present row. P2025 means the row
|
||||
// was already consumed, which is the guard succeeding, so proceed.
|
||||
try {
|
||||
await prisma.telegramPromptContext.delete({
|
||||
where: { id: context.id },
|
||||
});
|
||||
} catch (err) {
|
||||
if (!isP2025(err)) return;
|
||||
}
|
||||
const userMsgId = message?.message_id;
|
||||
if (context.expiresAt.getTime() < Date.now()) {
|
||||
const resp = await sendTelegramMessage(
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { prisma } from "@/lib/db";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { isP2025 } from "@/lib/prisma-errors";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { apiSuccess, apiError, safeJson } from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
@@ -69,9 +70,15 @@ export const DELETE = apiHandler(async () => {
|
||||
const { user } = await requireAuth();
|
||||
annotate({ action: { name: "whoop.credentials.delete" } });
|
||||
|
||||
// A missing connection row (P2025) is a benign "already disconnected"
|
||||
// no-op; any other failure must propagate rather than be swallowed —
|
||||
// otherwise the encrypted OAuth tokens can be left orphaned while the user
|
||||
// is told the integration was disconnected.
|
||||
await prisma.whoopConnection
|
||||
.delete({ where: { userId: user.id } })
|
||||
.catch(() => {});
|
||||
.catch((err) => {
|
||||
if (!isP2025(err)) throw err;
|
||||
});
|
||||
|
||||
await prisma.user.update({
|
||||
where: { id: user.id },
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { prisma } from "@/lib/db";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { apiSuccess, apiError, safeJson } from "@/lib/api-response";
|
||||
import { isP2025 } from "@/lib/prisma-errors";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { encrypt } from "@/lib/crypto";
|
||||
import { withingsCredentialsSchema } from "@/lib/validations/withings";
|
||||
@@ -64,10 +65,15 @@ export const DELETE = apiHandler(async () => {
|
||||
const { user } = await requireAuth();
|
||||
annotate({ action: { name: "withings.credentials.delete" } });
|
||||
|
||||
// Remove connection first
|
||||
// Remove connection first. A missing connection row (P2025) is a benign
|
||||
// "already disconnected" no-op; any other failure must propagate rather
|
||||
// than be swallowed — otherwise the encrypted OAuth tokens can be left
|
||||
// orphaned while the user is told the integration was disconnected.
|
||||
await prisma.withingsConnection
|
||||
.delete({ where: { userId: user.id } })
|
||||
.catch(() => {});
|
||||
.catch((err) => {
|
||||
if (!isP2025(err)) throw err;
|
||||
});
|
||||
|
||||
// Remove credentials
|
||||
await prisma.user.update({
|
||||
|
||||
+11
-4
@@ -340,9 +340,11 @@ export async function requireAuth(
|
||||
*
|
||||
* The validation itself lives in the transport-agnostic `resolveBearerToken`
|
||||
* (`./auth/bearer`) — the single source of truth shared with the MCP wire. This
|
||||
* wrapper adds the HTTP-edge concerns: the `auth.bearer.*` audit trail for both
|
||||
* outcomes and the Wide-Event `auth_method: "bearer"` annotation, then maps the
|
||||
* result onto the `AuthContext` contract (`session.id` carries the token id).
|
||||
* wrapper adds the HTTP-edge concerns: the `auth.bearer.failure` audit trail
|
||||
* (only the failure path writes a durable audit row — the success row was
|
||||
* intentionally dropped for perf) and the Wide-Event `auth_method: "bearer"`
|
||||
* annotation, then maps the result onto the `AuthContext` contract
|
||||
* (`session.id` carries the token id).
|
||||
*
|
||||
* Authorisation contract (unchanged): a route that declares no
|
||||
* `requiredPermission` accepts any valid token; one that declares a scope
|
||||
@@ -648,7 +650,12 @@ async function reportToGlitchtip(
|
||||
level: "error",
|
||||
type: err.name || "Error",
|
||||
stack: redactOptional(err.stack),
|
||||
url: scrubbedUrl,
|
||||
// Query string is already stripped above, but path-segment secrets
|
||||
// (e.g. `/api/withings/webhook/<secret>`, `/api/whoop/webhook/<secret>`)
|
||||
// survive that strip. Run the same `redactSecrets` pass that guards the
|
||||
// message/stack so a `PATH_SECRET_PATHS`-registered secret cannot reach
|
||||
// the external incident UI.
|
||||
url: redactSecrets(scrubbedUrl),
|
||||
sourceTag: "healthlog-api-handler",
|
||||
requestId: evt.getRequestId(),
|
||||
},
|
||||
|
||||
@@ -117,10 +117,10 @@ describe("recordSignInDevice — new-device dedupe", () => {
|
||||
expect(dispatchNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("a unique-index race on create does not double-alert", async () => {
|
||||
it("a unique-index race (P2002) on create does not double-alert", async () => {
|
||||
vi.mocked(prisma.userKnownDevice.findUnique).mockResolvedValue(null);
|
||||
vi.mocked(prisma.userKnownDevice.create).mockRejectedValue(
|
||||
new Error("unique violation"),
|
||||
Object.assign(new Error("unique violation"), { code: "P2002" }),
|
||||
);
|
||||
|
||||
const res = await recordSignInDevice({
|
||||
@@ -133,6 +133,27 @@ describe("recordSignInDevice — new-device dedupe", () => {
|
||||
expect(dispatchNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("a non-P2002 create failure is NOT masked as a known device", async () => {
|
||||
// A real DB fault must not be misclassified as "device already known" —
|
||||
// that would silently skip the SECURITY_ALERT + audit row. The narrowed
|
||||
// catch rethrows it to the outer handler, which records the failure and
|
||||
// returns the safe `{ known: false, alerted: false }` verdict without
|
||||
// dispatching a bogus alert.
|
||||
vi.mocked(prisma.userKnownDevice.findUnique).mockResolvedValue(null);
|
||||
vi.mocked(prisma.userKnownDevice.create).mockRejectedValue(
|
||||
Object.assign(new Error("connection reset"), { code: "P1001" }),
|
||||
);
|
||||
|
||||
const res = await recordSignInDevice({
|
||||
userId: "u1",
|
||||
ip: "203.0.113.9",
|
||||
userAgent: UA,
|
||||
});
|
||||
|
||||
expect(res).toEqual({ known: false, alerted: false });
|
||||
expect(dispatchNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("never persists the raw User-Agent — only the salted hash + coarse label", async () => {
|
||||
vi.mocked(prisma.userKnownDevice.findUnique).mockResolvedValue(null);
|
||||
vi.mocked(prisma.userKnownDevice.create).mockResolvedValue({} as never);
|
||||
|
||||
@@ -10,6 +10,7 @@ vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
session: {
|
||||
findUnique: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
deleteMany: vi.fn(),
|
||||
update: vi.fn(),
|
||||
},
|
||||
@@ -46,7 +47,7 @@ vi.mock("next/headers", () => ({
|
||||
}));
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession, destroyAllSessions } from "../session";
|
||||
import { getSession, destroyAllSessions, destroySession } from "../session";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
@@ -74,6 +75,32 @@ describe("getSession", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("destroySession", () => {
|
||||
it("treats an already-deleted session row (P2025) as an idempotent logout", async () => {
|
||||
cookieState.sessionId = "sess-gone";
|
||||
vi.mocked(prisma.session.delete).mockRejectedValue(
|
||||
Object.assign(new Error("record not found"), { code: "P2025" }) as never,
|
||||
);
|
||||
|
||||
await expect(destroySession()).resolves.toBeUndefined();
|
||||
|
||||
expect(prisma.session.delete).toHaveBeenCalledWith({
|
||||
where: { id: "sess-gone" },
|
||||
});
|
||||
expect(cookieState.delete).toHaveBeenCalledWith("healthlog_session");
|
||||
expect(cookieState.delete).toHaveBeenCalledWith("hl_onboarding");
|
||||
});
|
||||
|
||||
it("propagates a real delete failure instead of reporting a false logout", async () => {
|
||||
cookieState.sessionId = "sess-live";
|
||||
vi.mocked(prisma.session.delete).mockRejectedValue(
|
||||
Object.assign(new Error("connection reset"), { code: "P1001" }) as never,
|
||||
);
|
||||
|
||||
await expect(destroySession()).rejects.toThrow(/connection reset/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("destroyAllSessions", () => {
|
||||
it("revokes web sessions, API tokens, and refresh tokens for the user", async () => {
|
||||
vi.mocked(prisma.session.deleteMany).mockResolvedValue({
|
||||
|
||||
@@ -24,6 +24,7 @@ import { dispatchNotification } from "@/lib/notifications/dispatcher";
|
||||
import { getServerTranslator } from "@/lib/i18n/server-translator";
|
||||
import { defaultLocale, type Locale } from "@/lib/i18n/config";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
import { isP2002 } from "@/lib/prisma-errors";
|
||||
|
||||
function isLocale(value: string | null | undefined): value is Locale {
|
||||
return (
|
||||
@@ -105,8 +106,15 @@ export async function recordSignInDevice(params: {
|
||||
await prisma.userKnownDevice.create({
|
||||
data: { userId, deviceHash, label: storedLabel },
|
||||
});
|
||||
} catch {
|
||||
return { known: true, alerted: false };
|
||||
} catch (err) {
|
||||
// ONLY the unique-index race (a concurrent first-login inserting the
|
||||
// same (userId, deviceHash)) is a legitimate "already known" outcome —
|
||||
// swallow it and skip the alert so we never double-alert. Any other
|
||||
// failure (a real DB fault) must NOT be silently classified as a known
|
||||
// device, which would suppress the SECURITY_ALERT + audit row: rethrow
|
||||
// so the outer catch records it via `addWarning`.
|
||||
if (isP2002(err)) return { known: true, alerted: false };
|
||||
throw err;
|
||||
}
|
||||
|
||||
if (!alertOnNew || isBaseline) return { known: false, alerted: false };
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { User } from "@/generated/prisma/client";
|
||||
import { ensureDbCompatibility } from "@/lib/db-compat";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
import { shouldEmitSecureCookie } from "@/lib/auth/secure-cookie";
|
||||
import { isP2025 } from "@/lib/prisma-errors";
|
||||
|
||||
const SESSION_COOKIE = "healthlog_session";
|
||||
const SESSION_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
|
||||
@@ -179,7 +180,13 @@ export async function destroySession(): Promise<void> {
|
||||
const cookieStore = await cookies();
|
||||
const sessionId = cookieStore.get(SESSION_COOKIE)?.value;
|
||||
if (sessionId) {
|
||||
await prisma.session.delete({ where: { id: sessionId } }).catch(() => {});
|
||||
// Logout is idempotent: a session row that no longer exists (P2025) is a
|
||||
// no-op success — the cookie clear below still runs. Any other failure is
|
||||
// a real delete error that must NOT be masked as a successful logout, so
|
||||
// it propagates rather than being swallowed.
|
||||
await prisma.session.delete({ where: { id: sessionId } }).catch((err) => {
|
||||
if (!isP2025(err)) throw err;
|
||||
});
|
||||
}
|
||||
cookieStore.delete(SESSION_COOKIE);
|
||||
cookieStore.delete(ONBOARDING_COOKIE);
|
||||
|
||||
@@ -13,12 +13,27 @@
|
||||
* unique-constraint code.
|
||||
*/
|
||||
|
||||
/** Narrow a thrown Prisma error to the P2002 unique-constraint code. */
|
||||
export function isP2002(err: unknown): boolean {
|
||||
/** Narrow a thrown Prisma error to a specific known-request code. */
|
||||
function hasPrismaCode(err: unknown, code: string): boolean {
|
||||
return (
|
||||
typeof err === "object" &&
|
||||
err !== null &&
|
||||
"code" in err &&
|
||||
(err as { code: unknown }).code === "P2002"
|
||||
(err as { code: unknown }).code === code
|
||||
);
|
||||
}
|
||||
|
||||
/** Narrow a thrown Prisma error to the P2002 unique-constraint code. */
|
||||
export function isP2002(err: unknown): boolean {
|
||||
return hasPrismaCode(err, "P2002");
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrow a thrown Prisma error to the P2025 "record not found" code — the
|
||||
* error `delete` / `update` raise when the target row no longer exists. Lets
|
||||
* an idempotent delete treat "already gone" as success while still surfacing a
|
||||
* genuine failure (a real DB fault raises a different code and rethrows).
|
||||
*/
|
||||
export function isP2025(err: unknown): boolean {
|
||||
return hasPrismaCode(err, "P2025");
|
||||
}
|
||||
Reference in new issue
Block a user