From 8da64833446056caf32b055540273d4c8b17655b Mon Sep 17 00:00:00 2001 From: os-warren Date: Sat, 5 Sep 2026 09:08:54 +0000 Subject: [PATCH 1/3] fix(plugin-security): retire seven dead `{ records }` find-result limbs, and stop the permission-set loader inventing an empty page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six of the seven `Array.isArray(x) ? x : x.records` blocks in this plugin carried an unreachable envelope limb over an engine `find()` result. The limb is removed on a MEASUREMENT rather than on the declared type: `IDataEngine.find` says `Promise`, but a declared type is not proof here — this repo also carries a `find()` that resolves a `QueryResult` envelope and never an array. A real `ObjectQL` over a real `SqlDriver` was booted and each seam driven through the shipped function that owns it; every one answered a bare array with no own `records` key, on a populated page and an empty one alike. Each of the six keeps its existing disposition for a non-array. Removing a dead limb must not quietly convert a seam that gaps into one that invents an empty. The seventh block is the opposite defect and is repaired in the opposite direction. `SecurityPlugin`'s `sys_permission_set` loader swallowed a thrown read into `[]` and mapped an unreadable result to `[]` too, so three distinct facts left by one door. This is the enforcement plane: "no permission sets" silently withdraws grants that exist while every request still looks normal, and the swallow made `PermissionEvaluator.resolvePermissionSets`' own "db lookup failed" warn unreachable — the diagnostic this repo had already built for exactly this loss. The read fault now propagates and an unreadable page is refused with `DATABASE_ERROR`; a page carrying a non-row refuses too, where the trailing filter used to drop it in silence. Enforcement is unchanged in both directions — an unanswered read still grants nothing — but it is now sayable. Seven pins, one per block, each driving its own seam against the real engine. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/quiet-pans-repair.md | 9 + .../src/auto-org-admin-grant.ts | 9 +- .../src/bootstrap-declared-permissions.ts | 7 +- .../src/claim-seed-ownership.ts | 17 +- .../src/engine-find-bare-array.pin.test.ts | 445 ++++++++++++++++++ .../plugins/plugin-security/src/errors.ts | 55 +++ .../src/normalize-managed-by.ts | 4 +- .../plugin-security/src/security-plugin.ts | 88 +++- .../plugin-security/src/seed-name-lookup.ts | 27 +- 9 files changed, 633 insertions(+), 28 deletions(-) create mode 100644 .changeset/quiet-pans-repair.md create mode 100644 packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts diff --git a/.changeset/quiet-pans-repair.md b/.changeset/quiet-pans-repair.md new file mode 100644 index 0000000000..526c903873 --- /dev/null +++ b/.changeset/quiet-pans-repair.md @@ -0,0 +1,9 @@ +--- +'@objectstack/plugin-security': patch +--- + +Remove seven dead `{ records }` union-normalizer limbs on engine `find()` results, and repair the one that was silently dropping instead of gapping. + +Six seams in this plugin normalized an engine read as `Array.isArray(x) ? x : x.records`. The envelope limb was unreachable: `ObjectQL.find` resolves a bare array of row objects, measured by booting a real engine over a real `SqlDriver` and driving each seam through the shipped function that owns it, rather than inferred from `IDataEngine.find`'s declared `Promise` (a declared type is not proof — this repo also has a `find()` that resolves an envelope). Each seam keeps its existing disposition for a non-array; only the dead limb is gone. + +The seventh is repaired in the opposite direction. `SecurityPlugin`'s `sys_permission_set` loader mapped three different facts onto one value: a read that succeeded on an empty catalog, a read that threw, and a read that resolved something it could not read all left as `[]`. On the enforcement plane that silently withdraws grants that exist while every request still looks normal, and it made `PermissionEvaluator`'s existing "db lookup failed" warning unreachable — so a transient database error and an empty catalog produced identical, undiagnosable 403s. The loader now lets the read fault propagate and refuses an unreadable result with `DATABASE_ERROR`. Enforcement is unchanged in both directions: an unanswered read still grants nothing. What changes is that it is now reported instead of silent. diff --git a/packages/plugins/plugin-security/src/auto-org-admin-grant.ts b/packages/plugins/plugin-security/src/auto-org-admin-grant.ts index 62447b9fcc..49b0ac3dc0 100644 --- a/packages/plugins/plugin-security/src/auto-org-admin-grant.ts +++ b/packages/plugins/plugin-security/src/auto-org-admin-grant.ts @@ -149,7 +149,14 @@ async function tryFind( ): Promise { try { const rows = await ql.find(object, { where, limit }, { context }); - return Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : []; + // Bare array, driven — see `engine-find-bare-array.pin.test.ts`, which boots + // a real engine over a real `SqlDriver` and pins this seam. The `{ records }` + // limb removed from here was dead code that read as a contract. + // + // The `[]` arm is left exactly as it was: this function's whole contract is + // `Promise` best-effort, and turning it into a gap is a different + // change with a different blast radius than removing an unreachable limb. + return Array.isArray(rows) ? rows : []; } catch (e) { // Reads legitimately fail before the tables exist (boot ordering), so this // is debug rather than warn — but it is no longer nothing (#4640). diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index f113622f11..b83f836070 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -91,7 +91,12 @@ async function defaultLookup(ql: any, name: string, organizationId?: string): Pr } catch { return { status: 'unknown' }; } - const list = Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : null; + // Bare array, driven against a real engine over a real `SqlDriver` — not + // inferred from `IDataEngine.find`'s declared `Promise`, which is not + // proof (this repo has a `find()` that resolves an envelope instead). The + // `{ records }` limb that stood here was dead; `engine-find-bare-array.pin.test.ts` + // pins this seam. A non-array is still `unknown` — never "no such row". + const list = Array.isArray(rows) ? rows : null; if (list === null) return { status: 'unknown' }; // [#10103] This organization's own row answers; an organization-less leftover // is reported beside `absent` and never returned as `present`. One spelling of diff --git a/packages/plugins/plugin-security/src/claim-seed-ownership.ts b/packages/plugins/plugin-security/src/claim-seed-ownership.ts index d5512db31c..b772de37ab 100644 --- a/packages/plugins/plugin-security/src/claim-seed-ownership.ts +++ b/packages/plugins/plugin-security/src/claim-seed-ownership.ts @@ -173,13 +173,18 @@ function affectedRowCount(value: unknown): number | undefined { return value; } -/** Ids from a `find` result, tolerating both the array and `{ records }` shapes. */ +/** + * Ids from a `find` result. + * + * `ObjectQL.find` resolves a BARE array — driven against a real engine over a + * real `SqlDriver` through this module's own paging fallback, the only path that + * reaches here (`engine-find-bare-array.pin.test.ts`). It is driven rather than + * read off `IDataEngine.find`'s declared `Promise` because a declared + * type is not proof: this repo also has a `find()` that resolves an envelope. + * The `{ records }` limb this carried was dead. + */ function idsFrom(rows: any): string[] { - const list: any[] = Array.isArray(rows) - ? rows - : Array.isArray(rows?.records) - ? rows.records - : []; + const list: any[] = Array.isArray(rows) ? rows : []; const out: string[] = []; for (const r of list) if (r?.id) out.push(String(r.id)); return out; diff --git a/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts b/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts new file mode 100644 index 0000000000..919f318a18 --- /dev/null +++ b/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts @@ -0,0 +1,445 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15598] What `ObjectQL.find` actually resolves at the seven seams that used + * to normalize it — one pin per seam, each driven against a REAL engine. + * + * ## The class, and why a type was not allowed to settle it + * + * Seven blocks in this plugin carried `Array.isArray(x) ? x : x.records` over an + * engine `find()` result. `IDataEngine.find` is declared `Promise`, so + * the envelope limb looked provably unreachable — but a declared type is NOT + * proof in this repo: `ObjectStackAdapter.find()` resolves a `QueryResult` + * envelope and never an array, which is exactly the counter-case that makes + * "the type says so" the wrong instrument. The limbs were therefore removed on + * a MEASUREMENT: a real `ObjectQL` over a real `SqlDriver` (better-sqlite3 + * `:memory:`), driven once per seam through the shipped function that owns it. + * Every seam answered `[object Array]`, with no own `records` key, on a + * populated page and an empty one alike. + * + * ⚠️ These are seven pins, not one pin repeated. A single reading of "the engine + * returns an array" would say nothing about whether a given seam is even + * reachable — six of them sit behind a trigger (a truncated page, a refused + * bulk write, a publish-materializer call path), and a seam nothing reaches is + * precisely how a dead limb survives review. So each case DRIVES its own block + * through the real function and asserts the answer that block produced, and + * {@link assertBareArrayPage} then reads back the value the engine handed it. + * + * ## The seventh is the opposite defect, and its legs say so + * + * `security-plugin.ts`'s `sys_permission_set` loader was not merely carrying a + * dead limb: it swallowed a THROWN read into `[]` and mapped an unreadable + * result to `[]` as well, so three different facts left as one value. On the + * enforcement plane that is a silent withdrawal of grants that exist. Its legs + * below pin the repaired direction — the fault PROPAGATES, the refusal carries + * an envelope, and `PermissionEvaluator`'s #2565 warn (unreachable while the + * loader swallowed) fires while the request stays fail-closed. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; + +import { SysPosition } from './objects/sys-position.object.js'; +import { SysPermissionSet } from './objects/sys-permission-set.object.js'; +import { SysPositionPermissionSet } from './objects/sys-position-permission-set.object.js'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; +import { ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; + +import { buildExistingByName } from './seed-name-lookup.js'; +import { upsertPackagePermissionSet } from './bootstrap-declared-permissions.js'; +import { reconcileOrgAdminGrant } from './auto-org-admin-grant.js'; +import { claimSeedOwnership } from './claim-seed-ownership.js'; +import { normalizeManagedByVocab } from './normalize-managed-by.js'; +import { SecurityPlugin } from './security-plugin.js'; +import { PermissionEvaluator } from './permission-evaluator.js'; + +const SYS = { context: { isSystem: true } } as any; +const ORG = 'org_a'; +const ADMIN = 'usr_admin'; + +/** A business object with an `owner_id`, so the seed-ownership pass has work. */ +const PROBE_OBJECT: any = { + name: 'probe_deal', + label: 'Probe Deal', + fields: { + id: { type: 'text', label: 'Id', primary: true }, + name: { type: 'text', label: 'Name' }, + owner_id: { type: 'text', label: 'Owner' }, + }, +}; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(): Promise { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.find-shape-15598', + name: 'Find shape', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, + SysUserPosition, SysUserPermissionSet, + SysOrganization, SysUser, SysMember, PROBE_OBJECT, + ], + } as any); + await engine.syncSchemas(); + engines.push(engine); + await (engine as any).insert('sys_organization', { id: ORG, name: ORG }, SYS); + await (engine as any).insert('sys_user', { id: ADMIN, name: 'admin', email: 'admin@example.test' }, SYS); + return engine; +} + +/** Every `find` result a seam received, in order. */ +type Seen = unknown[]; + +/** + * The real engine, with `find` OBSERVED — forwarded verbatim, never replaced. + * + * A recorder rather than a double, deliberately: the value under test is the one + * the real engine produced, so anything that answered on its behalf would make + * every case below a statement about the fixture instead of about `ObjectQL`. + */ +function observed(engine: any, seen: Seen, extra: Record = {}): any { + return { + registry: engine.registry, + registerMiddleware: (...a: any[]) => engine.registerMiddleware?.(...a), + find: async (o: string, q?: any, opt?: any) => { + const r = await engine.find(o, q, opt); + seen.push(r); + return r; + }, + findOne: (o: string, q?: any, opt?: any) => engine.findOne(o, q, opt), + insert: (o: string, d: any, opt?: any) => engine.insert(o, d, opt), + insertMany: (o: string, d: any, opt?: any) => engine.insertMany?.(o, d, opt), + update: (o: string, d: any, opt?: any) => engine.update(o, d, opt), + delete: (o: string, id: any, opt?: any) => engine.delete(o, id, opt), + ...extra, + }; +} + +/** + * The reading this whole file exists to record: what the engine handed the seam. + * + * ⛔ `toBeInstanceOf(Array)` is not enough and `.length` is not enough — the + * removed limb read `x.records`, so the assertion that retires it has to say + * the value carries no such key of its own. The element check is the other + * half: the seventh seam's repair refuses a page carrying a non-row, and a + * reading that never looked at the elements could not tell that apart. + */ +function assertBareArrayPage(seen: Seen, label: string): void { + expect(seen.length, `${label}: the seam issued no read at all — nothing was driven`).toBeGreaterThan(0); + for (const page of seen) { + expect(Array.isArray(page), `${label}: expected a bare array`).toBe(true); + expect(Object.prototype.toString.call(page)).toBe('[object Array]'); + expect( + Object.prototype.hasOwnProperty.call(page as object, 'records'), + `${label}: the engine answered an ENVELOPE — the removed limb was NOT dead`, + ).toBe(false); + for (const row of page as unknown[]) { + expect(row === null ? 'null' : typeof row).toBe('object'); + } + } +} + +/** A logger that records what a seeder said, so a fallback can be asserted absent. */ +function recordingLogger() { + const warns: string[] = []; + const infos: string[] = []; + return { + warns, + infos, + logger: { + info: (m: string) => { infos.push(m); }, + warn: (m: string) => { warns.push(m); }, + debug: () => { /* noop */ }, + error: (m: string) => { warns.push(m); }, + }, + }; +} + +describe('[#15598] block 1 — seed-name-lookup readNamePage (the BATCHED page)', () => { + it('answers from one bare array, so the batched read is live and does not degrade', async () => { + const engine = await boot(); + await (engine as any).insert( + 'sys_permission_set', + { name: 'ps_batched', label: 'Batched', managed_by: 'admin', organization_id: ORG }, + SYS, + ); + const seen: Seen = []; + const { warns, logger } = recordingLogger(); + + const index = await buildExistingByName( + observed(engine, seen), 'sys_permission_set', ['ps_batched'], logger as any, ORG, + ); + const found = await index.get('ps_batched'); + + assertBareArrayPage(seen, 'readNamePage'); + // The batched page ANSWERED. This is the discriminating half: a normalizer + // that could not read the page degrades to the per-item oracle, which would + // still resolve `present` — so the result alone proves nothing here. + expect(warns.filter((w) => w.includes('falling back to one read per item'))).toEqual([]); + expect(found.status).toBe('present'); + }, 120_000); +}); + +describe('[#15598] block 2 — seed-name-lookup perItemIndex (the DEGRADATION read)', () => { + it('answers from one bare array on the per-item path', async () => { + const engine = await boot(); + await (engine as any).insert( + 'sys_permission_set', + { name: 'ps_peritem', label: 'Per item', managed_by: 'admin', organization_id: ORG }, + SYS, + ); + const seen: Seen = []; + // TRIGGER ONLY: the batched read is refused so the per-item path is taken. + // The per-item read itself reaches the real engine untouched — it is the + // subject, and nothing stands in for it. + const batchedRefused = observed(engine, seen, { + find: async (o: string, q?: any, opt?: any) => { + const name = q?.where?.name; + if (name && typeof name === 'object' && '$in' in name) throw new Error('batched read refused (trigger)'); + const r = await engine.find(o, q, opt); + seen.push(r); + return r; + }, + }); + + const index = await buildExistingByName(batchedRefused, 'sys_permission_set', ['ps_peritem'], undefined, ORG); + const found = await index.get('ps_peritem'); + + assertBareArrayPage(seen, 'perItemIndex'); + // `present`, never `unknown`: a seam that cannot read its page reports the + // read as un-happened, which is the outcome this pin discriminates against. + expect(found.status).toBe('present'); + }, 120_000); +}); + +describe('[#15598] block 3 — bootstrap-declared-permissions defaultLookup', () => { + it('answers from one bare array on the publish-materializer path', async () => { + const engine = await boot(); + const seen: Seen = []; + + // No `existingByName` — the ADR-0086 P2 publish materializer path, which is + // the only caller that reaches `defaultLookup`'s own read. + const outcome = await upsertPackagePermissionSet( + observed(engine, seen), + { name: 'ps_published', label: 'Published', objects: {} } as any, + 'com.acme.crm', + undefined, + {} as any, + ); + + assertBareArrayPage(seen, 'defaultLookup'); + // `unreadable` is what a seam that cannot read its page reports, and it is + // one-hot with `seeded` — so this pair is the discriminator. + expect(outcome.unreadable).toBe(0); + expect(outcome.seeded).toBe(1); + }, 120_000); +}); + +describe('[#15598] block 4 — auto-org-admin-grant tryFind', () => { + it('reads the set and the membership out of bare arrays, and grants on them', async () => { + const engine = await boot(); + // Both reads this reconciler makes are `tryFind`s through the repaired + // block: the org-admin set it points a grant AT, and the membership that + // QUALIFIES the pair. Either one reading empty produces `skipped`, so the + // `granted` verdict below is only reachable when both answered. + await (engine as any).insert( + 'sys_permission_set', + { id: 'ps_orgadmin', name: ORGANIZATION_ADMIN_NO_BYPASS, label: 'Org admin', managed_by: 'platform' }, + SYS, + ); + await (engine as any).insert('sys_member', { user_id: ADMIN, organization_id: ORG, role: 'admin' }, SYS); + const seen: Seen = []; + + const report = await reconcileOrgAdminGrant(observed(engine, seen), ADMIN, ORG, {} as any); + + assertBareArrayPage(seen, 'auto-org-admin-grant tryFind'); + expect(report.action).toBe('granted'); + const grants = await (engine as any).find('sys_user_permission_set', { where: { user_id: ADMIN } }, SYS); + expect(grants.length).toBe(1); + }, 120_000); +}); + +describe('[#15598] block 5 — claim-seed-ownership idsFrom (the PAGING fallback)', () => { + it('pages ids out of one bare array and re-owns the row', async () => { + const engine = await boot(); + await (engine as any).insert('probe_deal', { id: 'd1', name: 'Deal', owner_id: null }, SYS); + const seen: Seen = []; + + // TRIGGER ONLY: the whole-set write is refused for its per-row-hook budget, + // which is the sole path that reaches `idsFrom`. Refusing the WRITE keeps + // the READ — the subject — on the real engine, and costs no 10,000-row + // fixture to reach the same branch. + let refuseOnce = true; + const budgetRefusal = Object.assign(new Error('over the per-row-hook ceiling (trigger)'), { + code: 'ERR_BULK_PER_ROW_HOOK_LIMIT', + }); + const io = observed(engine, seen, { + update: async (o: string, d: any, opt?: any) => { + if (refuseOnce && opt?.multi && opt?.where) { refuseOnce = false; throw budgetRefusal; } + return engine.update(o, d, opt); + }, + }); + + await claimSeedOwnership(io, ADMIN, {} as any); + + assertBareArrayPage(seen, 'claim-seed-ownership readPage'); + // Re-owned THROUGH the id page: an `idsFrom` that read nothing stops the + // pass with "matched no rows to page" and leaves the row unowned. + const rows = await (engine as any).find('probe_deal', { where: { id: 'd1' } }, SYS); + expect(rows[0].owner_id).toBe(ADMIN); + }, 120_000); +}); + +describe('[#15598] block 6 — normalize-managed-by tryFind', () => { + it('reads the legacy rows out of one bare array and rewrites them', async () => { + const engine = await boot(); + await (engine as any).insert('sys_position', { name: 'pos_legacy', label: 'Legacy', managed_by: 'system' }, SYS); + const seen: Seen = []; + + const counts = await normalizeManagedByVocab(observed(engine, seen), {}); + + assertBareArrayPage(seen, 'normalize-managed-by tryFind'); + // The rewrite happened, which it cannot if the scan read nothing. + expect(counts.positions).toBe(1); + const rows = await (engine as any).find('sys_position', { where: { name: 'pos_legacy' } }, SYS); + expect(rows[0].managed_by).toBe('platform'); + }, 120_000); +}); + +describe('[#15598] block 7 — security-plugin sys_permission_set loader (the DROP shape)', () => { + /** Boot the real plugin against `engine`, and hand back its private loader. */ + async function loaderOver(engine: any, seen: Seen, findOverride?: (o: string, q?: any, opt?: any) => Promise) { + const plugin = new SecurityPlugin(); + const svc = observed(engine, seen, findOverride ? { find: findOverride } : {}); + const { warns, logger } = recordingLogger(); + const ctx: any = { + logger, + registerService: () => { /* noop */ }, + registerMiddleware: () => { /* noop */ }, + getService: (n: string) => { + if (n === 'objectql') return svc; + if (n === 'metadata') return { list: async () => [] }; + if (n === 'manifest') return { register: () => { /* noop */ } }; + return undefined; + }, + }; + await plugin.init(ctx); + await plugin.start(ctx); + const loader = (plugin as any).dbLoaderFor?.(ORG); + expect(typeof loader, 'the loader was never built — the boot bailed out').toBe('function'); + return { loader: loader as (names: string[]) => Promise, warns }; + } + + it('loads the DB-authored set out of one bare array', async () => { + const engine = await boot(); + await (engine as any).insert( + 'sys_permission_set', + { name: 'ps_db', label: 'DB authored', managed_by: 'admin', organization_id: ORG, active: true }, + SYS, + ); + const seen: Seen = []; + const { loader } = await loaderOver(engine, seen); + seen.length = 0; // isolate the loader's OWN read + + const sets = await loader(['ps_db']); + + assertBareArrayPage(seen, 'dbLoaderFor'); + expect(sets.map((s: any) => s.name)).toEqual(['ps_db']); + }, 120_000); + + it('PROPAGATES a thrown read instead of swallowing it into "no permission sets"', async () => { + const engine = await boot(); + const seen: Seen = []; + const outage = Object.assign(new Error("Datasource 'primary' is declared but not connected"), { + code: 'ERR_DATASOURCE_UNAVAILABLE', + }); + const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { + if (o === 'sys_permission_set' && q?.where?.name?.$in) throw outage; + return engine.find(o, q, opt); + }); + + // The whole repair: this used to resolve `[]`. An outage and an empty + // catalog are different facts and must not leave by the same door. + await expect(loader(['ps_db'])).rejects.toBe(outage); + }, 120_000); + + it('REFUSES a non-array result with an envelope, rather than inventing an empty page', async () => { + const engine = await boot(); + const seen: Seen = []; + const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { + // The #13706 shape, planted deliberately: a `find()` that resolves an + // ENVELOPE. It is what the removed limb claimed to handle, and the point + // of the repair is that it is refused rather than silently normalized. + if (o === 'sys_permission_set' && q?.where?.name?.$in) return { records: [{ name: 'ps_db' }] } as any; + return engine.find(o, q, opt); + }); + + // ⛔ Not `toThrow()` alone — the envelope is the assertion. A bare "it threw" + // passes against any accident on this path. + await expect(loader(['ps_db'])).rejects.toMatchObject({ + code: 'DATABASE_ERROR', + status: 500, + name: 'PermissionSetReadUnansweredError', + }); + }, 120_000); + + it('REFUSES a page carrying a non-row, which the trailing filter used to drop in silence', async () => { + const engine = await boot(); + const seen: Seen = []; + const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { + if (o === 'sys_permission_set' && q?.where?.name?.$in) return ['ps_db'] as any; + return engine.find(o, q, opt); + }); + + await expect(loader(['ps_db'])).rejects.toMatchObject({ + code: 'DATABASE_ERROR', + status: 500, + name: 'PermissionSetReadUnansweredError', + }); + }, 120_000); + + it('stays FAIL-CLOSED through the evaluator, and re-arms the #2565 warn the swallow had made unreachable', async () => { + const engine = await boot(); + const seen: Seen = []; + const outage = Object.assign(new Error("Datasource 'primary' is declared but not connected"), { + code: 'ERR_DATASOURCE_UNAVAILABLE', + }); + const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { + if (o === 'sys_permission_set' && q?.where?.name?.$in) throw outage; + return engine.find(o, q, opt); + }); + + const warns: Array<{ msg: string; meta: any }> = []; + const resolved = await new PermissionEvaluator().resolvePermissionSets( + ['ps_db'], + { list: async () => [] }, + [], + loader, + { logger: { warn: (msg: string, meta?: any) => { warns.push({ msg, meta }); } } }, + ); + + // Enforcement is UNCHANGED — the unresolved set still grants nothing. + expect(resolved).toEqual([]); + // What changed is that the loss is now sayable. While the loader swallowed + // its own read failure this warn could never fire, so a DB outage and an + // empty catalog produced identical, undiagnosable 403s. + expect(warns.map((w) => w.msg).join('\n')).toContain('db lookup failed'); + }, 120_000); +}); diff --git a/packages/plugins/plugin-security/src/errors.ts b/packages/plugins/plugin-security/src/errors.ts index 091b7e286c..0c43033c31 100644 --- a/packages/plugins/plugin-security/src/errors.ts +++ b/packages/plugins/plugin-security/src/errors.ts @@ -243,6 +243,61 @@ export class MaskedValueWriteError extends Error { } } +/** + * The DB-authored permission-set read did not answer → `500 DATABASE_ERROR`. + * + * ## Why this exists at all — a read that did not happen is not an empty answer + * + * `SecurityPlugin`'s `sys_permission_set` loader is the ENFORCEMENT plane's door + * to DB-authored sets. It used to map three different facts onto one output, the + * empty list: the read succeeded and the catalog is empty; the read THREW; the + * read resolved something the loader could not read. The last two are invented + * answers, and the invention is not benign — "this principal has no permission + * sets" silently withdraws grants that exist, while every request keeps looking + * completely normal. That is the failure the loader's own neighbouring comment + * calls out as the thing not to do: dropping a row "revokes standing access with + * no signal at the moment of loss". + * + * ## Why REFUSING is the right direction, and not this seam's own invention + * + * The consumer already declares the handling. `PermissionEvaluator + * .resolvePermissionSets` catches a throwing loader, keeps the request + * fail-closed (the unresolved sets grant nothing — unchanged), and NAMES the + * failure in a warn, because "without the warn, a transient DB error makes + * custom permission sets silently vanish and the resulting 403s are + * undiagnosable" (#2565). A loader that swallowed its own read failure made + * that warn unreachable: the diagnostic the repo had already built could never + * fire. Throwing restores it. It also matches the maintainer's 2026-08-11 + * store-fault ruling for this plugin — FAIL-CLOSED, and a fault PROPAGATES; + * absent means absent and only absent. + * + * ⚠️ Enforcement is unchanged in both directions: before and after, a read that + * did not answer grants nothing. What changes is that it is now DISTINGUISHABLE + * from an empty catalog. + * + * `DATABASE_ERROR` comes from ADR-0112's closed vocabulary rather than a new + * spelling — the condition is exactly "a database operation did not answer". + * The status is declared for consistency with this file's other classes, not + * because the error is expected to reach a transport: its only consumer catches + * it one frame up. + */ +export class PermissionSetReadUnansweredError extends Error { + readonly code = 'DATABASE_ERROR'; + readonly status = 500; + readonly statusCode = 500; + /** The set names the unanswered read was asked for. */ + readonly names: readonly string[]; + constructor(names: readonly string[], detail: string) { + super( + `[Security] The sys_permission_set read for [${names.join(', ')}] did not answer: ${detail}. ` + + `Refusing rather than reporting an empty catalog — an unanswered read reported as "no permission ` + + `sets" silently withdraws grants that exist while every request still looks normal.`, + ); + this.name = 'PermissionSetReadUnansweredError'; + this.names = [...names]; + } +} + export function isPermissionDeniedError(e: unknown): e is PermissionDeniedError { if (!e || typeof e !== 'object') return false; const anyE = e as any; diff --git a/packages/plugins/plugin-security/src/normalize-managed-by.ts b/packages/plugins/plugin-security/src/normalize-managed-by.ts index 0a0f5b96cf..fa32719306 100644 --- a/packages/plugins/plugin-security/src/normalize-managed-by.ts +++ b/packages/plugins/plugin-security/src/normalize-managed-by.ts @@ -51,8 +51,10 @@ interface NormalizeOptions { async function tryFind(ql: any, object: string, where: any): Promise { try { const rows = await ql.find(object, { where, limit: 10_000, fields: ['id', 'managed_by'] }, { context: SYSTEM_CTX }); + // Bare array, driven — `engine-find-bare-array.pin.test.ts` boots a real + // engine over a real `SqlDriver` and pins this seam. The `{ records }` limb + // that stood here was dead code that read as a contract. if (Array.isArray(rows)) return rows; - if (Array.isArray(rows?.records)) return rows.records; return []; } catch { return []; diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 5aef543375..44f433c709 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -110,6 +110,7 @@ import { DetailRecordNotFoundError, MasterReferenceMissingError, MaskedValueWriteError, + PermissionSetReadUnansweredError, } from './errors.js'; import { assertEngineOwnedWriteAllowed } from './system-write-guard.js'; import { bootstrapPlatformAdmin, shouldReplayBootstrapFor } from './bootstrap-platform-admin.js'; @@ -746,6 +747,53 @@ function userFacingDenialMessage( return renderOperationMessage({ messageKey }, { locale, translate }); } +/** + * The `sys_permission_set` page the enforcement-plane loader asked for, or a + * REFUSAL — never a different empty. + * + * ## The shape, driven rather than declared + * + * `ObjectQL.find` resolves a BARE array of row objects. That is MEASURED, not + * read off `IDataEngine.find`'s declared `Promise`: a declared type is + * not proof here, because this repo also carries a `find()` that resolves a + * `QueryResult` envelope and never an array. `engine-find-bare-array.pin.test.ts` + * boots a real `ObjectQL` over a real `SqlDriver`, starts this plugin against + * it, and pins what this very loader receives — on a populated page and an + * empty one. The `{ records }` limb this function replaced was therefore + * unreachable: dead code that read as a contract. + * + * ## Why anything else REFUSES instead of returning `[]` + * + * This is the enforcement plane. A value this loader cannot read is a read that + * did not answer, and the one thing it must never become is "this principal has + * no permission sets" — a statement that silently withdraws grants that exist + * while the request keeps looking normal. `PermissionEvaluator + * .resolvePermissionSets` catches the refusal one frame up, keeps the request + * fail-closed exactly as before, and reports it once (#2565's warn, which the + * old swallow made unreachable). + * + * A non-object ELEMENT refuses for the same reason, and that half is the + * repair's other direction: it used to be dropped in silence by the trailing + * `r?.name === name` filter, which is the same invention one row at a time. + */ +function permissionSetPageOrRefuse(rows: unknown, names: readonly string[]): any[] { + if (!Array.isArray(rows)) { + throw new PermissionSetReadUnansweredError( + names, + `the engine resolved ${rows === null ? 'null' : typeof rows}, not an array of rows`, + ); + } + for (const row of rows) { + if (!row || typeof row !== 'object') { + throw new PermissionSetReadUnansweredError( + names, + `the page carried a ${row === null ? 'null' : typeof row} where a row object was contracted`, + ); + } + } + return rows; +} + export class SecurityPlugin implements Plugin { name = 'com.objectstack.security'; /** @@ -1279,17 +1327,35 @@ export class SecurityPlugin implements Plugin { // multiple of the names asked for, never unbounded. const dbLoaderFor = ql ? (organizationId?: string) => async (names: string[]) => { - let rows: any; - try { - rows = await ql.find( - 'sys_permission_set', - { where: { name: { $in: names } }, limit: Math.max(names.length * 4, 20) }, - { context: seedCtx(organizationId) }, - ); - } catch { - rows = []; - } - const fetched = Array.isArray(rows) ? rows : rows?.records ?? []; + // ⛔ NO `catch` here, and ⛔ no invented empty below — this seam is + // the DROP-shaped half of this cleanup and is repaired in the OPPOSITE + // direction from the dead limbs elsewhere in this plugin. + // + // Three distinct facts used to leave here as one value, `[]`: the read + // succeeded on an empty catalog; the read THREW; the read resolved + // something this loader could not read. Only the first is an answer. + // The other two were inventions, and this is the enforcement plane — + // "no permission sets" silently withdraws grants that EXIST while + // every request still looks completely normal. The residue comment + // below already names that failure ("revokes standing access with no + // signal at the moment of loss"); the swallow above it produced the + // same loss for a whole page at once. + // + // The refusal is not this seam's invention: `resolvePermissionSets` + // ALREADY declares how a throwing loader is handled — it catches, the + // unresolved sets grant nothing (fail closed, unchanged), and the + // failure is named in a warn, because without it "a transient DB error + // makes custom permission sets silently vanish and the resulting 403s + // are undiagnosable" (#2565). Swallowing here made that warn + // UNREACHABLE. Letting the read fault propagate is what re-arms a + // diagnostic this repo had already built, and it is the direction the + // 2026-08-11 store-fault ruling settles: a fault propagates. + const rows = await ql.find( + 'sys_permission_set', + { where: { name: { $in: names } }, limit: Math.max(names.length * 4, 20) }, + { context: seedCtx(organizationId) }, + ); + const fetched = permissionSetPageOrRefuse(rows, names); // One row per NAME: this organization's own where it has one, an // organization-less leftover only where it does not. // diff --git a/packages/plugins/plugin-security/src/seed-name-lookup.ts b/packages/plugins/plugin-security/src/seed-name-lookup.ts index 0775e020d9..83629f9870 100644 --- a/packages/plugins/plugin-security/src/seed-name-lookup.ts +++ b/packages/plugins/plugin-security/src/seed-name-lookup.ts @@ -281,13 +281,20 @@ async function readNamePage( } catch { return { ok: false, cause: 'unreadable', budget }; } - // Some drivers wrap the page (`{ records }`) — a wrapped array is still an - // answer. Anything else (undefined/null/a scalar) is not. - const page: any[] | null = Array.isArray(rows) - ? rows - : Array.isArray(rows?.records) - ? (rows.records as any[]) - : null; + // `ObjectQL.find` resolves a BARE array — MEASURED, not read off the + // declared type. `IDataEngine.find` says `Promise`, and a declared type + // is not proof here: this repo's own `ObjectStackAdapter.find()` resolves a + // `QueryResult` envelope and never an array. So the engine behind this seam + // was booted for real (a real `ObjectQL` over a real `SqlDriver`) and driven, + // and it answered `[object Array]` with no own `records` key on a populated + // page and on an empty one alike — see `engine-find-bare-array.pin.test.ts`, + // which pins this seam. The `{ records }` limb this used to carry was + // therefore unreachable: dead code that read as a contract. + // + // What is NOT changed: a non-array is still `null` here, and `null` is still + // `unreadable` below. Removing a dead limb must not quietly convert a seam + // that GAPS into one that invents an empty answer. + const page: any[] | null = Array.isArray(rows) ? rows : null; if (page === null) return { ok: false, cause: 'unreadable', budget }; if (page.length > budget) return { ok: false, cause: 'truncated', budget }; return { ok: true, rows: page }; @@ -325,7 +332,11 @@ function perItemIndex( } catch { return UNKNOWN; } - const list = Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : null; + // Bare array, driven — the same measurement `readNamePage` above records, + // taken at THIS seam too (`engine-find-bare-array.pin.test.ts` drives the + // per-item path separately, because one seam's reading is not the other's). + // A non-array stays `UNKNOWN`: a read that did not answer is never "absent". + const list = Array.isArray(rows) ? rows : null; if (list === null) return UNKNOWN; return resolveForOrganization(list, organizationId); }, From 31f059524931a652024434ce61fd52a6a3ca75bf Mon Sep 17 00:00:00 2001 From: os-warren Date: Sat, 5 Sep 2026 09:56:41 +0000 Subject: [PATCH 2/3] test(plugin-security): route the pin's engine seam through the producer predicates, and re-anchor the rotted system-context rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gate repairs on the #15598 pins, neither a behaviour change. `check:engine-double-contract`: the pin file's `observed()` recorder forwards every verb to a real `ObjectQL`, but a seam that merely forwards is exactly the shape that reads as "not a double" and then admits a call the real engine would refuse. Its `update`/`findOne`/`delete` now open with the producer's own predicates (`assertEngineUpdateDispatch` / `assertEngineFindOnePredicate` / `assertEngineDeleteDispatch` from `@objectstack/metadata-core`), and the ledger learns about the newly pinned double — the ratchet grows, it is not weakened. `check:system-context-census`: line rot, not a finding. The loader repair added a net 66 lines to `security-plugin.ts` (77 added / 11 deleted), and every rotted anchor the census reported was off by exactly 66. Repaired with the gate's own `--fix`; only line numbers in `content/docs/permissions/system-context.mdx` change, no prose. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- content/docs/permissions/system-context.mdx | 14 +++++----- .../src/engine-find-bare-array.pin.test.ts | 26 ++++++++++++++++--- scripts/engine-double-contract.pinned.json | 10 +++++++ 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 6d1f6564ca..b1f3eb980d 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -87,14 +87,14 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 1 | **The whole security middleware short-circuits** before any gate runs | plugin-security | Get: every CRUD/FLS/tenant/owner gate below skipped in one branch. Lose: all of rows 2–6 at once — this is the single largest behaviour on the page | `security-plugin.ts:1615` | -| 2 | **`owner_id` is not auto-stamped on INSERT** (the step 3.5 anchor guard is inside the block row 1 skips) | plugin-security | Lose: the row lands `owner_id = NULL`, so the default `owner_only_writes` policy hides it **from its own creator**. Get: nothing — this is a gap, not a capability | guard at `security-plugin.ts:2541` (the step 3.5 block), skipped by `:1615` | -| 3 | Row-level read filter resolves to "no filter" | plugin-security | Get: unscoped reads. Lose: row-level scoping entirely | `security-plugin.ts:4344` | -| 4 | Field-level security returns **all** fields | plugin-security | Get: every column readable. Lose: field masking | `security-plugin.ts:4495` | -| 5 | Export permission granted unconditionally | plugin-security | Get: `canExport` is `true` | `security-plugin.ts:4573` | -| 6 | Write bypass = `true`, effective write scope = `org` | plugin-security | Get: widest write scope without holding any capability | `security-plugin.ts:1442`, `:1464` | +| 1 | **The whole security middleware short-circuits** before any gate runs | plugin-security | Get: every CRUD/FLS/tenant/owner gate below skipped in one branch. Lose: all of rows 2–6 at once — this is the single largest behaviour on the page | `security-plugin.ts:1681` | +| 2 | **`owner_id` is not auto-stamped on INSERT** (the step 3.5 anchor guard is inside the block row 1 skips) | plugin-security | Lose: the row lands `owner_id = NULL`, so the default `owner_only_writes` policy hides it **from its own creator**. Get: nothing — this is a gap, not a capability | guard at `security-plugin.ts:2607` (the step 3.5 block), skipped by `:1681` | +| 3 | Row-level read filter resolves to "no filter" | plugin-security | Get: unscoped reads. Lose: row-level scoping entirely | `security-plugin.ts:4410` | +| 4 | Field-level security returns **all** fields | plugin-security | Get: every column readable. Lose: field masking | `security-plugin.ts:4561` | +| 5 | Export permission granted unconditionally | plugin-security | Get: `canExport` is `true` | `security-plugin.ts:4639` | +| 6 | Write bypass = `true`, effective write scope = `org` | plugin-security | Get: widest write scope without holding any capability | `security-plugin.ts:1508`, `:1530` | | 7 | Metadata-plane schema masking exempt (ADR-0106 D4) | metadata-core | Get: unmasked object schema. Note: the exemption is a **caller** property — it short-circuits before the security service is consulted | `object-schema-fls.ts:228` | -| 8 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `security-plugin.ts:3857` | +| 8 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `security-plugin.ts:3923` | | 9 | Anonymous-deny treats the caller as authenticated | core | Get: passes the 401 seam with no `userId` | `anonymous-deny.ts:154` | | 10 | Permission-set projection middleware skipped | plugin-security | Lose: projection of permission-set-derived columns | `permission-set-projection.ts:1015` | | 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1412` | diff --git a/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts b/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts index 919f318a18..0fda95893b 100644 --- a/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts +++ b/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts @@ -47,6 +47,11 @@ import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; import { ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; +import { + assertEngineUpdateDispatch, + assertEngineFindOnePredicate, + assertEngineDeleteDispatch, +} from '@objectstack/metadata-core'; import { buildExistingByName } from './seed-name-lookup.js'; import { upsertPackagePermissionSet } from './bootstrap-declared-permissions.js'; @@ -113,6 +118,11 @@ type Seen = unknown[]; * A recorder rather than a double, deliberately: the value under test is the one * the real engine produced, so anything that answered on its behalf would make * every case below a statement about the fixture instead of about `ObjectQL`. + * + * ⚠️ It is still a seam, and a seam that merely FORWARDS is exactly the shape + * that reads as "not a double" and then admits a call the real engine would + * refuse. So the three dispatch-shaped verbs open with the PRODUCER's own + * predicates (`check:engine-double-contract`) rather than a hand-mirrored guard. */ function observed(engine: any, seen: Seen, extra: Record = {}): any { return { @@ -123,11 +133,20 @@ function observed(engine: any, seen: Seen, extra: Record = {}): any seen.push(r); return r; }, - findOne: (o: string, q?: any, opt?: any) => engine.findOne(o, q, opt), + findOne: (o: string, q?: any, opt?: any) => { + assertEngineFindOnePredicate(o, q); + return engine.findOne(o, q, opt); + }, insert: (o: string, d: any, opt?: any) => engine.insert(o, d, opt), insertMany: (o: string, d: any, opt?: any) => engine.insertMany?.(o, d, opt), - update: (o: string, d: any, opt?: any) => engine.update(o, d, opt), - delete: (o: string, id: any, opt?: any) => engine.delete(o, id, opt), + update: (o: string, d: any, opt?: any) => { + assertEngineUpdateDispatch(d, opt); + return engine.update(o, d, opt); + }, + delete: (o: string, id: any, opt?: any) => { + assertEngineDeleteDispatch(opt); + return engine.delete(o, id, opt); + }, ...extra, }; } @@ -292,6 +311,7 @@ describe('[#15598] block 5 — claim-seed-ownership idsFrom (the PAGING fallback }); const io = observed(engine, seen, { update: async (o: string, d: any, opt?: any) => { + assertEngineUpdateDispatch(d, opt); if (refuseOnce && opt?.multi && opt?.where) { refuseOnce = false; throw budgetRefusal; } return engine.update(o, d, opt); }, diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 996640222d..a5e04900b9 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2551,6 +2551,16 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/explain-engine.test.ts", "verb": "findOne", From 4cc60aa7a72ba5094a26bbbaef2e8fff3aabf23c Mon Sep 17 00:00:00 2001 From: os-warren Date: Sat, 5 Sep 2026 13:28:09 +0000 Subject: [PATCH 3/3] =?UTF-8?q?docs(changeset):=20qualify=20the=20enforcem?= =?UTF-8?q?ent=20claim=20=E2=80=94=20unchanged=20on=20every=20reachable=20?= =?UTF-8?q?input,=20fail-closed=20on=20the=20two=20that=20are=20not?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clause-② review measured `PermissionEvaluator.resolvePermissionSets` through this PR's own `dbLoaderFor(ORG)` over a real ObjectQL/SqlDriver, at HEAD and at the merge-base blob, on five engine conditions. A healthy page, a thrown read and `undefined` resolve identically before and after — every result the shipped engine actually produces. An envelope and a page carrying a non-object element do NOT: they granted at base and refuse at HEAD. Both are fail-closed and both are unreachable on the measured engine, which is what the eleven pins establish, so this is a declared narrowing rather than a discovered move — but "unchanged in both directions" is an unqualified claim about behaviour and it sits in the changeset, which feeds release notes. The reviewer's wording replaces it verbatim. ⛔ No source change: the refusal itself was reviewed and passed exactly as it stands. Changeset and PR body carry identical wording. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/quiet-pans-repair.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/quiet-pans-repair.md b/.changeset/quiet-pans-repair.md index 526c903873..b88b4b5d68 100644 --- a/.changeset/quiet-pans-repair.md +++ b/.changeset/quiet-pans-repair.md @@ -6,4 +6,4 @@ Remove seven dead `{ records }` union-normalizer limbs on engine `find()` result Six seams in this plugin normalized an engine read as `Array.isArray(x) ? x : x.records`. The envelope limb was unreachable: `ObjectQL.find` resolves a bare array of row objects, measured by booting a real engine over a real `SqlDriver` and driving each seam through the shipped function that owns it, rather than inferred from `IDataEngine.find`'s declared `Promise` (a declared type is not proof — this repo also has a `find()` that resolves an envelope). Each seam keeps its existing disposition for a non-array; only the dead limb is gone. -The seventh is repaired in the opposite direction. `SecurityPlugin`'s `sys_permission_set` loader mapped three different facts onto one value: a read that succeeded on an empty catalog, a read that threw, and a read that resolved something it could not read all left as `[]`. On the enforcement plane that silently withdraws grants that exist while every request still looks normal, and it made `PermissionEvaluator`'s existing "db lookup failed" warning unreachable — so a transient database error and an empty catalog produced identical, undiagnosable 403s. The loader now lets the read fault propagate and refuses an unreadable result with `DATABASE_ERROR`. Enforcement is unchanged in both directions: an unanswered read still grants nothing. What changes is that it is now reported instead of silent. +The seventh is repaired in the opposite direction. `SecurityPlugin`'s `sys_permission_set` loader mapped three different facts onto one value: a read that succeeded on an empty catalog, a read that threw, and a read that resolved something it could not read all left as `[]`. On the enforcement plane that silently withdraws grants that exist while every request still looks normal, and it made `PermissionEvaluator`'s existing "db lookup failed" warning unreachable — so a transient database error and an empty catalog produced identical, undiagnosable 403s. The loader now lets the read fault propagate and refuses an unreadable result with `DATABASE_ERROR`. Enforcement is unchanged for every result the shipped engine produces; an envelope or a non-row element now refuses (fail-closed) where the old code read through it. An unanswered read still grants nothing; what changes is that it is now reported instead of silent.