From 41b6e2ed2b21cc8f84b54511c4efdf150f2b0839 Mon Sep 17 00:00:00 2001 From: Ajay Ghanwat Date: Wed, 5 Aug 2026 02:10:24 +0530 Subject: [PATCH] fix(authz): freeze catalog values after boot; correct compile-time-check claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit frozenMap only blocked the Map's own mutators, so catalog.roles.get("editor").push("*") escalated a role to a full wildcard past an error string claiming the catalog is frozen after boot; the same applied to permission/attribute metadata objects and binding arrays. mergeCatalogs now stores frozen copies of each, so the original declaring module's objects are never mutated either. Also corrects two docstrings (codegen.ts, the design doc) that claimed `wrnexus authz generate`'s output makes a permission typo a type error — can(), guardPermission(), and decideFor() all take a bare string and nothing consumes the generated union automatically. Documents what it actually is: a Permission/Role union to type your own helpers/constants against. Also adds a README note on the subject.id contract (must be a non-empty string; owner() compares with Object.is). Co-Authored-By: Claude Opus 5 --- .../2026-08-04-authz-permissions-design.md | 6 +- packages/authz/README.md | 23 ++++++++ packages/authz/src/catalog.ts | 18 ++++-- packages/authz/src/codegen.ts | 9 ++- packages/authz/test/catalog.test.ts | 58 +++++++++++++++++++ 5 files changed, 106 insertions(+), 8 deletions(-) diff --git a/docs/plans/2026-08-04-authz-permissions-design.md b/docs/plans/2026-08-04-authz-permissions-design.md index 14a72a16..f02f6665 100644 --- a/docs/plans/2026-08-04-authz-permissions-design.md +++ b/docs/plans/2026-08-04-authz-permissions-design.md @@ -229,8 +229,10 @@ request. - `wrnexus authz list` — merged catalog across the workspace, with conflicts. - `wrnexus authz generate` — emits `app/authz/permissions.gen.ts` exporting - `type Permission = "post:read" | "post:write" | ...`, so `can()` is checked at compile time. - Runs automatically in `build.ts`, mirroring `regenerateQueries`. + `type Permission = "post:read" | "post:write" | ...`. `can()`, `guardPermission()`, + and `decideFor()` all take a bare `string` and nothing consumes this union + automatically — it exists to type your own helpers/constants against the + registered catalog. Runs automatically in `build.ts`, mirroring `regenerateQueries`. - `wrnexus authz init` — scaffolds the migration and a seed helper for default roles. - Admin UI: `.wrn` components for listing subjects and assigning roles, shipped in `@wrnexus/ui` behind the existing eject mechanism. diff --git a/packages/authz/README.md b/packages/authz/README.md index 207ae431..4247ad62 100644 --- a/packages/authz/README.md +++ b/packages/authz/README.md @@ -204,6 +204,16 @@ import { getDb } from "@wrnexus/db"; export default authzMiddleware({ catalog: getAuthzCatalog(), store: dbPermissionStore(getDb()) }); ``` +> **`subject.id` must be a non-empty string.** The engine denies (and logs to +> stderr) whenever `ctx.user.id` is present but not a non-empty string — this +> includes the common case of an integer primary key. Coerce it before it +> reaches `ctx.user`, e.g. `user.id = String(row.id)`, or every request for +> that user denies with "Invalid subject" instead of resolving normally. +> `owner()` (the built-in ownership policy) compares subject and resource ids +> with `Object.is`, so both sides must be the same type too — `owner()` on a +> numeric `resource.authorId` against a stringified `subject.id` never +> matches even when they represent "the same" id. + There is no per-route `middleware` export — `app/middleware/*.ts` is the only place middleware is registered. To gate part of the app, branch on the request the same way any other conditional middleware does (compare @@ -279,3 +289,16 @@ wrnexus authz list # every registered permission, role, and policy wrnexus authz generate # app/authz/permissions.gen.ts type unions wrnexus authz init # scaffold the assignment-table migration ``` + +`wrnexus authz generate`'s output is a plain `Permission | Role` string-literal +union — `can()`, `guardPermission()`, and `decideFor()` all take a bare +`string` and nothing reads this file automatically, so import it to type your +own helpers/constants against the registered catalog, e.g.: + +```ts +import type { Permission } from "app/authz/permissions.gen.ts"; + +function guard(permission: Permission) { + return guardPermission(permission); +} +``` diff --git a/packages/authz/src/catalog.ts b/packages/authz/src/catalog.ts index 0ffb77b5..f8df76ba 100644 --- a/packages/authz/src/catalog.ts +++ b/packages/authz/src/catalog.ts @@ -71,11 +71,19 @@ export function mergeCatalogs(sources: CatalogSource[]): AuthzCatalog { for (const { source, module } of sources) { for (const [id, meta] of Object.entries(module.permissions ?? {})) { claim("permission", id, source, permissions.get(id), meta); - permissions.set(id, meta); + // Freeze a COPY, not the app's own declared object: `frozenMap` only + // blocks the Map's mutators, so `catalog.permissions.get("x").risk = + // "low"` would otherwise silently rewrite metadata past a catalog that + // claims to be frozen after boot. Copying also avoids freezing (and + // thus permanently locking) an object the declaring module might still + // hold a live reference to. + permissions.set(id, Object.freeze({ ...meta })); } for (const [name, grants] of Object.entries(module.roles ?? {})) { claim("role", name, source, roles.get(name), grants); - roles.set(name, grants); + // Same reasoning: without this, `catalog.roles.get("editor").push("*")` + // succeeds and silently escalates a role to a full wildcard. + roles.set(name, Object.freeze([...grants])); } for (const [name, policy] of Object.entries(module.policies ?? {})) { // Two closures are never deep-equal, so identity is the only sane test. @@ -90,7 +98,7 @@ export function mergeCatalogs(sources: CatalogSource[]): AuthzCatalog { } for (const [name, meta] of Object.entries(module.attributes ?? {})) { claim("attribute", name, source, attributes.get(name), meta); - attributes.set(name, meta); + attributes.set(name, Object.freeze({ ...meta })); } for (const [permission, names] of Object.entries(module.bindings ?? {})) { const set = bindings.get(permission) ?? new Set(); @@ -114,6 +122,8 @@ export function mergeCatalogs(sources: CatalogSource[]): AuthzCatalog { roles: frozenMap(roles), policies: frozenMap(policies), attributes: frozenMap(attributes), - bindings: frozenMap([...bindings].map(([k, v]) => [k, [...v]] as [string, readonly string[]])), + bindings: frozenMap( + [...bindings].map(([k, v]) => [k, Object.freeze([...v])] as [string, readonly string[]]), + ), }; } diff --git a/packages/authz/src/codegen.ts b/packages/authz/src/codegen.ts index 144f5a19..885a1f91 100644 --- a/packages/authz/src/codegen.ts +++ b/packages/authz/src/codegen.ts @@ -13,8 +13,13 @@ function union(values: string[]): string { } /** - * Emit compile-time unions for the registered permissions and roles, so a - * typo in can(ctx, "post:wrtie") is a type error rather than a silent false. + * Emit `Permission`/`Role` string-literal unions from the registered catalog. + * + * This does NOT make `can(ctx, "post:wrtie")` a type error — `can()`, + * `guardPermission()`, and `decideFor()` all take a bare `string`, and + * nothing in the framework consumes this generated file automatically. + * Import the unions yourself to type your OWN helpers/constants, e.g. + * `const PERM: Permission = "post:write"` or a typed wrapper around `can()`. */ export function generatePermissionTypes(catalog: AuthzCatalog): string { return `// Generated by \`wrnexus authz generate\`. DO NOT EDIT. diff --git a/packages/authz/test/catalog.test.ts b/packages/authz/test/catalog.test.ts index cc9a00db..e255d428 100644 --- a/packages/authz/test/catalog.test.ts +++ b/packages/authz/test/catalog.test.ts @@ -71,6 +71,64 @@ describe("mergeCatalogs", () => { expect(() => (catalog.permissions as Map).set("x:y", {} as never)).toThrow(); }); + test("a role's granted-entries array cannot be mutated to escalate it after boot", () => { + // frozenMap only blocks the Map's own mutators (set/delete/clear) — the + // VALUES it holds are a separate concern. Without freezing them too, + // catalog.roles.get("editor").push("*") would succeed and silently + // escalate "editor" to a full wildcard past an error string that claims + // the catalog is frozen after boot. + const catalog = mergeCatalogs([ + { source: "a.ts", module: defineAuthz({ roles: { editor: ["post:write"] } }) }, + ]); + const editorRole = catalog.roles.get("editor")!; + expect(() => (editorRole as string[]).push("*")).toThrow(); + expect(catalog.roles.get("editor")).toEqual(["post:write"]); + }); + + test("a permission's metadata object cannot be mutated after boot", () => { + const catalog = mergeCatalogs([ + { + source: "a.ts", + module: defineAuthz({ permissions: { "post:delete": { risk: "low" } } }), + }, + ]); + const meta = catalog.permissions.get("post:delete")!; + expect(() => { + (meta as { risk?: string }).risk = "high"; + }).toThrow(); + expect(catalog.permissions.get("post:delete")!.risk).toBe("low"); + }); + + test("an attribute's metadata object cannot be mutated after boot", () => { + const catalog = mergeCatalogs([ + { + source: "a.ts", + module: defineAuthz({ attributes: { department: { description: "org unit" } } }), + }, + ]); + const meta = catalog.attributes.get("department")!; + expect(() => { + (meta as { description?: string }).description = "tampered"; + }).toThrow(); + expect(catalog.attributes.get("department")!.description).toBe("org unit"); + }); + + test("a binding's policy-name array cannot be mutated after boot", () => { + const catalog = mergeCatalogs([ + { + source: "a.ts", + module: defineAuthz({ + permissions: { "post:write": {} }, + policies: { ownsPost: async () => ({ allowed: true }) }, + bindings: { "post:write": ["ownsPost"] }, + }), + }, + ]); + const names = catalog.bindings.get("post:write")!; + expect(() => (names as string[]).push("injectedPolicy")).toThrow(); + expect(catalog.bindings.get("post:write")).toEqual(["ownsPost"]); + }); + test("emptyCatalog has no entries", () => { expect(emptyCatalog().permissions.size).toBe(0); });