Skip to content

BETTER_AUTH_MANAGED_OBJECTS deny-list has already drifted (17/22) — derive managed-object write denies from the registry (ADR-0092/0103 follow-through) #3325

Description

@os-zhuang

Follow-through of #3220 / ADR-0103 (merged in #3315). All findings below were verified against current main; file:line references are included so this is directly implementable.

Problem

packages/plugins/plugin-security/src/objects/default-permission-sets.ts:28-46 hard-codes BETTER_AUTH_MANAGED_OBJECTS, whose comment claims it "mirrors the managedBy: 'better-auth' flag" — but nothing reads the schemas at runtime, and no test pins the list. This is exactly the hand-maintained-list drift ADR-0092 forbids, and it has already happened:

22 schemas declare managedBy: 'better-auth'; the list has 17. Missing: sys_scim_provider, sys_sso_provider, sys_oauth_resource, sys_oauth_client_resource, sys_oauth_client_assertion.

Consequence: the four write-granting default sets (organization_admin :150, member_default :298, viewer_readonly :461, MCP write set :581) wildcard-grant create/edit/delete on those 5 identity tables at the permission-evaluator layer. Not currently exploitable — plugin-auth's identity write guard (ADR-0092, identity-write-guard.ts) 403s the actual write at the engine — but the permission layer now disagrees with the schemas and only the last-line guard holds. That is the ADR-0049 anti-pattern (an unenforced security property) plus a defense-in-depth regression waiting for any guard refactor.

Two verified facts that dictate the design (do NOT skip these)

  1. A DB-row-only fix is dead code. PermissionEvaluator.resolvePermissionSets (permission-evaluator.ts:287-360) resolves the default set names from metadata.list('permission') and then the in-memory bootstrapPermissionSets fallback — the DB sys_permission_set.object_permissions JSON is never consulted for these names (resolve-authz-context.ts:258-286 reads only names / system / tab permissions). The transform must mutate the in-memory PermissionSet objects. Those objects are reference-shared across all consumers — manifest.register({ permissions }) (security-plugin.ts:372), the evaluator fallback, and bootstrapPlatformAdmin's row serialization (bootstrap-platform-admin.ts:104) hold the same instances (SchemaRegistry.registerItem stores by reference, objectql/src/registry.ts:976-1001) — so one in-place mutation updates every path atomically.
  2. sys_user is an intentional divergence — do NOT derive from resolveCrudAffordances. sys_user declares userActions: { edit: true } (opens name/image editing for the UI + the guard's field whitelist), yet the deny-list correctly denies allowEdit — permission-set booleans cannot express a field-level whitelist. The transform must hard-deny writes for the whole better-auth bucket ignoring userActions, byte-preserving today's behavior. An affordance-derived list would silently widen sys_user edit.

Implementation

Timing: kernel:ready — the registry is fully populated there (ordering dependency documented at security-plugin.ts:1531-1535; readDeclared(engine, 'object') in bootstrap-declared-permissions.ts:61-69_registry.listItems('object')getAllObjects() proves enumeration works at that point).

  • New module packages/plugins/plugin-security/src/managed-object-write-denies.ts (header style: mirror system-write-guard.ts; cite ADR-0092/0103 and the deferral rationale below). Exports:
    • MANAGED_DENY_ENTRY = { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false } — exact shape of today's entries (no viewAllRecords key).
    • MANAGED_DENY_TARGET_SETS = ['organization_admin', 'member_default', 'viewer_readonly', <MCP write set name>] — explicit allowlist. Never admin_full_access (deliberately zero per-object entries; the admin bypass is pinned at security-plugin.test.ts:966-1010), never the MCP read/restricted sets.
    • applyManagedWriteDenies(sets, schemas): collect names where schema.managedBy === 'better-auth' (sorted, deterministic); for each allowlisted set with an objects map: if (!(name in set.objects)) set.objects[name] = { ...MANAGED_DENY_ENTRY }. Never override an existing explicit entry (protects org-admin's RBAC read-only block at default-permission-sets.ts:152-156). In-place, idempotent. Return { applied, skippedExisting }.
  • default-permission-sets.ts: add the 5 missing names now (immediate fix, independent of the transform); export the constant; rewrite its comment to "compile-time baseline, unioned with the live registry at kernel:ready by applyManagedWriteDenies; pinned by test". Keep the baseline: it covers the pre-kernel:ready window and hook-less test-stub kernels (security-plugin.ts:1534-1535 falls back to immediate execution with a possibly-empty registry).
  • Wire in security-plugin.ts — in runBootstrap (~:1543), before bootstrapPlatformAdmin(...), behind a once-flag (pattern: envProjectionWired :1542; runBootstrap re-runs after the first sys_user insert):
    applyManagedWriteDenies(this.bootstrapPermissionSets, readDeclared(ql, 'object'));
    (readDeclared is already exported and imported here.) Log the applied count. The comment MUST state the reference-sharing contract from fact Add metamodel interfaces for ObjectQL/ObjectUI contract #1.
  • Resync / idempotency (document in the same comment): fresh DB → enriched objects serialize into the seed row; existing DB → insert-once leaves the row's JSON stale, harmless for enforcement (fact Add metamodel interfaces for ObjectQL/ObjectUI contract #1), and os meta resync reconciles platform rows deterministically. Objects registered by packages installed after boot are NOT covered — the ADR-0092/0103 engine guards remain the enforcement there (explicit non-goal).
  • Changeset: one patch for @objectstack/plugin-security only. Keep the helper internal (no index.ts export). No new ADR — mechanical follow-through of ADR-0092/0103.

Tests

  • New managed-object-write-denies.test.ts: hits exactly the four target sets; skips admin / MCP-read / restricted; never overrides an existing explicit entry (fixture mirroring org-admin's RBAC block); ignores platform / config / system / append-only buckets (pins the deferral); a better-auth schema with userActions.edit: true (the sys_user shape) still gets the hard deny; double-apply is a no-op.
  • New objects/default-permission-sets.test.ts (the drift pin): import the identity schemas from @objectstack/platform-objects (already a dependency, package.json:22); assert list ↔ schemas bidirectionally AND that each of the four sets carries a deny entry per listed name AND admin_full_access has zero per-object entries. (Written against today's 17-name list this test must FAIL — proof it bites.)
  • Extend bootstrap-platform-admin.test.ts (reuse its makeQl pattern): enriched entries land in the object_permissions JSON; resync: true reconciles an old row missing them.
  • One integration assertion in security-plugin.test.ts: after start + kernel:ready, resolvePermissionSets(['member_default'], …) yields sys_sso_provider with allowCreate: false — this pins the reference-sharing assumption, the one real implementation risk.

Verification: pnpm --filter @objectstack/plugin-security test and build. Optional dogfood: boot showcase, as a member GET /me/permissions → the 5 previously-missing objects report allowCreate: false.

Explicitly out of scope

  • No deny entries for engine-owned system / append-only objects. Per-object entries fully override the wildcard (lookup, not merge — default-permission-sets.ts:74-77), so injecting them would silently drop viewAllRecords / modifyAllRecords for e.g. sys_audit_log on organization_admin — a read-side narrowing this issue must not smuggle in. Enforcement is already complete via assertEngineOwnedWriteAllowed (security-plugin.ts:690-694) and the hono /me/permissions clamp. Note this in the module header.
  • The row-level managed_by provenance vocabulary (normalizeManagedByVocab, ADR-0066) — an unrelated axis sharing the word; do not touch.

Known / accepted side effects (call out in the PR)

  1. organization_admin loses wildcard viewAllRecords / modifyAllRecords on the 5 newly-denied objects — identical posture to the existing 17 (allowRead: true retained).
  2. Writes to the 5 tables under member_default now fail at the evaluator instead of the engine guard — same outcome, different error origin.
  3. Residual risk: if the registry ever switches to cloning stored items, the in-place mutation silently stops reaching the evaluator — that is exactly what the integration test above exists to catch.

Related

#3220 (root taxonomy split) · #3315 (implementation) · ADR-0092 (registry-driven guard, no hardcoded lists) · ADR-0103 (resolved-affordance write policy) · ADR-0049 (no unenforced security properties)

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions