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
46 changes: 46 additions & 0 deletions .changeset/find-data-wire-context.md
Original file line numberDiff line numberDiff line change
@@ -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`.
23 changes: 23 additions & 0 deletions packages/metadata-protocol/src/protocol.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.
Expand Down
93 changes: 93 additions & 0 deletions packages/metadata-protocol/src/protocol.wire-context.test.ts
Original file line numberDiff line numberDiff line change
@@ -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();
});
});
Loading