feat(diagnostics): correlate staff operations errors and audit records
This commit is contained in:
1 parent
440ce4e556
commit
a2954e4408
14 files changed
+473
-110
No files matched your search
File diff suppressed because it is too large.
Load diff
+111
-94
File diff suppressed because it is too large.
Load diff
@@ -58,3 +58,20 @@ it("creates only recognized local operation links", () => {
|
||||
])
|
||||
expect(errorOperationLink({ ...base, context: { path } })).toBeNull();
|
||||
});
|
||||
|
||||
it("links server operation IDs to the audit search without trusting browser IDs", () => {
|
||||
const record = createErrorRecord("server", "test", "failed", {
|
||||
operationId: "op-123",
|
||||
});
|
||||
expect(summarizeErrorGroup([record]).auditLink).toBe(
|
||||
"/admin/logs/audit?search=op-123",
|
||||
);
|
||||
expect(
|
||||
summarizeErrorGroup([{ ...record, source: "browser" }]).auditLink,
|
||||
).toBeNull();
|
||||
expect(
|
||||
summarizeErrorGroup([
|
||||
{ ...record, context: { operationId: "../../secrets" } },
|
||||
]).auditLink,
|
||||
).toBeNull();
|
||||
});
|
||||
@@ -69,5 +69,11 @@ export function summarizeErrorGroup(events: CmsErrorRecord[]) {
|
||||
releases: [...releases.values()],
|
||||
latest,
|
||||
operationLink: latest ? errorOperationLink(latest) : null,
|
||||
auditLink:
|
||||
latest?.source === "server" &&
|
||||
typeof latest.context.operationId === "string" &&
|
||||
/^[a-zA-Z0-9-]{1,100}$/.test(latest.context.operationId)
|
||||
? `/admin/logs/audit?search=${encodeURIComponent(latest.context.operationId)}`
|
||||
: null,
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,27 @@
|
||||
import { expect, it } from "vitest";
|
||||
import {
|
||||
createStore,
|
||||
getOperationContext,
|
||||
runWithStore,
|
||||
setContextUserId,
|
||||
} from "./request-context";
|
||||
import type { IpAddress, UserId } from "./types";
|
||||
|
||||
it("isolates operation IDs and users across concurrent asynchronous requests", async () => {
|
||||
const first = createStore("test" as IpAddress),
|
||||
second = createStore("test" as IpAddress);
|
||||
expect(first.requestId).not.toBe(second.requestId);
|
||||
await Promise.all(
|
||||
[first, second].map((store, index) =>
|
||||
runWithStore(store, async () => {
|
||||
setContextUserId((index + 1) as UserId);
|
||||
await new Promise((resolve) => setTimeout(resolve, 5 - index));
|
||||
expect(getOperationContext()).toEqual({
|
||||
operationId: store.requestId,
|
||||
userId: index + 1,
|
||||
});
|
||||
}),
|
||||
),
|
||||
);
|
||||
expect(getOperationContext()).toEqual({});
|
||||
});
|
||||
@@ -38,3 +38,17 @@ export function setContextUserId(userId: UserId): void {
|
||||
const store = als.getStore();
|
||||
if (store) store.userId = userId;
|
||||
}
|
||||
|
||||
/** Stable correlation data exists only inside an active request or job. */
|
||||
export function getOperationContext(): {
|
||||
operationId?: string;
|
||||
userId?: number;
|
||||
} {
|
||||
const store = als.getStore();
|
||||
return store
|
||||
? {
|
||||
operationId: store.requestId,
|
||||
...(store.userId === null ? {} : { userId: Number(store.userId) }),
|
||||
}
|
||||
: {};
|
||||
}
|
||||
@@ -0,0 +1,49 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
createStore,
|
||||
getOperationContext,
|
||||
runWithStore,
|
||||
setContextUserId,
|
||||
} from "./foundation/request-context";
|
||||
import type { IpAddress, UserId } from "./foundation/types";
|
||||
|
||||
const state = vi.hoisted(() => ({
|
||||
records: [] as { context: Record<string, unknown> }[],
|
||||
}));
|
||||
vi.mock("./error-monitor", async (original) => {
|
||||
const actual = await original<typeof import("./error-monitor")>();
|
||||
return {
|
||||
...actual,
|
||||
captureCmsError: (...args: Parameters<typeof actual.createErrorRecord>) => {
|
||||
const record = actual.createErrorRecord(...args);
|
||||
state.records.push(record);
|
||||
return record.id;
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import { logger } from "./logger";
|
||||
|
||||
describe("logger correlation boundaries", () => {
|
||||
beforeEach(() => {
|
||||
state.records = [];
|
||||
});
|
||||
it("keeps trusted operation context when metadata is large or supplies conflicting IDs", () => {
|
||||
const store = createStore("test" as IpAddress);
|
||||
runWithStore(store, () => {
|
||||
setContextUserId(42 as UserId);
|
||||
logger.error("failed", {
|
||||
...Object.fromEntries(
|
||||
Array.from({ length: 30 }, (_, index) => [`field${index}`, index]),
|
||||
),
|
||||
operationId: "forged",
|
||||
userId: 99,
|
||||
});
|
||||
});
|
||||
expect(state.records[0].context).toMatchObject({
|
||||
operationId: store.requestId,
|
||||
userId: 42,
|
||||
});
|
||||
expect(getOperationContext()).toEqual({});
|
||||
});
|
||||
});
|
||||
+10
-6
@@ -1,6 +1,7 @@
|
||||
import pino from "pino";
|
||||
import { env } from "@/env";
|
||||
import { captureCmsError } from "./error-monitor";
|
||||
import { getOperationContext } from "./foundation/request-context";
|
||||
|
||||
type LogLevel = "debug" | "info" | "warn" | "error";
|
||||
|
||||
@@ -31,16 +32,16 @@ export function generateRequestId(): string {
|
||||
return `${Date.now().toString(36)}-${requestIdCounter.toString(36)}`;
|
||||
}
|
||||
|
||||
/** Compatibility wrapper — existing call sites use (message, meta). */
|
||||
/** Compatibility wrapper — existing call sites use (message, meta). */
|
||||
export const logger = {
|
||||
debug(message: string, meta: Record<string, unknown> = {}): void {
|
||||
pinoLogger.debug(meta, message);
|
||||
pinoLogger.debug({ ...meta, ...getOperationContext() }, message);
|
||||
},
|
||||
info(message: string, meta: Record<string, unknown> = {}): void {
|
||||
pinoLogger.info(meta, message);
|
||||
pinoLogger.info({ ...meta, ...getOperationContext() }, message);
|
||||
},
|
||||
warn(message: string, meta: Record<string, unknown> = {}): void {
|
||||
pinoLogger.warn(meta, message);
|
||||
pinoLogger.warn({ ...meta, ...getOperationContext() }, message);
|
||||
},
|
||||
error(message: string, meta: Record<string, unknown> = {}): string {
|
||||
const error =
|
||||
@@ -55,8 +56,11 @@ export const logger = {
|
||||
? meta.error
|
||||
: message,
|
||||
);
|
||||
const errorId = captureCmsError("server", message, error, meta);
|
||||
pinoLogger.error({ ...meta, errorId }, message);
|
||||
const operation = getOperationContext();
|
||||
// Keep trusted correlation first so bounded error context retains it.
|
||||
const context = { ...operation, ...meta, ...operation };
|
||||
const errorId = captureCmsError("server", message, error, context);
|
||||
pinoLogger.error({ ...context, errorId }, message);
|
||||
return errorId;
|
||||
},
|
||||
};
|
||||
@@ -36,8 +36,10 @@ describe("audit query filters", () => {
|
||||
expect(query.sql).not.toContain("Alice");
|
||||
expect(query.sql).toContain("`admin_audit_log`.`user_id` in (select");
|
||||
expect(query.sql).toContain("`admin_audit_log`.`created_at` >= ?");
|
||||
expect(query.sql).toContain("`admin_audit_log`.`details` like ?");
|
||||
expect(query.sql).toContain("`admin_audit_log`.`created_at` < ?");
|
||||
expect(query.params.slice(0, 6)).toEqual([
|
||||
expect(query.params.slice(0, 7)).toEqual([
|
||||
"%user%",
|
||||
"%user%",
|
||||
"%user%",
|
||||
"update",
|
||||
|
||||
@@ -49,6 +49,12 @@ vi.mock("@/lib/db", () => ({
|
||||
|
||||
vi.mock("@/env", () => ({ env: {} }));
|
||||
|
||||
import {
|
||||
createStore,
|
||||
runWithStore,
|
||||
setContextUserId,
|
||||
} from "@/lib/foundation/request-context";
|
||||
import type { IpAddress, UserId } from "@/lib/foundation/types";
|
||||
import { getAuditLogs, logAudit } from "./audit";
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -56,6 +62,25 @@ beforeEach(() => {
|
||||
});
|
||||
|
||||
describe("logAudit", () => {
|
||||
it("stores the same trusted operation ID across asynchronous audit writes", async () => {
|
||||
const store = createStore("test" as IpAddress);
|
||||
await runWithStore(store, async () => {
|
||||
setContextUserId(42 as UserId);
|
||||
await Promise.resolve();
|
||||
await logAudit({
|
||||
userId: 42,
|
||||
action: "update",
|
||||
target: "user",
|
||||
after: { name: "Alice" },
|
||||
});
|
||||
});
|
||||
expect(JSON.parse(insertValues.mock.calls[0][0].details)).toEqual({
|
||||
operationId: store.requestId,
|
||||
userId: 42,
|
||||
});
|
||||
await logAudit({ userId: 42, action: "outside", target: "user" });
|
||||
expect(JSON.parse(insertValues.mock.calls[1][0].details)).toEqual({});
|
||||
});
|
||||
it("creates an audit entry", async () => {
|
||||
insertValues.mockResolvedValue({ id: 1 });
|
||||
await logAudit({
|
||||
@@ -189,6 +214,40 @@ describe("getAuditLogs", () => {
|
||||
});
|
||||
|
||||
describe("audit response privacy", () => {
|
||||
it("exposes only a validated operation ID, never arbitrary stored details", async () => {
|
||||
selectRows.mockResolvedValue([
|
||||
{
|
||||
id: 1,
|
||||
userId: 42,
|
||||
diff: null,
|
||||
before: null,
|
||||
after: null,
|
||||
operationDetails: JSON.stringify({
|
||||
operationId: "op-123",
|
||||
token: "private-token",
|
||||
ip: "private-ip",
|
||||
}),
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
userId: 42,
|
||||
diff: null,
|
||||
before: null,
|
||||
after: null,
|
||||
operationDetails: JSON.stringify({
|
||||
operationId: "../../other?token=private",
|
||||
}),
|
||||
},
|
||||
]);
|
||||
selectCount.mockResolvedValue([{ value: 2 }]);
|
||||
selectUsers.mockResolvedValue([]);
|
||||
const result = await getAuditLogs();
|
||||
expect(result.rows[0].operationId).toBe("op-123");
|
||||
expect(result.rows[1].operationId).toBeNull();
|
||||
expect(JSON.stringify(result.rows)).not.toContain("private-token");
|
||||
expect(JSON.stringify(result.rows)).not.toContain("private-ip");
|
||||
expect(result.rows[0]).not.toHaveProperty("operationDetails");
|
||||
});
|
||||
it("does not serialize legacy snapshot secrets to the client", async () => {
|
||||
selectRows.mockResolvedValue([
|
||||
{
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { and, count, desc, eq, gte, inArray, like, lt, or } from "drizzle-orm";
|
||||
import { AdminAuditLog, db, User } from "@/lib/db";
|
||||
import { getOperationContext } from "@/lib/foundation/request-context";
|
||||
import { readAuditChanges } from "./audit-diff";
|
||||
import { type AuditFilters, normalizeAuditFilters } from "./audit-filters";
|
||||
|
||||
@@ -60,6 +61,7 @@ export async function logAudit(entry: AuditEntry): Promise<void> {
|
||||
|
||||
await db.insert(AdminAuditLog).values({
|
||||
userId: entry.userId,
|
||||
details: JSON.stringify(getOperationContext()),
|
||||
action: entry.action,
|
||||
target: entry.target,
|
||||
targetId: entry.targetId,
|
||||
@@ -79,6 +81,7 @@ export async function getAuditLogs(options: AuditFilters = {}) {
|
||||
? or(
|
||||
like(AdminAuditLog.action, `%${search}%`),
|
||||
like(AdminAuditLog.target, `%${search}%`),
|
||||
like(AdminAuditLog.details, `%${search}%`),
|
||||
)
|
||||
: undefined,
|
||||
action ? eq(AdminAuditLog.action, action) : undefined,
|
||||
@@ -105,6 +108,7 @@ export async function getAuditLogs(options: AuditFilters = {}) {
|
||||
target: AdminAuditLog.target,
|
||||
targetId: AdminAuditLog.targetId,
|
||||
diff: AdminAuditLog.diff,
|
||||
operationDetails: AdminAuditLog.details,
|
||||
before: AdminAuditLog.before,
|
||||
after: AdminAuditLog.after,
|
||||
createdAt: AdminAuditLog.createdAt,
|
||||
@@ -131,8 +135,19 @@ export async function getAuditLogs(options: AuditFilters = {}) {
|
||||
|
||||
const enrichedRows = rows.map((r) => {
|
||||
const details = readAuditChanges(r.diff, r.before, r.after);
|
||||
let operationId: string | null = null;
|
||||
try {
|
||||
const meta = JSON.parse(r.operationDetails ?? "{}");
|
||||
if (
|
||||
typeof meta.operationId === "string" &&
|
||||
/^[a-zA-Z0-9-]{1,100}$/.test(meta.operationId)
|
||||
)
|
||||
operationId = meta.operationId;
|
||||
} catch {}
|
||||
const { operationDetails: _privateDetails, ...safeRow } = r;
|
||||
return {
|
||||
...r,
|
||||
...safeRow,
|
||||
operationId,
|
||||
// Only sanitized changes may cross the server/client boundary.
|
||||
before: null,
|
||||
after: null,
|
||||
|
||||
Reference in new issue
Block a user