fix: minimize durable audit metadata

docs/branch-review-ledger.md

@@ -583,3 +583,16 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD
 | 2026-07-17 | codex/performance-latency-remediation-20260717 | f277d13512e85dcaae4f9b94d028a8087e41375a | end-to-end database, network, middleware, client rendering, migration, privacy, and merge-readiness review | Fixed all confirmed performance findings plus three final review blockers: title-vocabulary backfill now installs its privacy trigger first, canonical schema replay preserves hardened trigger-function ACLs, and medication auto-seeding no longer aborts its own owner-cache flight. The isolated worktree also uses Next's supported Webpack fallback only when shared dependencies resolve outside the project. No remaining high-confidence P0-P2 defect was found in the reviewed diff. | Docker scratch replay and drift-manifest regeneration passed; live read-only plans selected lexical, table-fact trigram, and chunk HNSW indexes; exact TypeScript passed; focused Vitest owner-cache 9/9, runner safety 12/12, bundle budget 10/10, offline RAG 291/291, and earlier aggregate unit run 2,657 passed with its sole corrected schema assertion subsequently validated by direct ordering/ACL checks; production build and bundle budget passed at 1,363,382 gzip bytes; targeted Chromium dashboard deferral and NDJSON search passed, while the viewer trace reached all hydration/preview assertions and its corrected download-control selector remains for exact-head hosted UI. No OpenAI calls, live Supabase writes/migrations, deployment, or production mutation ran. |
 | 2026-07-17 | codex/mobile-search-phone-refresh-20260717 (supersedes PR #700) | 42a3e3ce65dc5a0e1dce386e0b91fccd23d13d6c + reviewed follow-up diff | phone universal-search command-panel recovery and merge-readiness review | Recovered the still-useful behavior from PR #700 onto current `main`, including its hydration fix and wide-touch regression coverage. Hosted Production UI then exposed one desktop focus race: capability state intentionally initializes false for hydration safety, but an input could receive focus before the post-hydration effect synchronized the real browser state. The follow-up recomputes the same guarded predicate synchronously on focus; it requires the placement breakpoint plus either a fine pointer or a zero-touch desktop fallback, so wide touch devices remain suppressed while desktop keeps the first command-panel interaction. No remaining high-confidence P0-P2 defect was found in the scoped diff. | Focused Vitest 7/7; `npm run ensure` verified the project at `http://localhost:3751`; hosted static, safety, coverage, build, advisory UI, Semgrep, Gitleaks, and GitGuardian passed; the first hosted Production UI run isolated the nine desktop regressions. The focused browser proof reproduced the desktop race while the wide-touch regression passed, and exact-head hosted Production UI remains required after the focus fix. `format:changed -- --check` and `git diff --check` passed before the final follow-up. No Supabase/OpenAI/product-provider command ran. |
 | 2026-07-17 | final historical branch/worktree cleanup against `origin/main` | 5d195d7ca8752b2ae4006725c6b145c5662bb687 | branch-cleanup | Merged PRs #716 and #717, closed superseded PR #700, and removed the three clean task worktrees. Deleted exact remote refs for `codex/mobile-search-phone-fix-20260717` (`590f32b73`), `claude/audit-findings-review-phgz92` (`ea3b8f95b8`), `codex/chat-forms-import-6914` (`b05da82f82`), `codex/chat-supabase-migration-preflight-b463` (`1ef0faee95`), and `codex/dsm-diagnosis-mode` (`f6cda83ca6`); the merged #716/#717 branches were deleted automatically. Deleted 14 unregistered local refs only after direct-main ancestry, exact ledger deletion-pending proof, or exact merged-PR commit provenance. Final inventory found zero remote branches without an open PR or registered worktree, zero locally merged or exact deletion-pending orphan refs, and no retired target refs or paths. Thirteen non-ancestor local refs and 25 registered worktrees remain preserved because they are backups, patch-unique/unresolved, open-PR-owned, or ownership could not be safely disproved. | Fresh fetch/prune; exact GitHub PR/head/merge associations; cherry-pick-aware logs; DSM PR #661 exact commit/file provenance; exact leased remote deletes; exact-old-value local `update-ref` deletes; clean-worktree and path/process checks; worktree prune; final zero-orphan inventory. The Codex task registry lookup timed out, so ambiguous registered worktrees were conservatively retained. No Supabase, OpenAI, production-data, or live clinical workflow ran. |
+| 2026-07-17 | codex/security-governance-hardening-20260717 | bfa0ed3db0c64a4ca860c83d05024e1c7b29151e | security, privacy, and clinical-governance attack-path review | Confirmed P1 privacy defect: service-role-only `audit_logs` are intentionally retained indefinitely, but upload, rename, and delete paths persisted user-controlled filenames/titles and content hashes in JSON metadata. A patient identifier in a document name therefore survived document deletion in a durable governance log. Fixed the durable write boundary with an action-specific metadata allowlist, removed sensitive call-site fields, added a migration to minimize existing rows when deployed, and added regression coverage. No P0 cross-owner, service-role exposure, signed-URL, or prompt-injection policy bypass was reproduced in the reviewed scope. | Local source/sink inspection; focused test command could not run because the worktree has no Vitest/TypeScript/Next dependencies; scoped Prettier and `git diff --check` passed. Client-bundle scan requires a production `.next/static` build and was not runnable. Provider-backed Supabase/OpenAI/production-readiness checks were not run under the confirmation boundary. |
+
+---
+
+## Review: codex/review-and-fix-top-security-defects (CI fix pass)
+
+- **Date**: 2026-07-17
+- **Branch**: codex/review-and-fix-top-security-defects
+- **HEAD**: a96567eceb0193f6d52e82d28f8700e96dcdbb47
+- **Scope**: pr-ci-fix
+- **Outcome**: Fixed migration replay CI failure. Renamed migration `20260717170000_minimize_audit_log_metadata.sql` → `20260717174000_minimize_audit_log_metadata.sql` to eliminate timestamp collision with `20260717170000_registry_projection_cleanup.sql` already on main. Committed and pushed.
+- **Checks run**: local pr-policy.mjs validation (pass); npm run lint (pass)
+- **Remaining**: PR Policy CI still requires PR body update (## Summary, ## Verification, ## Clinical Governance Preflight, ## Risk and rollout sections) — Cloud Agent token lacks PR write permission; user must update PR body manually.

docs/privacy-impact-assessment.md

@@ -193,12 +193,12 @@ of the most important privacy items before real patient use (PIA-1).
 All three log tables are **owner-stamped** and **RLS-enabled** (owner-read for authenticated users;
 service-role for writes). Redaction is applied centrally at every write site.
 
-| Table                | Raw query stored?     | Redaction mechanism                                                                                                                                   | Other sensitive columns                                                                                                 | RLS                                                       |
-| -------------------- | --------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------- |
-| `rag_queries`        | No (hash placeholder) | `queryTextForStorage` / `normalizedQueryTextForStorage` ([query-privacy.ts:33-39](src/lib/query-privacy.ts)); centralized write in `insertRagQuery`   | `answer` is null by default and stored only with explicit `RAG_PERSIST_ANSWER_TEXT=true`; `source_chunk_ids` (own data) | owner-read, [schema.sql:3932](supabase/schema.sql)        |
-| `rag_query_misses`   | No (hash placeholder) | same helpers; writes in [search/route.ts:558-559](src/app/api/search/route.ts), [interaction/route.ts:88-89](src/app/api/search/interaction/route.ts) | `metadata.query_hash`                                                                                                   | owner-read, [schema.sql:3935](supabase/schema.sql)        |
-| `rag_retrieval_logs` | No (hash placeholder) | same helpers; write at [search/route.ts:556-559](src/app/api/search/route.ts)                                                                         | retrieval telemetry only                                                                                                | owner-read, [schema.sql:3938](supabase/schema.sql)        |
-| `audit_logs`         | N/A (no query text)   | action/resource metadata only; error strings pass through `redactLogValue` ([privacy.ts:5-31](src/lib/privacy.ts))                                    | `owner_id`, `action`, `resource_id`                                                                                     | service-role-only, [schema.sql:3959](supabase/schema.sql) |
+| Table                | Raw query stored?     | Redaction mechanism                                                                                                                                                                                                                                 | Other sensitive columns                                                                                                 | RLS                                                       |
+| -------------------- | --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------- |
+| `rag_queries`        | No (hash placeholder) | `queryTextForStorage` / `normalizedQueryTextForStorage` ([query-privacy.ts:33-39](src/lib/query-privacy.ts)); centralized write in `insertRagQuery`                                                                                                 | `answer` is null by default and stored only with explicit `RAG_PERSIST_ANSWER_TEXT=true`; `source_chunk_ids` (own data) | owner-read, [schema.sql:3932](supabase/schema.sql)        |
+| `rag_query_misses`   | No (hash placeholder) | same helpers; writes in [search/route.ts:558-559](src/app/api/search/route.ts), [interaction/route.ts:88-89](src/app/api/search/interaction/route.ts)                                                                                               | `metadata.query_hash`                                                                                                   | owner-read, [schema.sql:3935](supabase/schema.sql)        |
+| `rag_retrieval_logs` | No (hash placeholder) | same helpers; write at [search/route.ts:556-559](src/app/api/search/route.ts)                                                                                                                                                                       | retrieval telemetry only                                                                                                | owner-read, [schema.sql:3938](supabase/schema.sql)        |
+| `audit_logs`         | N/A (no query text)   | action/resource metadata only; the write boundary allowlists operational metadata and excludes user-controlled filenames/titles/content hashes ([audit.ts](../src/lib/audit.ts)). Migration `20260717170000` minimizes existing rows on deployment. | `owner_id`, `action`, `resource_id`                                                                                     | service-role-only, [schema.sql:3959](supabase/schema.sql) |
 
 ### 5.1 M15 HMAC query-hash fix — verified present, enforced in production
 

src/app/api/documents/[id]/route.ts

@@ -178,7 +178,9 @@ export async function PATCH(request: Request, { params }: { params: Promise<{ id
       action: "document_rename",
       resourceType: "document",
       resourceId: id,
-      metadata: { previousTitle: document.title, newTitle: body.title },
+      // Audit the rename event and resource without retaining user-controlled
+      // titles indefinitely in the service-role audit log.
+      metadata: {},
     });
     return NextResponse.json({ document: updated });
   } catch (error) {
@@ -241,7 +243,9 @@ export async function DELETE(request: Request, { params }: { params: Promise<{ i
       action: "document_delete",
       resourceType: "document",
       resourceId: id,
-      metadata: { title: result.document_title, storageRemoved: cleanup.storageRemoved },
+      // The deleted title can contain patient information. Retain only the
+      // operational cleanup result alongside the resource id and action.
+      metadata: { storageRemoved: cleanup.storageRemoved },
     });
     return NextResponse.json({
       deleted: true,

src/app/api/upload/route.ts

@@ -307,7 +307,10 @@ export async function POST(request: Request) {
       action: "document_upload",
       resourceType: "document",
       resourceId: documentId,
-      metadata: { fileName: file.name, fileType: file.type, fileSize: file.size, contentHash },
+      // `audit_logs` is retained indefinitely. Keep only operational facts there;
+      // the user-controlled filename and content hash remain on the scoped document
+      // record, not in the durable audit trail.
+      metadata: { fileType: file.type, fileSize: file.size },
     });
 
     return NextResponse.json({ document, job }, { status: 201 });

src/lib/audit.ts

@@ -15,6 +15,34 @@ export type AuditLogEntry = {
   metadata?: Record<string, unknown>;
 };
 
+/**
+ * Audit rows are retained indefinitely, so their metadata must be an operational
+ * event summary rather than a second copy of document metadata. In particular,
+ * upload filenames and rename/delete titles are user-controlled and can contain
+ * patient identifiers. Keep this allowlist at the durable-write boundary so a
+ * future caller cannot accidentally reintroduce free text into `audit_logs`.
+ */
+function minimumAuditMetadata(entry: AuditLogEntry): Record<string, boolean | number | string> {
+  const metadata = entry.metadata ?? {};
+
+  switch (entry.action) {
+    case "document_upload": {
+      const fileType = typeof metadata.fileType === "string" ? metadata.fileType : undefined;
+      const fileSize =
+        typeof metadata.fileSize === "number" && Number.isFinite(metadata.fileSize) ? metadata.fileSize : undefined;
+      return {
+        ...(fileType ? { fileType } : {}),
+        ...(fileSize !== undefined ? { fileSize } : {}),
+      };
+    }
+    case "document_delete":
+      return typeof metadata.storageRemoved === "boolean" ? { storageRemoved: metadata.storageRemoved } : {};
+    case "document_rename":
+    case "document_label_change":
+      return {};
+  }
+}
+
 // Minimal structural type so we do not couple to the full Supabase client type.
 type AuditInsertClient = {
   from: (table: string) => {
@@ -29,7 +57,7 @@ export async function writeAuditLog(supabase: AuditInsertClient, entry: AuditLog
       action: entry.action,
       resource_type: entry.resourceType ?? null,
       resource_id: entry.resourceId ?? null,
-      metadata: entry.metadata ?? {},
+      metadata: minimumAuditMetadata(entry),
     });
     if (error) {
       logger.error("Audit log write failed", { action: entry.action, message: error.message });

supabase/migrations/20260717174000_minimize_audit_log_metadata.sql

@@ -0,0 +1,31 @@
+-- Audit logs are retained indefinitely for clinical governance. Historical writes
+-- included user-controlled document names/titles and content hashes in metadata,
+-- which could preserve patient identifiers after the source document was renamed
+-- or deleted. Keep the immutable event/resource record but remove free text and
+-- retain only operational facts needed for audit triage.
+
+update public.audit_logs
+set metadata = case action
+  when 'document_upload' then jsonb_strip_nulls(jsonb_build_object(
+    'fileType', case when jsonb_typeof(metadata -> 'fileType') = 'string' then metadata -> 'fileType' end,
+    'fileSize', case when jsonb_typeof(metadata -> 'fileSize') = 'number' then metadata -> 'fileSize' end
+  ))
+  when 'document_delete' then jsonb_strip_nulls(jsonb_build_object(
+    'storageRemoved', case when jsonb_typeof(metadata -> 'storageRemoved') = 'boolean' then metadata -> 'storageRemoved' end
+  ))
+  else '{}'::jsonb
+end
+where metadata <> '{}'::jsonb;
+
+do $$
+declare
+  unsafe_metadata_count integer;
+begin
+  select count(*) into unsafe_metadata_count
+  from public.audit_logs
+  where metadata ?| array['fileName', 'contentHash', 'previousTitle', 'newTitle', 'title'];
+
+  if unsafe_metadata_count > 0 then
+    raise exception 'audit-log metadata minimization incomplete: % rows still contain user-controlled document text', unsafe_metadata_count;
+  end if;
+end $$;

tests/audit.test.ts

@@ -43,6 +43,42 @@ describe("writeAuditLog", () => {
     });
   });
 
+  it("drops user-controlled document text from indefinitely retained audit metadata", async () => {
+    const client = mockClient(async () => ({ error: null }));
+
+    await writeAuditLog(client, {
+      ownerId: "owner-1",
+      action: "document_upload",
+      resourceId: "doc-1",
+      metadata: {
+        fileName: "Jane Doe MRN 123456.pdf",
+        contentHash: "content-derived-identifier",
+        fileType: "application/pdf",
+        fileSize: 1234,
+        nested: { title: "Patient Jane Doe" },
+      },
+    });
+
+    expect(client.insertSpy).toHaveBeenCalledWith(
+      expect.objectContaining({ metadata: { fileType: "application/pdf", fileSize: 1234 } }),
+    );
+    expect(JSON.stringify(client.insertSpy.mock.calls[0]?.[0])).not.toContain("Jane Doe");
+    expect(JSON.stringify(client.insertSpy.mock.calls[0]?.[0])).not.toContain("content-derived-identifier");
+  });
+
+  it("does not retain previous or replacement titles for rename events", async () => {
+    const client = mockClient(async () => ({ error: null }));
+
+    await writeAuditLog(client, {
+      ownerId: "owner-1",
+      action: "document_rename",
+      resourceId: "doc-1",
+      metadata: { previousTitle: "Jane Doe", newTitle: "Patient Smith" },
+    });
+
+    expect(client.insertSpy).toHaveBeenCalledWith(expect.objectContaining({ metadata: {} }));
+  });
+
   it("does not throw when the insert returns an error", async () => {
     const client = mockClient(async () => ({ error: { message: "insert failed" } }));
     await expect(writeAuditLog(client, { ownerId: "o", action: "document_rename" })).resolves.toBeUndefined();