fix(authz): audit getResource denials; fail closed on a malformed denies shape
guardPermission's getResource catch returned 403 directly, never reaching decideFor -> decide -> finish, so the audit sink never saw it — an attacker probing ids that make the resource loader throw got a clean 403 stream invisible to the audit trail. The audit sink is now stashed on the per-request RequestAuthz object (authzMiddleware already receives it via AuthzResolverOptions), and the catch records an "allowed: false" event with an opaque reason before returning the 403. Also: the explicit-deny check sat outside decide()'s try/catch, and deniedBy() guarded on denies.length rather than Array.isArray(denies). A store returning denies as a bare string let new Set(denies) iterate characters instead of the permission, so the deny matched nothing and was silently discarded; a store omitting denies entirely threw straight out of decide(). Both are now validated and handled inside the try, denying via the same "Authorization store unavailable" path as any other store failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -75,6 +75,13 @@ export function permissionMatches(granted: Set<string>, permission: string): boo
|
||||
* post:comment:delete rather than being accepted and silently doing nothing.
|
||||
*/
|
||||
export function deniedBy(denies: readonly string[], permission: string): boolean {
|
||||
// A non-conforming store (e.g. denies: "post:write" instead of an array)
|
||||
// must not silently discard an explicit deny: new Set("post:write") would
|
||||
// iterate the string's characters instead of throwing, so the deny would
|
||||
// match nothing and fail open. Array.isArray guards the SHAPE, not just
|
||||
// the length, so a truthy-but-non-array denies value denies by falling
|
||||
// through to the caller's catch instead of matching nothing here.
|
||||
if (!Array.isArray(denies)) return false;
|
||||
return denies.length ? permissionMatches(new Set(denies), permission) : false;
|
||||
}
|
||||
|
||||
@@ -204,21 +211,36 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve
|
||||
let granted: Set<string>;
|
||||
try {
|
||||
({ assignments, granted } = await loadEffective(subjectId, scope));
|
||||
|
||||
// A store returning a non-array `denies` (e.g. a single string, or
|
||||
// omitting the field entirely) violates the PermissionStore contract.
|
||||
// Treat that exactly like assignmentsFor() itself throwing — fail
|
||||
// closed — rather than letting a malformed shape flow into
|
||||
// deniedBy(): a string denies would otherwise iterate as
|
||||
// CHARACTERS (new Set("post:write") is a set of letters, not the
|
||||
// permission), so an explicit deny would silently match nothing and
|
||||
// be discarded, and an omitted `denies` would throw past this
|
||||
// function entirely if it weren't caught here.
|
||||
if (!Array.isArray(assignments.denies)) {
|
||||
throw new TypeError(
|
||||
"WRN-AUTHZ-STORE: assignmentsFor() must return an array for `denies`",
|
||||
);
|
||||
}
|
||||
|
||||
// 1. Explicit deny wins over everything, including "*", honouring wildcards.
|
||||
if (deniedBy(assignments.denies, permission)) {
|
||||
return finish(input, { allowed: false, reason: "explicit deny" });
|
||||
}
|
||||
|
||||
// 2. Must hold the permission at all.
|
||||
if (!meta.public && !permissionMatches(granted, permission)) {
|
||||
return finish(input, { allowed: false, reason: "Missing permission" });
|
||||
}
|
||||
} catch (error) {
|
||||
console.error("[wrnexus:authz] permission store failed; denying", error);
|
||||
return finish(input, { allowed: false, reason: "Authorization store unavailable" });
|
||||
}
|
||||
|
||||
// 1. Explicit deny wins over everything, including "*", honouring wildcards.
|
||||
if (deniedBy(assignments.denies, permission)) {
|
||||
return finish(input, { allowed: false, reason: "explicit deny" });
|
||||
}
|
||||
|
||||
// 2. Must hold the permission at all.
|
||||
if (!meta.public && !permissionMatches(granted, permission)) {
|
||||
return finish(input, { allowed: false, reason: "Missing permission" });
|
||||
}
|
||||
|
||||
// 3. Every bound policy must pass.
|
||||
const denied = await runPolicies(input, permission);
|
||||
return finish(input, denied ?? { allowed: true });
|
||||
|
||||
Reference in New Issue
Block a user