Skip to content

Commit 24ac73d

Browse files
claude[bot]claude
andauthored
test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams (#13839)
* test(rest): add the org-scoping history assertions to the read-scope harness Measurement checkpoint: over the CURRENT unfiltered stub the positive case is green (vacuously) and the two cross-tenant controls are red, which is what proves those controls are not writable against a stub that ignores the where. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk * test(rest): the read-scope stub honours the where and the limit on both history seams Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk * chore(gates): ratchet the objectql-double-limit baseline down for the repaired stub Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8c6a7fc commit 24ac73d

2 files changed

Lines changed: 163 additions & 18 deletions

File tree

‎packages/rest/src/rest-server-meta-read-org-scope.test.ts‎

Lines changed: 163 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
6969
/** The value every read assertion looks for. */
7070
const MARKER = 'AUTHORED_AT_RUNTIME';
7171

72+
/** The second revision's marker — two PUTs, two history events. */
73+
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';
74+
7275
/**
7376
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
7477
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
7780
* nothing to do with org scoping. Each shape was measured against the real
7881
* validator, not guessed.
7982
*/
80-
function bodyFor(type: string, name: string): Record<string, unknown> {
81-
const marker = { name, label: MARKER };
83+
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
84+
const marker = { name, label };
8285
switch (type) {
8386
case 'view':
8487
// [#7741] the inline arm requires the object-binding pair.
@@ -114,25 +117,41 @@ interface Row {
114117
state: string; metadata: string; checksum?: string; version?: number;
115118
}
116119

120+
interface HistoryRow {
121+
id: string; type: string; name: string;
122+
organization_id: string | null;
123+
version: number; event_seq: number;
124+
operation_type: string; metadata: string | null;
125+
}
126+
117127
const keyOf = (w: Record<string, unknown>) =>
118128
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;
119129

120-
function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
130+
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
121131
for (const [k, v] of Object.entries(where)) {
122132
if (k === '$or') {
123133
const clauses = v as Array<Record<string, unknown>>;
124134
if (!clauses.some((c) => matchesWhere(r, c))) return false;
125135
continue;
126136
}
137+
// ⛔ REFUSE any other combinator rather than reading it as a field
138+
// name. `$or` is the only one the read paths under test emit, and a
139+
// double that answered `$and` by looking for a column literally called
140+
// `$and` would return a well-formed WRONG answer — the same silent
141+
// class as a double that drops the predicate entirely. Refusing loudly
142+
// is the convention the sibling harness already follows.
143+
if (k.startsWith('$')) {
144+
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
145+
}
127146
if (v === undefined) continue;
128-
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
147+
if (r[k] !== v) return false;
129148
}
130149
return true;
131150
}
132151

133152
function makeStubEngine() {
134153
const rows = new Map<string, Row>();
135-
const historyRows: any[] = [];
154+
const historyRows: HistoryRow[] = [];
136155
let nextId = 0;
137156

138157
const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
@@ -145,25 +164,63 @@ function makeStubEngine() {
145164
const r = rows.get(k);
146165
if (r) return { key: k, row: r };
147166
}
148-
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
167+
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
149168
return null;
150169
};
151170

152171
const engine: any = {
153172
async findOne(table: string, opts: { where: Record<string, unknown> }) {
154173
assertEngineFindOnePredicate(table, opts);
155-
if (table === 'sys_metadata_history') return null;
174+
if (table === 'sys_metadata_history') {
175+
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
176+
// changing it: the unconditional null is NOT load-bearing for
177+
// the PUT path this fixture drives — production reaches this
178+
// seam only from `getByHash`, `restoreVersion` and
179+
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
180+
// write path reads history through `find`
181+
// (`nextEventSeq` / `nextItemVersion`).
182+
return historyRows.find(
183+
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
184+
) ?? null;
185+
}
156186
return findRow(opts.where)?.row ?? null;
157187
},
158-
async find(table: string, opts?: { where?: Record<string, unknown> }) {
159-
if (table === 'sys_metadata_history') return historyRows;
160-
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
188+
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
189+
// [#13764] Two silences repaired on one seam, for one reason.
190+
//
191+
// WHERE: this branch used to `return historyRows` UNFILTERED.
192+
// `SysMetadataRepository.history()` and `diffMetaItem` filter
193+
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
194+
// an unfiltered answer made the org predicate a no-op: an
195+
// org-scoping assertion for `/history` was green whether or not the
196+
// door forwarded the organization. Measured, not argued — the
197+
// `#13764` block at the bottom of this file is green over the old
198+
// stub in BOTH states and reddens over this one when the org is
199+
// dropped.
200+
//
201+
// LIMIT: applied AFTER the filter and BY PRESENCE
202+
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
203+
// than everything, and bounding never decides WHICH rows survive
204+
// the predicate — only how many of the survivors come back. Every
205+
// call this fixture makes passes no bound and is untouched.
206+
// `check:objectql-double-limit`. Applied on BOTH tables so the two
207+
// branches cannot disagree.
208+
if (table === 'sys_metadata_history') {
209+
const matched = historyRows.filter(
210+
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
211+
);
212+
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
213+
}
214+
const matched = Array.from(rows.values()).filter(
215+
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
216+
);
217+
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
161218
},
162219
async insert(table: string, data: Record<string, unknown>) {
163220
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
164221
if (table === 'sys_metadata_history') {
165222
nextId += 1;
166-
historyRows.push({ ...data, id: `h_${nextId}` });
223+
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
167224
return { id: `h_${nextId}` };
168225
}
169226
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
@@ -203,7 +260,7 @@ function makeStubEngine() {
203260
isPackageDisabled: () => false,
204261
},
205262
};
206-
return { engine, rows };
263+
return { engine, rows, historyRows };
207264
}
208265

209266
// ── REST harness: real protocol, real routes, one boot ────────────────────
@@ -236,7 +293,7 @@ function mockRes() {
236293
* can read the same store on the same boot (the cross-tenant control).
237294
*/
238295
function boot() {
239-
const { engine, rows } = makeStubEngine();
296+
const { engine, rows, historyRows } = makeStubEngine();
240297
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
241298
protocol.getDiscovery = async () => ({
242299
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
@@ -269,17 +326,24 @@ function boot() {
269326

270327
return {
271328
rows,
329+
historyRows,
272330
as(tenantId: string | undefined) {
273331
session = tenantId === undefined
274332
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
275333
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
276334
},
277-
put: (type: string, name: string) =>
278-
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
335+
put: (type: string, name: string, label = MARKER) =>
336+
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
279337
get: (type: string, name: string) =>
280338
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
281339
list: (type: string) =>
282340
drive('GET', `${META}/:type`, { params: { type } }),
341+
history: (type: string, name: string) =>
342+
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
343+
/** The fixture proof every history assertion below is gated on. */
344+
historyRowsFor: (type: string, name: string, org: string | null) =>
345+
historyRows.filter((h) => h.type === type && h.name === name
346+
&& (h.organization_id ?? null) === org),
283347
};
284348
}
285349

@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
407471
});
408472
});
409473
});
474+
475+
// ── #13764 — the instrument's own discriminating power, pinned ────────────
476+
//
477+
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
478+
// seams: `findOne` answered `null` unconditionally and `find` handed back every
479+
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
480+
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
481+
// that stub the org predicate was a NO-OP — an org-scoping assertion for
482+
// `/history` was green whether or not the door forwarded the organization.
483+
//
484+
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
485+
// org B`. The positive case below cannot do that job: un-partition the stub
486+
// again and it stays GREEN, because an unfiltered read still contains the rows
487+
// it looks for. Only the cross-tenant case reddens, because only it asks for an
488+
// answer the unfiltered stub cannot give. It is here for the stub, not for the
489+
// door.
490+
//
491+
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
492+
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
493+
// stub is the positive control this repair was calibrated against, and which
494+
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
495+
// asserted here is the narrow fact that THIS harness can now tell a forwarded
496+
// org from a dropped one.
497+
describe('#13764 the history seams of this harness honour the org partition', () => {
498+
let b: ReturnType<typeof boot>;
499+
beforeEach(() => { b = boot(); });
500+
501+
it('serves the org-scoped change log of an item the active org authored', async () => {
502+
// The measurement that names the repair: with the door's org dropped
503+
// this reads the ENV partition and answers zero events. Over the old
504+
// unfiltered stub it answered two in BOTH states.
505+
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
506+
expect(first.status, 'the fixture never wrote').toBe(200);
507+
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);
508+
509+
// Fixture proof first — "the read is org-scoped" is worthless if the
510+
// fixture never created an org-scoped row.
511+
expect(
512+
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
513+
'nothing landed in the org partition; the read below would then pass '
514+
+ 'or fail for a reason unrelated to org scoping',
515+
).toBe(2);
516+
expect(
517+
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
518+
'the write also landed env-wide — the partition is not real',
519+
).toBe(0);
520+
521+
const read = await b.history(CACHED_ARM, 'authored_at_runtime');
522+
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
523+
expect(read.status).toBe(200);
524+
expect(
525+
read.body?.events?.length,
526+
'the door answered an empty change log for an item whose org partition holds two events',
527+
).toBe(2);
528+
});
529+
530+
it('does not serve org A history to org B on the same boot', async () => {
531+
// ⭐ The one that reddens if the stub is ever un-partitioned again.
532+
await b.put(UNCACHED_ARM, 'tenant_bound');
533+
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
534+
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);
535+
536+
b.as(ORG_B);
537+
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
538+
expect(read.status).toBe(200);
539+
expect(
540+
read.body?.events ?? [],
541+
'org B was served org A\'s change log',
542+
).toEqual([]);
543+
});
544+
545+
it('does not serve an org-scoped change log to a caller that named no org', async () => {
546+
await b.put(CACHED_ARM, 'org_a_only');
547+
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);
548+
549+
b.as(undefined);
550+
const read = await b.history(CACHED_ARM, 'org_a_only');
551+
expect(read.status).toBe(200);
552+
expect(
553+
read.body?.events ?? [],
554+
'an org-less caller was served an org-scoped change log',
555+
).toEqual([]);
556+
});
557+
});

‎scripts/objectql-double-limit.baseline.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,6 @@
714714
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
715715
"unjudged": 1
716716
},
717-
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
718-
"blind": 1
719-
},
720717
"packages/rest/src/rest-server-timing.test.ts": {
721718
"unjudged": 1
722719
},

0 commit comments

Comments
 (0)