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>
This commit is contained in:
@@ -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");
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user