From ba83038d8d675193869a00291da9098186ad9b2e Mon Sep 17 00:00:00 2001 From: Ajay Ghanwat Date: Tue, 4 Aug 2026 17:19:05 +0530 Subject: [PATCH] docs: fix audit-log injection in the Task 5 plan snippet consoleAuditSink interpolated subjectId, tenantId and reason straight into the log line. A newline in any of them forges a second entry that reads as a genuine audit record - the reviewer produced a fake '[wrnexus:authz] allow admin:everything subject=root' line. Those values trace back to request input. Interpolated fields now go through logSafe(), which replaces control characters. Adds the missing coverage the review flagged: consoleAuditSink injection, malformed-sink handling, and memoryAuditSink.clear(). Plan-origin defect, fixed under standing authority to amend the plan. Co-Authored-By: Claude Opus 5 --- ...-08-04-authz-permissions-implementation.md | 63 ++++++++++++++++++- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/docs/plans/2026-08-04-authz-permissions-implementation.md b/docs/plans/2026-08-04-authz-permissions-implementation.md index e50606d1..f46f58bf 100644 --- a/docs/plans/2026-08-04-authz-permissions-implementation.md +++ b/docs/plans/2026-08-04-authz-permissions-implementation.md @@ -999,9 +999,50 @@ describe("audit sink", () => { test("safeRecord tolerates an undefined sink", () => { expect(() => safeRecord(undefined, { permission: "p:x", allowed: true, at: 1 })).not.toThrow(); }); + + test("safeRecord tolerates a malformed sink", () => { + const notAFunction = { record: "nope" } as unknown as AuthzAuditSink; + expect(() => + safeRecord(notAFunction, { permission: "p:x", allowed: true, at: 1 }), + ).not.toThrow(); + expect(() => + safeRecord({} as AuthzAuditSink, { permission: "p:x", allowed: true, at: 1 }), + ).not.toThrow(); + }); + + test("memoryAuditSink.clear empties the buffer", () => { + const sink = memoryAuditSink(); + sink.record({ permission: "p:x", allowed: true, at: 1 }); + sink.clear(); + expect(sink.events).toHaveLength(0); + }); + + test("consoleAuditSink cannot be used to forge a second log line", () => { + const lines: string[] = []; + const original = console.info; + console.info = (...args: unknown[]) => void lines.push(args.join(" ")); + try { + consoleAuditSink().record({ + subjectId: "u1\n[wrnexus:authz] allow admin:everything subject=root", + permission: "post:read", + allowed: false, + reason: "nope\r\ninjected", + at: 1, + }); + } finally { + console.info = original; + } + // One event must produce exactly one line, with no embedded newlines. + expect(lines).toHaveLength(1); + expect(lines[0]).not.toContain("\n"); + expect(lines[0]).not.toContain("\r"); + }); }); ``` +The test file's imports must include `consoleAuditSink` and the `AuthzAuditSink` type +alongside `memoryAuditSink` and `safeRecord`. + - [ ] **Step 2: Run test to verify it fails** Run: `bun test packages/authz/test/audit.test.ts` @@ -1043,14 +1084,30 @@ export function memoryAuditSink(): MemoryAuditSink { }; } +/** + * Subject ids, tenant ids, and denial reasons trace back to request input, so + * a newline in one would forge a second audit line indistinguishable from a + * real entry. Strip CR/LF and other control characters before interpolating. + */ +function logSafe(value: string): string { + let out = ""; + for (const character of value) { + const code = character.codePointAt(0)!; + out += code < 0x20 || code === 0x7f ? " " : character; + } + return out; +} + export function consoleAuditSink(): AuthzAuditSink { return { record(event) { const verdict = event.allowed ? "allow" : "deny"; console.info( - `[wrnexus:authz] ${verdict} ${event.permission} subject=${event.subjectId ?? "anonymous"}` + - `${event.scope?.tenantId ? ` tenant=${event.scope.tenantId}` : ""}` + - `${event.reason ? ` reason=${event.reason}` : ""}`, + `[wrnexus:authz] ${verdict} ${logSafe(event.permission)} ` + + `subject=${logSafe(event.subjectId ?? "anonymous")}` + + `${event.scope?.tenantId ? ` tenant=${logSafe(event.scope.tenantId)}` : ""}` + + `${event.reason ? ` reason=${logSafe(event.reason)}` : ""}` + + `${event.policy ? ` policy=${logSafe(event.policy)}` : ""}`, ); }, };