From 99f1aa16393a5a5b91db9f84c53bfb3db81d77c8 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 23 Jun 2026 19:21:46 +0800 Subject: [PATCH] =?UTF-8?q?feat(authz):=20ADR-0066=20D4=20=E2=80=94=20acti?= =?UTF-8?q?on=20dual-surface=20gate=20(server=20source=20of=20truth)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - spec: ActionSchema gains `requiredPermissions: string[]` (mirrors App/Object). - runtime: http-dispatcher.handleActions enforces it BEFORE executing — resolves the action def from the object schema and rejects (403) when the caller's systemPermissions don't cover requiredPermissions. isSystem bypasses; an unauthenticated caller (no caps) is denied. One declaration gates both surfaces (objectui ActionRunner derives the UI hide/disable from the same field — separate PR), removing the "UI-gated but server-open" footgun. - liveness ledger: classify action.requiredPermissions (live). api-surface unchanged. - Tests: spec (+2) + full-dispatcher gate (+5: deny/allow/system-bypass/ungated/anon). Co-Authored-By: Claude Opus 4.8 --- packages/runtime/src/http-dispatcher.test.ts | 56 +++++++++++++++++++- packages/runtime/src/http-dispatcher.ts | 39 ++++++++++++++ packages/spec/liveness/action.json | 5 ++ packages/spec/src/ui/action.test.ts | 17 ++++++ packages/spec/src/ui/action.zod.ts | 10 ++++ 5 files changed, 126 insertions(+), 1 deletion(-) diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 30819df9a8..0659868aa9 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -1845,4 +1845,58 @@ describe('HttpDispatcher', () => { expect(memberQL.find).not.toHaveBeenCalled(); }); }); -}); \ No newline at end of file +}); + + +describe('HttpDispatcher — ADR-0066 D4 action requiredPermissions gate', () => { + const gated = { name: 'issue_and_sign', label: 'Issue', type: 'api', requiredPermissions: ['manage_platform_settings'] }; + const make = (actionDef: any, execCtx: any) => { + const executeAction = vi.fn().mockResolvedValue({ ran: true }); + const schemaOf = (name: string) => ({ name, actions: actionDef ? [actionDef] : [] }); + const ql: any = { + executeAction, + getSchema: schemaOf, + // getObjectQLService only returns a service when `svc.registry` is truthy. + registry: { getObject: schemaOf }, + find: vi.fn().mockResolvedValue([]), + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + }; + const kernel: any = { context: { getService: (n: string) => (n === 'objectql' ? ql : null) } }; + const dispatcher = new HttpDispatcher(kernel); + const ctx: any = { request: {}, environmentId: 'platform', executionContext: execCtx }; + return { dispatcher, executeAction, ctx }; + }; + + it('rejects (403) when the caller lacks the action capability', async () => { + const { dispatcher, executeAction, ctx } = make(gated, { userId: 'u1', systemPermissions: [] }); + const res = await dispatcher.handleActions('/sys_license/issue_and_sign', 'POST', {}, ctx); + expect(res.response.status).toBe(403); + expect(executeAction).not.toHaveBeenCalled(); + }); + + it('allows when the caller holds the capability', async () => { + const { dispatcher, executeAction, ctx } = make(gated, { userId: 'u1', systemPermissions: ['manage_platform_settings'] }); + const res = await dispatcher.handleActions('/sys_license/issue_and_sign', 'POST', {}, ctx); + expect(executeAction).toHaveBeenCalledTimes(1); + expect(res.response.status).not.toBe(403); + }); + + it('bypasses the gate for a system context', async () => { + const { dispatcher, executeAction, ctx } = make(gated, { isSystem: true }); + await dispatcher.handleActions('/sys_license/issue_and_sign', 'POST', {}, ctx); + expect(executeAction).toHaveBeenCalledTimes(1); + }); + + it('does not gate an action without requiredPermissions', async () => { + const { dispatcher, executeAction, ctx } = make({ name: 'mark_done', label: 'Mark', type: 'script', execute: 'true' }, { userId: 'u1', systemPermissions: [] }); + await dispatcher.handleActions('/task/mark_done', 'POST', {}, ctx); + expect(executeAction).toHaveBeenCalledTimes(1); + }); + + it('denies an unauthenticated caller for a gated action', async () => { + const { dispatcher, executeAction, ctx } = make(gated, undefined); + const res = await dispatcher.handleActions('/sys_license/issue_and_sign', 'POST', {}, ctx); + expect(res.response.status).toBe(403); + expect(executeAction).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 6e271dda61..0d9212343a 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -2590,6 +2590,45 @@ export class HttpDispatcher { return { handled: true, response: this.error('Data engine not available', 503) }; } + // [ADR-0066 D4] Dual-surface action gate — the server is the source of + // truth. Resolve the action's declared `requiredPermissions` from the + // object schema and reject (403) when the caller's systemPermissions + // don't cover them. The objectui ActionRunner hides/disables the same + // action from the identical declaration, so a UI-hidden action is also + // server-closed (and the inverse footgun is removed). System/engine + // self-invocation (isSystem) bypasses; an unauthenticated caller holds + // no capabilities and is therefore denied for a gated action. + try { + const actionSchema: any = + (typeof ql.getSchema === 'function' ? ql.getSchema(objectName) : undefined) ?? + ql.registry?.getObject?.(objectName); + const actionDef: any = Array.isArray(actionSchema?.actions) + ? actionSchema.actions.find((a: any) => a?.name === actionName) + : undefined; + const required: string[] = Array.isArray(actionDef?.requiredPermissions) + ? actionDef.requiredPermissions + : []; + if (required.length > 0) { + const execCtx: any = _context?.executionContext; + if (!execCtx?.isSystem) { + const held = new Set(execCtx?.systemPermissions ?? []); + const missing = required.filter((perm) => !held.has(perm)); + if (missing.length > 0) { + return { + handled: true, + response: this.error( + `Action '${actionName}' on '${objectName}' requires capability ` + + `[${required.join(', ')}] — caller is missing [${missing.join(', ')}]`, + 403, + ), + }; + } + } + } + } catch { + /* schema unresolved → no declared gate to enforce (handler-only action) */ + } + // Resolve the handler — fall back to wildcard '*' if the object-specific key is missing. // Since engine.executeAction throws when the key is unknown, we probe via the internal // map by attempting the call inside a try/catch and rotating to '*'. diff --git a/packages/spec/liveness/action.json b/packages/spec/liveness/action.json index 372cf0f59c..3a6abd1be9 100644 --- a/packages/spec/liveness/action.json +++ b/packages/spec/liveness/action.json @@ -84,6 +84,11 @@ "status": "live", "note": "objectui — CEL, fail-closed." }, + "requiredPermissions": { + "status": "live", + "evidence": "packages/runtime/src/http-dispatcher.ts", + "note": "ADR-0066 D4 dual-surface action gate. Server is source of truth: handleActions rejects (403) when the caller's systemPermissions don't cover action.requiredPermissions; objectui ActionRunner derives the same UI hide/disable. Unit-proven in packages/runtime/src/http-dispatcher.test.ts + packages/spec/src/ui/action.test.ts." + }, "disabled": { "status": "live", "evidence": "objectui: packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx:169", diff --git a/packages/spec/src/ui/action.test.ts b/packages/spec/src/ui/action.test.ts index d304467d06..b8c025db65 100644 --- a/packages/spec/src/ui/action.test.ts +++ b/packages/spec/src/ui/action.test.ts @@ -909,4 +909,21 @@ describe('ACTION_LOCATIONS — canonical source of truth', () => { }) ).toThrow(); }); + + + it('[ADR-0066 D4] ActionSchema accepts requiredPermissions', () => { + const action = ActionSchema.parse({ + name: 'issue_and_sign', + label: 'Issue & Sign', + type: 'api', + target: '/api/v1/cloud/licenses/issue', + requiredPermissions: ['manage_platform_settings'], + }); + expect(action.requiredPermissions).toEqual(['manage_platform_settings']); + }); + + it('[ADR-0066 D4] requiredPermissions is optional (absent ⇒ undefined)', () => { + const action = ActionSchema.parse({ name: 'mark_done', label: 'Mark Done', type: 'script', execute: 'true' }); + expect(action.requiredPermissions).toBeUndefined(); + }); }); diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 88d6f826d2..84a70d66d2 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -373,6 +373,16 @@ export const ActionSchema = lazySchema(() => z.object({ visible: ExpressionInputSchema.optional().describe('Visibility predicate (CEL).'), disabled: z.union([z.boolean(), ExpressionInputSchema]).optional().describe('Boolean or predicate (CEL) — action is disabled when TRUE.'), + /** + * [ADR-0066 D4] System capabilities required to INVOKE this action — a + * dual-surface gate from ONE declaration: the server (action route) rejects + * the call with 403 when the caller's systemPermissions don't cover these (the + * source of truth), and the objectui ActionRunner hides/disables the button + * using the same requirement. Independent of `visible` (CEL): this is the RBAC + * capability contract, mirroring `App.requiredPermissions`. + */ + requiredPermissions: z.array(z.string()).optional().describe('[ADR-0066 D4] Capabilities required to invoke this action (server-enforced 403 + UI hide/disable).'), + /** Keyboard Shortcut */ shortcut: z.string().optional().describe('Keyboard shortcut to trigger this action (e.g., "Ctrl+S")'),