diff --git a/.changeset/action-body-execution-context.md b/.changeset/action-body-execution-context.md new file mode 100644 index 0000000000..b69da78cfe --- /dev/null +++ b/.changeset/action-body-execution-context.md @@ -0,0 +1,41 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): action bodies execute under a real execution context — every owner-scoped write no longer dies FORBIDDEN + +An action body's `ctx.api` was never bound. The sandbox's `buildSandboxApi` +walked its whole fallback chain — no `actionCtx.api`, and the raw `ObjectQL` +engine has no `.object()` (that lives on `ScopedContext`, reachable only via +`engine.createContext()`, which the action path never called) — and landed on a +repo facade that proxied every call to the engine with **no `context`**. +`ctx.engine` had the identical hole. + +Context-less is not "trusted", it is **identity-less**, and identity-less is +strictly worse than either coherent posture: plugin-sharing's write gate +short-circuits on `!context.userId` (no user to own the record) and its bypass +needs `context.isSystem` (never set). So a `type: 'script'` action whose body +called `ctx.api.object('crm_case').update(...)` failed with +`FORBIDDEN: insufficient privileges to update crm_case` — **as the built-in +admin** — while the `[action-audit]` line on the same request announced +RLS-bypassing TRUSTED execution. Objects with a `public` sharing model, no +owner field, or a bypass listing passed the gate early, so only *some* actions +broke and the defect read as object-dependent flakiness. + +Both dispatch paths (REST `/actions/:object/:action` and MCP `run_action`) now +bind `ctx.api` to `engine.createContext(...)` and thread the same envelope +through `ctx.engine`, matching what hook bodies already get from the engine's +`buildHookApi`. The envelope is the caller's `ExecutionContext` elevated with +`isSystem: true` — the posture the action surface already documents and gates +for at invoke time (the ADR-0066 D4 capability gate and the `ai.exposed` gate +are what admit a body to trusted execution). The caller's fields are spread +first, so a body's writes stay attributable (`userId` stamps +`created_by`/`updated_by`), org-scoped (`tenantId` stamps the org column and +drives driver-level tenant isolation), and joined to an open transaction — +rather than the unattributable, org-less rows a bare `{ isSystem: true }` would +write. + +No authoring change is required: `ctx.api.object(name)` inside a `body` now +does what the docs always said it does. Bodies that worked before (public / +owner-less objects) are unaffected apart from their writes now being correctly +attributed and org-stamped. diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index 4296669246..53abf17252 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -125,9 +125,11 @@ export const onEnable = async (ctx: { ql: { registerAction: (...args: unknown[]) themselves. A `script` action whose `target` names a handler that was never `registerAction`-ed compiles fine but throws `Action 'x' on object 'y' not found` at click time. (An action with *neither* `body` nor `target` is -rejected at authoring time.) Also note the handler's `engine` facade is -**trusted** — it bypasses row- and field-level security, so enforce any -caller-specific rules yourself. +rejected at authoring time.) Also note that both data surfaces a body reaches +— `ctx.api.object(name)` and the handler's `ctx.engine` facade — are +**trusted**: they run under the caller's identity elevated to system, so they +bypass row- and field-level security (writes stay attributed to the caller and +scoped to their organization). Enforce any caller-specific rules yourself. diff --git a/packages/runtime/src/action-body-identity.test.ts b/packages/runtime/src/action-body-identity.test.ts new file mode 100644 index 0000000000..afd8c54993 --- /dev/null +++ b/packages/runtime/src/action-body-identity.test.ts @@ -0,0 +1,323 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#3914] Action bodies must execute under a REAL execution context. + * + * The regression: an action body's `ctx.api` was never bound, so the sandbox + * synthesized a repo facade against the raw engine and proxied every call with + * NO `context` — no `userId` to own the record, no `isSystem` to bypass. That + * is not "trusted", it is identity-less, and plugin-sharing's write gate + * denies it (`canEdit` short-circuits on `!context.userId`; the bypass needs + * `context.isSystem`). Every owner-scoped write from an action body died + * FORBIDDEN — as the built-in admin — while the `[action-audit]` line on the + * same request announced RLS-bypassing TRUSTED execution. `ctx.engine` had the + * identical hole. + * + * These tests pin BOTH surfaces on BOTH dispatch paths (REST + MCP), and use a + * sharing-shaped engine stub that denies a context-less write the way the real + * gate does — so a future context-less regression fails here, not in an app. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { + buildActionApi, + buildActionEngineFacade, + buildActionExecutionContext, + invokeBusinessAction, +} from './action-execution.js'; +import { actionBodyRunnerFactory } from './sandbox/body-runner.js'; +import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js'; + +/** + * An engine stub shaped like the real write path: a write carrying neither + * `context.userId` nor `context.isSystem` is refused exactly as plugin-sharing + * refuses it. `createContext` returns a ScopedContext-alike so the runtime's + * `ctx.api` binding is exercised end-to-end. + */ +function makeSharingEngine(extra: Record = {}) { + const writes: Array<{ op: string; object: string; context: any }> = []; + const gate = (op: string, object: string, context: any) => { + writes.push({ op, object, context }); + if (!context?.isSystem && !context?.userId) { + throw new Error(`FORBIDDEN: insufficient privileges to ${op} ${object}`); + } + }; + const ql: any = { + writes, + async insert(object: string, data: any, options?: any) { + gate('insert', object, options?.context); + return { id: data?.id ?? 'rec_new' }; + }, + async update(object: string, _data: any, options?: any) { + gate('update', object, options?.context); + return { ok: true }; + }, + async delete(object: string, options?: any) { + gate('delete', object, options?.context); + return { ok: true }; + }, + async find(object: string, options?: any) { + gate('find', object, options?.context); + return []; + }, + async count(object: string, options?: any) { + gate('count', object, options?.context); + return 0; + }, + createContext(context: any) { + return { + __context: context, + object: (object: string) => ({ + find: (o?: any) => ql.find(object, { ...(o ?? {}), context }), + count: (o?: any) => ql.count(object, { ...(o ?? {}), context }), + insert: (data: any) => ql.insert(object, data, { context }), + update: (data: any, o?: any) => ql.update(object, data, { ...(o ?? {}), context }), + delete: (o?: any) => ql.delete(object, { ...(o ?? {}), context }), + }), + }; + }, + ...extra, + }; + return ql; +} + +const deps: any = { resolveService: () => undefined, getObjectQL: async () => undefined }; + +describe('#3914 — buildActionExecutionContext', () => { + it('elevates the caller envelope instead of replacing it', () => { + const ctx = buildActionExecutionContext({ + userId: 'user_42', + tenantId: 'org_acme', + positions: ['sales_rep'], + email: 'rep@acme.test', + }); + // isSystem is what plugin-sharing's bypass keys on… + expect(ctx.isSystem).toBe(true); + // …and the caller's identity still rides along, so the write stays + // attributable (created_by/updated_by) and org-scoped. + expect(ctx.userId).toBe('user_42'); + expect(ctx.tenantId).toBe('org_acme'); + expect(ctx.positions).toEqual(['sales_rep']); + expect(ctx.email).toBe('rep@acme.test'); + }); + + it('yields a usable elevated context for an anonymous / self-invoked call', () => { + expect(buildActionExecutionContext(undefined)).toEqual({ isSystem: true }); + }); +}); + +describe('#3914 — ctx.engine (buildActionEngineFacade)', () => { + it('threads the elevated context through every verb', async () => { + const ql = makeSharingEngine(); + const engine = buildActionEngineFacade(deps, ql, { userId: 'user_42', tenantId: 'org_acme' }); + + await engine.insert('crm_case', { subject: 'x' }); + await engine.update('crm_case', 'case_1', { status: 'closed' }); + await engine.delete('crm_case', 'case_1'); + await engine.find('crm_case', { status: 'open' }); + + expect(ql.writes.map((w: any) => w.op)).toEqual(['insert', 'update', 'delete', 'find']); + for (const w of ql.writes) { + expect(w.context).toMatchObject({ isSystem: true, userId: 'user_42', tenantId: 'org_acme' }); + } + }); + + it('does not die FORBIDDEN on an owner-scoped update (the reported symptom)', async () => { + const ql = makeSharingEngine(); + const engine = buildActionEngineFacade(deps, ql, { userId: 'admin' }); + await expect(engine.update('crm_case', 'case_1', { status: 'closed' })).resolves.toBeUndefined(); + }); + + it('still passes the caller filter on find (context is additive, not a replacement)', async () => { + const ql = makeSharingEngine(); + const engine = buildActionEngineFacade(deps, ql, { userId: 'u1' }); + await engine.find('crm_case', { status: 'open' }); + expect(ql.writes[0].context).toMatchObject({ isSystem: true, userId: 'u1' }); + // the caller's predicate must survive alongside the injected context + expect((ql.writes as any)[0]).toBeDefined(); + }); +}); + +describe('#3914 — ctx.api (buildActionApi)', () => { + it('binds to engine.createContext() with the elevated caller envelope', async () => { + const ql = makeSharingEngine(); + const api = buildActionApi(deps, ql, { userId: 'user_42', tenantId: 'org_acme' }); + expect(api).toBeDefined(); + expect(typeof api.object).toBe('function'); + await api.object('crm_case').update({ status: 'closed' }, { where: { id: 'case_1' } }); + expect(ql.writes[0].context).toMatchObject({ isSystem: true, userId: 'user_42' }); + }); + + it('falls back to a bare elevated context when the caller envelope is unparseable', () => { + const ql: any = { + createContext(ctx: any) { + if (ctx.userId !== undefined && typeof ctx.userId !== 'string') throw new Error('bad envelope'); + return { __context: ctx, object: () => ({}) }; + }, + }; + const api = buildActionApi(deps, ql, { userId: 42 }); + expect(api.__context).toEqual({ isSystem: true }); + }); + + it('returns undefined for an engine with no createContext (sandbox fallback stays in charge)', () => { + expect(buildActionApi(deps, { insert: async () => ({}) }, { userId: 'u1' })).toBeUndefined(); + }); +}); + +describe('#3914 — REST /actions dispatch binds ctx.api and ctx.engine', () => { + const schemaOf = (name: string) => ({ + name, + actions: [{ name: 'close_case', label: 'Close', type: 'script', execute: 'true' }], + }); + + const makeDispatcher = (executionContext: any) => { + const executeAction = vi.fn(async () => ({ ok: true })); + const ql = makeSharingEngine({ executeAction, getSchema: schemaOf, registry: { getObject: schemaOf } }); + const kernel: any = { context: { getService: (n: string) => (n === 'objectql' ? ql : null) } }; + const dispatcher = new HttpDispatcher(kernel); + const ctx: any = { request: {}, environmentId: 'platform', executionContext }; + return { dispatcher, executeAction, ql, ctx }; + }; + + it('hands the handler a ctx.api whose writes carry the elevated context', async () => { + const { dispatcher, executeAction, ql, ctx } = makeDispatcher({ + userId: 'user_42', positions: [], permissions: [], tenantId: 'org_acme', + }); + await dispatcher.handleActions('/crm_case/close_case', 'POST', { recordId: 'case_1' }, ctx); + + const actionCtx = executeAction.mock.calls[0]?.[2]; + expect(actionCtx.api).toBeDefined(); + expect(typeof actionCtx.api.object).toBe('function'); + + await actionCtx.api.object('crm_case').update({ status: 'closed' }, { where: { id: 'case_1' } }); + const write = ql.writes.find((w: any) => w.op === 'update'); + expect(write.context).toMatchObject({ isSystem: true, userId: 'user_42', tenantId: 'org_acme' }); + }); + + it('hands the handler a ctx.engine whose writes carry the elevated context', async () => { + const { dispatcher, executeAction, ql, ctx } = makeDispatcher({ + userId: 'user_42', positions: [], permissions: [], + }); + await dispatcher.handleActions('/crm_case/close_case', 'POST', { recordId: 'case_1' }, ctx); + + const actionCtx = executeAction.mock.calls[0]?.[2]; + // Was: `ql.update(object, data, { where })` with no context → FORBIDDEN. + await expect(actionCtx.engine.update('crm_case', 'case_1', { status: 'closed' })).resolves.toBeUndefined(); + expect(ql.writes.find((w: any) => w.op === 'update').context).toMatchObject({ + isSystem: true, userId: 'user_42', + }); + }); + + it('carries the same envelope on ctx.executionContext for the sandbox fallback', async () => { + const { dispatcher, executeAction, ctx } = makeDispatcher({ userId: 'user_42', positions: [], permissions: [] }); + await dispatcher.handleActions('/crm_case/close_case', 'POST', {}, ctx); + expect(executeAction.mock.calls[0]?.[2]?.executionContext).toMatchObject({ + isSystem: true, userId: 'user_42', + }); + }); + + it('still elevates for an anonymous / self-invoked call', async () => { + const { dispatcher, executeAction, ql, ctx } = makeDispatcher(undefined); + await dispatcher.handleActions('/crm_case/close_case', 'POST', {}, ctx); + const actionCtx = executeAction.mock.calls[0]?.[2]; + await actionCtx.engine.update('crm_case', 'case_1', { status: 'closed' }); + expect(ql.writes.find((w: any) => w.op === 'update').context).toMatchObject({ isSystem: true }); + }); +}); + +describe('#3914 — MCP run_action dispatch binds ctx.api and ctx.engine', () => { + const action = { + name: 'close_case', + label: 'Close', + objectName: 'crm_case', + type: 'script', + target: 'closeCase', + ai: { exposed: true, description: 'Close a case.' }, + }; + const obj = { name: 'crm_case', actions: [action] }; + + it('threads the caller identity into the body ctx (was context-less)', async () => { + const executeAction = vi.fn(async () => ({ ok: true })); + const ql = makeSharingEngine({ executeAction }); + const mcpDeps: any = { + resolveService: async () => null, + getObjectQL: async () => ql, + }; + const ec = { userId: 'user_42', tenantId: 'org_acme', positions: [], permissions: [] }; + + await invokeBusinessAction(mcpDeps, 'close_case', { recordId: 'case_1' }, { + driver: undefined, + envId: 'platform', + ec, + getMeta: () => ({ listObjects: async () => [obj] }), + callData: async () => ({ record: { id: 'case_1' } }), + }); + + const actionCtx = executeAction.mock.calls[0]?.[2]; + expect(actionCtx.api).toBeDefined(); + await actionCtx.api.object('crm_case').update({ status: 'closed' }, { where: { id: 'case_1' } }); + await actionCtx.engine.insert('crm_task', { subject: 'follow up' }); + for (const w of ql.writes.filter((x: any) => x.op !== 'find')) { + expect(w.context).toMatchObject({ isSystem: true, userId: 'user_42', tenantId: 'org_acme' }); + } + }); +}); + +describe('#3914 — sandboxed action body reaches the bound ctx.api', () => { + const runner = new QuickJSScriptRunner(); + + it('uses the host-supplied ctx.api rather than synthesizing a context-less facade', async () => { + const ql = makeSharingEngine(); + const factory = actionBodyRunnerFactory(runner, { ql, appId: 'crm' }); + const fn = factory({ + name: 'close_case', + object: 'crm_case', + type: 'script', + body: { + language: 'js', + source: "await ctx.api.object('crm_case').update({ status: 'closed' }, { where: { id: ctx.recordId } }); return { ok: true };", + capabilities: ['api.write'], + }, + } as any); + expect(typeof fn).toBe('function'); + + await fn!({ + record: { id: 'case_1' }, + params: {}, + api: buildActionApi(deps, ql, { userId: 'user_42' }), + executionContext: buildActionExecutionContext({ userId: 'user_42' }), + }); + + const write = ql.writes.find((w: any) => w.op === 'update'); + expect(write).toBeDefined(); + expect(write.context).toMatchObject({ isSystem: true, userId: 'user_42' }); + }); + + it('elevates the last-resort repo facade from ctx.executionContext', async () => { + // Engine with NO createContext and NO .object() — the shape that made + // buildSandboxApi fall all the way through to a context-less facade. + const ql = makeSharingEngine(); + delete (ql as any).createContext; + const factory = actionBodyRunnerFactory(runner, { ql, appId: 'crm' }); + const fn = factory({ + name: 'close_case', + object: 'crm_case', + type: 'script', + body: { + language: 'js', + source: "await ctx.api.object('crm_case').update({ status: 'closed' }, { where: { id: ctx.recordId } }); return { ok: true };", + capabilities: ['api.write'], + }, + } as any); + + await fn!({ + record: { id: 'case_1' }, + params: {}, + executionContext: buildActionExecutionContext({ userId: 'user_42' }), + }); + + const write = ql.writes.find((w: any) => w.op === 'update'); + expect(write.context).toMatchObject({ isSystem: true, userId: 'user_42' }); + }); +}); diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index b686cae484..c84e9913ee 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -231,11 +231,12 @@ export function actionPermissionError(deps: ActionExecutionDeps, actionDef: any, * into the AI surface with `ai.exposed: true`, or `null` when exposed. * * This gate is the REAL agent-facing boundary for actions: script/body - * handlers execute as TRUSTED application code (the engine facade carries no - * ExecutionContext — see {@link buildActionEngineFacade}), so once invoked, a - * body's reads/writes are NOT bounded by the caller's RLS/FLS or an agent's - * data ceiling (ADR-0090 D10). The author's explicit opt-in — not a data-layer - * backstop — therefore decides what AI may trigger. Fail-closed by default. + * handlers execute as TRUSTED application code (the engine facade and + * `ctx.api` run `isSystem` — see {@link buildActionExecutionContext}), so once + * invoked, a body's reads/writes are NOT bounded by the caller's RLS/FLS or an + * agent's data ceiling (ADR-0090 D10). The author's explicit opt-in — not a + * data-layer backstop — therefore decides what AI may trigger. Fail-closed by + * default. */ export function actionAiExposureError(deps: ActionExecutionDeps, actionDef: any, objectName?: string): string | null { if (actionDef?.ai?.exposed === true) return null; @@ -521,27 +522,91 @@ export function buildActionSession(deps: ActionExecutionDeps, ec: any): any | un }; } -export function buildActionEngineFacade(deps: ActionExecutionDeps, ql: any): any { +/** + * The ExecutionContext an action BODY's own reads/writes execute under — + * the caller's envelope, elevated with `isSystem: true` (#3914). + * + * This is what makes the `[action-audit]` line TRUE. Before #3914 the body's + * engine facade called `ql.update(...)` with NO context at all, which is not + * "trusted" — it is IDENTITY-LESS, and identity-less is strictly WORSE than + * either coherent posture: plugin-sharing's write gate short-circuits on + * `!context.userId` (there is no user to own anything) and its bypass needs + * `context.isSystem`, so every owner-scoped write from an action body died + * `FORBIDDEN` — as the built-in admin — while the audit line announced + * RLS-bypassing trusted execution. Objects with a `public` sharing model or + * no owner field passed the gate early, which is why only *some* actions + * broke and the defect read as object-dependent flakiness. + * + * Elevating (rather than binding to the caller's RLS) is the posture #2849 + * already documents and gates for: an action body is trusted code, admitted + * by the invoke-time capability + `ai.exposed` checks, and hook bodies + * already get exactly this (the engine's `buildHookApi` falls back to + * `{ isSystem: true }`). Spreading the caller's envelope FIRST keeps the + * write attributable and correctly scoped — `userId` stamps `created_by` / + * `updated_by`, `tenantId` stamps the org column and drives driver-level + * tenant isolation, `transaction` joins an open transaction — instead of the + * unattributable, org-less rows a bare `{ isSystem: true }` would write. + */ +export function buildActionExecutionContext(ec: any): Record { + const base = ec && typeof ec === 'object' ? { ...(ec as Record) } : {}; + return { ...base, isSystem: true }; +} + +/** + * Build the action-body `ctx.api` — a real `ScopedContext` bound to + * {@link buildActionExecutionContext}, mirroring what hook bodies get from + * `HookContext.api` (#3914). + * + * Without this the sandbox's `buildSandboxApi` fell through to a repo facade + * synthesized against the raw engine's CRUD primitives: the raw `ObjectQL` + * engine has no `.object()` (that lives on `ScopedContext`, reachable only + * via `engine.createContext()`, which the action path never called), so the + * facade proxied every call context-less. Returns `undefined` when the engine + * predates `createContext`, leaving the sandbox's own fallback in charge. + */ +export function buildActionApi(deps: ActionExecutionDeps, ql: any, ec: any): any | undefined { + if (!ql || typeof ql.createContext !== 'function') return undefined; + try { + return ql.createContext(buildActionExecutionContext(ec)); + } catch { + // A malformed caller envelope must not sink the action — fall back to + // the bare elevated context (the same shape hooks default to). + try { + return ql.createContext({ isSystem: true }); + } catch { + return undefined; + } + } +} + +/** + * Build the action-body `ctx.engine` — the slim CRUD surface handler suites + * use. Every call carries {@link buildActionExecutionContext} so `ctx.engine` + * and `ctx.api` write under the SAME identity (#3914); passing `ec` is what + * separates a trusted write from a context-less one. + */ +export function buildActionEngineFacade(deps: ActionExecutionDeps, ql: any, ec?: any): any { + const context = buildActionExecutionContext(ec); return { async insert(object: string, data: Record): Promise<{ id: string }> { - const res = await ql.insert(object, data); + const res = await ql.insert(object, data, { context }); const id = (res && (res as any).id) ?? (data as any).id; return { id }; }, async update(object: string, id: string, data: Record): Promise { - await ql.update(object, data, { where: { id } }); + await ql.update(object, data, { where: { id }, context }); }, // Tolerant of both the single-id and array conventions handler suites // use (CRM handlers pass one id; todo handlers pass an id array). async delete(object: string, idOrIds: string | string[]): Promise { const ids = Array.isArray(idOrIds) ? idOrIds : [idOrIds]; for (const id of ids) { - if (id != null) await ql.delete(object, { where: { id } }); + if (id != null) await ql.delete(object, { where: { id }, context }); } }, async find(object: string, query: Record): Promise>> { - const opts = query && Object.keys(query).length ? { where: query } : undefined; - const rows = await ql.find(object, opts as any); + const where = query && Object.keys(query).length ? { where: query } : {}; + const rows = await ql.find(object, { ...where, context } as any); return Array.isArray(rows) ? rows : ((rows as any)?.value ?? []); }, }; @@ -556,12 +621,14 @@ export function buildActionEngineFacade(deps: ActionExecutionDeps, ql: any): any * Throws on denial / not-found / handler failure so the tool surfaces a * clean tool-error. No service-ai dependency. * - * SECURITY MODEL (#2849): all gating happens at INVOKE time. A script/body - * handler then runs as trusted code — its engine facade performs - * context-less reads/writes that bypass RLS/FLS (SECURITY-DEFINER-like), so - * the caller's permissions and an agent's ADR-0090 D10 data ceiling do NOT - * bound what the body does internally. Flow actions differ: the flow engine - * receives the caller's identity below and honours `runAs` (ADR-0049). + * SECURITY MODEL (#2849, #3914): all gating happens at INVOKE time. A + * script/body handler then runs as trusted code — its `ctx.engine` and + * `ctx.api` perform `isSystem` reads/writes that bypass RLS/FLS + * (SECURITY-DEFINER-like), so the caller's permissions and an agent's + * ADR-0090 D10 data ceiling do NOT bound what the body does internally. The + * caller's identity still RIDES the elevated context so those writes stay + * attributable and org-scoped. Flow actions differ: the flow engine receives + * the caller's identity below and honours `runAs` (ADR-0049). */ export async function invokeBusinessAction(deps: ActionExecutionDeps, name: string, @@ -654,18 +721,27 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps, if (!ql || typeof ql.executeAction !== 'function') { throw new Error('Data engine not available for action dispatch'); } - // [#2849] Trusted-mode elevation must be AUDIBLE: the body's engine - // facade bypasses RLS/FLS, so record who triggered which action. + // [#2849] Trusted-mode elevation must be AUDIBLE: the body's `ctx.engine` + // and `ctx.api` bypass RLS/FLS, so record who triggered which action. + // [#3914] Wording tracks what the body ACTUALLY gets — a system-elevated + // context carrying the caller's identity, not a context-less engine. console.info( `[action-audit] MCP run_action '${action.name}' on '${objectName}' — body executes TRUSTED ` + - `(context-less engine, RLS/FLS-bypassing) for user '${ec?.userId ?? 'anonymous'}'` + + `(system-elevated context, RLS/FLS-bypassing) for user '${ec?.userId ?? 'anonymous'}'` + (ec?.principalKind === 'agent' ? ` (AGENT on behalf of '${ec?.onBehalfOf?.userId ?? 'unknown'}')` : ''), ); const actionContext: any = { record, user, session: buildActionSession(deps, ec), - engine: buildActionEngineFacade(deps, ql), + engine: buildActionEngineFacade(deps, ql, ec), + // [#3914] `ctx.api` — the ScopedContext a body's `ctx.api.object(...)` + // resolves to. Absent here, the sandbox synthesized a context-less + // facade and every owner-scoped write died FORBIDDEN. `executionContext` + // is the same envelope, carried so the sandbox's own last-resort facade + // is elevated identically instead of falling back to no identity. + api: buildActionApi(deps, ql, ec), + executionContext: buildActionExecutionContext(ec), params: { ...params, recordId, objectName }, }; // Handler key: body-based actions register under `name` (AppPlugin); diff --git a/packages/runtime/src/domains/actions.ts b/packages/runtime/src/domains/actions.ts index 9087e6c09b..944da0ec71 100644 --- a/packages/runtime/src/domains/actions.ts +++ b/packages/runtime/src/domains/actions.ts @@ -47,9 +47,11 @@ export function createActionsDomain(deps: DomainHandlerDeps): DomainRoute { * * Body shape: `{ recordId?: string, params?: Record }`. * The handler is invoked with an `ActionContext` of: - * `{ record, user, engine, params }` + * `{ record, user, session, engine, api, params }` * where `engine` exposes the slimmed CRUD surface used by CRM handlers - * (`insert`, `update`, `delete`, `find`). + * (`insert`, `update`, `delete`, `find`) and `api` is the ScopedContext a + * sandboxed body reaches through `ctx.api.object(...)`. Both are bound to the + * caller's ExecutionContext elevated with `isSystem` (#3914). * * Dispatch follows the DECLARED action type (#3915): * - `script` (and any action with no resolvable declaration, which is @@ -59,6 +61,10 @@ export function createActionsDomain(deps: DomainHandlerDeps): DomainRoute { * - `url` / `modal` / `form` / `api` → 400. They dispatch on `target` in the * client (or at another endpoint entirely) and have no server dispatch * here; saying so beats the registry's `not found`. + * + * A `flow` action is NOT trusted-elevated: the flow engine receives the + * caller's identity and honours `runAs` (ADR-0049). The `isSystem` elevation + * above is a script-BODY property only. */ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string, method: string, body: any, _context: HttpProtocolContext): Promise { if (method.toUpperCase() !== 'POST') { @@ -175,28 +181,6 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string } if (record && (record as any).id == null && recordId) (record as any).id = recordId; - // Slim engine facade matching the ActionContext.engine shape used by CRM - // handlers. ⚠️ TRUSTED — context-less, RLS/FLS-bypassing by design; see - // buildActionEngineFacade for the full security-model rationale (#2849). - const engineFacade = { - async insert(object: string, data: Record): Promise<{ id: string }> { - const res = await ql.insert(object, data); - const id = (res && (res as any).id) ?? (data as any).id; - return { id }; - }, - async update(object: string, id: string, data: Record): Promise { - await ql.update(object, data, { where: { id } }); - }, - async delete(object: string, id: string): Promise { - await ql.delete(object, { where: { id } }); - }, - async find(object: string, query: Record): Promise>> { - const opts = query && Object.keys(query).length ? { where: query } : undefined; - const rows = await ql.find(object, opts as any); - return Array.isArray(rows) ? rows : ((rows as any)?.value ?? []); - }, - }; - // Resolve the caller identity from the request's ExecutionContext — the // single source `dispatch()` populates via `resolveExecutionContext`, // the same envelope the MCP `runAction` and record-change trigger paths @@ -224,7 +208,18 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string record, user: userFromAuth, session: actionExec.buildActionSession(deps, ec), - engine: engineFacade, + // Slim engine facade matching the ActionContext.engine shape used by + // CRM handlers. ⚠️ TRUSTED — system-elevated, RLS/FLS-bypassing by + // design; see buildActionEngineFacade + buildActionExecutionContext + // for the full security-model rationale (#2849, #3914). + engine: actionExec.buildActionEngineFacade(deps, ql, ec), + // [#3914] `ctx.api` — the ScopedContext a body's `ctx.api.object(...)` + // resolves to. Absent here, the sandbox synthesized a context-less + // facade and every owner-scoped write died FORBIDDEN. `executionContext` + // is the same envelope, carried so the sandbox's own last-resort facade + // is elevated identically instead of falling back to no identity. + api: actionExec.buildActionApi(deps, ql, ec), + executionContext: actionExec.buildActionExecutionContext(ec), params: { ...reqParams, recordId, objectName }, }; @@ -247,9 +242,11 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string } // [#2849] Same trusted-mode elevation as the MCP path — keep it audible. + // [#3914] Wording tracks what the body ACTUALLY gets — a system-elevated + // context carrying the caller's identity, not a context-less engine. console.info( `[action-audit] REST action '${objectName}/${actionName}' — body executes TRUSTED ` + - `(context-less engine, RLS/FLS-bypassing) for user '${userFromAuth.id}'`, + `(system-elevated context, RLS/FLS-bypassing) for user '${userFromAuth.id}'`, ); // ── script/body dispatch ── diff --git a/packages/runtime/src/sandbox/body-runner.ts b/packages/runtime/src/sandbox/body-runner.ts index c0cf89f483..2538b1f02a 100644 --- a/packages/runtime/src/sandbox/body-runner.ts +++ b/packages/runtime/src/sandbox/body-runner.ts @@ -158,45 +158,61 @@ function applyMutationsToInput(engineCtx: any, result: ScriptResult): void { } } -function buildEngineRepoFacade(ql: any, objectName: string) { - // Minimal repository surface that proxies to the raw engine. - // Actions execute as the system user (no user context at HTTP boundary - // beyond `actionCtx.user`); ObjectQL's permission middleware will still - // gate writes via the engine's standard insert/update path. +/** + * Last-resort repository surface proxying to the raw engine, used only when + * the host context carried no `api` and the engine exposes no `.object()`. + * + * `context` is the ExecutionContext every call is threaded with. It is NOT + * optional in spirit: proxying context-less is what #3914 was — the write + * reaches plugin-sharing's gate with no `userId` to own the record and no + * `isSystem` to bypass, so it is denied FORBIDDEN even for an admin. A caller + * that has no context to give gets the same identity-less behavior as before, + * which is why the action path now always supplies one. + */ +function buildEngineRepoFacade(ql: any, objectName: string, context?: any) { + const withCtx = (opts?: any) => + context ? { ...(opts && typeof opts === 'object' ? opts : {}), context } : opts; return { - async find(opts?: any) { return ql.find(objectName, opts); }, + async find(opts?: any) { return ql.find(objectName, withCtx(opts)); }, async findOne(opts?: any) { - const rows = await ql.find(objectName, opts); + const rows = await ql.find(objectName, withCtx(opts)); return Array.isArray(rows) ? rows[0] ?? null : null; }, async count(opts?: any) { - if (typeof ql.count === 'function') return ql.count(objectName, opts); - const rows = await ql.find(objectName, opts); + if (typeof ql.count === 'function') return ql.count(objectName, withCtx(opts)); + const rows = await ql.find(objectName, withCtx(opts)); return Array.isArray(rows) ? rows.length : 0; }, - async insert(data: any) { return ql.insert(objectName, data); }, - async update(data: any, opts?: any) { return ql.update(objectName, data, opts); }, + async insert(data: any) { return ql.insert(objectName, data, withCtx(undefined)); }, + async update(data: any, opts?: any) { return ql.update(objectName, data, withCtx(opts)); }, async upsert(data: any, opts?: any) { - if (typeof ql.upsert === 'function') return ql.upsert(objectName, data, opts); - return ql.insert(objectName, data); + if (typeof ql.upsert === 'function') return ql.upsert(objectName, data, withCtx(opts)); + return ql.insert(objectName, data, withCtx(undefined)); }, - async delete(opts?: any) { return ql.delete(objectName, opts); }, + async delete(opts?: any) { return ql.delete(objectName, withCtx(opts)); }, }; } function buildSandboxApi(engineCtx: any, ql: any, errLabel: string) { const engineApi = engineCtx?.api; if (engineApi && typeof engineApi.object === 'function') return engineApi; + // [#3914] The host's own execution envelope, when it supplied one. Hooks get + // `api` from the engine and never reach here; actions now supply both, so + // this stays the fallback for hosts that predate either. + const execCtx = engineCtx?.executionContext; return { object: (objectName: string) => { if (!ql) throw new Error(`ObjectQL engine unavailable to ${errLabel}`); // Prefer the engine's own ScopedContext-based `.object()` when // present; otherwise synthesize a minimal repo facade against the // engine's CRUD primitives (so the body can call .insert/.find/etc). + if (typeof ql.createContext === 'function' && execCtx) { + try { return ql.createContext(execCtx).object(objectName); } catch { /* fall through */ } + } if (typeof ql.object === 'function') { try { return ql.object(objectName); } catch { /* fall through */ } } - return buildEngineRepoFacade(ql, objectName); + return buildEngineRepoFacade(ql, objectName, execCtx); }, }; }