Skip to content

Commit 429ec1e

Browse files
claude[bot]claude
andauthored
fix(runtime): consume the caller-scope load verdict at the flow and script action doors (#16854)
* fix(runtime): consume the caller-scope load verdict at the flow and script action doors `loadActionSubjectRecord` computes one `recordLoadDenied` verdict for every action door, and exactly one door consumed it as a refusal — the declarative update. The flow door and the script/body door spread it into the context as a field and proceeded, so MCP `run_action` on a `type: 'flow'` action answered `ok: true` and started a persisted run for a `recordId` the caller cannot read, and identically for an id that names nothing at all. Both remaining doors now consume the verdict on both surfaces (REST `/actions` and the MCP `run_action` bridge) through one shared refusal, `refuseDeniedSubjectLoad`, placed before the automation run is created and before a trusted body is entered. The envelope is the shared not-found one (`RECORD_NOT_FOUND`, 404), so an unreadable row and an id that names nothing stay one answer. Record-less and new-record actions never attempt a load, so their verdict can never be `true` and their stamp behaviour is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * test(runtime): re-point the action fixtures the door refusal moved Nine assertions across six files dispatched a row-scoped action with a `recordId` against a double that answered every read with `[]`, so the caller-scope subject load never delivered and the door now refuses. Four rigs answer the by-id pre-load instead — their subject is the elevated `ctx.api` binding, the mounted route's auth gate, handler-key addressing and flow dispatch, none of which is the load. Two fixtures pinned the branch this card closes and are re-pinned: a degraded engine with no `find` now fails closed on a row-scoped call (404, never a 500) and still runs the record-less one, and the flow route's "seeds recordId even when the record never loaded" case is split into the refusal it now is plus the new-record invocation it conflated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * test(runtime): bind the two new dispatch responses through a local, so the type-check debt ledger does not grow Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 12babac commit 429ec1e

11 files changed

Lines changed: 849 additions & 96 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
An action whose caller-scope record load was DENIED is now refused at every action door, not at one of the three.
6+
7+
`loadActionSubjectRecord` computes one verdict — `recordLoadDenied` — for every door, and exactly one door consumed it as a refusal: the declarative update. The flow door and the script/body door spread the same verdict into the context as a field and proceeded. So MCP `run_action` on a `type: 'flow'` action answered `ok: true` and started a persisted run for a `recordId` the caller cannot read — and, identically, for an id that names nothing at all — while `get_record` answered "not found" and `update_record` answered "no access" for that same id in the same session. Nothing in the response told the calling agent the row had not been delivered.
8+
9+
Both remaining doors now consume the verdict, on both surfaces (the REST `/actions` route and the MCP `run_action` bridge), through one shared refusal:
10+
11+
- **What is refused.** A row-scoped invocation whose caller-scope load was attempted and did not deliver the row. The refusal lands before the automation run is created and before a trusted, RLS/FLS-bypassing action body is entered — not after, which would answer an error with the run already persisted.
12+
- **The envelope is the shared not-found one**`RECORD_NOT_FOUND`, 404, the same `recordNotFoundError` the read path and the declarative door already answer. Not a 403 and not a new "denied" code: the read path collapses "filtered out by row-level security" and "this id names nothing" on purpose, so answering the two differently would make this door disclose existence where every other door declines to.
13+
- **Record-less and new-record actions are unchanged.** The verdict can only be `true` when a load was actually attempted — a `recordId` was supplied and the action key is not object-less — so an object-less ("global") action and an invocation with no `recordId` never reach the refusal, and both still receive the `recordId` stamp on `ctx.record` exactly as before. The predicate is the load's own verdict, deliberately not the `locations`-derived `requiresRecord` of an action listing, which an author may omit entirely.
14+
15+
`AutomationContext.recordLoadDenied` and the handler-side `ctx.recordLoadDenied` are untouched and still populated by the same producer; an author guard written against either keeps working. What changed is that the platform no longer depends on that guard being written.

packages/runtime/src/action-body-identity.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,13 @@ function makeSharingEngine(extra: Record<string, unknown> = {}) {
5959
},
6060
async find(object: string, options?: any) {
6161
gate('find', object, options?.context);
62-
return [];
62+
// [#16370] The by-id pre-load has to be ANSWERED: an action door now
63+
// refuses a row-scoped invocation whose caller-scope subject load did
64+
// not deliver the row, so a rig that answered every read with `[]`
65+
// would collect a 404 before the handler this file is about is ever
66+
// built. Every other read still reads empty — the WRITES above are
67+
// this file's subject, and they are untouched.
68+
return options?.where?.id ? [{ id: options.where.id }] : [];
6369
},
6470
async count(object: string, options?: any) {
6571
gate('count', object, options?.context);

packages/runtime/src/action-ctx-user-shape.test.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,22 @@ async function dispatchRest(ec: any, ql: any, context?: HttpProtocolContext) {
102102
return { response: res.response, actionCtx: ql.executeAction.mock.calls[0]?.[2] };
103103
}
104104

105+
/**
106+
* REST — the same action with NO `recordId`, so no subject load is attempted.
107+
* [#16370] The row-scoped door refuses a load that did not deliver, so a case
108+
* whose subject is something OTHER than the load reaches the handler here.
109+
*/
110+
async function dispatchRestNoRecord(ec: any, ql: any, context?: HttpProtocolContext) {
111+
const kernel: any = {
112+
context: { getService: (n: string) => (n === 'objectql' || n === 'data' ? ql : null) },
113+
};
114+
const ctx = context ?? ({ request: {}, environmentId: 'platform', executionContext: ec } as any);
115+
const res: any = await new HttpDispatcher(kernel).handleActions(
116+
'/crm_case/close_case', 'POST', {}, ctx,
117+
);
118+
return { response: res.response, actionCtx: ql.executeAction.mock.calls[0]?.[2] };
119+
}
120+
105121
/** MCP — `run_action`. Returns the body ctx. */
106122
async function dispatchMcp(ec: any, ql: any) {
107123
const deps: any = { resolveService: async () => null, getObjectQL: async () => ql };
@@ -247,14 +263,30 @@ describe('#5372 — the FAILURE MODE: an unresolvable name is quiet', () => {
247263
it('an engine with no `find` at all does not break the dispatch', async () => {
248264
const ql = makeQl(DEV_ADMIN);
249265
delete (ql as any).find;
250-
// The record pre-load needs `find` too, so this also proves the name
251-
// resolution is not what turns a degraded engine into a 500.
252-
const { response, actionCtx } = await dispatchRest(makeEc(), ql);
266+
// No `recordId`, so no subject load is attempted and the ONE degraded
267+
// read left is the name resolution — which is this case's subject: an
268+
// unresolvable name falls back to the id and the action still runs.
269+
const { response, actionCtx } = await dispatchRestNoRecord(makeEc(), ql);
253270

254271
expect(response.status).toBe(200);
255272
expect(actionCtx.user.name).toBe('usr_admin');
256273
});
257274

275+
it('[#16370] …and a ROW-SCOPED call on that engine fails CLOSED, not with a 500', async () => {
276+
const ql = makeQl(DEV_ADMIN);
277+
delete (ql as any).find;
278+
// The subject pre-load needs `find` too, so on a degraded engine it
279+
// cannot deliver the row. Since #16370 the door consumes that verdict:
280+
// the refusal is the shared not-found envelope, ⛔ never a 500 and ⛔
281+
// never a dispatch onto a row nobody read. The case above is the firing
282+
// control that says this 404 is the LOAD's, not the name resolution's.
283+
const { response } = await dispatchRest(makeEc(), ql);
284+
285+
expect(response.status).toBe(404);
286+
expect(response.body.error.code).toBe('RECORD_NOT_FOUND');
287+
expect(ql.executeAction).not.toHaveBeenCalled();
288+
});
289+
258290
it('the read is system-elevated — resolving WHO the caller is cannot depend on their own grants', async () => {
259291
const ql = makeQl(DEV_ADMIN);
260292
await dispatchRest(makeEc(), ql);

0 commit comments

Comments
 (0)