Skip to content

Commit a548550

Browse files
os-muskclaude
andauthored
fix(objectql): elevate ObjectRepository.execute() to REST/MCP's trusted posture (#15051)
* fix(objectql): elevate ObjectRepository.execute() to REST/MCP's trusted posture Director ruling 决裁批 #24 (2026-09-01), clause 2: the census (repo + examples/ + apps/, production + test) found ZERO real callers of ObjectRepository.execute() anywhere — every hit was prose describing the shape, never an invocation — so the premise (callers are internal / test / single-digit) holds in its strongest form, and the elevation is implemented. ObjectRepository.execute() now hands the action handler a ScopedContext bound to { ...callerContext, isSystem: true } (ctx.api) and the same elevated envelope as ctx.executionContext — the identical sudo()-shaped formula buildActionExecutionContext (REST /actions, MCP run_action) and recomputeSummaries's systemCtx already use. Before this, the handler got neither api nor executionContext: a handler composing a sibling write via ctx.api.object(x).update(y) got ctx.api === undefined, and the sandbox's own last-resort fallback ran that write as a non-system caller, so the engine's static readonly strip applied to this path and not to REST /actions or MCP run_action. Fixes #13866 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * fix(docs): re-anchor system-context census row after #13866's insertion Pure line rot: content/docs/permissions/system-context.mdx:183 pointed at engine.ts:14463, which the elevation fix's inserted JSDoc pushed down to 14496 (the ScopedContext.isSystem getter itself is unchanged). Repaired via `node scripts/check-system-context-census.mjs --fix`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * fix(objectql): make the elevation pin's fake find() honor limit check:objectql-double-limit flagged the fake driver's find() in engine-repo-execute-elevation.test.ts as a new limit-blind ObjectQL double. Apply the caller's bound after the filter, by presence — the gate's own suggested fix — so the baseline stays at zero new limit-blind doubles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * fix(objectql): make the elevation pin's fake where-matcher refuse combinators check:where-matcher flagged the fake driver's matches() in engine-repo-execute-elevation.test.ts as a new silently-wrong WHERE matcher (a $-prefixed combinator key would be read as a literal field name instead of being rejected). Refuse it loudly instead, matching the exact idiom engine-readonly-strip-caller-values.test.ts's own fake driver already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f794e4e commit a548550

4 files changed

Lines changed: 267 additions & 2 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
`ObjectRepository.execute()` (the `repo.execute(actionName, params)` face a hook or action body reaches as `ctx.api.object(name).execute(...)`) now dispatches the action handler under the same elevated `ScopedContext` REST `/actions` and MCP `run_action` already give an action body — closing the third of three `executeAction` callers that #3914 argues must never run identity-less.
6+
7+
Before this change, the handler's `ctx` carried `params`, `userId`, `tenantId` and `roles` but neither `api` nor `executionContext`: a handler composing a sibling write via `ctx.api.object(x).update(y)` got `ctx.api === undefined`, and the sandbox's own last-resort fallback ran that write as a non-system caller — so the engine's static `readonly` strip (`!opCtx.context?.isSystem`) applied to a write made through this path and not to the identical write made through REST `/actions` or MCP `run_action`.
8+
9+
`ctx.api` is now a real `ScopedContext` bound to `{ ...callerContext, isSystem: true }` — the caller's own envelope, elevated — the same `sudo()`-shaped formula `buildActionExecutionContext` and `recomputeSummaries`'s `systemCtx` already use, so `userId`/`tenantId` still stamp the write and an open transaction still joins rather than escapes. `ctx.executionContext` carries the same elevated envelope, matching the REST/MCP shape exactly.
10+
11+
**What widens**: a `readonly: true` field a handler writes through `ctx.api.object(x).update(y)` when reached via `repo.execute()` now lands instead of being silently stripped, matching REST `/actions` and MCP `run_action`. A repo-wide census (production + test, `examples/` and `apps/` included) found no existing caller of `ObjectRepository.execute()` — every hit in the tree was prose describing the shape, never an invocation — so no shipped write changes behaviour.

content/docs/permissions/system-context.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ a reader tracing where elevation travels needs them.
180180
| # | Site | Package | What it does |
181181
|:--|:---|:---|:---|
182182
| 62 | `objectql/src/engine.ts:3543` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183-
| 63 | `objectql/src/engine.ts:14463` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
183+
| 63 | `objectql/src/engine.ts:14496` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
184184
| 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report |
185185
| 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across |
186186

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// #13866 — Director ruling 决裁批 #24 (2026-09-01), clause 2: give
4+
// `ObjectRepository.execute()` the same elevated `ScopedContext` REST
5+
// `/actions` and MCP `run_action` already supply an action body (#13832),
6+
// so all three `executeAction` dispatch paths behave identically under the
7+
// platform's documented trusted posture and #3914's identity-less shape is
8+
// gone from the third one.
9+
//
10+
// Before this fix, `ObjectRepository.execute()` handed the handler
11+
// `{ ...params, userId, tenantId, roles }` — no `api`, no `executionContext`.
12+
// A handler reaching a sibling write via `ctx.api.object(x).update(y)` (the
13+
// in-process action-composition shape `action-execution.ts` and
14+
// `body-runner.ts` document as this method's own reason to exist) got
15+
// `ctx.api === undefined` and threw, or — for the sandbox's own last-resort
16+
// facade — a context-less repo whose writes ran as a non-system caller, so
17+
// the engine's static `readonly` strip (`!opCtx.context?.isSystem`,
18+
// `validation/rule-validator.ts`) applied to it and NOT to the same write
19+
// made through REST `/actions` or MCP `run_action`. This suite pins the
20+
// fixed shape: `ctx.api` is a real `ScopedContext` bound to
21+
// `{ ...callerContext, isSystem: true }` — the same `sudo()`-shaped
22+
// elevation `buildActionExecutionContext` uses — so a `readonly: true`
23+
// column a handler writes through `ctx.api` now LANDS on this path exactly
24+
// as it already does on the other two.
25+
//
26+
// The census this ruling required (repo + `examples/` + `apps/`, production
27+
// and test) found ZERO existing callers of `ObjectRepository.execute()` —
28+
// every hit was prose describing the shape (`action-execution.ts`,
29+
// `body-runner.ts`, `validate-readonly-action-writes.ts`, this method's own
30+
// call site), never an invocation — so nothing shipped today depends on the
31+
// old, context-less behaviour this suite retires.
32+
33+
import { describe, it, expect } from 'vitest';
34+
import type { ExecutionContext } from '@objectstack/spec/kernel';
35+
import { ObjectQL, ScopedContext } from './engine.js';
36+
37+
function makeDriver() {
38+
const stores = new Map<string, Map<string, any>>();
39+
const storeFor = (o: string) => {
40+
let s = stores.get(o);
41+
if (!s) { s = new Map(); stores.set(o, s); }
42+
return s;
43+
};
44+
const matches = (row: any, where: any): boolean => {
45+
if (!where || typeof where !== 'object') return true;
46+
return Object.entries(where).every(([k, v]: [string, any]) => {
47+
// [check:where-matcher] REFUSE a combinator this fixture does not
48+
// implement, rather than silently reading it as a field name — the
49+
// exact fake-driver idiom `engine-readonly-strip-caller-values.test.ts`
50+
// already carries.
51+
if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`);
52+
return row?.[k] === v;
53+
});
54+
};
55+
let n = 0;
56+
const driver: any = {
57+
name: 'memory', version: '0.0.0', supports: {},
58+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
59+
async find(object: string, ast: any) {
60+
const rows = Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where));
61+
// [check:objectql-double-limit] Apply the caller's bound AFTER the
62+
// filter, by PRESENCE — this fixture is not under test for pagination,
63+
// but a `find` double that silently ignores `limit` is exactly the
64+
// shape that gate exists to catch.
65+
return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
66+
},
67+
async findOne(object: string, ast: any) {
68+
for (const r of storeFor(object).values()) if (matches(r, ast?.where)) return r;
69+
return null;
70+
},
71+
async create(object: string, data: Record<string, unknown>) {
72+
n += 1;
73+
const id = (data.id as string) ?? `r_${n}`;
74+
const row = { ...data, id };
75+
storeFor(object).set(id, row);
76+
return row;
77+
},
78+
async update(object: string, id: string, data: Record<string, unknown>) {
79+
const s = storeFor(object);
80+
const row = { ...s.get(id), ...data, id };
81+
s.set(id, row);
82+
return row;
83+
},
84+
async updateMany() { return 0; },
85+
async delete(object: string, id: string) { return storeFor(object).delete(id); },
86+
async count() { return 0; },
87+
async bulkCreate(object: string, rows: Record<string, unknown>[]) {
88+
return Promise.all(rows.map((r) => this.create(object, r, undefined)));
89+
},
90+
async bulkUpdate() { return []; }, async bulkDelete() {},
91+
async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; },
92+
async commit() {}, async rollback() {},
93+
};
94+
return { driver, storeFor };
95+
}
96+
97+
function makeRig() {
98+
const engine = new ObjectQL({});
99+
const d = makeDriver();
100+
engine.registerDriver(d.driver, true);
101+
engine.registry.registerObject({
102+
name: 'os_repo_execute_probe',
103+
fields: {
104+
title: { type: 'text' },
105+
// Author-declared lock — the exact gate `!opCtx.context?.isSystem`
106+
// guards (`validation/rule-validator.ts`'s `stripReadonlyFields`).
107+
stamped_by: { type: 'text', readonly: true },
108+
},
109+
} as any);
110+
return { engine, storeFor: d.storeFor };
111+
}
112+
113+
describe('ObjectRepository.execute() elevation (#13866, 决裁批 #24)', () => {
114+
it('THE FIX: a readonly-field write through ctx.api LANDS, matching REST/MCP', async () => {
115+
const { engine, storeFor } = makeRig();
116+
await engine.init();
117+
storeFor('os_repo_execute_probe').set('p_1', { id: 'p_1', title: 'A', stamped_by: null });
118+
119+
engine.registerAction('os_repo_execute_probe', 'stamp', async (ctx: any) => {
120+
// The in-process composition shape this method exists for: a handler
121+
// reaching a sibling write via `ctx.api.object(x).update(y)`.
122+
await ctx.api.object('os_repo_execute_probe').update({ id: ctx.id, stamped_by: 'action-body' });
123+
return { ok: true };
124+
});
125+
126+
const callerCtx: ExecutionContext = { userId: 'u_1', tenantId: 't_1' } as any;
127+
const repo = new ScopedContext(callerCtx, engine as any).object('os_repo_execute_probe');
128+
const result = await repo.execute('stamp', { id: 'p_1' });
129+
130+
expect(result).toEqual({ ok: true });
131+
// THE REGRESSION, stated as the value it must NOT be: before the fix
132+
// `ctx.api` was `undefined` (throwing) or a context-less facade whose
133+
// write the static strip silently discarded, leaving `stamped_by: null`.
134+
expect(storeFor('os_repo_execute_probe').get('p_1').stamped_by).toBe('action-body');
135+
});
136+
137+
it('ctx.executionContext carries isSystem: true — the same envelope buildActionExecutionContext builds', async () => {
138+
const { engine } = makeRig();
139+
await engine.init();
140+
let seenExecutionContext: any;
141+
let seenApi: any;
142+
engine.registerAction('os_repo_execute_probe', 'inspect', async (ctx: any) => {
143+
seenExecutionContext = ctx.executionContext;
144+
seenApi = ctx.api;
145+
return { ok: true };
146+
});
147+
148+
const callerCtx: ExecutionContext = { userId: 'u_2', tenantId: 't_2' } as any;
149+
await new ScopedContext(callerCtx, engine as any).object('os_repo_execute_probe').execute('inspect', {});
150+
151+
expect(seenExecutionContext).toMatchObject({ userId: 'u_2', tenantId: 't_2', isSystem: true });
152+
expect(seenApi).toBeInstanceOf(ScopedContext);
153+
});
154+
155+
it('the elevation is ATTRIBUTABLE, not anonymous: userId/tenantId still ride the elevated context', async () => {
156+
// The reason `buildActionExecutionContext` spreads the caller's envelope
157+
// FIRST rather than handing over a bare `{ isSystem: true }` — pinned
158+
// here on the third path exactly as `recomputeSummaries`' `systemCtx`
159+
// pins it on the second.
160+
const { engine } = makeRig();
161+
await engine.init();
162+
let capturedUserId: unknown;
163+
let capturedTenantId: unknown;
164+
engine.registerAction('os_repo_execute_probe', 'capture_identity', async (ctx: any) => {
165+
capturedUserId = ctx.executionContext.userId;
166+
capturedTenantId = ctx.executionContext.tenantId;
167+
return { ok: true };
168+
});
169+
170+
const callerCtx: ExecutionContext = { userId: 'u_3', tenantId: 't_3' } as any;
171+
await new ScopedContext(callerCtx, engine as any).object('os_repo_execute_probe').execute('capture_identity', {});
172+
173+
expect(capturedUserId).toBe('u_3');
174+
expect(capturedTenantId).toBe('t_3');
175+
});
176+
177+
it('an already-elevated caller composing through repo.execute() stays elevated (no regression)', async () => {
178+
const { engine, storeFor } = makeRig();
179+
await engine.init();
180+
storeFor('os_repo_execute_probe').set('p_2', { id: 'p_2', title: 'B', stamped_by: null });
181+
182+
engine.registerAction('os_repo_execute_probe', 'stamp2', async (ctx: any) => {
183+
await ctx.api.object('os_repo_execute_probe').update({ id: ctx.id, stamped_by: 'still-elevated' });
184+
return { ok: true };
185+
});
186+
187+
const systemCtx: ExecutionContext = { isSystem: true } as any;
188+
await new ScopedContext(systemCtx, engine as any).object('os_repo_execute_probe').execute('stamp2', { id: 'p_2' });
189+
190+
expect(storeFor('os_repo_execute_probe').get('p_2').stamped_by).toBe('still-elevated');
191+
});
192+
193+
it('PARITY: the same landed value a non-system caller updating directly with { context: { isSystem: true } } produces', async () => {
194+
// Not a REST/MCP integration test (those live in `packages/runtime`,
195+
// which cannot be imported from here without a circular dependency) —
196+
// this asserts the same OUTCOME that posture produces on the write path
197+
// both those doors and this one now share: a readonly write elevated by
198+
// `isSystem: true` lands, non-elevated does not.
199+
const { engine, storeFor } = makeRig();
200+
await engine.init();
201+
storeFor('os_repo_execute_probe').set('p_3', { id: 'p_3', title: 'C', stamped_by: null });
202+
storeFor('os_repo_execute_probe').set('p_4', { id: 'p_4', title: 'D', stamped_by: null });
203+
204+
// The REST/MCP-equivalent direct write.
205+
await engine.update(
206+
'os_repo_execute_probe',
207+
{ id: 'p_3', stamped_by: 'direct-elevated' },
208+
{ context: { isSystem: true } } as any,
209+
);
210+
211+
// The repo.execute()-mediated write, now under the same posture.
212+
engine.registerAction('os_repo_execute_probe', 'stamp3', async (ctx: any) => {
213+
await ctx.api.object('os_repo_execute_probe').update({ id: ctx.id, stamped_by: 'via-repo-execute' });
214+
});
215+
const callerCtx: ExecutionContext = { userId: 'u_4' } as any;
216+
await new ScopedContext(callerCtx, engine as any).object('os_repo_execute_probe').execute('stamp3', { id: 'p_4' });
217+
218+
expect(storeFor('os_repo_execute_probe').get('p_3').stamped_by).toBe('direct-elevated');
219+
expect(storeFor('os_repo_execute_probe').get('p_4').stamped_by).toBe('via-repo-execute');
220+
});
221+
});

packages/objectql/src/engine.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14072,14 +14072,47 @@ export class ObjectRepository implements IScopedObjectRepository {
1407214072
});
1407314073
}
1407414074

14075-
/** Execute a named action registered on this object */
14075+
/**
14076+
* Execute a named action registered on this object.
14077+
*
14078+
* [#13866, Director ruling 决裁批 #24 2026-09-01] Elevated to the SAME
14079+
* trusted posture REST `/actions` (`domains/actions.ts`) and MCP
14080+
* `run_action` (`action-execution.ts`) already give an action body (#13832,
14081+
* #2849). This is the third `executeAction` caller those two files' own
14082+
* comments name `ScopedRepo.execute()` — until now it handed the body
14083+
* neither `api` nor `executionContext`, the context-less facade #3914
14084+
* argues an action body must never get. A caller reaching another action
14085+
* via `ctx.api.object(x).execute(y)` (an in-process handler composing a
14086+
* sibling handler, `action-execution.ts`'s own description of this path)
14087+
* now dispatches under the same identity its own body runs under.
14088+
*
14089+
* `{ ...this.context, isSystem: true }` is the `sudo()`-shaped elevation
14090+
* `buildActionExecutionContext` and `recomputeSummaries`'s `systemCtx` both
14091+
* use, not a bare `{ isSystem: true }`: spreading the caller's envelope
14092+
* FIRST keeps the resulting write attributable and correctly scoped —
14093+
* `userId` stamps `created_by`/`updated_by`, `tenantId` stamps the org
14094+
* column and drives driver-level tenant isolation, an open `transaction`
14095+
* joins rather than escapes — instead of the unattributable, org-less rows
14096+
* a bare `{ isSystem: true }` would produce.
14097+
*
14098+
* The census behind this change (repo + `examples/` + `apps/`, production
14099+
* and test) found ZERO existing callers of this method anywhere — every
14100+
* `ObjectRepository.execute()` / `ScopedRepo.execute()` hit in the tree was
14101+
* prose describing the shape, never an invocation — so this widens what a
14102+
* FUTURE caller's write is accepted to do (the static `readonly` strip now
14103+
* skips this path exactly as it already skips REST `/actions` and MCP
14104+
* `run_action`) without changing any write anyone ships today.
14105+
*/
1407614106
async execute(actionName: string, params?: any): Promise<any> {
1407714107
if (this.engine.executeAction) {
14108+
const executionContext: ExecutionContext = { ...this.context, isSystem: true };
1407814109
return this.engine.executeAction(this.objectName, actionName, {
1407914110
...params,
1408014111
userId: this.context.userId,
1408114112
tenantId: this.context.tenantId,
1408214113
roles: this.context.positions,
14114+
executionContext,
14115+
api: new ScopedContext(executionContext, this.engine),
1408314116
});
1408414117
}
1408514118
throw new Error(`Actions not supported by engine`);

0 commit comments

Comments
 (0)