Skip to content

Commit bc03179

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol): fold the ADR-0010 lock gate's type key at its producer (#9161)
* fix(metadata-protocol): fold getEffectiveLock's type key at the producer (#9009) `getEffectiveLock` handed `type` to its two limbs verbatim and they did not read it the same way: the artifact limb folded, the overlay limb queried `sys_metadata` with the raw spelling. A miss there falls through to `lock: 'none'` — the verdict "the author declared no protection" — so an ADR-0010 `_lock` was addressable around from the wire. Fold once with `canonicalMetaType` and use that key for both limbs. No reachable caller changes behaviour; a future unfolded one is now refused instead of admitted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj * test(metadata-protocol): record the reverse-verification counts as measured (#9009) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8914915 commit bc03179

3 files changed

Lines changed: 384 additions & 13 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
Fold the metadata lock gate's type key at its producer, so an ADR-0010 `_lock` can no longer be addressed around by spelling.
6+
7+
`getEffectiveLock` handed `type` to its two limbs verbatim, and the limbs did not read it the same way: the artifact limb folded (`lookupArtifactItem` resolves the singular and retries the raw spelling), while the overlay limb queried `sys_metadata` with the raw `type`. Since `SysMetadataRepository` stores rows under the canonical spelling with no at-rest fallback, a non-canonical `type` missed the stored active row and the gate fell through to `lock: 'none'` — not a neutral value but the verdict "the author declared no protection", which `evaluateLockForWrite` / `evaluateLockForDelete` turn into "allow".
8+
9+
`getEffectiveLock` now folds once with `canonicalMetaType` and uses that one key for both limbs. Nothing changes for any caller reachable today — `saveMetaItem`, `deleteMetaItem`, `rollbackMetaItem`, `publishMetaItem` and `publishPackageDrafts` all fold their own request first, which was measured rather than assumed. What changes is the failure mode of a future caller that does *not* fold: the lock is now found, and the write is refused instead of silently admitted.
10+
11+
The fold uses the URL map rather than the manifest-collection map on purpose — the latter omits `field`, `seed`, `external_catalog` and `translation`, i.e. it would canonicalize the types that never needed it and leave the four that do.
Lines changed: 301 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,301 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#9009] The ADR-0010 lock gate folds its OWN type key — the class closed at
5+
* the producer, not one entry point at a time.
6+
*
7+
* ---------------------------------------------------------------------------
8+
* The seam
9+
* ---------------------------------------------------------------------------
10+
* `getEffectiveLock` has two limbs and they did not read `type` the same way:
11+
*
12+
* artifact limb `lookupArtifactItem` → `PLURAL_TO_SINGULAR[type] ?? type`,
13+
* then the raw spelling as a second lookup. FOLDED.
14+
* overlay limb `engine.findOne('sys_metadata', { where: { type, … } })`.
15+
* RAW — and `SysMetadataRepository.whereFor` emits the
16+
* CANONICAL spelling with no at-rest fallback, so the stored
17+
* active row lives under a `type` this query never asked for.
18+
*
19+
* A miss on that query falls through to `lock: 'none'`, which is not a neutral
20+
* placeholder: it is the verdict "the author declared no protection" (#5706),
21+
* and `evaluateLockForWrite` / `evaluateLockForDelete` turn it straight into
22+
* "allow". So an `_lock` was addressable AROUND by spelling the type
23+
* differently, and the gate answered "unlocked" with complete confidence.
24+
*
25+
* #8769 closed that on `publishMetaItem` and #8819 on `rollbackMetaItem`, each
26+
* by folding ITS OWN request — and `rollbackMetaItem`'s comment said so, then
27+
* promised a card for the producer that was never filed. This is that fold.
28+
*
29+
* ---------------------------------------------------------------------------
30+
* Why these tests call the gate directly, and why that is the honest level
31+
* ---------------------------------------------------------------------------
32+
* Every live caller folds first — measured before the fix was written:
33+
* `saveMetaItem`, `deleteMetaItem`, `rollbackMetaItem` and `publishMetaItem`
34+
* all run `canonicalizeMetaRequestType` at their top, and `publishPackageDrafts`
35+
* folds `d.type` with `canonicalMetaType` before `promoteDraftForPublish`. There
36+
* is therefore NO wire request that reaches the gate unfolded: a plural is
37+
* refused at the boundary by `metaUrlSpellingRefusal` (400) long before it gets
38+
* here. Driving this through a route would measure the boundary fold a second
39+
* time and say nothing at all about the producer.
40+
*
41+
* The subject is the gate's own contract — "callers must fold", an invariant
42+
* that lived in no type, no signature and no assertion — so the gate is where
43+
* these tests address it. That is the same reachability discipline #8820
44+
* landed on: trace to each caller's PRODUCER, and put the guard where the
45+
* invariant is, not where today's traffic happens to enter.
46+
*
47+
* ---------------------------------------------------------------------------
48+
* Reverse verification, direction predicted BEFORE running
49+
* ---------------------------------------------------------------------------
50+
* Ordinary red. Restoring the two raw `type` reads
51+
* (`lookupArtifactItem(type, name)` and `where: { type, … }`) must turn the
52+
* folding cases red and leave every control green:
53+
*
54+
* plural, manifest-PRESENT (`views`) full/overlay → 'none' → RED
55+
* the overlay query's own `where.type` 'view' → 'views' → RED
56+
* plural, manifest-ABSENT (`translations`) full/overlay → 'none' → RED
57+
* write refusal envelope on a plural 403 → null (allowed) → RED
58+
* delete gate on a plural 500 loud → null (allowed)→ RED
59+
* canonical spelling, locked unchanged → GREEN
60+
* canonical spelling, genuine miss unchanged → GREEN
61+
* ARTIFACT lock under a plural unchanged → GREEN
62+
* canonical, locked delete unchanged → GREEN
63+
*
64+
* The third green is the one that carries an argument rather than a count: the
65+
* artifact limb already folded, so it was never the hole — and a "fix" that
66+
* changed its answer would be changing something that was already right.
67+
*
68+
* Predicted 5 red / 4 green; measured 5 red / 4 green, each red for its
69+
* predicted reason rather than merely in the predicted count —
70+
* `expected 'none' to be 'full'` twice (the two plural classes, one per map),
71+
* `expected { type: 'views', … } to match object { type: 'view', … }` (the
72+
* query itself), `expected null not to be null` (no refusal was produced at
73+
* all) and `expected a rejection, but the call resolved with null` (the delete
74+
* gate admitted it). Quoted in the PR body as taken.
75+
*/
76+
77+
import { describe, it, expect, vi } from 'vitest';
78+
import { ObjectStackProtocolImplementation } from './protocol.js';
79+
80+
/**
81+
* An overlay row exactly as `SysMetadataRepository` stores it: keyed by the
82+
* CANONICAL `type`, never by whatever spelling a caller used.
83+
*/
84+
function overlayRow(type: string, name: string, lock: string) {
85+
return {
86+
id: `row-${type}-${name}`,
87+
type,
88+
name,
89+
state: 'active',
90+
organization_id: null,
91+
package_id: null,
92+
checksum: 'sha256:stored-head',
93+
metadata: JSON.stringify({ name, _lock: lock, _lockReason: 'governed by ops' }),
94+
};
95+
}
96+
97+
type Harness = {
98+
engine: any;
99+
/** Every `sys_metadata` where-clause the gate issued, in order. */
100+
overlayQueries: Array<Record<string, unknown>>;
101+
};
102+
103+
/**
104+
* A store that answers ONLY the key it actually holds — which is the whole
105+
* point. A double that matched on `name` alone would resolve the row under any
106+
* spelling and erase the defect in the harness (#7743: a gate is proven where
107+
* it is exercised and absent where it is used).
108+
*
109+
* `registryItems` is keyed `'type/name'` for the same reason: the artifact limb
110+
* must be measurable as having resolved under one spelling and not another.
111+
*/
112+
function harness(opts: {
113+
rows?: Array<{ type: string; name: string; lock: string }>;
114+
registryItems?: Record<string, unknown>;
115+
}): Harness {
116+
const rows = (opts.rows ?? []).map((r) => overlayRow(r.type, r.name, r.lock));
117+
const items = opts.registryItems ?? {};
118+
const overlayQueries: Array<Record<string, unknown>> = [];
119+
120+
const engine: any = {
121+
registry: {
122+
getObject: () => undefined,
123+
getItem: (type: string, name: string) => items[`${type}/${name}`],
124+
listItems: () => [],
125+
applyNavContributions: (x: unknown) => x,
126+
isPackageDisabled: () => false,
127+
getObjectOwner: () => undefined,
128+
},
129+
findOne: vi.fn(async (object: string, query: any) => {
130+
if (object !== 'sys_metadata') return null;
131+
const where = (query?.where ?? {}) as Record<string, unknown>;
132+
overlayQueries.push(where);
133+
return rows.find((r) =>
134+
r.type === where['type']
135+
&& r.name === where['name']
136+
&& r.state === where['state']) ?? null;
137+
}),
138+
find: vi.fn(async () => []),
139+
insert: vi.fn(async (_object: string, values: any) => ({ id: 'inserted', ...values })),
140+
count: vi.fn(async () => 0),
141+
execute: vi.fn(async () => ({})),
142+
getObjectSchema: vi.fn(async () => undefined),
143+
};
144+
return { engine, overlayQueries };
145+
}
146+
147+
/** Tenant scope — the control plane (`environmentId` undefined) skips the gate. */
148+
function protocolFor(h: Harness) {
149+
return new ObjectStackProtocolImplementation(h.engine, undefined, 'env_1') as any;
150+
}
151+
152+
/** Capture a rejection without letting a resolve pass silently. */
153+
async function rejection(run: () => Promise<unknown>): Promise<any> {
154+
let caught: any;
155+
let resolved: unknown;
156+
let didResolve = false;
157+
try {
158+
resolved = await run();
159+
didResolve = true;
160+
} catch (e) {
161+
caught = e;
162+
}
163+
expect(
164+
didResolve,
165+
`expected a rejection, but the call resolved with ${JSON.stringify(resolved)}`,
166+
).toBe(false);
167+
return caught;
168+
}
169+
170+
describe('[#9009] getEffectiveLock resolves the overlay row under the canonical type', () => {
171+
it('finds the lock when handed a manifest-PRESENT plural (`views`)', async () => {
172+
const h = harness({ rows: [{ type: 'view', name: 'v1', lock: 'full' }] });
173+
174+
const state = await protocolFor(h).getEffectiveLock('views', 'v1', null);
175+
176+
// The regression, verbatim: this used to answer `'none'` — "the author
177+
// declared no protection" — for a row that declares `_lock: 'full'`.
178+
expect(state.lock).toBe('full');
179+
expect(state.lockSource).toBe('overlay');
180+
expect(state.lockReason).toBe('governed by ops');
181+
});
182+
183+
it('addresses `sys_metadata` by the canonical key, not the caller spelling', async () => {
184+
const h = harness({ rows: [{ type: 'view', name: 'v1', lock: 'full' }] });
185+
186+
await protocolFor(h).getEffectiveLock('views', 'v1', null);
187+
188+
// The direct measurement of the fold: what the query ASKED FOR, rather
189+
// than what it happened to get back.
190+
expect(h.overlayQueries).toHaveLength(1);
191+
expect(h.overlayQueries[0]).toMatchObject({
192+
type: 'view',
193+
name: 'v1',
194+
state: 'active',
195+
organization_id: null,
196+
});
197+
});
198+
199+
it('finds the lock for a manifest-ABSENT plural (`translations`) too', async () => {
200+
// The class `PLURAL_TO_SINGULAR` structurally cannot cover: `field`,
201+
// `seed`, `external_catalog` and `translation` are not stack
202+
// collections, so the manifest map omits them and a fold written
203+
// against THAT map would leave exactly these four unfolded — the
204+
// "tolerant AND INCOMPLETE" shape #8908 rejected. `canonicalMetaType`
205+
// reads the URL map, which carries them.
206+
const h = harness({ rows: [{ type: 'translation', name: 't1', lock: 'full' }] });
207+
208+
const state = await protocolFor(h).getEffectiveLock('translations', 't1', null);
209+
210+
expect(state.lock).toBe('full');
211+
expect(state.lockSource).toBe('overlay');
212+
expect(h.overlayQueries[0]).toMatchObject({ type: 'translation', name: 't1' });
213+
});
214+
215+
it('the canonical spelling is unchanged — the control the fold must not move', async () => {
216+
const h = harness({ rows: [{ type: 'view', name: 'v1', lock: 'full' }] });
217+
218+
const state = await protocolFor(h).getEffectiveLock('view', 'v1', null);
219+
220+
expect(state.lock).toBe('full');
221+
expect(state.lockSource).toBe('overlay');
222+
expect(h.overlayQueries[0]).toMatchObject({ type: 'view' });
223+
});
224+
225+
it('a genuine miss still answers `none` — folding must not refuse everything', async () => {
226+
// Without this, "the gate is fail-closed" would be satisfiable by a gate
227+
// that locks every item in the deployment.
228+
const h = harness({ rows: [{ type: 'view', name: 'v1', lock: 'full' }] });
229+
230+
const state = await protocolFor(h).getEffectiveLock('view', 'v_unlocked', null);
231+
232+
expect(state.lock).toBe('none');
233+
expect(state.lockSource).toBeUndefined();
234+
});
235+
236+
it('the ARTIFACT limb was never the hole — and still answers the same', async () => {
237+
// Green BEFORE and AFTER, deliberately. `lookupArtifactItem` already
238+
// folded through the manifest map and then retried the raw spelling, so
239+
// a packaged `_lock` resolved under a plural all along. Pinning it keeps
240+
// the fix from being credited with something that was already right —
241+
// and pins the ASYMMETRY that made the two limbs able to disagree.
242+
const h = harness({
243+
registryItems: {
244+
'view/v_pkg': { name: 'v_pkg', _packageId: 'pkg-a', _lock: 'full', _lockReason: 'shipped locked' },
245+
},
246+
});
247+
248+
const state = await protocolFor(h).getEffectiveLock('views', 'v_pkg', null);
249+
250+
expect(state.lock).toBe('full');
251+
expect(state.lockSource).toBe('artifact');
252+
// The overlay limb is never reached when the artifact wins.
253+
expect(h.overlayQueries).toHaveLength(0);
254+
});
255+
});
256+
257+
describe('[#9009] the write gate refuses an unfolded caller instead of admitting it', () => {
258+
it('answers the ADR-0112 `ITEM_LOCKED` envelope for a plural-addressed write', async () => {
259+
const h = harness({ rows: [{ type: 'view', name: 'v1', lock: 'full' }] });
260+
261+
const refusal = await protocolFor(h).lockWriteRefusal({
262+
type: 'views', name: 'v1', operation: 'save',
263+
});
264+
265+
// Pre-fix this was `null` — no refusal at all, and the save proceeded.
266+
expect(refusal).not.toBeNull();
267+
expect((refusal.err as any).code).toBe('ITEM_LOCKED');
268+
expect((refusal.err as any).status).toBe(403);
269+
expect((refusal.err as any).lock).toBe('full');
270+
expect(refusal.audit.outcome).toBe('denied');
271+
});
272+
273+
it('the delete gate now fails LOUDLY on an unfolded caller, not silently open', async () => {
274+
// What the fold does to the one gate that writes its audit row where the
275+
// verdict is reached: `getEffectiveLock` finds the lock, and the refusal
276+
// is then reported under the CALLER's spelling — which
277+
// `recordMetadataAudit` refuses outright (#8908's assert, deliberately
278+
// NOT softened by folding the ledger key here as well).
279+
//
280+
// So the unfolded caller gets a 500 it cannot miss, where before the fix
281+
// it got `null` and the delete went through. Both are refusals of the
282+
// delete; only one of them is visible.
283+
const h = harness({ rows: [{ type: 'view', name: 'v1', lock: 'full' }] });
284+
285+
const caught = await rejection(() => protocolFor(h).assertLockAllowsDelete({
286+
type: 'views', name: 'v1',
287+
}));
288+
289+
expect(caught?.code).toBe('AUDIT_TYPE_NOT_CANONICAL');
290+
expect(caught?.status).toBe(500);
291+
});
292+
293+
it('a canonical, locked delete is refused exactly as before', async () => {
294+
const h = harness({ rows: [{ type: 'view', name: 'v1', lock: 'full' }] });
295+
296+
const err = await protocolFor(h).assertLockAllowsDelete({ type: 'view', name: 'v1' });
297+
298+
expect((err as any)?.code).toBe('ITEM_LOCKED');
299+
expect((err as any)?.status).toBe(403);
300+
});
301+
});

0 commit comments

Comments
 (0)