Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions .changeset/org-scoped-meta-read-door.md
Original file line numberDiff line numberDiff line change
@@ -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)

<!-- adr-0087: not-required (no-migration-prescription) No authorable key is
added, renamed, retired or tombstoned. One new exported predicate in
`metadata-core` (`organizationIdForMetaRead`), one widened optional request
member on an existing protocol method (`getMetaItemCached`'s `organizationId`),
and caller-side threading in `packages/rest`. The accept/reject behaviour of
every write door is unchanged — this card is read-side only. -->

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.
48 changes: 48 additions & 0 deletions packages/metadata-core/src/meta-write-org-scope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
}
56 changes: 53 additions & 3 deletions packages/metadata-protocol/src/protocol.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -9244,7 +9244,18 @@ export class ObjectStackProtocolImplementation implements
// Metadata Caching
// ==========================================

async getMetaItemCached(request: { type: string, name: string, cacheRequest?: MetadataCacheRequest, locale?: string }): Promise<MetadataCacheResponse> {
/**
* [#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<MetadataCacheResponse> {
// #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.
Expand All@@ -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) {
Expand DownExpand Up@@ -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
Expand Down
Loading
Loading