fix(plugin-security): bring organization_admin_no_bypass under registry-driven managed-write denies (#14129)

* fix(plugin-security): bring organization_admin_no_bypass under registry-driven managed-write denies (#14029)

The derived wall-less org-admin variant holds a write-granting '*' wildcard
but was not in MANAGED_DENY_TARGET_SETS, so applyManagedWriteDenies walked it
and skipped it at kernel:ready — and because deriveWallLessOrgAdmin takes a
shallow copy at module load, injections into the parent could never propagate
either. Add the variant to the target list, and replace the self-referential
membership pin with one that derives the required floor (write-granting
wildcard sets) from the real seeded sets and diffs it against the list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs

* fix(plugin-security): widen the #14029 floor to the modifyAllRecords write route (contract-review blocking item)

The independent-property floor in default-permission-sets.test.ts recognised
only the three CRUD flags, but the evaluator grants writes by a second route:
MODIFY_ALL_WRITE_KEYS + objPerm.modifyAllRecords (permission-evaluator.ts:224)
covers allowEdit, allowDelete and the destructive class. A future default set
shaped '*': { allowRead: true, modifyAllRecords: true } therefore held
edit/delete on every future identity table in the evaluator's own terms, yet
had all three CRUD flags false, escaped the floor and tripped no pin —
contradicting the MANAGED_DENY_TARGET_SETS docblock promise that a
write-granting set not listed there fails a pin. Same "a set that should be a
member can hide" class this card exists to kill, one size smaller.

- default-permission-sets.test.ts: add `|| wc.modifyAllRecords === true` to
  the floor filter. Value test (`=== true`) kept deliberately: Zod
  materialises both superuser bits with .default(false)
  (permission.zod.ts:268), so they are present-as-false and a key-existence
  test would misfire today.
- managed-object-write-denies.ts: align the docblock — the floor is "grants
  any generic write class via the three write flags OR modifyAllRecords",
  not "grants create/edit/delete".
- .changeset: the static baseline covers 28 managed tables
  (BETTER_AUTH_MANAGED_OBJECTS), not 30 (non-blocking item).

Zero behaviour delta on today's tree: organization_admin and
admin_full_access carry all three CRUD flags true, so the derived floor set
is unchanged. Measured: with a temporary '*': { allowRead: true,
modifyAllRecords: true } probe set absent from MANAGED_DENY_TARGET_SETS, the
widened pin goes red naming exactly the probe; the old three-flag filter
stays green on the same probe (the blind spot). Probe removed; restore
proven by blob hash against HEAD. plugin-security suite 94 files / 1770
tests green; all three tsc programs green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-steve

.changeset/no-bypass-managed-deny-target.md

@@ -0,0 +1,34 @@
+---
+"@objectstack/plugin-security": patch
+---
+
+fix(plugin-security): registry-driven managed-object write denies now reach `organization_admin_no_bypass` (#14029)
+
+`MANAGED_DENY_TARGET_SETS` named four default sets, and `applyManagedWriteDenies`
+matches on it exactly — so at `kernel:ready` the injection walked the derived
+`organization_admin_no_bypass` variant and skipped it. The variant is a shallow
+copy of `organization_admin` taken at module load (`deriveWallLessOrgAdmin`
+strips only the `viewAllRecords`/`modifyAllRecords` superuser bits), which means
+its `'*'` wildcard still grants create/edit/delete AND entries injected into the
+parent's `objects` can never propagate to it. Its own docblock declares
+"managed-write denies … carried over verbatim"; the behaviour violated that
+declared contract.
+
+No gap opens on today's tree — the static `BETTER_AUTH_MANAGED_OBJECTS`
+baseline covers the 28 declared managed tables and is copied into the variant at
+derivation. The gap was the next `managedBy: 'better-auth'` schema that lands
+without a hand edit to that list: `organization_admin` would receive the
+injected deny while the wall-less variant's wildcard kept granting raw CRUD on
+an identity table — precisely the drift the registry-driven module exists to
+close (ADR-0092), on the posture (`auto-org-admin-grant` under a wall-less
+deployment) where the bits are least bounded.
+
+- `ORGANIZATION_ADMIN_NO_BYPASS` is now a member of `MANAGED_DENY_TARGET_SETS`.
+  The variant's pre-existing explicit entries (static baseline, RBAC read-only
+  block) survive unchanged — the injection skips any object a set already
+  names.
+- The membership pin no longer checks the list against itself: the required
+  floor ("default sets holding a write-granting `'*'` wildcard") is derived
+  from the real seeded sets and diffed against the list; a non-empty
+  difference is red. `admin_full_access` stays deliberately excluded (admin
+  rescue path) and that exclusion is pinned exactly.

packages/plugins/plugin-security/src/managed-object-write-denies.test.ts

@@ -8,6 +8,7 @@ import {
   MANAGED_DENY_TARGET_SETS,
 } from './managed-object-write-denies.js';
 import { MCP_AGENT_PERMISSION_SET_WRITE, MCP_AGENT_PERMISSION_SET_READ } from '@objectstack/spec/ai';
+import { ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec';
 
 // Minimal PermissionSet-shaped fixtures (only name + objects matter here).
 const set = (name: string, objects: Record<string, unknown> = {}): any => ({ name, objects });
@@ -25,16 +26,17 @@ const schemas = [
 ];
 
 describe('applyManagedWriteDenies (#3325)', () => {
-  it('injects a read-only-write deny for every better-auth object into the four target sets', () => {
+  it('injects a read-only-write deny for every better-auth object into the five target sets', () => {
     const sets = [
       set('organization_admin'),
+      set(ORGANIZATION_ADMIN_NO_BYPASS), // #14029 — derived at module load, so it needs its own injection
       set('member_default'),
       set('viewer_readonly'),
       set(MCP_AGENT_PERMISSION_SET_WRITE),
     ];
     const res = applyManagedWriteDenies(sets, schemas);
-    // 2 better-auth objects × 4 sets = 8 injections.
-    expect(res.applied).toBe(8);
+    // 2 better-auth objects × 5 sets = 10 injections.
+    expect(res.applied).toBe(10);
     expect(res.skippedExisting).toBe(0);
     for (const s of sets) {
       expect(s.objects.sys_user).toEqual(DENY);
@@ -100,9 +102,20 @@ describe('applyManagedWriteDenies (#3325)', () => {
     expect(() => applyManagedWriteDenies([{ name: 'member_default' } as any], schemas)).not.toThrow();
   });
 
-  it('the target allowlist is exactly the four write-granting sets', () => {
+  it('the target allowlist is exactly the five known sets (see the independent-property pin for the floor)', () => {
+    // Exact membership record. This is deliberately NOT the only pin on the
+    // list: `objects/default-permission-sets.test.ts` derives the required
+    // floor ("holds a write-granting '*' wildcard") from the real default sets
+    // and diffs it against this list, so a set that SHOULD be a member cannot
+    // hide behind an assertion that only restates the list (#14029).
     expect([...MANAGED_DENY_TARGET_SETS].sort()).toEqual(
-      ['member_default', 'organization_admin', 'viewer_readonly', MCP_AGENT_PERMISSION_SET_WRITE].sort(),
+      [
+        'member_default',
+        'organization_admin',
+        ORGANIZATION_ADMIN_NO_BYPASS,
+        'viewer_readonly',
+        MCP_AGENT_PERMISSION_SET_WRITE,
+      ].sort(),
     );
   });
 });

packages/plugins/plugin-security/src/managed-object-write-denies.ts

@@ -4,12 +4,14 @@
  * ADR-0092 / ADR-0103 — registry-driven managed-object write denies for the
  * default permission sets.
  *
- * The default sets this module targets (`organization_admin`, `member_default`,
- * `viewer_readonly`, and the MCP write set) DENY writes on the
+ * The default sets this module targets (`organization_admin`, its derived
+ * `organization_admin_no_bypass` variant, `member_default`, `viewer_readonly`,
+ * and the MCP write set) DENY writes on the
  * better-auth-managed identity tables so mutations must flow through the auth
  * pipeline (ADR-0092). What that entry narrows differs per set, and the
- * difference is load-bearing for the rest of this file: `organization_admin` and
- * the MCP write set grant CRUD through an `objects['*']` wildcard, so their
+ * difference is load-bearing for the rest of this file: `organization_admin`,
+ * its no-bypass variant and the MCP write set grant CRUD through an
+ * `objects['*']` wildcard, so their
  * managed-table entries are a narrowing overlay; `viewer_readonly`'s wildcard is
  * read-only, so its entries are belt-and-suspenders over a wildcard that already
  * denies; and `member_default` carries NO wildcard at all since #5491 (the
@@ -59,6 +61,7 @@
  */
 
 import type { PermissionSet } from '@objectstack/spec/security';
+import { ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec';
 import { MCP_AGENT_PERMISSION_SET_WRITE } from '@objectstack/spec/ai';
 
 /**
@@ -76,16 +79,38 @@ export const MANAGED_DENY_ENTRY = {
 
 /**
  * The default sets that must carry an explicit entry for every managed table.
- * Membership is NOT "holds a write-granting `'*'` wildcard" — only
- * `organization_admin` and the MCP write set hold one; `viewer_readonly`'s
- * wildcard is read-only and `member_default` has held none since #5491 (see the
- * module docblock for what the injected entry does in each). Explicit allowlist
- * — `admin_full_access` is deliberately excluded (it keeps its unqualified
- * wildcard so an admin can rescue data directly; the runtime guards are its
- * boundary), as are the MCP read / restricted sets (they grant no writes).
+ *
+ * Holding a write-granting `'*'` wildcard is the FLOOR of membership, not its
+ * definition: every default set whose wildcard grants any generic write class
+ * — via the three write flags (`allowCreate`/`allowEdit`/`allowDelete`) OR via
+ * `modifyAllRecords`, whose super-user bypass grants edit/delete and the
+ * destructive class by the evaluator's second route (`MODIFY_ALL_WRITE_KEYS`,
+ * `permission-evaluator.ts`) — must be listed here (or carry a documented
+ * exclusion below), because the wildcard is what would otherwise grant raw
+ * writes on a newly-declared identity table — that floor is what
+ * `default-permission-sets.test.ts` derives independently and diffs against
+ * this list (#14029), so a future write-granting set that is not added here
+ * fails a pin instead of silently keeping its wildcard.
+ * Membership is WIDER than the floor: `viewer_readonly`'s wildcard is read-only
+ * and `member_default` has held none since #5491 — their injected entries are
+ * belt-and-suspenders and the read grant itself, respectively (see the module
+ * docblock for what the injected entry does in each).
+ *
+ * `organization_admin_no_bypass` is a member in its own right (#14029): it is
+ * derived from `organization_admin` by a SHALLOW copy taken at module load
+ * (`deriveWallLessOrgAdmin`), so entries injected into the parent's `objects`
+ * at `kernel:ready` can never propagate to it — dropping only the superuser
+ * bits leaves `allowCreate`/`allowEdit`/`allowDelete` true on its wildcard,
+ * exactly the shape this module exists to narrow.
+ *
+ * Documented exclusions: `admin_full_access` is deliberately NOT a member (it
+ * keeps its unqualified wildcard so an admin can rescue data directly; the
+ * runtime guards are its boundary), and the MCP read / restricted sets grant
+ * no writes for a deny to narrow.
  */
 export const MANAGED_DENY_TARGET_SETS: readonly string[] = [
   'organization_admin',
+  ORGANIZATION_ADMIN_NO_BYPASS,
   'member_default',
   'viewer_readonly',
   MCP_AGENT_PERMISSION_SET_WRITE,

packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts

@@ -7,7 +7,7 @@ import * as PlatformObjects from '@objectstack/platform-objects';
 import { PermissionSetSchema } from '@objectstack/spec/security';
 import { ADMIN_FULL_ACCESS_CAPABILITIES } from '@objectstack/spec';
 import { defaultPermissionSets, BETTER_AUTH_MANAGED_OBJECTS } from './default-permission-sets.js';
-import { MANAGED_DENY_TARGET_SETS } from '../managed-object-write-denies.js';
+import { applyManagedWriteDenies, MANAGED_DENY_TARGET_SETS } from '../managed-object-write-denies.js';
 
 // Every object schema the platform-objects package exports whose bucket is
 // `better-auth` — the ground truth the static baseline must mirror.
@@ -368,3 +368,116 @@ describe('admin_full_access imports the kernel capability declaration unchanged
     expect(admin.systemPermissions).toEqual(ADMIN_FULL_ACCESS_CAPABILITIES.systemPermissions);
   });
 });
+
+/**
+ * [#14029] The managed-deny target list, pinned against an INDEPENDENT
+ * property instead of against itself.
+ *
+ * The old shape of this pin iterated `MANAGED_DENY_TARGET_SETS` to assert
+ * membership, so it was structurally unable to see a set that SHOULD have been
+ * a member — which is exactly how `organization_admin_no_bypass` (a shallow
+ * copy of `organization_admin` taken at module load, write-granting wildcard
+ * intact) sat outside the list while `applyManagedWriteDenies` walked it and
+ * skipped it at `kernel:ready`. The floor is therefore derived here from the
+ * REAL seeded sets — "holds a `'*'` wildcard granting any generic write
+ * class" — and diffed against the list; a non-empty difference is red.
+ */
+describe('managed-deny targets — independent-property floor + registry union reaches the derived variant (#14029)', () => {
+  // Derived from `defaultPermissionSets`, never from the list under test.
+  // "Grants a write" in the evaluator's own terms: the three CRUD flags OR
+  // `modifyAllRecords` — the super-user bypass grants edit/delete and the
+  // destructive class by a second route (`MODIFY_ALL_WRITE_KEYS`,
+  // `permission-evaluator.ts`), so `'*': { modifyAllRecords: true }` is
+  // write-granting even with all three CRUD flags false. Value tests
+  // (`=== true`), not key-existence: Zod materialises the superuser bits with
+  // `.default(false)` (`permission.zod.ts`), so they are present-as-false.
+  const writeGrantingWildcardSets: string[] = defaultPermissionSets
+    .filter((s: any) => {
+      const wc = s.objects?.['*'];
+      return (
+        !!wc &&
+        (wc.allowCreate === true ||
+          wc.allowEdit === true ||
+          wc.allowDelete === true ||
+          wc.modifyAllRecords === true)
+      );
+    })
+    .map((s) => s.name)
+    .sort();
+
+  /**
+   * The one documented exclusion: `admin_full_access` keeps its unqualified
+   * wildcard so an admin can rescue data directly (recorded in the
+   * `MANAGED_DENY_TARGET_SETS` docblock; runtime guards are its boundary).
+   * Pinned exactly, like `EDIT_EXCEPTIONS` above: widening it is an edit HERE,
+   * which is the moment a reviewer is asked why the new set may keep raw CRUD
+   * on identity tables.
+   */
+  const WILDCARD_DENY_EXCLUSIONS = ['admin_full_access'];
+
+  it('the property derivation is live (found the known write-granting sets)', () => {
+    // If the filter silently matched nothing, the difference below would be
+    // vacuously empty — guard the probe itself.
+    expect(writeGrantingWildcardSets).toContain('organization_admin');
+    expect(writeGrantingWildcardSets).toContain('organization_admin_no_bypass');
+    expect(writeGrantingWildcardSets.length).toBeGreaterThanOrEqual(3);
+  });
+
+  it('every write-granting wildcard set is a managed-deny target or a documented exclusion', () => {
+    const missing = writeGrantingWildcardSets.filter(
+      (name) => !MANAGED_DENY_TARGET_SETS.includes(name) && !WILDCARD_DENY_EXCLUSIONS.includes(name),
+    );
+    expect(missing, 'write-granting sets missing from MANAGED_DENY_TARGET_SETS').toEqual([]);
+  });
+
+  it('the exclusion list is exactly admin_full_access, and it really is outside the target list', () => {
+    expect(WILDCARD_DENY_EXCLUSIONS).toEqual(['admin_full_access']);
+    expect([...MANAGED_DENY_TARGET_SETS]).not.toContain('admin_full_access');
+  });
+
+  // ── The behaviour the membership buys, measured on the REAL derived set ──
+  // (clones so the module-level instances other tests read stay unmutated;
+  // the kernel path hands the same objects to the same function in place).
+
+  const FUTURE = 'sys_future_identity_table';
+  const DENY = { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false };
+
+  it('a managedBy:better-auth object OUTSIDE the static list now reaches the variant (the card)', () => {
+    const sets: any[] = structuredClone(defaultPermissionSets as any);
+    const variant = sets.find((s) => s.name === 'organization_admin_no_bypass');
+    const parent = sets.find((s) => s.name === 'organization_admin');
+    expect(variant.objects[FUTURE]).toBeUndefined(); // genuinely not in the compile-time baseline
+    applyManagedWriteDenies(sets, [{ name: FUTURE, managedBy: 'better-auth' }]);
+    expect(variant.objects[FUTURE]).toEqual(DENY);
+    // The fix ADDS a target; the parent keeps receiving its injection too.
+    expect(parent.objects[FUTURE]).toEqual(DENY);
+  });
+
+  it('reverse control: the variant pre-existing explicit entries survive the injection unchanged', () => {
+    const sets: any[] = structuredClone(defaultPermissionSets as any);
+    const variant = sets.find((s) => s.name === 'organization_admin_no_bypass');
+    const before = structuredClone(variant.objects);
+    const registry = [
+      ...BETTER_AUTH_MANAGED_OBJECTS.map((n) => ({ name: n, managedBy: 'better-auth' })),
+      { name: FUTURE, managedBy: 'better-auth' },
+    ];
+    applyManagedWriteDenies(sets, registry);
+    for (const name of BETTER_AUTH_MANAGED_OBJECTS) {
+      expect(variant.objects[name], `variant entry ${name}`).toEqual(before[name]);
+    }
+    // Wildcard and the anti-escalation RBAC read-only block untouched as well.
+    expect(variant.objects['*']).toEqual(before['*']);
+    expect(variant.objects.sys_position).toEqual(before.sys_position);
+    // Only the future table was new on the variant.
+    expect(variant.objects[FUTURE]).toEqual(DENY);
+    expect(Object.keys(variant.objects).sort()).toEqual([...Object.keys(before), FUTURE].sort());
+  });
+
+  it('control: admin_full_access is untouched by the injection (admin rescue path)', () => {
+    const sets: any[] = structuredClone(defaultPermissionSets as any);
+    const admin = sets.find((s) => s.name === 'admin_full_access');
+    const before = structuredClone(admin);
+    applyManagedWriteDenies(sets, [{ name: FUTURE, managedBy: 'better-auth' }]);
+    expect(admin).toEqual(before);
+  });
+});

packages/plugins/plugin-security/src/objects/default-permission-sets.ts

@@ -1037,6 +1037,14 @@ const baseDefaultPermissionSets: PermissionSet[] = [
  * silent privilege difference. The only intended delta is the superuser bits,
  * so the only thing this function may do is remove them.
  *
+ * ⚠️ The copy is SHALLOW and taken at MODULE LOAD, so what it carries over is
+ * the compile-time baseline only — the registry-driven managed-write denies
+ * that `applyManagedWriteDenies` injects into the parent's `objects` at
+ * `kernel:ready` can never propagate here. "Carried over verbatim" holds for
+ * the registry union because the variant is its own member of
+ * `MANAGED_DENY_TARGET_SETS` and receives the same injection directly
+ * (#14029), not because the derivation sees it.
+ *
  * `auto-org-admin-grant` picks between the two by posture: a wall-enforcing
  * posture bounds the bits (grant `organization_admin`); a wall-less one does
  * not (grant this).