Skip to content

Commit d2b6fa0

Browse files
os-muskclaude
andauthored
fix(metadata-protocol): stop persisting derived provenance keys, and restate tenant authorship at hydration for every type (#16796)
* fix(metadata-protocol): stop persisting and re-reading derived provenance keys wip Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(objectql): pin both #16702 doors against a real ObjectQL engine Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * chore: changeset for the derived-provenance write door and hydration restatement Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(objectql): existing hydration pins now state tenant authorship Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(objectql): the new stub driver honours the caller's limit bound Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * chore(changeset): raise @objectstack/metadata-protocol to minor `check-changeset-no-major`'s level axis refuses a PR that declares `Clause-②: yes` while grading a package whose `packages/*/src/**` it moves as `patch`. Route 1 of the two the gate names: the declaration is right and the level is wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(objectql): pin door 1's type-agnostic scope on the stored `object` row F5 of the contract review on PR #16796. `stripDerivedProvenance` runs before `saveMetaItem`'s type branch, so an `object` body loses the same three keys at rest — the criterion-4 case wrote `_packageId` / `_provenance` into the request but read back only the hydrated entry, which door 2's restatement would answer `org` on its own even if the strip had skipped `object`. `_packageVersion` is now sent too, so all three keys the strip names are exercised rather than two. F2 of the same review, as a disclosure in the changeset: door 2 corrects the authorization verdict, not the served document, and the cloud#2069 backfill stays load-bearing for rows already poisoned at rest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ec5db7b commit d2b6fa0

6 files changed

Lines changed: 622 additions & 8 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/metadata-protocol': minor
3+
---
4+
5+
Stop persisting the caller's `_packageId` / `_packageVersion` / `_provenance`, and restate tenant authorship at hydration for every metadata type.
6+
7+
Two seams let a tenant lock themselves out of their own metadata. `saveMetaItem` persisted those three keys verbatim — `metadata-read-decorations.ts` deliberately does not strip `_provenance` from a served document, so the ordinary Studio `GET /meta/app/x` → edit → `PUT /meta/app/x` round trip wrote `_provenance: 'package'` into the tenant's own `sys_metadata` row. For every non-`object` type, boot and read-side hydration then registered that stored body as-is, so `SchemaRegistry.getArtifactItem`'s bare-key fallback accepted the overlay as a code artifact, `isArtifactBacked` turned true, and every later write was refused `NOT_OVERRIDABLE` — permanently, because the next boot re-derived the same verdict from the same row. The refusal said the item is "provided by a code package" when no code package published it at all.
8+
9+
Both halves are closed, because they cover different populations:
10+
11+
- `saveMetaItem` now drops exactly those three keys from the body it persists, so future writes stop poisoning the corpus. The `_lock*` family is deliberately untouched — a lock is author-declarable and dropping one is the fail-open direction.
12+
- `hydrateOverlayIntoRegistry` — the one choke point boot, read-side and write-through hydration already share — now states `_provenance: 'org'` on a copy before merging the artifact envelope, so rows already written become harmless without being rewritten. That also covers the column path: `getMetaItems` re-stamps `_packageId` onto the body from the row's `package_id` column, which the write-door strip cannot reach.
13+
14+
The three keys are read-side derived — `mergeArtifactProtection` recomputes them from the artifact on every read — so nothing an author wrote is lost and no accepted key or value changes. Where a real artifact exists its envelope still wins over both the stored copy and the restatement: ADR-0010 §3.3 precedence is unchanged, and an item genuinely shipped by a code package is still refused `NOT_OVERRIDABLE`.
15+
16+
One residual is deliberately left open: hydration corrects the AUTHORIZATION verdict, not the SERVED document. `getMetaItem` / `getMetaItems` return the overlay row's own body, and `mergeArtifactProtection` only fires where an artifact exists — so a row already poisoned at rest becomes editable again while `GET /meta/app/x` keeps serving `_provenance: 'package'` (and the UI keeps badging it as package-provided) until that row is re-saved through the write door or backfilled. The `objectstack-ai/cloud#2069` backfill stays load-bearing for that population; this change does not retire it.

packages/metadata-protocol/src/protocol.ts

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1172,6 +1172,84 @@ function mergeArtifactProtection(item: unknown, artifactItem: unknown): unknown
11721172
return out;
11731173
}
11741174

1175+
/**
1176+
* [#16702] ADR-0010 §3.3 — the three protection keys that are READ-SIDE
1177+
* DERIVED, and therefore must never be persisted from a caller's body.
1178+
*
1179+
* {@link mergeArtifactProtection} recomputes all three from the artifact on
1180+
* every read, so a copy stored inside a `sys_metadata` body is never
1181+
* load-bearing: removing it is observable ONLY where that copy was a lie.
1182+
*
1183+
* ⛔ The `_lock*` family is deliberately NOT here, though it shares the
1184+
* underscore spelling and the same ADR-0010 envelope. A lock is
1185+
* AUTHOR-DECLARABLE (`protection.lock`, translated into `_lock*` by
1186+
* `applyProtection`), so dropping one is the FAIL-OPEN direction — cloud PR
1187+
* #2065 drew that line at its own producer and this door follows it.
1188+
*
1189+
* ⛔ Nor is this a second `METADATA_READ_DECORATIONS`. That list is shared
1190+
* with every consumer that re-parses a SERVED document (`spec`'s
1191+
* `metadata-read-decorations.ts`), and its header states on purpose that the
1192+
* protection envelope stays on a served body so provenance survives a
1193+
* re-parse. This strip is scoped to the WRITE door alone.
1194+
*/
1195+
const DERIVED_PROVENANCE_KEYS = ['_packageId', '_packageVersion', '_provenance'] as const;
1196+
1197+
/**
1198+
* [#16702] Remove {@link DERIVED_PROVENANCE_KEYS} from a body about to be
1199+
* PERSISTED into `sys_metadata`.
1200+
*
1201+
* A **silent** strip, for the same reason {@link stripReadDecorations} is
1202+
* silent: the standard Studio `GET` → edit → `PUT` round-trip echoes whatever
1203+
* the served document carried, and refusing that round-trip would be hostile
1204+
* for keys the server stamped itself. What it restores is the invariant the
1205+
* `_provenance: 'org'` docblock in {@link
1206+
* ObjectStackProtocolImplementation.applyObjectRegistryMutation} already
1207+
* states: every row this door writes is tenant-authored by definition, so the
1208+
* server states that fact rather than reading it back from the caller.
1209+
*
1210+
* Returns the SAME reference when there is nothing to strip, so the common
1211+
* path allocates nothing. Non-object inputs pass through — the caller's own
1212+
* validation owns those.
1213+
*/
1214+
function stripDerivedProvenance(item: unknown): unknown {
1215+
if (!item || typeof item !== 'object' || Array.isArray(item)) return item;
1216+
const dict = item as Record<string, unknown>;
1217+
if (!DERIVED_PROVENANCE_KEYS.some((k) => k in dict)) return item;
1218+
const next = { ...dict };
1219+
for (const k of DERIVED_PROVENANCE_KEYS) delete next[k];
1220+
return next;
1221+
}
1222+
1223+
/**
1224+
* [#16702] State what every `sys_metadata` row IS — tenant-authored, ADR-0010
1225+
* `_provenance: 'org'` — on a COPY of a body about to be REGISTERED.
1226+
*
1227+
* The same sentence {@link
1228+
* ObjectStackProtocolImplementation.applyObjectRegistryMutation} and the boot
1229+
* `object` limb already write, said once for every OTHER type at the one
1230+
* hydration choke point they do not share. Without it the row's own bytes
1231+
* decide: `isCodeArtifactBody` accepts a truthy non-sentinel `_packageId` with
1232+
* non-`org` provenance, `SchemaRegistry.getArtifactItem`'s bare-key fallback
1233+
* returns the overlay AS an artifact, `isArtifactBacked` turns true, and
1234+
* `saveMetaItem`'s overlay gate refuses the tenant's next write to their own
1235+
* item with `NOT_OVERRIDABLE` — permanently, because the next boot re-derives
1236+
* the same verdict from the same row (cloud#970's shape, for non-`object`
1237+
* types).
1238+
*
1239+
* ⚠️ Its ONE caller applies it BEFORE {@link mergeArtifactProtection}, and the
1240+
* order is the whole contract: where a real artifact exists the artifact's
1241+
* envelope still overwrites `_provenance` (and `_packageId` /
1242+
* `_packageVersion` / `_lock*`) on the way out, so package protection is
1243+
* untouched — ADR-0010 §3.3 precedence is unchanged in both directions.
1244+
*
1245+
* On a COPY, always: `registerItem` hands the body to `applyProtection`, which
1246+
* mutates in place, and the callers own their `data`.
1247+
*/
1248+
function stateTenantAuthorship(data: unknown): unknown {
1249+
if (!data || typeof data !== 'object' || Array.isArray(data)) return data;
1250+
return { ...(data as Record<string, unknown>), _provenance: 'org' };
1251+
}
1252+
11751253
/**
11761254
* ADR-0048 (#1828) — composite dedup identity for the unscoped metadata list.
11771255
*
@@ -14050,7 +14128,19 @@ export class ObjectStackProtocolImplementation implements
1405014128
const registry: any = (this.engine as any)?.registry;
1405114129
if (!registry || typeof registry.registerItem !== 'function') return false;
1405214130
const artifact = this.lookupArtifactItem(type, (data as any).name, options.packageId ?? undefined);
14053-
registry.registerItem(type, mergeArtifactProtection(data, artifact), 'name' as any);
14131+
// [#16702] Say what this row IS before the artifact envelope is grafted
14132+
// on top of it. Every body reaching this hydrator came out of a
14133+
// `sys_metadata` write and is therefore tenant-authored by definition
14134+
// (ADR-0010 `_provenance: 'org'`) — the same sentence the `object`
14135+
// branches of this class already write, said once here for every OTHER
14136+
// type. It makes rows ALREADY poisoned at rest harmless without
14137+
// rewriting them, and it covers the column path the write-door strip
14138+
// cannot reach: `getMetaItems` re-stamps `_packageId` onto the body
14139+
// from the row's `package_id` COLUMN a few frames up, before handing it
14140+
// here. ⚠️ BEFORE the merge, never after — where a real artifact
14141+
// exists its envelope must still win (ADR-0010 §3.3), and it does,
14142+
// because {@link mergeArtifactProtection} overwrites `_provenance` last.
14143+
registry.registerItem(type, mergeArtifactProtection(stateTenantAuthorship(data), artifact), 'name' as any);
1405414144
this.hydrateExpandedViewItems(type, data, options, registry);
1405514145
return true;
1405614146
}
@@ -14911,6 +15001,21 @@ export class ObjectStackProtocolImplementation implements
1491115001
// Placed first so the destructive-change diff, the schema gate, the
1491215002
// authoring gate and the persisted body all see the same document.
1491315003
request.item = stripReadDecorations(request.item);
15004+
// [#16702] …and the three DERIVED protection keys, for the same reason
15005+
// one beat later. `metadata-read-decorations.ts` deliberately does NOT
15006+
// strip `_provenance` from a SERVED document (a served body must keep
15007+
// its provenance on re-parse), so the very same round-trip echoed
15008+
// `_packageId` / `_packageVersion` / `_provenance: 'package'` straight
15009+
// back into the tenant's own row — and for every non-`object` type the
15010+
// hydrated row's own bytes then decided it was a code artifact, closing
15011+
// the tenant out of their own item with `NOT_OVERRIDABLE` forever. The
15012+
// read side recomputes all three from the artifact on every read
15013+
// ({@link mergeArtifactProtection}), so nothing is lost by not storing
15014+
// them. See {@link DERIVED_PROVENANCE_KEYS} for why `_lock*` is NOT in
15015+
// the set. Placed alongside the decoration strip so the
15016+
// destructive-change diff, the schema gate, the authoring gate and the
15017+
// persisted body all still see one document.
15018+
request.item = stripDerivedProvenance(request.item);
1491415019
// [#6562] …and OUR OWN injected system columns, for the same reason and
1491515020
// at the same moment. `governServedItem` now serves the EFFECTIVE object
1491615021
// schema, so the very same Studio round-trip would otherwise persist

packages/objectql/src/plugin.integration.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,9 +1035,14 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => {
10351035

10361036
// Assert — items should be restored into the registry
10371037
const registry = (kernel.getService('objectql') as any).registry;
1038+
// [#16702] `_provenance: 'org'` is the SERVER's own sentence about every
1039+
// `sys_metadata` row, stated by the shared hydrator — the same one the
1040+
// `object` branch has always made. Kept as an exact-shape assertion so an
1041+
// UNEXPECTED extra key on a restored app still reds this pin.
10381042
expect(registry.getAllApps()).toContainEqual({
10391043
name: 'custom_crm',
10401044
label: 'Custom CRM',
1045+
_provenance: 'org',
10411046
});
10421047
});
10431048

packages/objectql/src/protocol-boot-hydration-scoped.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,18 @@ describe('loadMetaFromDb — ADR-0048 package-scoped protection graft at boot (#
135135
expect(direct._provenance).toBe('package');
136136
});
137137

138-
it('registers the row unchanged when artifacts have not loaded yet (boot-order no-op)', async () => {
138+
it('grafts NO artifact envelope when artifacts have not loaded yet (boot-order no-op)', async () => {
139139
// Empty registry at hydration time — the scoped lookup finds
140140
// nothing, exactly like the unscoped one did, and the row
141141
// registers without a grafted envelope. Artifact-after-hydration
142142
// boot orders are unaffected by the scoping.
143+
//
144+
// [#16702] What the row does carry is the SERVER's own sentence about
145+
// it — `_provenance: 'org'`, the same one the `object` branch has
146+
// always stated — because every row this hydrator sees came out of a
147+
// `sys_metadata` write. That is a statement of authorship, not a graft
148+
// from an artifact: `_lock` and `_packageId` stay absent, which is what
149+
// this case is about.
143150
const registry = new SchemaRegistry({ multiTenant: false });
144151
registry.logLevel = 'silent';
145152
const rows = [
@@ -159,7 +166,8 @@ describe('loadMetaFromDb — ADR-0048 package-scoped protection graft at boot (#
159166
expect(direct.label).toBe('B Home (customized)');
160167
expect(direct._lock).toBeUndefined();
161168
expect(direct._packageId).toBeUndefined();
162-
expect(direct._provenance).toBeUndefined();
169+
// [#16702] NOT `undefined` — the hydrator states tenant authorship.
170+
expect(direct._provenance).toBe('org');
163171
});
164172

165173
it('keeps the legacy best-effort graft for package-less (global) rows', async () => {

0 commit comments

Comments
 (0)