From 28d51acc376419d6378c829dad2777582512f7bb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 12:22:13 +0000 Subject: [PATCH 1/2] fix(runtime,webhooks): the path object wins on /data/:object/query, and the webhook envelope owns its keys (#3946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 sat in the same block of REST code, so the pattern was swept across all 1313 non-test TypeScript files in packages/. Nine candidates; one real, one worth hardening, seven verified clean (recorded in #3946). runtime `/data` domain: `POST /data/:object/query` built `{ object: objectName, ...body }`, so a body `object` key moved the read to another object than the URL named. Not an authorization bypass — `callData` gates exposure on `params.object`, so the gate followed the body and agreed with the read (the tests pin this) — but the URL stopped describing the operation for audit trails, logs and path-keyed middleware, and the endpoint spoke a second dialect of the contract REST had just standardised on. Its sibling handlers never had it: they nest caller data instead of splatting it. plugin-webhooks: the delivery envelope's own keys were overridable by the event payload. Behaviour-neutral for the engine's publishers (`data.record.*` payloads nest record fields under `after`), but the shape was wrong — envelope keys are written last now. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TzLE9cw4gZKNyPN2ZP4iTt --- .changeset/data-query-path-object.md | 37 +++++++ .../plugin-webhooks/src/auto-enqueuer.ts | 11 ++- .../src/domains/data-path-object.test.ts | 96 +++++++++++++++++++ packages/runtime/src/domains/data.ts | 21 +++- 4 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 .changeset/data-query-path-object.md create mode 100644 packages/runtime/src/domains/data-path-object.test.ts diff --git a/.changeset/data-query-path-object.md b/.changeset/data-query-path-object.md new file mode 100644 index 0000000000..3e0ccaa3f5 --- /dev/null +++ b/.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. diff --git a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts b/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts index 33869b6a8b..e3e64813bc 100644 --- a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts +++ b/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', { diff --git a/packages/runtime/src/domains/data-path-object.test.ts b/packages/runtime/src/domains/data-path-object.test.ts new file mode 100644 index 0000000000..6d0a953e0f --- /dev/null +++ b/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 = {}) { + 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); + }); +}); diff --git a/packages/runtime/src/domains/data.ts b/packages/runtime/src/domains/data.ts index 31be1f804e..cebe9c8730 100644 --- a/packages/runtime/src/domains/data.ts +++ b/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) }; } From c982848863fa2d0b38a71545620f8ed74e6615cd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 12:24:28 +0000 Subject: [PATCH 2/2] docs(webhooks): the delivery envelope wins over the event payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The payload-format section documented the old precedence — "any additional fields the event carried are spread in after these four" — which is exactly the ordering this branch reversed. It also contradicted its own opening sentence ("a small fixed prefix merged on top"): a prefix that anything can overwrite is not fixed. Surfaced by the docs-drift check on #3947. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TzLE9cw4gZKNyPN2ZP4iTt --- content/docs/automation/webhooks.mdx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/content/docs/automation/webhooks.mdx b/content/docs/automation/webhooks.mdx index 6548cde80b..687fc6c287 100644 --- a/content/docs/automation/webhooks.mdx +++ b/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: