Skip to content

Commit 1e79aa4

Browse files
Sam Altmanclaude
andauthored
test(security): probe the trash/restore door for package-declared permission sets — pin the unreachability, measure the residual (#12021)
* test(security): pin the permission-set restore leg — the write door the packaged-set lock does not cover Probe deliverable, not a fix. The 2026-08-24 "lock the base, clone to customize" ruling is enforced on the write-through's insert and update legs; its author named the restore leg as the one write point it does not guard, and that leg re-authors a restored record's body into metadata with no provenance check and catches rather than throws. Four cases, in the order they answer the question: a CONTROL that identifies the gate answering an update (the lock, by its own message — ADR-0005's tier gate answers the same code for the same row), the MEASUREMENT of the restore leg on an identical fixture, the FENCE explaining why it is not a live defect (the delete leg resets an artifact-backed definition and never reaches the driver, so no restorable row exists), and the BLIND SPOT in the outer two-doors gate that would otherwise pre-empt it (it keys on `managed_by`, which the lock module records as measurably the wrong fact). The measurement case pins the RESIDUAL, deliberately: extending the lock to this leg must invert it in the same PR, which is what makes the extension visible instead of silent. Not re-pinned here (one spelling): that `restore` is not an engine middleware dispatch verb (objectql's #7809 vocabulary weld) and that it never derives as an API operation (spec's api-derivation suite). Those two are what make the leg unreachable today. Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(security): pin the restore-leg fake engine to the producer's update dispatch The double's `update()` now opens with `assertEngineUpdateDispatch(data, options)` and its `find()` REFUSES the operator shapes it does not implement, so a fixture that drifts to a call shape `ObjectQL` would reject fails loudly instead of quietly matching nothing. `tryUpdate` in the subject swallows a throw, so a double looser than the engine would have been invisible twice over. `scripts/engine-double-contract.pinned.json` learns the new pinned coverage (additive; the shrink-only baseline is untouched). Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1394768 commit 1e79aa4

2 files changed

Lines changed: 361 additions & 0 deletions

File tree

Lines changed: 356 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,356 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* THE RESTORE LEG — the one write point of `createPermissionSetWriteThrough`
5+
* that the 2026-08-24 "lock the base, clone to customize" ruling does NOT
6+
* guard, pinned as a MEASUREMENT plus a reachability fence.
7+
*
8+
* The lock (`packaged-permission-set-lock.ts`, wired into the `insert` and
9+
* `update` legs of `permission-set-projection.ts`) refuses a save that targets
10+
* a package-declared permission set. Its author named the gap rather than
11+
* leaving it to be found: the `restore` leg re-authors a restored record's
12+
* body into metadata with no provenance check at all, and — unlike its
13+
* neighbours — it CATCHES rather than throws, because it runs after the engine
14+
* has already un-trashed the row.
15+
*
16+
* ## What this file pins, and what it deliberately does not
17+
*
18+
* It pins four facts, in the order they answer the question:
19+
*
20+
* 1. ⭐ CONTROL — the harness reaches the LOCK, and the lock is what answers.
21+
* An `update` on an artifact-backed name is refused with `NOT_OVERRIDABLE`
22+
* / 403 AND the lock's own message (it names the clone path). The code
23+
* alone would not identify the gate: ADR-0005's tier gate inside
24+
* `saveMetaItem` answers with the same code for the same row, and its
25+
* message instead says the type "has not opted into per-org overlay
26+
* writes". Without this leg, case 2 below could pass for the wrong reason
27+
* — a probe answered by a gate that is not the one under test.
28+
*
29+
* 2. THE MEASUREMENT — the same fixture, the same set, through `restore`:
30+
* the body is re-authored, no refusal is raised, and the definition of a
31+
* package-declared set lands in the environment overlay store.
32+
*
33+
* ⚠️ This case pins the RESIDUAL, not a desired behaviour. The follow-up
34+
* that extends the lock to this leg MUST invert this case in the same PR;
35+
* that inversion is the whole point of pinning it. Do not "fix" the
36+
* assertion to match a lock you added elsewhere.
37+
*
38+
* 3. THE FENCE — why the residual is not a live defect: the write-through's
39+
* `delete` leg cannot put a package-declared set into a restorable state.
40+
* An artifact-backed definition tombstones its overlay (an ADR-0005 RESET)
41+
* and the record re-projects to the shipped body; the driver delete never
42+
* runs, so there is no trashed row for a `restore` to act on.
43+
*
44+
* 4. THE BLIND SPOT in the gate that WOULD pre-empt it. `security-plugin.ts`'s
45+
* `assertPackageManagedWriteGate` runs OUTSIDE this middleware and admits
46+
* `restore` into its package-row refusal — but it keys on the `managed_by`
47+
* COLUMN, and the lock's own header records that column as measurably the
48+
* wrong fact. On the `provenance_skip` shape (`permission-set-drift.ts`) a
49+
* genuinely package-declared set carries `managed_by` other than
50+
* `'package'`, so the outer gate does not fire and the unlocked leg is
51+
* what the row reaches. Pinned through the exported classifier, which is
52+
* the fact the lock reads.
53+
*
54+
* ⛔ NOT re-pinned here, on purpose (Prime Directive #8 — one spelling):
55+
* - that `restore` is not an engine middleware dispatch verb. That is
56+
* `packages/objectql/src/engine-middleware-operation-vocabulary.test.ts`
57+
* (#7809), which welds the 7-member union to `engine.ts` source AND drives
58+
* a real engine through every public method. A second copy here would be a
59+
* claim about this file rather than about the engine.
60+
* - that `restore` never derives as an API operation. That is
61+
* `packages/spec/src/data/api-derivation.test.ts` against
62+
* `API_METHOD_DERIVATION.restore.flag`, permanently `false` since
63+
* `enable.trash` was retired (#2377 / ADR-0049); a real recycle bin is
64+
* parked at #3146.
65+
*
66+
* Those two are what make this leg unreachable TODAY. The day either goes red,
67+
* case 2 stops being a curiosity and becomes the defect it describes.
68+
*/
69+
70+
import { describe, it, expect } from 'vitest';
71+
import { assertEngineUpdateDispatch } from '@objectstack/metadata-core';
72+
import { PermissionSetSchema } from '@objectstack/spec/security';
73+
import {
74+
createPermissionSetWriteThrough,
75+
permissionSetBodyFromRow,
76+
} from './permission-set-projection.js';
77+
import { classifyPackagedPermissionSet } from './packaged-permission-set-lock.js';
78+
79+
/** The package-declared body every case in this file turns on. */
80+
const declaredBody = () => ({
81+
name: 'crm_rep',
82+
label: 'CRM Representative',
83+
objects: { crm_lead: { allowRead: true } },
84+
fields: {},
85+
systemPermissions: ['pkg.baseline'],
86+
rowLevelSecurity: [],
87+
tabPermissions: {},
88+
_packageId: 'com.example.crm',
89+
});
90+
91+
/**
92+
* A `sys_permission_set` row projected from that declaration.
93+
*
94+
* `managed_by` is a parameter because the two gates in play read DIFFERENT
95+
* facts about the same row: the outer two-doors gate reads this column, the
96+
* lock reads the artifact registry. Case 4 is about exactly that difference.
97+
*/
98+
const packagedRow = (managedBy: string) => ({
99+
id: 'ps_pkg',
100+
name: 'crm_rep',
101+
label: 'CRM Representative',
102+
managed_by: managedBy,
103+
package_id: 'com.example.crm',
104+
object_permissions: JSON.stringify({ crm_lead: { allowRead: true } }),
105+
field_permissions: JSON.stringify({}),
106+
system_permissions: JSON.stringify(['pkg.baseline']),
107+
row_level_security: JSON.stringify([]),
108+
tab_permissions: JSON.stringify({}),
109+
});
110+
111+
/**
112+
* Minimal engine double over `sys_permission_set` + the env overlay store.
113+
*
114+
* `registry.listItems('permission')` is the source the lock reads — an
115+
* in-memory array, which is the whole reason the lock uses it (no page, no
116+
* cap). Seeding it IS seeding "this name is package-declared".
117+
*/
118+
function makeQl(rows: any[], declared: any[]) {
119+
const permRows = [...rows];
120+
const overlays: any[] = [];
121+
return {
122+
permRows,
123+
overlays,
124+
/** True once the DRIVER delete actually ran (i.e. the row was removed). */
125+
driverDeleted: false,
126+
registry: { listItems: (type: string) => (type === 'permission' ? declared : []) },
127+
async find(object: string, q: any) {
128+
if (object !== 'sys_permission_set') return [];
129+
const where = q?.where ?? {};
130+
// REFUSES what it does not implement, rather than quietly matching
131+
// nothing: a double that answered `[]` for an operator it never learned
132+
// would report "no such row", and every case here would still pass while
133+
// measuring the double instead of the middleware.
134+
return permRows.filter((r) =>
135+
Object.entries(where).every(([k, v]) => {
136+
if (k.startsWith('$')) throw new Error(`fake engine: unsupported operator ${k}`);
137+
if (v && typeof v === 'object') throw new Error(`fake engine: unsupported operand for ${k}`);
138+
return r[k] === v;
139+
}),
140+
);
141+
},
142+
async findOne(object: string, q: any) {
143+
return (await this.find(object, q))[0] ?? null;
144+
},
145+
// Opens with the PRODUCER's own dispatch predicate, never a hand-mirrored
146+
// guard: a fixture that drifts to a call shape `ObjectQL` would refuse must
147+
// fail loudly here rather than collect a green the real engine would not
148+
// have given. `tryUpdate` in the subject SWALLOWS a throw, so a double
149+
// looser than the engine would be invisible twice over.
150+
async update(object: string, data: any, options?: any) {
151+
const dispatch = assertEngineUpdateDispatch(data, options);
152+
const targets = dispatch.kind === 'by-id'
153+
? permRows.filter((r) => r.id === dispatch.id)
154+
: await this.find(object, options);
155+
for (const r of targets) Object.assign(r, data);
156+
return dispatch.kind === 'by-id' ? (targets[0] ?? null) : targets.length;
157+
},
158+
async insert(_object: string, data: any) {
159+
permRows.push({ ...data });
160+
return { id: data.id };
161+
},
162+
};
163+
}
164+
165+
/**
166+
* Metadata protocol double.
167+
*
168+
* `tierGate: 'open'` models the documented `OS_METADATA_WRITABLE=permission`
169+
* operator hatch — the deployment shape this whole family of cards came from,
170+
* where the producer's own ADR-0005 refusal is switched off and the write-door
171+
* lock is the ONLY thing left. `tierGate: 'closed'` is the default posture.
172+
*
173+
* The real `PermissionSetSchema` runs on every accepted save, exactly as
174+
* `saveMetaItem` runs it, so a body this double accepts is a body production
175+
* would accept too.
176+
*/
177+
function makeProtocol(ql: any, declaredNames: string[], tierGate: 'open' | 'closed') {
178+
const codeFor = (name: string) =>
179+
declaredNames.includes(name) ? declaredBody() : null;
180+
return {
181+
saves: [] as any[],
182+
deletes: [] as any[],
183+
async saveMetaItem(req: { type: string; name: string; item: any }) {
184+
if (tierGate === 'closed' && declaredNames.includes(req.name)) {
185+
const err: any = new Error(
186+
`[not_overridable] Metadata item 'permission/${req.name}' is provided by a code package `
187+
+ 'and the type has not opted into per-org overlay writes (allowOrgOverride=false).',
188+
);
189+
err.code = 'NOT_OVERRIDABLE';
190+
err.status = 403;
191+
throw err;
192+
}
193+
const parsed = PermissionSetSchema.safeParse(req.item);
194+
if (!parsed.success) {
195+
const err: any = new Error(
196+
`[invalid_metadata] permission/${req.name} failed spec validation: `
197+
+ parsed.error.issues.map((i: any) => `${i.path.join('.') || '<root>'}: ${i.message}`).join('; '),
198+
);
199+
err.code = 'INVALID_METADATA';
200+
err.status = 422;
201+
throw err;
202+
}
203+
ql.overlays.push({ name: req.name, item: req.item });
204+
this.saves.push({ ...req });
205+
return { success: true };
206+
},
207+
async deleteMetaItem(req: { type: string; name: string }) {
208+
const i = ql.overlays.findIndex((o: any) => o.name === req.name);
209+
if (i >= 0) ql.overlays.splice(i, 1);
210+
this.deletes.push({ ...req });
211+
// Artifact-backed: the overlay is tombstoned and the definition RESETS
212+
// to the shipped body — the record is never removed. `reset: true` is
213+
// the real protocol's own answer for this case.
214+
return { success: true, reset: declaredNames.includes(req.name) };
215+
},
216+
async getMetaItemLayered(req: { type: string; name: string }) {
217+
const code = codeFor(req.name);
218+
const overlay = ql.overlays.find((o: any) => o.name === req.name)?.item ?? null;
219+
return {
220+
type: 'permission',
221+
name: req.name,
222+
code,
223+
overlay,
224+
overlayScope: overlay ? 'env' : null,
225+
effective: overlay ?? code,
226+
};
227+
},
228+
};
229+
}
230+
231+
/** Run the middleware, reporting whether the engine's own write (`next`) ran. */
232+
async function run(mw: any, opCtx: any): Promise<boolean> {
233+
let nextCalled = false;
234+
await mw(opCtx, async () => { nextCalled = true; });
235+
return nextCalled;
236+
}
237+
238+
const userCtx = { userId: 'usr_admin' };
239+
240+
describe('[#11725] the restore leg of the permission-set write-through', () => {
241+
it('CONTROL: an UPDATE on the same set is refused by the #11702 LOCK — named by its message, not just its code', async () => {
242+
// The gate identification this whole file rests on. `NOT_OVERRIDABLE`/403
243+
// alone does NOT name a gate: ADR-0005's tier gate answers with the same
244+
// envelope for the same row. The lock is distinguished by its MESSAGE,
245+
// which teaches the sanctioned path — and by `saves.length === 0`, which
246+
// says the refusal happened BEFORE the metadata write rather than inside
247+
// it. Both halves are asserted, because either alone is ambiguous.
248+
const ql = makeQl([packagedRow('package')], [declaredBody()]);
249+
// Hatch OPEN: the producer's tier gate is switched off, so anything that
250+
// refuses here can only be the write-door lock.
251+
const protocol = makeProtocol(ql, ['crm_rep'], 'open');
252+
const mw = createPermissionSetWriteThrough({ ql, getProtocol: () => protocol });
253+
254+
let caught: any;
255+
try {
256+
await run(mw, {
257+
object: 'sys_permission_set',
258+
operation: 'update',
259+
data: { id: 'ps_pkg', label: 'hijacked' },
260+
context: userCtx,
261+
});
262+
} catch (e) { caught = e; }
263+
264+
expect(caught, 'the update was refused').toBeDefined();
265+
expect(caught).toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 });
266+
expect(caught.name).toBe('PackagedPermissionSetLockedError');
267+
// The wording IS the contract here (it is what separates this refusal from
268+
// the tier gate's), so it is asserted on top of the envelope, never instead.
269+
expect(caught.message).toContain("is declared by package 'com.example.crm'");
270+
expect(caught.message).toContain('clone');
271+
expect(protocol.saves.length, 'refused BEFORE the metadata write').toBe(0);
272+
expect(ql.overlays.length, 'no overlay of a packaged set was minted').toBe(0);
273+
});
274+
275+
it('MEASURED RESIDUAL: the same set through RESTORE is re-authored with no lock consulted and no refusal', async () => {
276+
// ⚠️ Pins the residual, not a desired behaviour. See this file's header:
277+
// the follow-up that extends the lock to this leg inverts this case.
278+
//
279+
// Identical fixture to the CONTROL above — same ql, same registry, same
280+
// protocol posture, same row. The ONLY difference is the operation, so the
281+
// difference in outcome is attributable to the leg and nothing else.
282+
const ql = makeQl([packagedRow('package')], [declaredBody()]);
283+
const protocol = makeProtocol(ql, ['crm_rep'], 'open');
284+
const errors: any[] = [];
285+
const mw = createPermissionSetWriteThrough({
286+
ql,
287+
getProtocol: () => protocol,
288+
logger: { error: (m: string, e?: Error) => errors.push({ m, e }), info: () => {}, warn: () => {} },
289+
});
290+
291+
const nextCalled = await run(mw, {
292+
object: 'sys_permission_set',
293+
operation: 'restore',
294+
options: { where: { id: 'ps_pkg' } },
295+
context: userCtx,
296+
});
297+
298+
expect(nextCalled, 'the engine un-trash runs first — the leg is a post-pass').toBe(true);
299+
expect(errors, 'nothing was refused, so nothing was reported').toEqual([]);
300+
expect(protocol.saves.length, 'the package-declared body WAS re-authored').toBe(1);
301+
expect(protocol.saves[0].name).toBe('crm_rep');
302+
// The definition of a package-declared set now lives in the environment
303+
// overlay store — authored through a door that never asked the lock.
304+
expect(ql.overlays.map((o: any) => o.name)).toEqual(['crm_rep']);
305+
expect(ql.overlays[0].item).toEqual(permissionSetBodyFromRow(packagedRow('package')));
306+
});
307+
308+
it('FENCE: DELETE cannot produce a restorable row — the packaged definition RESETS and the driver delete never runs', async () => {
309+
// Why the residual above is not a live defect through this door: there is
310+
// nothing to restore. The write-through owns the delete, tombstones the
311+
// overlay, and returns WITHOUT calling `next()` — so the engine never
312+
// removes (or trashes) the record.
313+
const ql = makeQl([packagedRow('package')], [declaredBody()]);
314+
const protocol = makeProtocol(ql, ['crm_rep'], 'open');
315+
const mw = createPermissionSetWriteThrough({ ql, getProtocol: () => protocol });
316+
317+
const nextCalled = await run(mw, {
318+
object: 'sys_permission_set',
319+
operation: 'delete',
320+
options: { where: { id: 'ps_pkg' } },
321+
context: userCtx,
322+
});
323+
324+
expect(nextCalled, 'the driver delete is never reached').toBe(false);
325+
expect(protocol.deletes.length, 'the overlay tombstone IS the delete').toBe(1);
326+
expect(protocol.deletes[0]).toMatchObject({ type: 'permission', name: 'crm_rep' });
327+
expect(ql.permRows.length, 'the record survives — a packaged set cannot be removed').toBe(1);
328+
expect(ql.permRows[0].name).toBe('crm_rep');
329+
});
330+
331+
it('BLIND SPOT: the outer two-doors gate keys on `managed_by`, which the lock records as the WRONG fact', async () => {
332+
// `assertPackageManagedWriteGate` (security-plugin.ts) admits `restore`
333+
// into its package-row refusal and would therefore pre-empt this leg — but
334+
// only for rows whose `managed_by` column reads `'package'`. The
335+
// `provenance_skip` shape is a genuinely package-declared set whose column
336+
// does not, and on that row the outer gate returns without refusing while
337+
// the lock's own fact says `packaged`. That disagreement is the reason
338+
// "the outer gate already covers it" is not an answer.
339+
const declared = [declaredBody()];
340+
const provenanceSkipRow = packagedRow('user'); // the drift-report shape
341+
const ql = makeQl([provenanceSkipRow], declared);
342+
343+
// The lock's fact, read from the artifact registry the lock actually uses.
344+
expect(classifyPackagedPermissionSet('crm_rep', ql)).toEqual({
345+
status: 'packaged',
346+
packageId: 'com.example.crm',
347+
});
348+
// The outer gate's fact, on the very same row.
349+
expect(provenanceSkipRow.managed_by, 'the column the outer gate reads').not.toBe('package');
350+
351+
// Control: the classifier is not simply answering `packaged` for
352+
// everything — a name the registry does not declare comes back `org`,
353+
// which is what makes the assertion above evidence of anything.
354+
expect(classifyPackagedPermissionSet('my_own_set', ql)).toEqual({ status: 'org' });
355+
});
356+
});

‎scripts/engine-double-contract.pinned.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,6 +1456,11 @@
14561456
"verb": "update",
14571457
"pinned": 1
14581458
},
1459+
{
1460+
"file": "packages/plugins/plugin-security/src/packaged-permission-set-restore-leg.test.ts",
1461+
"verb": "update",
1462+
"pinned": 1
1463+
},
14591464
{
14601465
"file": "packages/plugins/plugin-security/src/per-organization-catalog.test.ts",
14611466
"verb": "update",

0 commit comments

Comments
 (0)