Skip to content

Commit 4c165f6

Browse files
committed
test(plugin-security): pin the by-id heal, the lazy layered read and the refused existence read
Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5ccce70 commit 4c165f6

3 files changed

Lines changed: 394 additions & 0 deletions

File tree

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#22169] `bootstrapPlatformAdmin`'s existence read: a read that did not
5+
* answer is not an absent row.
6+
*
7+
* The seed loop reads each default permission set BY NAME and inserts when it
8+
* finds none. Its `tryFind` answers `[]` for a refused read as well as for an
9+
* empty table, and the loop inserted on both — so on a table without the
10+
* per-organization unique index a refused read minted a second row of a set
11+
* that already existed, and a table holding such duplicates can never build
12+
* that index again. Measured on a real engine with that index dropped: two
13+
* declared sets, two INSERTs, under a refused read.
14+
*
15+
* The double below has no `update` or `delete` member — the seed pass without
16+
* `resync` calls neither — so it is not an engine double those dispatch
17+
* contracts govern. Its `find` either answers from its rows or throws, which
18+
* are exactly the two inputs the pass must tell apart.
19+
*/
20+
21+
import { describe, it, expect } from 'vitest';
22+
import { bootstrapPlatformAdmin } from './bootstrap-platform-admin.js';
23+
24+
function makeQl(opts: { refuseReads: boolean; rows?: any[] }) {
25+
const rows: any[] = (opts.rows ?? []).map((r) => ({ ...r }));
26+
const inserted: any[] = [];
27+
return {
28+
rows,
29+
inserted,
30+
async find(object: string, q: any) {
31+
if (object === 'sys_permission_set' && opts.refuseReads) {
32+
throw new Error('fake outage: sys_permission_set read refused');
33+
}
34+
if (object !== 'sys_permission_set') return [];
35+
const where = q?.where ?? {};
36+
return rows.filter((r) => Object.entries(where).every(([k, v]) => {
37+
if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`);
38+
return r[k] === v;
39+
}));
40+
},
41+
async insert(object: string, data: any) {
42+
if (object !== 'sys_permission_set') return null;
43+
inserted.push({ ...data });
44+
rows.push({ ...data });
45+
return { id: data.id };
46+
},
47+
};
48+
}
49+
50+
const set = (name: string) => ({ name, label: name, objects: {}, systemPermissions: [] }) as any;
51+
52+
function capture() {
53+
const warns: Array<{ msg: string; meta?: any }> = [];
54+
return { warns, logger: { info: () => {}, warn: (msg: string, meta?: any) => warns.push({ msg, meta }) } };
55+
}
56+
57+
describe('#22169 — bootstrapPlatformAdmin declines the insert when its existence read was refused', () => {
58+
it('a refused read inserts NOTHING, and says so once for the whole pass', async () => {
59+
const ql = makeQl({ refuseReads: true, rows: [{ id: 'ps_existing', name: 'admin_full_access' }] });
60+
const { warns, logger } = capture();
61+
62+
const r = await bootstrapPlatformAdmin(ql, [set('admin_full_access'), set('member_default'), set('viewer_readonly')], { logger });
63+
64+
expect(ql.inserted, '⛔ no row minted on a read that did not answer').toEqual([]);
65+
expect(r.seeded).toBe(0);
66+
const lines = warns.filter((w) => w.msg.includes('could not be read'));
67+
expect(lines, 'one line per pass, never one per set').toHaveLength(1);
68+
expect(lines[0]!.meta).toMatchObject({
69+
unreadable: 3, total: 3, names: ['admin_full_access', 'member_default', 'viewer_readonly'],
70+
});
71+
// The promotion half reports the read that failed, not a set that is "missing".
72+
expect(r.reason).toBe('admin_permission_set_unreadable');
73+
});
74+
75+
it('control: an EMPTY answer is still "absent" — a fresh install seeds every default set', async () => {
76+
const ql = makeQl({ refuseReads: false });
77+
const { warns, logger } = capture();
78+
79+
const r = await bootstrapPlatformAdmin(ql, [set('admin_full_access'), set('member_default')], { logger });
80+
81+
expect(ql.inserted.map((row) => row.name)).toEqual(['admin_full_access', 'member_default']);
82+
expect(r.seeded).toBe(2);
83+
expect(warns.filter((w) => w.msg.includes('could not be read'))).toEqual([]);
84+
});
85+
86+
it('control: an existing row is found and left alone — no insert, no unreadable line', async () => {
87+
const ql = makeQl({ refuseReads: false, rows: [{ id: 'ps_existing', name: 'member_default' }] });
88+
const { warns, logger } = capture();
89+
90+
await bootstrapPlatformAdmin(ql, [set('member_default')], { logger });
91+
92+
expect(ql.inserted).toEqual([]);
93+
expect(warns.filter((w) => w.msg.includes('could not be read'))).toEqual([]);
94+
});
95+
});
Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#22169] The boot heal converges on a table holding DUPLICATE rows of a name.
5+
*
6+
* `reconcilePermissionSetProjection` pages `sys_permission_set` and heals every
7+
* overlay-less, env-authored row that drifted from its declared body. It used
8+
* to hand each drifted row to a by-NAME upsert, which re-read "the" row as the
9+
* first one of that name (lowest id) — so with two or more rows per name only
10+
* that one was ever written, and every later row was warned about as
11+
* "re-projected" and left drifted, on every boot. Measured on a real engine
12+
* (8 names × 3 rows): 16 of 24 rows still drifted and 16 drift warns on boot 2,
13+
* 3, … . The same by-name read had an INSERT branch that a refused read reached
14+
* (`tryFind` answers `[]` for a throw), minting a duplicate of a row the loop
15+
* was holding. And a layered metadata read (four serial statements on the real
16+
* protocol) was awaited for every candidate row although a readable
17+
* SchemaRegistry makes the trust rule ignore it.
18+
*
19+
* ## The harness, and why each piece is real
20+
*
21+
* A REAL `ObjectQL` over a REAL `SqlDriver` (better-sqlite3 `:memory:`), both
22+
* aliased to source by this package's `vitest.config.ts`. The by-name read the
23+
* defect lives in is the engine's own `ORDER BY id LIMIT 1`, and the heal's
24+
* write is the engine's own update by id — a fake table would answer for its
25+
* author instead. The per-organization unique index is DROPPED after schema
26+
* sync: that is the shape of a remote table built before the index retrofit,
27+
* the only shape that can hold duplicates, and a table that already holds them
28+
* can never build the index again.
29+
*
30+
* The engine is OBSERVED, never replaced: every verb forwards to it, and the
31+
* dispatch-shaped verbs open with the producer's own predicates
32+
* (`check:engine-double-contract`). The protocol is a counting stub — what is
33+
* pinned about it is whether it is CALLED, which a stub answers exactly.
34+
*/
35+
36+
import { describe, it, expect, afterEach } from 'vitest';
37+
import { ObjectQL } from '@objectstack/objectql';
38+
import { SqlDriver } from '@objectstack/driver-sql';
39+
import { assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/metadata-core';
40+
41+
import { SysPermissionSet } from './objects/sys-permission-set.object.js';
42+
import {
43+
permissionSetRowFields,
44+
reconcilePermissionSetProjection,
45+
recordDiffersFromBody,
46+
} from './permission-set-projection.js';
47+
48+
const SYS = { context: { isSystem: true } } as any;
49+
50+
/** Just enough `sys_metadata` for the overlay pass to read an empty table. */
51+
const SYS_METADATA_OBJECT: any = {
52+
name: 'sys_metadata',
53+
label: 'System Metadata',
54+
fields: {
55+
id: { name: 'id', label: 'ID', type: 'text', primaryKey: true },
56+
type: { name: 'type', label: 'Type', type: 'text', required: true },
57+
name: { name: 'name', label: 'Name', type: 'text', required: true },
58+
organization_id: { name: 'organization_id', label: 'Org', type: 'text' },
59+
package_id: { name: 'package_id', label: 'Package', type: 'text' },
60+
metadata: { name: 'metadata', label: 'Body', type: 'textarea' },
61+
state: { name: 'state', label: 'State', type: 'text' },
62+
},
63+
};
64+
65+
const declaredBody = (name: string) => ({
66+
name,
67+
label: `Declared ${name}`,
68+
objects: { crm_lead: { allowRead: true } },
69+
systemPermissions: ['declared.baseline'],
70+
});
71+
72+
/** A row of `name` whose facets drifted from the declaration. */
73+
const driftedRow = (id: string, name: string) => ({
74+
id,
75+
name,
76+
managed_by: 'admin',
77+
active: true,
78+
...permissionSetRowFields(declaredBody(name)),
79+
system_permissions: '["drifted.edit"]',
80+
});
81+
82+
const engines: ObjectQL[] = [];
83+
afterEach(async () => {
84+
while (engines.length) {
85+
try { await engines.pop()?.destroy(); } catch { /* noop */ }
86+
}
87+
});
88+
89+
async function boot(declaredNames: string[], rows: Array<Record<string, any>>): Promise<ObjectQL> {
90+
const engine = new ObjectQL();
91+
const driver = new SqlDriver({
92+
client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true,
93+
});
94+
engine.registerDriver(driver, true);
95+
await engine.init();
96+
engine.registerApp({
97+
id: 'com.objectstack.heal-by-id-22169',
98+
name: 'Heal by id',
99+
version: '1.0.0',
100+
type: 'plugin',
101+
scope: 'system',
102+
objects: [SysPermissionSet, SYS_METADATA_OBJECT],
103+
} as any);
104+
await engine.syncSchemas();
105+
engines.push(engine);
106+
await (driver as any).getKnex().raw('DROP INDEX IF EXISTS uniq_sys_permission_set_organization_id_name');
107+
for (const name of declaredNames) {
108+
engine.registry.registerItem('permission', declaredBody(name), 'name', 'com.example.pkg');
109+
}
110+
for (const row of rows) await (engine as any).insert('sys_permission_set', row, SYS);
111+
return engine;
112+
}
113+
114+
/** The real engine with its verbs counted; `refuseNameReads` makes every by-name read of the table throw. */
115+
function observe(engine: any, opts: { withRegistry?: boolean } = {}) {
116+
const log = { updatedIds: [] as string[], inserts: 0, nameReads: 0 };
117+
const state = { refuseNameReads: false };
118+
const ql: any = {
119+
...(opts.withRegistry === false ? {} : { registry: engine.registry }),
120+
find: async (o: string, q?: any, opt?: any) => {
121+
if (o === 'sys_permission_set' && q?.where && typeof q.where === 'object' && 'name' in q.where) {
122+
log.nameReads += 1;
123+
if (state.refuseNameReads) throw new Error('fake outage: the by-name read was refused');
124+
}
125+
return engine.find(o, q, opt);
126+
},
127+
findOne: (o: string, q?: any, opt?: any) => {
128+
assertEngineFindOnePredicate(o, q);
129+
return engine.findOne(o, q, opt);
130+
},
131+
insert: async (o: string, d: any, opt?: any) => {
132+
if (o === 'sys_permission_set') log.inserts += 1;
133+
return engine.insert(o, d, opt);
134+
},
135+
update: async (o: string, d: any, opt?: any) => {
136+
assertEngineUpdateDispatch(d, opt);
137+
if (o === 'sys_permission_set') log.updatedIds.push(String(d?.id));
138+
return engine.update(o, d, opt);
139+
},
140+
};
141+
return { ql, log, state };
142+
}
143+
144+
/** A protocol that answers the layered read from the declarations and counts every call. */
145+
function countingProtocol(declaredNames: string[]) {
146+
const layeredReads: string[] = [];
147+
return {
148+
layeredReads,
149+
async getMetaItemLayered(req: { type: string; name: string }) {
150+
layeredReads.push(req.name);
151+
const code = declaredNames.includes(req.name) ? declaredBody(req.name) : null;
152+
return { type: 'permission', name: req.name, code, overlay: null, effective: code };
153+
},
154+
};
155+
}
156+
157+
function captureWarns() {
158+
const warns: Array<{ msg: string; meta?: any }> = [];
159+
return {
160+
warns,
161+
driftWarns: () => warns.filter((w) => w.msg.includes('drifted from its metadata definition')),
162+
logger: { info: () => {}, warn: (msg: string, meta?: any) => warns.push({ msg, meta }) },
163+
};
164+
}
165+
166+
async function allRows(engine: ObjectQL): Promise<any[]> {
167+
return (await (engine as any).find('sys_permission_set', { where: {}, limit: 100 }, SYS)) as any[];
168+
}
169+
170+
describe('#22169 — the boot heal writes the row it read, by id', () => {
171+
it('a duplicated name: each drifted row is written once, by its own id, and the next boot logs no drift warn', async () => {
172+
const engine = await boot(['member_default'], [
173+
driftedRow('ps_a', 'member_default'),
174+
driftedRow('ps_b', 'member_default'),
175+
]);
176+
const { ql, log } = observe(engine);
177+
const protocol = countingProtocol(['member_default']);
178+
179+
const first = captureWarns();
180+
const out1 = await reconcilePermissionSetProjection(protocol, { ql, logger: first.logger });
181+
182+
expect([...log.updatedIds].sort(), 'each drifted row written once, by ITS id').toEqual(['ps_a', 'ps_b']);
183+
expect(log.inserts).toBe(0);
184+
expect(out1.driftHealed).toBe(2);
185+
// The warn is said per row healed, AFTER its write, and names the row.
186+
expect(first.driftWarns().map((w) => w.meta?.id).sort()).toEqual(['ps_a', 'ps_b']);
187+
for (const row of await allRows(engine)) {
188+
expect(recordDiffersFromBody(row, declaredBody('member_default')), `${row.id} converged`).toBe(false);
189+
}
190+
191+
// Boot 2: nothing drifted, so nothing is written and nothing is warned.
192+
log.updatedIds.length = 0;
193+
const second = captureWarns();
194+
const out2 = await reconcilePermissionSetProjection(protocol, { ql, logger: second.logger });
195+
expect(second.driftWarns(), 'the next boot logs no drift warn').toEqual([]);
196+
expect(log.updatedIds).toEqual([]);
197+
expect(out2.driftHealed).toBe(0);
198+
expect(await allRows(engine)).toHaveLength(2); // ⛔ duplicates are healed, never deleted here
199+
});
200+
201+
it('control: a single drifted row heals as it always did — one write, one warn, then silence', async () => {
202+
const engine = await boot(['viewer_readonly'], [driftedRow('ps_v', 'viewer_readonly')]);
203+
const { ql, log } = observe(engine);
204+
const protocol = countingProtocol(['viewer_readonly']);
205+
206+
const first = captureWarns();
207+
const out1 = await reconcilePermissionSetProjection(protocol, { ql, logger: first.logger });
208+
expect(log.updatedIds).toEqual(['ps_v']);
209+
expect(out1.driftHealed).toBe(1);
210+
expect(first.driftWarns()).toHaveLength(1);
211+
const [row] = await allRows(engine);
212+
expect(JSON.parse(row.system_permissions)).toEqual(['declared.baseline']);
213+
expect(row.managed_by, 'facets only — provenance untouched').toBe('admin');
214+
215+
const second = captureWarns();
216+
await reconcilePermissionSetProjection(protocol, { ql, logger: second.logger });
217+
expect(second.driftWarns()).toEqual([]);
218+
});
219+
220+
it('issues NO layered metadata read while a SchemaRegistry answers — on the healing boot or after it', async () => {
221+
const engine = await boot(['member_default', 'viewer_readonly'], [
222+
driftedRow('ps_a', 'member_default'),
223+
driftedRow('ps_b', 'member_default'),
224+
driftedRow('ps_v', 'viewer_readonly'),
225+
]);
226+
const { ql } = observe(engine);
227+
const protocol = countingProtocol(['member_default', 'viewer_readonly']);
228+
229+
await reconcilePermissionSetProjection(protocol, { ql, logger: captureWarns().logger });
230+
await reconcilePermissionSetProjection(protocol, { ql, logger: captureWarns().logger });
231+
expect(protocol.layeredReads).toEqual([]);
232+
});
233+
234+
it('without a SchemaRegistry the layered read is still the source — asked once per NAME, not per row', async () => {
235+
const engine = await boot([], [
236+
driftedRow('ps_a', 'member_default'),
237+
driftedRow('ps_b', 'member_default'),
238+
]);
239+
const { ql, log } = observe(engine, { withRegistry: false });
240+
const protocol = countingProtocol(['member_default']);
241+
242+
const out = await reconcilePermissionSetProjection(protocol, { ql, logger: captureWarns().logger });
243+
expect(protocol.layeredReads).toEqual(['member_default']);
244+
expect([...log.updatedIds].sort()).toEqual(['ps_a', 'ps_b']);
245+
expect(out.driftHealed).toBe(2);
246+
});
247+
248+
it('a refused by-name existence read inserts nothing — the heal holds the row and writes it by id', async () => {
249+
const engine = await boot(['member_default'], [
250+
driftedRow('ps_a', 'member_default'),
251+
driftedRow('ps_b', 'member_default'),
252+
]);
253+
const { ql, log, state } = observe(engine);
254+
state.refuseNameReads = true;
255+
const protocol = countingProtocol(['member_default']);
256+
257+
await reconcilePermissionSetProjection(protocol, { ql, logger: captureWarns().logger });
258+
expect(log.inserts, '⛔ no row minted on a read that did not answer').toBe(0);
259+
expect(await allRows(engine)).toHaveLength(2);
260+
expect([...log.updatedIds].sort()).toEqual(['ps_a', 'ps_b']);
261+
});
262+
});

‎packages/plugins/plugin-security/src/permission-set-projection.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,3 +1845,40 @@ describe('#11097 — a read that CANNOT ANSWER is not the answer "none exist"',
18451845
expect(ql.permRows).toHaveLength(1);
18461846
});
18471847
});
1848+
1849+
/**
1850+
* [#22169] The heal's layered read is cached per NAME for the pass — and a
1851+
* backfill is the one write in this pass that changes that name's answer, so it
1852+
* drops the cached one. Without the drop, a second row of a name this pass had
1853+
* just backfilled would see the pre-write "no definition" answer and backfill
1854+
* AGAIN, replacing the definition with whichever duplicate the page served last.
1855+
* This suite's double has no SchemaRegistry, which is the only shape that reads
1856+
* the layered item at all.
1857+
*/
1858+
describe('#22169 — the per-name layered read is re-asked after this pass backfills the name', () => {
1859+
it('a second row of a just-backfilled name heals to the backfilled body instead of backfilling again', async () => {
1860+
const ql = makeQl();
1861+
const protocol = makeProtocol(ql);
1862+
const layeredReads: string[] = [];
1863+
const layered = protocol.getMetaItemLayered.bind(protocol);
1864+
(protocol as any).getMetaItemLayered = async (req: { type: string; name: string }) => {
1865+
layeredReads.push(req.name);
1866+
return layered(req);
1867+
};
1868+
const legacy = (id: string, systemPermissions: string[]) => ({
1869+
id, name: 'support_agent', managed_by: 'admin', active: true,
1870+
...permissionSetRowFields(envBody({ name: 'support_agent', systemPermissions })),
1871+
});
1872+
ql.permRows.push(legacy('ps_first', ['support.use']), legacy('ps_second', ['support.other']));
1873+
1874+
const out = await reconcilePermissionSetProjection(protocol, { ql, logger: { warn: () => {} } });
1875+
1876+
expect(protocol.saves, 'ONE definition is backfilled for the name').toHaveLength(1);
1877+
expect(out.backfilledIntoMetadata).toBe(1);
1878+
expect(layeredReads, 'asked for the first row, and again after the backfill wrote').toEqual(['support_agent', 'support_agent']);
1879+
expect(out.driftHealed).toBe(1);
1880+
for (const row of ql.permRows) {
1881+
expect(JSON.parse(row.system_permissions), `${row.id} carries the backfilled body`).toEqual(['support.use']);
1882+
}
1883+
});
1884+
});

0 commit comments

Comments
 (0)