fix(plugin-auth): refuse a self-registration grant on a malformed permission-set row, and remove the 14 dead find() envelope limbs behind it

.changeset/plugin-auth-find-envelope-limbs.md

@@ -0,0 +1,16 @@
+---
+"@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, 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<unknown>` (`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.
+
+**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.

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<unknown>();
       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', {

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<any[]> {
   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 [];
   }

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<string, unknown>, options?: unknown): Promise<unknown>;
+  find(
+    object: string,
+    query: Record<string, unknown>,
+    options?: unknown,
+  ): Promise<Array<Record<string, unknown>>>;
 }
 
 /**
@@ -179,12 +183,6 @@ export interface SignInReachabilityFacts {
 
 const SYSTEM = { context: { isSystem: true } };
 
-const asRows = (raw: unknown): Record<string, unknown>[] => {
-  if (Array.isArray(raw)) return raw as Record<string, unknown>[];
-  const records = (raw as { records?: unknown } | null | undefined)?.records;
-  return Array.isArray(records) ? (records as Record<string, unknown>[]) : [];
-};
-
 const usable = (engine: BootProbeEngine | undefined): engine is BootProbeEngine =>
   !!engine && typeof engine.find === 'function';
 
@@ -203,8 +201,10 @@ export async function probeHumanUsersPresence(
 ): Promise<BootStorePresence> {
   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<BootStorePresence> {
   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';

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<string, unknown>, options?: unknown): Promise<unknown>;
+  find(
+    object: string,
+    query: Record<string, unknown>,
+    options?: unknown,
+  ): Promise<Array<Record<string, unknown>>>;
 }
 
 /**
@@ -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<string, unknown>[] {
-  if (Array.isArray(raw)) return raw as Record<string, unknown>[];
-  const records = (raw as { records?: unknown } | null | undefined)?.records;
-  return Array.isArray(records) ? (records as Record<string, unknown>[]) : [];
-}
-
 /**
  * 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<unknown>();
     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' };
 

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<any[]> {
   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 [];
   }