Skip to content

fix(metadata-protocol): let an org-scoped caller see env-wide sys_metadata rows (#7819 tier 2)#7871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
huangyiirene merged 1 commit into from
Aug 12, 2026
Merged

fix(metadata-protocol): let an org-scoped caller see env-wide sys_metadata rows (#7819 tier 2) #7871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
136 changes: 136 additions & 0 deletions .changeset/duplicate-adopt-org-scope.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
---
"@objectstack/metadata-protocol": patch
---

fix(metadata-protocol): let an org-scoped caller see env-wide `sys_metadata` rows in `duplicatePackage` / `reassignOrphanedMetadata` (#7819, tier 2)

Both methods scanned `sys_metadata` with a strict `organization_id` equality:

```ts
if (request.organizationId) where.organization_id = request.organizationId;
```

`organization_id = 'org'` matches no row whose column is NULL, so an org-scoped
caller could not see any row recorded env-wide. Both scans now accept org-scoped
**or** env-wide rows — the same `$or` `deletePackage` (#7705), `listCommits`
(#7779) and the tier-1 sites (#7857) already carry.

## These two were filed UNVERIFIED, so step one was a measurement

#7819 carried four sites. Tier 1 shipped on measured evidence; these two were a
grep match with a plausible mechanism, on a **different table** (`sys_metadata`,
not `sys_metadata_commit`) with callers nobody had driven. "Latent, not live"
would have been a complete outcome and no fix. Reachability was checked on a
real engine before a line was edited, and both halves came back live:

1. **A caller passes an org.** One production caller each, both in
`packages/runtime/src/domains/packages.ts`: `POST /packages/:id/duplicate`
and `POST /packages/:id/adopt-orphans`, each forwarding
`resolveActiveOrganizationId` — the same door tier 1 measured.
2. **Env-wide rows exist in that table.** Not incidentally: a `saveMetaItem`
from a session with no active org writes `organization_id = NULL`, and
`resolveActiveOrganizationId` answers `undefined` both for such a session and
for any throw on the auth seam. For the orphan site, a `saveMetaItem` naming
**no package at all still succeeds today** and lands `package_id = null,
organization_id = null` — the current write path mints exactly the orphan the
scan could not see, so that population is live rather than the legacy residue
the docstring can be read as describing.

Both projected symptoms then reproduced, and both were worse than projected.

## `duplicatePackage` — a partial copy reporting success, and a copy wired back to its source

Measured before the fix: a source package holding one env-wide row and one
org-scoped row, duplicated by an org caller, answered
`{success: true, copiedCount: 1, failedCount: 0}`.

The sharper consequence is the **rename map**, which is built only from the rows
the scan returns. With the env-wide `object` rows missing it came out empty, so a
copied view was renamed `iojn2_list` while its `data.object` still read
`iojn_widget` — a duplicate silently wired back to the base it was cloned from,
reporting success. An all-env-wide source degraded just as quietly the other way:
`{success: false, copiedCount: 0, failedCount: 0}`, nothing copied and nothing
named as failed.

### Widening the scan alone was **not** a fix

With the scan widened and the write left as it was, the object copy landed in
`failed[]` with `NOT_OVERRIDABLE`: `object` is declared `allowOrgOverride=false`,
so stamping the request's org onto the copy is refused — boot hydration loads
env-wide rows only, and an org-scoped `object` row would vanish on the next
restart (ADR-0005, #6190).

Since an `object` therefore **cannot exist org-scoped**, every object row in a
source package is env-wide, and an org-scoped `duplicatePackage` could never copy
a single one. Objects being what a base is mostly made of, ADR-0070 D4's
"duplicate base" gesture was structurally unable to duplicate a base whenever an
org was active — a larger defect than the card projected.

So each copy now lands in **the scope of the row it came from**, not the
request's: the same rule #7559 gave `revertCommit`, for the same stated reason —
this loop now processes a batch that "legitimately mixes an env-wide artifact
with an org overlay". Scoped to the org-scoped door alone; with no
`organizationId` every copy is still written env-wide exactly as before.

### One hazard this fix introduces rather than inherits

Widening the scan makes a collision newly possible: an item can now appear twice,
as an env-wide row **plus** this org's overlay of it. Both copies would land on
the same target key (`type, name, organization_id, COALESCE(package_id, '')`), so
the surviving body would be decided by driver row order. The caller's own org now
shadows env-wide — ADR-0005 overlay precedence, the same order
`resolveMetaItemOrgScope` applies — and that is pinned as its own case.

## `reassignOrphanedMetadata` — the sharper member

Measured before the fix: two orphans, one env-wide and one org-scoped, adopted by
an org caller answered `{success: true, reassignedCount: 1}`, leaving the
env-wide orphan at `package_id = null` with nothing reporting it skipped.
**Finding orphans is this method's entire purpose**, so a class of orphan it
structurally cannot see is a wrong answer, not a partial one.

ADR-0070 D5 settles the scope question the widening raises (an org-scoped caller
now rebinds rows every org can see): the unit is explicitly the **environment** —
"bulk-assign legacy orphans to a default base named for the environment",
completing when "an environment has no orphans" — in a deployment model whose own
words are "there is no per-org overlay dimension here… the relevant axis is code
package vs writable base, not 'org'". Under the model this method was designed
for, every orphan is env-wide, so the strict equality made it **inert** for an
org-scoped caller in precisely that deployment.

## The no-org branch is deliberately NOT narrowed

On both sites, exactly as #7705, #7779 and tier 1 left theirs. The exposure is
worst at `reassignOrphanedMetadata`, whose no-org `where` is `{}` and already
scans every organization's rows; narrowing either door to `organization_id IS
NULL` would re-create this bug pointed the other way. Both doors are pinned as
they stand so they cannot drift silently. Whether the orphan door *should* be
that wide is #7780's open product question — a maintainer call, not decided here.

## Pin

`packages/runtime/src/package-duplicate-adopt-org-scope.integration.test.ts` — a
real `ObjectQL` over a real `SqlDriver` on better-sqlite3, seeded through the real
publish path, because the question is whether `organization_id = 'org'` matches a
NULL column: a property of the driver's SQL, not of a stub's `filter()`. Every
existing suite over these two methods either stubs `engine.find`
(`packages/objectql/src/protocol-package-lifecycle.test.ts`) or never passes an
org (the ADR-0070 dogfood), which is exactly why none could see this family. It
lives in `packages/runtime` because `metadata-protocol` cannot import `objectql`
(dependency cycle).

Twelve cases: the premise measured out of SQLite; the live-orphan producer; the
positive for each site; the reference-rewrite consequence; the org-shadows-env
precedence; both negative directions per site (another organization's rows,
another package's rows, owned rows); and the no-org door on each site.

**Reverse verification**, direction predicted before running: restoring the strict
equality turns red exactly the two positives, the reference-rewrite case, and the
two orphan cases that assert the env-wide orphan is adopted — five — leaving the
negative directions and both no-org doors green, since strict equality is
*narrower* than the `$or`. Measured: **5 failed | 7 passed**, exactly those five.

⚠️ These suites resolve `@objectstack/metadata-protocol` through its **`dist`**
and source-map traces back to `src`, so a source-only revert measures nothing
while looking like it measured something. The package was rebuilt between every
measurement above.
135 changes: 131 additions & 4 deletions packages/metadata-protocol/src/protocol.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11789,8 +11789,67 @@ export class ObjectStackProtocolImplementation implements
request.targetNamespace ?? (request.targetPackageId.split('.').pop() ?? request.targetPackageId);

const where: Record<string, unknown> = { package_id: request.sourcePackageId, state: 'active' };
if (request.organizationId) where.organization_id = request.organizationId;
const rows = (await this.engine.find('sys_metadata', { where })) as any[];
// [#7819 tier 2] Copy the source's env-wide (`organization_id IS NULL`)
// rows too, not just the ones this org happens to own — the same `$or`
// {@link deletePackage} (#7705) and {@link listCommits} (#7779) carry.
// Unlike the tier-1 sites this really is plain scan scoping (`where` is
// keyed on package + state, not on `id`), so the family remedy applies
// without their authorization question.
//
// Measured on a real driver before the fix: a source package holding one
// env-wide row and one org-scoped row duplicated by an org caller
// answered `{success: true, copiedCount: 1, failedCount: 0}` — a PARTIAL
// copy reported as a whole one, because `organization_id = <org>` matches
// no NULL column. The mixed state is ordinary, not contrived: a publish
// made before an active org was selected lands its `sys_metadata` row
// env-wide (`saveMetaItem` writes `organization_id = NULL`), and
// `resolveActiveOrganizationId` yields `undefined` for such a session
// *and* for any throw on the auth seam.
//
// The sharper consequence is the rename map below, which is built ONLY
// from the rows this scan returns. With the env-wide OBJECT rows missing
// it came out empty, so a copied view was renamed `iojn2_list` while its
// `data.object` still pointed at the SOURCE package's `iojn_widget` — a
// duplicate silently wired back to the base it was cloned from, reporting
// success. An all-env-wide source degraded differently and just as
// quietly: `{success: false, copiedCount: 0, failedCount: 0}`, nothing
// copied and nothing named as failed.
//
// The no-org branch is deliberately NOT narrowed to `organization_id IS
// NULL`, exactly as #7705 / #7779 / tier 1 left theirs: that door copies
// every scope today, and restricting it to env-wide rows would drop every
// org-scoped row from the copy — the same bug pointed the other way.
if (request.organizationId) {
where.$or = [
{ organization_id: request.organizationId },
{ organization_id: null },
];
}
const scanned = (await this.engine.find('sys_metadata', { where })) as any[];

// [#7819 tier 2] ADR-0005 overlay precedence — the caller's OWN org
// shadows env-wide ({@link resolveMetaItemOrgScope} states the same rule
// for history lineages). Widening the scan makes a collision newly
// possible that could not occur while it was a strict equality: one item
// can now appear TWICE, as an env-wide row PLUS this org's overlay of it.
// Every copy is written under `request.organizationId`, so both would
// land on the same target key — overlay uniqueness is
// `(type, name, organization_id, COALESCE(package_id, ''))` — and which
// body survived would be decided by driver row order. Keep the org
// overlay: it is what this caller already reads everywhere else.
let rows = scanned;
if (request.organizationId) {
const byKey = new Map<string, any>();
for (const row of scanned) {
const key = `${row?.type}\u0000${row?.name}`;
const kept = byKey.get(key);
const keptIsEnvWide = kept != null && (kept.organization_id ?? null) === null;
if (kept == null || (keptIsEnvWide && (row?.organization_id ?? null) !== null)) {
byKey.set(key, row);
}
}
rows = [...byKey.values()];
}

// Map only OBJECT names that carry the source namespace prefix; views/etc.
// are renamed by the same prefix swap and reference-rewritten via the map.
Expand DownExpand Up@@ -11932,14 +11991,44 @@ export class ObjectStackProtocolImplementation implements
}
const rewritten = deepRewrite(item);
if (rewritten && typeof rewritten === 'object' && !Array.isArray(rewritten)) rewritten.name = newName;
// [#7819 tier 2] The copy lands in the SCOPE OF THE ROW IT CAME
// FROM, not the request's — the same rule #7559 gave `revertCommit`
// ({@link resolveMetaItemOrgScope}) for the same reason, now that
// widening the scan above means this loop, too, processes a batch
// that "legitimately mixes an env-wide artifact with an org
// overlay".
//
// Not cosmetic: without it the read fix alone cannot produce a
// working duplicate. Stamping the request's org on every copy is
// REFUSED for any type the metadata-type registry declares
// `allowOrgOverride=false` — `object` among them — with
// `NOT_OVERRIDABLE`, because boot hydration loads env-wide rows
// only and an org-scoped `object` row would vanish on the next
// restart (ADR-0005, #6190). Since an `object` therefore CANNOT
// exist org-scoped, every object row in a source package is
// env-wide, and an org-scoped `duplicatePackage` could not copy a
// single one: before this card the strict equality hid them, and
// with only the scan widened they would land in `failed[]`
// instead. Objects being what a base is mostly made of, ADR-0070
// D4's "duplicate base" gesture was structurally unable to
// duplicate a base whenever an org was active.
//
// Scoped to the org-scoped door alone. With no `organizationId` on
// the request the scan returns every organization's rows and each
// copy is written env-wide exactly as before — that door's
// behaviour is deliberately left byte-identical, as this card
// leaves all of its no-org branches.
const copyOrgId: string | null = request.organizationId
? ((row?.organization_id ?? null) as string | null)
: null;
try {
await this.saveMetaItem({
type: row.type,
name: newName,
item: rewritten,
mode: 'publish',
packageId: request.targetPackageId,
...(request.organizationId ? { organizationId: request.organizationId } : {}),
...(copyOrgId ? { organizationId: copyOrgId } : {}),
...(request.actor ? { actor: request.actor } : {}),
});
copied.push({ type: row.type, name: newName });
Expand DownExpand Up@@ -11978,7 +12067,45 @@ export class ObjectStackProtocolImplementation implements
targetPackageId: string;
}> {
const where: Record<string, unknown> = {};
if (request.organizationId) where.organization_id = request.organizationId;
// [#7819 tier 2] See env-wide (`organization_id IS NULL`) orphans too.
// This is the sharper member of the family, because FINDING ORPHANS IS
// THE ENTIRE PURPOSE of this method: a class of orphan it structurally
// cannot see is not a partial answer, it is a wrong one. Measured on a
// real driver before the fix — two orphans, one env-wide and one
// org-scoped, adopted by an org caller: `{success: true,
// reassignedCount: 1}`, with the env-wide orphan left at
// `package_id = null` and nothing reporting that it was skipped.
//
// Not a legacy-only population, which is what makes this live rather
// than latent. The docstring above calls orphans a pre-package-first
// residue, and ADR-0070 D1 does reject NEW orphans that name a
// read-only package (`WRITABLE_PACKAGE_REQUIRED`) — but a
// `saveMetaItem` that names NO package at all still succeeds today and
// lands `package_id = null, organization_id = null`, i.e. the current
// write path mints exactly the orphan this scan could not see.
//
// ADR-0070 D5 settles the scope question this widening raises (an
// org-scoped caller now rebinds rows every org can see): the unit is
// explicitly the ENVIRONMENT — "bulk-assign legacy orphans to a default
// base named for the environment", completing when "an environment has
// no orphans", in a deployment model whose own words are "there is no
// per-org overlay dimension here… the relevant axis is code package vs
// writable base, not 'org'". Under that model every orphan is env-wide,
// so the strict equality made this method inert for an org-scoped
// caller in precisely the deployment it was designed for.
//
// ⛔ The no-org branch stays `{}` — deliberately un-narrowed, and this
// is the exposure the card flagged as worst: that door already scans
// EVERY organization's rows. Narrowing it to `organization_id IS NULL`
// would re-create this same bug pointed the other way. Whether that
// door should be that wide is #7780's open product question, which is
// a maintainer call and explicitly NOT decided here.
if (request.organizationId) {
where.$or = [
{ organization_id: request.organizationId },
{ organization_id: null },
];
}
const rows = (await this.engine.find('sys_metadata', { where })) as any[];
const orphans = rows.filter(
(r) => r?.package_id == null || r.package_id === '' || r.package_id === 'sys_metadata',
Expand Down
Loading
Loading