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 <noreply@anthropic.com>
This commit is contained in:
@@ -999,9 +999,50 @@ describe("audit sink", () => {
|
|||||||
test("safeRecord tolerates an undefined sink", () => {
|
test("safeRecord tolerates an undefined sink", () => {
|
||||||
expect(() => safeRecord(undefined, { permission: "p:x", allowed: true, at: 1 })).not.toThrow();
|
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**
|
- [ ] **Step 2: Run test to verify it fails**
|
||||||
|
|
||||||
Run: `bun test packages/authz/test/audit.test.ts`
|
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 {
|
export function consoleAuditSink(): AuthzAuditSink {
|
||||||
return {
|
return {
|
||||||
record(event) {
|
record(event) {
|
||||||
const verdict = event.allowed ? "allow" : "deny";
|
const verdict = event.allowed ? "allow" : "deny";
|
||||||
console.info(
|
console.info(
|
||||||
`[wrnexus:authz] ${verdict} ${event.permission} subject=${event.subjectId ?? "anonymous"}` +
|
`[wrnexus:authz] ${verdict} ${logSafe(event.permission)} ` +
|
||||||
`${event.scope?.tenantId ? ` tenant=${event.scope.tenantId}` : ""}` +
|
`subject=${logSafe(event.subjectId ?? "anonymous")}` +
|
||||||
`${event.reason ? ` reason=${event.reason}` : ""}`,
|
`${event.scope?.tenantId ? ` tenant=${logSafe(event.scope.tenantId)}` : ""}` +
|
||||||
|
`${event.reason ? ` reason=${logSafe(event.reason)}` : ""}` +
|
||||||
|
`${event.policy ? ` policy=${logSafe(event.policy)}` : ""}`,
|
||||||
);
|
);
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user