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

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions