Review fixes for #271: audit_events RLS/erasure/CSV-injection + tests

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;

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);

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);

backend/src/lib/__tests__/audit.test.ts

@@ -0,0 +1,87 @@
+import { describe, it, expect } from "vitest";
+import { recordChatTurn } from "../audit";
+
+type Insert = Record<string, unknown>;
+
+/**
+ * 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"]);
+    });
+});

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"]);
 

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,

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),
     ];
 

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,
     };
 }