diff --git a/packages/authz/src/engine.ts b/packages/authz/src/engine.ts index ef5ae6d0..21bb348c 100644 --- a/packages/authz/src/engine.ts +++ b/packages/authz/src/engine.ts @@ -53,6 +53,15 @@ export function permissionMatches(granted: Set, permission: string): boo 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 { return (process.env.NODE_ENV ?? "development") === "production"; } @@ -73,13 +82,20 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve return { assignments, granted }; }; - const permissionsFor = async (subjectId: string, scope?: AuthzScope): Promise> => - (await loadEffective(subjectId, scope)).granted; + const permissionsFor = async (subjectId: string, scope?: AuthzScope): Promise> => { + const { assignments, granted } = await loadEffective(subjectId, scope); + if (!assignments.denies.length) return granted; + const effective = new Set(); + for (const entry of granted) { + if (!deniedBy(assignments.denies, entry)) effective.add(entry); + } + return effective; + }; const finish = (input: DecideInput, result: AuthorizationDecision): AuthorizationDecision => { if (!result.allowed || options.auditAllows) { safeRecord(audit, { - subjectId: input.subject?.id, + subjectId: typeof input.subject?.id === "string" ? input.subject.id : undefined, scope: input.scope, permission: input.permission, allowed: result.allowed, @@ -91,11 +107,49 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve return result; }; + /** + * Run every policy bound to `permission`. Returns the denial verdict of the + * first failing/missing/throwing policy, or `null` if all bound policies + * passed (including "no policies bound" — an implicit allow). + */ + const runPolicies = async ( + input: DecideInput, + permission: string, + ): Promise => { + const { subject, resource } = input; + for (const name of catalog.bindings.get(permission) ?? []) { + const policy = catalog.policies.get(name); + if (!policy) { + console.error(`[wrnexus:authz] policy '${name}' is not registered; denying`); + return { allowed: false, reason: "Policy unavailable", policy: name }; + } + try { + const verdict = await ( + policy as unknown as ( + s: unknown, + r: unknown, + ) => AuthorizationDecision | Promise + )(subject, resource); + 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 { permissionsFor, async decide(input) { - const { subject, permission, resource, scope } = input; + const { permission, scope } = input; const meta = catalog.permissions.get(permission); if (!meta) { @@ -111,14 +165,19 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve }); } - const subjectId = subject?.id; + const rawId: unknown = input.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) { - return finish( - input, - meta.public - ? { allowed: true, reason: "public permission" } - : { allowed: false, reason: "Authentication required" }, - ); + if (!meta.public) { + return finish(input, { allowed: false, reason: "Authentication required" }); + } + const denied = await runPolicies(input, permission); + return finish(input, denied ?? { allowed: true, reason: "public permission" }); } let assignments; @@ -130,8 +189,8 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve return finish(input, { allowed: false, reason: "Authorization store unavailable" }); } - // 1. Explicit deny wins over everything, including "*". - if (assignments.denies.includes(permission)) { + // 1. Explicit deny wins over everything, including "*", honouring wildcards. + if (deniedBy(assignments.denies, permission)) { return finish(input, { allowed: false, reason: "explicit deny" }); } @@ -141,26 +200,8 @@ export function createAuthzResolver(options: AuthzResolverOptions): AuthzResolve } // 3. Every bound policy must pass. - for (const name of catalog.bindings.get(permission) ?? []) { - const policy = catalog.policies.get(name); - if (!policy) continue; - try { - const verdict = await ( - policy as unknown as ( - s: unknown, - r: unknown, - ) => AuthorizationDecision | Promise - )(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 }); + const denied = await runPolicies(input, permission); + return finish(input, denied ?? { allowed: true }); }, }; } diff --git a/packages/authz/test/engine.test.ts b/packages/authz/test/engine.test.ts index c085ec9d..432c6377 100644 --- a/packages/authz/test/engine.test.ts +++ b/packages/authz/test/engine.test.ts @@ -1,9 +1,11 @@ import { describe, expect, test } from "bun:test"; +import type { DecisionPolicy } from "../src/advanced.ts"; import { defineAuthz } from "../src/registry.ts"; import { mergeCatalogs } from "../src/catalog.ts"; import { memoryPermissionStore } from "../src/store.ts"; import { memoryAuditSink } from "../src/audit.ts"; import { createAuthzResolver, expandRoles, permissionMatches } from "../src/engine.ts"; +import type { AuthzCatalog } from "../src/types.ts"; const catalog = mergeCatalogs([ { @@ -206,3 +208,98 @@ describe("createAuthzResolver.decide", () => { expect(outside.allowed).toBe(false); }); }); + +describe("createAuthzResolver fail-closed regressions", () => { + test("a public permission bound to an always-denying policy denies for an anonymous subject", async () => { + const publicPolicyCatalog = mergeCatalogs([ + { + source: "pub.ts", + module: defineAuthz({ + permissions: { "feed:view": { public: true } }, + policies: { + neverAllow: async () => ({ + allowed: false, + reason: "embargoed", + policy: "neverAllow", + }), + }, + bindings: { "feed:view": ["neverAllow"] }, + }), + }, + ]); + const resolver = createAuthzResolver({ + catalog: publicPolicyCatalog, + store: memoryPermissionStore(), + strict: false, + }); + const result = await resolver.decide({ subject: null, permission: "feed:view" }); + expect(result.allowed).toBe(false); + expect(result.policy).toBe("neverAllow"); + }); + + test("a policy returning a truthy non-boolean 'allowed' denies", async () => { + const truthyCatalog = mergeCatalogs([ + { + source: "truthy.ts", + module: defineAuthz({ + permissions: { "x:truthy": {} }, + policies: { + truthy: (async () => ({ allowed: "yes" })) as unknown as DecisionPolicy, + }, + bindings: { "x:truthy": ["truthy"] }, + }), + }, + ]); + const store = memoryPermissionStore(); + await store.grant("u1", "x:truthy", "allow"); + const resolver = createAuthzResolver({ catalog: truthyCatalog, store, strict: false }); + const result = await resolver.decide({ subject: { id: "u1" }, permission: "x:truthy" }); + expect(result.allowed).toBe(false); + }); + + test("a binding naming a policy the catalog lacks denies", async () => { + const missingPolicyCatalog: AuthzCatalog = { + permissions: new Map([["x:missing", {}]]), + roles: new Map(), + policies: new Map(), + attributes: new Map(), + bindings: new Map([["x:missing", ["ghostPolicy"]]]), + }; + const store = memoryPermissionStore(); + await store.grant("u1", "x:missing", "allow"); + const resolver = createAuthzResolver({ catalog: missingPolicyCatalog, store, strict: false }); + const result = await resolver.decide({ subject: { id: "u1" }, permission: "x:missing" }); + expect(result.allowed).toBe(false); + expect(result.policy).toBe("ghostPolicy"); + }); + + test("a wildcard deny blocks a permission the role explicitly grants", async () => { + const { store, resolver } = make(); + await store.assignRole("u1", "admin"); + await store.grant("u1", "post:*", "deny"); + const result = await resolver.decide({ subject: { id: "u1" }, permission: "post:delete" }); + expect(result.allowed).toBe(false); + expect(result.reason).toMatch(/explicit deny/i); + }); + + test("permissionsFor subtracts permissions covered by a wildcard deny", async () => { + const { store, resolver } = make(); + await store.assignRole("u1", "editor"); + await store.grant("u1", "post:*", "deny"); + const granted = await resolver.permissionsFor("u1"); + expect(permissionMatches(granted, "post:delete")).toBe(false); + }); + + test("non-string subject ids deny rather than falling back to anonymous", async () => { + const { resolver } = make(); + const invalidIds: unknown[] = [0, "", 123, {}]; + for (const id of invalidIds) { + const result = await resolver.decide({ + subject: { id } as unknown as { id?: string }, + permission: "post:read", + }); + expect(result.allowed).toBe(false); + expect(result.reason).toMatch(/invalid subject/i); + } + }); +});