Skip to content

Commit 4bb6f01

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol): an org-scoped overlay row no longer reaches the process-wide SchemaRegistry (#6602) (#6779)
Both runtime hydration seams gated on `environmentId` alone and said nothing about `organization_id`, so on an unscoped (control-plane) kernel a per-org overlay reached the shared registry under the plain key — through the #4521 write-through, and again through the `getMetaItems` read hydration one listing call later. The row-scope verdict now lives in `hydrateOverlayIntoRegistry`, the one choke point all three hydration callers already share, with a REQUIRED `organizationId` argument so a fourth caller cannot forget it. The kernel-scope gate stays with the callers. Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw Co-authored-by: Claude <noreply@anthropic.com>
1 parent e15e679 commit 4bb6f01

3 files changed

Lines changed: 636 additions & 10 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): an org-scoped overlay row no longer reaches the process-wide SchemaRegistry (#6602)
6+
7+
ADR-0005 (revised 2026-05) says only **env-wide** rows (`organization_id IS NULL`)
8+
enter the process-wide `SchemaRegistry`; per-org overlays are served on demand and
9+
never grafted into the registry every org in the process shares. The registry has
10+
exactly one plain key per `(type, name)` and no org dimension to hold two orgs'
11+
bodies apart, so a per-org body sitting under that key IS the other orgs' body.
12+
13+
Boot obeyed the rule — `loadMetaFromDb` filters `organization_id: null` and says so
14+
in its own comment. Both **runtime** seams did not:
15+
16+
- **The write-through.** `applyRegistryWriteThrough` gated on `environmentId` alone.
17+
Its TSDoc already claimed the rule ("a project-scoped row must not be registered
18+
into a registry that unscoped callers share. The write must not be more permissive
19+
about that than the read is") while the code said nothing about `organization_id`.
20+
On an unscoped kernel a per-org `view` write hydrated straight into the registry
21+
under the plain key.
22+
- **The read hydration.** `getMetaItems` merges this caller's org rows into the
23+
env-wide set and then hydrated the whole merged set under the same
24+
`environmentId === undefined` gate — so one org-scoped listing call grafted that
25+
org's bodies too, and would have undone a write-side-only fix at the next listing.
26+
27+
Both were observable rather than theoretical: once org A's body sat under the plain
28+
key, org B's listing started from org A's body, and where the names did not collide
29+
org A's item was simply **in** org B's list. Per #5086 a host config boots
30+
`new ObjectQLPlugin()` with no `environmentId`, so the flagship showcase runs on
31+
exactly this kernel shape.
32+
33+
**The fix restores the stated invariant at both seams at once, in one place.**
34+
`hydrateOverlayIntoRegistry` is the single choke point all three hydration callers
35+
(boot, read-side, write-through) already route through since #4521, so the row-scope
36+
verdict now lives there — and its `organizationId` argument is **required**, not
37+
optional: an omitted org would default to "env-wide" and reinstate the hole, while a
38+
required one makes every caller state the row's scope to compile. The kernel-scope
39+
gate (`environmentId === undefined`) stays with the callers, because that is a fact
40+
about the kernel, not about the row.
41+
42+
Not changed, deliberately:
43+
44+
- **What org readers see.** The merged listing, `getMetaItem`'s org-preferred read,
45+
and the org-scoped write itself are all untouched — this closes a registry leak,
46+
never a write or a read. Per-org overlays keep working exactly as ADR-0005
47+
designed them: served on demand.
48+
- **#4521 read-your-writes.** An env-wide save is still dispatchable the moment it
49+
lands, with no listing call in between.
50+
- **The `object` branch.** An `object` is `allowOrgOverride: false` and its physical
51+
table is env-wide, so the registry entry backing it is env-wide too;
52+
`assertObjectRegistered` fails closed on a missing entry, so gating that branch
53+
would make a runtime-created object unreachable for data CRUD rather than merely
54+
un-listed. That branch has never carried the `environmentId` gate either, for the
55+
same reason.
56+
- **The delete chain.** `restoreArtifactRegistryView` stays `(type, name)`-addressed:
57+
with both entry seams refusing org rows there is nothing org-scoped in the registry
58+
for it to mis-address, so no re-keying is needed (pinned in both directions).

packages/metadata-protocol/src/protocol.ts

Lines changed: 112 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3471,7 +3471,11 @@ export class ObjectStackProtocolImplementation implements
34713471
if (recPkg && data && typeof data === 'object' && (data as any)._packageId === undefined) {
34723472
(data as any)._packageId = recPkg;
34733473
}
3474-
return { data, packageId: recPkg };
3474+
// [#6602] The row's own scope travels with its body. The
3475+
// merged set below is env-wide rows PLUS this org's rows,
3476+
// and the two are only distinguishable here, at the row.
3477+
const recOrg = (record as { organization_id?: string | null }).organization_id ?? null;
3478+
return { data, packageId: recPkg, organizationId: recOrg };
34753479
});
34763480

34773481
// ADR-0048 (#1828) — package-aware merge: a package-scoped row
@@ -3496,9 +3500,22 @@ export class ObjectStackProtocolImplementation implements
34963500
// shared {@link hydrateOverlayIntoRegistry} that both callers
34973501
// use: a read and a write that register differently would put
34983502
// the registry in two different states for the same row.
3503+
//
3504+
// [#6602] The kernel gate below is only half the rule, and the
3505+
// half that was missing is the ROW's: `overlays` is the MERGED
3506+
// env-wide + org-scoped set, so this loop used to graft this
3507+
// caller's org bodies into the registry every other org in the
3508+
// process reads from — one listing call was enough, and it also
3509+
// undid the write-side gate for anything already saved. The
3510+
// per-row verdict now lives in the shared hydrator, which each
3511+
// row's own `organizationId` answers to; the merged LIST above
3512+
// is unchanged, so org readers still get their overlays.
34993513
if (this.environmentId === undefined) {
3500-
for (const { data, packageId: recPkg } of overlays) {
3501-
this.hydrateOverlayIntoRegistry(request.type, data, recPkg);
3514+
for (const { data, packageId: recPkg, organizationId: recOrg } of overlays) {
3515+
this.hydrateOverlayIntoRegistry(request.type, data, {
3516+
packageId: recPkg,
3517+
organizationId: recOrg,
3518+
});
35023519
}
35033520
}
35043521
}
@@ -7668,14 +7685,52 @@ export class ObjectStackProtocolImplementation implements
76687685
* so a colliding overlay no longer grafts the first-registered package's
76697686
* provenance/lock onto another package's row.
76707687
*
7671-
* Returns whether anything was registered (bodies without a `name`, and
7672-
* registry doubles without `registerItem`, are no-ops).
7688+
* ── [#6602] THE ROW-SCOPE GATE LIVES HERE, AND ITS ARGUMENT IS REQUIRED ──
7689+
*
7690+
* ADR-0005 (revised 2026-05): **only env-wide rows
7691+
* (`organization_id IS NULL`) enter the process-wide SchemaRegistry.**
7692+
* Per-org overlays are served on demand by `getMetaItem` /
7693+
* `getMetaItems` and never grafted into the shared registry, because that
7694+
* registry has exactly one plain key per `(type, name)` and no org
7695+
* dimension to hold them apart.
7696+
*
7697+
* Boot already obeyed this — `loadMetaFromDb` filters
7698+
* `organization_id: null` and states the rule in its own comment — but
7699+
* the two RUNTIME seams did not: {@link applyRegistryWriteThrough} gated
7700+
* on `environmentId` alone (its TSDoc claimed the rule and the code said
7701+
* nothing about org), and the `getMetaItems` hydration loop walked the
7702+
* merged env-wide + org record set. Measured on an unscoped kernel, an
7703+
* org-scoped `view` write landed in the registry under the plain key, and
7704+
* one org-scoped listing call did the same — so org B's next listing
7705+
* started from org A's body (#6602).
7706+
*
7707+
* `organizationId` is therefore a REQUIRED parameter and not an optional
7708+
* one: an omitted org would default to "env-wide" and reinstate the exact
7709+
* hole, whereas a required one makes every caller state the row's scope.
7710+
* Declared = enforced, at the ONE choke point all three hydration callers
7711+
* (boot, read-side, write-through) already share — a fourth caller cannot
7712+
* forget a gate it has to answer to compile.
7713+
*
7714+
* The KERNEL-scope gate (`environmentId === undefined`) deliberately
7715+
* stays with the callers: that is a fact about the kernel this protocol
7716+
* instance serves, not about the row in hand.
7717+
*
7718+
* Returns whether anything was registered (org-scoped rows, bodies
7719+
* without a `name`, and registry doubles without `registerItem`, are
7720+
* no-ops).
76737721
*/
7674-
private hydrateOverlayIntoRegistry(type: string, data: unknown, packageId?: string | null): boolean {
7722+
private hydrateOverlayIntoRegistry(
7723+
type: string,
7724+
data: unknown,
7725+
options: { packageId?: string | null; organizationId: string | null },
7726+
): boolean {
7727+
// [#6602] ADR-0005 — a per-org overlay is served on demand, never
7728+
// grafted into the registry every org in this process shares.
7729+
if (options.organizationId !== null && options.organizationId !== undefined) return false;
76757730
if (!data || typeof data !== 'object' || !('name' in data)) return false;
76767731
const registry: any = (this.engine as any)?.registry;
76777732
if (!registry || typeof registry.registerItem !== 'function') return false;
7678-
const artifact = this.lookupArtifactItem(type, (data as any).name, packageId ?? undefined);
7733+
const artifact = this.lookupArtifactItem(type, (data as any).name, options.packageId ?? undefined);
76797734
registry.registerItem(type, mergeArtifactProtection(data, artifact), 'name' as any);
76807735
return true;
76817736
}
@@ -7712,15 +7767,43 @@ export class ObjectStackProtocolImplementation implements
77127767
* gate the read-side hydration carries: a project-scoped row must not be
77137768
* registered into a registry that unscoped (control-plane) callers share.
77147769
* The write must not be more permissive about that than the read is.
7770+
*
7771+
* [#6602] That sentence was true of the ENVIRONMENT dimension and false
7772+
* of the ORGANIZATION one: the gate above says nothing about
7773+
* `organization_id`, so on an unscoped kernel a per-org overlay write
7774+
* hydrated straight into the process-wide registry under the plain key —
7775+
* the designed per-org overlay leaking out of its org. `organizationId`
7776+
* is now part of this request and is handed to
7777+
* {@link hydrateOverlayIntoRegistry}, which owns the row-scope verdict
7778+
* for all three hydration paths. Callers pass the SAME `orgId` they wrote
7779+
* the row with, so the registry's view cannot disagree with the row's
7780+
* scope.
77157781
*/
7716-
private applyRegistryWriteThrough(request: { type: string; name: string; item?: any; packageId?: string | null }): void {
7782+
private applyRegistryWriteThrough(request: {
7783+
type: string;
7784+
name: string;
7785+
item?: any;
7786+
packageId?: string | null;
7787+
/** The row's org scope — `null` for an env-wide row. [#6602] */
7788+
organizationId: string | null;
7789+
}): void {
77177790
if (request.type === 'object' || request.type === 'objects') {
7791+
// NOT org-gated, deliberately: an `object` is `allowOrgOverride:
7792+
// false` (ADR-0005) and its physical TABLE is env-wide, so the
7793+
// registry entry backing it is env-wide too — `assertObjectRegistered`
7794+
// fails CLOSED on a missing entry, and refusing to register here
7795+
// would make a runtime-created object unreachable for data CRUD
7796+
// rather than merely un-listed. This branch has never carried the
7797+
// `environmentId` gate either, for the same reason.
77187798
this.applyObjectRegistryMutation(request);
77197799
return;
77207800
}
77217801
if (this.environmentId !== undefined) return;
77227802
try {
7723-
this.hydrateOverlayIntoRegistry(request.type, request.item, request.packageId ?? undefined);
7803+
this.hydrateOverlayIntoRegistry(request.type, request.item, {
7804+
packageId: request.packageId ?? undefined,
7805+
organizationId: request.organizationId,
7806+
});
77247807
} catch (err: any) {
77257808
// Best-effort, exactly like the object branch: the row is already
77267809
// persisted, so a registry hiccup must not fail the write that
@@ -8412,6 +8495,9 @@ export class ObjectStackProtocolImplementation implements
84128495
name: request.name,
84138496
item: request.item,
84148497
packageId: request.packageId ?? null,
8498+
// [#6602] The SAME scope the row was just written with —
8499+
// a per-org overlay stays out of the shared registry.
8500+
organizationId: orgId,
84158501
});
84168502
await this.ensureObjectStorage(request.type, request.name);
84178503
}
@@ -9043,6 +9129,8 @@ export class ObjectStackProtocolImplementation implements
90439129
name: args.name,
90449130
item: args.body,
90459131
packageId: args.packageId,
9132+
// [#6602] The promoted draft carries the org it was drafted in.
9133+
organizationId: args.orgId,
90469134
});
90479135
// Create the object's table now so it's CRUD-able without a restart.
90489136
await this.ensureObjectStorage(args.requestType, args.name);
@@ -10597,6 +10685,9 @@ export class ObjectStackProtocolImplementation implements
1059710685
name: request.name,
1059810686
item: result.item.body,
1059910687
packageId: rollbackPackageId,
10688+
// [#6602] A rollback restores the row IN ITS OWN SCOPE — an
10689+
// org-scoped restore must not graft the body process-wide.
10690+
organizationId: orgId,
1060010691
});
1060110692
return {
1060210693
success: true,
@@ -11188,10 +11279,21 @@ export class ObjectStackProtocolImplementation implements
1118811279
// When artifacts load after this hydration the merge
1118911280
// finds nothing and the row registers unchanged — same
1119011281
// as before, scoped or not.
11282+
//
11283+
// [#6602] The org argument states what the WHERE
11284+
// clause above already selected for. It is a no-op
11285+
// today by construction — and that is the point: the
11286+
// rule this branch's comment states ("hydrate only
11287+
// env-wide rows") stops depending on a query filter
11288+
// staying correct, because the hydrator refuses an
11289+
// org-scoped row whatever selected it.
1119111290
this.hydrateOverlayIntoRegistry(
1119211291
normalizedType,
1119311292
data,
11194-
(record as { package_id?: string | null }).package_id ?? undefined,
11293+
{
11294+
packageId: (record as { package_id?: string | null }).package_id ?? undefined,
11295+
organizationId: (record as { organization_id?: string | null }).organization_id ?? null,
11296+
},
1119511297
);
1119611298
}
1119711299
loaded++;

0 commit comments

Comments
 (0)