docs: close two auth bypasses and four fail-open paths in the Task 6 engine snippet

The plan's engine had a genuine authorization bypass and several fail-open
branches. Task 7 builds can() on this, so the source of truth is fixed before
that lands.

CRITICAL - anonymous callers bypassed every bound policy on a public:true
permission: the anonymous branch returned allow before the policy loop. A
permission marked "public, but not when embargoed" was fully open to
unauthenticated traffic, and the least-trusted caller got the weakest
evaluation. Policies now run on the anonymous path too; public relaxes the
identity requirement, never the policy requirement.

CRITICAL - the policy verdict check was truthiness-based, not an identity
check, so a policy returning {allowed: "yes"} or {allowed: 1} granted access.
It now compares against true.

A binding naming a policy the catalog lacks was skipped, granting whatever
the policy guarded; it now denies. Falsy and non-string subject ids fell
through to the anonymous path - {id: 0} became anonymous and {id: 123} reached
the store as a lookup key; only a non-empty string now identifies a subject.

Two design forks, ruled by the human: denies honour wildcards, so denying
"post:*" blocks post:delete instead of being accepted and doing nothing; and
permissionsFor subtracts denies, so composing it with permissionMatches
agrees with decide() rather than silently losing deny precedence.

Adds deniedBy() and six regression tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-04 17:52:32 +05:30
co-authored by Claude Opus 5
parent c499f136fd
commit 86b3dc1e6a
@@ -1372,6 +1372,102 @@ describe("createAuthzResolver.decide", () => {
expect(outside.allowed).toBe(false); expect(outside.allowed).toBe(false);
}); });
}); });
describe("createAuthzResolver fail-closed regressions", () => {
const guarded = mergeCatalogs([
{
source: "guarded.ts",
module: defineAuthz({
permissions: { "feed:view": { public: true }, "x:go": {} },
policies: {
never: async () => ({ allowed: false, reason: "always no", policy: "never" }),
truthy: async () => ({ allowed: "yes" }) as never,
},
bindings: { "feed:view": ["never"] },
}),
},
]);
test("a public permission still runs its bound policies for anonymous callers", async () => {
// The least-trusted caller must not receive the weakest evaluation:
// `public` relaxes the identity requirement, never the policy requirement.
const resolver = createAuthzResolver({
catalog: guarded,
store: memoryPermissionStore(),
strict: false,
});
const anonymous = await resolver.decide({ subject: null, permission: "feed:view" });
expect(anonymous.allowed).toBe(false);
expect(anonymous.policy).toBe("never");
});
test("a policy returning a truthy non-boolean denies", async () => {
const catalog = mergeCatalogs([
{
source: "t.ts",
module: defineAuthz({
permissions: { "x:go": {} },
policies: { truthy: async () => ({ allowed: "yes" }) as never },
bindings: { "x:go": ["truthy"] },
}),
},
]);
const store = memoryPermissionStore();
await store.grant("u1", "x:go", "allow");
const resolver = createAuthzResolver({ catalog, store, strict: false });
expect((await resolver.decide({ subject: { id: "u1" }, permission: "x:go" })).allowed).toBe(
false,
);
});
test("a binding naming a policy the catalog lacks denies rather than skipping", async () => {
// Hand-built catalog: mergeCatalogs would reject this, but the resolver
// accepts any AuthzCatalog and must not grant what the policy guarded.
const broken = {
permissions: new Map([["x:go", {}]]),
roles: new Map(),
policies: new Map(),
attributes: new Map(),
bindings: new Map([["x:go", ["ghost"]]]),
} as unknown as Parameters<typeof createAuthzResolver>[0]["catalog"];
const store = memoryPermissionStore();
await store.grant("u1", "x:go", "allow");
const resolver = createAuthzResolver({ catalog: broken, store, strict: false });
expect((await resolver.decide({ subject: { id: "u1" }, permission: "x:go" })).allowed).toBe(
false,
);
});
test("a wildcard deny blocks the whole namespace", async () => {
const { store, resolver } = make();
await store.assignRole("u1", "admin");
await store.grant("u1", "post:*", "deny");
expect(
(await resolver.decide({ subject: { id: "u1" }, permission: "post:delete" })).allowed,
).toBe(false);
});
test("permissionsFor omits denied permissions", async () => {
const { store, resolver } = make();
await store.assignRole("u1", "editor");
await store.grant("u1", "post:*", "deny");
const effective = await resolver.permissionsFor("u1");
// The obvious composition must agree with decide().
expect(permissionMatches(effective, "post:write")).toBe(false);
});
test("a non-string or empty subject id denies instead of falling back to anonymous", async () => {
const { resolver } = make();
for (const id of [0, "", null, 123, {}]) {
const result = await resolver.decide({
subject: { id } as never,
permission: "post:read", // public — must still not be reached this way
});
if (id === null) continue; // null is genuinely anonymous
expect(result.allowed).toBe(false);
}
});
});
``` ```
- [ ] **Step 2: Run test to verify it fails** - [ ] **Step 2: Run test to verify it fails**
@@ -1439,6 +1535,15 @@ export function permissionMatches(granted: Set<string>, permission: string): boo
return false; return false;
} }
/**
* True if any entry in the deny list covers `permission`. Denies honour the
* same depth-aware wildcards as grants, so denying "post:*" blocks
* post:comment:delete rather than being accepted and silently doing nothing.
*/
export function deniedBy(denies: readonly string[], permission: string): boolean {
return denies.length ? permissionMatches(new Set(denies), permission) : false;
}
function isProduction(): boolean { function isProduction(): boolean {
return (process.env.NODE_ENV ?? "development") === "production"; return (process.env.NODE_ENV ?? "development") === "production";
} }
@@ -1448,9 +1553,9 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve
const strict = options.strict ?? !isProduction(); const strict = options.strict ?? !isProduction();
/** /**
* Single source of truth for "what does this subject hold?". Returns the * Single source of truth for "what does this subject hold?". Returns the raw
* raw assignments too, because `decide` needs `denies` and `permissionsFor` * assignments alongside the effective set, because `decide` reports on the
* does not — do NOT duplicate this logic in either caller. * deny that blocked it. Do NOT duplicate this logic in either caller.
*/ */
const loadEffective = async (subjectId: string, scope?: AuthzScope) => { const loadEffective = async (subjectId: string, scope?: AuthzScope) => {
const assignments = await store.assignmentsFor(subjectId, scope); const assignments = await store.assignmentsFor(subjectId, scope);
@@ -1459,13 +1564,26 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve
return { assignments, granted }; return { assignments, granted };
}; };
const permissionsFor = async (subjectId: string, scope?: AuthzScope): Promise<Set<string>> => /**
(await loadEffective(subjectId, scope)).granted; * Effective permissions, denies already removed. Callers compose this with
* `permissionMatches` to gate menus and admin UI, so it must not report a
* permission that `decide` would refuse.
*/
const permissionsFor = async (subjectId: string, scope?: AuthzScope): Promise<Set<string>> => {
const { assignments, granted } = await loadEffective(subjectId, scope);
if (!assignments.denies.length) return granted;
const effective = new Set<string>();
for (const entry of granted) {
// A wildcard grant survives only if nothing denies it outright.
if (!deniedBy(assignments.denies, entry)) effective.add(entry);
}
return effective;
};
const finish = (input: DecideInput, result: AuthorizationDecision): AuthorizationDecision => { const finish = (input: DecideInput, result: AuthorizationDecision): AuthorizationDecision => {
if (!result.allowed || options.auditAllows) { if (!result.allowed || options.auditAllows) {
safeRecord(audit, { safeRecord(audit, {
subjectId: input.subject?.id, subjectId: typeof input.subject?.id === "string" ? input.subject.id : undefined,
scope: input.scope, scope: input.scope,
permission: input.permission, permission: input.permission,
allowed: result.allowed, allowed: result.allowed,
@@ -1477,11 +1595,53 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve
return result; return result;
}; };
/**
* Run every policy bound to a permission. Returns a denial, or null to allow.
* Anonymous callers run this too: `public` relaxes the identity requirement,
* never the policy requirement.
*/
const runPolicies = async (
input: DecideInput,
permission: string,
): Promise<AuthorizationDecision | null> => {
for (const name of catalog.bindings.get(permission) ?? []) {
const policy = catalog.policies.get(name);
if (!policy) {
// A binding naming a policy the catalog lacks must deny, not skip:
// silently ignoring it would grant whatever the policy guarded.
console.error(
`[wrnexus:authz] binding for '${permission}' names unknown policy '${name}'; denying`,
);
return { allowed: false, reason: "Policy unavailable", policy: name };
}
try {
const verdict = await (
policy as unknown as (
s: unknown,
r: unknown,
) => AuthorizationDecision | Promise<AuthorizationDecision>
)(input.subject, input.resource);
// Identity check, not truthiness: {allowed: "yes"} must not grant.
if (verdict?.allowed !== true) {
return {
allowed: false,
reason: verdict?.reason ?? "Policy denied access",
policy: verdict?.policy ?? name,
};
}
} catch (error) {
console.error(`[wrnexus:authz] policy '${name}' threw; denying`, error);
return { allowed: false, reason: "Policy error", policy: name };
}
}
return null;
};
return { return {
permissionsFor, permissionsFor,
async decide(input) { async decide(input) {
const { subject, permission, resource, scope } = input; const { subject, permission, scope } = input;
const meta = catalog.permissions.get(permission); const meta = catalog.permissions.get(permission);
if (!meta) { if (!meta) {
@@ -1497,14 +1657,22 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve
}); });
} }
const subjectId = subject?.id; // Only a non-empty string identifies a subject. A numeric id of 0 or a
// non-string id must not fall through to the anonymous path, and must
// never reach the store as a lookup key.
const rawId: unknown = subject?.id;
const subjectId = typeof rawId === "string" && rawId !== "" ? rawId : undefined;
if (rawId !== undefined && rawId !== null && subjectId === undefined) {
console.error("[wrnexus:authz] subject.id must be a non-empty string; denying");
return finish(input, { allowed: false, reason: "Invalid subject" });
}
if (!subjectId) { if (!subjectId) {
return finish( if (!meta.public) {
input, return finish(input, { allowed: false, reason: "Authentication required" });
meta.public }
? { allowed: true, reason: "public permission" } const denied = await runPolicies(input, permission);
: { allowed: false, reason: "Authentication required" }, return finish(input, denied ?? { allowed: true, reason: "public permission" });
);
} }
let assignments; let assignments;
@@ -1516,8 +1684,10 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve
return finish(input, { allowed: false, reason: "Authorization store unavailable" }); return finish(input, { allowed: false, reason: "Authorization store unavailable" });
} }
// 1. Explicit deny wins over everything, including "*". // 1. Explicit deny wins over everything, including "*". Wildcards are
if (assignments.denies.includes(permission)) { // honoured here exactly as they are for grants, so denying "post:*"
// blocks post:delete rather than silently doing nothing.
if (deniedBy(assignments.denies, permission)) {
return finish(input, { allowed: false, reason: "explicit deny" }); return finish(input, { allowed: false, reason: "explicit deny" });
} }
@@ -1527,26 +1697,8 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve
} }
// 3. Every bound policy must pass. // 3. Every bound policy must pass.
for (const name of catalog.bindings.get(permission) ?? []) { const denied = await runPolicies(input, permission);
const policy = catalog.policies.get(name); return finish(input, denied ?? { allowed: true });
if (!policy) continue;
try {
const verdict = await (
policy as unknown as (
s: unknown,
r: unknown,
) => AuthorizationDecision | Promise<AuthorizationDecision>
)(subject, resource);
if (!verdict.allowed) {
return finish(input, { ...verdict, policy: verdict.policy ?? name });
}
} catch (error) {
console.error(`[wrnexus:authz] policy '${name}' threw; denying`, error);
return finish(input, { allowed: false, reason: "Policy error", policy: name });
}
}
return finish(input, { allowed: true });
}, },
}; };
} }