fix(core-audit): keyed 128-bit pseudonyms + salted DSR certificate
pseudonymize() used an unkeyed sha256 over 'salt:id' truncated to 64 bits, and the DSR deletion certificate hashed the raw subjectId with no salt at all (audit finding A13). Both now use HMAC-SHA256 keyed by AUDIT_PSEUDONYM_SALT, truncated to 128 bits. Rotation semantics are documented on pseudonymize(): a key rotation changes future pseudonyms only — stored rows keep old tokens and erasure still matches by real actorId — and the certificate change likewise affects new certificates only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -25,7 +25,10 @@ describe("PayloadAuditLog.record", () => {
|
|||||||
await log.record(sample);
|
await log.record(sample);
|
||||||
|
|
||||||
expect(mockCreate).toHaveBeenCalledOnce();
|
expect(mockCreate).toHaveBeenCalledOnce();
|
||||||
const call = mockCreate.mock.calls[0]![0] as { collection: string; data: Record<string, unknown> };
|
const call = mockCreate.mock.calls[0]![0] as {
|
||||||
|
collection: string;
|
||||||
|
data: Record<string, unknown>;
|
||||||
|
};
|
||||||
expect(call.collection).toBe("audit-logs");
|
expect(call.collection).toBe("audit-logs");
|
||||||
expect(call.data.actorId).toBe("user_1");
|
expect(call.data.actorId).toBe("user_1");
|
||||||
expect(call.data.action).toBe("UPDATE");
|
expect(call.data.action).toBe("UPDATE");
|
||||||
@@ -101,14 +104,21 @@ describe("PayloadAuditLog.eraseSubject", () => {
|
|||||||
// update called for each doc
|
// update called for each doc
|
||||||
expect(mockUpdate).toHaveBeenCalledTimes(2);
|
expect(mockUpdate).toHaveBeenCalledTimes(2);
|
||||||
const updateCalls = mockUpdate.mock.calls as Array<
|
const updateCalls = mockUpdate.mock.calls as Array<
|
||||||
[{ collection: string; id: string; data: Record<string, unknown>; overrideAccess: boolean }]
|
[
|
||||||
|
{
|
||||||
|
collection: string;
|
||||||
|
id: string;
|
||||||
|
data: Record<string, unknown>;
|
||||||
|
overrideAccess: boolean;
|
||||||
|
},
|
||||||
|
]
|
||||||
>;
|
>;
|
||||||
expect(updateCalls[0]![0].id).toBe("doc_a");
|
expect(updateCalls[0]![0].id).toBe("doc_a");
|
||||||
expect(updateCalls[1]![0].id).toBe("doc_b");
|
expect(updateCalls[1]![0].id).toBe("doc_b");
|
||||||
|
|
||||||
// both updates replace actorId with the same pseudonym
|
// both updates replace actorId with the same pseudonym
|
||||||
const pseudonym = updateCalls[0]![0].data["actorId"] as string;
|
const pseudonym = updateCalls[0]![0].data["actorId"] as string;
|
||||||
expect(pseudonym).toMatch(/^erased-[0-9a-f]{16}$/);
|
expect(pseudonym).toMatch(/^erased-[0-9a-f]{32}$/);
|
||||||
expect(updateCalls[1]![0].data["actorId"]).toBe(pseudonym);
|
expect(updateCalls[1]![0].data["actorId"]).toBe(pseudonym);
|
||||||
|
|
||||||
// overrideAccess bypasses the append-only rule
|
// overrideAccess bypasses the append-only rule
|
||||||
@@ -118,7 +128,9 @@ describe("PayloadAuditLog.eraseSubject", () => {
|
|||||||
it("mode='pseudonymize' with no matching docs does not call update", async () => {
|
it("mode='pseudonymize' with no matching docs does not call update", async () => {
|
||||||
const mockFind = vi.fn().mockResolvedValue({ docs: [] });
|
const mockFind = vi.fn().mockResolvedValue({ docs: [] });
|
||||||
const mockUpdate = vi.fn();
|
const mockUpdate = vi.fn();
|
||||||
const mockGetPayload = vi.fn().mockResolvedValue({ find: mockFind, update: mockUpdate });
|
const mockGetPayload = vi
|
||||||
|
.fn()
|
||||||
|
.mockResolvedValue({ find: mockFind, update: mockUpdate });
|
||||||
const log = new PayloadAuditLog({} as never, mockGetPayload);
|
const log = new PayloadAuditLog({} as never, mockGetPayload);
|
||||||
|
|
||||||
await log.eraseSubject("unknown_user", "pseudonymize");
|
await log.eraseSubject("unknown_user", "pseudonymize");
|
||||||
|
|||||||
@@ -21,13 +21,28 @@ describe("pseudonymize", () => {
|
|||||||
expect(result).toMatch(/^erased-/);
|
expect(result).toMatch(/^erased-/);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("produces exactly 16 hex chars after the prefix", () => {
|
it("produces exactly 32 hex chars (128 bits) after the prefix (A13)", () => {
|
||||||
const result = pseudonymize("user_42");
|
const result = pseudonymize("user_42");
|
||||||
const hex = result.slice("erased-".length);
|
const hex = result.slice("erased-".length);
|
||||||
expect(hex).toHaveLength(16);
|
expect(hex).toHaveLength(32);
|
||||||
expect(hex).toMatch(/^[0-9a-f]+$/);
|
expect(hex).toMatch(/^[0-9a-f]+$/);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("matches HMAC-SHA256(key, actorId) - keyed, not a bare hash (A13)", async () => {
|
||||||
|
const { createHmac, createHash } = await import("node:crypto");
|
||||||
|
const expected = createHmac("sha256", "test-salt-1")
|
||||||
|
.update("user_42")
|
||||||
|
.digest("hex")
|
||||||
|
.slice(0, 32);
|
||||||
|
expect(pseudonymize("user_42")).toBe(`erased-` + expected);
|
||||||
|
// and it must NOT be the legacy unkeyed sha256("salt:id") scheme
|
||||||
|
const legacy = createHash("sha256")
|
||||||
|
.update("test-salt-1:user_42")
|
||||||
|
.digest("hex")
|
||||||
|
.slice(0, 32);
|
||||||
|
expect(pseudonymize("user_42")).not.toBe(`erased-` + legacy);
|
||||||
|
});
|
||||||
|
|
||||||
it("is deterministic — same salt + actorId always yields the same token", () => {
|
it("is deterministic — same salt + actorId always yields the same token", () => {
|
||||||
const a = pseudonymize("user_42");
|
const a = pseudonymize("user_42");
|
||||||
const b = pseudonymize("user_42");
|
const b = pseudonymize("user_42");
|
||||||
@@ -53,6 +68,6 @@ describe("pseudonymize", () => {
|
|||||||
delete process.env["AUDIT_PSEUDONYM_SALT"];
|
delete process.env["AUDIT_PSEUDONYM_SALT"];
|
||||||
// Should not throw; just use the fallback.
|
// Should not throw; just use the fallback.
|
||||||
const result = pseudonymize("user_1");
|
const result = pseudonymize("user_1");
|
||||||
expect(result).toMatch(/^erased-[0-9a-f]{16}$/);
|
expect(result).toMatch(/^erased-[0-9a-f]{32}$/);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,22 +1,34 @@
|
|||||||
import { createHash } from "node:crypto";
|
import { createHmac } from "node:crypto";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Produces a stable, irreversible token for a GDPR-erased actorId.
|
* Produces a stable, irreversible token for a GDPR-erased actorId.
|
||||||
*
|
*
|
||||||
* Format: `erased-<first-16-hex-chars-of-sha256(salt:actorId)>`
|
* Format: `erased-<first-32-hex-chars-of-HMAC-SHA256(key, actorId)>` —
|
||||||
|
* 128 bits of a KEYED digest (audit finding A13). The previous scheme was an
|
||||||
|
* unkeyed `sha256("salt:actorId")` truncated to 64 bits, which invited both
|
||||||
|
* brute-force reversal of small id spaces and birthday collisions.
|
||||||
*
|
*
|
||||||
* The salt is read from `AUDIT_PSEUDONYM_SALT` env at call time so that
|
* The HMAC key is read from `AUDIT_PSEUDONYM_SALT` at call time so that
|
||||||
* production binding can pre-validate the var at boot (see `bindAudit`)
|
* production binding can pre-validate the var at boot (see `bindAudit`)
|
||||||
* while tests can override it per-test via `process.env`.
|
* while tests can override it per-test via `process.env`.
|
||||||
*
|
*
|
||||||
* Fallback salt is intentionally weak and labelled so that any token
|
* Rotation expectations (documented, by design):
|
||||||
|
* - Rotating the key changes pseudonyms produced FROM THEN ON only. Audit
|
||||||
|
* rows already pseudonymized keep tokens derived from the previous key;
|
||||||
|
* nothing re-keys stored rows, so a subject's pre- and post-rotation
|
||||||
|
* tokens no longer correlate. That linkage break is acceptable — the
|
||||||
|
* token's only job is severing PII linkage, not long-term correlation.
|
||||||
|
* - Re-erasing a subject after rotation still works: erasure matches rows
|
||||||
|
* by the REAL actorId, not by a previous pseudonym.
|
||||||
|
* - Rotate by replacing the env value (e.g. `openssl rand -hex 32`); keep
|
||||||
|
* retired keys only if you have an explicit need to re-correlate old rows.
|
||||||
|
*
|
||||||
|
* Fallback key is intentionally weak and labelled so that any token
|
||||||
* produced with it is recognisable as a dev/test artefact.
|
* produced with it is recognisable as a dev/test artefact.
|
||||||
*/
|
*/
|
||||||
export function pseudonymize(actorId: string): string {
|
export function pseudonymize(actorId: string): string {
|
||||||
const salt =
|
const key =
|
||||||
process.env["AUDIT_PSEUDONYM_SALT"] ?? "dev-fallback-salt-replace-in-prod";
|
process.env["AUDIT_PSEUDONYM_SALT"] ?? "dev-fallback-salt-replace-in-prod";
|
||||||
const hash = createHash("sha256")
|
const digest = createHmac("sha256", key).update(actorId).digest("hex");
|
||||||
.update(`${salt}:${actorId}`)
|
return `erased-${digest.slice(0, 32)}`;
|
||||||
.digest("hex");
|
|
||||||
return `erased-${hash.slice(0, 16)}`;
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,7 +1,6 @@
|
|||||||
import { getPayload as _getPayload } from "payload";
|
import { getPayload as _getPayload } from "payload";
|
||||||
import type { SanitizedConfig } from "payload";
|
import type { SanitizedConfig } from "payload";
|
||||||
import { randomUUID } from "node:crypto";
|
import { randomUUID, createHmac } from "node:crypto";
|
||||||
import { createHash } from "node:crypto";
|
|
||||||
import type { AuditLogProtocol } from "@repo/core-shared/di";
|
import type { AuditLogProtocol } from "@repo/core-shared/di";
|
||||||
import {
|
import {
|
||||||
RETENTION_TOMBSTONE_FIELD,
|
RETENTION_TOMBSTONE_FIELD,
|
||||||
@@ -321,9 +320,20 @@ export class PayloadDataDelete implements IDataDelete {
|
|||||||
correlationId: string,
|
correlationId: string,
|
||||||
affected: DeletionAffected[],
|
affected: DeletionAffected[],
|
||||||
): DeletionCertificate {
|
): DeletionCertificate {
|
||||||
|
// Salted, keyed pseudonym (audit finding A13): the certificate used to
|
||||||
|
// hash the raw subjectId with NO salt (truncated to 64 bits), letting a
|
||||||
|
// certificate holder brute-force small id spaces offline. Now
|
||||||
|
// HMAC-SHA256 keyed by the operator secret AUDIT_PSEUDONYM_SALT (the
|
||||||
|
// same secret the audit-log pseudonymizer uses), truncated to 128 bits.
|
||||||
|
// NOTE: this changes tokens on FUTURE certificates only — certificates
|
||||||
|
// issued under the old scheme keep their historical value, and rotating
|
||||||
|
// the key likewise affects only certificates issued afterwards.
|
||||||
|
const certKey =
|
||||||
|
process.env["AUDIT_PSEUDONYM_SALT"] ??
|
||||||
|
"dev-fallback-salt-replace-in-prod";
|
||||||
const certSubjectId =
|
const certSubjectId =
|
||||||
mode === "cascade-hard"
|
mode === "cascade-hard"
|
||||||
? `erased-${createHash("sha256").update(subjectId).digest("hex").slice(0, 16)}`
|
? `erased-${createHmac("sha256", certKey).update(subjectId).digest("hex").slice(0, 32)}`
|
||||||
: subjectId;
|
: subjectId;
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
Reference in New Issue
Block a user