From eca6a2cf36b97cf78afbcb56df7431f1c964d063 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:23:02 +0000 Subject: [PATCH 1/4] wip(plugin-auth): remove the dead {records}/{data} envelope limbs, refuse on a malformed permission-set row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 14 union-normalizer blocks (13 `records`-limb, 1 `data`-limb), all reading the same concrete engine: the ObjectQL instance registered as the `objectql`/`data` kernel service. Driven rather than inferred — every read answers a bare array. The `settleSelfRegistrationGrant` block is the opposite defect (#15092's DROP shape) and is fixed in the opposite direction: its trailing filter no longer silently drops a malformed row. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../plugins/plugin-auth/src/auth-manager.ts | 50 +++++++++++++------ .../src/backfill-account-issuer.ts | 3 +- .../src/boot-sign-in-reachability.ts | 20 ++++---- .../plugin-auth/src/dev-admin-seed-gate.ts | 42 +++++++--------- .../src/ensure-default-organization.ts | 3 +- .../plugin-auth/src/member-role-canonical.ts | 3 +- .../plugin-auth/src/phone-sms-texts.ts | 12 ++--- .../plugin-auth/src/reconcile-membership.ts | 3 +- .../plugin-auth/src/tenancy-service.ts | 3 +- .../src/walled-owner-verification-path.ts | 11 ++-- 10 files changed, 73 insertions(+), 77 deletions(-) diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index c3b78c6798..30ee873658 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -4180,10 +4180,9 @@ export class AuthManager { if (!engine || typeof (engine as any).find !== 'function') return false; try { const reader = withSystemReadContext(engine) as any; - const raw = await reader.find(SystemObjectName.USER, { + const rows: any[] = await reader.find(SystemObjectName.USER, { limit: AuthManager.BOOTSTRAP_USER_PROBE_LIMIT, }); - const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : []; if (rows.some(isHumanUserRow)) return false; // A page that came back FULL of non-human rows cannot prove there is no // human on the next page — fail closed rather than guess. @@ -4256,14 +4255,13 @@ export class AuthManager { const seenIds = new Set(); for (let pageIndex = 0; pageIndex < AuthManager.PENDING_INVITATION_PROBE_MAX_PAGES; pageIndex++) { const offset = pageIndex * page; - const raw = await reader.find('sys_invitation', { + const rows: any[] = await reader.find('sys_invitation', { where: { status: 'pending', email: target }, limit: page, // Omitted on the first page so the ordinary single-page read sends // exactly the option shape every driver already answers. ...(offset > 0 ? { offset } : {}), }); - const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : []; const idsBefore = seenIds.size; for (const row of rows) { if (row?.id != null) seenIds.add(row.id); @@ -4340,11 +4338,10 @@ export class AuthManager { if (!target) return false; try { const reader = withSystemReadContext(engine) as any; - const raw = await reader.find(SystemObjectName.USER, { + const rows: any[] = await reader.find(SystemObjectName.USER, { where: { email: target }, limit: AuthManager.EXISTING_USER_PROBE_LIMIT, }); - const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : []; return rows.some( (row) => typeof row?.email === 'string' && row.email.trim().toLowerCase() === target, ); @@ -4382,8 +4379,7 @@ export class AuthManager { if (!engine || typeof (engine as any).find !== 'function') return []; try { const reader = withSystemReadContext(engine) as any; - const raw = await reader.find('sys_permission_set', { where: { name: setName }, limit: 50 }); - return Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : []; + return await reader.find('sys_permission_set', { where: { name: setName }, limit: 50 }); } catch { return []; } @@ -4606,9 +4602,36 @@ export class AuthManager { } catch { organizationId = null; } - const rows = (await this.findPermissionSetRows(staged.setName)).filter( - (r) => r?.active !== false && typeof r?.id === 'string' && r.id, + // ACTIVE is a selection predicate — a deactivated set legitimately does + // not resolve, so it stays a filter. A missing or blank `id` is NOT a + // selection: it is a MALFORMED row, and dropping it silently is the + // opposite defect from a dead limb, wrong in two ways that both look + // normal from outside. (1) The family narrows to nothing and the report + // below blames "no active row named X" while an active row named X is + // sitting right there — and that report is the only signal, because + // nothing retries this. (2) Worse, when the malformed row is the + // ORG-SCOPED one, the `organization_id == null` arm below then resolves + // the GLOBAL set and grants a permission set the organization never + // declared, and the `rows.length === 1` arm can fire on a family that + // was never singular — both computed AFTER the silent removal, so the + // "unambiguous single row" this code believes it selected is not that. + // So a malformed candidate REFUSES the grant and names itself, which is + // the same gap-not-empty-answer direction the rest of this method takes. + const candidates = (await this.findPermissionSetRows(staged.setName)).filter( + (r) => r?.active !== false, ); + const malformed = candidates.filter((r) => !(typeof r?.id === 'string' && r.id)); + if (malformed.length > 0) { + this.reportUngrantedSelfRegistrant( + userId, + staged.setName, + `${malformed.length} active sys_permission_set row(s) named '${staged.setName}' carry no usable id, ` + + 'so which row this grant would resolve to cannot be decided — refusing rather than ' + + 'silently dropping them and granting whichever row is left', + ); + return; + } + const rows = candidates; const row = (organizationId ? rows.find((r) => r?.organization_id === organizationId) : undefined) ?? rows.find((r) => r?.organization_id == null) ?? @@ -4623,15 +4646,10 @@ export class AuthManager { ); return; } - const existingRaw = await sys.find('sys_user_permission_set', { + const existing: any[] = await sys.find('sys_user_permission_set', { where: { user_id: userId, permission_set_id: row.id }, limit: 1, }); - const existing: any[] = Array.isArray(existingRaw) - ? existingRaw - : Array.isArray(existingRaw?.records) - ? existingRaw.records - : []; if (existing.length > 0) return; const id = `ups_${Date.now().toString(36)}${Math.random().toString(36).slice(2, 10)}`; await sys.insert('sys_user_permission_set', { diff --git a/packages/plugins/plugin-auth/src/backfill-account-issuer.ts b/packages/plugins/plugin-auth/src/backfill-account-issuer.ts index 2e853657fe..376a1a2a22 100644 --- a/packages/plugins/plugin-auth/src/backfill-account-issuer.ts +++ b/packages/plugins/plugin-auth/src/backfill-account-issuer.ts @@ -118,8 +118,7 @@ export function oauthIssuerFor(providerId: string): string { async function tryFind(ql: any, object: string, where: any, limit: number): Promise { try { - const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); - return Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : []; + return await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); } catch { return []; } diff --git a/packages/plugins/plugin-auth/src/boot-sign-in-reachability.ts b/packages/plugins/plugin-auth/src/boot-sign-in-reachability.ts index cd094e5a30..3b39367002 100644 --- a/packages/plugins/plugin-auth/src/boot-sign-in-reachability.ts +++ b/packages/plugins/plugin-auth/src/boot-sign-in-reachability.ts @@ -151,7 +151,11 @@ export const NO_SIGN_IN_ACCOUNT_AT_BOOT = 'no_sign_in_account_at_boot'; * this, so the two probes cannot drift apart on what they require of a store. */ export interface BootProbeEngine { - find(object: string, query: Record, options?: unknown): Promise; + find( + object: string, + query: Record, + options?: unknown, + ): Promise>>; } /** @@ -179,12 +183,6 @@ export interface SignInReachabilityFacts { const SYSTEM = { context: { isSystem: true } }; -const asRows = (raw: unknown): Record[] => { - if (Array.isArray(raw)) return raw as Record[]; - const records = (raw as { records?: unknown } | null | undefined)?.records; - return Array.isArray(records) ? (records as Record[]) : []; -}; - const usable = (engine: BootProbeEngine | undefined): engine is BootProbeEngine => !!engine && typeof engine.find === 'function'; @@ -203,8 +201,10 @@ export async function probeHumanUsersPresence( ): Promise { if (!usable(engine)) return 'unknown'; try { - const page = asRows( - await engine.find(SystemObjectName.USER, { limit: HUMAN_POPULATION_PROBE_LIMIT }, SYSTEM), + const page = await engine.find( + SystemObjectName.USER, + { limit: HUMAN_POPULATION_PROBE_LIMIT }, + SYSTEM, ); // A full page of non-humans cannot prove absence: it reads as populated. const humansExist = page.some(isHumanUserRow) || page.length >= HUMAN_POPULATION_PROBE_LIMIT; @@ -225,7 +225,7 @@ export async function probeSignInAccountsPresence( ): Promise { if (!usable(engine)) return 'unknown'; try { - const rows = asRows(await engine.find(SystemObjectName.ACCOUNT, { limit: 1 }, SYSTEM)); + const rows = await engine.find(SystemObjectName.ACCOUNT, { limit: 1 }, SYSTEM); return rows.length > 0 ? 'present' : 'absent'; } catch { return 'unknown'; diff --git a/packages/plugins/plugin-auth/src/dev-admin-seed-gate.ts b/packages/plugins/plugin-auth/src/dev-admin-seed-gate.ts index 58f39341ca..d3fbc7505d 100644 --- a/packages/plugins/plugin-auth/src/dev-admin-seed-gate.ts +++ b/packages/plugins/plugin-auth/src/dev-admin-seed-gate.ts @@ -63,7 +63,11 @@ import { SystemObjectName } from '@objectstack/spec/system'; /** The bounded reads this probe performs — every data engine satisfies them. */ export interface DevAdminSeedProbeEngine { - find(object: string, query: Record, options?: unknown): Promise; + find( + object: string, + query: Record, + options?: unknown, + ): Promise>>; } /** @@ -94,12 +98,6 @@ const SYSTEM_READ = { context: { isSystem: true } } as const; /** better-auth's local password provider — the one the dev seed provisions. */ export const CREDENTIAL_PROVIDER_ID = 'credential'; -function asRows(raw: unknown): Record[] { - if (Array.isArray(raw)) return raw as Record[]; - const records = (raw as { records?: unknown } | null | undefined)?.records; - return Array.isArray(records) ? (records as Record[]) : []; -} - /** * Decide whether the dev-admin seed should provision on this boot. * @@ -124,34 +122,28 @@ export async function decideDevAdminSeedGate( const spellings = [...new Set([seedEmail, seedEmail.trim().toLowerCase()])]; const seedUserIds = new Set(); for (const spelling of spellings) { - for (const row of asRows( - await engine.find( - SystemObjectName.USER, - { where: { email: spelling }, limit: 5 }, - SYSTEM_READ, - ), + for (const row of await engine.find( + SystemObjectName.USER, + { where: { email: spelling }, limit: 5 }, + SYSTEM_READ, )) { if (row?.id != null) seedUserIds.add(row.id); } } for (const userId of seedUserIds) { - const accounts = asRows( - await engine.find( - SystemObjectName.ACCOUNT, - { where: { user_id: userId }, limit: 1 }, - SYSTEM_READ, - ), + const accounts = await engine.find( + SystemObjectName.ACCOUNT, + { where: { user_id: userId }, limit: 1 }, + SYSTEM_READ, ); if (accounts.length > 0) return { act: false, reason: 'seed-address-claimed' }; } // (2) Does any local password login exist at all? - const credentials = asRows( - await engine.find( - SystemObjectName.ACCOUNT, - { where: { provider_id: CREDENTIAL_PROVIDER_ID }, limit: 1 }, - SYSTEM_READ, - ), + const credentials = await engine.find( + SystemObjectName.ACCOUNT, + { where: { provider_id: CREDENTIAL_PROVIDER_ID }, limit: 1 }, + SYSTEM_READ, ); if (credentials.length > 0) return { act: false, reason: 'local-login-exists' }; diff --git a/packages/plugins/plugin-auth/src/ensure-default-organization.ts b/packages/plugins/plugin-auth/src/ensure-default-organization.ts index a9f99eb0af..6222b5c726 100644 --- a/packages/plugins/plugin-auth/src/ensure-default-organization.ts +++ b/packages/plugins/plugin-auth/src/ensure-default-organization.ts @@ -141,8 +141,7 @@ const SYSTEM_CTX = { isSystem: true }; async function tryFind(ql: any, object: string, where: any, limit = 100): Promise { try { - const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); - return Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : []; + return await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); } catch { return []; } diff --git a/packages/plugins/plugin-auth/src/member-role-canonical.ts b/packages/plugins/plugin-auth/src/member-role-canonical.ts index c65741a033..e7ea8e30cd 100644 --- a/packages/plugins/plugin-auth/src/member-role-canonical.ts +++ b/packages/plugins/plugin-auth/src/member-role-canonical.ts @@ -377,8 +377,7 @@ export async function canonicalizeStoredMemberRoles( let rows: any[] = []; try { - const found = await ql.find(MEMBER_OBJECT, { limit }, { context: SYSTEM_CTX }); - rows = Array.isArray(found) ? found : Array.isArray(found?.records) ? found.records : []; + rows = await ql.find(MEMBER_OBJECT, { limit }, { context: SYSTEM_CTX }); } catch (e: any) { // No membership table yet (fresh boot, mock mode) — nothing to converge. logger?.debug?.('[MemberRoleCanonical] sys_member not readable — skipping the pass', { diff --git a/packages/plugins/plugin-auth/src/phone-sms-texts.ts b/packages/plugins/plugin-auth/src/phone-sms-texts.ts index fc33d49d02..084a06f584 100644 --- a/packages/plugins/plugin-auth/src/phone-sms-texts.ts +++ b/packages/plugins/plugin-auth/src/phone-sms-texts.ts @@ -147,19 +147,13 @@ export function builtinPhoneSmsBody(topic: string, locale: string | undefined): /** Minimal engine surface the loader/seeder needs. */ export interface PhoneSmsTemplateEngine { - find(objectName: string, query?: unknown): Promise; + find(objectName: string, query?: unknown): Promise>>; insert(objectName: string, data: unknown, options?: unknown): Promise; } const TEMPLATE_OBJECT = 'sys_notification_template'; const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; -function rowsOf(result: unknown): Array> { - if (Array.isArray(result)) return result as Array>; - const data = (result as { data?: unknown } | null)?.data; - return Array.isArray(data) ? (data as Array>) : []; -} - /** * Load the tenant's template body for `(topic, 'sms', locale chain)`. * Best-effort: any lookup error (missing table, no engine) yields `null` @@ -179,7 +173,7 @@ export async function loadPhoneSmsTemplateBody( limit: 1, context: SYSTEM_CTX, }); - const row = rowsOf(result)[0]; + const row = result[0]; const body = row?.body; if (typeof body === 'string' && body.trim()) return body; } catch { @@ -207,7 +201,7 @@ export async function seedPhoneSmsTemplates( limit: 1, context: SYSTEM_CTX, }); - if (rowsOf(existing).length > 0) continue; + if (existing.length > 0) continue; await engine.insert(TEMPLATE_OBJECT, { ...tpl }, { context: SYSTEM_CTX }); } catch (err) { logger?.warn( diff --git a/packages/plugins/plugin-auth/src/reconcile-membership.ts b/packages/plugins/plugin-auth/src/reconcile-membership.ts index 1a5476f9d8..3b75122e62 100644 --- a/packages/plugins/plugin-auth/src/reconcile-membership.ts +++ b/packages/plugins/plugin-auth/src/reconcile-membership.ts @@ -183,8 +183,7 @@ async function findRows( ): Promise { if (!engine || typeof engine.find !== 'function') return []; try { - const rows = await engine.find(object, { where, limit }, { context: SYSTEM_CTX }); - return Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : []; + return await engine.find(object, { where, limit }, { context: SYSTEM_CTX }); } catch { return []; } diff --git a/packages/plugins/plugin-auth/src/tenancy-service.ts b/packages/plugins/plugin-auth/src/tenancy-service.ts index 706c7d2fc7..39a4eeec3f 100644 --- a/packages/plugins/plugin-auth/src/tenancy-service.ts +++ b/packages/plugins/plugin-auth/src/tenancy-service.ts @@ -147,8 +147,7 @@ async function findRows( ): Promise { if (!engine || typeof engine.find !== 'function') return []; try { - const rows = await engine.find(object, { where, limit }, { context: SYSTEM_CTX }); - return Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : []; + return await engine.find(object, { where, limit }, { context: SYSTEM_CTX }); } catch { return []; } diff --git a/packages/plugins/plugin-auth/src/walled-owner-verification-path.ts b/packages/plugins/plugin-auth/src/walled-owner-verification-path.ts index e6434fea3f..9a670a20f3 100644 --- a/packages/plugins/plugin-auth/src/walled-owner-verification-path.ts +++ b/packages/plugins/plugin-auth/src/walled-owner-verification-path.ts @@ -238,11 +238,6 @@ export async function probeWalledOwnerAccountState( const config = resolvePlatformAdminEmails(); if (config.emails.length === 0 || !engine || typeof engine.find !== 'function') return 'unknown'; const SYSTEM = { context: { isSystem: true } }; - const asRows = (raw: unknown): Record[] => { - if (Array.isArray(raw)) return raw as Record[]; - const records = (raw as { records?: unknown } | null | undefined)?.records; - return Array.isArray(records) ? (records as Record[]) : []; - }; try { // Both spellings for EVERY declared address, exactly as the standing // resolver queries them — the as-typed forms come from the parser's own @@ -250,8 +245,10 @@ export async function probeWalledOwnerAccountState( const spellings = [...new Set([...config.emails, ...config.declaredSpellings])]; const byId = new Map>(); for (const spelling of spellings) { - for (const row of asRows( - await engine.find(SystemObjectName.USER, { where: { email: spelling }, limit: 5 }, SYSTEM), + for (const row of await engine.find( + SystemObjectName.USER, + { where: { email: spelling }, limit: 5 }, + SYSTEM, )) { if (row?.id) byId.set(row.id, row); } From 1e8b674b4a8160f9765d49244ff6c79636947f8c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:43:06 +0000 Subject: [PATCH 2/4] wip(plugin-auth): pin the concrete find() shape per block, and the DROP fix's direction Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../src/find-envelope-limb-removal.test.ts | 484 ++++++++++++++++++ 1 file changed, 484 insertions(+) create mode 100644 packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts diff --git a/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts new file mode 100644 index 0000000000..f693b7f7ce --- /dev/null +++ b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts @@ -0,0 +1,484 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15597] The fourteen `{ records }` / `{ data }` union-normalizer blocks this + * package carried, and the shape each one's REAL engine actually returns. + * + * ## What was removed, and why a test is owed for it + * + * Every block looked like `Array.isArray(x) ? x : x.records ?? []` (five of them + * in the guard-clause spelling, one on a `data` limb). The envelope limb was + * dead: nothing in this tree ever produced that shape. But "dead" is a claim + * about RUNTIME, and the declared type cannot establish it — `IDataEngine.find` + * is declared `Promise` and #13706 is this repo's own counter-example of + * a `find()` that did not resolve to an array. So the limbs were removed on a + * MEASUREMENT, and these cases are that measurement, kept. + * + * ## The measurement + * + * All fourteen blocks read the same concrete engine: the `ObjectQL` instance the + * kernel registers as the `objectql` / `data` service (`auth-plugin.ts` resolves + * it with `ctx.getService('objectql')`; `AuthManager` reads it through + * `withSystemReadContext`, which forwards `find` without touching its result). + * Each case below boots a REAL `ObjectQL` over a REAL `SqlDriver`, issues the + * exact read its block issues, and pins that the answer is a bare array — + * populated AND empty, because an empty read answering `[]` rather than a + * nullish value is half of why the limb was unreachable. + * + * ## Why these cases are not vacuous + * + * `expect(Array.isArray(x)).toBe(true)` is the kind of assertion that can pass + * because nothing could have made it fail. It could have here: `ObjectQL.find` + * returns `hookContext.result` on its hook path, so an `afterFind` handler CAN + * replace the result with an envelope — measured, not supposed (see the control + * case at the bottom, which drives exactly that and asserts every pin above it + * goes red). That is also the reason removal is right rather than merely safe: + * a hook that corrupted `find()` into `{ records }` would be a contract + * violation, and the limb did not repair it — it silently absorbed it at these + * fourteen sites while the ~140 other `find()` call sites in this package broke + * anyway. Fourteen sites of false immunity is worse than one visible failure. + * + * ## The fifteenth case is a different defect + * + * `settleSelfRegistrationGrant` also carried #15092's DROP shape, and it is + * fixed in the OPPOSITE direction — see the `describe` at the end of the file. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { AuthManager } from './auth-manager.js'; +import { authIdentityObjects } from './manifest.js'; +import { withSystemReadContext } from './objectql-adapter.js'; +import { probeHumanUsersPresence, probeSignInAccountsPresence } from './boot-sign-in-reachability.js'; +import { decideDevAdminSeedGate } from './dev-admin-seed-gate.js'; +import { loadPhoneSmsTemplateBody, seedPhoneSmsTemplates } from './phone-sms-texts.js'; +import { resolveDefaultOrgId } from './tenancy-service.js'; +import { backfillAccountIssuer } from './backfill-account-issuer.js'; +import { canonicalizeStoredMemberRoles } from './member-role-canonical.js'; + +const SECRET = 'test-secret-at-least-32-chars-long-15597'; +const SYSTEM = { context: { isSystem: true } } as never; +const SYSTEM_CTX = { isSystem: true }; + +/** + * Two objects the auth manifest does not declare, spelled with only the columns + * the blocks under test read — the `sso-register-platform-admin-gate` / + * `signup-existing-address-refusal` precedent, so a fixture adds no dependency + * edge to plugin-auth. + */ +const sysPermissionSet = { + name: 'sys_permission_set', + label: 'Permission Set', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const }, + label: { name: 'label', type: 'text' as const }, + active: { name: 'active', type: 'boolean' as const }, + organization_id: { name: 'organization_id', type: 'text' as const }, + }, +}; + +const sysUserPermissionSet = { + name: 'sys_user_permission_set', + label: 'User Permission Set', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + user_id: { name: 'user_id', type: 'text' as const }, + permission_set_id: { name: 'permission_set_id', type: 'text' as const }, + organization_id: { name: 'organization_id', type: 'text' as const }, + }, +}; + +const sysNotificationTemplate = { + name: 'sys_notification_template', + label: 'Notification Template', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + topic: { name: 'topic', type: 'text' as const }, + channel: { name: 'channel', type: 'text' as const }, + locale: { name: 'locale', type: 'text' as const }, + body: { name: 'body', type: 'text' as const }, + is_active: { name: 'is_active', type: 'boolean' as const }, + }, +}; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + const engine = engines.pop(); + try { + await (engine as unknown as { destroy?(): Promise })?.destroy?.(); + } catch { + /* noop */ + } + } +}); + +async function bootEngine(): Promise { + const engine = new ObjectQL(); + engines.push(engine); + engine.registerDriver( + new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), + true, + ); + await engine.init(); + for (const object of authIdentityObjects) { + engine.registry.registerObject(object as never, '@objectstack/plugin-auth'); + } + engine.registry.registerObject(sysPermissionSet as never, '@objectstack/plugin-security'); + engine.registry.registerObject(sysUserPermissionSet as never, '@objectstack/plugin-security'); + engine.registry.registerObject(sysNotificationTemplate as never, '@objectstack/service-messaging'); + await engine.syncSchemas(); + return engine; +} + +/** One row per object every block below reads, so the POPULATED path is driven. */ +async function seedAll(engine: ObjectQL): Promise { + await engine.insert('sys_user', { id: 'usr_1', name: 'Alice', email: 'alice@corp.example' }, SYSTEM); + await engine.insert('sys_organization', { id: 'org_1', name: 'Default', slug: 'default' }, SYSTEM); + await engine.insert('sys_member', { id: 'mem_1', user_id: 'usr_1', organization_id: 'org_1', role: 'member' }, SYSTEM); + await engine.insert('sys_account', { id: 'acc_1', user_id: 'usr_1', provider_id: 'credential', account_id: 'alice@corp.example' }, SYSTEM); + await engine.insert('sys_invitation', { id: 'inv_1', email: 'alice@corp.example', status: 'pending', organization_id: 'org_1', inviter_id: 'usr_1', expires_at: new Date(Date.now() + 86_400_000).toISOString() }, SYSTEM); + await engine.insert('sys_permission_set', { id: 'ps_1', name: 'member_default', label: 'Member' }, SYSTEM); + await engine.insert('sys_user_permission_set', { id: 'ups_1', user_id: 'usr_1', permission_set_id: 'ps_1' }, SYSTEM); + await engine.insert('sys_notification_template', { id: 'nt_1', topic: 'otp', channel: 'sms', locale: 'en', body: 'code {{code}}', is_active: true }, SYSTEM); +} + +/** + * The assertion every shape pin makes. + * + * Both halves matter. `Array.isArray` is the limb's own test, so pinning it + * pins exactly the branch that was removed; the key check states the positive + * fact the deleted code claimed was possible, so a future envelope fails HERE + * with a readable message rather than somewhere downstream. + */ +function expectBareArray(value: unknown, label: string): void { + expect(Array.isArray(value), `${label}: expected a bare array, got ${JSON.stringify(value)}`).toBe(true); + expect(value === null || typeof value !== 'object' || !('records' in (value as object)), `${label}: carries a 'records' envelope`).toBe(true); + expect(value === null || typeof value !== 'object' || !('data' in (value as object)), `${label}: carries a 'data' envelope`).toBe(true); +} + +/** + * The fourteen blocks, each paired with the read it performs — same object, + * same query, same call facade (`withSystemReadContext` for the `AuthManager` + * blocks; the three-argument `ql.find(o, q, { context })` for the standalone + * migration/probe modules; the context-inside-the-query form for + * `phone-sms-texts`, which is the only block that spells it that way). + */ +const BLOCKS: Array<{ + id: string; + site: string; + populated: (engine: ObjectQL) => Promise; + empty: (engine: ObjectQL) => Promise; +}> = [ + { + id: 'B1 isBootstrapCreation', + site: 'auth-manager.ts — sys_user page probe', + populated: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user', { limit: 50 }), + empty: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user', { where: { email: 'nobody@nowhere' }, limit: 50 }), + }, + { + id: 'B2 hasPendingInvitationFor', + site: 'auth-manager.ts — sys_invitation paged probe', + populated: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_invitation', { where: { status: 'pending', email: 'alice@corp.example' }, limit: 100 }), + // The second page: the `offset` arm the loop only sends past page one. + empty: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_invitation', { where: { status: 'pending', email: 'alice@corp.example' }, limit: 100, offset: 100 }), + }, + { + id: 'B3 hasExistingUserFor', + site: 'auth-manager.ts — #15738 uniqueness probe', + populated: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user', { where: { email: 'alice@corp.example' }, limit: 50 }), + empty: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user', { where: { email: 'ghost@corp.example' }, limit: 50 }), + }, + { + id: 'B4 findPermissionSetRows', + site: 'auth-manager.ts — sys_permission_set by name', + populated: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_permission_set', { where: { name: 'member_default' }, limit: 50 }), + empty: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_permission_set', { where: { name: 'ghost' }, limit: 50 }), + }, + { + id: 'B5 settleSelfRegistrationGrant', + site: 'auth-manager.ts — sys_user_permission_set existence read', + populated: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user_permission_set', { where: { user_id: 'usr_1', permission_set_id: 'ps_1' }, limit: 1 }), + empty: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user_permission_set', { where: { user_id: 'ghost', permission_set_id: 'ps_1' }, limit: 1 }), + }, + { + id: 'B6 ensureDefaultOrganization.tryFind', + site: 'ensure-default-organization.ts', + populated: (e) => (e as never as { find: Function }).find('sys_organization', { where: { slug: 'default' }, limit: 100 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_organization', { where: { slug: 'ghost' }, limit: 100 }, { context: SYSTEM_CTX }), + }, + { + id: 'B7 tenancy-service.findRows', + site: 'tenancy-service.ts', + populated: (e) => (e as never as { find: Function }).find('sys_organization', { where: { slug: 'default' }, limit: 1 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_organization', { where: { slug: 'ghost' }, limit: 1 }, { context: SYSTEM_CTX }), + }, + { + id: 'B8 reconcile-membership.findRows', + site: 'reconcile-membership.ts', + populated: (e) => (e as never as { find: Function }).find('sys_member', { where: { user_id: 'usr_1' }, limit: 1 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_member', { where: { user_id: 'ghost' }, limit: 1 }, { context: SYSTEM_CTX }), + }, + { + id: 'B9 backfillAccountIssuer.tryFind', + site: 'backfill-account-issuer.ts', + populated: (e) => (e as never as { find: Function }).find('sys_account', { where: { provider_id: 'credential' }, limit: 5000 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_account', { where: { provider_id: 'ghost' }, limit: 5000 }, { context: SYSTEM_CTX }), + }, + { + id: 'B10 canonicalizeStoredMemberRoles', + site: 'member-role-canonical.ts', + populated: (e) => (e as never as { find: Function }).find('sys_member', { limit: 5000 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_member', { where: { user_id: 'ghost' }, limit: 5000 }, { context: SYSTEM_CTX }), + }, + { + id: 'B11 decideDevAdminSeedGate.asRows', + site: 'dev-admin-seed-gate.ts', + populated: (e) => (e as never as { find: Function }).find('sys_account', { where: { provider_id: 'credential' }, limit: 1 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_account', { where: { user_id: 'ghost' }, limit: 1 }, { context: SYSTEM_CTX }), + }, + { + id: 'B12 probeWalledOwnerAccountState.asRows', + site: 'walled-owner-verification-path.ts', + populated: (e) => (e as never as { find: Function }).find('sys_user', { where: { email: 'alice@corp.example' }, limit: 5 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_user', { where: { email: 'ghost@corp.example' }, limit: 5 }, { context: SYSTEM_CTX }), + }, + { + id: 'B13 probeHumanUsersPresence.asRows', + site: 'boot-sign-in-reachability.ts', + populated: (e) => (e as never as { find: Function }).find('sys_user', { limit: 50 }, { context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_account', { where: { user_id: 'ghost' }, limit: 1 }, { context: SYSTEM_CTX }), + }, + { + id: 'B14 phone-sms-texts.rowsOf', + site: 'phone-sms-texts.ts — the `data`-limb block', + populated: (e) => (e as never as { find: Function }).find('sys_notification_template', { where: { topic: 'otp', channel: 'sms', locale: 'en', is_active: true }, limit: 1, context: SYSTEM_CTX }), + empty: (e) => (e as never as { find: Function }).find('sys_notification_template', { where: { topic: 'ghost' }, limit: 1, context: SYSTEM_CTX }), + }, +]; + +describe('#15597 — the concrete shape each removed limb was guarding against', () => { + for (const block of BLOCKS) { + it(`${block.id}: its real engine read answers a bare array, populated and empty (${block.site})`, async () => { + const engine = await bootEngine(); + await seedAll(engine); + + const populated = await block.populated(engine); + expectBareArray(populated, `${block.id} populated`); + expect((populated as unknown[]).length, `${block.id}: the populated read found no row, so it proved nothing`).toBeGreaterThan(0); + + // The empty read is the other half of why the limb was unreachable: it + // answers `[]`, never `null`/`undefined`, so the `: []` tail never ran either. + const empty = await block.empty(engine); + expectBareArray(empty, `${block.id} empty`); + expect((empty as unknown[]).length).toBe(0); + }); + } +}); + +describe('#15597 — the control: these pins CAN go red', () => { + /** + * The discrimination check for all fourteen cases above, and the reason the + * removal argument is about hooks rather than about drivers. + * + * `ObjectQL.find` ends with `return hookContext.result` on its hook path, so + * an `afterFind` handler that assigns a non-array makes `find()` resolve to + * one. Nothing in this tree does that — the only registered `afterFind` in + * the repo is plugin-audit's read recorder, which never touches `ctx.result` + * — but the mechanism EXISTS, which is what makes `expectBareArray` a real + * assertion instead of a tautology. + */ + it('an afterFind hook that returns an envelope turns every shape pin red', async () => { + const engine = await bootEngine(); + await seedAll(engine); + + (engine as never as { registerHook: Function }).registerHook( + 'afterFind', + (ctx: { result: unknown }) => { + ctx.result = { records: [{ id: 'ENVELOPE' }] }; + }, + { packageId: 'test.15597-control' }, + ); + + const survivors: string[] = []; + for (const block of BLOCKS) { + const value = await block.populated(engine); + // Under the mutation the read really does answer an envelope… + expect(Array.isArray(value), `${block.id}: the hook did not take effect`).toBe(false); + // …and the pin's own assertion rejects it. + try { + expectBareArray(value, block.id); + survivors.push(block.id); + } catch { + /* expected: the pin discriminates */ + } + } + expect(survivors, 'these pins passed on an envelope — they do not discriminate').toEqual([]); + }); +}); + +describe('#15597 — the blocks driven through their real production entry points', () => { + it('boot-sign-in-reachability answers present/absent off the bare array (B13)', async () => { + const engine = await bootEngine(); + expect(await probeHumanUsersPresence(engine as never)).toBe('absent'); + expect(await probeSignInAccountsPresence(engine as never)).toBe('absent'); + await seedAll(engine); + expect(await probeHumanUsersPresence(engine as never)).toBe('present'); + expect(await probeSignInAccountsPresence(engine as never)).toBe('present'); + }); + + it('dev-admin-seed-gate reads the credential store off the bare array (B11)', async () => { + const engine = await bootEngine(); + expect(await decideDevAdminSeedGate(engine as never, 'admin@objectos.ai')).toEqual({ act: true }); + await seedAll(engine); + // A local `credential` account now exists, so the seed declines. + expect(await decideDevAdminSeedGate(engine as never, 'admin@objectos.ai')).toEqual({ + act: false, + reason: 'local-login-exists', + }); + }); + + it('phone-sms template load + seed read the bare array (B14, the `data` limb)', async () => { + const engine = await bootEngine(); + await seedAll(engine); + expect(await loadPhoneSmsTemplateBody(engine as never, 'otp', 'en')).toBe('code {{code}}'); + expect(await loadPhoneSmsTemplateBody(engine as never, 'nosuchtopic', 'en')).toBeNull(); + // The seeder's existence read is the second `rowsOf` call site: the row + // above is already present, so it must not be duplicated. + await seedPhoneSmsTemplates(engine as never); + const rows = await engine.find('sys_notification_template', { where: { topic: 'otp', channel: 'sms', locale: 'en' }, limit: 100 }, SYSTEM); + expect((rows as unknown[]).length).toBe(1); + }); + + it('tenancy resolveDefaultOrgId reads the bare array (B7)', async () => { + const engine = await bootEngine(); + expect(await resolveDefaultOrgId(engine)).toBeNull(); + await seedAll(engine); + expect(await resolveDefaultOrgId(engine)).toBe('org_1'); + }); + + it('backfillAccountIssuer and canonicalizeStoredMemberRoles scan the bare array (B9, B10)', async () => { + const engine = await bootEngine(); + await seedAll(engine); + const backfill = await backfillAccountIssuer(engine); + expect(backfill.scanned).toBeGreaterThan(0); + const canon = await canonicalizeStoredMemberRoles(engine); + // One membership row was seeded, and the scan saw it — the count comes + // straight off the array the removed limb used to normalise. + expect(canon.scanned).toBe(1); + }); + + it('AuthManager.findPermissionSetRows reads the bare array (B4)', async () => { + const engine = await bootEngine(); + await seedAll(engine); + const manager = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine as never } as never); + const rows = await (manager as never as { findPermissionSetRows: Function }).findPermissionSetRows('member_default'); + expectBareArray(rows, 'B4 via findPermissionSetRows'); + expect(rows.length).toBe(1); + expect(rows[0].id).toBe('ps_1'); + }); +}); + +/** + * ## The fifteenth block: #15092's DROP shape, fixed in the OPPOSITE direction + * + * `settleSelfRegistrationGrant` filtered its permission-set candidates with + * `r?.active !== false && typeof r?.id === 'string' && r.id`. The first clause + * is a selection predicate and stays. The second silently DROPPED a malformed + * row, and these cases pin why that is the opposite defect from a dead limb: + * the branch is reachable, and its old behaviour was wrong in two ways that + * both looked normal from outside. + */ +describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row instead of dropping it', () => { + const USER = { id: 'usr_new', name: 'New Person', email: 'new@corp.example' }; + + async function settle(engine: ObjectQL, setName: string, lines: string[]): Promise { + const manager = new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + dataEngine: engine as never, + logger: { error: (m: string) => lines.push(String(m)), warn: (m: string) => lines.push(String(m)), info: () => {} }, + } as never); + (manager as never as { stageSelfRegistrationGrant: Function }).stageSelfRegistrationGrant(USER.email, setName); + await (manager as never as { settleSelfRegistrationGrant: Function }).settleSelfRegistrationGrant(USER); + } + + const grants = (engine: ObjectQL): Promise => + engine.find('sys_user_permission_set', { limit: 100 }, SYSTEM) as Promise; + + it('a WELL-FORMED family still grants — the fix costs the happy path nothing', async () => { + const engine = await bootEngine(); + await engine.insert('sys_user', USER, SYSTEM); + await engine.insert('sys_permission_set', { id: 'ps_ok', name: 'portal_user', label: 'Portal' }, SYSTEM); + + const lines: string[] = []; + await settle(engine, 'portal_user', lines); + + const rows = await grants(engine); + expect(rows.length).toBe(1); + expect((rows[0] as { permission_set_id: string }).permission_set_id).toBe('ps_ok'); + }); + + it('a MALFORMED sole row is refused and NAMED — not reported as "no row named X resolves"', async () => { + const engine = await bootEngine(); + await engine.insert('sys_user', USER, SYSTEM); + // Active, named exactly right, and present — only its id is unusable. + await engine.insert('sys_permission_set', { id: '', name: 'portal_user', label: 'Portal' }, SYSTEM); + + const lines: string[] = []; + await settle(engine, 'portal_user', lines); + + expect(await grants(engine)).toEqual([]); + const report = lines.join('\n'); + // The cause names the real fact. Before the fix the row was dropped and + // this said "no active sys_permission_set row named 'portal_user' + // resolves" — false, and it sends an operator to look for a missing row. + expect(report, `nothing reported; logged: ${JSON.stringify(lines)}`).toContain('no usable id'); + expect(report).not.toContain('resolves for organization'); + }); + + it('⭐ the wrong-GRANT case: a malformed ORG row no longer silently falls through to the GLOBAL set', async () => { + // This is the case that makes the direction matter rather than being + // cosmetic. Two rows carry the declared name: the organization's own row + // (malformed id) and a global one (well-formed). The old trailing filter + // dropped the org row, `rows.find((r) => r.organization_id == null)` then + // matched the GLOBAL row, and the self-registrant was granted a permission + // set their organization never declared — silently, with a success log. + const engine = await bootEngine(); + await engine.insert('sys_user', USER, SYSTEM); + await engine.insert('sys_organization', { id: 'org_1', name: 'Default', slug: 'default' }, SYSTEM); + await engine.insert('sys_permission_set', { id: '', name: 'portal_user', label: 'Org scoped', organization_id: 'org_1' }, SYSTEM); + await engine.insert('sys_permission_set', { id: 'ps_global', name: 'portal_user', label: 'Global' }, SYSTEM); + + const lines: string[] = []; + await settle(engine, 'portal_user', lines); + + // Refused, and in particular NOT granted the global set. + const rows = await grants(engine); + expect(rows, `granted anyway: ${JSON.stringify(rows)}`).toEqual([]); + expect(lines.join('\n')).toContain('no usable id'); + }); + + it('a DEACTIVATED row is still an ordinary non-resolution, not a malformed-row refusal', async () => { + // The `active !== false` clause is a selection predicate and stays one: + // deactivating a set must keep reporting "does not resolve", not start + // reporting a malformed row. This is the boundary between the two clauses. + const engine = await bootEngine(); + await engine.insert('sys_user', USER, SYSTEM); + await engine.insert('sys_permission_set', { id: 'ps_off', name: 'portal_user', label: 'Portal', active: false }, SYSTEM); + + const lines: string[] = []; + await settle(engine, 'portal_user', lines); + + expect(await grants(engine)).toEqual([]); + const report = lines.join('\n'); + expect(report).toContain("no active sys_permission_set row named 'portal_user'"); + expect(report).not.toContain('no usable id'); + }); +}); From e9993ef72cf39f642ff364bd1d8fd4feb938d00a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:52:33 +0000 Subject: [PATCH 3/4] wip(plugin-auth): changeset, ablation record, unused import Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/plugin-auth-find-envelope-limbs.md | 14 ++++++++++++++ .../src/find-envelope-limb-removal.test.ts | 11 ++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 .changeset/plugin-auth-find-envelope-limbs.md diff --git a/.changeset/plugin-auth-find-envelope-limbs.md b/.changeset/plugin-auth-find-envelope-limbs.md new file mode 100644 index 0000000000..f866134068 --- /dev/null +++ b/.changeset/plugin-auth-find-envelope-limbs.md @@ -0,0 +1,14 @@ +--- +"@objectstack/plugin-auth": patch +--- + +A self-registration grant is refused, not silently redirected, when a permission-set row is malformed — and the fourteen dead `{ records }` / `{ data }` normalizer limbs behind that code are gone. + +`plugin-auth` carried fourteen array-or-envelope normalizer blocks of the shape `Array.isArray(x) ? x : x.records ?? []` (thirteen on a `records` limb, one on a `data` limb, five of them written as a guard clause rather than a ternary). All fourteen read the same concrete engine — the `ObjectQL` instance the kernel registers as the `objectql` / `data` service — which answers a bare array on every path, populated or empty. The envelope limb was unreachable code that read as a contract, so the next author writing a defensive normalizer here believed an envelope was possible. The limbs are removed, and the four local engine ports that declared `Promise` (`BootProbeEngine`, `DevAdminSeedProbeEngine`, `PhoneSmsTemplateEngine`) now declare the array they always returned. + +The user-visible change is in `settleSelfRegistrationGrant`, which carried the opposite defect. Its candidate filter dropped any permission-set row whose `id` was missing or blank, silently, before choosing which row to grant: + +- When the malformed row was the only one, the operator was told `no active sys_permission_set row named 'X' resolves` — false, since an active row named exactly that was present. That report is the only signal this path emits, and nothing retries it. +- When the malformed row was the **organization-scoped** one and a global row also carried the declared name, dropping it let the `organization_id == null` arm match instead, and the self-registrant was granted the **global** permission set their organization never declared — with a success log and no other trace. + +`active !== false` remains a selection predicate: a deactivated set still reports the ordinary "does not resolve". A malformed row is no longer a selection at all — the grant is refused and the report names the malformed row, so the ambiguity is surfaced instead of resolved by accident. A well-formed family grants exactly as before. diff --git a/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts index f693b7f7ce..f9662e53cd 100644 --- a/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts +++ b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts @@ -44,7 +44,7 @@ * fixed in the OPPOSITE direction — see the `describe` at the end of the file. */ -import { describe, it, expect, afterEach, vi } from 'vitest'; +import { describe, it, expect, afterEach } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { AuthManager } from './auth-manager.js'; @@ -459,6 +459,15 @@ describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row inst const lines: string[] = []; await settle(engine, 'portal_user', lines); + // Reverse-verified by ablation (recorded because the pre-fix behaviour is + // the whole argument): with the old trailing filter restored and nothing + // else changed, this case goes red with a granted row — + // `permission_set_id: 'ps_global'`, `organization_id: null` — i.e. the + // self-registrant really was handed the global set. The sibling case above + // goes red at the same time with `Cause: no active sys_permission_set row + // named 'portal_user' resolves`, which is the false cause. The other two + // cases in this describe stay GREEN under that ablation, by design: they + // are the no-regression and boundary guards, not the discriminators. // Refused, and in particular NOT granted the global set. const rows = await grants(engine); expect(rows, `granted anyway: ${JSON.stringify(rows)}`).toEqual([]); From 180187383471faa7c19c7495f5374717c2c3f8a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 10:09:44 +0000 Subject: [PATCH 4/4] fix(plugin-auth): state the Case D refusal cost, pin the org-stamped wrong grant, correct two counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - changeset: name the family that now gets a refusal where it previously got a grant (an active, correctly-named row with a missing/blank id — including a malformed GLOBAL row beside a well-formed org-scoped one), what it now sees, and that repairing or deleting the row restores the grant with no code change. - the wrong-GRANT pin now RESOLVES an organization via `getTenancy`, so it drives the shape that actually bit: `ps_global` stamped `organization_id: 'org_1'`. Asserted by column, not only by row count. - counts corrected: four guard-clause spellings (not five — the fifth match was `member-role-canonical`'s `raw.join(',')`, a different helper); and this package has 47 `.find(` sites in non-test source, a count that already includes `Array#find` (the earlier "~140" was unmeasured). Also "four local engine ports" -> "three", which is what that sentence names. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/plugin-auth-find-envelope-limbs.md | 4 +- .../src/find-envelope-limb-removal.test.ts | 41 ++++++++++++++++--- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/.changeset/plugin-auth-find-envelope-limbs.md b/.changeset/plugin-auth-find-envelope-limbs.md index f866134068..2eeba4b964 100644 --- a/.changeset/plugin-auth-find-envelope-limbs.md +++ b/.changeset/plugin-auth-find-envelope-limbs.md @@ -4,7 +4,7 @@ A self-registration grant is refused, not silently redirected, when a permission-set row is malformed — and the fourteen dead `{ records }` / `{ data }` normalizer limbs behind that code are gone. -`plugin-auth` carried fourteen array-or-envelope normalizer blocks of the shape `Array.isArray(x) ? x : x.records ?? []` (thirteen on a `records` limb, one on a `data` limb, five of them written as a guard clause rather than a ternary). All fourteen read the same concrete engine — the `ObjectQL` instance the kernel registers as the `objectql` / `data` service — which answers a bare array on every path, populated or empty. The envelope limb was unreachable code that read as a contract, so the next author writing a defensive normalizer here believed an envelope was possible. The limbs are removed, and the four local engine ports that declared `Promise` (`BootProbeEngine`, `DevAdminSeedProbeEngine`, `PhoneSmsTemplateEngine`) now declare the array they always returned. +`plugin-auth` carried fourteen array-or-envelope normalizer blocks of the shape `Array.isArray(x) ? x : x.records ?? []` (thirteen on a `records` limb, one on a `data` limb, four of them written as a guard clause rather than a ternary). All fourteen read the same concrete engine — the `ObjectQL` instance the kernel registers as the `objectql` / `data` service — which answers a bare array on every path, populated or empty. The envelope limb was unreachable code that read as a contract, so the next author writing a defensive normalizer here believed an envelope was possible. The limbs are removed, and the three local engine ports that declared `Promise` (`BootProbeEngine`, `DevAdminSeedProbeEngine`, `PhoneSmsTemplateEngine`) now declare the array they always returned. The user-visible change is in `settleSelfRegistrationGrant`, which carried the opposite defect. Its candidate filter dropped any permission-set row whose `id` was missing or blank, silently, before choosing which row to grant: @@ -12,3 +12,5 @@ The user-visible change is in `settleSelfRegistrationGrant`, which carried the o - When the malformed row was the **organization-scoped** one and a global row also carried the declared name, dropping it let the `organization_id == null` arm match instead, and the self-registrant was granted the **global** permission set their organization never declared — with a success log and no other trace. `active !== false` remains a selection predicate: a deactivated set still reports the ordinary "does not resolve". A malformed row is no longer a selection at all — the grant is refused and the report names the malformed row, so the ambiguity is surfaced instead of resolved by accident. A well-formed family grants exactly as before. + +**Upgrade note — one family now gets a refusal where it previously got a grant.** If a deployment's `sys_permission_set` already contains a row that is active and carries the declared name but whose `id` is missing or blank, self-registration grants against that name now stop and report, including the case where the malformed row is one nobody was relying on: a malformed **global** row sitting alongside a well-formed **organization-scoped** row used to be dropped silently, letting the org row be granted, and is now refused. This is deliberate — the old behaviour could not tell that family apart from the one where the silent drop granted the *wrong* set — and it is fully reversible without a code change: repair or delete the malformed row and the grant proceeds exactly as before. The refusal is loud and names the row, so it is visible rather than something to discover later; nothing is written while it stands. diff --git a/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts index f9662e53cd..e3b1bbc77f 100644 --- a/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts +++ b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts @@ -6,7 +6,7 @@ * * ## What was removed, and why a test is owed for it * - * Every block looked like `Array.isArray(x) ? x : x.records ?? []` (five of them + * Every block looked like `Array.isArray(x) ? x : x.records ?? []` (four of them * in the guard-clause spelling, one on a `data` limb). The envelope limb was * dead: nothing in this tree ever produced that shape. But "dead" is a claim * about RUNTIME, and the declared type cannot establish it — `IDataEngine.find` @@ -35,8 +35,10 @@ * goes red). That is also the reason removal is right rather than merely safe: * a hook that corrupted `find()` into `{ records }` would be a contract * violation, and the limb did not repair it — it silently absorbed it at these - * fourteen sites while the ~140 other `find()` call sites in this package broke - * anyway. Fourteen sites of false immunity is worse than one visible failure. + * fourteen sites while this package's remaining `find()` call sites broke anyway + * (47 `.find(` sites in its non-test source in total, a count that already + * includes `Array#find`). Fourteen sites of false immunity is worse than one + * visible failure. * * ## The fifteenth case is a different defect * @@ -398,11 +400,23 @@ describe('#15597 — the blocks driven through their real production entry point describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row instead of dropping it', () => { const USER = { id: 'usr_new', name: 'New Person', email: 'new@corp.example' }; - async function settle(engine: ObjectQL, setName: string, lines: string[]): Promise { + async function settle( + engine: ObjectQL, + setName: string, + lines: string[], + /** + * The organization `settleSelfRegistrationGrant` resolves, read the way the + * method itself reads it — `this.config.getTenancy?.()` then `defaultOrgId()`. + * Omitted, nothing resolves and `organizationId` stays null; supplied, the + * org-scoped arm of the row selection is the one that runs. + */ + orgId?: string, + ): Promise { const manager = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine as never, + ...(orgId ? { getTenancy: () => ({ defaultOrgId: async () => orgId }) } : {}), logger: { error: (m: string) => lines.push(String(m)), warn: (m: string) => lines.push(String(m)), info: () => {} }, } as never); (manager as never as { stageSelfRegistrationGrant: Function }).stageSelfRegistrationGrant(USER.email, setName); @@ -450,6 +464,14 @@ describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row inst // dropped the org row, `rows.find((r) => r.organization_id == null)` then // matched the GLOBAL row, and the self-registrant was granted a permission // set their organization never declared — silently, with a success log. + // + // The organization is RESOLVED here (`getTenancy`), which is the shape that + // actually bit and the reason this case does not lean on its sibling: with + // an org in hand the wrong grant is not merely "some global row", it is + // `ps_global` STAMPED `organization_id: 'org_1'` — the write spreads the + // resolved org onto the row — so the store ends up asserting that org_1 + // granted a set org_1 never declared. Asserted by column below, not just by + // row count, so the stamp itself is pinned. const engine = await bootEngine(); await engine.insert('sys_user', USER, SYSTEM); await engine.insert('sys_organization', { id: 'org_1', name: 'Default', slug: 'default' }, SYSTEM); @@ -457,7 +479,7 @@ describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row inst await engine.insert('sys_permission_set', { id: 'ps_global', name: 'portal_user', label: 'Global' }, SYSTEM); const lines: string[] = []; - await settle(engine, 'portal_user', lines); + await settle(engine, 'portal_user', lines, 'org_1'); // Reverse-verified by ablation (recorded because the pre-fix behaviour is // the whole argument): with the old trailing filter restored and nothing @@ -471,6 +493,15 @@ describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row inst // Refused, and in particular NOT granted the global set. const rows = await grants(engine); expect(rows, `granted anyway: ${JSON.stringify(rows)}`).toEqual([]); + // The specific wrong write, named: no row may claim org_1 granted ps_global. + expect( + rows.some( + (r) => + (r as { permission_set_id?: unknown }).permission_set_id === 'ps_global' && + (r as { organization_id?: unknown }).organization_id === 'org_1', + ), + 'ps_global was granted STAMPED with org_1 — the organization is now recorded as having granted a set it never declared', + ).toBe(false); expect(lines.join('\n')).toContain('no usable id'); });