fix(metadata-protocol): findData must not take its execution context from the request (#3960) (#3961)

From the #3946 sweep's leftover question — whether expand's advanced usage (a
caller-supplied Record<string, QueryAST> whose sub-ASTs carry their own `object`)
is a cross-object read channel. It is NOT: expandRelatedRecords takes its target
from the parent schema, re-enters engine.find so the referenced object's RLS+FLS
both run, $and-merges a nested `where` instead of spreading it over the id
filter, and caps depth.

One layer down is the problem. findData built `{ ...request.query }` then assigned
`context` from `request.context` CONDITIONALLY. `request.query` is the caller's
raw bag (the REST POST /data/:object/query route passes req.body straight in) and
`context` is in the known-params set, so it survived and became the execution
context whenever no server context resolved. plugin-security opens with
`if (opCtx.context?.isSystem) return next()` — the whole RLS/FLS/CRUD chain
skipped — and `__expandRead` collects the #2850 waiver; neither is schema-stripped
on the read path.

enforceAuth kept it unreachable, so this was a fail-open default rather than a
live exploit. Any inbound `context` is now dropped unconditionally: the protocol
owns this invariant and must not delegate it upward.

Author: os-zhuang

.changeset/find-data-wire-context.md

@@ -0,0 +1,46 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(metadata-protocol): findData must not take its execution context from the request (#3960)
+
+Came out of the #3946 sweep's leftover question — whether `expand`'s "advanced
+usage" (a caller-supplied `Record<string, QueryAST>` whose sub-ASTs each carry an
+`object`) is a cross-object read channel. **It is not**, and that needs saying
+because the answer is load-bearing: `expandRelatedRecords` takes its target from
+the parent schema (the expand KEY must be a real `reference` field; the sub-AST's
+`object` is never read), re-enters `engine.find` so the referenced object's RLS +
+FLS both run, `$and`-merges a nested `where` instead of spreading it over the id
+filter, and caps depth. No change needed there.
+
+What the investigation did turn up is one layer down. `findData` built its engine
+options as `{ ...request.query }` and then assigned `context` from
+`request.context` **conditionally**:
+
+- `request.query` is the caller's raw bag on every ingress — the REST
+  `POST /data/:object/query` route passes `req.body` straight in as `query`;
+- `context` sits in the known-params set, so it was not swept into the
+  implicit-filter bucket either — it survived the spread untouched;
+- so when no server context resolved, the caller's `context` *became* the
+  operation's execution context.
+
+Everything hangs off that value. plugin-security's middleware opens with
+`if (opCtx.context?.isSystem) return next()` — the entire RLS / FLS / CRUD chain
+skipped — and `__expandRead: true` collects the #2850 waiver on the object-level
+CRUD gate. Neither is ever schema-stripped on the read path:
+`ExecutionContextSchema.parse` runs only in `engine.createContext`, which reads
+do not use.
+
+Route-level `enforceAuth` is what kept this unreachable: anonymous data requests
+are refused unless a deployment sets `requireAuth: false`. That makes it a
+fail-OPEN default rather than a live exploit — and not something the protocol
+should delegate upward. `findData` now drops any inbound `context`
+unconditionally before the assignment, so the execution context can only come
+from `request.context`.
+
+Verified end-to-end at the protocol layer (a forged
+`{ isSystem, userId, __expandRead }` reached `engine.find` verbatim before, is
+dropped after). The anonymous HTTP reachability half is NOT verified — see #3960
+for exactly what was and was not reproduced. No caller regresses: the only
+in-repo builder of these args (`rest/src/import-runner.ts` `findArgsBase`) passes
+`context` at the top level, never inside `query`.

packages/metadata-protocol/src/protocol.ts

@@ -2659,6 +2659,29 @@ export class ObjectStackProtocolImplementation implements
         // probed for query-shape validity (nor reach the driver as a table).
         this.assertObjectRegistered(request.object);
         const options: any = { ...request.query };
+        // The execution context is SERVER-DERIVED and never caller input.
+        //
+        // `request.query` is the raw request bag on every ingress that reaches
+        // here — the REST `POST /data/:object/query` route hands `req.body`
+        // straight in as `query`. `context` is in the known-params set below, so
+        // it was not swept into the implicit-filter bucket either: a caller's
+        // `context` survived this spread and, because the assignment below is
+        // conditional, became the operation's execution context whenever no
+        // server context resolved (an anonymous request on a deployment that
+        // set `requireAuth: false`).
+        //
+        // What rides on it is total: plugin-security's middleware opens with
+        // `if (opCtx.context?.isSystem) return next()` — the entire RLS / FLS /
+        // CRUD chain skipped — and `__expandRead` waives the object-level CRUD
+        // gate for public objects (#2850). Neither is ever schema-stripped on
+        // this path: `ExecutionContextSchema.parse` runs only in
+        // `engine.createContext`, which the read path does not use.
+        //
+        // Route-level `enforceAuth` is what kept that from being reachable, so
+        // this was a fail-OPEN default one layer down. Drop any inbound
+        // `context` unconditionally: the protocol must not depend on a gate
+        // above it staying switched on.
+        delete options.context;
         // Forward the dispatcher's ExecutionContext so RBAC/RLS middleware
         // can apply per-request enforcement. The protocol layer is purely
         // a normalizer — it must never strip security context.

packages/metadata-protocol/src/protocol.wire-context.test.ts

@@ -0,0 +1,93 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// The execution context must never come off the wire.
+//
+// `findData` built its engine options as `{ ...request.query }` and then
+// overwrote `context` from `request.context` ONLY when that was defined. The
+// query bag is caller-controlled on every ingress that reaches here (the REST
+// `POST /data/:object/query` route hands `req.body` straight in as `query`), and
+// `context` is in the schema's known-params set, so it was NOT swept into the
+// implicit-filter bucket — it survived into `engine.find` as the operation's
+// execution context whenever no server context resolved.
+//
+// What rides on that context is total: plugin-security's middleware opens with
+// `if (opCtx.context?.isSystem) return next()` — the whole RLS/FLS/CRUD chain
+// skipped — and `__expandRead` waives the object-level CRUD gate for public
+// objects. Neither is ever schema-stripped on the read path
+// (`ExecutionContextSchema.parse` runs only in `createContext`).
+//
+// The auth gate is what stands between that and a live exploit: `enforceAuth`
+// refuses anonymous data requests unless a deployment sets `requireAuth: false`.
+// That makes this a fail-OPEN default one layer down — the protocol should not
+// depend on a route-level gate staying on. These tests pin the invariant at the
+// layer that owns it.
+
+import { describe, it, expect, vi } from 'vitest';
+import { ObjectStackProtocolImplementation } from './protocol.js';
+
+const SCHEMA = { name: 'invoice', fields: { title: { name: 'title', type: 'text' } } };
+
+function makeProtocol() {
+    const find = vi.fn(async () => []);
+    const count = vi.fn(async () => 0);
+    const engine = {
+        registry: { getObject: (n: string) => (n === 'invoice' ? SCHEMA : undefined) },
+        find,
+        count,
+    };
+    return { p: new ObjectStackProtocolImplementation(engine as any), find };
+}
+
+/** The engine options `findData` handed to `engine.find`. */
+const optionsFrom = (find: any) => find.mock.calls[0][1];
+
+describe('findData never takes its execution context from the caller', () => {
+    it('drops a body-supplied context when no server context resolved (the anonymous shape)', async () => {
+        const { p, find } = makeProtocol();
+        await p.findData({
+            object: 'invoice',
+            query: { context: { isSystem: true, userId: 'root', __expandRead: true } },
+        } as any);
+
+        expect(find).toHaveBeenCalledTimes(1);
+        expect(optionsFrom(find).context).toBeUndefined();
+    });
+
+    it('the resolved context wins and is not merged with the caller\'s', async () => {
+        const { p, find } = makeProtocol();
+        const resolved = { userId: 'u1', positions: [] };
+        await p.findData({
+            object: 'invoice',
+            query: { context: { isSystem: true } },
+            context: resolved,
+        } as any);
+
+        expect(optionsFrom(find).context).toBe(resolved);
+        expect((optionsFrom(find).context as any).isSystem).toBeUndefined();
+    });
+
+    it('a caller `context` does not become an implicit field filter either', async () => {
+        // It must be dropped, not folded into `where` as `where.context` — that
+        // would turn a forged principal into a query against a column that does
+        // not exist (a confusing 400 instead of a clean ignore).
+        const { p, find } = makeProtocol();
+        await p.findData({ object: 'invoice', query: { context: { isSystem: true } } } as any);
+
+        const opts = optionsFrom(find);
+        expect(opts.where).toBeUndefined();
+        expect(opts.context).toBeUndefined();
+    });
+
+    it('still forwards a real query alongside a forged context', async () => {
+        const { p, find } = makeProtocol();
+        await p.findData({
+            object: 'invoice',
+            query: { where: { title: 'x' }, limit: 5, context: { isSystem: true } },
+        } as any);
+
+        const opts = optionsFrom(find);
+        expect(opts.where).toEqual({ title: 'x' });
+        expect(opts.limit).toBe(5);
+        expect(opts.context).toBeUndefined();
+    });
+});