fix(security): derive better-auth managed-object write denies from the registry (#3325) (#3326)

os-zhuang · web-flow · commit 9d897b347347 · 2026-07-20T09:50:15.000+08:00
Derive the better-auth managed-object write deny-list from the live registry instead of a hand-maintained list (which had drifted 17/22). New applyManagedWriteDenies unions the denies into the shared in-memory default permission sets at kernel:ready; static baseline completed to 22 and pinned by test; engine-owned system/append-only objects deliberately excluded. plugin-security 539 passed. Closes #3325. Follows #3220 / ADR-0092 / ADR-0103 / ADR-0049.
diff --git a/.changeset/managed-deny-registry-drift.md b/.changeset/managed-deny-registry-drift.md
@@ -0,0 +1,11 @@
+---
+"@objectstack/plugin-security": patch
+---
+
+**Derive the better-auth managed-object write denies from the live registry (#3325, follow-through of ADR-0092 / ADR-0103).** The default permission sets deny generic writes on better-auth identity tables via a hand-maintained `BETTER_AUTH_MANAGED_OBJECTS` list — exactly the drift ADR-0092 forbids, and it had already drifted (the list carried 17 names while 22 schemas declare `managedBy: 'better-auth'`, leaving `sys_scim_provider`, `sys_sso_provider`, and three `sys_oauth_*` tables wildcard-granted for writes at the permission-evaluator layer; the identity write guard still 403'd the actual write, so this was a defense-in-depth gap, not a live hole).
+
+- New `applyManagedWriteDenies` (`managed-object-write-denies.ts`) injects a read-only-write deny for every registered `managedBy: 'better-auth'` object into the four write-granting default sets (`organization_admin`, `member_default`, `viewer_readonly`, MCP write) at `kernel:ready`, mutating the shared in-memory `bootstrapPermissionSets` in place (the array the evaluator resolves and the seeder serializes — a DB-row-only fix would be dead code). Never touches `admin_full_access`, never overrides an existing explicit entry, ignores `userActions` (the better-auth bucket is hard-denied — `sys_user`'s `userActions.edit` opens only a field-level whitelist the identity guard enforces).
+- The static `BETTER_AUTH_MANAGED_OBJECTS` list is completed to 22 and kept as a compile-time baseline (covers the pre-`kernel:ready` window), now pinned bidirectionally against the `@objectstack/platform-objects` schemas by a test so it cannot silently rot again.
+- Engine-owned `system`/`append-only` objects are deliberately NOT given deny entries — a per-object entry overrides the wildcard and would drop `viewAllRecords`; their writes are already rejected by the ADR-0103 engine guard.
+
+No public API change; the helper is internal. Behavior is byte-preserving for the 17 already-listed tables and closes the gap on the 5 that had drifted.
diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin.test.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin.test.ts
@@ -15,6 +15,7 @@
 
 import { describe, it, expect } from 'vitest';
 import { bootstrapPlatformAdmin } from './bootstrap-platform-admin.js';
+import { applyManagedWriteDenies } from './managed-object-write-denies.js';
 
 /** Minimal in-memory ql. Only `sys_permission_set` is modeled; the admin-
  *  promotion tables return empty, so promotion short-circuits and we assert the
@@ -107,3 +108,36 @@ describe('bootstrapPlatformAdmin — insert-once vs resync (#2705)', () => {
     expect(JSON.parse(row(ql)!.system_permissions)).toEqual(['setup.access']);
   });
 });
+
+// #3325 — the transform runs upstream in runBootstrap; here we confirm that once
+// a set has been enriched by applyManagedWriteDenies, bootstrapPlatformAdmin
+// serializes the injected denies into the seeded object_permissions JSON.
+describe('bootstrapPlatformAdmin — managed write denies land in the seed row (#3325)', () => {
+  const schemas = [
+    { name: 'sys_sso_provider', managedBy: 'better-auth' },
+    { name: 'crm_lead', managedBy: 'platform' },
+  ];
+  // member_default is a target set; give it the wildcard shape the real one has.
+  const enriched = () => {
+    const set = { name: 'member_default', label: 'Member', objects: { '*': { allowRead: true, allowCreate: true } } } as any;
+    applyManagedWriteDenies([set], schemas);
+    return set;
+  };
+
+  it('fresh DB: the injected deny serializes into object_permissions', async () => {
+    const ql = makeQl([]);
+    await bootstrapPlatformAdmin(ql, [enriched()]);
+    const perms = JSON.parse(row(ql)!.object_permissions);
+    expect(perms.sys_sso_provider).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false });
+    expect(perms.crm_lead).toBeUndefined(); // platform bucket never denied
+  });
+
+  it('resync: reconciles a stale platform-owned row that predates the deny', async () => {
+    const ql = makeQl([
+      { id: 'ps_old', name: 'member_default', system_permissions: '[]', object_permissions: '{}', managed_by: null },
+    ]);
+    const r = await bootstrapPlatformAdmin(ql, [enriched()], { resync: true });
+    expect(r.resynced).toBe(1);
+    expect(JSON.parse(row(ql)!.object_permissions).sys_sso_provider.allowCreate).toBe(false);
+  });
+});
diff --git a/packages/plugins/plugin-security/src/managed-object-write-denies.test.ts b/packages/plugins/plugin-security/src/managed-object-write-denies.test.ts
@@ -0,0 +1,108 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+// ADR-0092 / ADR-0103 / #3325 — registry-driven managed-object write denies.
+
+import { describe, it, expect } from 'vitest';
+import {
+  applyManagedWriteDenies,
+  MANAGED_DENY_ENTRY,
+  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';
+
+// Minimal PermissionSet-shaped fixtures (only name + objects matter here).
+const set = (name: string, objects: Record<string, unknown> = {}): any => ({ name, objects });
+
+const DENY = { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false };
+
+const schemas = [
+  { name: 'sys_user', managedBy: 'better-auth', userActions: { edit: true } }, // intentional divergence
+  { name: 'sys_sso_provider', managedBy: 'better-auth' },
+  { name: 'crm_lead', managedBy: 'platform' },
+  { name: 'sys_setting', managedBy: 'system' },
+  { name: 'sys_audit_log', managedBy: 'append-only' },
+  { name: 'sys_sharing_rule', managedBy: 'config' },
+  { name: 'sys_no_bucket' }, // unset
+];
+
+describe('applyManagedWriteDenies (#3325)', () => {
+  it('injects a read-only-write deny for every better-auth object into the four target sets', () => {
+    const sets = [
+      set('organization_admin'),
+      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);
+    expect(res.skippedExisting).toBe(0);
+    for (const s of sets) {
+      expect(s.objects.sys_user).toEqual(DENY);
+      expect(s.objects.sys_sso_provider).toEqual(DENY);
+    }
+  });
+
+  it('hard-denies sys_user despite userActions.edit:true (permission-set booleans cannot whitelist fields)', () => {
+    const s = set('member_default');
+    applyManagedWriteDenies([s], schemas);
+    expect(s.objects.sys_user).toEqual(DENY);
+    expect(s.objects.sys_user.allowEdit).toBe(false);
+  });
+
+  it('ignores platform / config / system / append-only / unset buckets (pins the ADR-0103 deferral)', () => {
+    const s = set('member_default');
+    applyManagedWriteDenies([s], schemas);
+    for (const name of ['crm_lead', 'sys_setting', 'sys_audit_log', 'sys_sharing_rule', 'sys_no_bucket']) {
+      expect(s.objects[name]).toBeUndefined();
+    }
+  });
+
+  it('never touches admin_full_access or the MCP read/restricted sets', () => {
+    const admin = set('admin_full_access', { '*': { allowRead: true, allowCreate: true } });
+    const mcpRead = set(MCP_AGENT_PERMISSION_SET_READ, { '*': { allowRead: true } });
+    const res = applyManagedWriteDenies([admin, mcpRead], schemas);
+    expect(res.applied).toBe(0);
+    expect(admin.objects).toEqual({ '*': { allowRead: true, allowCreate: true } });
+    expect(mcpRead.objects.sys_user).toBeUndefined();
+  });
+
+  it('never overrides an existing explicit entry (protects the org-admin RBAC block / static baseline)', () => {
+    // Mirror org-admin: sys_user already carved out read-only; keep it verbatim.
+    const existing = { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, viewAllRecords: true };
+    const s = set('organization_admin', { sys_user: existing });
+    const res = applyManagedWriteDenies([s], schemas);
+    expect(s.objects.sys_user).toBe(existing); // same reference, untouched (viewAllRecords preserved)
+    expect(res.skippedExisting).toBe(1); // sys_user skipped
+    expect(res.applied).toBe(1); // sys_sso_provider injected
+    expect(s.objects.sys_sso_provider).toEqual(DENY);
+  });
+
+  it('is idempotent — a second apply injects nothing', () => {
+    const s = set('viewer_readonly');
+    const first = applyManagedWriteDenies([s], schemas);
+    const second = applyManagedWriteDenies([s], schemas);
+    expect(first.applied).toBe(2);
+    expect(second.applied).toBe(0);
+    expect(second.skippedExisting).toBe(2);
+  });
+
+  it('injects a distinct object per entry (no shared reference across tables)', () => {
+    const s = set('member_default');
+    applyManagedWriteDenies([s], schemas);
+    expect(s.objects.sys_user).not.toBe(s.objects.sys_sso_provider);
+    expect(s.objects.sys_user).not.toBe(MANAGED_DENY_ENTRY);
+  });
+
+  it('tolerates empty / malformed input', () => {
+    expect(applyManagedWriteDenies([], schemas)).toEqual({ applied: 0, skippedExisting: 0 });
+    expect(applyManagedWriteDenies([set('member_default')], [])).toEqual({ applied: 0, skippedExisting: 0 });
+    // a target set with no objects map is skipped, not thrown on
+    expect(() => applyManagedWriteDenies([{ name: 'member_default' } as any], schemas)).not.toThrow();
+  });
+
+  it('the target allowlist is exactly the four write-granting sets', () => {
+    expect([...MANAGED_DENY_TARGET_SETS].sort()).toEqual(
+      ['member_default', 'organization_admin', 'viewer_readonly', MCP_AGENT_PERMISSION_SET_WRITE].sort(),
+    );
+  });
+});
diff --git a/packages/plugins/plugin-security/src/managed-object-write-denies.ts b/packages/plugins/plugin-security/src/managed-object-write-denies.ts
@@ -0,0 +1,128 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * ADR-0092 / ADR-0103 — registry-driven managed-object write denies for the
+ * default permission sets.
+ *
+ * The default write-granting permission sets (`organization_admin`,
+ * `member_default`, `viewer_readonly`, and the MCP write set) grant CRUD via a
+ * `'*'` wildcard, then DENY writes on the better-auth-managed identity tables so
+ * mutations must flow through the auth pipeline (ADR-0092). That deny-list was a
+ * hand-maintained object-name array (`BETTER_AUTH_MANAGED_OBJECTS` in
+ * `default-permission-sets.ts`) — precisely the drift ADR-0092 forbids, and it
+ * HAD drifted (the static list missed schemas that later declared
+ * `managedBy: 'better-auth'`). This module derives the deny-list from the live
+ * registry so the permission layer can never silently disagree with the schemas.
+ *
+ * ── Reference-sharing contract (why this mutates in place) ──
+ * The runtime permission evaluator resolves the default sets from the
+ * in-memory `bootstrapPermissionSets` objects, NOT from the seeded
+ * `sys_permission_set.object_permissions` DB JSON (a DB-row-only fix would be
+ * dead code). Those same `PermissionSet` instances are the ones registered via
+ * `manifest.register({ permissions })`, handed to the evaluator fallback, AND
+ * serialized into the seed row — the registry stores items by reference. So a
+ * single in-place mutation of `set.objects` here, run once at `kernel:ready`
+ * before the platform-admin seeder, updates every consumer atomically.
+ *
+ * ── Why the better-auth bucket is hard-denied, ignoring `userActions` ──
+ * `sys_user` declares `userActions: { edit: true }` (it opens name/image edits
+ * for the UI and the identity guard's field whitelist), so its resolved CRUD
+ * affordance grants `edit`. But a permission-set boolean cannot express a
+ * field-level whitelist, so the default sets must still deny `allowEdit` on
+ * `sys_user` — the field-level narrowing is the identity write guard's job. We
+ * therefore key off the raw `managedBy: 'better-auth'` bucket and deny writes
+ * unconditionally, rather than deriving from `resolveCrudAffordances` (which
+ * would wrongly widen `sys_user`). This byte-preserves the prior behavior.
+ *
+ * ── Deliberately NOT covered: engine-owned system / append-only objects ──
+ * ADR-0103's engine-owned objects (`sys_audit_log`, `sys_automation_run`, …)
+ * are also wildcard-granted in these sets, but we do NOT inject deny entries for
+ * them: a per-object entry FULLY OVERRIDES the wildcard (lookup, not merge — see
+ * `default-permission-sets.ts`), so injecting `{ allowRead: true, ...writes:false }`
+ * would silently drop the wildcard's `viewAllRecords` / `modifyAllRecords` — a
+ * read-side narrowing. Their user-context writes are already rejected at the
+ * engine by `assertEngineOwnedWriteAllowed` (ADR-0103) and reflected in the
+ * `/me/permissions` clamp, so the permission-set layer needs no change for them.
+ */
+
+import type { PermissionSet } from '@objectstack/spec/security';
+import { MCP_AGENT_PERMISSION_SET_WRITE } from '@objectstack/spec/ai';
+
+/**
+ * The write posture forced onto every managed identity table: readable, but no
+ * generic create / edit / delete. Same shape as the entries the static
+ * `denyWritesOnManagedObjects()` baseline emits (deliberately no
+ * `viewAllRecords` / `modifyAllRecords` key).
+ */
+export const MANAGED_DENY_ENTRY = {
+  allowRead: true,
+  allowCreate: false,
+  allowEdit: false,
+  allowDelete: false,
+};
+
+/**
+ * The default sets whose `'*'` wildcard grants writes and therefore must carry
+ * the managed-table denies. 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).
+ */
+export const MANAGED_DENY_TARGET_SETS: readonly string[] = [
+  'organization_admin',
+  'member_default',
+  'viewer_readonly',
+  MCP_AGENT_PERMISSION_SET_WRITE,
+];
+
+interface SchemaLike {
+  name?: string;
+  managedBy?: string;
+}
+
+export interface ApplyManagedWriteDeniesResult {
+  /** Number of (set, object) deny entries newly injected. */
+  applied: number;
+  /** Number of (set, object) pairs left untouched because an entry already existed. */
+  skippedExisting: number;
+}
+
+/**
+ * Inject a read-only-write deny entry for every `managedBy: 'better-auth'`
+ * object into each target set that lacks an explicit entry for it. In-place and
+ * idempotent (never overrides an existing entry — so the static baseline and
+ * any deliberate per-object carve-out, e.g. org-admin's RBAC read-only block,
+ * are preserved). Returns counts for logging / tests.
+ */
+export function applyManagedWriteDenies(
+  sets: PermissionSet[],
+  schemas: SchemaLike[],
+): ApplyManagedWriteDeniesResult {
+  const managedNames = Array.from(
+    new Set(
+      (schemas ?? [])
+        .filter((s) => s?.managedBy === 'better-auth' && typeof s?.name === 'string')
+        .map((s) => s.name as string),
+    ),
+  ).sort();
+
+  const targets = new Set(MANAGED_DENY_TARGET_SETS);
+  let applied = 0;
+  let skippedExisting = 0;
+
+  for (const set of sets ?? []) {
+    if (!set || !targets.has((set as { name?: string }).name ?? '')) continue;
+    const objects = (set as { objects?: Record<string, unknown> }).objects;
+    if (!objects || typeof objects !== 'object') continue;
+    for (const name of managedNames) {
+      if (name in objects) {
+        skippedExisting++;
+        continue;
+      }
+      objects[name] = { ...MANAGED_DENY_ENTRY };
+      applied++;
+    }
+  }
+
+  return { applied, skippedExisting };
+}
diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts
@@ -0,0 +1,62 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+// #3325 — pin the better-auth managed-object deny-list against the real schemas
+// so it can never silently drift again (ADR-0092: registry-driven, no rot).
+
+import { describe, it, expect } from 'vitest';
+import * as PlatformObjects from '@objectstack/platform-objects';
+import { defaultPermissionSets, BETTER_AUTH_MANAGED_OBJECTS } from './default-permission-sets.js';
+import { 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.
+const betterAuthSchemaNames = Object.values(PlatformObjects as Record<string, any>)
+  .filter((v) => v && typeof v === 'object' && typeof v.name === 'string' && v.managedBy === 'better-auth')
+  .map((v) => v.name as string)
+  .sort();
+
+const listNames = [...BETTER_AUTH_MANAGED_OBJECTS].sort();
+const setByName = (name: string): any => defaultPermissionSets.find((s) => s.name === name);
+
+describe('BETTER_AUTH_MANAGED_OBJECTS ↔ schemas (drift pin, #3325)', () => {
+  it('found the better-auth identity schemas to compare against', () => {
+    // Guard against a broken import silently passing the bidirectional check.
+    expect(betterAuthSchemaNames.length).toBeGreaterThanOrEqual(20);
+  });
+
+  it('every listed name is a real object declaring managedBy:"better-auth"', () => {
+    const notBetterAuth = listNames.filter((n) => !betterAuthSchemaNames.includes(n));
+    expect(notBetterAuth).toEqual([]);
+  });
+
+  it('every better-auth schema is in the list (this is the drift that #3325 fixes)', () => {
+    const missing = betterAuthSchemaNames.filter((n) => !listNames.includes(n));
+    expect(missing).toEqual([]);
+  });
+
+  it('the list has no duplicates', () => {
+    expect(listNames.length).toBe(new Set(listNames).size);
+  });
+});
+
+describe('default permission sets carry the managed denies (static baseline)', () => {
+  it('each write-granting target set denies create/edit/delete on every managed object', () => {
+    for (const setName of MANAGED_DENY_TARGET_SETS) {
+      const set = setByName(setName);
+      expect(set, `set ${setName} exists`).toBeTruthy();
+      for (const obj of BETTER_AUTH_MANAGED_OBJECTS) {
+        const entry = set.objects[obj];
+        expect(entry, `${setName} has entry for ${obj}`).toBeTruthy();
+        expect(entry.allowCreate).toBe(false);
+        expect(entry.allowEdit).toBe(false);
+        expect(entry.allowDelete).toBe(false);
+        expect(entry.allowRead).toBe(true);
+      }
+    }
+  });
+
+  it('admin_full_access keeps its bare wildcard (zero per-object entries) — admin rescue path', () => {
+    const admin = setByName('admin_full_access');
+    expect(admin).toBeTruthy();
+    expect(Object.keys(admin.objects)).toEqual(['*']);
+  });
+});
diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts
@@ -22,10 +22,18 @@ import {
  * permission sets keep their `*` wildcard so they can rescue data
  * directly when needed.
  *
- * Each entry mirrors the `managedBy: 'better-auth'` flag declared on
- * the corresponding object schema in `packages/platform-objects/src/identity/`.
+ * This is the COMPILE-TIME BASELINE. At `kernel:ready` it is unioned with the
+ * live registry by `applyManagedWriteDenies` (see `managed-object-write-denies.ts`),
+ * which injects a deny entry for every registered `managedBy: 'better-auth'`
+ * object the baseline missed — so a newly-declared identity table is covered
+ * automatically without editing this list. The baseline still matters: it covers
+ * the pre-`kernel:ready` window and hook-less test-stub kernels where the
+ * registry may be empty. `objects/default-permission-sets.test.ts` pins this
+ * list bidirectionally against the `@objectstack/platform-objects` schemas so it
+ * cannot silently rot again (the drift that motivated ADR-0092's registry-driven
+ * rule).
  */
-const BETTER_AUTH_MANAGED_OBJECTS = [
+export const BETTER_AUTH_MANAGED_OBJECTS = [
   'sys_user',
   'sys_account',
   'sys_session',
@@ -39,10 +47,15 @@ const BETTER_AUTH_MANAGED_OBJECTS = [
   'sys_verification',
   'sys_jwks',
   'sys_device_code',
+  'sys_scim_provider',
+  'sys_sso_provider',
   'sys_oauth_application',
   'sys_oauth_access_token',
   'sys_oauth_refresh_token',
   'sys_oauth_consent',
+  'sys_oauth_resource',
+  'sys_oauth_client_resource',
+  'sys_oauth_client_assertion',
 ] as const;
 
 const denyWritesOnManagedObjects = (): Record<string, {
diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts
diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts
