security: redact sensitive fields in audit DB entries (#46)

createAuditEntry now deep-walks before/after/metadata and replaces
values of password, newPassword, currentPassword, passwordHash, token,
accessToken, refreshToken, sessionToken, apiKey, authorization, cookie,
secret, totpSecret, backupCode(s) with "[REDACTED]" before the JSONB
write.

The pino logger already redacts these paths for stdout (see
lib/logger.ts), but DB writes had no equivalent guard — the AI chat
loop at assistant-chat-loop.ts:265 blindly stores parsedArgs from tool
calls (e.g. set_user_password, create_user) into the AuditLog table.

Matching is case-insensitive; nested objects and arrays are recursed to
a depth of 8. Diffs are computed post-redaction so UPDATE entries that
only changed a sensitive field are correctly collapsed to no-op.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

packages/api/src/__tests__/audit-redaction.test.ts

@@ -0,0 +1,177 @@
+import { describe, expect, it, vi } from "vitest";
+import { __test__, createAuditEntry } from "../lib/audit.js";
+
+const { redactSensitive } = __test__;
+
+describe("audit log redaction", () => {
+  describe("redactSensitive", () => {
+    it("redacts top-level password fields", () => {
+      const result = redactSensitive({ userId: "u1", password: "hunter2" });
+      expect(result).toEqual({ userId: "u1", password: "[REDACTED]" });
+    });
+
+    it("redacts nested password fields", () => {
+      const result = redactSensitive({
+        params: { userId: "u1", password: "hunter2" },
+        executed: true,
+      });
+      expect(result).toEqual({
+        params: { userId: "u1", password: "[REDACTED]" },
+        executed: true,
+      });
+    });
+
+    it("redacts password inside arrays", () => {
+      const result = redactSensitive({
+        users: [
+          { id: "1", password: "secret" },
+          { id: "2", password: "other" },
+        ],
+      });
+      expect(result).toEqual({
+        users: [
+          { id: "1", password: "[REDACTED]" },
+          { id: "2", password: "[REDACTED]" },
+        ],
+      });
+    });
+
+    it("is case-insensitive", () => {
+      const result = redactSensitive({
+        Password: "x",
+        PASSWORD: "y",
+        newPassword: "z",
+        currentPassword: "a",
+      });
+      expect(result).toEqual({
+        Password: "[REDACTED]",
+        PASSWORD: "[REDACTED]",
+        newPassword: "[REDACTED]",
+        currentPassword: "[REDACTED]",
+      });
+    });
+
+    it("redacts tokens, secrets, and cookies", () => {
+      const result = redactSensitive({
+        token: "t",
+        accessToken: "a",
+        refreshToken: "r",
+        apiKey: "k",
+        secret: "s",
+        totpSecret: "ts",
+        authorization: "Bearer x",
+        cookie: "sid=abc",
+      });
+      for (const v of Object.values(result as Record<string, unknown>)) {
+        expect(v).toBe("[REDACTED]");
+      }
+    });
+
+    it("leaves non-sensitive fields untouched", () => {
+      const result = redactSensitive({ name: "Alice", email: "a@b.c", count: 42, flag: true });
+      expect(result).toEqual({ name: "Alice", email: "a@b.c", count: 42, flag: true });
+    });
+
+    it("handles null, undefined, and primitives", () => {
+      expect(redactSensitive(null)).toBe(null);
+      expect(redactSensitive(undefined)).toBe(undefined);
+      expect(redactSensitive("string")).toBe("string");
+      expect(redactSensitive(123)).toBe(123);
+    });
+
+    it("stops recursion at MAX_REDACT_DEPTH", () => {
+      // Build a ~15-deep nested object; redaction should still work near the
+      // top but bail past the depth limit without throwing.
+      let v: Record<string, unknown> = { password: "leaf" };
+      for (let i = 0; i < 15; i++) {
+        v = { nested: v };
+      }
+      expect(() => redactSensitive(v)).not.toThrow();
+    });
+  });
+
+  describe("createAuditEntry", () => {
+    it("redacts passwords in `after` before the DB write", async () => {
+      const create = vi.fn().mockResolvedValue({});
+      const db = { auditLog: { create } };
+
+      await createAuditEntry({
+        db: db as never,
+        entityType: "AiToolExecution",
+        entityId: "call_1",
+        action: "CREATE",
+        after: { params: { userId: "u1", password: "cleartext" }, executed: true },
+      });
+
+      expect(create).toHaveBeenCalledTimes(1);
+      const data = create.mock.calls[0]![0]!.data;
+      const changes = data.changes as { after?: { params?: { password?: string } } };
+      expect(changes.after?.params?.password).toBe("[REDACTED]");
+      expect(changes.after?.params).toMatchObject({ userId: "u1" });
+    });
+
+    it("redacts passwords in before/after when non-sensitive fields also changed", async () => {
+      const create = vi.fn().mockResolvedValue({});
+      const db = { auditLog: { create } };
+
+      await createAuditEntry({
+        db: db as never,
+        entityType: "User",
+        entityId: "u1",
+        action: "UPDATE",
+        before: { password: "old", name: "Alice" },
+        after: { password: "new", name: "Bob" },
+      });
+
+      expect(create).toHaveBeenCalledTimes(1);
+      const changes = create.mock.calls[0]![0]!.data.changes as {
+        before?: Record<string, unknown>;
+        after?: Record<string, unknown>;
+        diff?: Record<string, { old: unknown; new: unknown }>;
+      };
+      expect(changes.before?.["password"]).toBe("[REDACTED]");
+      expect(changes.after?.["password"]).toBe("[REDACTED]");
+      // The name change survives in the diff, but the password diff collapses
+      // (both values are the same placeholder).
+      expect(changes.diff).toEqual({ name: { old: "Alice", new: "Bob" } });
+    });
+
+    it("skips UPDATE when both snapshots redact to the same value (empty diff)", async () => {
+      const create = vi.fn().mockResolvedValue({});
+      const db = { auditLog: { create } };
+
+      await createAuditEntry({
+        db: db as never,
+        entityType: "User",
+        entityId: "u1",
+        action: "UPDATE",
+        before: { password: "old" },
+        after: { password: "new" },
+      });
+
+      // Both redact to [REDACTED], diff is empty, create should NOT be called.
+      expect(create).not.toHaveBeenCalled();
+    });
+
+    it("redacts sensitive fields in metadata", async () => {
+      const create = vi.fn().mockResolvedValue({});
+      const db = { auditLog: { create } };
+
+      await createAuditEntry({
+        db: db as never,
+        entityType: "Webhook",
+        entityId: "wh_1",
+        action: "CREATE",
+        after: { url: "https://example.com/hook" },
+        metadata: { signingSecret: "ss", apiKey: "leak" },
+      });
+
+      const changes = create.mock.calls[0]![0]!.data.changes as {
+        metadata?: Record<string, unknown>;
+      };
+      expect(changes.metadata?.["apiKey"]).toBe("[REDACTED]");
+      // signingSecret is not in the set — verify the list is intentional
+      expect(changes.metadata?.["signingSecret"]).toBe("ss");
+    });
+  });
+});

packages/api/src/lib/audit.ts

@@ -20,6 +20,61 @@ interface CreateAuditEntryParams {
 
 const INTERNAL_FIELDS = new Set(["id", "createdAt", "updatedAt"]);
 
+// Field names whose values are never safe to persist into the audit log.
+// Matching is case-insensitive and applied at every level of the object graph.
+const SENSITIVE_FIELD_NAMES = new Set([
+  "password",
+  "newpassword",
+  "currentpassword",
+  "oldpassword",
+  "passwordhash",
+  "passwordconfirmation",
+  "confirmpassword",
+  "token",
+  "accesstoken",
+  "refreshtoken",
+  "sessiontoken",
+  "apikey",
+  "authorization",
+  "cookie",
+  "secret",
+  "totpsecret",
+  "backupcode",
+  "backupcodes",
+]);
+
+const REDACTED_PLACEHOLDER = "[REDACTED]";
+const MAX_REDACT_DEPTH = 8;
+
+/**
+ * Recursively strip values of fields whose names appear in SENSITIVE_FIELD_NAMES.
+ * Used to prevent password/token leaks into the audit log JSONB column.
+ *
+ * The pino logger has its own redact config for stdout; this function is the
+ * DB-write equivalent.
+ */
+function redactSensitive(value: unknown, depth: number = 0): unknown {
+  if (depth > MAX_REDACT_DEPTH) return value;
+  if (value === null || value === undefined) return value;
+  if (Array.isArray(value)) {
+    return value.map((v) => redactSensitive(v, depth + 1));
+  }
+  if (typeof value === "object") {
+    const out: Record<string, unknown> = {};
+    for (const [k, v] of Object.entries(value as Record<string, unknown>)) {
+      if (SENSITIVE_FIELD_NAMES.has(k.toLowerCase())) {
+        out[k] = REDACTED_PLACEHOLDER;
+      } else {
+        out[k] = redactSensitive(v, depth + 1);
+      }
+    }
+    return out;
+  }
+  return value;
+}
+
+export const __test__ = { redactSensitive, SENSITIVE_FIELD_NAMES };
+
 /**
  * Compare two snapshots and return only the changed fields.
  * Skips internal fields (id, createdAt, updatedAt).
@@ -91,15 +146,34 @@ export function generateSummary(
  */
 export async function createAuditEntry(params: CreateAuditEntryParams): Promise<void> {
   try {
-    const { db, entityType, entityId, entityName, action, userId, before, after, source, metadata } = params;
+    const {
+      db,
+      entityType,
+      entityId,
+      entityName,
+      action,
+      userId,
+      before,
+      after,
+      source,
+      metadata,
+    } = params;
     const auditLog = (db as Partial<PrismaClient>).auditLog;
 
     if (!auditLog || typeof auditLog.create !== "function") {
       return;
     }
 
+    // Redact sensitive field values before anything else — diffs and summaries
+    // must all be derived from already-sanitised snapshots.
+    const safeBefore = before ? (redactSensitive(before) as Record<string, unknown>) : undefined;
+    const safeAfter = after ? (redactSensitive(after) as Record<string, unknown>) : undefined;
+    const safeMetadata = metadata
+      ? (redactSensitive(metadata) as Record<string, unknown>)
+      : undefined;
+
     // Compute diff if both snapshots are available
-    const diff = before && after ? computeDiff(before, after) : undefined;
+    const diff = safeBefore && safeAfter ? computeDiff(safeBefore, safeAfter) : undefined;
 
     // Skip UPDATE entries where nothing actually changed
     if (action === "UPDATE" && diff && Object.keys(diff).length === 0) {
@@ -111,10 +185,10 @@ export async function createAuditEntry(params: CreateAuditEntryParams): Promise<
 
     // Build the changes JSONB payload
     const changes: Record<string, unknown> = {};
-    if (before) changes.before = before;
-    if (after) changes.after = after;
+    if (safeBefore) changes.before = safeBefore;
+    if (safeAfter) changes.after = safeAfter;
     if (diff) changes.diff = diff;
-    if (metadata) changes.metadata = metadata;
+    if (safeMetadata) changes.metadata = safeMetadata;
 
     await auditLog.create({
       data: {
@@ -130,6 +204,9 @@ export async function createAuditEntry(params: CreateAuditEntryParams): Promise<
     });
   } catch (error) {
     // Fire-and-forget: log but never propagate
-    logger.error({ err: error, entityType: params.entityType, entityId: params.entityId }, "Failed to create audit entry");
+    logger.error(
+      { err: error, entityType: params.entityType, entityId: params.entityId },
+      "Failed to create audit entry",
+    );
   }
 }