fix(security): surface swallowed permission-set resolution failures (#2565)

.changeset/permset-resolve-observability.md

@@ -0,0 +1,13 @@
+---
+"@objectstack/plugin-security": patch
+---
+
+fix(security): surface swallowed permission-set resolution failures (#2565)
+
+`PermissionEvaluator.resolvePermissionSets` swallowed metadata `list()` and
+`sys_permission_set` dbLoader failures silently — fail-closed (unresolvable
+sets grant nothing), but a transient DB error made custom permission sets
+vanish with no trace, leaving the resulting 403s undiagnosable. The evaluator
+now accepts an optional `{ logger }` and emits one `warn` per failed source,
+naming the unresolved permission sets and the error. SecurityPlugin wires its
+plugin logger into both call sites. Resolution behavior is byte-identical.

packages/plugins/plugin-security/src/permission-evaluator.ts

@@ -266,9 +266,15 @@ export class PermissionEvaluator {
     /**
      * Optional async loader for permission set names that aren't found in
      * metadata or bootstrap. Lets callers query user-defined permission
-     * sets persisted in `sys_permission_set`. Failures are swallowed.
+     * sets persisted in `sys_permission_set`. Failures are swallowed
+     * (fail-closed: unresolvable sets grant nothing) but SURFACED via
+     * `options.logger` — see #2565: without the warn, a transient DB error
+     * makes custom permission sets silently vanish and the resulting 403s
+     * are undiagnosable.
      */
-    dbLoader?: (unresolved: string[]) => Promise<PermissionSet[]>
+    dbLoader?: (unresolved: string[]) => Promise<PermissionSet[]>,
+    /** Optional logger; only `warn` is used. Resolution behavior is unchanged. */
+    options: { logger?: { warn?: (msg: string, meta?: Record<string, any>) => void } } = {},
   ): Promise<PermissionSet[]> {
     if (identifiers.length === 0) return [];
 
@@ -283,8 +289,12 @@ export class PermissionEvaluator {
         ?? metadataService?.list?.('permissions')
         ?? [];
       allPermSets = typeof (listed as any)?.then === 'function' ? await listed : listed;
-    } catch {
+    } catch (e) {
       allPermSets = [];
+      options.logger?.warn?.(
+        '[security] permission-set metadata list() failed — falling back to bootstrap/db sources (#2565)',
+        { requested: identifiers, error: (e as Error)?.message },
+      );
     }
     if (!Array.isArray(allPermSets)) allPermSets = [];
 
@@ -322,9 +332,16 @@ export class PermissionEvaluator {
               result.push(ps);
             }
           }
-        } catch {
+        } catch (e) {
           // Swallow — the request shouldn't fail just because the DB
-          // lookup is unavailable.
+          // lookup is unavailable (fail-closed: the unresolved sets simply
+          // grant nothing). But surface it: without this warn a transient
+          // DB error silently drops custom permission sets and the
+          // resulting 403s point nowhere near the cause (#2565).
+          options.logger?.warn?.(
+            '[security] sys_permission_set db lookup failed — unresolved sets grant nothing this request (#2565)',
+            { unresolved, error: (e as Error)?.message },
+          );
         }
       }
     }

packages/plugins/plugin-security/src/security-plugin.test.ts

@@ -1069,6 +1069,51 @@ describe('PermissionEvaluator', () => {
     expect(result).toEqual([psAdmin]);
   });
 
+  it('warns (and keeps resolving) when the dbLoader throws — #2565 observability', async () => {
+    const evaluator = new PermissionEvaluator();
+    const psAdmin = { name: 'admin_full_access' };
+    const metadata = { list: vi.fn().mockReturnValue([psAdmin]) };
+    const warns: Array<{ msg: string; meta?: Record<string, any> }> = [];
+    const result = await evaluator.resolvePermissionSets(
+      ['admin_full_access', 'custom_sales'],
+      metadata,
+      [],
+      async () => { throw new Error('db down'); },
+      { logger: { warn: (msg, meta) => warns.push({ msg, meta }) } },
+    );
+    // behavior unchanged: metadata-resolved set still returned, unresolved grants nothing
+    expect(result).toEqual([psAdmin]);
+    // ...but the swallow is surfaced, naming the unresolved sets
+    expect(warns.length).toBe(1);
+    expect(warns[0].msg).toContain('db lookup failed');
+    expect(warns[0].meta?.unresolved).toEqual(['custom_sales']);
+    expect(warns[0].meta?.error).toBe('db down');
+  });
+
+  it('warns (and keeps resolving via bootstrap) when metadata list() throws — #2565', async () => {
+    const evaluator = new PermissionEvaluator();
+    const metadata = { list: vi.fn().mockImplementation(() => { throw new Error('index broken'); }) };
+    const bootstrap = [{ name: 'member_default', objects: {} } as any];
+    const warns: string[] = [];
+    const result = await evaluator.resolvePermissionSets(
+      ['member_default'],
+      metadata,
+      bootstrap,
+      undefined,
+      { logger: { warn: (msg) => warns.push(msg) } },
+    );
+    expect(result.map((p) => p.name)).toEqual(['member_default']);
+    expect(warns.some((w) => w.includes('metadata list() failed'))).toBe(true);
+  });
+
+  it('stays silent when no logger is provided (back-compat)', async () => {
+    const evaluator = new PermissionEvaluator();
+    const metadata = { list: vi.fn().mockReturnValue([]) };
+    await expect(
+      evaluator.resolvePermissionSets(['x'], metadata, [], async () => { throw new Error('db down'); }),
+    ).resolves.toEqual([]);
+  });
+
   it('matches by both role and explicit permission-set identifiers', async () => {
     const evaluator = new PermissionEvaluator();
     const sets = [

packages/plugins/plugin-security/src/security-plugin.ts

@@ -929,6 +929,7 @@ export class SecurityPlugin implements Plugin {
       this.metadata,
       this.bootstrapPermissionSets,
       this.dbLoader,
+      { logger: this.logger },
     );
     // Post-resolution fallback — closes the fail-open hole where a populated
     // `roles` array maps to no permission set yet (no sys_role binding), which
@@ -943,6 +944,7 @@ export class SecurityPlugin implements Plugin {
         this.metadata,
         this.bootstrapPermissionSets,
         this.dbLoader,
+        { logger: this.logger },
       );
     }
     return permissionSets;