diff --git a/.changeset/getmetaitems-org-read-gate.md b/.changeset/getmetaitems-org-read-gate.md new file mode 100644 index 0000000000..0c2f9a210b --- /dev/null +++ b/.changeset/getmetaitems-org-read-gate.md @@ -0,0 +1,102 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): `getMetaItems` applies the registry read gate itself, so a sweep that reads more than one type per request is scoped per type (#14683) + +`getMetaItems` applied no organization gate of its own: whatever `organizationId` +arrived was spent on whatever `type` arrived. The scope of a metadata sweep was +therefore decided per type **by the caller** — which a request carrying one +organization can only get right when it sweeps **one** type. It now resolves +`organizationIdForMetaRead(request.type, request.organizationId)` once, after the +canonical type fold, and both the active-overlay read and the `previewDrafts` +read spend that one resolution. + +Three live callers sweep more than one type and could not have been right: + +- `getMetaDiagnostics` with no `type` — `targetTypes` is the whole registry, the + five `allowOrgOverride: true` types and every other declared type together, + under one request-level organization. +- `findReferencesToMeta` — `request.type` is the **target**; the organization is + spent on `matcher.fromType`, the **sources**, so the target's own registry flag + says nothing about the types actually read. +- the runtime's package export sweep (`assemblePackageManifest`) — every plural + key of `PLURAL_TO_SINGULAR`, with one raw active organization. + +**The harm class is resurrection, not concealment**, and which one it is decides +that the registry-gated predicate is the right instrument. +`SysMetadataRepository.history()` filters `organization_id` by strict equality, +so naming the tenant *there* hides an `allowOrgOverride: false` type's rows. On +this path the two `queryByOrg` reads are UNIONed, so naming it can only **add** — +and what it adds are the pre-#6190 phantoms: org-scoped rows of types with no +per-org read channel, which `loadMetaFromDb` walks past and +`reportUnhydratableOrgScopedRows` exists to warn about. Read back, they surface +in the admin "Used by" panel and the Studio governance directory, inside a +clearance rendered before a destructive action — where a resurrected row is worse +than an omission because it reads as evidence. + +**Why `patch`, from this change's own lineage.** A published `/meta` read door's +row set changing is not a new class here — it is the class this predicate was +born in, and all three landed instances shipped `patch`: + +| commit | what changed | level | +|:--|:--|:--| +| `b6c769019` (#9454 / #9727) | the row set every `/meta` read door returns — org rows **added** | `metadata-core`, `metadata-protocol`, `rest`: all `patch` | +| `26f3588fb` (#10340 / #10519) | which partition two spellings read — rows **moved** | `rest`, `metadata-core`: `patch` | +| `67ceb9aef` (#11553) | the same fold-before-scope repair on the dispatcher door | `runtime`: `patch` | + +The first of those is the commit that introduced `organizationIdForMetaRead` +itself. Adding the org partition to every read door was `patch`; moving which +partition two spellings read was `patch`; this change — withholding the org +partition from types that never had a read channel for it — is the same class, +one verb further in, and takes the same level. + +⛔ Not `minor`, and in this repo that is a statement rather than a rounding +choice. `scripts/check-changeset-no-major.mjs` refuses `major` outright, so +during the launch window a genuinely breaking change ships as `minor` (pre-1.0, +whole-stack lockstep) — #13925 is exactly that, `"@objectstack/core": minor` +carrying a bolded incompatibility banner and an `adr-0087:` marker for a +narrowed published accept set. But the implication runs ONE WAY ONLY, and the +gate's own header is explicit that it does: during the window `minor` is the +union of ordinary new-functionality bumps and banner-marked breaking ones +(`87ad30c10`, `3c1bbd2a8` are new-export `minor`s carrying no banner at all), +so the bump level "tells a consumer nothing about whether the release breaks +them". The carriers of breaking-ness are the bolded banner in the body and the +ADR-0087 disposition — "during the window they are the only signal there is". + +⇒ So `minor` here would not claim an incompatibility; it would claim NOTHING +about compatibility, which is precisely the cost the header names. This change +carries neither carrier because it owes neither — nothing is retired, no accept +set narrows, and `check-adr-0087-registration` reads it as non-breaking. The +level is `patch` because the lineage above is `patch` and no export is added, +not because `patch` rebuts something `minor` would have asserted. + +**Nothing here is incompatible, and the reason is what the withheld rows are.** +They are the #6190 phantoms: org-scoped rows of types with no per-org read +channel. The platform has refused to mint them since `ac244ad09` / `6155c3c24`, +boot hydration skips them, `reportUnhydratableOrgScopedRows` audits them, and +**every REST `/meta` read door has already withheld them since `b6c769019`**. +The only doors still serving them were the dispatcher list +(`runtime/src/domains/meta.ts:921`) and the runtime manifest and publish-flip +reads (`packages.ts:1160`, `:603`) — so this change aligns those three with the +published `/meta` surface rather than departing from it. A consumer reading +those rows was reading through a door inconsistent with `/meta`, on data the +platform had already ruled dead. + +⛔ Not "only a refactor of where the predicate lives" either: the predicate's new +position does change which rows three doors serve. That is why this is a +behaviour entry rather than an internal note — and, per the lineage above, why +the level for it is `patch`. + +**Callers that already gate are unaffected, and that is proved rather than +asserted.** `organizationIdForMetaRead` answers either its argument or +`undefined`, so a second application over the same type is a no-op; the load- +bearing half is that it *is* the same type. The REST `GET /meta/:type` list door +gates on `canonicalMetaUrlType(req.params.type)` and passes the raw segment, which +`canonicalizeMetaRequestType` folds through the identical map — the identical +string. The other four `organizationIdForMetaRead` call sites in `rest-server.ts` +reach `getMetaItemLayered` / `getMetaItem` / `historyMetaItem` / `diffMetaItem` +and never this method. `get-meta-items-org-read-gate.test.ts` §3 measures both +halves over the complete accepted-spelling population (61 spellings, derived from +`META_URL_TO_SINGULAR` unioned with the registry) rather than a hand-listed +sample. diff --git a/packages/metadata-protocol/src/get-meta-items-org-read-gate.test.ts b/packages/metadata-protocol/src/get-meta-items-org-read-gate.test.ts new file mode 100644 index 0000000000..6dad643466 --- /dev/null +++ b/packages/metadata-protocol/src/get-meta-items-org-read-gate.test.ts @@ -0,0 +1,431 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14683] `getMetaItems` applies the registry read gate ITSELF, so a sweep + * that reads MORE THAN ONE type per request is scoped per type instead of per + * request. + * + * ── The defect ──────────────────────────────────────────────────────────── + * + * `getMetaItems` used to apply no gate at all: whatever `organizationId` + * arrived was spent on whatever `type` arrived. The scope of a metadata sweep + * was therefore decided per type BY THE CALLER — which a request carrying ONE + * organization can only get right when it sweeps ONE type. Three live callers + * sweep more: + * + * • `getMetaDiagnostics` with no `type` — `targetTypes` is the whole + * registry, five `allowOrgOverride: true` types and every other declared + * type together, under one request-level organization. + * • `findReferencesToMeta` — `request.type` is the TARGET; the organization + * is spent on `matcher.fromType`, the SOURCES, so the target's own flag + * says nothing about the types actually read. + * • the runtime's package export sweep (`runtime/src/domains/packages.ts`, + * `assemblePackageManifest`) — every plural key of `PLURAL_TO_SINGULAR`, + * one raw active organization. + * + * ── ⭐ The harm class is RESURRECTION, not concealment ──────────────────── + * + * Stated because the sibling card was filed against the opposite premise and a + * fix aimed at concealment would be aiming at the wrong failure. + * `SysMetadataRepository.history()` filters `organization_id` by strict + * equality, so naming the tenant THERE hides an `allowOrgOverride: false` + * type's rows. On THIS path the two `queryByOrg` reads are UNIONed, so naming + * it can only ADD rows — and the rows it adds are the pre-#6190 phantoms: + * org-scoped rows of types with no per-org read channel, which `loadMetaFromDb` + * walks past and `reportUnhydratableOrgScopedRows` exists to warn about. They + * come back inside the admin "Used by" panel and the Studio governance + * directory — a clearance rendered before a destructive action, where a + * resurrected row is worse than an omission because it reads as evidence. + * + * ── ⭐ §3 is the IDEMPOTENCE PROOF the ruling made this change conditional on + * + * Direction A was ruled conditional on demonstrating that moving the predicate + * INSIDE does not CHANGE the scope any already-gating call site receives. §3 + * discharges that mechanically rather than by argument, over the COMPLETE + * population of accepted URL spellings (`META_URL_TO_SINGULAR`, unioned with + * every registry singular) rather than over a hand-listed sample — a new type + * or a changed fold cannot slip past it. + * + * ── Why the observation channel is the WHERE multiset ───────────────────── + * + * §3–§5 seed NO rows and read the `organization_id` partitions the engine was + * asked for. That is the whole of what this change moves — which partitions + * are read — and observing it directly keeps the sweep body-independent, so + * the pin covers every declared type instead of the handful with a + * hand-written schema-valid body. §1/§2 pay for that by asserting real merged + * items on both sides of the gate, so neither half rests on a query nobody + * proved returns rows. + */ + +import { describe, expect, it } from 'vitest'; +import { organizationIdForMetaRead } from '@objectstack/metadata-core'; +import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel'; +import { META_URL_TO_SINGULAR, canonicalMetaUrlType } from '@objectstack/spec/shared'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +const ORG = 'org_acme'; + +interface StoredRow { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; +} + +const storedRow = ( + type: string, + name: string, + extra: Partial = {}, +): StoredRow => ({ + id: `r_${type}_${name}_${extra.organization_id ?? 'env'}_${extra.state ?? 'active'}`, + type, + name, + organization_id: null, + package_id: null, + state: 'active', + metadata: JSON.stringify({ name, label: `${extra.organization_id ?? 'env'} ${name}` }), + ...extra, +}); + +/** + * The engine double: `find` over a row table, plus the registry surface the + * overlay path touches. + * + * ⛔ No `findOne` / `insert` / `update` / `delete`, deliberately — the read + * path under test issues exactly one verb, and a double declaring verbs no + * case exercises would owe `check:engine-double-contract` a dispatch contract + * that protects nothing. Same shape `meta-overlay-cache.test.ts` drives. + */ +function makeHarness(rows: StoredRow[]) { + const finds: Array> = []; + const engine: any = { + async find(table: string, opts?: { where?: Record; limit?: number }) { + if (table !== 'sys_metadata') return []; + const where = opts?.where ?? {}; + finds.push({ ...where }); + // `check:where-matcher` — a hand-written matcher with no combinator + // branch reads `$and` as a field name and answers the wrong + // question rather than failing. Refuse the shape this double does + // not implement, matching the sibling doubles' convention. + for (const k of Object.keys(where)) { + if (k.startsWith('$')) { + throw new Error(`[test double] unsupported WHERE combinator '${k}'`); + } + } + const matched = rows.filter((r) => + Object.entries(where).every(([k, v]) => { + if (v === undefined) return true; + return (r as unknown as Record)[k] === v; + }), + ); + // `check:objectql-double-limit` (#10978) — hold the caller's bound, + // applied AFTER the filter and BY PRESENCE. + return opts?.limit === undefined ? matched : matched.slice(0, opts.limit); + }, + registry: { + registerItem: () => undefined, + registerObject: () => undefined, + listItems: () => [], + getItem: () => undefined, + getObject: () => undefined, + getPackage: () => undefined, + getArtifactItem: () => undefined, + isPackageDisabled: () => false, + applyNavContributions: (app: unknown) => app, + }, + }; + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + return { protocol, finds }; +} + +/** Every `organization_id` partition the engine was asked for, deduplicated. */ +const partitions = (finds: Array>): Array => + [...new Set(finds.map((f) => (f.organization_id ?? null) as string | null))].sort( + (a, b) => String(a).localeCompare(String(b)), + ); + +/** The same, folded per canonical type — for the multi-type sweeps. */ +function partitionsByType(finds: Array>): Map> { + const out = new Map>(); + for (const f of finds) { + const t = canonicalMetaUrlType(String(f.type)); + if (!out.has(t)) out.set(t, new Set()); + out.get(t)!.add((f.organization_id ?? null) as string | null); + } + return out; +} + +const names = (res: any): string[] => + (Array.isArray(res) ? res : res.items).map((i: any) => i.name).sort(); + +const labels = (res: any): Record => + Object.fromEntries( + (Array.isArray(res) ? res : res.items).map((i: any) => [i.name, i.label]), + ); + +/** + * The COMPLETE accepted-spelling population: every URL spelling the `/meta` + * doors fold, unioned with every registry singular. Derived, never listed — + * a newly declared type arrives in this sweep on its own. + */ +const ALL_SPELLINGS: string[] = [ + ...new Set([ + ...Object.keys(META_URL_TO_SINGULAR), + ...DEFAULT_METADATA_TYPE_REGISTRY.map((e) => e.type), + ]), +].sort(); + +/** + * The org-overridable canonical types, derived. Widened to `string[]` + * deliberately: the registry's `type` is a literal union, and every use below + * asks the question of a type read back OUT of a WHERE clause or a fold — + * a plain `string` by construction. Narrowing the probe to the union would + * make `includes` a compile error against exactly the values this file exists + * to judge. + */ +const OVERRIDABLE: readonly string[] = DEFAULT_METADATA_TYPE_REGISTRY + .filter((e) => e.allowOrgOverride) + .map((e) => e.type); + +// ═══════════════════════════════════════════════════════════════════════════ +// §0 — the population this rests on, pinned so a registry change is visible +// ═══════════════════════════════════════════════════════════════════════════ + +describe('§0 the org-overridable set', () => { + it('is exactly the ADR-0005 tier-A five', () => { + expect([...OVERRIDABLE].sort()).toEqual( + ['dashboard', 'email_template', 'report', 'translation', 'view'], + ); + }); + + it('covers every accepted spelling — the sweep below is not a sample', () => { + expect(ALL_SPELLINGS.length).toBeGreaterThan(DEFAULT_METADATA_TYPE_REGISTRY.length); + for (const s of ALL_SPELLINGS) expect(typeof canonicalMetaUrlType(s)).toBe('string'); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §1 — the gate, in BOTH directions, on real merged items +// ═══════════════════════════════════════════════════════════════════════════ + +describe('§1 a raw active organization is gated per type', () => { + it('a NON-overridable type does not resurrect its org-scoped phantom rows', async () => { + const h = makeHarness([ + storedRow('object', 'task'), + storedRow('object', 'task', { organization_id: ORG }), + storedRow('object', 'phantom_only', { organization_id: ORG }), + ]); + + const res = await h.protocol.getMetaItems({ type: 'object', organizationId: ORG }); + + // The org partition is never asked for at all — not asked and + // discarded, which would still pay for the read and still put the row + // one refactor away from the answer. + expect(partitions(h.finds)).toEqual([null]); + expect(names(res)).toEqual(['task']); + expect(labels(res).task).toBe('env task'); + }); + + it('an OVERRIDABLE type still unions both partitions, org winning on collision', async () => { + const h = makeHarness([ + storedRow('view', 'tasks_list'), + storedRow('view', 'tasks_list', { organization_id: ORG }), + storedRow('view', 'org_only', { organization_id: ORG }), + ]); + + const res = await h.protocol.getMetaItems({ type: 'view', organizationId: ORG }); + + expect(partitions(h.finds)).toEqual([null, ORG]); + expect(names(res)).toEqual(['org_only', 'tasks_list']); + expect(labels(res).tasks_list).toBe(`${ORG} tasks_list`); + }); + + it('an org-less caller reads exactly what it read before, either way', async () => { + const h = makeHarness([ + storedRow('view', 'tasks_list'), + storedRow('view', 'org_only', { organization_id: ORG }), + ]); + + const res = await h.protocol.getMetaItems({ type: 'view' }); + + expect(partitions(h.finds)).toEqual([null]); + expect(names(res)).toEqual(['tasks_list']); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §2 — the `previewDrafts` arm spends the SAME resolution +// ═══════════════════════════════════════════════════════════════════════════ + +describe('§2 the draft-preview arm carries the same gate', () => { + it('does not preview an org-scoped DRAFT of a non-overridable type', async () => { + const h = makeHarness([ + storedRow('object', 'task'), + storedRow('object', 'task', { organization_id: ORG, state: 'draft' }), + storedRow('object', 'draft_phantom', { organization_id: ORG, state: 'draft' }), + ]); + + const res = await h.protocol.getMetaItems({ + type: 'object', + organizationId: ORG, + previewDrafts: true, + }); + + expect(partitions(h.finds)).toEqual([null]); + expect(names(res)).toEqual(['task']); + }); + + it('still previews an org-scoped DRAFT of an overridable type', async () => { + const h = makeHarness([ + storedRow('view', 'tasks_list'), + storedRow('view', 'org_draft', { organization_id: ORG, state: 'draft' }), + ]); + + const res = await h.protocol.getMetaItems({ + type: 'view', + organizationId: ORG, + previewDrafts: true, + }); + + expect(partitions(h.finds)).toEqual([null, ORG]); + expect(names(res)).toContain('org_draft'); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §3 — ⭐ THE IDEMPOTENCE PROOF, over the complete spelling population +// ═══════════════════════════════════════════════════════════════════════════ + +describe('§3 an already-gating call site receives unchanged read scope', () => { + /** + * The algebra, stated first because §3's behavioural half only measures + * that the implementation obeys it: `organizationIdForMetaRead` answers + * either its argument or `undefined`, so a second application over the + * SAME type is a no-op. The load-bearing half is "the same type" — which + * §3b measures. + */ + it.each(ALL_SPELLINGS)('f(t, f(t, org)) === f(t, org) for spelling %s', (spelling) => { + const t = canonicalMetaUrlType(spelling); + const once = organizationIdForMetaRead(t, ORG); + expect(organizationIdForMetaRead(t, once)).toBe(once); + }); + + /** + * §3b — the type this method gates on is the type the door gated on. + * + * The REST `GET /meta/:type` list door computes + * `organizationIdForMetaRead(canonicalMetaUrlType(req.params.type), …)` + * and then passes the RAW segment as `type`. Reproduced here for every + * accepted spelling: the partitions read must equal the partitions the + * pre-change implementation read for that same argument — which it spent + * verbatim, so `expected` below IS the old behaviour, written out. + */ + it.each(ALL_SPELLINGS)('gated caller: read scope unchanged for spelling %s', async (spelling) => { + const gated = organizationIdForMetaRead(canonicalMetaUrlType(spelling), ORG); + const h = makeHarness([]); + + await h.protocol.getMetaItems({ + type: spelling, + ...(gated ? { organizationId: gated } : {}), + }); + + // Pre-change semantics, verbatim: `queryByOrg(null)` always, plus + // `queryByOrg(orgId)` iff the caller named one. + const expected: Array = gated ? [null, gated] : [null]; + expect(partitions(h.finds)).toEqual(expected.sort((a, b) => String(a).localeCompare(String(b)))); + }); + + /** + * §3c — the complement: an UNGATED caller IS narrowed, which is the + * repair. Without this the suite would pass on an implementation that + * changed nothing at all. + */ + it.each(ALL_SPELLINGS)('ungated caller: raw org is gated for spelling %s', async (spelling) => { + const canonical = canonicalMetaUrlType(spelling); + const overridable = OVERRIDABLE.includes(canonical); + const h = makeHarness([]); + + await h.protocol.getMetaItems({ type: spelling, organizationId: ORG }); + + expect(partitions(h.finds)).toEqual(overridable ? [null, ORG] : [null]); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §4 — `getMetaDiagnostics`, the whole-registry sweep (door 1 on the card) +// ═══════════════════════════════════════════════════════════════════════════ + +describe('§4 GET /meta/diagnostics with no ?type= is scoped per swept type', () => { + it('names the organization only for the types whose registry entry allows it', async () => { + const h = makeHarness([]); + + await h.protocol.getMetaDiagnostics({ organizationId: ORG }); + + const byType = partitionsByType(h.finds); + expect(byType.size).toBeGreaterThan(1); // it really did sweep more than one type + + for (const [type, parts] of byType) { + expect(parts.has(null)).toBe(true); + expect(parts.has(ORG)).toBe(OVERRIDABLE.includes(type)); + } + // …and the sweep genuinely reached at least one of each kind, so the + // loop above is not vacuously true on a single-kind sweep. + expect([...byType.keys()].some((t) => OVERRIDABLE.includes(t))).toBe(true); + expect([...byType.keys()].some((t) => !OVERRIDABLE.includes(t))).toBe(true); + }); + + it('the ?type= arm is unchanged — one type, one organization, still correct', async () => { + for (const type of ['view', 'object']) { + const h = makeHarness([]); + // What the REST door passes once it gates: the predicate's answer. + const gated = organizationIdForMetaRead(type, ORG); + await h.protocol.getMetaDiagnostics({ type, ...(gated ? { organizationId: gated } : {}) }); + expect(partitions(h.finds)).toEqual(gated ? [null, ORG] : [null]); + } + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §5 — `findReferencesToMeta`, where the org is spent on the SOURCES +// ═══════════════════════════════════════════════════════════════════════════ + +describe('§5 GET /meta/:type/:name/references is scoped per SOURCE type', () => { + it('gates each matcher.fromType on its own flag, not on the target’s', async () => { + const h = makeHarness([]); + + // Target `object` is NON-overridable; its sources include `view`, + // which IS. Gating on the target would have answered env-wide for the + // org-scoped views this panel exists to find. + await h.protocol.findReferencesToMeta({ type: 'object', name: 'task', organizationId: ORG }); + + const byType = partitionsByType(h.finds); + expect(byType.size).toBeGreaterThan(1); + for (const [type, parts] of byType) { + expect(parts.has(null)).toBe(true); + expect(parts.has(ORG)).toBe(OVERRIDABLE.includes(type)); + } + // The load-bearing source: an org-overridable type really is among the + // sources of `object`, so the `true` branch above is exercised. + expect([...byType.keys()].some((t) => OVERRIDABLE.includes(t))).toBe(true); + }); + + it('an org-scoped view referencing the target is FOUND — the false clearance closes', async () => { + const h = makeHarness([ + storedRow('view', 'org_tasks', { + organization_id: ORG, + metadata: JSON.stringify({ name: 'org_tasks', label: 'Org tasks', object: 'task' }), + }), + ]); + + const res = await h.protocol.findReferencesToMeta({ + type: 'object', + name: 'task', + organizationId: ORG, + }); + + expect(res.references.map((r: any) => `${r.type}/${r.name}`)).toContain('view/org_tasks'); + }); +}); diff --git a/packages/metadata-protocol/src/meta-overlay-cache.test.ts b/packages/metadata-protocol/src/meta-overlay-cache.test.ts index 6a539e524e..e12dee1ebc 100644 --- a/packages/metadata-protocol/src/meta-overlay-cache.test.ts +++ b/packages/metadata-protocol/src/meta-overlay-cache.test.ts @@ -530,16 +530,34 @@ describe('[#11967] §7 distinct reads never share an entry', () => { expect((scoped.items as any[]).map((i) => i.name)).toEqual(['beta']); }); + // ⚠️ [#14683] `view`, NOT `object`, and the type is LOAD-BEARING here in a + // way it is not in this section's three siblings. `getMetaItems` now + // resolves its own read scope through `organizationIdForMetaRead`, so a + // type the registry declares NON-overridable has exactly one partition to + // read: an `organizationId` handed in for `object` is gated to `undefined` + // before it ever reaches the cache key, and the two reads below would + // legitimately share one entry — this case would then be asserting that a + // key separation exists where the platform deliberately has none. + // + // The INVARIANT is unchanged and is what this case still pins: two reads + // at genuinely different org scopes must not answer from each other. It + // just has to be exercised on a type that HAS two org scopes, which since + // ADR-0005 tier A means one of `view` / `dashboard` / `report` / + // `translation` / `email_template`. See + // `get-meta-items-org-read-gate.test.ts` for the gate itself and for the + // `object` side of this — that an org-scoped read of a non-overridable + // type reads the env partition alone is pinned there, as behaviour rather + // than as a cache-key fact. it('an org-scoped read does not answer from the env-wide entry', async () => { const h = makeHarness([ - storedRow('object', 'alpha'), - storedRow('object', 'org_only', { organization_id: 'org_1', id: 'r_object_org' }), + storedRow('view', 'alpha'), + storedRow('view', 'org_only', { organization_id: 'org_1', id: 'r_view_org' }), ]); - const envWide = await h.protocol.getMetaItems({ type: 'object' }); + const envWide = await h.protocol.getMetaItems({ type: 'view' }); const findsAfterEnvWide = h.finds.length; - const orgScoped = await h.protocol.getMetaItems({ type: 'object', organizationId: 'org_1' }); + const orgScoped = await h.protocol.getMetaItems({ type: 'view', organizationId: 'org_1' }); expect(h.finds.length).toBeGreaterThan(findsAfterEnvWide); expect((envWide.items as any[]).map((i) => i.name)).toEqual(['alpha']); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index c7ce3813fc..13b37b5d46 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -6768,6 +6768,78 @@ export class ObjectStackProtocolImplementation implements // overlay row shadowed the entire code-authored listing. request = canonicalizeMetaRequestType(request); const { packageId } = request; + // ── [#14683] The registry read gate, resolved ONCE, HERE ────────────────── + // + // {@link organizationIdForMetaRead} — the predicate the REST `/meta` + // read doors have applied since #9454, twin of the write side's + // `organizationIdForMetaWrite` (#6190 / #7018). Until this line + // `getMetaItems` applied NO gate of its own: whatever organization + // arrived was used for whatever type arrived, so the scope of a + // metadata sweep was decided per type, BY THE CALLER — and a request + // carrying ONE `organizationId` can only be correct when it sweeps ONE + // type. Three live callers sweep more than one: `getMetaDiagnostics`' + // untyped arm walks the whole registry; `findReferencesToMeta` spends + // the request's organization on `matcher.fromType` — the SOURCES — + // while `request.type` is the TARGET, so the target's own flag says + // nothing about the types actually read; and the runtime's package + // export sweep (`domains/packages.ts`) walks every plural key with one + // raw active organization. + // + // ⭐ THE HARM IS RESURRECTION, NOT CONCEALMENT, and which one it is + // decides that the registry-gated predicate is the right instrument. + // `SysMetadataRepository.history()` filters `organization_id` by strict + // equality, so naming the tenant THERE hides an `allowOrgOverride: + // false` type's rows. Here the two `queryByOrg` reads are UNIONed, so + // naming it can only ADD — and what it adds are the pre-#6190 + // phantoms: org-scoped rows of types with no per-org read channel, + // which `loadMetaFromDb` walks past and + // {@link reportUnhydratableOrgScopedRows} exists to say out loud. Read + // back, they surface in the admin "Used by" panel and the Studio + // governance directory — inside a clearance shown before a + // destructive action, where a resurrected row is worse than an + // omission because it reads as evidence. + // + // ── Why this is a repair, not a tenancy read-scope change ─────────── + // + // Let `f(t, o) = organizationIdForMetaRead(t, o)`. `f` answers `o` when + // the registry declares `t` per-org overridable and `undefined` + // otherwise, so `f(t, f(t, o)) === f(t, o)` for EVERY `t` and `o`. A + // caller that already gates therefore sees no change, provided it gated + // on the same type this line gates on. Every such caller does: + // + // • `packages/rest`'s `GET /meta/:type` list door — the only door + // that both gates and reaches this method — computes + // `organizationIdForMetaRead(canonicalMetaUrlType(req.params.type), + // ctx?.tenantId)` and then passes `type: req.params.type`, the RAW + // segment. The first statement of this method folds that segment + // through {@link canonicalizeMetaRequestType}, which IS + // `canonicalMetaUrlType` — so `request.type` here is the identical + // STRING the door gated on, and the second application is the + // algebraic no-op above. + // • the search sweep's page read below gates on `'page'` and passes + // `'page'`; `page` is non-overridable, so both readings are + // `undefined` whatever the session holds. + // • the four remaining `organizationIdForMetaRead` call sites in + // `rest-server.ts` (`/layers`, the by-name read, `/history`, + // `/diff`) reach `getMetaItemLayered` / `getMetaItem` / + // `historyMetaItem` / `diffMetaItem` — never this method — so this + // line cannot move them at all. + // + // ⛔ Gate AFTER the fold, never before it. `declaresOrgOverride` + // tolerates the MANIFEST plurals and not the URL-only ones + // (`translations` / `email_templates` have no manifest key), and + // #10340 measured what that costs when the raw segment reaches the + // predicate: one item in two partitions, addressed by spelling. Folding + // happens at the boundary and only there; this line reads what the + // boundary produced. + // + // ⚠️ ONE resolution for BOTH arms, deliberately — the active-overlay + // read below and the `previewDrafts` read further down both spend it. + // A gate threaded into only one arm would leave the draft preview + // resurrecting exactly the phantoms the active list had just stopped + // serving, which is the half-fix shape #9454's own hoist comment + // refuses one package over. + const orgId = organizationIdForMetaRead(request.type, request.organizationId); let items: unknown[] = []; // Unscoped kernels (control plane): read everything from SchemaRegistry. @@ -6808,7 +6880,6 @@ export class ObjectStackProtocolImplementation implements // (when an active org is provided) and env-wide (organization_id IS NULL) // overlays; org-scoped rows win on name collision. try { - const orgId = (request as any).organizationId as string | undefined; const queryByOrg = async (oid: string | null): Promise => { const whereClause: Record = { type: request.type, @@ -7040,7 +7111,6 @@ export class ObjectStackProtocolImplementation implements // process-wide registry or to non-preview reads. if (request.previewDrafts) { try { - const orgId = (request as any).organizationId as string | undefined; const queryDrafts = async (oid: string | null): Promise => { const whereClause: Record = { type: request.type, state: 'draft', organization_id: oid }; if (packageId) whereClause.package_id = packageId; diff --git a/packages/objectql/src/protocol-meta.test.ts b/packages/objectql/src/protocol-meta.test.ts index f15ab827b7..daaa92accf 100644 --- a/packages/objectql/src/protocol-meta.test.ts +++ b/packages/objectql/src/protocol-meta.test.ts @@ -127,24 +127,44 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { expect((result.item as any).label).toBe('Env Default'); }); + // [#14683] Re-spelled from `app` to `view`, the READ-side twin of the + // `[#6190]` re-spelling three cases up — same reason, one verb over. + // `getMetaItems` now resolves its own read scope through + // `organizationIdForMetaRead`, so an `organizationId` handed in for a + // type the registry declares NON-overridable is gated to `undefined` + // and the org partition is never queried. `app` rolled back to + // `allowOrgOverride: false` in #6483, so on `app` this case was + // asserting a union the platform must NOT perform: the org rows it + // seeded are the pre-#6190 phantoms `loadMetaFromDb` walks past, and + // reading them back is the resurrection #14683 closes. + // + // The CLAIM is unchanged and is what this case still pins — env-wide + // and org rows union, org winning on collision. It just has to be + // measured on a type that has an org partition to union. + // + // ⚠️ Its two `getMetaItem` (SINGULAR) siblings above keep `app` + // deliberately: that verb is untouched here. It takes one type per + // request, so a caller CAN be right about its scope, and its REST door + // already gates before calling. Only the LIST verb, which callers + // sweep across many types with one organization, moved. it('getMetaItems unions env-wide and org-specific rows (org wins on collision)', async () => { mockEngine.find.mockImplementation((_table: string, opts: any) => { if (opts?.where?.organization_id === 'org_alpha') { return Promise.resolve([ - { type: 'app', name: 'shared', state: 'active', metadata: JSON.stringify({ name: 'shared', label: 'Org Alpha' }) }, - { type: 'app', name: 'alpha_only', state: 'active', metadata: JSON.stringify({ name: 'alpha_only', label: 'Alpha Only' }) }, + { type: 'view', name: 'shared', state: 'active', metadata: JSON.stringify({ name: 'shared', label: 'Org Alpha' }) }, + { type: 'view', name: 'alpha_only', state: 'active', metadata: JSON.stringify({ name: 'alpha_only', label: 'Alpha Only' }) }, ]); } if (opts?.where?.organization_id === null) { return Promise.resolve([ - { type: 'app', name: 'shared', state: 'active', metadata: JSON.stringify({ name: 'shared', label: 'Env Default' }) }, - { type: 'app', name: 'env_only', state: 'active', metadata: JSON.stringify({ name: 'env_only', label: 'Env Only' }) }, + { type: 'view', name: 'shared', state: 'active', metadata: JSON.stringify({ name: 'shared', label: 'Env Default' }) }, + { type: 'view', name: 'env_only', state: 'active', metadata: JSON.stringify({ name: 'env_only', label: 'Env Only' }) }, ]); } return Promise.resolve([]); }); const result = await protocol.getMetaItems({ - type: 'app', organizationId: 'org_alpha', + type: 'view', organizationId: 'org_alpha', }); const names = (result.items as any[]).map((i) => i.name).sort(); expect(names).toEqual(['alpha_only', 'env_only', 'shared']);