fix(runtime): a declarative update action refuses a caller with no edit grant 403 before the record lookup, as the data door does

.changeset/30848-declarative-update-refusal-order.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/runtime': patch
+---
+
+fix(runtime): a declarative update action refuses a caller with no edit grant `403 PERMISSION_DENIED`, as the data door does, whether or not the record exists or is visible to them
+
+Clause-②: no
+Metadata-protocol: none
+
+An `operation: 'update'` action (REST `POST /actions/:object/:action/:id`, MCP `run_action`) now judges the caller's update permission on the object before it answers on the record lookup, the order the data door already keeps ("Refusals come before the record lookup"). A caller who may read the object but holds no edit grant on it, writing a record their access hides or an id that names no record, was answered `404 RECORD_NOT_FOUND` by the action and `403 PERMISSION_DENIED` by `PATCH /data` and MCP `update_record` for the same write. Both doors now answer `403 PERMISSION_DENIED`, with the data door's sentence.
+
+A caller who may update the object keeps `404 RECORD_NOT_FOUND`, one answer for a hidden record and a missing id, so neither door tells a caller whether a record it cannot see exists. A record the caller can read but not edit was already answered `403` by both doors and is unchanged.
+
+No request that succeeded is refused, and no shape changes: for a caller who may not update the object, one refusal's code moves from `404 RECORD_NOT_FOUND` to the data door's `403 PERMISSION_DENIED`.

packages/qa/dogfood/test/showcase-anonymous-deny-surfaces.dogfood.test.ts

@@ -581,8 +581,18 @@ describe('showcase: anonymous posture is uniform across surfaces (#2567)', () =>
   // ⛔ The case above is left as it was. Narrowing IT to 404 would make this
   // file's anonymity proof fail for reasons belonging to another proof, which
   // is what its own comment warns against.
-  it('[#7867] a member hitting the action with a NONEXISTENT record id gets 404 RECORD_NOT_FOUND', async () => {
-    const r = await stack.apiAs(memberToken, 'POST', ACTION, { params: {} });
+  //
+  // [#30848] `showcase_mark_done` is a declarative update (`operation:
+  // 'update'`), and a declarative update is judged as the write it performs:
+  // a caller who may not update the object is refused before the record is
+  // looked up, as the data door's PATCH refuses it. The member holds no edit
+  // grant on `showcase_task`, so its answer for the ghost id is that refusal,
+  // 403 PERMISSION_DENIED, and no longer the lookup's 404. #7867's proof, that
+  // a ghost id answers the lookup's 404 rather than a 400 from further down
+  // the pipeline, is carried by a caller who passes that refusal: the dev
+  // administrator, who holds the edit grant.
+  it('[#7867] a caller holding the edit grant hitting the action with a NONEXISTENT record id gets 404 RECORD_NOT_FOUND', async () => {
+    const r = await stack.apiAs(adminToken, 'POST', ACTION, { params: {} });
 
     expect(r.status, 'a ghost record id must answer 404, not a 400 from further down the pipeline').toBe(404);
     const body = (await r.json()) as Record<string, unknown>;
@@ -596,32 +606,52 @@ describe('showcase: anonymous posture is uniform across surfaces (#2567)', () =>
     expect(String(err.message ?? '')).not.toMatch(/HookConditionError/);
   });
 
-  it('[#7867] …the SAME answer the REST data surface gives for that same id', async () => {
+  it('[#30848] a member with no edit grant hitting the action with a NONEXISTENT record id gets 403 PERMISSION_DENIED', async () => {
+    const r = await stack.apiAs(memberToken, 'POST', ACTION, { params: {} });
+
+    expect(r.status, 'a caller who may not update the object is refused the write before the lookup').toBe(403);
+    const body = (await r.json()) as Record<string, unknown>;
+    const err = (isRecord(body.error) ? body.error : {}) as Record<string, unknown>;
+    expect(err.code).toBe('PERMISSION_DENIED');
+    expect(err.httpStatus).toBe(403);
+    // Still never the 400 shapes #7867 removed.
+    expect(String(err.message ?? '')).not.toMatch(/not bound for this operation/);
+    expect(String(err.message ?? '')).not.toMatch(/HookConditionError/);
+  });
+
+  it('[#7867, #30848] …the SAME answer the REST data surface gives that caller for that same id', async () => {
     // The comparison that made #7867 measurable in the first place: one id, one
     // object, one process, one second — and two surfaces that disagreed
     // (`/actions` → 400, `/data` → 404). Both are asserted here, so they cannot
     // drift apart again without this gate saying so.
     //
-    // The data surface that answers this the way the action does is the record
-    // READ: the action loads its record through the read door, and the member
-    // reads `showcase_task`. A PATCH is not that surface for this caller: an
-    // update by id is judged before the record is looked up, as a delete is, and
-    // the member holds no edit grant on `showcase_task` — so it is refused the
-    // write whatever the id (`update-by-id-refusal-order.dogfood.test.ts`).
+    // The data surface that answers this the way the action does is the
+    // record's UPDATE, the write the action performs: an update by id is
+    // judged before the record is looked up, as a delete is
+    // (`update-by-id-refusal-order.dogfood.test.ts`). For the member, who holds
+    // no edit grant on `showcase_task`, both refuse the write whatever the id;
+    // for the administrator, who holds it, both answer the lookup's not-found.
     const ghostId = ACTION.slice(ACTION.lastIndexOf('/') + 1);
-    const rest = await stack.apiAs(memberToken, 'GET', `/data/showcase_task/${ghostId}`);
 
-    expect(rest.status).toBe(404);
-    const body = (await rest.json()) as Record<string, unknown>;
     // `@objectstack/rest` answers the flat envelope; the dispatcher-mounted
     // `/actions` answers its own wrapper. Different shells, one code — each read
     // in its own shape, with no `??` chain across the two (#5632's rule, which
     // this file already enforces for the 401 bodies).
-    expect(body.code).toBe('RECORD_NOT_FOUND');
-
-    const patch = await stack.apiAs(memberToken, 'PATCH', `/data/showcase_task/${ghostId}`, { done: true });
-    expect(patch.status).toBe(403);
-    expect(((await patch.json()) as Record<string, unknown>).code).toBe('PERMISSION_DENIED');
+    const memberPatch = await stack.apiAs(memberToken, 'PATCH', `/data/showcase_task/${ghostId}`, { done: true });
+    expect(memberPatch.status).toBe(403);
+    expect(((await memberPatch.json()) as Record<string, unknown>).code).toBe('PERMISSION_DENIED');
+    const memberAction = await stack.apiAs(memberToken, 'POST', ACTION, { params: {} });
+    expect(memberAction.status).toBe(403);
+    const memberActionBody = (await memberAction.json()) as Record<string, unknown>;
+    expect(((isRecord(memberActionBody.error) ? memberActionBody.error : {}) as Record<string, unknown>).code).toBe('PERMISSION_DENIED');
+
+    const adminPatch = await stack.apiAs(adminToken, 'PATCH', `/data/showcase_task/${ghostId}`, { done: true });
+    expect(adminPatch.status).toBe(404);
+    expect(((await adminPatch.json()) as Record<string, unknown>).code).toBe('RECORD_NOT_FOUND');
+    const adminAction = await stack.apiAs(adminToken, 'POST', ACTION, { params: {} });
+    expect(adminAction.status).toBe(404);
+    const adminActionBody = (await adminAction.json()) as Record<string, unknown>;
+    expect(((isRecord(adminActionBody.error) ? adminActionBody.error : {}) as Record<string, unknown>).code).toBe('RECORD_NOT_FOUND');
   });
 
   // ── /automation (dispatcher-mounted; runtime domains/automation.ts) ─────

packages/runtime/src/action-execution.ts

@@ -24,6 +24,7 @@ import type { ExecutionContext } from '@objectstack/spec/kernel';
 import { isStoredMetadataBodyObject } from '@objectstack/spec/kernel';
 import type { IHttpServer, IObjectQLEngine, InternalFieldEvaluationQuery, QueryEvaluationGate, ServiceSlotContract, ServiceSlotContracts } from '@objectstack/spec/contracts';
 import { standardErrorCodeForHttpStatus, type MetadataProtocol } from '@objectstack/spec/api';
+import { renderOperationMessage } from '@objectstack/spec/system';
 // [#15942 / #16293] The confirmation member's SPELLING is the contract, so it
 // is imported, never hand-spelled: the door that refuses and the client that
 // retries have to agree on it, and two doors each writing their own 'confirm'
@@ -3709,6 +3710,88 @@ export function refuseDeniedSubjectLoad(
     throw recordNotFoundError(objectName, recordId ?? '');
 }
 
+/**
+ * [#30848] The declarative update's object-level update refusal, judged before
+ * its subject-load verdict is: the data door's order, "the update is judged
+ * before the record is looked up" (`http-protocol.mdx`).
+ *
+ * ## What was wrong
+ *
+ * A caller who may read an object but holds no edit grant on it, writing a
+ * row their scope hides (or an id that names nothing), was answered
+ * `404 RECORD_NOT_FOUND` by a declarative `operation: 'update'` action,
+ * through REST `/actions` and MCP `run_action` both, while the data door
+ * answered the same write `403 PERMISSION_DENIED` (`PATCH /data`, MCP
+ * `update_record`). The action door consumed {@link refuseDeniedSubjectLoad}'s
+ * 404 first, so the write, which would have been refused for the object
+ * before any row was read, was never asked. On a row the caller can see, the
+ * write is asked and both doors already answered 403.
+ *
+ * ## The ask, and why it is not a second decision
+ *
+ * Asked only when the subject load was denied: on a delivered row the write
+ * below carries the caller's identity and the data plane answers its own
+ * refusal, the same one this ask stands in for. The question is the security
+ * service's `explain` for `update` on the object, the contract's "would the
+ * middleware allow this operation?" bottom line, computed by the enforcement
+ * walk and not re-derived here from permission sets (the REST import-template
+ * gate asks `create` the same way). ⛔ No row is named: the answer must not
+ * depend on whether the id names a row, or the door becomes an existence
+ * oracle.
+ *
+ * - `allowed` ⇒ no refusal here; the caller who may update the object keeps
+ *   the not-found answer, one answer for a hidden row and a missing id.
+ * - a denial, or an `explain` that throws ⇒ the data door's own refusal:
+ *   `403 PERMISSION_DENIED` with the middleware's user-facing sentence
+ *   (`permission_denied`, the caller's locale, the deployment's override).
+ *   A throw is a denial, never read as a grant.
+ * - no security service, or one without `explain` ⇒ no refusal here: no
+ *   permission set exists to deny with, as the data door. (A system context
+ *   is `explain`'s own bypass, as it is the middleware's: asked, not skipped.)
+ */
+export async function refuseUngrantedDeclarativeUpdate(
+    deps: ActionExecutionDeps,
+    requestContext: HttpProtocolContext | undefined,
+    objectName: string,
+    ec: any,
+    envId?: string,
+): Promise<void> {
+    let security: { explain?: (request: { object: string; operation: string }, context?: any) => Promise<{ allowed?: boolean } | undefined> } | undefined;
+    try {
+        security = await deps.resolveService(requestContext as HttpProtocolContext, 'security', envId);
+    } catch {
+        security = undefined;
+    }
+    if (!security || typeof security.explain !== 'function') return;
+    let allowed: boolean;
+    try {
+        allowed = (await security.explain({ object: objectName, operation: DECLARATIVE_UPDATE_OPERATION }, ec))?.allowed === true;
+    } catch {
+        allowed = false;
+    }
+    if (allowed) return;
+    let translate: ((key: string, locale: string, params?: Record<string, unknown>) => string) | undefined;
+    try {
+        const i18n: { t?: unknown } | null | undefined = await deps.resolveService(requestContext as HttpProtocolContext, 'i18n', envId);
+        if (i18n && typeof i18n.t === 'function') {
+            const t = i18n.t as (key: string, locale: string, params?: Record<string, unknown>) => string;
+            translate = (key, locale, params) => t.call(i18n, key, locale, params);
+        }
+    } catch {
+        translate = undefined;
+    }
+    const message = renderOperationMessage(
+        { messageKey: 'permission_denied' },
+        { locale: typeof ec?.locale === 'string' ? ec.locale : undefined, translate },
+    );
+    throw Object.assign(new Error(message), {
+        name: 'PermissionDeniedError',
+        code: 'PERMISSION_DENIED',
+        status: 403,
+        statusCode: 403,
+    });
+}
+
 /**
  * Contract point 3 as the two action doors' dispatch reads it, for every
  * branch: {@link refuseDeniedSubjectLoad}, except for a same-origin
@@ -4088,6 +4171,11 @@ function declarativeUpdateRefusal(message: string, status: number): Error {
  * inventing a 403 for the pair would answer a question the platform declines to
  * answer everywhere else. See the note on `loadActionSubjectRecord`.
  *
+ * [#30848] Except for a caller who may not update the OBJECT at all: the data
+ * door refuses that write `403` before it looks the record up, so the pair
+ * gets that 403 here too ({@link refuseUngrantedDeclarativeUpdate}), one
+ * answer for both members of the pair, and the pair stays undistinguished.
+ *
  * ## What is NOT checked here
  *
  * `visible` — contract point 6. It is a per-record UI predicate the client
@@ -4166,6 +4254,14 @@ export async function executeDeclarativeUpdateAction(
     // the flow door and the script/body door now call too. Behaviour here is
     // unchanged to the byte — what changed is that this is no longer the only
     // door that reads the verdict, so the rule cannot be inert two doors over.
+    //
+    // [#30848] …but the object-level update refusal comes first, as at the
+    // data door: a caller with no edit grant gets the write's 403 whether or
+    // not the id names a row it can read. Asked only on a denied load: a
+    // delivered row reaches the write below, which answers that 403 itself.
+    if (subject.recordLoadDenied) {
+        await refuseUngrantedDeclarativeUpdate(_deps, wiring.requestContext, objectName, ec, envId);
+    }
     refuseDeniedSubjectLoad(objectName, recordId, subject);
 
     // ── contract point 2: ONE data-plane update, AS THE CALLER ───────────────