fix(security): audit-logger PII masking parity with unified logger (DC-110) [glm-grade=A]
audit-logger.js (StateManager write path into audit-log.json) starred only 6 sensitive keys at the middleware layer; email-bearing resource paths (/invites/<email>/accept), DC-048 details.userEmail attribution, and emails in non-sensitive body keys landed RAW — while the parallel unified-logger path has masked at every sink since DC-095. DC-110 closes the parity gap using the SAME canonical primitives (sa****@example.com): log() masks resource (maskEmailsInString) and deep-masks details (maskEmails) at the single write-point, covering middleware AND direct route calls. The security-event mirror now uses the masked entry.resource for target/message (judge round-1 fix-first: the raw parameter leaked emails into security-events.jsonl). logging.js exports maskEmails (export-only). maskEmails clones — caller details objects are never mutated. Judge: GLM-5.3 cold read (standing Sami authorization 2026-08-17; Codex quota dead until 2026-08-29). Round 1 (deleg_ebb83285) C fix-first — caught the event-store mirror leak. Round 2 (deleg_923f9076) after in-commit fix + mirror test: grade A, ship. Verdict URN urn:ump:mwtxoj6dbdq7bfeba3l2am2rgx34zjimcjde5f6sxz6tozsjbskq (GET readback verified: grade A, topic codex-judge-verdict). Deferred (judge-accepted): one-time scrub of historical raw-email lines in the live 16MB security-events.jsonl — queued follow-up. Tests: 123 suites / 2801 green (+6 DC-110 pins: resource+details mask, non-mutation, middleware e2e with *** survival, idempotence, no-email regression, masked mirror target/message).
This commit is contained in:
@@ -0,0 +1,173 @@
|
|||||||
|
/**
|
||||||
|
* Tests for audit-logger PII masking parity [DC-110]:
|
||||||
|
* - audit-logger.js (the StateManager write path) must mask emails with
|
||||||
|
* the SAME canonical primitives as the unified logger (DC-095):
|
||||||
|
* resource strings (URL paths like /invites/<email>/accept) and deep
|
||||||
|
* details objects (req.body.email, DC-048 userEmail attribution).
|
||||||
|
* - Masking happens at the single write-point log(), so middleware AND
|
||||||
|
* direct route calls are both covered.
|
||||||
|
* - Middleware's sensitive-key '***' redaction (password/token/…)
|
||||||
|
* survives — masking runs on the already-sanitized object.
|
||||||
|
* - The caller's `details` object is never mutated (maskEmails clones).
|
||||||
|
*
|
||||||
|
* Hermetic: AUDIT_LOG_FILE and SECURITY_EVENT_LOG_FILE are pointed at a
|
||||||
|
* tmp dir BEFORE the require — both modules resolve paths at load time.
|
||||||
|
*
|
||||||
|
* Read discipline: StateManager writes via fs.writeFile (truncate-then-
|
||||||
|
* write, NOT atomic) and middleware fires log() unawaited, so a fixed
|
||||||
|
* sleep can observe a 0-byte file mid-write. waitForEntries() polls for
|
||||||
|
* the expected entry COUNT — deterministic under lock retries.
|
||||||
|
*/
|
||||||
|
|
||||||
|
const os = require('os');
|
||||||
|
const path = require('path');
|
||||||
|
const fs = require('fs');
|
||||||
|
|
||||||
|
const TMP_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'dc110-audit-'));
|
||||||
|
process.env.AUDIT_LOG_FILE = path.join(TMP_DIR, 'audit-log.json');
|
||||||
|
process.env.SECURITY_EVENT_LOG_FILE = path.join(TMP_DIR, 'security-events.jsonl');
|
||||||
|
|
||||||
|
const AuditLogger = require('../src/security/audit-logger');
|
||||||
|
|
||||||
|
async function waitForEntries(count, timeoutMs = 5000) {
|
||||||
|
const deadline = Date.now() + timeoutMs;
|
||||||
|
for (;;) {
|
||||||
|
try {
|
||||||
|
const entries = JSON.parse(fs.readFileSync(process.env.AUDIT_LOG_FILE, 'utf8'));
|
||||||
|
if (Array.isArray(entries) && entries.length >= count) return entries;
|
||||||
|
} catch (_) { /* not yet: 0-byte mid-write or unparsed */ }
|
||||||
|
if (Date.now() > deadline) throw new Error(`timed out waiting for ${count} audit entries`);
|
||||||
|
await new Promise(r => setTimeout(r, 15));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('AuditLogger [DC-110] PII masking parity', () => {
|
||||||
|
test('log() masks emails in resource path and deep details', async () => {
|
||||||
|
await AuditLogger.log({
|
||||||
|
action: 'invite.create',
|
||||||
|
resource: 'invites/john.doe@example.com/accept',
|
||||||
|
details: {
|
||||||
|
body: { email: 'jane.doe@example.com', role: 'admin' },
|
||||||
|
userEmail: 'sami@example.org',
|
||||||
|
},
|
||||||
|
outcome: 'success',
|
||||||
|
ip: '10.1.2.3',
|
||||||
|
});
|
||||||
|
const entries = await waitForEntries(1);
|
||||||
|
expect(entries).toHaveLength(1);
|
||||||
|
const e = entries[0];
|
||||||
|
// resource: local part truncated to 2 chars + **** + domain, path suffix kept
|
||||||
|
expect(e.resource).toBe('invites/jo****@example.com/accept');
|
||||||
|
// deep details masked with the canonical shape
|
||||||
|
expect(e.details.body.email).toBe('ja****@example.com');
|
||||||
|
expect(e.details.userEmail).toBe('sa****@example.org');
|
||||||
|
expect(e.details.body.role).toBe('admin'); // non-PII untouched
|
||||||
|
// structural fields untouched
|
||||||
|
expect(e.action).toBe('invite.create');
|
||||||
|
expect(e.outcome).toBe('success');
|
||||||
|
expect(e.ip).toBe('10.1.2.3');
|
||||||
|
expect(e.id).toMatch(/^[0-9a-f-]{36}$/);
|
||||||
|
// no raw email anywhere in the serialized file
|
||||||
|
const raw = fs.readFileSync(process.env.AUDIT_LOG_FILE, 'utf8');
|
||||||
|
expect(raw).not.toContain('john.doe@example.com');
|
||||||
|
expect(raw).not.toContain('jane.doe@example.com');
|
||||||
|
expect(raw).not.toContain('sami@example.org');
|
||||||
|
expect(raw).not.toContain('.doe@'); // no partial-local leaks either
|
||||||
|
});
|
||||||
|
|
||||||
|
test("caller's details object is never mutated", async () => {
|
||||||
|
const details = { body: { email: 'orig@example.com' }, userEmail: 'orig2@example.net' };
|
||||||
|
const before = JSON.stringify(details);
|
||||||
|
await AuditLogger.log({ action: 'x.y', resource: 'r', details, outcome: 'success', ip: '' });
|
||||||
|
const entries = await waitForEntries(2);
|
||||||
|
expect(JSON.stringify(details)).toBe(before); // untouched at the call site
|
||||||
|
expect(entries[0].details.body.email).toBe('or****@example.com'); // masked only in the entry
|
||||||
|
});
|
||||||
|
|
||||||
|
test('middleware end-to-end: body, note, userEmail land masked; *** redaction survives', async () => {
|
||||||
|
const mw = AuditLogger.middleware();
|
||||||
|
const req = {
|
||||||
|
method: 'POST',
|
||||||
|
path: '/api/v1/invites',
|
||||||
|
ip: '192.168.1.50',
|
||||||
|
body: {
|
||||||
|
email: 'invitee@example.com',
|
||||||
|
note: 'for jane.doe@corp.example.com',
|
||||||
|
password: 'hunter2',
|
||||||
|
token: 'abc123',
|
||||||
|
},
|
||||||
|
params: {},
|
||||||
|
user: { id: 'u1', role: 'admin', email: 'admin@example.io' },
|
||||||
|
};
|
||||||
|
const res = { json: jest.fn() };
|
||||||
|
mw(req, res, () => {});
|
||||||
|
res.json({ success: true });
|
||||||
|
const entries = await waitForEntries(3);
|
||||||
|
const e = entries[0];
|
||||||
|
expect(e.details.body.email).toBe('in****@example.com');
|
||||||
|
expect(e.details.body.note).toBe('for ja****@corp.example.com');
|
||||||
|
// sensitive-key redaction (middleware sanitize) intact alongside masking
|
||||||
|
expect(e.details.body.password).toBe('***');
|
||||||
|
expect(e.details.body.token).toBe('***');
|
||||||
|
// DC-048 attribution intact + masked
|
||||||
|
expect(e.details.userId).toBe('u1');
|
||||||
|
expect(e.details.userEmail).toBe('ad****@example.io');
|
||||||
|
expect(e.outcome).toBe('success');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('already-masked entries stay stable (idempotent shape)', async () => {
|
||||||
|
await AuditLogger.log({
|
||||||
|
action: 'x.masked',
|
||||||
|
resource: 'users/jo****@example.com/reset',
|
||||||
|
details: { body: { email: 'jo****@example.com' } },
|
||||||
|
outcome: 'success',
|
||||||
|
ip: '',
|
||||||
|
});
|
||||||
|
const entries = await waitForEntries(4);
|
||||||
|
const e = entries[0];
|
||||||
|
// '*' is not in the local-part class, so the masked form does not re-match
|
||||||
|
expect(e.resource).toBe('users/jo****@example.com/reset');
|
||||||
|
expect(e.details.body.email).toBe('jo****@example.com');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('entries without emails are structurally unchanged', async () => {
|
||||||
|
await AuditLogger.log({
|
||||||
|
action: 'service.create',
|
||||||
|
resource: 'services/nginx',
|
||||||
|
details: { body: { name: 'nginx', port: 8080 } },
|
||||||
|
outcome: 'success',
|
||||||
|
ip: '172.16.0.4',
|
||||||
|
});
|
||||||
|
const entries = await waitForEntries(5);
|
||||||
|
const e = entries[0];
|
||||||
|
expect(e.resource).toBe('services/nginx');
|
||||||
|
expect(e.details.body.name).toBe('nginx');
|
||||||
|
expect(e.details.body.port).toBe(8080);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('security-event mirror carries MASKED target/message (judge round-2 fix)', async () => {
|
||||||
|
await AuditLogger.log({
|
||||||
|
action: 'invite.create',
|
||||||
|
resource: 'invites/john.doe@example.com/accept',
|
||||||
|
details: { body: { email: 'jane.doe@example.com' } },
|
||||||
|
outcome: 'success',
|
||||||
|
ip: '10.5.5.5',
|
||||||
|
});
|
||||||
|
// The mirror write is queued by event-store — poll for our line to land.
|
||||||
|
const deadline = Date.now() + 5000;
|
||||||
|
let mirrorRaw = '';
|
||||||
|
for (;;) {
|
||||||
|
try { mirrorRaw = fs.readFileSync(process.env.SECURITY_EVENT_LOG_FILE, 'utf8'); } catch (_) {}
|
||||||
|
if (mirrorRaw.includes('invite.create')) break;
|
||||||
|
if (Date.now() > deadline) throw new Error('mirror line never landed in security-events.jsonl');
|
||||||
|
await new Promise(r => setTimeout(r, 15));
|
||||||
|
}
|
||||||
|
const line = mirrorRaw.split('\n').find(l => l.includes('invite.create'));
|
||||||
|
const ev = JSON.parse(line);
|
||||||
|
expect(ev.target).toBe('invites/jo****@example.com/accept');
|
||||||
|
expect(ev.message).toBe('invite.create success on invites/jo****@example.com/accept');
|
||||||
|
// no raw email anywhere in the mirror file
|
||||||
|
expect(mirrorRaw).not.toContain('john.doe@example.com');
|
||||||
|
expect(mirrorRaw).not.toContain('jane.doe@example.com');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -2,6 +2,9 @@ const path = require('path');
|
|||||||
const StateManager = require('../managers/state-manager');
|
const StateManager = require('../managers/state-manager');
|
||||||
const crypto = require('crypto');
|
const crypto = require('crypto');
|
||||||
const platformPaths = require('../../platform-paths');
|
const platformPaths = require('../../platform-paths');
|
||||||
|
// DC-110: canonical email-mask primitives from the unified logger — same
|
||||||
|
// EMAIL_RE + maskEmailAddress shape every other sink uses (DC-095/DC-109).
|
||||||
|
const { maskEmails, maskEmailsInString } = require('../utils/logging');
|
||||||
|
|
||||||
const AUDIT_LOG_FILE = process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json');
|
const AUDIT_LOG_FILE = process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json');
|
||||||
const MAX_ENTRIES = parseInt(process.env.AUDIT_MAX_ENTRIES || '1000', 10);
|
const MAX_ENTRIES = parseInt(process.env.AUDIT_MAX_ENTRIES || '1000', 10);
|
||||||
@@ -141,13 +144,23 @@ class AuditLogger {
|
|||||||
|
|
||||||
async log({ action, resource, details, outcome, ip }) {
|
async log({ action, resource, details, outcome, ip }) {
|
||||||
try {
|
try {
|
||||||
|
// DC-110: PII parity with the unified logger (DC-095). Every string
|
||||||
|
// that reaches audit-log.json gets email-masked with the SAME
|
||||||
|
// canonical mask (sa****@example.com) the other sinks use, so one
|
||||||
|
// consistent masked form everywhere. Applied HERE — the single
|
||||||
|
// write-point — instead of at each call site: covers middleware
|
||||||
|
// bodies, DC-048 userEmail attribution, direct route calls, and the
|
||||||
|
// security-event-store mirror below, regardless of caller. `action`
|
||||||
|
// is an internal token (service.create / dns.add-record) and never
|
||||||
|
// contains PII; `ip` is an address literal. maskEmails() clones, so
|
||||||
|
// the caller's `details` object is never mutated.
|
||||||
const entry = {
|
const entry = {
|
||||||
id: crypto.randomUUID(),
|
id: crypto.randomUUID(),
|
||||||
timestamp: new Date().toISOString(),
|
timestamp: new Date().toISOString(),
|
||||||
ip: ip || '',
|
ip: ip || '',
|
||||||
action: action || '',
|
action: action || '',
|
||||||
resource: resource || '',
|
resource: maskEmailsInString(resource || ''),
|
||||||
details: details || {},
|
details: maskEmails(details || {}),
|
||||||
outcome: outcome || 'unknown'
|
outcome: outcome || 'unknown'
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -172,11 +185,14 @@ class AuditLogger {
|
|||||||
source_host: hostname,
|
source_host: hostname,
|
||||||
source_type: 'api',
|
source_type: 'api',
|
||||||
actor: ip || null,
|
actor: ip || null,
|
||||||
target: resource || null,
|
// DC-110 round 2 (judge fix-first): use the MASKED entry.resource
|
||||||
|
// — the raw parameter leaked emails into security-events.jsonl
|
||||||
|
// via target and the message template.
|
||||||
|
target: entry.resource || null,
|
||||||
action: action || 'unknown',
|
action: action || 'unknown',
|
||||||
outcome: outcome || 'unknown',
|
outcome: outcome || 'unknown',
|
||||||
severity,
|
severity,
|
||||||
message: `${action} ${outcome} on ${resource}`.trim(),
|
message: `${action} ${outcome} on ${entry.resource}`.trim(),
|
||||||
metadata: {
|
metadata: {
|
||||||
method: details?.body && Object.keys(details.body)[0] ? '(see audit-log)' : undefined,
|
method: details?.body && Object.keys(details.body)[0] ? '(see audit-log)' : undefined,
|
||||||
audit_id: entry.id,
|
audit_id: entry.id,
|
||||||
|
|||||||
@@ -691,4 +691,5 @@ module.exports = {
|
|||||||
EMAIL_RE,
|
EMAIL_RE,
|
||||||
maskEmailAddress,
|
maskEmailAddress,
|
||||||
maskEmailsInString,
|
maskEmailsInString,
|
||||||
|
maskEmails,
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user