From 7f63e54ced4d43681464e6c287de1a0655b543ee Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 17:12:34 +0000 Subject: [PATCH] fix(metadata-protocol): serve runtime-authored view containers to org/environment-scoped reads (#13407) Fixes #13407 hydrateExpandedViewItems's object-derivation now checks the container's own top-level `object` field first (ViewSchema.object), before falling back to list.data.object -> form.data.object -> name. getMetaItems now also expands an aggregated view container inline, into that request's own response only -- never into the shared SchemaRegistry -- so an org-scoped or environment-scoped runtime-authored container reaches its own GET /meta/view?object= read, without touching either isolation gate (applyRegistryWriteThrough's environmentId gate, hydrateOverlayIntoRegistry's organizationId gate). --- ...r-runtime-org-environment-scope-closure.md | 37 ++++ packages/metadata-protocol/src/protocol.ts | 175 ++++++++++++++---- .../view-container-runtime-expansion.test.ts | 142 ++++++++++++++ 3 files changed, 313 insertions(+), 41 deletions(-) create mode 100644 .changeset/view-container-runtime-org-environment-scope-closure.md diff --git a/.changeset/view-container-runtime-org-environment-scope-closure.md b/.changeset/view-container-runtime-org-environment-scope-closure.md new file mode 100644 index 0000000000..6ccf038712 --- /dev/null +++ b/.changeset/view-container-runtime-org-environment-scope-closure.md @@ -0,0 +1,37 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): serve a runtime-authored aggregated view container to org-scoped and environment-scoped `getMetaItems` reads (#13407) + +A `defineView`-shaped container authored **live in the app** (`PUT /api/v1/meta/view/`) +was stored `active` and never reached `getViewsByObject` / `GET /meta/view?object=` — +the third occurrence of one defect, after #7163 and #7736. + +**#7163/#7736 scope-boundary, named:** #7736's `hydrateExpandedViewItems` registry +hydration is reached only for an unscoped (`environmentId === undefined`) kernel writing +an env-wide (`organizationId` unset) row — its own pin never exercised anything else. +Both `applyRegistryWriteThrough` (`environmentId !== undefined` → no write-through) and +`hydrateOverlayIntoRegistry` (any `organizationId` set → refused, ADR-0005 per-org +isolation) gate the SAME registry-mutating mechanism off for the exact case this card +reports: "a signed-in user with an **active org**" on "a live **multi-node EE +deployment**". Both gates are correct and untouched by this fix — the SchemaRegistry +they guard is shared by every org/environment a kernel serves. + +**What changed:** `getMetaItems` now also expands a container it reads **inline, into +that request's own response only** — never into the shared registry — so the response is +correct regardless of org/environment scope, without touching either isolation gate. +Object-name derivation (`hydrateExpandedViewItems`'s fallback chain) now also reads the +container's own top-level `object` field first (`ViewSchema.object`, previously never +consulted), before falling back to `list.data.object → form.data.object → name`. + +The container-enumeration drop in `getMetaItems` is untouched: the raw container is still +never listed, only its expanded ViewItems are now also present — restoring, for the +org/environment-scoped case, the same invariant #7736 already established for the +unscoped/env-wide one. + +**Out of scope, filed separately:** `getViewsByObject()` (`metadata-manager.ts`) reads a +different backing store and is not exercised by the card's own repro or pin; the artifact +loader (`packages/metadata/src/plugin.ts`) carries the identical object-derivation gap. + + diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 03be73cf57..68d810af1b 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -6512,6 +6512,41 @@ export class ObjectStackProtocolImplementation implements ); }); + // [#13407] Expand any aggregated `defineView` container this + // READ just merged in, INLINE into this response's own `items` + // — never into the SchemaRegistry. This is what actually closes + // the card: {@link hydrateExpandedViewItems} below is + // registry-mutating and is therefore gated off for exactly the + // rows the card's own repro used (an org-scoped write, on any + // kernel; ANY write on a real environment-scoped kernel) — see + // that method's "#13407 scope boundary" note for why widening + // those gates was rejected rather than attempted. This pass + // has no such gate to worry about: `overlays` is already this + // request's own correctly org/environment-scoped read (the + // `queryByOrg` merge above), so expanding it into the RETURNED + // list — and only the returned list — cannot leak one + // org/environment's container into another's. + // + // Upserted by name (not appended) so this stays idempotent + // against the registry-hydration path below: on an unscoped, + // env-wide kernel that path may have ALREADY registered the + // same expansion (from this or an earlier write's write- + // through), and re-deriving here from the row this call just + // read is a byte-identical, never-stale restatement of the + // same items — not a duplicate. + if (isView) { + const byName = new Map(); + for (const it of items as any[]) { + if (it && typeof it === 'object' && typeof it.name === 'string') byName.set(it.name, it); + } + for (const { data, packageId: recPkg } of overlays) { + for (const vi of this.expandRuntimeViewContainer(request.type, data, { packageId: recPkg })) { + byName.set(vi.name as string, vi); + } + } + items = Array.from(byName.values()); + } + // Only hydrate the global registry for unscoped (control-plane) // calls — scoped project entries must not leak process-wide. // #4521 — this loop is no longer the ONLY way an overlay reaches @@ -12871,10 +12906,74 @@ export class ObjectStackProtocolImplementation implements } /** - * [#7736] Expand an aggregated `defineView` container that arrived through - * the RUNTIME door into the same independent ViewItems the two SOURCE - * registrars produce — at the one hydration choke point all three runtime - * callers already share. + * [#7736, #13407] Expand an aggregated `defineView` container that arrived + * through the RUNTIME door into the same independent, fully-enriched + * ViewItems the two SOURCE registrars produce — object binding, artifact- + * protection merge and package provenance all applied, but nothing + * REGISTERED. Returns `[]` (never throws) for a non-view type, a body that + * is not a container, or a container with no derivable object binding — + * exactly the "no expansion" outcomes the two callers below already + * decide on independently. + * + * Pure and side-effect-free on purpose: it is shared by two callers with + * DIFFERENT scope rules for what may be registered where — + * {@link hydrateExpandedViewItems} (registry-mutating, org/environment- + * gated) and `getMetaItems`' inline per-request expansion (registry-free, + * ungated) — see the #13407 note on {@link hydrateExpandedViewItems} for + * why the registry path could not simply be widened instead. + * + * ## Object-name derivation (#13407 fix) + * + * The container's OWN top-level `object` field — `ViewSchema.object`, + * documented there as "how a stack-level `views: [...]` entry says which + * object its views belong to; read by `getViewsByObject()` / + * `GET /meta/view?object=`" — is the authorial, explicit signal and is + * consulted FIRST. Before #13407 this walked only `list.data.object` → + * `form.data.object` → the row's own name (mirroring `plugin.ts`) and + * never read `container.object` at all, so a container that set the + * top-level field but not `list.data.object` (or whose save `name` is not + * the object it binds — the two are the same value only by convention) + * expanded under the WRONG key or not at all. The three-deep fallback is + * kept, unchanged, for every container written before this field was + * consulted here. + */ + private expandRuntimeViewContainer( + type: string, + data: unknown, + options: { packageId?: string | null }, + ): Record[] { + if ((PLURAL_TO_SINGULAR[type] ?? type) !== 'view') return []; + if (!isAggregatedViewContainer(data)) return []; + const container = data as Record; + const viewObject = + (typeof container.object === 'string' && container.object ? container.object : undefined) + ?? container?.list?.data?.object + ?? container?.form?.data?.object + ?? (typeof container.name === 'string' ? container.name : undefined); + if (!viewObject) return []; + const out: Record[] = []; + for (const vi of expandViewContainer(viewObject, container)) { + // Carry the container's package provenance onto each expanded item + // so the package-disable filter and ADR-0048 artifact scoping judge + // them by the same owner the container has. + const item: Record = { ...(vi as any) }; + if (container._packageId !== undefined && item._packageId === undefined) { + item._packageId = container._packageId; + } + const viArtifact = this.lookupArtifactItem( + type, + vi.name, + (item._packageId as string | undefined) ?? options.packageId ?? undefined, + ); + out.push(mergeArtifactProtection(item, viArtifact) as Record); + } + return out; + } + + /** + * [#7736] Register an aggregated `defineView` container's expansion + * ({@link expandRuntimeViewContainer}) into the SchemaRegistry — at the + * one hydration choke point all three runtime callers already share. * * "Object has-many View" (ADR-0017 §2, §3.2) makes container ingestion * DUAL-READ: register the container under the bare `` key for @@ -12894,27 +12993,41 @@ export class ObjectStackProtocolImplementation implements * items that WOULD match the switcher, and both object-bound exits answered * zero. * - * Here rather than at either read exit deliberately. There are two - * independent object-bound readers — the REST route reads through - * `getMetaItems`, while `getViewsByObject()` reads `MetadataManager.list` - * — so expanding at one of them fixes the card's literal repro and leaves - * its sibling exit answering empty. This function is the ONE place all - * three runtime hydration callers (boot `loadMetaFromDb`, read-side - * `getMetaItems`, write-through `applyRegistryWriteThrough`) already - * funnel through, so one expansion serves every reader, survives a restart, - * and keeps read-your-writes — the "single, universally-applied location" - * #7163 asked for after the same defect was fixed one seam further in. - * * The canonical-shape filter in `getMetaItems` is deliberately left alone: * its invariant ("a container's expanded items are also present") is what * was false here, and this restores it rather than loosening the filter — * which would surface the legacy wrapper shape to every list consumer and * still show the switcher nothing, since a container carries no `viewKind`. * - * Object-name derivation mirrors `plugin.ts` (`list.data.object` → - * `form.data.object`), falling back to the row's own name — for a container - * the metadata door's save name IS the object. No derivable object means no - * expansion, exactly as the artifact loader already decides. + * ## [#13407] This function's OWN scope boundary — the reason it is not + * ## the whole fix + * + * This is registry-MUTATING, and both its callers gate it off exactly + * where {@link hydrateOverlayIntoRegistry}'s own comments say why: + * write-through (`applyRegistryWriteThrough`) never calls it when + * `this.environmentId !== undefined` (a real per-environment kernel — + * `assembleMetadataProtocol`'s own comment: "per-project (cloud) kernels + * source metadata from the control plane"), and `hydrateOverlayIntoRegistry` + * itself refuses ANY org-scoped row before ever reaching this function + * ("[#6602] a per-org overlay is served on demand, never grafted into the + * registry every org in this process shares") — REGARDLESS of + * `environmentId`. Both gates are deliberate isolation boundaries (ADR-0005 + * per-org isolation; the registry is shared by every org/request a given + * kernel serves) and #13407 does not touch either one. + * + * That is precisely why #7736's fix — this function, reached only through + * the registry-hydration callers — never helped the card's own repro ("a + * signed-in user with an ACTIVE ORG" on "a live multi-node EE deployment"): + * an org-scoped write NEVER reaches this function, on ANY kernel, container + * shape or object-derivation notwithstanding. #13407's actual fix is + * `getMetaItems`' separate, registry-free call to + * {@link expandRuntimeViewContainer} (below, in the overlay-merge section) + * — safe for org/environment isolation because it operates only on rows + * `getMetaItems` already read for THIS request's own scope, and never + * writes them anywhere shared. This function keeps serving the case it + * always did (env-wide rows on an unscoped/control-plane kernel — the ONLY + * combination #7736's own pin ever exercised), now with the corrected + * derivation chain. */ private hydrateExpandedViewItems( type: string, @@ -12922,28 +13035,8 @@ export class ObjectStackProtocolImplementation implements options: { packageId?: string | null; organizationId: string | null }, registry: any, ): void { - if ((PLURAL_TO_SINGULAR[type] ?? type) !== 'view') return; - if (!isAggregatedViewContainer(data)) return; - const container = data as Record; - const viewObject = - container?.list?.data?.object - ?? container?.form?.data?.object - ?? (typeof container.name === 'string' ? container.name : undefined); - if (!viewObject) return; - for (const vi of expandViewContainer(viewObject, container)) { - // Carry the container's package provenance onto each expanded item - // so the package-disable filter and ADR-0048 artifact scoping judge - // them by the same owner the container has. - const item: Record = { ...(vi as any) }; - if (container._packageId !== undefined && item._packageId === undefined) { - item._packageId = container._packageId; - } - const viArtifact = this.lookupArtifactItem( - type, - vi.name, - (item._packageId as string | undefined) ?? options.packageId ?? undefined, - ); - registry.registerItem(type, mergeArtifactProtection(item, viArtifact), 'name' as any); + for (const item of this.expandRuntimeViewContainer(type, data, options)) { + registry.registerItem(type, item, 'name' as any); } } diff --git a/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts b/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts index a370e557f4..52f810d47b 100644 --- a/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts +++ b/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts @@ -253,3 +253,145 @@ describe('#7736 a runtime-authored view container is served', () => { }); }); }); + +/** + * #13407 — the third occurrence of #7736's own defect, and why: #7736's + * `hydrateExpandedViewItems` registry-hydration is reached ONLY for an + * unscoped (`environmentId === undefined`) kernel writing an env-wide + * (`organizationId` unset) row — exactly the one combination the suite above + * exercises. Two gates, both correct and untouched here, exclude everything + * else: `applyRegistryWriteThrough` never write-through hydrates when + * `this.environmentId !== undefined` (a real per-environment/cloud kernel — + * `assembleMetadataProtocol`'s own comment: "per-project (cloud) kernels + * source metadata from the control plane"), and `hydrateOverlayIntoRegistry` + * refuses ANY org-scoped row before ever expanding it ("[#6602] a per-org + * overlay is served on demand, never grafted into the registry every org in + * this process shares" — ADR-0005). The card's own repro is "a signed-in user + * with an ACTIVE ORG" — exactly the excluded case, on any kernel. + * + * The fix is two independent, additive changes, neither touching either gate: + * + * 1. Object-name derivation now checks the container's own top-level + * `object` field FIRST (`ViewSchema.object`) — never consulted before. + * 2. `getMetaItems` expands a container it reads INLINE, into that request's + * own response only, never into the shared SchemaRegistry — safe for + * isolation because it operates only on rows already scoped to THIS + * request's own org/environment (the pre-existing `queryByOrg` merge). + */ +describe('#13407 org-scoped and environment-scoped runtime containers are served', () => { + it('derives the object binding from the containers own top-level `object` field, not just `list.data.object`', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // Saved under a name that is NOT the object, and `list` carries no + // `data.object` either — only the container's own top-level `object` + // field says what this binds to. Pre-fix this expanded under the + // WRONG key (derived from the save name) instead. + const bound = { + object: 'crm_lead', + list: { label: 'All Leads', type: 'grid', columns: [{ field: 'name' }] }, + }; + await protocol.saveMetaItem({ type: 'view', name: 'lead_views', item: bound }); + + const list: any = await protocol.getMetaItems({ type: 'view' }); + expect(switcherMatches(list.items, 'crm_lead').map((v: any) => v.name)).toEqual(['crm_lead.default']); + }); + + it('the cards own repro: a runtime container authored by a signed-in user with an ACTIVE ORG is now served', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await protocol.saveMetaItem({ + type: 'view', name: 'crm_lead', item: leadContainer, organizationId: 'org_acme', + }); + + const list: any = await protocol.getMetaItems({ type: 'view', organizationId: 'org_acme' } as any); + const served = switcherMatches(list.items, 'crm_lead').map((v: any) => v.name).sort(); + expect(served).toEqual(['crm_lead.default', 'crm_lead.pipeline']); + }); + + it('POSITIVE CONTROL: a pre-existing independent ViewItem for the same object is still served alongside the newly-expanded container', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'view', name: 'crm_lead', item: leadContainer, organizationId: 'org_acme', + }); + await protocol.saveMetaItem({ + type: 'view', + name: 'crm_lead.mine', + item: { + name: 'crm_lead.mine', object: 'crm_lead', viewKind: 'list', label: 'My Leads', + config: { type: 'grid', data: { provider: 'object', object: 'crm_lead' }, columns: [{ field: 'name' }] }, + }, + organizationId: 'org_acme', + }); + + const list: any = await protocol.getMetaItems({ type: 'view', organizationId: 'org_acme' } as any); + const served = switcherMatches(list.items, 'crm_lead').map((v: any) => v.name).sort(); + // A fix that merely "returned everything" could not distinguish these: + // the pre-existing independent item and the two newly-expanded + // container items must all be present, distinctly. + expect(served).toEqual(['crm_lead.default', 'crm_lead.mine', 'crm_lead.pipeline']); + }); + + it('ORG ISOLATION HELD: a container authored in org_acme is invisible reading as org_globex, and visible reading as org_acme', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'view', name: 'crm_lead', item: leadContainer, organizationId: 'org_acme', + }); + + const globex: any = await protocol.getMetaItems({ type: 'view', organizationId: 'org_globex' } as any); + expect(switcherMatches(globex.items, 'crm_lead')).toEqual([]); + + const acme: any = await protocol.getMetaItems({ type: 'view', organizationId: 'org_acme' } as any); + expect(switcherMatches(acme.items, 'crm_lead')).toHaveLength(2); + }); + + it('the isolation-safe path never registers an org-scoped expansion into the shared SchemaRegistry', async () => { + const { engine, registered } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'view', name: 'crm_lead', item: leadContainer, organizationId: 'org_acme', + }); + await protocol.getMetaItems({ type: 'view', organizationId: 'org_acme' } as any); + + // The RESPONSE carries the expansion (re-confirmed above); the SHARED + // registry — read by every org/environment this kernel serves — must + // carry none of it. This is what makes the fix isolation-safe without + // touching either scope gate. + expect(registered.get('view')?.size ?? 0).toBe(0); + }); + + it('an ENVIRONMENT-SCOPED kernel (a real per-project/cloud kernel, `assembleMetadataProtocol`s own construction shape) also serves a runtime-authored container', async () => { + const { engine } = makeStubEngine(); + // #7736's own pin only ever constructed an UNSCOPED protocol + // (`environmentId` omitted) — the control-plane instance. This + // constructs the OTHER real shape. + const protocol = new ObjectStackProtocolImplementation(engine, undefined, 'env_prod_1'); + + await protocol.saveMetaItem({ type: 'view', name: 'crm_lead', item: leadContainer }); + const list: any = await protocol.getMetaItems({ type: 'view' }); + const served = switcherMatches(list.items, 'crm_lead').map((v: any) => v.name).sort(); + expect(served).toEqual(['crm_lead.default', 'crm_lead.pipeline']); + }); + + describe('anti-vacuity — the closure does not become "expand anything"', () => { + it('a DIFFERENT object in the SAME org still serves nothing for it', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'view', name: 'crm_lead', item: leadContainer, organizationId: 'org_acme', + }); + const list: any = await protocol.getMetaItems({ type: 'view', organizationId: 'org_acme' } as any); + expect(switcherMatches(list.items, 'crm_account')).toEqual([]); + }); + + it('an org with no container written at all reads empty, not an error', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + const list: any = await protocol.getMetaItems({ type: 'view', organizationId: 'org_acme' } as any); + expect(list.items).toEqual([]); + }); + }); +});