diff --git a/packages/rest/src/rest-server-meta-read-org-scope.test.ts b/packages/rest/src/rest-server-meta-read-org-scope.test.ts index 22f0e705ac..a8557f39d8 100644 --- a/packages/rest/src/rest-server-meta-read-org-scope.test.ts +++ b/packages/rest/src/rest-server-meta-read-org-scope.test.ts @@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object'; /** The value every read assertion looks for. */ const MARKER = 'AUTHORED_AT_RUNTIME'; +/** The second revision's marker — two PUTs, two history events. */ +const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2'; + /** * A SPEC-VALID body per type, carrying `label` as the marker the reads assert * on. Real bodies, not `{ label }` stubs: the write door runs full spec @@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME'; * nothing to do with org scoping. Each shape was measured against the real * validator, not guessed. */ -function bodyFor(type: string, name: string): Record { - const marker = { name, label: MARKER }; +function bodyFor(type: string, name: string, label = MARKER): Record { + const marker = { name, label }; switch (type) { case 'view': // [#7741] the inline arm requires the object-binding pair. @@ -114,25 +117,41 @@ interface Row { state: string; metadata: string; checksum?: string; version?: number; } +interface HistoryRow { + id: string; type: string; name: string; + organization_id: string | null; + version: number; event_seq: number; + operation_type: string; metadata: string | null; +} + const keyOf = (w: Record) => `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; -function matchesWhere(r: Row, where: Record): boolean { +function matchesWhere(r: Record, where: Record): boolean { for (const [k, v] of Object.entries(where)) { if (k === '$or') { const clauses = v as Array>; if (!clauses.some((c) => matchesWhere(r, c))) return false; continue; } + // ⛔ REFUSE any other combinator rather than reading it as a field + // name. `$or` is the only one the read paths under test emit, and a + // double that answered `$and` by looking for a column literally called + // `$and` would return a well-formed WRONG answer — the same silent + // class as a double that drops the predicate entirely. Refusing loudly + // is the convention the sibling harness already follows. + if (k.startsWith('$')) { + throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``); + } if (v === undefined) continue; - if ((r as unknown as Record)[k] !== v) return false; + if (r[k] !== v) return false; } return true; } function makeStubEngine() { const rows = new Map(); - const historyRows: any[] = []; + const historyRows: HistoryRow[] = []; let nextId = 0; const findRow = (w: Record): { key: string; row: Row } | null => { @@ -145,25 +164,63 @@ function makeStubEngine() { const r = rows.get(k); if (r) return { key: k, row: r }; } - for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r }; + for (const [k, r] of rows) if (matchesWhere(r as unknown as Record, w)) return { key: k, row: r }; return null; }; const engine: any = { async findOne(table: string, opts: { where: Record }) { assertEngineFindOnePredicate(table, opts); - if (table === 'sys_metadata_history') return null; + if (table === 'sys_metadata_history') { + // [#13764] Was `return null` UNCONDITIONALLY. Measured before + // changing it: the unconditional null is NOT load-bearing for + // the PUT path this fixture drives — production reaches this + // seam only from `getByHash`, `restoreVersion` and + // `resolveMetaItemOrgScope`, none of which a PUT calls; the + // write path reads history through `find` + // (`nextEventSeq` / `nextItemVersion`). + return historyRows.find( + (h) => matchesWhere(h as unknown as Record, opts.where), + ) ?? null; + } return findRow(opts.where)?.row ?? null; }, - async find(table: string, opts?: { where?: Record }) { - if (table === 'sys_metadata_history') return historyRows; - return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {})); + async find(table: string, opts?: { where?: Record; limit?: number }) { + // [#13764] Two silences repaired on one seam, for one reason. + // + // WHERE: this branch used to `return historyRows` UNFILTERED. + // `SysMetadataRepository.history()` and `diffMetaItem` filter + // `organization_id` by STRICT EQUALITY and post-filter nothing, so + // an unfiltered answer made the org predicate a no-op: an + // org-scoping assertion for `/history` was green whether or not the + // door forwarded the organization. Measured, not argued — the + // `#13764` block at the bottom of this file is green over the old + // stub in BOTH states and reddens over this one when the org is + // dropped. + // + // LIMIT: applied AFTER the filter and BY PRESENCE + // (`typeof === 'number'`), so `limit: 0` returns nothing rather + // than everything, and bounding never decides WHICH rows survive + // the predicate — only how many of the survivors come back. Every + // call this fixture makes passes no bound and is untouched. + // `check:objectql-double-limit`. Applied on BOTH tables so the two + // branches cannot disagree. + if (table === 'sys_metadata_history') { + const matched = historyRows.filter( + (h) => matchesWhere(h as unknown as Record, opts?.where ?? {}), + ); + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; + } + const matched = Array.from(rows.values()).filter( + (r) => matchesWhere(r as unknown as Record, opts?.where ?? {}), + ); + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; }, async insert(table: string, data: Record) { if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; if (table === 'sys_metadata_history') { nextId += 1; - historyRows.push({ ...data, id: `h_${nextId}` }); + historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` }); return { id: `h_${nextId}` }; } if (table !== 'sys_metadata') return { id: 'side_effect_skip' }; @@ -203,7 +260,7 @@ function makeStubEngine() { isPackageDisabled: () => false, }, }; - return { engine, rows }; + return { engine, rows, historyRows }; } // ── REST harness: real protocol, real routes, one boot ──────────────────── @@ -236,7 +293,7 @@ function mockRes() { * can read the same store on the same boot (the cross-tenant control). */ function boot() { - const { engine, rows } = makeStubEngine(); + const { engine, rows, historyRows } = makeStubEngine(); const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; protocol.getDiscovery = async () => ({ version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' }, @@ -269,17 +326,24 @@ function boot() { return { rows, + historyRows, as(tenantId: string | undefined) { session = tenantId === undefined ? { userId: 'u1', systemPermissions: ['manage_metadata'] } : { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId }; }, - put: (type: string, name: string) => - drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }), + put: (type: string, name: string, label = MARKER) => + drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }), get: (type: string, name: string) => drive('GET', `${META}/:type/:name`, { params: { type, name } }), list: (type: string) => drive('GET', `${META}/:type`, { params: { type } }), + history: (type: string, name: string) => + drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }), + /** The fixture proof every history assertion below is gated on. */ + historyRowsFor: (type: string, name: string, org: string | null) => + historyRows.filter((h) => h.type === type && h.name === name + && (h.organization_id ?? null) === org), }; } @@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted' }); }); }); + +// ── #13764 — the instrument's own discriminating power, pinned ──────────── +// +// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history` +// seams: `findOne` answered `null` unconditionally and `find` handed back every +// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem` +// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over +// that stub the org predicate was a NO-OP — an org-scoping assertion for +// `/history` was green whether or not the door forwarded the organization. +// +// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to +// org B`. The positive case below cannot do that job: un-partition the stub +// again and it stays GREEN, because an unfiltered read still contains the rows +// it looks for. Only the cross-tenant case reddens, because only it asks for an +// answer the unfiltered stub cannot give. It is here for the stub, not for the +// door. +// +// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in +// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned +// stub is the positive control this repair was calibrated against, and which +// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is +// asserted here is the narrow fact that THIS harness can now tell a forwarded +// org from a dropped one. +describe('#13764 the history seams of this harness honour the org partition', () => { + let b: ReturnType; + beforeEach(() => { b = boot(); }); + + it('serves the org-scoped change log of an item the active org authored', async () => { + // The measurement that names the repair: with the door's org dropped + // this reads the ENV partition and answers zero events. Over the old + // unfiltered stub it answered two in BOTH states. + const first = await b.put(CACHED_ARM, 'authored_at_runtime'); + expect(first.status, 'the fixture never wrote').toBe(200); + await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2); + + // Fixture proof first — "the read is org-scoped" is worthless if the + // fixture never created an org-scoped row. + expect( + b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length, + 'nothing landed in the org partition; the read below would then pass ' + + 'or fail for a reason unrelated to org scoping', + ).toBe(2); + expect( + b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length, + 'the write also landed env-wide — the partition is not real', + ).toBe(0); + + const read = await b.history(CACHED_ARM, 'authored_at_runtime'); + expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined(); + expect(read.status).toBe(200); + expect( + read.body?.events?.length, + 'the door answered an empty change log for an item whose org partition holds two events', + ).toBe(2); + }); + + it('does not serve org A history to org B on the same boot', async () => { + // ⭐ The one that reddens if the stub is ever un-partitioned again. + await b.put(UNCACHED_ARM, 'tenant_bound'); + await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2); + expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2); + + b.as(ORG_B); + const read = await b.history(UNCACHED_ARM, 'tenant_bound'); + expect(read.status).toBe(200); + expect( + read.body?.events ?? [], + 'org B was served org A\'s change log', + ).toEqual([]); + }); + + it('does not serve an org-scoped change log to a caller that named no org', async () => { + await b.put(CACHED_ARM, 'org_a_only'); + expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1); + + b.as(undefined); + const read = await b.history(CACHED_ARM, 'org_a_only'); + expect(read.status).toBe(200); + expect( + read.body?.events ?? [], + 'an org-less caller was served an org-scoped change log', + ).toEqual([]); + }); +}); diff --git a/scripts/objectql-double-limit.baseline.json b/scripts/objectql-double-limit.baseline.json index 6675a77405..7f59dc3c5d 100644 --- a/scripts/objectql-double-limit.baseline.json +++ b/scripts/objectql-double-limit.baseline.json @@ -714,9 +714,6 @@ "packages/rest/src/rest-exec-ctx-principal-kind.test.ts": { "unjudged": 1 }, - "packages/rest/src/rest-server-meta-read-org-scope.test.ts": { - "blind": 1 - }, "packages/rest/src/rest-server-timing.test.ts": { "unjudged": 1 },