docs: fix memo-key cross-authorization in the Task 7 plan snippet

The middleware's per-request memo keyed resources by String(resource.id) with
an unserialisable fallback that shared one bucket. Six demonstrated cases
cross-authorized: {id:1} vs the primitive 1; {id:7} vs {id:"7"}; object ids;
and every circular / BigInt / throwing-getter row collapsing together so the
first verdict in a request became the verdict for all of them. filterCan
returned 3 of 3 rows where 1 was permitted - it leaked, rather than denied.

Object resources now memo by identity through a WeakMap; primitives key on
JSON-encoded [scope, permission, typeof, value] so 7 and "7" stay distinct
and a tenant id containing the separator cannot collide.

Scope is also read at decision time rather than frozen when the middleware
runs, and is part of the memo key, so switching tenant mid-request no longer
returns the previous tenant's verdict.

guardPermission additionally: denies instead of 500ing when getResource
throws (and no longer leaks the loader's message), skips redirectTo for API
requests using the same rule requireAuth applies, refuses a non-local
redirect target, and sets cache-control: private, no-store.

Adds eleven regression tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-04 18:35:05 +05:30
co-authored by Claude Opus 5
parent 984c6236d3
commit cc8085bcfa
@@ -1889,6 +1889,120 @@ describe("filterCan", () => {
const posts = [{ authorId: "u1" }, { authorId: "other" }, { authorId: "u1" }]; const posts = [{ authorId: "u1" }, { authorId: "other" }, { authorId: "u1" }];
expect(await filterCan(ctx, "post:delete", posts)).toHaveLength(2); expect(await filterCan(ctx, "post:delete", posts)).toHaveLength(2);
}); });
test("does not leak rows the memo cannot serialise", async () => {
const ctx = makeCtx({ id: "u1" });
const store = memoryPermissionStore();
await store.grant("u1", "post:delete", "allow");
await withMiddleware(ctx, store);
// BigInt columns and circular references are ordinary in ORM rows. A memo
// that serialises resources funnels all of these into one shared key and
// returns the first verdict for every later row.
const circular: Record<string, unknown> = { authorId: "other" };
circular.self = circular;
const rows = [{ authorId: "u1", views: 10n }, { authorId: "other", views: 11n }, circular];
expect(await filterCan(ctx, "post:delete", rows)).toEqual([rows[0]]);
});
test("returns an empty array for no items", async () => {
const ctx = makeCtx({ id: "u1" });
await withMiddleware(ctx);
expect(await filterCan(ctx, "post:delete", [])).toEqual([]);
});
});
describe("per-request memo isolation", () => {
test("distinct resources are never cross-authorized", async () => {
const ctx = makeCtx({ id: "u1" });
const store = memoryPermissionStore();
await store.grant("u1", "post:delete", "allow");
await withMiddleware(ctx, store);
// Same id, different owner; object ids; primitives of different type.
expect(await can(ctx, "post:delete", { id: 7, authorId: "u1" })).toBe(true);
expect(await can(ctx, "post:delete", { id: "7", authorId: "other" })).toBe(false);
expect(await can(ctx, "post:delete", { id: { t: "A" }, authorId: "u1" })).toBe(true);
expect(await can(ctx, "post:delete", { id: { t: "B" }, authorId: "other" })).toBe(false);
});
test("a changed row is not authorized against the stale copy", async () => {
const ctx = makeCtx({ id: "u1" });
const store = memoryPermissionStore();
await store.grant("u1", "post:delete", "allow");
await withMiddleware(ctx, store);
expect(await can(ctx, "post:delete", { id: "p1", authorId: "u1" })).toBe(true);
expect(await can(ctx, "post:delete", { id: "p1", authorId: "someone-else" })).toBe(false);
});
test("switching tenant mid-request re-evaluates", async () => {
const ctx = makeCtx({ id: "u1" }, "t1");
const store = memoryPermissionStore();
await store.assignRole("u1", "editor", { tenantId: "t1" });
await withMiddleware(ctx, store);
expect(await can(ctx, "post:write")).toBe(true);
(ctx as { tenant?: { id: string } }).tenant = { id: "t2" };
// Scope is read at decision time, so the t1 grant must not carry over.
expect(await can(ctx, "post:write")).toBe(false);
});
});
describe("guardPermission hardening", () => {
test("throws the setup error rather than calling next", async () => {
const ctx = makeCtx({ id: "u1" }); // no authzMiddleware
let reached = false;
await expect(
guardPermission("post:write")(ctx, async () => {
reached = true;
return new Response("passed");
}),
).rejects.toThrow(/authzMiddleware/);
expect(reached).toBe(false);
});
test("a throwing getResource denies instead of 500ing", async () => {
const ctx = makeCtx({ id: "u1" });
await withMiddleware(ctx);
const guard = guardPermission("post:delete", {
getResource: () => {
throw new Error("SELECT * FROM posts WHERE id=$1 failed");
},
});
const res = await guard(ctx, async () => new Response("passed"));
expect(res.status).toBe(403);
const body = await res.text();
expect(body).not.toContain("SELECT");
});
test("redirectTo applies to page requests but not API requests", async () => {
const page = makeCtx({ id: "u1" });
await withMiddleware(page);
const redirected = await guardPermission("post:write", { redirectTo: "/login" })(
page,
async () => new Response("passed"),
);
expect(redirected.status).toBe(303);
const api = makeCtx({ id: "u1" });
(api as { url: URL }).url = new URL("http://localhost/api/posts");
await withMiddleware(api);
const json = await guardPermission("post:write", { redirectTo: "/login" })(
api,
async () => new Response("passed"),
);
// An API caller must see the denial, not follow a redirect into a 200.
expect(json.status).toBe(403);
});
test("an off-site redirectTo is refused", async () => {
const ctx = makeCtx({ id: "u1" });
await withMiddleware(ctx);
for (const target of ["https://evil.example.com/harvest", "//evil.example.com"]) {
const res = await guardPermission("post:write", { redirectTo: target })(
ctx,
async () => new Response("passed"),
);
expect(res.status).toBe(403);
}
});
}); });
``` ```
@@ -1915,8 +2029,10 @@ export const AUTHZ_LOCALS_KEY = "_authz";
interface RequestAuthz { interface RequestAuthz {
resolver: AuthzResolver; resolver: AuthzResolver;
scope?: AuthzScope; /** Memo for object resources, keyed by identity so two rows never collide. */
memo: Map<string, Promise<AuthorizationDecision>>; byRef: WeakMap<object, Map<string, Promise<AuthorizationDecision>>>;
/** Memo for primitive and absent resources. */
byValue: Map<string, Promise<AuthorizationDecision>>;
} }
function readAuthz(ctx: Context): RequestAuthz { function readAuthz(ctx: Context): RequestAuthz {
@@ -1930,48 +2046,66 @@ function readAuthz(ctx: Context): RequestAuthz {
return value; return value;
} }
/** Install the per-request resolver. Register early, after sessionAuth. */ /**
* Read the tenant from the context at decision time, not at middleware time:
* a request that switches tenant mid-flight must not keep the old scope.
*/
function currentScope(ctx: Context): AuthzScope | undefined {
const tenantId = ctx.tenant?.id;
return typeof tenantId === "string" && tenantId !== "" ? { tenantId } : undefined;
}
/** Install the per-request resolver. Register after sessionAuth and tenantMiddleware. */
export function authzMiddleware(options: AuthzResolverOptions): Middleware { export function authzMiddleware(options: AuthzResolverOptions): Middleware {
const resolver = createAuthzResolver(options); const resolver = createAuthzResolver(options);
return (ctx, next) => { return (ctx, next) => {
const request: RequestAuthz = { ctx.locals[AUTHZ_LOCALS_KEY] = {
resolver, resolver,
scope: ctx.tenant?.id ? { tenantId: ctx.tenant.id } : undefined, byRef: new WeakMap(),
memo: new Map(), byValue: new Map(),
}; } satisfies RequestAuthz;
ctx.locals[AUTHZ_LOCALS_KEY] = request;
return next(); return next();
}; };
} }
/** Stable memo key. Resources without an id fall back to their JSON shape. */
function memoKey(permission: string, resource: unknown): string {
if (resource === undefined) return permission;
const id = (resource as { id?: unknown })?.id;
if (id !== undefined && id !== null) return `${permission}${String(id)}`;
try {
return `${permission}${JSON.stringify(resource)}`;
} catch {
return `${permission}<unserialisable>`;
}
}
export function decideFor( export function decideFor(
ctx: Context, ctx: Context,
permission: string, permission: string,
resource?: unknown, resource?: unknown,
): Promise<AuthorizationDecision> { ): Promise<AuthorizationDecision> {
const request = readAuthz(ctx); const request = readAuthz(ctx);
const key = memoKey(permission, resource); const scope = currentScope(ctx);
const cached = request.memo.get(key); // Scope is part of the key: the same permission decides differently per tenant.
if (cached) return cached; // JSON-encoded so a tenant id containing the separator cannot collide.
const pending = request.resolver.decide({ const key = JSON.stringify([scope?.tenantId ?? "", permission]);
const run = () =>
request.resolver.decide({
subject: ctx.user as { id?: string } | null | undefined, subject: ctx.user as { id?: string } | null | undefined,
permission, permission,
resource, resource,
scope: request.scope, scope,
}); });
request.memo.set(key, pending);
// Object resources memo by IDENTITY. Serialising them would let two distinct
// rows share a key and cross-authorize, and unserialisable ones (circular
// refs, BigInt fields, throwing getters) would all collapse into one bucket.
if (resource !== null && (typeof resource === "object" || typeof resource === "function")) {
let perResource = request.byRef.get(resource as object);
if (!perResource) request.byRef.set(resource as object, (perResource = new Map()));
const cached = perResource.get(key);
if (cached) return cached;
const pending = run();
perResource.set(key, pending);
return pending;
}
// typeof is part of the key so 7 and "7" are not the same resource.
const valueKey = JSON.stringify([key, typeof resource, String(resource)]);
const cached = request.byValue.get(valueKey);
if (cached) return cached;
const pending = run();
request.byValue.set(valueKey, pending);
return pending; return pending;
} }
@@ -1981,30 +2115,67 @@ export async function can(ctx: Context, permission: string, resource?: unknown):
export interface GuardOptions { export interface GuardOptions {
/** Load the resource a bound policy needs. */ /** Load the resource a bound policy needs. */
getResource?: (ctx: Context) => unknown | Promise<unknown>; getResource?: (ctx: Context) => unknown;
/** Include reason and policy name in the 403 body. Off by default. */ /** Include reason and policy name in the 403 body. Off by default. */
exposeReason?: boolean; exposeReason?: boolean;
/** Redirect page requests here instead of returning 403. */ /** Redirect page requests here instead of returning 403. Must be a local path. */
redirectTo?: string; redirectTo?: string;
} }
/** Same rule requireAuth uses, replicated because authz may only import TYPES from core. */
function wantsJson(ctx: Context): boolean {
if (ctx.url.pathname.startsWith("/api/")) return true;
const accept = ctx.req.headers.get("accept") ?? "";
return accept.includes("application/json") && !accept.includes("text/html");
}
/** Reject anything that could navigate off-site or inject a header. */
function isLocalPath(value: string): boolean {
if (!value.startsWith("/") || value.startsWith("//") || value.includes("\\")) return false;
for (const character of value) {
const code = character.codePointAt(0)!;
if (code < 0x20 || code === 0x7f) return false;
}
return true;
}
/** /**
* Guard a route on a registered permission. Named `guardPermission` because * Guard a route on a registered permission. Named `guardPermission` because
* `requirePermission(rbac, permission)` already exists with a different shape. * `requirePermission(rbac, permission)` already exists with a different shape.
*/ */
export function guardPermission(permission: string, options: GuardOptions = {}): Middleware { export function guardPermission(permission: string, options: GuardOptions = {}): Middleware {
return async (ctx, next) => { return async (ctx, next) => {
const resource = options.getResource ? await options.getResource(ctx) : undefined; let resource: unknown;
if (options.getResource) {
try {
resource = await options.getResource(ctx);
} catch (error) {
// Loading the resource failed, so the policy cannot be evaluated. Deny
// rather than 500 — and never leak the loader's message to the client.
console.error(`[wrnexus:authz] getResource for '${permission}' threw; denying`, error);
return Response.json({ ok: false, error: "Forbidden" }, { status: 403 });
}
}
const result = await decideFor(ctx, permission, resource); const result = await decideFor(ctx, permission, resource);
if (result.allowed) return next(); if (result.allowed) return next();
if (options.redirectTo) {
return new Response(null, { status: 303, headers: { location: options.redirectTo } }); if (options.redirectTo && !wantsJson(ctx)) {
if (!isLocalPath(options.redirectTo)) {
console.error(
`[wrnexus:authz] redirectTo must be a local path, got '${options.redirectTo}'; denying`,
);
} else {
return new Response(null, {
status: 303,
headers: { location: options.redirectTo, "cache-control": "private, no-store" },
});
}
} }
return Response.json( return Response.json(
options.exposeReason options.exposeReason
? { ok: false, error: "Forbidden", reason: result.reason, policy: result.policy } ? { ok: false, error: "Forbidden", reason: result.reason, policy: result.policy }
: { ok: false, error: "Forbidden" }, : { ok: false, error: "Forbidden" },
{ status: 403 }, { status: 403, headers: { "cache-control": "private, no-store" } },
); );
}; };
} }