fix(security): derive better-auth managed-object write denies from the registry (#3325)#3326
Merged
Merged
Conversation
…e registry (#3325) The default permission sets deny generic writes on better-auth identity tables via a hand-maintained BETTER_AUTH_MANAGED_OBJECTS list — the drift ADR-0092 forbids, and it had already drifted: 17 listed vs 22 schemas declaring managedBy:'better-auth', leaving sys_scim_provider, sys_sso_provider and three sys_oauth_* tables wildcard-granted for writes at the evaluator (the identity write guard still 403'd the write, so a defense-in-depth gap, not a live hole). - New applyManagedWriteDenies (managed-object-write-denies.ts) unions the denies from the live registry into the four write-granting default sets 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 entry, ignores userActions (better-auth is hard-denied: sys_user's userActions.edit opens only a field whitelist the identity guard enforces). - Static list completed to 22 as a compile-time baseline, now pinned bidirectionally against the @objectstack/platform-objects schemas by test so it cannot silently rot again. - Engine-owned system/append-only objects deliberately NOT denied here (a per-object entry overrides the wildcard and would drop viewAllRecords; their writes are already rejected by the ADR-0103 engine guard). Tests: helper behavior, drift pin, seed-row serialization + resync, and a reference-sharing integration pin. plugin-security 539 passed. Refs #3325, follows #3220 / ADR-0092 / ADR-0103 / ADR-0049. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fp9yZxRQ3mb7p4vVwqFXKE
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 11 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
3 tasks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
os-zhuang
marked this pull request as ready for review
July 20, 2026 01:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3325. Follow-through of #3220 / ADR-0103 (merged in #3315).
Problem
The default permission sets deny generic writes on better-auth identity tables via a hand-maintained
BETTER_AUTH_MANAGED_OBJECTSlist — the drift ADR-0092 forbids, and it had already drifted: 17 listed vs 22 schemas declaringmanagedBy: 'better-auth', leavingsys_scim_provider,sys_sso_provider,sys_oauth_resource,sys_oauth_client_resource,sys_oauth_client_assertionwildcard-granted for writes at the permission-evaluator layer. Not exploitable (plugin-auth's identity write guard still 403s the write), but a defense-in-depth gap + an ADR-0049 unenforced-property.Approach
Derive the deny-list from the live registry so the permission layer can't disagree with the schemas. Two facts shaped it (both pinned by tests):
bootstrapPermissionSets, not the seeded DB JSON. The transform mutates those sharedPermissionSetinstances in place atkernel:ready, before the seeder, so every consumer (evaluator,manifest.register, seed row) sees it atomically.sys_useris an intentional divergence — it declaresuserActions: { edit: true }(a field-level whitelist the identity guard enforces), so the deny must key off the rawbetter-authbucket and hard-deny writes, not derive fromresolveCrudAffordances(which would widensys_useredit).Changes
applyManagedWriteDenies(managed-object-write-denies.ts): injects a read-only-write deny for every registeredmanagedBy: 'better-auth'object into the four write-granting sets (organization_admin,member_default,viewer_readonly, MCP write). Never touchesadmin_full_access, never overrides an existing entry, idempotent.security-plugin.tsrunBootstrap(once-flag, beforebootstrapPlatformAdmin), reading the registry via the existingreadDeclared(ql, 'object').BETTER_AUTH_MANAGED_OBJECTScompleted to 22 and kept as a compile-time baseline (covers the pre-kernel:readywindow), now pinned bidirectionally against the@objectstack/platform-objectsschemas.Explicitly out of scope
Engine-owned
system/append-onlyobjects are not given deny entries — a per-object entry overrides the wildcard and would dropviewAllRecords; their writes are already rejected by the ADR-0103 engine guard (assertEngineOwnedWriteAllowed) and reflected in the/me/permissionsclamp. Documented in the module header.Verification
@objectstack/plugin-security— 539 passed (was 521; +18), build/DTS typecheck clean. New/extended tests: helper behavior, the drift pin (fails on the old 17-name list — proof it bites), seed-row serialization + resync, and a reference-sharing integration pin (a registry-only better-auth object lands on the shared default set).Accepted side effects (byte-preserving for the 17 already-listed tables)
organization_adminloses wildcardviewAllRecords/modifyAllRecordson the 5 newly-denied objects — identical posture to the existing 17 (allowReadretained).member_defaultnow fail at the evaluator instead of the engine guard — same outcome, different error origin.Related
#3220 · #3315 · ADR-0092 (registry-driven, no hardcoded lists) · ADR-0103 · ADR-0049
🤖 Generated with Claude Code
Generated by Claude Code