Skip to content

Commit 2ad1eba

Browse files
claude[bot]claude
andauthored
fix(objectql): withhold the org-scope predicate from federated objects (#7738) (#7833)
* fix(objectql): withhold the org-scope predicate from federated objects (#7738) `buildDriverOptions` folded the caller's `ExecutionContext.tenantId` into `DriverOptions.tenantId` for every object, including ADR-0015 federated ones. The SQL driver turns that into the platform's implicit tenant wall — `(organization_id = :tenant OR organization_id IS NULL)` — so an authenticated read of a correctly-bound external object issued that predicate against a remote table the platform does not own. On Postgres/MySQL that is a remote SQL error; on SQLite the quoted-identifier fallback reinterprets the unresolvable identifier as the string literal `'organization_id'`, both disjuncts go constant-false, and the object answers 0 rows with HTTP 200. `tenantId` (and the `group`-posture `tenantIds` union) is now withheld for an object with `external != null`, alongside the existing `tenancy.enabled: false` exemption (ADR-0066 / #3249). Withholding it at the engine covers every driver at the source rather than one driver's opt-out. The exemption is unconditional rather than conditioned on the object carrying an `organization_id` column, because that column is the platform's own: `resolveInjectedSystemColumns` injects it into every registered object with no `external` branch, and `SqlDriver.registerExternalObject` is DDL-free and runs no introspection. On a federated object the column's presence is always the injection and never evidence about the remote schema. The new pin asserts both directions on all four read doors. The load-bearing half is the negative one: an ordinary object still carries the wall for a normal non-system caller, on reads and on the write-side stamp, and a `tenantId` a caller passes by name still wins under both exemptions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Br98nPD61oPAYWBtsGGkAg * test(objectql): type the #7738 pin's engine inputs instead of erasing them Gate hygiene on the new test file only — `engine.ts`, the changeset and all 14 assertions are unchanged, and the suite still passes 14/14. Two repo-wide ratchets were red on 3bdf170, neither of them visible to the per-package `tsc --noEmit` this file was checked with: `check:query-options-erasure` — test surface 242 -> 252. Ten `as any` casts sat in argument positions 1-2 of `find`/`findOne`/`count`/`aggregate`, which is exactly the erasure #4918 bans: `EngineQueryOptions` is not `.strict()`, so an unknown key is silently DROPPED rather than rejected, and `tsc` is the only channel that enforces those keys for an internal caller. Nine are now properly typed — `ExecutionContext` for the caller identity (`accessible_org_ids` is a declared key, so the group-posture contexts needed no cast at all) and the inferred parameter types elsewhere. The tenth is deliberately off-contract: `tenantId` is a RUNTIME passthrough key (`ENGINE_DRIVER_PASSTHROUGH_KEYS`) that `EngineQueryOptionsSchema` does not declare, so it says so with `as unknown as EngineQueryOptions` — the form the rule accepts because it names the contract being bypassed and leaves the rest of the call checked. `check:type-check-debt --re-measure` — `@objectstack/objectql` TEST_DEBT 355 vs 358 measured. The three were not the casts: `registerObject` takes `(schema, packageId, …)` and all three call sites passed one argument (TS2554, the ledger's second-largest class at x93). Fixed by passing the package id rather than by raising the ledger — the ratchet may only shrink. Both re-measured locally on the merged base: erasure 252 -> 242 (at the ceiling), TEST_DEBT 358 -> 355 (equal to recorded, zero errors in this file). The 18 commits merged in from main contributed 0 to either number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Br98nPD61oPAYWBtsGGkAg --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a1686f9 commit 2ad1eba

3 files changed

Lines changed: 380 additions & 12 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): stop injecting the org-scope predicate onto federated (external) objects (#7738)
6+
7+
`ObjectQL.buildDriverOptions` folded the caller's `ExecutionContext.tenantId`
8+
into `DriverOptions.tenantId` for **every** object, including ADR-0015
9+
federated ones. The SQL driver turns that into the platform's implicit tenant
10+
wall — `(organization_id = :tenant OR organization_id IS NULL)` — so an
11+
authenticated read of a correctly-bound external object issued
12+
13+
```sql
14+
select * from `customers` where (`organization_id` = ? or `organization_id` is null)
15+
-- bindings=["org_msoroxgurm6423gz"]
16+
```
17+
18+
against a remote `customers` table whose columns are `id, created_at,
19+
updated_at, name, email, region, lifetime_value`. On Postgres/MySQL that is a
20+
remote SQL error. On SQLite it is worse: the quoted-identifier fallback
21+
reinterprets the unresolvable identifier as the string literal
22+
`'organization_id'`, both disjuncts go constant-false, and the object answers
23+
**0 rows with HTTP 200** — a declared, correctly-bound federated object
24+
silently reads empty, with nothing in the response to say so.
25+
26+
`tenantId` (and the `group`-posture `tenantIds` union) is now withheld for an
27+
object with `external != null`, alongside the existing `tenancy.enabled: false`
28+
exemption (ADR-0066 / #3249). Withholding it at the engine covers every driver
29+
at the source rather than one driver's opt-out.
30+
31+
**Why the exemption is unconditional**, rather than conditioned on whether the
32+
object carries an `organization_id` column: that column is the platform's own.
33+
`applySystemFields` (`resolveInjectedSystemColumns`) injects `organization_id`
34+
into every object it registers and has no `external` branch, and
35+
`SqlDriver.registerExternalObject` is DDL-free by design and runs no
36+
introspection — it computes the tenant column from the platform's field set,
37+
never from the remote's. On a federated object the column's presence is
38+
therefore always the injection and never evidence about the remote schema, so
39+
there is no shape in which scoping by it is known-correct.
40+
41+
**What does not change.** An ordinary object still carries the wall for a
42+
normal non-system caller, on every read door (`find`, `findOne`, `count`,
43+
`aggregate`) and on the write-side stamp; the `group`-posture `tenantIds` union
44+
is still threaded; and a `tenantId` a caller passes **by name** in the option
45+
bag still wins under both exemptions — the exemption governs what the engine
46+
folds in from the execution context, not what a caller asked for explicitly.
47+
Tenant isolation for federated data remains the remote's and the layers above
48+
(RBAC/RLS, the datasource binding).
49+
50+
Note this is the org-scope half only. The boot-ordering defect tracked
51+
separately by #7737 is untouched, and this fix does not depend on it.
Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,287 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// ── The org-scope wall is withheld from FEDERATED objects, and ONLY from them (#7738) ──
4+
//
5+
// `buildDriverOptions` folds the caller's `ExecutionContext.tenantId` into
6+
// `DriverOptions.tenantId` on every read. The SQL driver's `applyTenantScope`
7+
// turns that into `(organization_id = :tenant OR organization_id IS NULL)` —
8+
// the platform's implicit tenant wall. For an ADR-0015 federated object that
9+
// predicate is issued against a table the platform does not own:
10+
//
11+
// select * from `customers` where (`organization_id` = ? or `organization_id` is null)
12+
// -- bindings=["org_msoroxgurm6423gz"]
13+
//
14+
// against a remote `customers` whose columns are `id, created_at, updated_at,
15+
// name, email, region, lifetime_value`. On Postgres/MySQL that is a remote SQL
16+
// error; on SQLite the quoted-identifier fallback reinterprets the unresolvable
17+
// identifier as the string literal `'organization_id'`, both disjuncts go
18+
// constant-false, and a correctly-bound external object answers **0 rows, HTTP
19+
// 200** (#7738, measured on the #7737 lane).
20+
//
21+
// ## Why the platform may not scope a federated object at all
22+
//
23+
// Not "because the showcase table happens to lack the column" — because the
24+
// column it detects is **its own**. `applySystemFields`
25+
// (`resolveInjectedSystemColumns`) injects `organization_id` into EVERY object
26+
// it registers, external ones included: there is no `external` branch in that
27+
// derivation. `SqlDriver.registerExternalObject` is DDL-free by design (ADR-0015
28+
// forbids DDL on a remote schema) and runs no `columnInfo` introspection — it
29+
// computes the tenant column from the PLATFORM's field set. So on a federated
30+
// object `organization_id`'s presence is always the platform's injection and
31+
// never evidence about the remote schema, and scoping by it is a guess about a
32+
// table the platform does not own.
33+
//
34+
// ## This file asserts BOTH directions, and the negative one is load-bearing
35+
//
36+
// Withholding the tenant wall is punching a hole in tenant isolation for one
37+
// class of object. A test that proved only the permissive direction — "the
38+
// external read is no longer scoped" — would stay green if the fix withheld
39+
// `tenantId` from EVERY object, which is how a tenant leak ships green. So
40+
// every case below runs `it.each(READ_DOORS)` over an external object AND an
41+
// ordinary one, and the ordinary object must still carry the wall for a normal
42+
// non-system caller.
43+
//
44+
// ## The seam under test is `DriverOptions`, not SQL
45+
//
46+
// `@objectstack/objectql` cannot import `@objectstack/driver-sql` (the
47+
// dependency runs the other way), so this file pins the exact input the
48+
// driver's wall keys off: `applyTenantScope` early-returns an unmodified
49+
// builder when `options.tenantId` is `undefined | null | ''`, and
50+
// `injectTenantOnInsert` does the same. No `tenantId` in DriverOptions is
51+
// precisely "no `organization_id` predicate in the emitted SQL" — and it is
52+
// withheld at the ENGINE rather than in one driver so every driver is covered
53+
// at the source (the same reason `tenancy.enabled: false` is withheld here,
54+
// #3249).
55+
56+
import { describe, it, expect } from 'vitest';
57+
import type { EngineQueryOptions } from '@objectstack/spec/data';
58+
import type { ExecutionContext } from '@objectstack/spec/kernel';
59+
import { ObjectQL } from './engine.js';
60+
61+
/** A normal, non-system caller with an active org — the #7738 repro identity. */
62+
const MEMBER: ExecutionContext = { userId: 'u_member', tenantId: 'org_msoroxgurm6423gz' };
63+
64+
interface ObservedCall {
65+
object: string;
66+
method: string;
67+
options: Record<string, unknown> | undefined;
68+
}
69+
70+
function makeDriver(name: string, observed: ObservedCall[]) {
71+
const record = (object: string, method: string, options: any) => {
72+
observed.push({ object, method, options });
73+
};
74+
const driver: any = {
75+
name,
76+
version: '0.0.0',
77+
// No `aggregate` capability: the engine falls back to `find` + in-memory
78+
// aggregation, which is itself a read door built from buildDriverOptions.
79+
supports: {},
80+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
81+
async execute() { return null; },
82+
async find(object: string, _ast: any, options: any) { record(object, 'find', options); return []; },
83+
async findOne(object: string, _ast: any, options: any) { record(object, 'findOne', options); return null; },
84+
async count(object: string, _ast: any, options: any) { record(object, 'count', options); return 0; },
85+
async create(object: string, data: any, options: any) { record(object, 'create', options); return { id: 'r_1', ...data }; },
86+
async update(object: string, id: string, data: any, options: any) { record(object, 'update', options); return { id, ...data }; },
87+
async delete() { return true; },
88+
async bulkCreate() { return []; }, async bulkUpdate() { return []; }, async bulkDelete() {},
89+
// DDL-free federated registration — the ADR-0015 seam `syncObjectSchema`
90+
// routes an `external != null` object to. Present so this engine takes the
91+
// real external path rather than the managed one.
92+
registerExternalObject() {},
93+
async syncSchema() {},
94+
};
95+
return driver;
96+
}
97+
98+
/**
99+
* The federated object, declared as `examples/app-showcase` declares it: an
100+
* `external.remoteName` binding, and NO `organization_id` field of its own —
101+
* the one the registry will nevertheless inject.
102+
*/
103+
const EXTERNAL_OBJECT = {
104+
name: 'showcase_ext_customer',
105+
datasource: 'showcase_external',
106+
external: { remoteName: 'customers' },
107+
fields: {
108+
name: { type: 'text' },
109+
email: { type: 'text' },
110+
region: { type: 'text' },
111+
},
112+
} as any;
113+
114+
/** The owning package every object below is registered under (`registerObject` arg 2). */
115+
const PACKAGE_ID = 'com.example.showcase';
116+
117+
/** An ordinary managed object. Its wall must not move. */
118+
const MANAGED_OBJECT = {
119+
name: 'showcase_account',
120+
fields: {
121+
name: { type: 'text' },
122+
region: { type: 'text' },
123+
},
124+
} as any;
125+
126+
async function makeEngine(opts: { posture?: string } = {}) {
127+
const observed: ObservedCall[] = [];
128+
const engine = new ObjectQL();
129+
// Two drivers, as the showcase has: the platform's default, and the remote
130+
// the federated object is bound to by `datasource: 'showcase_external'`. Both
131+
// record into one log, so a read landing on the wrong one is still observed.
132+
engine.registerDriver(makeDriver('memory', observed), true);
133+
engine.registerDriver(makeDriver('showcase_external', observed));
134+
await engine.init();
135+
engine.registry.registerObject(EXTERNAL_OBJECT, PACKAGE_ID);
136+
engine.registry.registerObject(MANAGED_OBJECT, PACKAGE_ID);
137+
if (opts.posture) engine.setTenancyPostureProvider(() => opts.posture);
138+
return { engine, observed };
139+
}
140+
141+
/**
142+
* The premise every assertion below rests on: the registry injects
143+
* `organization_id` into the FEDERATED object too. If this ever stops being
144+
* true the defect changes shape (the driver's implicit detection would no
145+
* longer fire) and the rest of this file would be pinning a fix for a
146+
* mechanism that no longer exists — so it is asserted, not assumed.
147+
*/
148+
describe('#7738 premise — the platform injects its tenant column into a federated object', () => {
149+
it('registers `organization_id` on an external object that declares no such field', async () => {
150+
const { engine } = await makeEngine();
151+
const stored = engine.registry.getObject('showcase_ext_customer') as any;
152+
expect(EXTERNAL_OBJECT.fields.organization_id).toBeUndefined();
153+
expect(stored.fields.organization_id).toBeDefined();
154+
expect(stored.external).toEqual({ remoteName: 'customers' });
155+
});
156+
});
157+
158+
/**
159+
* Every read door that reaches the driver through `buildDriverOptions`, and the
160+
* driver method each one lands on. `aggregate` is included and lands on `find`:
161+
* this driver advertises no native aggregation, so the engine takes its
162+
* in-memory fallback — which still builds DriverOptions and still sends a read
163+
* to the remote.
164+
*/
165+
const READ_DOORS = [
166+
{ name: 'find', driverMethod: 'find', run: (e: ObjectQL, o: string, ctx: ExecutionContext) => e.find(o, {}, { context: ctx }) },
167+
// `findOne` refuses a predicate-free query (#4419), so it carries one. The
168+
// caller's own `where` is orthogonal to the wall this file measures.
169+
{ name: 'findOne', driverMethod: 'findOne', run: (e: ObjectQL, o: string, ctx: ExecutionContext) => e.findOne(o, { where: { region: 'EU' } }, { context: ctx }) },
170+
{ name: 'count', driverMethod: 'count', run: (e: ObjectQL, o: string, ctx: ExecutionContext) => e.count(o, {}, { context: ctx }) },
171+
{ name: 'aggregate', driverMethod: 'find', run: (e: ObjectQL, o: string, ctx: ExecutionContext) => e.aggregate(o, { aggregations: [{ function: 'count', field: 'id', alias: 'n' }] }, { context: ctx }) },
172+
] as const;
173+
174+
describe('#7738 — a federated read carries NO org-scope predicate', () => {
175+
it.each(READ_DOORS)(
176+
'$name: DriverOptions for an external object omit tenantId',
177+
async ({ driverMethod, run }) => {
178+
const { engine, observed } = await makeEngine();
179+
await run(engine, 'showcase_ext_customer', MEMBER);
180+
181+
const call = observed.find((c) => c.object === 'showcase_ext_customer' && c.method === driverMethod);
182+
expect(call, `driver.${driverMethod} was never reached`).toBeDefined();
183+
// `applyTenantScope` early-returns on undefined/null/'' — any of the
184+
// three means no `organization_id` predicate reaches the remote.
185+
expect(call!.options?.tenantId ?? undefined).toBeUndefined();
186+
// The `group`-posture union (`IN (...) OR IS NULL`) is the SAME predicate
187+
// on the same absent column, so it must not survive either.
188+
expect(call!.options?.tenantIds ?? undefined).toBeUndefined();
189+
},
190+
);
191+
192+
it('withholds the tenantIds union too under the `group` posture', async () => {
193+
const { engine, observed } = await makeEngine({ posture: 'group' });
194+
await engine.find(
195+
'showcase_ext_customer',
196+
{},
197+
{ context: { ...MEMBER, accessible_org_ids: ['org_a', 'org_b'] } },
198+
);
199+
const call = observed.find((c) => c.object === 'showcase_ext_customer' && c.method === 'find');
200+
expect(call!.options?.tenantId ?? undefined).toBeUndefined();
201+
expect(call!.options?.tenantIds ?? undefined).toBeUndefined();
202+
});
203+
});
204+
205+
// ── The load-bearing half ────────────────────────────────────────────────────
206+
//
207+
// If the fix over-reaches, THIS is what goes red — not the block above. A
208+
// change to an implicit tenant wall that only tests the permissive direction is
209+
// how a leak ships green, so these cases are the reason this file exists.
210+
describe('#7738 non-regression — an ORDINARY object is still org-scoped', () => {
211+
it.each(READ_DOORS)(
212+
'$name: DriverOptions for a managed object still carry tenantId for a non-system caller',
213+
async ({ driverMethod, run }) => {
214+
const { engine, observed } = await makeEngine();
215+
await run(engine, 'showcase_account', MEMBER);
216+
217+
const call = observed.find((c) => c.object === 'showcase_account' && c.method === driverMethod);
218+
expect(call, `driver.${driverMethod} was never reached`).toBeDefined();
219+
expect(call!.options?.tenantId).toBe('org_msoroxgurm6423gz');
220+
},
221+
);
222+
223+
it('still threads the `group`-posture tenantIds union for a managed object', async () => {
224+
const { engine, observed } = await makeEngine({ posture: 'group' });
225+
await engine.find(
226+
'showcase_account',
227+
{},
228+
{ context: { ...MEMBER, accessible_org_ids: ['org_a', 'org_b'] } },
229+
);
230+
const call = observed.find((c) => c.object === 'showcase_account' && c.method === 'find');
231+
expect(call!.options?.tenantId).toBe('org_msoroxgurm6423gz');
232+
expect(call!.options?.tenantIds).toEqual(['org_a', 'org_b']);
233+
});
234+
235+
it('still stamps the tenant column on an ordinary WRITE', async () => {
236+
// The write half of the same wall (`injectTenantOnInsert`) reads the same
237+
// `DriverOptions.tenantId`. The read-path exemption must not reach it.
238+
const { engine, observed } = await makeEngine();
239+
await engine.insert('showcase_account', { name: 'A-1' }, { context: MEMBER });
240+
const call = observed.find((c) => c.object === 'showcase_account' && c.method === 'create');
241+
expect(call!.options?.tenantId).toBe('org_msoroxgurm6423gz');
242+
});
243+
244+
it('leaves the pre-existing `tenancy.enabled: false` exemption exactly as it was', async () => {
245+
// ADR-0066 / #3249. A second, older reason to withhold the wall — asserted
246+
// here so the federation exemption is proved to be an ADDITION to it and
247+
// not a rewrite of it.
248+
const { engine, observed } = await makeEngine();
249+
engine.registry.registerObject({
250+
name: 'sys_license_probe',
251+
tenancy: { enabled: false },
252+
fields: { name: { type: 'text' } },
253+
} as any, PACKAGE_ID);
254+
await engine.find('sys_license_probe', {}, { context: MEMBER });
255+
const call = observed.find((c) => c.object === 'sys_license_probe' && c.method === 'find');
256+
expect(call!.options?.tenantId ?? undefined).toBeUndefined();
257+
});
258+
});
259+
260+
describe('#7738 — an explicitly-passed tenantId is still deliberate caller intent', () => {
261+
it('does not strip a tenantId the caller passed by name', async () => {
262+
// On `find`/`findOne`/`update`/`delete` the option bag IS the base of the
263+
// driver options (`ENGINE_DRIVER_PASSTHROUGH_KEYS`, #4371), and
264+
// `buildDriverOptions` documents that an explicit `base.tenantId` wins.
265+
//
266+
// The federation exemption governs what the engine FOLDS IN from the
267+
// execution context; it is not a scrubber for what a caller asked for by
268+
// name — a caller who names the column has asserted something about the
269+
// remote that the engine has no standing to contradict. This is also
270+
// exactly how the older `tenancy.enabled: false` exemption behaves, so the
271+
// two stay one shape rather than two.
272+
//
273+
// `tenantId` is a RUNTIME passthrough key, not a declared one:
274+
// `EngineQueryOptionsSchema` does not carry it, so the input is
275+
// deliberately off-contract at the type level and says so with
276+
// `as unknown as` rather than erasing the bag to `any` — that names the
277+
// contract being bypassed and leaves the rest of the call checked (#4918).
278+
const { engine, observed } = await makeEngine();
279+
await engine.find(
280+
'showcase_ext_customer',
281+
{ tenantId: 'org_explicit' } as unknown as EngineQueryOptions,
282+
{ context: MEMBER },
283+
);
284+
const call = observed.find((c) => c.object === 'showcase_ext_customer' && c.method === 'find');
285+
expect(call!.options?.tenantId ?? undefined).toBe('org_explicit');
286+
});
287+
});

0 commit comments

Comments
 (0)