mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 13:22:14 +00:00
fix(logging): carry the route template in http.route
http.route held the concrete pathname, so every record id minted a route of its own and grouping by route answered nothing. apiHandler recovers the template from the handler's params and puts it there; http.path keeps the concrete path, and path-secret redaction still runs on both.
This commit is contained in:
1 parent
fbbb535832
commit
72214c0208
3 files changed
+207
-1
No files matched your search
@@ -11,6 +11,7 @@ import { WideEventBuilder } from "./logging/event-builder";
|
||||
import { annotate, eventStorage, getEvent } from "./logging/context";
|
||||
import { emitIfSampled } from "./logging/transports";
|
||||
import { redactOptional, redactSecrets } from "./logging/redact";
|
||||
import { routeTemplateFor } from "./logging/route-template";
|
||||
import { getSession } from "./auth/session";
|
||||
import {
|
||||
hasSecondFactorEnrolled,
|
||||
@@ -241,7 +242,9 @@ export function apiHandler<T extends (...args: any[]) => Promise<Response>>(
|
||||
evt.setHttp({
|
||||
method: safeRequestProp(request, (r) => r.method, "GET"),
|
||||
path: url.pathname,
|
||||
route: url.pathname,
|
||||
// The handler's template (`/api/medications/[id]`), not the concrete
|
||||
// path: one route per handler, so dashboards can group on it.
|
||||
route: await routeTemplateFor(url.pathname, args[1]),
|
||||
status: 200,
|
||||
user_agent:
|
||||
safeRequestProp(request, (r) => r.headers.get("user-agent"), null) ??
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
import { routeTemplateFor, templatePath } from "../route-template";
|
||||
import { apiHandler } from "@/lib/api-handler";
|
||||
import { apiSuccess } from "@/lib/api-response";
|
||||
import { emitIfSampled } from "@/lib/logging/transports";
|
||||
|
||||
describe("templatePath", () => {
|
||||
it("puts a single param's name back in place of its value", () => {
|
||||
expect(templatePath("/api/medications/cm1abc", { id: "cm1abc" })).toBe(
|
||||
"/api/medications/[id]",
|
||||
);
|
||||
});
|
||||
|
||||
it("handles several params and static segments between them", () => {
|
||||
expect(
|
||||
templatePath("/api/managed-profiles/p1/guardians/g7", {
|
||||
id: "p1",
|
||||
grantId: "g7",
|
||||
}),
|
||||
).toBe("/api/managed-profiles/[id]/guardians/[grantId]");
|
||||
});
|
||||
|
||||
it("collapses a catch-all run into one segment", () => {
|
||||
expect(
|
||||
templatePath("/api/monitoring/umami/api/send/x", {
|
||||
path: ["api", "send", "x"],
|
||||
}),
|
||||
).toBe("/api/monitoring/umami/[...path]");
|
||||
});
|
||||
|
||||
it("places a value equal to an earlier static segment on the dynamic one", () => {
|
||||
expect(templatePath("/api/labs/labs", { id: "labs" })).toBe(
|
||||
"/api/labs/[id]",
|
||||
);
|
||||
});
|
||||
|
||||
it("matches an encoded segment against its decoded value", () => {
|
||||
expect(templatePath("/api/fhir/a%20b", { system: "a b" })).toBe(
|
||||
"/api/fhir/[system]",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns the pathname when a value is not in it", () => {
|
||||
expect(templatePath("/api/x/1", { id: "2" })).toBe("/api/x/1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("routeTemplateFor", () => {
|
||||
it("awaits Next's params promise", async () => {
|
||||
await expect(
|
||||
routeTemplateFor("/api/medications/m1", {
|
||||
params: Promise.resolve({ id: "m1" }),
|
||||
}),
|
||||
).resolves.toBe("/api/medications/[id]");
|
||||
});
|
||||
|
||||
it("leaves a static route as it is", async () => {
|
||||
await expect(routeTemplateFor("/api/version", undefined)).resolves.toBe(
|
||||
"/api/version",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the pathname when params reject", async () => {
|
||||
await expect(
|
||||
routeTemplateFor("/api/x/1", { params: Promise.reject(new Error("x")) }),
|
||||
).resolves.toBe("/api/x/1");
|
||||
});
|
||||
});
|
||||
|
||||
vi.mock("@/lib/db", () => ({ prisma: {} }));
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
|
||||
describe("apiHandler's wide event", () => {
|
||||
it("carries the template in http.route and the concrete path in http.path", async () => {
|
||||
const route = apiHandler(
|
||||
async (_req: unknown, _ctx: { params: Promise<{ id: string }> }) =>
|
||||
apiSuccess({ ok: true }),
|
||||
);
|
||||
await route(new NextRequest("http://localhost/api/medications/cm9"), {
|
||||
params: Promise.resolve({ id: "cm9" }),
|
||||
});
|
||||
|
||||
const event = vi.mocked(emitIfSampled).mock.calls.at(-1)![0] as {
|
||||
http: { path: string; route: string };
|
||||
};
|
||||
expect(event.http.path).toBe("/api/medications/cm9");
|
||||
expect(event.http.route).toBe("/api/medications/[id]");
|
||||
});
|
||||
|
||||
it("still scrubs a path secret from both fields", async () => {
|
||||
const route = apiHandler(async () => apiSuccess({ ok: true }));
|
||||
// No params handed in: the route falls back to the pathname, and the
|
||||
// redaction floor still hides the secret.
|
||||
await route(
|
||||
new NextRequest("http://localhost/api/withings/webhook/s3cr3t-value"),
|
||||
);
|
||||
const event = vi.mocked(emitIfSampled).mock.calls.at(-1)![0] as {
|
||||
http: { path: string; route: string };
|
||||
};
|
||||
expect(event.http.path).not.toContain("s3cr3t-value");
|
||||
expect(event.http.route).not.toContain("s3cr3t-value");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,92 @@
|
||||
/**
|
||||
* The route template a request matched, for the wide event's `http.route`.
|
||||
*
|
||||
* `http.route` carried the concrete pathname, so `/api/medications/<cuid>`
|
||||
* was a fresh route for every medication: a dashboard grouping by route
|
||||
* counted thousands of one-request "routes" and could not answer the one
|
||||
* question the field exists for — which handler is slow, which one fails.
|
||||
* `http.path` keeps the concrete path; this field now names the handler.
|
||||
*
|
||||
* Next.js does not hand a route handler its template, but it does hand a
|
||||
* dynamic one its `params`, and every dynamic segment's value is in there.
|
||||
* So the template is recovered by putting each param's name back where its
|
||||
* value sits: `[id]` for a single segment, `[...path]` for a catch-all run.
|
||||
* Params are matched right to left, the order the segments appear in reverse,
|
||||
* so a value that happens to equal an earlier static segment (`/api/x/x`) is
|
||||
* placed on the dynamic segment rather than the static one.
|
||||
*
|
||||
* A static route has no params and its pathname already is its template. When
|
||||
* a value cannot be found (a malformed encoding, a placeholder request in a
|
||||
* test), the pathname is returned unchanged rather than a guess: redaction of
|
||||
* path secrets runs on this field either way (`setHttp` → `redactSecrets`), so
|
||||
* a fallback never carries a secret a template would have hidden.
|
||||
*/
|
||||
|
||||
type Params = Record<string, unknown>;
|
||||
|
||||
function decodeSegment(segment: string): string {
|
||||
try {
|
||||
return decodeURIComponent(segment);
|
||||
} catch {
|
||||
return segment;
|
||||
}
|
||||
}
|
||||
|
||||
/** Put param names back in place of their values. Pure, for tests. */
|
||||
export function templatePath(pathname: string, params: Params): string {
|
||||
const segments = pathname.split("/");
|
||||
const decoded = segments.map(decodeSegment);
|
||||
const out: Array<string | null> = [...segments];
|
||||
// Index 0 is the empty string before the leading slash; never a param.
|
||||
let cursor = segments.length;
|
||||
|
||||
for (const [key, value] of Object.entries(params).reverse()) {
|
||||
if (Array.isArray(value)) {
|
||||
const run = value.filter((v): v is string => typeof v === "string");
|
||||
if (run.length === 0) continue;
|
||||
for (let start = cursor - run.length; start >= 1; start--) {
|
||||
if (run.every((v, i) => decoded[start + i] === v)) {
|
||||
out[start] = `[...${key}]`;
|
||||
for (let i = 1; i < run.length; i++) out[start + i] = null;
|
||||
cursor = start;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} else if (typeof value === "string" && value.length > 0) {
|
||||
for (let i = cursor - 1; i >= 1; i--) {
|
||||
if (decoded[i] === value) {
|
||||
out[i] = `[${key}]`;
|
||||
cursor = i;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return out.filter((s): s is string => s !== null).join("/");
|
||||
}
|
||||
|
||||
/**
|
||||
* The template for a request, from the route handler's second argument.
|
||||
* Awaits `params` (a Promise since Next.js 15; a plain object in some tests).
|
||||
*/
|
||||
export async function routeTemplateFor(
|
||||
pathname: string,
|
||||
context: unknown,
|
||||
): Promise<string> {
|
||||
const raw =
|
||||
context && typeof context === "object"
|
||||
? (context as { params?: unknown }).params
|
||||
: undefined;
|
||||
if (!raw) return pathname;
|
||||
let params: unknown;
|
||||
try {
|
||||
params = await raw;
|
||||
} catch {
|
||||
return pathname;
|
||||
}
|
||||
if (!params || typeof params !== "object" || Array.isArray(params)) {
|
||||
return pathname;
|
||||
}
|
||||
return templatePath(pathname, params as Params);
|
||||
}
|
||||
Reference in new issue
Block a user