Skip to content

managedBy is not enforced: generic CRUD bypasses better-auth on sys_team (data-integrity / security) #1591

Closed
#3213
Closed
managedBy is not enforced: generic CRUD bypasses better-auth on sys_team (data-integrity / security)#1591
#3213
Assignees
os-zhuang
Labels
bugSomething isn't working

Description

@xuyushun441-sys
xuyushun441-sys
opened on Jun 5, 2026

Summary

managedBy: 'better-auth' is documented as "generic CRUD is suppressed — use the action endpoints" but nothing enforces it. The generic data API will happily INSERT/UPDATE/DELETE against an externally-managed table, bypassing the owning subsystem's invariants, hooks, and authz path.

Evidence

packages/platform-objects/src/identity/sys-team.object.ts:

managedBy: 'better-auth',
// "Generic CRUD is suppressed (managedBy: 'better-auth'), so these are
//  the canonical entry points for create/update/delete."
...
enable: { apiMethods: ['get','list','create','update','delete'] }  // <- contradicts the comment
  • packages/objectql/src/engine.ts — no managedBy check in the insert/update/delete path (assertWriteAllowed() only gates external datasources).
  • packages/objectql/src/registry.ts:~189 — managedBy is only used to skip system-field injection (if (schema.managedBy === 'better-auth') return schema), not to block ops.
  • packages/rest/src/rest-server.ts create handler calls p.createData(...) directly; no metadata/apiMethods gate.

Result: a bare POST /api/v1/data/sys_team runs a generic INSERT. The canonical path is the create_team action → POST /api/v1/auth/organization/create-team, which lets better-auth own id/organizationId, enforce org-membership invariants, and apply its own authz.

Why this matters (not just an error-message bug)

  • Integrity: you can create a sys_team row better-auth doesn't know about, or one that violates its team↔org consistency / cascade-on-delete assumptions.
  • Security surface: the action routes through better-auth authz; the generic /data route uses ObjectQL RLS. These are not guaranteed equivalent.

Proposed change

Make managedBy (external owner) a real capability gate: for objects whose writes are externally managed, the generic data API should refuse create/update/delete with a clear 405/409 that names the canonical action (e.g. "use action create_team"), while leaving read (get/list) open (the UI needs it).

Also reconcile the contradictory metadata: either derive apiMethods from managedBy, or fail object registration when an externally-managed object advertises generic write verbs.

Open design points

  • Granularity: refuse all of create/update/delete? (My instinct: yes; reads always open.)
  • Generalize managedBy: 'better-auth' into a broader capability (externallyManaged / writeVia: '<action>') so third-party-managed objects get the same treatment, not just better-auth.
  • Enforcement layer: REST router (cheap, per-verb) vs. ObjectQL engine (defense-in-depth, covers non-REST callers). Engine-level is more robust.

Related

  • Provenance issue (system-field skip-by-name) — same managedBy/injection boundary (linked below).
  • Error-mapping safety net: branch fix/rest-map-schema-errors.

Found running cloud LOCAL-E2E-CHECKLIST B7.

Activity

  1. xuyushun441-sys
    added
    bugSomething isn't working
    on Jun 5, 2026

  2. xuyushun441-sys commented on Jun 5, 2026

    @xuyushun441-sys
    xuyushun441-sys
    on Jun 5, 2026
    CollaboratorAuthor

    Related set — three rungs of "the data API must honor object metadata before hitting the DB", surfaced together by LOCAL-E2E-CHECKLIST B7:

    Landed as the error-mapping safety net: branch fix/rest-map-schema-errors (maps the symptoms to 4xx; these issues are the causes).

  3. os-zhuang commented on Jun 15, 2026

    @os-zhuang
    os-zhuang
    on Jun 15, 2026
    Contributor

    Recommendation: fold into ADR-0049 / #1878 (metadata-liveness "enforce-or-remove") as an M2 security item — not a standalone fix.

    Why route rather than quick-fix:

    • This is literally an "enforce a parsed-but-unenforced property" item — the exact class [P0] Metadata property liveness audit: ~half of all spec properties are dead; a cluster of security props is parsed-but-unenforced #1878 / ADR-0049 owns, with active coordinated work. A one-off patch here would likely conflict with the holistic design (generalizing managedBy:'better-auth' → externallyManaged/writeVia, REST-layer vs engine-layer enforcement, the full set of managed tables).
    • Confirmed it is a live gap, not already mitigated: object-level there is no write restriction on sys_team etc. — assertWriteAllowed (engine.ts) only gates external datasources (sys_team is on default), and protection is ADR-0010 metadata protection, not row write-gating. (I did not trace the global auth/RLS layer, so residual exposure depends on that.)
    • Natural mount point when ADR-0049 picks it up: extend assertWriteAllowed with a managedBy/externallyManaged refusal (engine-level = defense-in-depth, covers non-REST callers), reads stay open. Blast-radius caveat to check first: confirm better-auth's adapter and the boot/seed path do NOT write these tables via generic ObjectQL insert, or the gate breaks auth/startup.

    Leaving open for the #1878 owner to fold/close. (At a startup stage with a small trusted user base, accepting the residual risk short-term is defensible — but track it, don't drop it.)

  4. os-zhuang commented on Jul 18, 2026

    @os-zhuang
    os-zhuang
    on Jul 18, 2026 – with Claude
    Contributor

    Re-review (2026-07-18): the core gap is CLOSED — landed via ADR-0092 D2, not #1878. What remains is metadata reconciliation + generalization. Development plan below.

    Status of the original report

    The headline vulnerability (generic /data CRUD bypassing better-auth on sys_team and every other identity table) is now enforced at the engine level:

    • Engine-level guard (defense-in-depth, covers non-REST callers) — packages/plugins/plugin-auth/src/identity-write-guard.ts registers beforeInsert/beforeUpdate/beforeDelete hooks that fail-closed reject USER-CONTEXT writes on every object whose registered schema declares managedBy: 'better-auth'. The flag is read from the schema registry at evaluation time — no hardcoded table list. Registered at kernel:ready in auth-plugin.ts.
    • Reads stay open (get/list untouched), exactly as proposed here. The only write opening is a per-object UPDATE whitelist (currently sys_user → {name, image}).
    • Blast-radius caveat from the previous comment is addressed by construction: better-auth's adapter calls carry no session context and plugin/system writes stamp isSystem — both bypass the guard, so boot/seed/adapter paths are unaffected.
    • Proven: unit suite identity-write-guard.test.ts + e2e dogfood single-tenant-identity-create.dogfood.test.ts (POST /data/sys_team → 403 PERMISSION_DENIED; system-context insert still succeeds).
    • Separately, apiEnabled/apiMethods are now真实 enforced at HTTP dispatch (packages/runtime/src/api-exposure.ts, ADR-0049 / [P0][security] Object apiEnabled/apiMethods not enforced by REST #1889 — 404/405).

    What remains open (from this issue's asks)

    1. Metadata contradiction not reconciled — sys_team.object.ts still advertises apiMethods: ['get','list','create','update','delete'] while its comment says generic CRUD is suppressed. Same for sys_member, sys_organization, sys_user, sys_account, sys_invitation, sys_api_key (others like sys_oauth_application/sys_sso_provider are already tightened to read-only).
    2. Generalization not done — the guard only recognizes managedBy === 'better-auth'; other buckets (e.g. managedBy: 'platform') are explicitly ignored (behavior pinned by a test). The externallyManaged/writeVia design point still belongs to [P0] Metadata property liveness audit: ~half of all spec properties are dead; a cluster of security props is parsed-but-unenforced #1878 / ADR-0049.
    3. Minor: the 403 message gives a generic "use the dedicated auth surface" hint rather than naming the object's canonical action (e.g. create_team).

    Development plan

    Phase 1 — tighten apiMethods on better-auth-managed objects (small, ships independently)

    • For each managedBy: 'better-auth' object in packages/platform-objects/src/identity/, remove the write verbs that the identity write guard rejects anyway; keep verbs a registered whitelist legitimately serves (sys_user keeps update for the profile-field whitelist).
    • Result: the HTTP layer answers with a semantically-correct 405 (via api-exposure.ts) before the engine's 403 backstop; the engine guard remains as defense-in-depth for non-REST callers.
    • Risk check: checkApiExposure is a no-op for system/internal contexts, and the better-auth adapter doesn't route through HTTP dispatch — boot/seed unaffected. Verify with the existing dogfood suites.
    • Acceptance: POST /api/v1/data/sys_team → 405; dogfood single-tenant-identity-create updated to assert the new status; UI affordances already clamped by the /me/permissions managed-write clamp (no UI regression expected).

    Phase 2 — registration-time consistency validation

    • Fail (or warn-then-derive) at object registration when a schema declares an external managedBy and advertises generic write verbs in apiMethods without a corresponding update whitelist — making the contradiction impossible to reintroduce.
    • Mount point: schema normalization in packages/objectql/src/registry.ts (where managedBy is already consulted).
    • Acceptance: a registry unit test registering a contradictory schema asserts the error/derivation.

    Phase 3 — generalize the guard (fold into #1878 / ADR-0049)

    Phase 4 (optional polish) — enrich the guard's rejection message to name the object's canonical action(s) from its actions metadata (e.g. "use action create_team").

    Proposal: land Phase 1+2 as one small PR referencing this issue, then close this issue; Phase 3 continues under #1878. Phase 4 is opportunistic.

    Generated by Claude Code

  5. os-zhuang
    self-assigned this
    on Jul 18, 2026
  6. os-zhuang
    added a commit that references this issue on Jul 18, 2026

    fix(identity): reconcile managedBy:'better-auth' apiMethods with the …

    865ccb9
  7. os-zhuang
    closed this as completedin #3213on Jul 18, 2026
  8. os-zhuang
    added a commit that references this issue on Jul 18, 2026

    fix(identity): close generic-write apiMethods hole on sys_presence & …

    06cb319
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions