fix(runtime,webhooks): the path object wins on /data/:object/query, and the webhook envelope owns its keys (#3946)

.changeset/data-query-path-object.md

@@ -0,0 +1,37 @@
+---
+"@objectstack/runtime": patch
+"@objectstack/plugin-webhooks": patch
+---
+
+fix(runtime,webhooks): the path object wins on /data/:object/query, and the webhook envelope owns its keys (#3946)
+
+Follow-up sweep for the shape behind #3897 and #3933 — a trusted, server-derived
+value written into an object literal with a caller-controlled bag spread OVER
+it. Both of those were in the same block of REST code, so the pattern was swept
+across all 1313 non-test TypeScript files in `packages/`. Nine candidate sites;
+one real, one worth hardening, seven verified clean (recorded in #3946 so the
+next sweep does not re-litigate them).
+
+**`POST /data/:object/query` (runtime dispatcher).** The `/data` domain built
+`{ object: objectName, ...body }`, so `{"object":"other", …}` in the body moved
+the read to a different object than the URL named.
+
+This is NOT an authorization bypass, and the tests pin why: `callData` gates
+API exposure on `params.object`, so the gate followed the body and agreed with
+the read — an object hidden by `apiEnabled: false` was refused either way. What
+broke is that the URL stopped describing the operation (audit trails, logs, and
+anything keyed on the request path saw object A while object B was read), and
+that one endpoint spoke a second dialect of the contract the REST side had just
+standardised on: the path object wins. The other handlers in that file never had
+the problem — they nest caller data (`data: body`, `query: normalized`) instead
+of splatting it, and the GET-by-id branch already allowlists its query params
+against exactly this pollution.
+
+**Webhook delivery envelope.** `auto-enqueuer` built
+`{ object, recordId, action, timestamp, ...payload }`, letting an event payload
+rewrite the envelope a subscriber receives. Behaviour-neutral for the engine's
+own publishers — `data.record.*` payloads are `{ recordId, after, changes }`
+with record fields nested under `after`, so none of those four keys collide
+today — but the shape was wrong, and the `payload.id` fallback right above it
+suggests publishers that flatten record fields do exist. Envelope keys are
+written last now.

content/docs/automation/webhooks.mdx

@@ -304,16 +304,16 @@ same time.
 
 ## 5. Payload format
 
-The POST body is the realtime event payload with a small fixed prefix merged
+The POST body is the realtime event payload with a small fixed envelope merged
 on top:
 
 ```json
 {
+ // ...fields from the originating event payload
  "object": "account",
  "recordId": "acc_123",
  "action": "updated",
  "timestamp": "2024-10-27T00:00:00.000Z"
- // ...remaining fields from the originating event payload
 }
 ```
 
@@ -326,7 +326,11 @@ Notes:
  unchanged from the originating realtime event's `timestamp` field — not
  an epoch-ms number).
 - Any additional fields the event carried (e.g. record snapshot data) are
- spread in after these four.
+ spread in first. These four are written **last**, so the envelope always
+ describes the event even if a publisher's payload happens to carry a field
+ with one of these names (#3946). The platform's own `data.record.*` events
+ nest the record under `after` / `changes`, so nothing collides in practice —
+ the ordering is what guarantees it.
 
 Idempotency and event correlation are carried in **headers**, not the body.
 Every attempt sends:

packages/plugins/plugin-webhooks/src/auto-enqueuer.ts

@@ -333,12 +333,21 @@ export class AutoEnqueuer {
  headers: sub.headers,
  signingSecret: sub.secret,
  timeoutMs: sub.timeoutMs,
+ // [#3946] Envelope keys are written LAST so the event payload
+ // cannot rewrite them. Behaviour-neutral for the engine's own
+ // publishers — `data.record.*` payloads are
+ // `{ recordId, after, changes }`, with record fields nested
+ // under `after`, so none of these four keys collide today. It
+ // is the shape that was wrong: a publisher that flattened
+ // record fields into the payload (the `payload.id` fallback
+ // above suggests some do) would have silently rewritten the
+ // `object` / `action` / `timestamp` a subscriber receives.
  payload: {
+ ...payload,
  object: event.object,
  recordId,
  action,
  timestamp: event.timestamp,
- ...payload,
  },
  }).catch((err) =>
  this.logger.warn?.('[webhook-auto-enqueuer] enqueue failed', {

packages/runtime/src/domains/data-path-object.test.ts

@@ -0,0 +1,96 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// [#3946] `POST /data/:object/query` spread the request body OVER
+// `{ object: objectName }`, so a body `object` key moved the read to a
+// different object than the URL named — the same shape #3933 fixed on the REST
+// bulk routes, found by the follow-up sweep for that pattern.
+//
+// These run through the REAL `callData`, so they pin both halves at once: the
+// object the ADR-0049 exposure gate consults and the object actually queried
+// are the same one, and it is the one in the path.
+
+import { describe, it, expect, vi } from 'vitest';
+import { handleDataRequest } from './data.js';
+
+/** Records what the protocol service was asked for. */
+function setup(objectDefs: Record<string, any> = {}) {
+ const findData = vi.fn(async (req: any) => ({ object: req.object, records: [], total: 0 }));
+ const protocol = { findData };
+ const metadata = { getObject: async (name: string) => objectDefs[name] };
+ const deps: any = {
+ resolveService: async (name: string) => (name === 'protocol' ? protocol : name === 'metadata' ? metadata : null),
+ getService: () => null,
+ getObjectQL: async () => null,
+ getRequestKernelService: async () => null,
+ isMultiTenantHost: () => false,
+ success: (data: any) => ({ status: 200, body: data }),
+ error: (message: string, code = 500) => ({ status: code, body: { error: message } }),
+ routeNotFound: (route: string) => ({ status: 404, body: { route } }),
+ errorFromThrown: (e: any) => ({ status: e?.statusCode ?? e?.status ?? 500, body: { error: e?.message } }),
+ resolveActiveOrganizationId: async () => undefined,
+ announceKernelEvent: async () => {},
+ };
+ const context: any = { dataDriver: undefined, environmentId: undefined, executionContext: { userId: 'u1' } };
+ return { deps, context, findData };
+}
+
+const post = (deps: any, context: any, path: string, body: any) =>
+ handleDataRequest(deps, path, 'POST', body, {}, context);
+
+describe('POST /data/:object/query binds to the object in the path (#3946)', () => {
+ it('a body `object` cannot move the read to another object', async () => {
+ const { deps, context, findData } = setup();
+ const res = await post(deps, context, 'crm_account/query', {
+ object: 'sys_user',
+ where: { name: 'x' },
+ });
+
+ expect(res.handled).toBe(true);
+ expect(findData).toHaveBeenCalledTimes(1);
+ expect(findData.mock.calls[0][0].object).toBe('crm_account');
+ });
+
+ it('the exposure gate and the read agree on the path object', async () => {
+ // `sys_user` is hidden from the API; `crm_account` is not. Pointing the
+ // URL at the exposed object and naming the hidden one in the body must
+ // not read the hidden one — and must not be refused on its behalf
+ // either. Both decisions follow the path.
+ const { deps, context, findData } = setup({
+ crm_account: { apiEnabled: true },
+ sys_user: { apiEnabled: false },
+ });
+
+ const res: any = await post(deps, context, 'crm_account/query', { object: 'sys_user' });
+ expect(res.response.status).toBe(200);
+ expect(findData.mock.calls[0][0].object).toBe('crm_account');
+
+ // Addressed directly, the hidden object is still refused. The gate
+ // THROWS a `{ statusCode }` shape; the dispatcher above turns it into
+ // an envelope (`errorFromThrown`), so assert the throw here.
+ await expect(post(deps, context, 'sys_user/query', {})).rejects.toMatchObject({ statusCode: 404 });
+ expect(findData).toHaveBeenCalledTimes(1); // the refused call never read
+ });
+
+ it('still forwards the rest of the body as the query', async () => {
+ const { deps, context, findData } = setup();
+ await post(deps, context, 'crm_account/query', { where: { status: 'open' }, limit: 5 });
+
+ const req = findData.mock.calls[0][0];
+ expect(req.object).toBe('crm_account');
+ expect(req.query).toMatchObject({ where: { status: 'open' }, limit: 5 });
+ });
+
+ it('still honours an explicit `query` envelope', async () => {
+ const { deps, context, findData } = setup();
+ await post(deps, context, 'crm_account/query', { query: { where: { status: 'open' } } });
+
+ expect(findData.mock.calls[0][0].query).toEqual({ where: { status: 'open' } });
+ });
+
+ it('threads the caller execution context through unchanged', async () => {
+ const { deps, context, findData } = setup();
+ await post(deps, context, 'crm_account/query', { context: { userId: 'root', isSystem: true } });
+
+ expect(findData.mock.calls[0][0].context).toBe(context.executionContext);
+ });
+});

packages/runtime/src/domains/data.ts

@@ -52,8 +52,25 @@ export async function handleDataRequest(deps: DomainHandlerDeps, path: string, m
 
  // POST /data/:object/query
  if (action === 'query' && m === 'POST') {
- // Spec: returns FindDataResponse = { object, records, total?, hasMore? }
- const result = await actionExec.callData(deps, 'query', { object: objectName, ...body }, _context.dataDriver, _context.environmentId, _context.executionContext);
+ // [#3946] The PATH object is written LAST. The body used to be
+ // spread OVER `object: objectName`, so `{"object":"other", …}`
+ // moved the read to a different object than the URL named — the
+ // same shape #3933 fixed on the REST bulk routes, found by the
+ // follow-up sweep.
+ //
+ // Not an authorization bypass here: `callData` gates exposure on
+ // `params.object` (action-execution.ts), so the gate followed the
+ // body and agreed with the read. What broke is that the URL stopped
+ // describing the operation — audit trails, logs and anything keyed
+ // on the request path saw object A while object B was read — and
+ // that one endpoint spoke a second dialect of a contract the REST
+ // side had just standardised (path wins).
+ //
+ // The sibling handlers below never had this: they nest the caller's
+ // data (`data: body`, `query: normalized`) instead of splatting it,
+ // and the GET-by-id branch even allowlists its query params against
+ // exactly this kind of parameter pollution.
+ const result = await actionExec.callData(deps, 'query', { ...body, object: objectName }, _context.dataDriver, _context.environmentId, _context.executionContext);
  return { handled: true, response: deps.success(result) };
  }