diff --git a/.changeset/ai-route-user-system-permissions.md b/.changeset/ai-route-user-system-permissions.md new file mode 100644 index 0000000000..d8f62b2beb --- /dev/null +++ b/.changeset/ai-route-user-system-permissions.md @@ -0,0 +1,43 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): carry the capability channel onto an AI route's `req.user` (#4705) + +`/ai/*` was the one route domain in the platform where a capability check could +not be written. The dispatcher builds `req.user` from the request's +ExecutionContext, and `resolveAuthzContext` resolves a caller into **two** lists +that look alike and are not: + +| ExecutionContext field | Carries | +|---|---| +| `permissions` | permission-**set names** (`admin_full_access`, `organization_admin`, `member_default`) plus the synthesized `ai_seat` | +| `systemPermissions` | **capabilities** — `manage_metadata`, `studio.access`, `setup.access`, … — the union of every resolved set's `systemPermissions[]` | + +Only the first was copied. Every other surface gates on the second +(`domains/meta.ts`'s `manage_metadata` check, `action-execution.ts`, +`rest-server.ts`), so the same test written against an AI route's +`req.user.permissions` was **permanently false** — a gate built on it would not +have tightened the route, it would have closed it on platform admins too. That +is what blocked the capability gate on +`POST /api/v1/ai/tools/:toolName/execute`, where any authenticated user can +currently run any registered tool (`create_object`, `apply_blueprint`, +`create_seed`) in the default configuration. + +`req.user` now carries `systemPermissions` alongside `permissions`, with the +same fail-closed default the neighbouring fields use: a non-array — or an +ExecutionContext that has none, since the field is optional — becomes `[]`, +never `undefined`. The two channels are copied **side by side and never merged**: +flattening either into the other would corrupt every existing reader of +`permissions` while appearing to fix this. + +This is transport only. No route in this package gates on the new field, and the +declared-but-unenforced `route.permissions` mechanism is untouched — consumers +decide policy, on the platform's existing `systemPermissions` contract. + +The other producer of an AI-route `req.user` — `dispatcher-plugin`'s +`resolveRequestUser`, backing the concrete per-route mounts — has no +ExecutionContext to read and stays capability-less on purpose. It now says so in +the same shape (`systemPermissions: []`, spelled out rather than omitted) so a +consumer never sees `undefined` on one path and `[]` on the other, and so needs +no fallback of its own to tell them apart. diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index 7575e3e1f1..f6eff73d88 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -1164,6 +1164,25 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu // populated from the ExecutionContext by the /ai/* dispatch path // (http-dispatcher → resolveExecutionContext, the single scope-correct // source). This concrete-route resolver returns an empty set. + // + // [#4705] `systemPermissions` — the CAPABILITY channel + // (`manage_metadata`, `studio.access`, …) that domains/ai.ts + // now carries across from the ExecutionContext — is spelled + // out here as an empty array for the same reason + // `permissions` is: this resolver has no ExecutionContext to + // read, and inventing a capability source for it would hand + // out authority the platform never granted. Written + // explicitly rather than omitted so the two producers of an + // AI-route `req.user` agree on the SHAPE: a consumer sees + // "holds no capabilities", never `undefined`, so it never + // needs a `?? []` of its own to tell the two apart. + // + // Reachability, for the record: these concrete mounts are + // shadowed in practice — `registerAIRoutes` mounts the + // `${prefix}/ai/*` method-wildcards through + // `dispatcher.dispatch()` EARLIER in this same `start()`, so + // the ExecutionContext-backed path is the one that answers a + // real `/api/v1/ai/...` request. return { userId, id: userId, @@ -1171,6 +1190,7 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu email: sessionData?.user?.email, positions: [], permissions: [], + systemPermissions: [], organizationId: sessionData?.session?.activeOrganizationId, }; } catch { diff --git a/packages/runtime/src/domains/ai-request-user-capability-channel.test.ts b/packages/runtime/src/domains/ai-request-user-capability-channel.test.ts new file mode 100644 index 0000000000..c47e5ecd8b --- /dev/null +++ b/packages/runtime/src/domains/ai-request-user-capability-channel.test.ts @@ -0,0 +1,261 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4705 — the AI routes' `req.user` must carry the CAPABILITY channel, and it + * must stay a channel of its own. + * + * `resolveAuthzContext` resolves a caller into two lists that look alike and + * are not (core/src/security/resolve-authz-context.ts): + * + * - `permissions` — permission-SET NAMES (`admin_full_access`, + * `organization_admin`, `member_default`) plus the + * synthesized `ai_seat`. + * - `systemPermissions` — CAPABILITIES (`manage_metadata`, `studio.access`, + * `setup.access`, …), the union of every resolved + * set's `systemPermissions[]`. + * + * `resolveExecutionContext` surfaces both, side by side. `domains/ai.ts` copied + * only the first onto `req.user`, which made `/ai/*` the single route domain in + * the repo where a capability gate could not be written: every other surface + * tests `systemPermissions` (`domains/meta.ts`'s `manage_metadata` gate, + * `action-execution.ts`, `rest/src/rest-server.ts`), so the same test written + * against an AI route's `req.user.permissions` is permanently false — which + * shuts the route on platform admins instead of tightening it. That is the + * blocker cloud#1015 hit when it went to gate + * `POST /api/v1/ai/tools/:toolName/execute`. + * + * These tests pin the transport, not a policy: no route in this repo gates on + * the field. Three things must hold, and the third is why the first two are not + * enough on their own — + * + * 1. a capability held on the ExecutionContext ARRIVES on + * `req.user.systemPermissions`; + * 2. a caller holding none gets `[]`, never `undefined` (fail-closed, and it + * spares every consumer a `?? []` of its own); + * 3. `permissions` still carries set names + `ai_seat` VERBATIM — the two + * channels are copied side by side, never merged. Flattening one into the + * other would corrupt every existing reader of `permissions` while looking + * like it fixed this. + */ + +import { describe, it, expect } from 'vitest'; + +import { handleAIRequest } from './ai.js'; +import { createDispatcherPlugin } from '../dispatcher-plugin.js'; +import type { DomainHandlerDeps } from '../domain-handler-registry.js'; +import type { HttpProtocolContext } from '../http-dispatcher.js'; + +const TOOL_ROUTE = '/api/v1/ai/tools/:toolName/execute'; + +/** Captures the `req` the AI route handler is dispatched with. */ +function makeDeps(seen: { req?: any }): DomainHandlerDeps { + const aiService: any = { chat: async () => ({ text: 'ok' }) }; + return { + resolveService: (async (name: string) => (name === 'ai' ? aiService : undefined)) as any, + getRegisteredAiRoutes: () => [ + { + method: 'POST', + path: TOOL_ROUTE, + auth: true, + handler: async (req: any) => { + seen.req = req; + return { status: 200, body: { success: true, data: { ok: true } } }; + }, + }, + ], + success: (data: any) => ({ status: 200, body: { success: true, data } }), + error: (message: string, httpStatus = 500) => ({ status: httpStatus, body: { success: false, error: { message } } }), + routeNotFound: (route: string) => ({ status: 404, body: { success: false, error: { code: 'ROUTE_NOT_FOUND', route } } }), + } as unknown as DomainHandlerDeps; +} + +/** Dispatch `POST /ai/tools/create_object/execute` under `executionContext`. */ +async function dispatchToolExecute(executionContext: any) { + const seen: { req?: any } = {}; + const context = { executionContext } as unknown as HttpProtocolContext; + const result = await handleAIRequest( + makeDeps(seen), + '/ai/tools/create_object/execute', + 'POST', + {}, + {}, + context, + ); + return { result, user: seen.req?.user, req: seen.req }; +} + +describe('#4705 — /ai/* req.user carries the capability channel', () => { + it('surfaces ec.systemPermissions on req.user.systemPermissions', async () => { + // A platform admin as `resolveAuthzContext` actually resolves one: the + // set NAME in `permissions`, the capabilities it grants in + // `systemPermissions` (plugin-security's `admin_full_access` declares + // `manage_metadata` / `studio.access` / `setup.access` there). + const { user, result } = await dispatchToolExecute({ + userId: 'usr_admin', + userEmail: 'admin@objectos.ai', + positions: ['platform_admin'], + permissions: ['admin_full_access', 'ai_seat'], + systemPermissions: ['manage_users', 'manage_metadata', 'studio.access', 'setup.access'], + tenantId: 'org_1', + }); + + expect(result.handled).toBe(true); + // The whole point of the issue: a capability gate on an AI route can + // now be written and can actually pass for someone who holds it. + expect(user.systemPermissions).toEqual([ + 'manage_users', 'manage_metadata', 'studio.access', 'setup.access', + ]); + expect(new Set(user.systemPermissions).has('manage_metadata')).toBe(true); + }); + + it('keeps `permissions` = permission-set names + ai_seat, unmerged with capabilities', async () => { + const { user } = await dispatchToolExecute({ + userId: 'usr_admin', + permissions: ['admin_full_access', 'ai_seat'], + systemPermissions: ['manage_metadata', 'studio.access'], + positions: ['platform_admin'], + }); + + // Verbatim — the set-name channel is untouched by the addition, and + // `ai_seat` (synthesized by resolveExecutionContext) still rides it. + expect(user.permissions).toEqual(['admin_full_access', 'ai_seat']); + expect(user.roles).toEqual(['platform_admin']); + // …and neither list has absorbed the other. A capability must NOT be + // readable off `permissions`, nor a set name off `systemPermissions`: + // that conflation is the failure mode this issue exists to prevent. + expect(user.permissions).not.toContain('manage_metadata'); + expect(user.systemPermissions).not.toContain('admin_full_access'); + expect(user.systemPermissions).not.toContain('ai_seat'); + }); + + it('gives a caller with no capabilities [] — not undefined', async () => { + // An ordinary member: holds an AI seat, holds no capability. The + // ExecutionContext omits the field entirely (it is optional on + // `ExecutionContextSchema`), which is the shape a consumer would + // otherwise have to tolerate with a `?? []` of its own. + const { user } = await dispatchToolExecute({ + userId: 'usr_member', + permissions: ['member_default', 'ai_seat'], + positions: ['member'], + }); + + expect(user.systemPermissions).toEqual([]); + expect(user.systemPermissions).not.toBeUndefined(); + expect(new Set(user.systemPermissions).has('manage_metadata')).toBe(false); + }); + + it('fails closed when ec.systemPermissions is not an array', async () => { + const { user } = await dispatchToolExecute({ + userId: 'usr_member', + permissions: ['member_default'], + // A malformed/stringified value must never become authority. + systemPermissions: 'manage_metadata', + }); + + expect(user.systemPermissions).toEqual([]); + }); +}); + +// ── The OTHER producer of an AI-route `req.user` ──────────────────────────── +// `dispatcher-plugin`'s `resolveRequestUser` backs the concrete per-route +// mounts. It has no ExecutionContext to read, so it stays capability-less on +// purpose — but it must say so in the SAME shape, otherwise a consumer sees +// `undefined` on one path and `[]` on the other and reaches for a fallback to +// paper over the difference. + +function makeFakeServer() { + const routes: string[] = []; + const handlers: Record any> = {}; + const rec = (verb: string) => (path: string, handler: any) => { + routes.push(`${verb} ${path}`); + handlers[`${verb} ${path}`] = handler; + }; + return { + routes, + handlers, + server: { + get: rec('GET'), post: rec('POST'), put: rec('PUT'), + delete: rec('DELETE'), patch: rec('PATCH'), + }, + }; +} + +function makeCtx(fakeServer: any, aiRoutes: any[], onRequest: (req: any) => void) { + const kernel: any = { + getService: () => undefined, + getServiceAsync: async () => undefined, + // The AIServicePlugin's cross-plugin cache the dispatcher recovers + // routes from when the `ai:routes` hook fired before it was listening. + __aiRoutes: aiRoutes.map((r) => ({ + ...r, + handler: async (req: any) => { onRequest(req); return { status: 200, body: { ok: true } }; }, + })), + }; + const authService: any = { + api: { + getSession: async () => ({ + user: { id: 'usr_admin', name: 'Admin', email: 'admin@objectos.ai' }, + session: { activeOrganizationId: 'org_1' }, + }), + }, + }; + return { + getKernel: () => kernel, + getService: (name: string) => + name === 'http.server' ? fakeServer : name === 'auth' ? authService : undefined, + environmentId: undefined, + logger: { info() {}, warn() {}, error() {}, debug() {} }, + hook: () => {}, + on: () => {}, + } as any; +} + +describe('#4705 — the concrete-mount producer agrees on the shape', () => { + it('resolveRequestUser emits systemPermissions: [] (fail-closed, never undefined)', async () => { + const { server, handlers } = makeFakeServer(); + let seen: any; + const ctx = makeCtx( + server, + [{ method: 'POST', path: '/ai/tools/:toolName/execute', description: 'x', auth: true }], + (req) => { seen = req; }, + ); + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.(ctx); + + const handler = handlers[`POST ${TOOL_ROUTE}`]; + expect(handler).toBeTypeOf('function'); + const res = { status() { return res; }, header() { return res; }, json() { return res; } } as any; + await handler({ headers: {}, body: {}, params: { toolName: 'create_object' }, query: {} }, res); + + expect(seen.user.userId).toBe('usr_admin'); + // No ExecutionContext here → no authority, stated explicitly. Same + // shape as the dispatch path, so a consumer needs no `?? []`. + expect(seen.user.permissions).toEqual([]); + expect(seen.user.systemPermissions).toEqual([]); + }); + + it('mounts the /ai/* dispatch wildcard BEFORE the concrete AI routes', async () => { + // Why the capability-less resolver above is not the live path: the + // method-wildcards `registerAIRoutes` mounts go through + // `dispatcher.dispatch()` → `domains/ai.ts`, i.e. the + // ExecutionContext-backed `req.user`. They are registered earlier in + // `start()`, so they answer a real `/api/v1/ai/...` request first. + // Reordering these two would silently hand `/ai/*` back to the + // capability-less producer — and a gate built on cloud#1015's contract + // would start 403-ing platform admins again. + const { server, routes } = makeFakeServer(); + const ctx = makeCtx( + server, + [{ method: 'POST', path: '/ai/tools/:toolName/execute', description: 'x', auth: true }], + () => {}, + ); + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.(ctx); + + const wildcard = routes.indexOf('POST /api/v1/ai/*'); + const concrete = routes.indexOf(`POST ${TOOL_ROUTE}`); + expect(wildcard).toBeGreaterThanOrEqual(0); + expect(concrete).toBeGreaterThanOrEqual(0); + expect(wildcard).toBeLessThan(concrete); + }); +}); diff --git a/packages/runtime/src/domains/ai.ts b/packages/runtime/src/domains/ai.ts index bae579adc8..e92e621534 100644 --- a/packages/runtime/src/domains/ai.ts +++ b/packages/runtime/src/domains/ai.ts @@ -142,6 +142,35 @@ export async function handleAIRequest(deps: DomainHandlerDeps, subPath: string, // `ai_seat` is synthesized into ec.permissions by resolveExecutionContext // (the single, scope-correct source — security/resolve-execution-context.ts), // so it flows through here with no extra per-request lookup. + // + // [#4705] `permissions` and `systemPermissions` are TWO channels, and + // both have to cross this seam — they are not interchangeable and must + // never be flattened into one another: + // + // - `ec.permissions` → permission-SET NAMES (`admin_full_access`, + // `organization_admin`, `member_default`) plus + // the synthesized `ai_seat`. + // (core/src/security/resolve-authz-context.ts, + // `grants.permissions.push(ps.name)`) + // - `ec.systemPermissions` → CAPABILITIES (`manage_metadata`, + // `studio.access`, `setup.access`, …), the + // union of every resolved permission set's + // `systemPermissions[]`. (same file, the + // `grants.systemPermissions.push(p)` loop) + // + // Only the first used to be copied, which made `/ai/*` the one route + // domain in the repo where a capability check was impossible: every + // other surface reads `systemPermissions` (domains/meta.ts, the + // `manage_metadata` gate; action-execution.ts; rest-server.ts), so a + // capability test written against `req.user.permissions` is + // permanently false — closing the route on platform admins too rather + // than tightening it. Copying the channel through is transport only: + // no route in THIS repo gates on it; the consumer decides. + // + // Fail-closed default, same as `roles`/`permissions`: a non-array (or + // absent — `ExecutionContext.systemPermissions` is optional) becomes + // `[]`, never `undefined`, so a consumer reads "holds nothing" instead + // of having to tolerate a missing field. const user = ec?.userId ? { userId: ec.userId, @@ -150,6 +179,7 @@ export async function handleAIRequest(deps: DomainHandlerDeps, subPath: string, email: ec.userEmail, roles: Array.isArray(ec.positions) ? ec.positions : [], permissions: Array.isArray(ec.permissions) ? ec.permissions : [], + systemPermissions: Array.isArray(ec.systemPermissions) ? ec.systemPermissions : [], organizationId: ec.tenantId, } : undefined;