diff --git a/packages/core/src/security/admin-standing-surface.test.ts b/packages/core/src/security/admin-standing-surface.test.ts index 547e338c77..6ade76ef49 100644 --- a/packages/core/src/security/admin-standing-surface.test.ts +++ b/packages/core/src/security/admin-standing-surface.test.ts @@ -79,7 +79,7 @@ function makeRecordingQl(tables: Record>>, key in row ? row[key] : row[camelOf(key)]; return { - async find(object: string, opts: { where?: Record } = {}) { + async find(object: string, opts: { where?: Record; limit?: number } = {}) { if (!seen.has(object)) seen.set(object, new Set()); const where = opts?.where ?? {}; for (const key of Object.keys(where)) { @@ -96,7 +96,12 @@ function makeRecordingQl(tables: Record>>, return raw(row, key) === cond; }), ); - return rows.map( + // [#10978] Enforce the caller's bound — presence, not truthiness, so + // `limit: 0` returns nothing rather than everything. Bounding BEFORE the + // Proxy wrap keeps the column-observation ledger honest: a row the real + // read would never have returned must not record column reads either. + const page = typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; + return page.map( (row) => new Proxy(row, { get(target, prop, receiver) { diff --git a/packages/core/src/security/api-key.test.ts b/packages/core/src/security/api-key.test.ts index 0302338595..aa1f1246c5 100644 --- a/packages/core/src/security/api-key.test.ts +++ b/packages/core/src/security/api-key.test.ts @@ -13,13 +13,20 @@ import { effectiveTenancyPosture, } from './api-key.js'; -/** In-memory sys_api_key store exposing the `find` shape the verifier uses. */ +/** + * In-memory sys_api_key store exposing the `find` shape the verifier uses. + * + * [#10978] `limit` is ENFORCED — presence, not truthiness, so `limit: 0` returns + * nothing rather than everything. A double that drops the bound makes any limit + * change on this read green by construction; the verifier reads with `limit: 1`. + */ function makeQl(rows: any[]) { return { find: async (object: string, opts: any) => { if (object !== 'sys_api_key') return []; const where = opts?.where ?? {}; - return rows.filter((r) => Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); return r[k] === v; })); + const matched = rows.filter((r) => Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); return r[k] === v; })); + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; }, }; } diff --git a/packages/core/src/security/resolve-authz-context.test.ts b/packages/core/src/security/resolve-authz-context.test.ts index 36e7889670..3d75075369 100644 --- a/packages/core/src/security/resolve-authz-context.test.ts +++ b/packages/core/src/security/resolve-authz-context.test.ts @@ -13,17 +13,44 @@ import type { AuthzPosture } from '@objectstack/spec/security'; * sys_user_position / sys_position_permission_set / platform_admin / ai_seat). */ -// Minimal in-memory ObjectQL: find(object, { where }) with `===` + `$in` match. +// Minimal in-memory ObjectQL: find(object, { where, limit }) with `===` + `$in` +// match, and the caller's `limit` ENFORCED. +// +// [#10978] The bound is not decoration. A double that matches `where` and hands +// back every row it matched cannot tell a read bounded at 200 from the same read +// bounded at 1000, or from one carrying no bound at all — so raising a limit, +// lowering it, or folding two reads that carry different ones is green BY +// CONSTRUCTION, and the production symptom is a silently truncated result set +// rather than an error. On this file's path that truncation is an authorization +// input: `resolveUserAuthzGrants` reads `sys_member` twice, `{user_id}` at 200 +// and `{organization_id}` at 1000, and the obvious "same object, fold them" +// cleanup silently caps the fellow-org peer list (`org_user_ids`, an RLS input) +// at 200 for any organization with more members. +// +// PRESENCE, not truthiness — `limit: 0` means "return no records" and `0` is +// falsy, so `opts.limit ? …` would answer a request for NOTHING with the WHOLE +// table. That is a measured door in this repo, not a hypothetical: see the +// `query.limit !== undefined` comment in `driver-memory`'s `memory-driver.ts`. +// Bounding AFTER the filter matches the real read path (filter → sort → offset → +// limit); these doubles implement no ordering, and no read on this path asks for +// one. +function bounded(rows: T[], opts: any): T[] { + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; +} + function makeQl(tables: Record) { return { async find(object: string, opts: any) { const rows = tables[object] ?? []; const where = opts?.where ?? {}; - return rows.filter((r) => - Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); - if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(r[k]); - return r[k] === v; - }), + return bounded( + rows.filter((r) => + Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(r[k]); + return r[k] === v; + }), + ), + opts, ); }, }; @@ -123,11 +150,14 @@ function makeCountingQl(tables: Record) { counts[object] = (counts[object] ?? 0) + 1; const rows = tables[object] ?? []; const where = opts?.where ?? {}; - return rows.filter((r) => - Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); - if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(r[k]); - return r[k] === v; - }), + return bounded( + rows.filter((r) => + Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(r[k]); + return r[k] === v; + }), + ), + opts, ); }, }; @@ -1139,3 +1169,58 @@ describe('[#8613] the `active` flag on the grant catalogues (ADR-0049)', () => { expect(grants.positions).not.toContain('contributor'); }); }); + +/** + * [#10978] The instrument's own contract. + * + * Every assertion in this file stands on `makeQl`, and a double that drops + * `opts.limit` cannot fail a limit regression: raising a bound, lowering it, or + * folding two reads that carry different ones all produce identical rows. These + * cases pin the bound itself, so the blindness cannot come back unnoticed — + * without them the `slice` is unverified and deleting it fails nothing. + * + * The measured population when this landed: 49 limit-blind query-honouring + * doubles across 43 files, all 49 reached at runtime and 44 handed a real bound + * (values 1 … 10000). Teaching all 49 to honour it broke 0 of 1062 tests — the + * class was unobservable, not wrong. + */ +describe('the in-memory ObjectQL double honours `limit` (#10978)', () => { + const rows = (n: number, org: string) => + Array.from({ length: n }, (_, i) => ({ user_id: `u${i}`, organization_id: org, role: 'member' })); + + it('bounds a matched read at the caller\'s limit', async () => { + const ql = makeQl({ sys_member: rows(205, 'o1') }); + expect(await ql.find('sys_member', { where: { organization_id: 'o1' }, limit: 200 })).toHaveLength(200); + expect(await ql.find('sys_member', { where: { organization_id: 'o1' } })).toHaveLength(205); + }); + + it('tells two bounds apart on the same read — the fold this card exists for', async () => { + // `resolveUserAuthzGrants` reads sys_member twice: `{user_id}` at 200 and + // `{organization_id}` at 1000. Folding them into one read would cap the + // fellow-org peer list (`org_user_ids`, an RLS input) at 200. Under a + // limit-blind double both bounds return 1005 rows and the fold is invisible. + const ql = makeQl({ sys_member: rows(1005, 'o1') }); + const atOrgBound = await ql.find('sys_member', { where: { organization_id: 'o1' }, limit: 1000 }); + const atUserBound = await ql.find('sys_member', { where: { organization_id: 'o1' }, limit: 200 }); + expect(atOrgBound).toHaveLength(1000); + expect(atUserBound).toHaveLength(200); + expect(atOrgBound.length).not.toBe(atUserBound.length); + }); + + it('reads the bound by PRESENCE, not truthiness — `limit: 0` returns nothing', async () => { + // `0` is falsy, so `opts.limit ? …` answers a request for NOTHING with the + // WHOLE table. Measured door in this repo: `driver-memory` carries the same + // fix as `query.limit !== undefined`. + const ql = makeQl({ sys_member: rows(3, 'o1') }); + expect(await ql.find('sys_member', { where: { organization_id: 'o1' }, limit: 0 })).toHaveLength(0); + }); + + it('applies the bound AFTER the filter, never before it', async () => { + // Bounding first would return rows the `where` excludes — a double that is + // silently WRONG rather than merely unbounded. + const ql = makeQl({ sys_member: [...rows(5, 'other'), ...rows(5, 'o1')] }); + const found = await ql.find('sys_member', { where: { organization_id: 'o1' }, limit: 3 }); + expect(found).toHaveLength(3); + expect(found.every((r: any) => r.organization_id === 'o1')).toBe(true); + }); +}); diff --git a/packages/runtime/src/security/resolve-execution-context.test.ts b/packages/runtime/src/security/resolve-execution-context.test.ts index ccbae72664..3ceba57d7a 100644 --- a/packages/runtime/src/security/resolve-execution-context.test.ts +++ b/packages/runtime/src/security/resolve-execution-context.test.ts @@ -10,18 +10,35 @@ import { hashApiKey } from './api-key.js'; * (sys_member, permission-set link tables, …) resolves to an empty set so the * tests isolate the API-key verify path. */ +/** + * [#10978] Enforce the caller's `limit`, the way a real driver does. + * + * A double that matches `where` and returns every matched row cannot tell a read + * bounded at 200 from the same read bounded at 1000, or from an unbounded one — + * so any limit change is green by construction, and the production symptom is a + * silently truncated result set rather than an error. The reads this file drives + * carry real bounds (1, 10, 100, 200, 500, 1000). + * + * PRESENCE, not truthiness: `limit: 0` means "return no records" and `0` is + * falsy, so `opts.limit ? …` would answer a request for NOTHING with the WHOLE + * table — the door `driver-memory` fixed for itself (`query.limit !== undefined`). + */ +function bounded(rows: T[], opts: any): T[] { + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; +} + function makeQl(apiKeyRows: any[]) { return { async find(object: string, opts: any) { const where = opts?.where ?? {}; if (object !== 'sys_api_key') return []; - return apiKeyRows.filter((row) => { + return bounded(apiKeyRows.filter((row) => { for (const [k, v] of Object.entries(where)) { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); if (row[k] !== v) return false; } return true; - }); + }), opts); }, }; } @@ -192,13 +209,13 @@ describe('resolveExecutionContext — localization (timezone + locale)', () => { async find(object: string, opts: any) { const rows = tables[object] ?? []; const where = opts?.where ?? {}; - return rows.filter((row) => { + return bounded(rows.filter((row) => { for (const [k, v] of Object.entries(where)) { if (v !== null && typeof v === 'object') continue; // skip $in/operators if (row[k] !== v) return false; } return true; - }); + }), opts); }, }; return { @@ -314,7 +331,7 @@ describe('resolveExecutionContext — platform-scoped (null-org) grants (ADR-006 async find(object, opts) { const rows = tables[object] ?? []; const where = opts?.where ?? {}; - return rows.filter((row) => { + return bounded(rows.filter((row) => { for (const [k, v] of Object.entries(where)) { if (v !== null && typeof v === 'object') { if (Array.isArray(v.$in) && !v.$in.includes(row[k])) return false; @@ -323,7 +340,7 @@ describe('resolveExecutionContext — platform-scoped (null-org) grants (ADR-006 if ((v ?? null) !== (row[k] ?? null)) return false; } return true; - }); + }), opts); }, }; } @@ -374,7 +391,7 @@ describe('resolveExecutionContext — posture plumbing (#2947)', () => { async find(object: string, opts: any) { const rows = tables[object] ?? []; const where = opts?.where ?? {}; - return rows.filter((row) => { + return bounded(rows.filter((row) => { for (const [k, v] of Object.entries(where)) { if (v !== null && typeof v === 'object') { if (Array.isArray((v as any).$in) && !(v as any).$in.includes(row[k])) return false; @@ -383,7 +400,7 @@ describe('resolveExecutionContext — posture plumbing (#2947)', () => { if ((v ?? null) !== (row[k] ?? null)) return false; } return true; - }); + }), opts); }, }; } @@ -488,13 +505,13 @@ describe('resolveExecutionContext — ADR-0090 D10 agent principal (OAuth on /mc }; return { async find(object: string, opts: any) { - return (tables[object] ?? []).filter((row) => + return bounded((tables[object] ?? []).filter((row) => Object.entries(opts?.where ?? {}).every(([k, v]) => v !== null && typeof v === 'object' ? (Array.isArray((v as any).$in) ? (v as any).$in.includes(row[k]) : true) : (v ?? null) === (row[k] ?? null), ), - ); + ), opts); }, }; };