diff --git a/backend/migrations/20260728_audit_events.sql b/backend/migrations/20260728_audit_events.sql index 21395f47d..778c3f949 100644 --- a/backend/migrations/20260728_audit_events.sql +++ b/backend/migrations/20260728_audit_events.sql @@ -1,5 +1,10 @@ --- Audit history of user actions (Clue custom; queried via the service-role --- backend only — no RLS policies needed, like other app tables). +-- Audit history of user actions, queried via the service-role backend only. +-- Like every other backend-owned table (see 20260508_01), the browser +-- anon/authenticated roles are revoked and RLS is enabled with no policies as +-- defense in depth. Without this, a hosted Supabase project's default ACLs +-- leave the whole table readable and writable with the public anon key — any +-- visitor could dump every user's email, chat titles and prompt excerpts, or +-- forge/delete audit rows. create table if not exists public.audit_events ( id uuid primary key default gen_random_uuid(), created_at timestamptz not null default now(), @@ -18,3 +23,11 @@ create table if not exists public.audit_events ( ); create index if not exists audit_events_user_created on public.audit_events (user_id, created_at desc); create index if not exists audit_events_project_created on public.audit_events (project_id, created_at desc); + +-- Backend-only access: revoke the browser roles, enable RLS (no policies), and +-- grant the service_role the privileges the backend needs. The explicit grant +-- keeps a fresh plain-Postgres apply working even where service_role has no +-- default ACL for new tables. +revoke all on public.audit_events from anon, authenticated; +alter table public.audit_events enable row level security; +grant select, insert, update, delete on public.audit_events to service_role; diff --git a/backend/schema.sql b/backend/schema.sql index 25f31c9d7..421479c91 100644 --- a/backend/schema.sql +++ b/backend/schema.sql @@ -861,6 +861,31 @@ alter table public.courtlistener_opinion_cluster_index enable row level security -- backend verifies the user's JWT. Do not grant the browser anon/authenticated -- roles direct table privileges for backend-owned data. +-- Audit history of user actions (queried via the service-role backend only). +-- Defined here — above the service_role grant block — so `grant ... on all +-- tables in schema public` below covers it on a fresh install. Like every other +-- backend-owned table, direct browser roles are revoked and RLS is enabled with +-- no policies (defense in depth; service_role bypasses RLS for the backend path). +create table if not exists public.audit_events ( + id uuid primary key default gen_random_uuid(), + created_at timestamptz not null default now(), + user_id uuid not null, + user_email text, + action text not null, + status text not null default 'completed', + title text, + surface text, + project_id uuid, + chat_id uuid, + document_id uuid, + review_id uuid, + model text, + detail jsonb +); +create index if not exists audit_events_user_created on public.audit_events (user_id, created_at desc); +create index if not exists audit_events_project_created on public.audit_events (project_id, created_at desc); +alter table public.audit_events enable row level security; + revoke all on public.user_profiles from anon, authenticated; revoke all on public.projects from anon, authenticated; revoke all on public.project_subfolders from anon, authenticated; @@ -885,34 +910,22 @@ revoke all on public.user_mcp_connector_tools from anon, authenticated; revoke all on public.user_mcp_tool_audit_logs from anon, authenticated; revoke all on public.courtlistener_citation_index from anon, authenticated; revoke all on public.courtlistener_opinion_cluster_index from anon, authenticated; +revoke all on public.audit_events from anon, authenticated; -- Tables created by this file are owned by the database bootstrap role. The -- backend connects as service_role, so grant it only the data privileges that -- the direct browser roles above intentionally do not have. RLS is still -- enabled as defense in depth; service_role bypasses it for the backend path. +-- +-- NOTE: this grant targets `all tables in schema public`, so every table it +-- must cover has to already exist above this point. audit_events is therefore +-- defined *before* this block (not after it) — otherwise a fresh plain-Postgres +-- install would create the table with no service_role privileges and the +-- backend's inserts would fail permission-denied (silently, since recordAudit +-- swallows errors). grant select, insert, update, delete on all tables in schema public to service_role; grant usage, select on all sequences in schema public to service_role; - --- Audit history of user actions (queried via the service-role backend only). -create table if not exists public.audit_events ( - id uuid primary key default gen_random_uuid(), - created_at timestamptz not null default now(), - user_id uuid not null, - user_email text, - action text not null, - status text not null default 'completed', - title text, - surface text, - project_id uuid, - chat_id uuid, - document_id uuid, - review_id uuid, - model text, - detail jsonb -); -create index if not exists audit_events_user_created on public.audit_events (user_id, created_at desc); -create index if not exists audit_events_project_created on public.audit_events (project_id, created_at desc); diff --git a/backend/src/app.ts b/backend/src/app.ts index 0565bd389..f91d168f3 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -138,6 +138,7 @@ app.post("/projects/:projectId/documents", uploadLimiter); app.get("/user/export", exportLimiter); app.get("/user/chats/export", exportLimiter); app.get("/user/tabular-reviews/export", exportLimiter); +app.get("/audit/export", exportLimiter); app.delete("/user/account", dataDeleteLimiter); app.delete("/user/chats", dataDeleteLimiter); app.delete("/user/projects", dataDeleteLimiter); diff --git a/backend/src/lib/__tests__/audit.test.ts b/backend/src/lib/__tests__/audit.test.ts new file mode 100644 index 000000000..357b6cb2f --- /dev/null +++ b/backend/src/lib/__tests__/audit.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect } from "vitest"; +import { recordChatTurn } from "../audit"; + +type Insert = Record; + +/** + * Minimal Supabase mock that captures every audit_events insert so tests can + * assert on the exact rows recordChatTurn mines from a turn's events. + */ +function makeDb() { + const inserts: Insert[] = []; + const db = { + from(_table: string) { + return { + insert(row: Insert) { + inserts.push(row); + return Promise.resolve({ error: null }); + }, + }; + }, + }; + return { db: db as any, inserts }; +} + +const base = { + userId: "u1", + userEmail: "u1@example.com", + chatId: "chat1", + projectId: null, + title: "My chat", + model: "claude-x", +}; + +describe("recordChatTurn artifact mining", () => { + it("records a chat.message row plus mined artifact rows", async () => { + const { db, inserts } = makeDb(); + await recordChatTurn(db, base, [ + { type: "doc_created", filename: "brief.docx", document_id: "d1" }, + { type: "doc_edited", filename: "memo.docx", document_id: "d2" }, + { type: "workflow_applied", workflow_id: "wf1", title: "Cleanup" }, + ]); + + expect(inserts.map((r) => r.action)).toEqual([ + "chat.message", + "document.generated", + "document.edited", + "workflow.applied", + ]); + expect(inserts[1]).toMatchObject({ title: "brief.docx", document_id: "d1" }); + expect(inserts[3]).toMatchObject({ + action: "workflow.applied", + detail: { workflow_id: "wf1" }, + }); + }); + + it("mines doc_replicated from its copies, not the source filename/id", async () => { + const { db, inserts } = makeDb(); + await recordChatTurn(db, base, [ + { + type: "doc_replicated", + filename: "source-template.docx", // the SOURCE, not a produced copy + count: 2, + copies: [ + { new_filename: "copy-a.docx", document_id: "da", version_id: "va" }, + { new_filename: "copy-b.docx", document_id: "db", version_id: "vb" }, + ], + }, + ]); + + // chat.message + one document.generated per copy. + const artifacts = inserts.filter((r) => r.action === "document.generated"); + expect(artifacts).toHaveLength(2); + expect(artifacts.map((r) => r.title)).toEqual(["copy-a.docx", "copy-b.docx"]); + expect(artifacts.map((r) => r.document_id)).toEqual(["da", "db"]); + // The source filename must never leak in as a title, and the (absent) + // top-level document_id must never produce a null-id row. + expect(inserts.some((r) => r.title === "source-template.docx")).toBe(false); + }); + + it("emits no artifact rows for a doc_replicated with empty copies", async () => { + const { db, inserts } = makeDb(); + await recordChatTurn(db, base, [ + { type: "doc_replicated", filename: "src.docx", count: 0, copies: [] }, + ]); + expect(inserts.map((r) => r.action)).toEqual(["chat.message"]); + }); +}); diff --git a/backend/src/lib/__tests__/userDataCleanup.test.ts b/backend/src/lib/__tests__/userDataCleanup.test.ts index 35a8ed1d6..b13dfa1b1 100644 --- a/backend/src/lib/__tests__/userDataCleanup.test.ts +++ b/backend/src/lib/__tests__/userDataCleanup.test.ts @@ -371,6 +371,11 @@ describe("deleteUserAccountData", () => { { id: "w1", user_id: "u1" }, { id: "w-other", user_id: "u2" }, ], + audit_events: [ + { id: "a1", user_id: "u1" }, + { id: "a2", user_id: "u1" }, + { id: "a-other", user_id: "u2" }, + ], }); it("removes the user's rows, files, and share references everywhere", async () => { @@ -390,6 +395,10 @@ describe("deleteUserAccountData", () => { expect(tables.workflow_open_source_submissions).toEqual([]); expect(ids(tables.workflows)).toEqual(["w-other"]); + // Audit rows carry PII (email, titles, prompt excerpts) and must be + // purged on account deletion — only the other user's row survives. + expect(ids(tables.audit_events)).toEqual(["a-other"]); + // Shares by the user and shares to the user's email are both removed. expect(ids(tables.workflow_shares)).toEqual(["ws-keep"]); diff --git a/backend/src/lib/audit.ts b/backend/src/lib/audit.ts index fa6d8e048..b4293acae 100644 --- a/backend/src/lib/audit.ts +++ b/backend/src/lib/audit.ts @@ -52,6 +52,14 @@ type TurnEvent = { document_id?: string; title?: string; workflow_id?: string; + // doc_replicated nests each produced copy under `copies` (see the + // AssistantEvent union in chat/streaming.ts); the top-level `filename` is the + // *source* document and there is no top-level document_id. + copies?: Array<{ + new_filename?: string; + document_id?: string; + version_id?: string; + }>; }; /** @@ -87,16 +95,34 @@ export async function recordChatTurn( }); for (const raw of events ?? []) { const ev = raw as TurnEvent; + // A single doc_replicated event can produce several copies; emit one + // document.generated row per copy, reading the copy's own new_filename and + // document_id rather than the (source) top-level filename / absent id. + if (ev?.type === "doc_replicated") { + for (const copy of ev.copies ?? []) { + await recordAudit(db, { + userId: base.userId, + userEmail: base.userEmail, + action: "document.generated", + title: copy.new_filename ?? null, + surface, + projectId: base.projectId ?? null, + chatId: base.chatId, + documentId: copy.document_id ?? null, + model: base.model, + detail: null, + }); + } + continue; + } const action = ev?.type === "doc_created" ? "document.generated" : ev?.type === "doc_edited" ? "document.edited" - : ev?.type === "doc_replicated" - ? "document.generated" - : ev?.type === "workflow_applied" - ? "workflow.applied" - : null; + : ev?.type === "workflow_applied" + ? "workflow.applied" + : null; if (!action) continue; await recordAudit(db, { userId: base.userId, diff --git a/backend/src/lib/userDataCleanup.ts b/backend/src/lib/userDataCleanup.ts index aa812e619..b71adbeb6 100644 --- a/backend/src/lib/userDataCleanup.ts +++ b/backend/src/lib/userDataCleanup.ts @@ -333,6 +333,11 @@ export async function deleteUserAccountData( .eq("shared_with_email", userEmail.trim().toLowerCase()) : Promise.resolve({ error: null }), db.from("workflows").delete().eq("user_id", userId), + // Audit rows carry the user's id, email, chat/document titles and prompt + // excerpts. Deleting them here keeps account erasure complete (GDPR) — + // they are keyed by user_id and would otherwise be orphaned forever + // after the chats/projects they reference are gone. + db.from("audit_events").delete().eq("user_id", userId), db.from("projects").delete().eq("user_id", userId), ]; diff --git a/backend/src/lib/userDataExport.ts b/backend/src/lib/userDataExport.ts index 76749f0c6..6991407ed 100644 --- a/backend/src/lib/userDataExport.ts +++ b/backend/src/lib/userDataExport.ts @@ -180,6 +180,7 @@ export async function buildUserAccountExport( tabularReviews, sharedProjects, sharedTabularReviews, + auditEvents, ] = await Promise.all([ selectAll(db, "user_profiles", (query) => query.eq("user_id", userId)), loadApiKeyStatus(db, userId), @@ -238,6 +239,11 @@ export async function buildUserAccountExport( "id, user_id, project_id, title, practice, created_at, updated_at", ) : Promise.resolve([]), + selectAll(db, "audit_events", (query) => + query + .eq("user_id", userId) + .order("created_at", { ascending: true }), + ), ]); const projectIds = idsFrom(projects); @@ -281,5 +287,6 @@ export async function buildUserAccountExport( projects: sharedProjects, tabular_reviews: sharedTabularReviews, }, + audit_events: auditEvents, }; } diff --git a/backend/src/routes/__tests__/audit.test.ts b/backend/src/routes/__tests__/audit.test.ts new file mode 100644 index 000000000..b7605459c --- /dev/null +++ b/backend/src/routes/__tests__/audit.test.ts @@ -0,0 +1,179 @@ +import { describe, it, expect } from "vitest"; +import { + csvCell, + parseQuery, + queryEvents, + accessibleProjectIds, +} from "../audit"; + +// --------------------------------------------------------------------------- +// csvCell — spreadsheet formula-injection escaping (F3) +// --------------------------------------------------------------------------- + +describe("csvCell", () => { + it("prefixes a single quote to values that begin with a formula trigger", () => { + for (const trigger of ["=", "+", "-", "@", "\t", "\r"]) { + const payload = `${trigger}HYPERLINK("http://evil","x")`; + const cell = csvCell(payload); + // Leading quote neutralizes evaluation; the whole value is then + // quoted because it contains characters requiring CSV quoting. + expect(cell.startsWith(`"'${trigger}`)).toBe(true); + } + }); + + it("neutralizes a bare leading = even without other special chars", () => { + expect(csvCell("=1")).toBe("'=1"); + }); + + it("quotes and escapes embedded quotes, commas, and newlines", () => { + expect(csvCell('a,b')).toBe('"a,b"'); + expect(csvCell('he said "hi"')).toBe('"he said ""hi"""'); + expect(csvCell("line1\r\nline2")).toBe('"line1\r\nline2"'); + }); + + it("leaves ordinary values untouched and renders null as empty", () => { + expect(csvCell("brief.docx")).toBe("brief.docx"); + expect(csvCell(null)).toBe(""); + expect(csvCell(undefined)).toBe(""); + }); +}); + +// --------------------------------------------------------------------------- +// parseQuery — page clamping (F7) + date validation (F8) +// --------------------------------------------------------------------------- + +describe("parseQuery", () => { + it("clamps an absurd page so the offset can't overflow", () => { + const result = parseQuery({ page: "99999999999999" }, 50); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.query.page).toBe(100_000); + // offset stays well within Postgres' integer range. + expect((result.query.page - 1) * result.query.limit).toBeLessThan( + 2_147_483_647, + ); + } + }); + + it("floors non-positive or non-numeric pages to 1", () => { + for (const page of ["0", "-5", "abc", ""]) { + const result = parseQuery({ page }, 50); + expect(result.ok && result.query.page).toBe(1); + } + }); + + it("rejects from/to that are not bare YYYY-MM-DD", () => { + expect(parseQuery({ to: "2026-07-30T12:00:00Z" }, 50)).toEqual({ + ok: false, + error: expect.stringContaining("to"), + }); + expect(parseQuery({ from: "not-a-date" }, 50)).toEqual({ + ok: false, + error: expect.stringContaining("from"), + }); + }); + + it("accepts well-formed dates and trims free-text filters", () => { + const result = parseQuery( + { from: "2026-07-01", to: "2026-07-31", q: " hello ", action: " chat.message " }, + 50, + ); + expect(result).toMatchObject({ + ok: true, + query: { + from: "2026-07-01", + to: "2026-07-31", + q: "hello", + action: "chat.message", + }, + }); + }); +}); + +// --------------------------------------------------------------------------- +// queryEvents / accessibleProjectIds — visibility scoping +// --------------------------------------------------------------------------- + +/** + * Chainable Supabase mock. `projects` select responses are keyed by whether the + * query used .eq (owned) or .contains (shared). The audit_events builder + * records the .or / .eq filter it was given so tests can assert scoping. + */ +function makeDb(owned: string[], shared: string[]) { + const calls: { or?: string; eq?: [string, unknown] } = {}; + + function projectsBuilder() { + let mode: "owned" | "shared" = "owned"; + const b: any = { + select: () => b, + eq: () => { + mode = "owned"; + return b; + }, + contains: () => { + mode = "shared"; + return b; + }, + then: (resolve: (v: { data: { id: string }[] }) => unknown) => + Promise.resolve({ + data: (mode === "owned" ? owned : shared).map((id) => ({ id })), + }).then(resolve), + }; + return b; + } + + function auditBuilder() { + const b: any = { + select: () => b, + or: (expr: string) => { + calls.or = expr; + return b; + }, + eq: (col: string, val: unknown) => { + calls.eq = [col, val]; + return b; + }, + ilike: () => b, + gte: () => b, + lte: () => b, + order: () => b, + range: () => + Promise.resolve({ data: [], error: null, count: 0 }), + }; + return b; + } + + const db = { + from(table: string) { + return table === "projects" ? projectsBuilder() : auditBuilder(); + }, + }; + return { db: db as any, calls }; +} + +describe("queryEvents visibility scoping", () => { + const query = { page: 1, limit: 50 } as const; + + it("scopes to own events OR accessible project events (owned + shared)", async () => { + const { db, calls } = makeDb(["p-own"], ["p-shared"]); + await queryEvents(db, "u1", "u1@example.com", query); + expect(calls.or).toBe("user_id.eq.u1,project_id.in.(p-own,p-shared)"); + expect(calls.eq).toBeUndefined(); + }); + + it("falls back to own-events-only when no projects are accessible", async () => { + const { db, calls } = makeDb([], []); + await queryEvents(db, "u1", "u1@example.com", query); + expect(calls.or).toBeUndefined(); + expect(calls.eq).toEqual(["user_id", "u1"]); + }); + + it("de-duplicates owned and shared project ids", async () => { + const both = await accessibleProjectIds( + makeDb(["p1", "p2"], ["p2", "p3"]).db, + "u1", + "u1@example.com", + ); + expect([...both].sort()).toEqual(["p1", "p2", "p3"]); + }); +}); diff --git a/backend/src/routes/audit.ts b/backend/src/routes/audit.ts index aedde11c3..770ec4677 100644 --- a/backend/src/routes/audit.ts +++ b/backend/src/routes/audit.ts @@ -3,7 +3,7 @@ // that are shared with their email. import { Router } from "express"; -import { requireAuth } from "../middleware/auth"; +import { requireAuth, requireMfaIfEnrolled } from "../middleware/auth"; import { createServerSupabase } from "../lib/supabase"; export const auditRouter = Router(); @@ -11,8 +11,13 @@ auditRouter.use(requireAuth); const PAGE_SIZE = 50; const EXPORT_LIMIT = 2000; +// Clamp the requested page. Without a bound, ?page=99999999999999 produces an +// offset of ~5e15, which PostgREST rejects and surfaces as a 500. Capping the +// page keeps the offset well inside Postgres' integer range. +const MAX_PAGE = 100_000; +const DATE_RE = /^\d{4}-\d{2}-\d{2}$/; -async function accessibleProjectIds( +export async function accessibleProjectIds( db: ReturnType, userId: string, email: string | undefined, @@ -39,21 +44,42 @@ type AuditQuery = { limit: number; }; -function parseQuery(raw: Record, limit: number): AuditQuery { +export type ParseQueryResult = + | { ok: true; query: AuditQuery } + | { ok: false; error: string }; + +export function parseQuery( + raw: Record, + limit: number, +): ParseQueryResult { const str = (v: unknown) => typeof v === "string" && v.trim() ? v.trim() : undefined; - const page = Math.max(Number.parseInt(String(raw.page ?? "1"), 10) || 1, 1); + // Clamp page into [1, MAX_PAGE] so a huge ?page= can't overflow the offset. + const parsedPage = Number.parseInt(String(raw.page ?? "1"), 10) || 1; + const page = Math.min(Math.max(parsedPage, 1), MAX_PAGE); + const from = str(raw.from); + const to = str(raw.to); + // Date filters come from and are compared as calendar + // days. Reject anything that isn't a bare YYYY-MM-DD — a value like + // "2026-07-30T12:00:00Z" would become "...ZT23:59:59.999Z" (F8) and 500. + if (from && !DATE_RE.test(from)) + return { ok: false, error: "Invalid 'from' date; expected YYYY-MM-DD" }; + if (to && !DATE_RE.test(to)) + return { ok: false, error: "Invalid 'to' date; expected YYYY-MM-DD" }; return { - q: str(raw.q)?.slice(0, 200), - action: str(raw.action)?.slice(0, 60), - from: str(raw.from), - to: str(raw.to), - page, - limit, + ok: true, + query: { + q: str(raw.q)?.slice(0, 200), + action: str(raw.action)?.slice(0, 60), + from, + to, + page, + limit, + }, }; } -async function queryEvents( +export async function queryEvents( db: ReturnType, userId: string, email: string | undefined, @@ -84,22 +110,32 @@ auditRouter.get("/", async (req, res) => { const userId = res.locals.userId as string; const email = res.locals.userEmail as string | undefined; const db = createServerSupabase(); - const q = parseQuery(req.query as Record, PAGE_SIZE); + const parsed = parseQuery(req.query as Record, PAGE_SIZE); + if (!parsed.ok) return void res.status(400).json({ detail: parsed.error }); + const q = parsed.query; const { data, error, count } = await queryEvents(db, userId, email, q); if (error) return void res.status(500).json({ detail: error.message }); res.json({ events: data ?? [], total: count ?? 0, page: q.page, pageSize: PAGE_SIZE }); }); -function csvCell(v: unknown): string { - const s = v == null ? "" : String(v); - return /[",\n]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s; +export function csvCell(v: unknown): string { + let s = v == null ? "" : String(v); + // Neutralize spreadsheet formula injection: Excel/Sheets evaluate any cell + // whose text begins with = + - @, a tab or a carriage return as a formula on + // open. Titles are attacker-controllable across shared projects, so an + // =HYPERLINK(...) payload would execute in the victim's spreadsheet. Prefix a + // single quote to force the value to be treated as literal text. + if (/^[=+\-@\t\r]/.test(s)) s = `'${s}`; + return /[",\n\r]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s; } -auditRouter.get("/export", async (req, res) => { +auditRouter.get("/export", requireMfaIfEnrolled, async (req, res) => { const userId = res.locals.userId as string; const email = res.locals.userEmail as string | undefined; const db = createServerSupabase(); - const q = parseQuery(req.query as Record, EXPORT_LIMIT); + const parsed = parseQuery(req.query as Record, EXPORT_LIMIT); + if (!parsed.ok) return void res.status(400).json({ detail: parsed.error }); + const q = parsed.query; q.page = 1; const { data, error } = await queryEvents(db, userId, email, q); if (error) return void res.status(500).json({ detail: error.message }); diff --git a/backend/src/routes/projects.ts b/backend/src/routes/projects.ts index ea24f0711..b8982f3f8 100644 --- a/backend/src/routes/projects.ts +++ b/backend/src/routes/projects.ts @@ -1,6 +1,7 @@ import { Router } from "express"; import { requireAuth } from "../middleware/auth"; import { createServerSupabase } from "../lib/supabase"; +import { recordAudit } from "../lib/audit"; import { createClient } from "@supabase/supabase-js"; import { attachActiveVersionPaths, @@ -1047,6 +1048,19 @@ export async function handleDocumentUpload( active_version_number: 1, } : updated; + // Audit the project upload. The library/assistant upload path + // (documents.ts) records this too; this handler is the project-scoped + // duplicate and was previously uninstrumented, so project uploads never + // appeared in history. + void recordAudit(db, { + userId, + userEmail: res.locals.userEmail as string | undefined, + action: "document.uploaded", + title: filename, + surface: projectId ? "project" : "assistant", + projectId, + documentId: (updated as { id?: string } | null)?.id ?? null, + }); return void res.status(201).json(responseDoc); } catch (e) { await db.from("documents").update({ status: "error" }).eq("id", doc.id); diff --git a/frontend/src/app/(pages)/history/page.tsx b/frontend/src/app/(pages)/history/page.tsx index 621c44b77..5ebe90678 100644 --- a/frontend/src/app/(pages)/history/page.tsx +++ b/frontend/src/app/(pages)/history/page.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { Download, Loader2, Search } from "lucide-react"; import { getAuditHistory, @@ -60,30 +60,53 @@ function HistoryTable() { const [total, setTotal] = useState(0); const [page, setPage] = useState(1); const [loading, setLoading] = useState(true); + const [error, setError] = useState(false); const [exporting, setExporting] = useState(false); const [q, setQ] = useState(""); const [action, setAction] = useState(""); const [from, setFrom] = useState(""); const [to, setTo] = useState(""); + // Tracks the in-flight request so a newer load can abort an older one. + // Without this, rapid filter changes race: a slow first response could land + // after a faster second one and overwrite it with stale rows, and a + // "Load more" issued mid-filter-change would append a page from the old + // filter (duplicate key={e.id} warnings and wrong rows). + const controllerRef = useRef(null); + const load = useCallback( async (nextPage: number, append: boolean) => { + controllerRef.current?.abort(); + const controller = new AbortController(); + controllerRef.current = controller; setLoading(true); try { - const out = await getAuditHistory({ - q: q || undefined, - action: action || undefined, - from: from || undefined, - to: to || undefined, - page: nextPage, - }); + const out = await getAuditHistory( + { + q: q || undefined, + action: action || undefined, + from: from || undefined, + to: to || undefined, + page: nextPage, + }, + controller.signal, + ); + if (controller.signal.aborted) return; + setError(false); setEvents((cur) => (append ? [...cur, ...out.events] : out.events)); setTotal(out.total); setPage(nextPage); - } catch { - if (!append) setEvents([]); + } catch (err) { + // A superseded request rejects with an AbortError — ignore it so + // it neither clears the newer results nor shows a false error. + if (controller.signal.aborted) return; + if (!append) { + setEvents([]); + setTotal(0); + } + setError(true); } finally { - setLoading(false); + if (controllerRef.current === controller) setLoading(false); } }, [q, action, from, to], @@ -91,6 +114,7 @@ function HistoryTable() { useEffect(() => { void load(1, false); + return () => controllerRef.current?.abort(); // eslint-disable-next-line react-hooks/exhaustive-deps -- reload on filter change only }, [action, from, to]); @@ -242,7 +266,24 @@ function HistoryTable() { ); })} - {!loading && events.length === 0 && ( + {!loading && error && events.length === 0 && ( + + + Couldn't load your history.{" "} + + + + )} + {!loading && !error && events.length === 0 && ( ; -/** History nav icon (Clue custom) — same asset pattern as AppSidebarSkeuoIcons. */ +/** History nav icon — same asset pattern as AppSidebarSkeuoIcons. */ export function HistorySkeuoIcon({ className, ...props }: IconProps) { return ( | null; } -export async function getAuditHistory(params: { - q?: string; - action?: string; - from?: string; - to?: string; - page?: number; -}): Promise<{ events: AuditEvent[]; total: number; page: number; pageSize: number }> { +export async function getAuditHistory( + params: { + q?: string; + action?: string; + from?: string; + to?: string; + page?: number; + }, + signal?: AbortSignal, +): Promise<{ events: AuditEvent[]; total: number; page: number; pageSize: number }> { const qs = new URLSearchParams(); if (params.q) qs.set("q", params.q); if (params.action) qs.set("action", params.action); if (params.from) qs.set("from", params.from); if (params.to) qs.set("to", params.to); if (params.page) qs.set("page", String(params.page)); - return apiRequest(`/audit?${qs.toString()}`); + return apiRequest(`/audit?${qs.toString()}`, { signal }); } export async function exportAuditHistory(params: {