diff --git a/.changeset/org-scoped-meta-read-door.md b/.changeset/org-scoped-meta-read-door.md new file mode 100644 index 0000000000..6302655e67 --- /dev/null +++ b/.changeset/org-scoped-meta-read-door.md @@ -0,0 +1,74 @@ +--- +"@objectstack/metadata-core": patch +"@objectstack/metadata-protocol": patch +"@objectstack/rest": patch +--- + +fix(rest): org-overridable metadata is served back by every `/meta` read door, not just persisted (#9454) + + + +A runtime `PUT` of an org-overridable metadata type — `view`, `dashboard`, +`report`, `translation`, `email_template` — answered **200** with a receipt +reporting `state: 'active'` plus a version and sequence number, **persisted the +row with its `organization_id`**, and was then served back by **nothing**: the +direct `GET` answered 404, the scoped listing was unchanged, the unfiltered +listing was missing it, and the browser rendered an empty view or "Dashboard Not +Found". The platform reported success in the same breath as not delivering the +work, which is declared ≠ enforced in the direction hardest for an author to +notice — the write path says everything worked. + +**The write door was correct as-is.** The row really is persisted, so the +receipt is truthful; this was persisted-but-not-served, never a silent write +no-op. **The overlay-resolution layer was correct too**, and type-agnostic: +`getMetaItem` resolves `(orgId ? findOverlay(orgId) : undefined) ?? +findOverlay(null)`, `getMetaItems` unions both scopes under org-wins precedence, +and `getMetaItemLayered` even reports `overlayScope`. The defect was that the +REST read doors **never stated the scope**, so every one of them asked for the +env-wide partition and the org partition was never consulted. + +**The repair is one registry-derived predicate, threaded at the read doors.** +`organizationIdForMetaRead` joins `organizationIdForMetaWrite` in +`metadata-core`, deriving from the same `allowOrgOverride` registry flag, so +read scope and write scope cannot drift and a registry entry flipping the flag +moves both doors together. It is threaded through the **already-memoised** +`resolveExecCtx`, so no new per-request organization resolution is introduced. + +⛔ **Not a bare `ctx?.tenantId` at each site**, and the reason is measurable +rather than stylistic: deployments predating the #6190 ruling hold **phantom +org-scoped rows for types the registry declares non-overridable** (the runtime +used to stamp `organization_id` on every type). Boot hydration deliberately +walks past those rows, so they are dead. A read door naming the org for *every* +type would resolve them again — serving, on the read side, a document that +vanishes at the next restart. + +**`getMetaItemCached` gains an `organizationId` member** — it was the only meta +read verb that could not express one, having hard-coded a two-key delegation to +`getMetaItem`. The organization is also folded into its **ETag**. The mechanism +differs from `locale` and the difference is stated rather than glossed: `locale` +is invisible to the hash (the body is translated after the validator runs), so +folding it in was the only way it could vary the validator at all, whereas the +org-resolved document *is* the thing hashed. No cache leak is claimed — the +directive is `private, no-cache` and there is no server-side cache entry keyed by +type+name. It is folded in because that makes scope a **declared** property of +the validator instead of an emergent property of the body. + +**Both REST branches are fixed, which is the half-fix this card could easily +have shipped instead.** `view` and `dashboard` share one mechanism but reach it +through two different arms: `view` takes the cached arm (`getMetaItemCached`), +while `dashboard` bypasses the cache via `isDashboardType` and takes the +uncached arm. Both omitted the org, so a fix applied to one arm would have +fixed exactly one type while the receipt kept claiming success for the other. +The scope is now resolved **above** the fork, so the two arms cannot disagree. + +The regression proof drives real REST routes against a real protocol over a stub +engine — write-then-read agreement on **one boot**, for all five types, through +both arms. Its most important assertions are the ones that do **not** merely +check the item comes back: an org-less caller and a **second organization** must +each be refused it. An org-blind overlay fallback would satisfy every other +assertion in the file while matching an arbitrary tenant's row. diff --git a/packages/metadata-core/src/meta-write-org-scope.ts b/packages/metadata-core/src/meta-write-org-scope.ts index 0af050f316..0c26426d48 100644 --- a/packages/metadata-core/src/meta-write-org-scope.ts +++ b/packages/metadata-core/src/meta-write-org-scope.ts @@ -115,3 +115,51 @@ export function organizationIdForMetaWrite( if (activeOrganizationId === undefined) return undefined; return declaresOrgOverride(type) ? activeOrganizationId : undefined; } + +/** + * [#9454] The read-side twin: the `organizationId` a metadata READ of `type` + * should carry, given the session's active organization. + * + * ── Why a read door has to ask this at all ──────────────────────────────── + * + * `organizationIdForMetaWrite` above stops the runtime MINTING org-scoped rows + * for types that have no per-org read channel. It says nothing about serving + * the rows that types WITH such a channel legitimately produce — and the REST + * `/meta` read doors were never told. A `PUT` of an org-overridable type + * (`view`, `dashboard`, `report`, `translation`, `email_template`) landed an + * org-scoped row, answered `200 state:'active'`, and then every REST read door + * asked for the row WITHOUT naming an organization. `getMetaItem` resolves + * `(orgId ? findOverlay(orgId) : undefined) ?? findOverlay(null)`, so an + * org-less read resolves the env-wide row only: the author's work was + * persisted, receipted as live, and served by nothing. That is #9454. + * + * ── Why it is registry-derived and NOT a bare `ctx?.tenantId` ───────────── + * + * ⛔ The tempting shorter fix — pass the active org at every read site — is + * wrong in a way that only shows on databases with history. Deployments that + * ran before the #6190 ruling contain PHANTOM org-scoped rows for types the + * registry declares non-overridable (`object`, `flow`, … — the runtime used to + * stamp `organization_id` on every type; `reportUnhydratableOrgScopedRows` is + * the audit that warns about the survivors). Boot hydration walks past those + * rows deliberately, so they are dead. A read door that named the org for + * EVERY type would resolve them again — resurrecting, on the read side, exactly + * the phantom writes #6190 stopped minting, and serving a document that + * vanishes at the next restart. Gating the read on the same static registry + * flag keeps the two sides answering one question. + * + * ⇒ This is deliberately the same predicate as the write side, not a parallel + * one: read scope and write scope CANNOT drift, because both are + * {@link declaresOrgOverride}. If a registry entry flips `allowOrgOverride`, + * both doors move together and there is nothing to keep in sync by hand. + * + * Returns the active org for a type the registry declares per-org overridable, + * and `undefined` — env-wide, today's behaviour for every read — otherwise. + * An anonymous or org-less caller reads exactly what it reads today. + */ +export function organizationIdForMetaRead( + type: string, + activeOrganizationId: string | undefined, +): string | undefined { + if (activeOrganizationId === undefined) return undefined; + return declaresOrgOverride(type) ? activeOrganizationId : undefined; +} diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index e02f89fe6c..7a0c4673b8 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -9244,7 +9244,18 @@ export class ObjectStackProtocolImplementation implements // Metadata Caching // ========================================== - async getMetaItemCached(request: { type: string, name: string, cacheRequest?: MetadataCacheRequest, locale?: string }): Promise { + /** + * [#9454] `organizationId` — the sole meta read verb that could not express + * an org, which is why the CACHED door (`view` and every org-overridable + * type that is not `dashboard`) served nothing back after a runtime `PUT`. + * Its siblings `getMetaItem` / `getMetaItems` / `getMetaItemLayered` have + * carried the member all along; this one hard-coded a two-key delegation + * and dropped whatever the caller knew. Threaded into `getMetaItem` below, + * so the ADR-0005 read order (`sys_metadata` org row → env-wide row → + * registry → MetadataService) is honoured at the same scope the caller + * named — the whole reason this method delegates rather than re-reading. + */ + async getMetaItemCached(request: { type: string, name: string, cacheRequest?: MetadataCacheRequest, locale?: string, organizationId?: string }): Promise { // #4432 — CANONICAL TYPE KEY. See {@link canonicalMetaType}. The ETag // and the cache entry are keyed by type, so two spellings would cache // the same item twice and invalidate only one of them. @@ -9253,7 +9264,18 @@ export class ObjectStackProtocolImplementation implements // Delegate to getMetaItem so the customization-overlay read order // (sys_metadata → registry → MetadataService) is honoured here too // (ADR-0005). Without this, cached reads silently bypass overlays. - const result = await this.getMetaItem({ type: request.type, name: request.name }); + const result = await this.getMetaItem({ + type: request.type, + name: request.name, + // [#9454] Spread, not an unconditional member: `getMetaItem` + // branches on `organizationId !== undefined`, so passing an + // explicit `undefined` is not the same statement as passing + // nothing. Mirrors the conditional-spread idiom the REST read + // doors use to reach here. + ...(request.organizationId !== undefined + ? { organizationId: request.organizationId } + : {}), + }); const item = (result as any)?.item; if (!item) { @@ -9284,8 +9306,36 @@ export class ObjectStackProtocolImplementation implements // carrying a stale-locale body — labels/headers stuck in the old // language until a hard refresh (issue #1319). Folding the resolved // locale into the hash gives each locale a distinct validator. + // + // [#9454] The ETag MUST also state the ORGANIZATION scope, and the + // mechanism differs from `locale` above in a way worth stating + // rather than glossing. `locale` is INVISIBLE to the hash (the body + // is translated AFTER this runs), so folding it in was the only way + // it could vary the validator at all. `organizationId` is VISIBLE — + // the org-resolved document is the very thing hashed — so two orgs + // whose overlays differ already get different validators, and no + // leak is claimed here: `Cache-Control` is `private, no-cache` and + // there is no server-side cache ENTRY keyed by type+name. + // + // It is folded in anyway because that makes the scope a DECLARED + // property of the validator instead of an emergent property of the + // body. Two orgs whose documents are byte-identical today share a + // validator by coincidence, not by statement; and any future path + // that resolves an org row but falls back to the env-wide body + // would answer a 304 pinning the caller to a wrong-scope document + // with nothing in the validator to show it. Prepended, and ONLY + // when present, so an org-less caller's validator stays byte-for- + // byte the one it is issued today. const content = JSON.stringify(item); - const hash = simpleHash(request.locale ? `${request.locale}\u0000${content}` : content); + const scope = [ + request.organizationId ? `org:${request.organizationId}` : undefined, + request.locale || undefined, + ].filter((part): part is string => part !== undefined); + const hash = simpleHash( + scope.length > 0 + ? `${scope.join('\u0000')}\u0000${content}` + : content, + ); const etag = { value: hash, weak: false }; // Check If-None-Match header 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 new file mode 100644 index 0000000000..752c48ca64 --- /dev/null +++ b/packages/rest/src/rest-server-meta-read-org-scope.test.ts @@ -0,0 +1,408 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #9454 — a runtime `PUT` of an org-overridable metadata type answered 200 with +// a `state:'active'` receipt, PERSISTED the row with its `organization_id`, and +// then no REST read door served it back. The author's work rendered as lost +// while the write path reported success: declared ≠ enforced, in the direction +// hardest for an author to notice. +// +// ── Where the defect was, and where it was NOT ──────────────────────────── +// +// NOT in the overlay-resolution layer. `getMetaItem` resolves +// `(orgId ? findOverlay(orgId) : undefined) ?? findOverlay(null)` and +// `getMetaItems` unions both scopes under org-wins precedence — both correct +// and type-agnostic. The REST read doors simply never STATED the scope, so the +// reader looked in the env-wide partition for a row that had landed in an org +// one. The write door is correct as-is: the row is persisted, so its receipt is +// truthful. (Direction (a) — make the read door serve it — settled on-card.) +// +// ── The two-branch trap this file exists to pin ─────────────────────────── +// +// `view` and `dashboard` share ONE mechanism but reach it through two DIFFERENT +// REST branches: `view` takes the cached arm (`getMetaItemCached`), `dashboard` +// bypasses the cache via `isDashboardType` and takes the uncached arm +// (`getMetaItem`). Both omitted the org, so a fix applied to one arm fixes +// exactly ONE type while the receipt keeps claiming success for the other. Both +// arms are driven below, on the same boot, for every org-overridable type. +// +// ── Why the harness is the REAL protocol, not a spy ─────────────────────── +// +// A spy asserting "the door passed `organizationId`" cannot tell a fix from a +// fix-shaped no-op: the claim is write-then-READ AGREEMENT, so the row has to +// actually land in a partition and actually come back out of it. These drive +// real REST routes against a real `ObjectStackProtocolImplementation` over a +// stub engine, so the assertions are round trips on one boot. +// +// ⛔ THE CONTROL THAT MATTERS MOST is `does not serve another org's row`. The +// refused repair for this card was to make the overlay lookup fall back to +// matching ANY org row when the caller names none — `matchesWhere` skips +// `undefined` keys, so that matches an ARBITRARY org's row. It is a cross-tenant +// disclosure, not a fix, and it would pass every other assertion in this file. +// The original reproduction could not have caught it: its confound control was +// "one `sys_organization` row, and it is the session's active org". So a SECOND +// org exists here for no other purpose. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server.js'; + +const META = '/api/v1/meta'; +const ORG_A = 'org_alpha'; +const ORG_B = 'org_beta'; + +/** + * The five types the registry declares `allowOrgOverride: true`. Listed as + * literals rather than derived, deliberately: the point of the card is that all + * five must be SERVED, so a registry change that drops one should turn this red + * and be looked at, not silently shrink the pin's coverage. + */ +const ORG_OVERRIDABLE = ['view', 'dashboard', 'report', 'translation', 'email_template'] as const; + +/** The two whose REST branches differ — the trap, named. */ +const CACHED_ARM = 'view'; // takes `getMetaItemCached` +const UNCACHED_ARM = 'dashboard'; // bypasses it via `isDashboardType` + +/** `allowOrgOverride: false` — must keep reading env-wide, never org-scoped. */ +const NON_OVERRIDABLE = 'object'; + +/** The value every read assertion looks for. */ +const MARKER = 'AUTHORED_AT_RUNTIME'; + +/** + * 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 + * validation (`INVALID_METADATA`, 422), so a thin fixture never reaches the + * store and every read assertion below would fail for a reason that has + * 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 }; + switch (type) { + case 'view': + // [#7741] the inline arm requires the object-binding pair. + return { ...marker, object: 'task', viewKind: 'list', columns: [{ field: 'name', label: 'Name' }] }; + case 'dashboard': + return { ...marker, widgets: [] }; + case 'report': + return { ...marker, dataset: 'orders_ds', values: ['order_count'] }; + case 'translation': + return { ...marker, locale: 'en-US' }; + case 'email_template': + return { ...marker, subject: 'Hi', bodyHtml: '

Hello

' }; + case 'object': + // [ADR-0090 D1] an authored `sharingModel` is required at the write + // door; without it this control fails on the WRITE and never + // reaches the read it exists to make. + return { + ...marker, + sharingModel: 'private', + fields: { title: { type: 'text', label: 'Title' } }, + }; + default: + throw new Error(`no fixture body for type ${type}`); + } +} + + +// ── stub engine (the `protocol.org-scoped-write-refused.test.ts` pattern) ── + +interface Row { + id: string; type: string; name: string; + organization_id: string | null; package_id: string | null; + state: string; metadata: string; checksum?: string; version?: number; +} + +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 { + 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; + } + if (v === undefined) continue; + if ((r as unknown as Record)[k] !== v) return false; + } + return true; +} + +function makeStubEngine() { + const rows = new Map(); + const historyRows: any[] = []; + let nextId = 0; + + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + if (w.package_id !== undefined) { + const k = keyOf(w); + 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 }; + return null; + }; + + const engine: any = { + async findOne(table: string, opts: { where: Record }) { + if (table === 'sys_metadata_history') return 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 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}` }); + return { id: `h_${nextId}` }; + } + if (table !== 'sys_metadata') return { id: 'side_effect_skip' }; + nextId += 1; + const row = { ...(data as unknown as Row), id: `r_${nextId}` }; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as unknown as Row) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: unknown, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + async syncObjectSchema() { return true; }, + registry: { + registerItem: () => {}, registerObject: () => {}, + listItems: () => [], getItem: () => undefined, + getObject: () => undefined, getPackage: () => undefined, + getArtifactItem: () => undefined, + // The LIST door prunes items belonging to disabled packages; a + // registry double without this answers 500, which would have read + // as "the listing still does not serve org rows". + isPackageDisabled: () => false, + }, + }; + return { engine, rows }; +} + +// ── REST harness: real protocol, real routes, one boot ──────────────────── + +function mockServer() { + const noop = () => {}; + return { + get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, + listen: async () => undefined, close: async () => undefined, + }; +} + +function mockRes() { + const res: any = { + statusCode: 200, + _body: undefined, + json(body: any) { this._body = body; return this; }, + send() { return this; }, + setHeader() { return this; }, + status(code: number) { this.statusCode = code; return this; }, + header() { return this; }, + }; + return res; +} + +/** + * One boot, one backing store. `session` is what `resolveExecCtx` resolves to — + * the SAME memoised seam the write doors read, which is why threading the read + * doors through it adds no new org resolution. Reassignable so a second tenant + * can read the same store on the same boot (the cross-tenant control). + */ +function boot() { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + protocol.getDiscovery = async () => ({ + version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' }, + }); + + const rest = new RestServer( + mockServer() as any, + protocol as any, + { api: { requireAuth: false } } as any, + ); + let session: any = { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId: ORG_A }; + (rest as any).resolveExecCtx = async () => session; + rest.registerRoutes(); + + const drive = async (method: string, path: string, req: Record = {}) => { + const found = (rest as any).getRoutes().find( + (r: any) => r.method === method && r.path === path, + ); + if (!found) throw new Error(`route not registered: ${method} ${path}`); + const res = mockRes(); + let thrown: any; + try { + await found.handler( + { method, path, params: {}, query: {}, headers: {}, body: {}, ...req } as any, + res, + ); + } catch (err) { thrown = err; } + return { status: res.statusCode, body: res._body, thrown }; + }; + + return { + rows, + 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) }), + get: (type: string, name: string) => + drive('GET', `${META}/:type/:name`, { params: { type, name } }), + list: (type: string) => + drive('GET', `${META}/:type`, { params: { type } }), + }; +} + +/** The document a GET served, whichever envelope shape the arm answers in. */ +function servedDocument(body: any): any { + if (!body || typeof body !== 'object') return undefined; + return body.item ?? body.data ?? body; +} + +/** Names present in a list response, whichever shape it answers in. */ +function listedNames(body: any): string[] { + const items = Array.isArray(body) ? body + : Array.isArray(body?.items) ? body.items + : []; + return items.map((i: any) => i?.name).filter(Boolean); +} + +describe('#9454 every REST /meta read door serves what the write door persisted', () => { + let b: ReturnType; + beforeEach(() => { b = boot(); }); + + describe('write-then-read agreement on one boot, both branches', () => { + it.each(ORG_OVERRIDABLE)( + '%s: the 200 state:active receipt is answered by the direct GET', + async (type) => { + const written = await b.put(type, 'authored_at_runtime'); + // The receipt half — unchanged by this card, asserted so a + // harness that could not write at all cannot pass the read half + // for the wrong reason. + expect(written.status, `PUT /${type} was not accepted`).toBe(200); + expect(written.body?.state).toBe('active'); + + const read = await b.get(type, 'authored_at_runtime'); + expect(read.thrown, `GET /${type} threw: ${read.thrown?.code}`).toBeUndefined(); + expect(read.status, `GET /${type} did not serve the item`).toBe(200); + expect(servedDocument(read.body)?.label).toBe(MARKER); + }, + ); + + it.each(ORG_OVERRIDABLE)( + '%s: the scoped listing contains it too', + async (type) => { + await b.put(type, 'authored_at_runtime'); + const listed = await b.list(type); + expect(listed.status).toBe(200); + expect(listedNames(listed.body)).toContain('authored_at_runtime'); + }, + ); + + it('covers BOTH REST branches, not one — the half-fix guard', async () => { + // The assertion is about ROUTE MECHANICS, so it is stated + // separately from the parametrised cases above: `view` and + // `dashboard` agreeing here is what proves the cached arm and the + // `isDashboardType` bypass were BOTH threaded. A fix to one arm + // leaves exactly one of these two red. + await b.put(CACHED_ARM, 'both_arms'); + await b.put(UNCACHED_ARM, 'both_arms'); + + const cached = await b.get(CACHED_ARM, 'both_arms'); + const uncached = await b.get(UNCACHED_ARM, 'both_arms'); + + expect(servedDocument(cached.body)?.label, 'cached arm (view) lost the overlay').toBe(MARKER); + expect(servedDocument(uncached.body)?.label, 'uncached arm (dashboard) lost the overlay').toBe(MARKER); + }); + }); + + describe('⛔ the scope is STATED, not guessed — cross-tenant controls', () => { + it('does not serve another org row to a caller that named no org', async () => { + // The refused option C, pinned. An org-blind fallback matching ANY + // org row passes every assertion above and fails only here. + await b.put(CACHED_ARM, 'org_a_only'); + b.as(undefined); + + const read = await b.get(CACHED_ARM, 'org_a_only'); + expect( + servedDocument(read.body)?.label, + 'an org-less caller was served an org-scoped row', + ).not.toBe(MARKER); + expect(listedNames((await b.list(CACHED_ARM)).body)).not.toContain('org_a_only'); + }); + + it('does not serve org A row to org B on the same boot', async () => { + // The control the original reproduction structurally could not run: + // it had exactly one organization. + await b.put(UNCACHED_ARM, 'tenant_bound'); + b.as(ORG_B); + + const read = await b.get(UNCACHED_ARM, 'tenant_bound'); + expect( + servedDocument(read.body)?.label, + 'org B was served org A metadata', + ).not.toBe(MARKER); + expect(listedNames((await b.list(UNCACHED_ARM)).body)).not.toContain('tenant_bound'); + }); + + it('leaves a NON-overridable type reading env-wide', async () => { + // The registry gate, not decoration. Naming the org for every type + // would resurrect #6190's phantom rows on the READ side — rows boot + // hydration deliberately walks past, so serving them means serving a + // document that vanishes at the next restart. + await b.put(NON_OVERRIDABLE, 'accounts'); + const row = Array.from(b.rows.values()).find((r) => r.name === 'accounts'); + expect(row?.organization_id ?? null, 'a non-overridable write went org-scoped').toBe(null); + + const read = await b.get(NON_OVERRIDABLE, 'accounts'); + expect(read.status).toBe(200); + expect(servedDocument(read.body)?.label).toBe(MARKER); + }); + }); + + describe('the row really is org-partitioned — the premise, re-measured', () => { + it('persists with organization_id, which is why an env-wide read missed it', async () => { + // Guards the card's own diagnosis: this is persisted-but-not-served, + // never a silent write no-op. If this turns red the defect has + // changed shape and the rest of this file is asserting the wrong + // thing. + const written = await b.put(CACHED_ARM, 'partitioned'); + const row = Array.from(b.rows.values()).find((r) => r.name === 'partitioned'); + expect( + row, + 'nothing was persisted at all; PUT answered ' + + `${written.status} ${JSON.stringify(written.body)} thrown=${written.thrown?.message}`, + ).toBeDefined(); + expect(row?.organization_id).toBe(ORG_A); + }); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 2e2017eedc..c7f27461b7 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -39,6 +39,7 @@ import { // (`domains/meta.ts`), not a REST-local restatement of it. See the module's // own header for why the decision belongs to the caller and why it lives in // `metadata-core`. + organizationIdForMetaRead, organizationIdForMetaWrite, } from '@objectstack/metadata-core'; import { RouteManager, type RouteEntry } from './route-manager.js'; @@ -2420,11 +2421,29 @@ export class RestServer { // not in its two callers, so both entry points answer identically. if (refuseRepeatedQueryParams(req, res, ['package'])) return; const layeredPackageId = req.query?.package || undefined; + // [#9454] State the ORG scope, exactly as the `/published` overlay read + // already does. Without it the layered view resolved the env-wide row + // only, so an author who had just saved an org overlay opened Studio to + // `overlay: null` and the code layer — the write receipted as live, the + // editor reporting it absent. This is the DIAGNOSTIC view of what is + // stored per layer, so an unstated scope does not merely miss a row: it + // misreports the very thing being diagnosed. + // ⚠️ NOT a new org-resolution seam — `resolveExecCtx` is memoised per + // request (WeakMap keyed by `req`), the same result 40+ handlers here + // already share. Registry-gated via `organizationIdForMetaRead` so a + // non-overridable type keeps reading env-wide (see that predicate for + // why naming the org unconditionally would resurrect #6190's phantoms). + const layeredCtx = await this.resolveExecCtx(environmentId, req) + .catch(() => undefined); + const layeredOrganizationId = organizationIdForMetaRead( + req.params.type, layeredCtx?.tenantId, + ); const layered = await p.getMetaItemLayered({ type: req.params.type, name: req.params.name, ...(layeredPackageId ? { packageId: layeredPackageId } : {}), ...(environmentId ? { environmentId } : {}), + ...(layeredOrganizationId ? { organizationId: layeredOrganizationId } : {}), }); // [ADR-0106 D5(4)] The layered view is a schema-bearing exit — // `code`, `overlay` and `effective` are each a full object schema. @@ -3805,11 +3824,25 @@ export class RestServer { // published-only world. const previewDrafts = typeof req.query?.preview === 'string' && req.query.preview.toLowerCase() === 'draft'; + // [#9454] The scoped listing is the second door the + // card measured absent (`?object=` unchanged after a + // runtime PUT). `getMetaItems` unions the env-wide and + // org scopes under org-wins precedence — but only when + // the caller names an org; unnamed, it returns the + // env-wide partition alone and the author's new item is + // simply not in the list. Same memoised `resolveExecCtx` + // and same registry gate as every other read door here. + const listCtx = await this.resolveExecCtx(environmentId, req) + .catch(() => undefined); + const listOrganizationId = organizationIdForMetaRead( + req.params.type, listCtx?.tenantId, + ); const items = await p.getMetaItems({ type: req.params.type, packageId, ...(previewDrafts ? { previewDrafts: true } : {}), ...(environmentId ? { environmentId } : {}), + ...(listOrganizationId ? { organizationId: listOrganizationId } : {}), } as any); // RBAC-filter app metadata for authenticated users so @@ -4616,6 +4649,22 @@ export class RestServer { // the cache exclusion here AND by the gate itself in // the uncached branch below; one predicate, two sites. const isAudienceGatedType = metaType === 'book' || metaType === 'doc'; + // [#9454] ONE org resolution for BOTH arms of the fork + // below, computed ABOVE it on purpose. `view` takes the + // cached arm; `dashboard` bypasses it via + // `isDashboardType` and takes the uncached arm. A scope + // threaded into only one arm fixes exactly ONE of the + // five org-overridable types while the receipt keeps + // claiming success for the rest — the half-fix this + // card's pin exists to forbid. Hoisting it makes the + // two arms incapable of disagreeing about scope. + // ⚠️ NOT a new seam: memoised per request, and this + // handler resolves the same context again further down. + const readCtx = await this.resolveExecCtx(environmentId, req) + .catch(() => undefined); + const readOrganizationId = organizationIdForMetaRead( + req.params.type, readCtx?.tenantId, + ); if (metadata.enableCache && p.getMetaItemCached && !isAppType && !isDashboardType && !isDraftRead && !previewDrafts && !packageScoped && !isAudienceGatedType) { // [ADR-0106 D3] When a projection applies, the // protocol is NOT allowed to judge the conditional @@ -4646,6 +4695,14 @@ export class RestServer { cacheRequest, ...(cacheLocale ? { locale: cacheLocale } : {}), ...(environmentId ? { environmentId } : {}), + // [#9454] The cached door is the `view` arm, and + // it used to hard-code a two-key delegation to + // `getMetaItem` — it could not express an org at + // all, so this threading is paired with a widened + // signature in `metadata-protocol`. The org also + // enters the ETag there, so the validator states + // the scope rather than inheriting it. + ...(readOrganizationId ? { organizationId: readOrganizationId } : {}), } as any); if (result.notModified) { @@ -4756,6 +4813,11 @@ export class RestServer { packageId, ...(stateParam === 'draft' ? { state: 'draft' } : {}), ...(previewDrafts ? { previewDrafts: true } : {}), + // [#9454] The uncached arm — `dashboard`'s route + // (`isDashboardType`), and every read the cache + // exclusions divert here. Same hoisted scope as + // the cached arm above, by construction. + ...(readOrganizationId ? { organizationId: readOrganizationId } : {}), } as any) as Record; // [#5563] `getMetaItem` answers the envelope @@ -5973,10 +6035,23 @@ export class RestServer { // read this route mirrors. if (refuseRepeatedQueryParams(req, res, ['package'])) return; const packageId = req.query?.package || undefined; + // [#9454] The compound-name door serves EVERY type + // through one generic `getMetaItem` — including the + // org-overridable ones — so it needs the same scope the + // single-segment read it mirrors now states. Left + // org-blind it would be the surviving route that keeps + // answering from the wrong partition after the other + // doors are fixed. + const compoundCtx = await this.resolveExecCtx(environmentId, req) + .catch(() => undefined); + const compoundOrganizationId = organizationIdForMetaRead( + req.params.type, compoundCtx?.tenantId, + ); const envelope = await p.getMetaItem({ type: req.params.type, name: compoundName, packageId, + ...(compoundOrganizationId ? { organizationId: compoundOrganizationId } : {}), } as any) as Record; // [ADR-0106 D5(4)] Compound names express sub-resources, // and no object uses one today — but this route serves