diff --git a/docs/plans/2026-08-04-authz-permissions-implementation.md b/docs/plans/2026-08-04-authz-permissions-implementation.md index 2b2a58d9..10c01502 100644 --- a/docs/plans/2026-08-04-authz-permissions-implementation.md +++ b/docs/plans/2026-08-04-authz-permissions-implementation.md @@ -2031,6 +2031,8 @@ interface RequestAuthz { resolver: AuthzResolver; /** Memo for object resources, keyed by identity so two rows never collide. */ byRef: WeakMap>>; + /** Memo for symbol resources, which also carry identity. */ + bySymbol: Map>>; /** Memo for primitive and absent resources. */ byValue: Map>; } @@ -2062,6 +2064,7 @@ export function authzMiddleware(options: AuthzResolverOptions): Middleware { ctx.locals[AUTHZ_LOCALS_KEY] = { resolver, byRef: new WeakMap(), + bySymbol: new Map(), byValue: new Map(), } satisfies RequestAuthz; return next(); @@ -2076,8 +2079,16 @@ export function decideFor( const request = readAuthz(ctx); const scope = currentScope(ctx); // Scope is part of the key: the same permission decides differently per tenant. - // JSON-encoded so a tenant id containing the separator cannot collide. - const key = JSON.stringify([scope?.tenantId ?? "", permission]); + // Subject and scope are both part of the key. A request that reassigns + // ctx.user (impersonation, step-up auth, session revocation) or ctx.tenant + // must not be served the previous principal's verdict from the memo. + const subjectId = (ctx.user as { id?: unknown } | null | undefined)?.id; + const key = JSON.stringify([ + scope?.tenantId ?? "", + permission, + typeof subjectId, + String(subjectId), + ]); const run = () => request.resolver.decide({ @@ -2090,6 +2101,19 @@ export function decideFor( // 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. + // Symbols carry identity that String() erases, so they memo by identity too. + // They are held in a plain Map rather than the WeakMap: the memo is discarded + // with the request, so there is nothing to leak. + if (typeof resource === "symbol") { + let perSymbol = request.bySymbol.get(resource); + if (!perSymbol) request.bySymbol.set(resource, (perSymbol = new Map())); + const cached = perSymbol.get(key); + if (cached) return cached; + const pending = run(); + perSymbol.set(key, pending); + return pending; + } + 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())); @@ -2100,8 +2124,10 @@ export function decideFor( 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)]); + // typeof is part of the key so 7 and "7" are not the same resource, and + // -0 keeps its sign because String(-0) is "0". + const rendered = Object.is(resource, -0) ? "-0" : String(resource); + const valueKey = JSON.stringify([key, typeof resource, rendered]); const cached = request.byValue.get(valueKey); if (cached) return cached; const pending = run(); @@ -2161,13 +2187,20 @@ export function guardPermission(permission: string, options: GuardOptions = {}): if (options.redirectTo && !wantsJson(ctx)) { if (!isLocalPath(options.redirectTo)) { + // JSON-encode: this branch exists precisely for values containing + // CR/LF, which would otherwise forge a second log line. console.error( - `[wrnexus:authz] redirectTo must be a local path, got '${options.redirectTo}'; denying`, + `[wrnexus:authz] redirectTo must be a local path, got ${JSON.stringify(options.redirectTo)}; denying`, ); } else { return new Response(null, { status: 303, - headers: { location: options.redirectTo, "cache-control": "private, no-store" }, + headers: { + // Header values must be Latin-1; a localized path like /accounts + // in non-ASCII would otherwise throw and 500 on a denial path. + location: encodeURI(options.redirectTo), + "cache-control": "private, no-store", + }, }); } }