Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/action-door-record-load-verdict-consumed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@objectstack/runtime": patch
---

An action whose caller-scope record load was DENIED is now refused at every action door, not at one of the three.

`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.

Both remaining doors now consume the verdict, on both surfaces (the REST `/actions` route and the MCP `run_action` bridge), through one shared refusal:

- **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.
- **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.
- **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.

`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.
8 changes: 7 additions & 1 deletion packages/runtime/src/action-body-identity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ function makeSharingEngine(extra: Record<string, unknown> = {}) {
},
async find(object: string, options?: any) {
gate('find', object, options?.context);
return [];
// [#16370] The by-id pre-load has to be ANSWERED: an action door now
// refuses a row-scoped invocation whose caller-scope subject load did
// not deliver the row, so a rig that answered every read with `[]`
// would collect a 404 before the handler this file is about is ever
// built. Every other read still reads empty — the WRITES above are
// this file's subject, and they are untouched.
return options?.where?.id ? [{ id: options.where.id }] : [];
},
async count(object: string, options?: any) {
gate('count', object, options?.context);
Expand Down
38 changes: 35 additions & 3 deletions packages/runtime/src/action-ctx-user-shape.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,22 @@ async function dispatchRest(ec: any, ql: any, context?: HttpProtocolContext) {
return { response: res.response, actionCtx: ql.executeAction.mock.calls[0]?.[2] };
}

/**
* REST — the same action with NO `recordId`, so no subject load is attempted.
* [#16370] The row-scoped door refuses a load that did not deliver, so a case
* whose subject is something OTHER than the load reaches the handler here.
*/
async function dispatchRestNoRecord(ec: any, ql: any, context?: HttpProtocolContext) {
const kernel: any = {
context: { getService: (n: string) => (n === 'objectql' || n === 'data' ? ql : null) },
};
const ctx = context ?? ({ request: {}, environmentId: 'platform', executionContext: ec } as any);
const res: any = await new HttpDispatcher(kernel).handleActions(
'/crm_case/close_case', 'POST', {}, ctx,
);
return { response: res.response, actionCtx: ql.executeAction.mock.calls[0]?.[2] };
}

/** MCP — `run_action`. Returns the body ctx. */
async function dispatchMcp(ec: any, ql: any) {
const deps: any = { resolveService: async () => null, getObjectQL: async () => ql };
Expand Down Expand Up @@ -247,14 +263,30 @@ describe('#5372 — the FAILURE MODE: an unresolvable name is quiet', () => {
it('an engine with no `find` at all does not break the dispatch', async () => {
const ql = makeQl(DEV_ADMIN);
delete (ql as any).find;
// The record pre-load needs `find` too, so this also proves the name
// resolution is not what turns a degraded engine into a 500.
const { response, actionCtx } = await dispatchRest(makeEc(), ql);
// No `recordId`, so no subject load is attempted and the ONE degraded
// read left is the name resolution — which is this case's subject: an
// unresolvable name falls back to the id and the action still runs.
const { response, actionCtx } = await dispatchRestNoRecord(makeEc(), ql);

expect(response.status).toBe(200);
expect(actionCtx.user.name).toBe('usr_admin');
});

it('[#16370] …and a ROW-SCOPED call on that engine fails CLOSED, not with a 500', async () => {
const ql = makeQl(DEV_ADMIN);
delete (ql as any).find;
// The subject pre-load needs `find` too, so on a degraded engine it
// cannot deliver the row. Since #16370 the door consumes that verdict:
// the refusal is the shared not-found envelope, ⛔ never a 500 and ⛔
// never a dispatch onto a row nobody read. The case above is the firing
// control that says this 404 is the LOAD's, not the name resolution's.
const { response } = await dispatchRest(makeEc(), ql);

expect(response.status).toBe(404);
expect(response.body.error.code).toBe('RECORD_NOT_FOUND');
expect(ql.executeAction).not.toHaveBeenCalled();
});

it('the read is system-elevated — resolving WHO the caller is cannot depend on their own grants', async () => {
const ql = makeQl(DEV_ADMIN);
await dispatchRest(makeEc(), ql);
Expand Down
Loading
Loading