fix(auth): canonicalise sys_member.role at the write, and converge existing rows

.changeset/member-role-canonical.md

@@ -0,0 +1,47 @@
+---
+"@objectstack/plugin-auth": patch
+---
+
+fix(plugin-auth): canonicalise `sys_member.role` at the write, so an org admin can no longer remove an owner (#8317)
+
+**Security — authorization inversion.** A membership stored with a non-canonical
+role — `Owner`, `' owner'`, `OWNER` — was an **owner** to every ObjectStack-side
+check and a **plain member** to better-auth.
+
+better-auth `1.7.0-rc.2` reads that column with a raw `role.split(",")`, with no
+`trim()` and no `toLowerCase()`, in three branches of
+`dist/plugins/organization/routes/crud-members.mjs`: `removeMember`'s "only an
+owner may remove an owner", `updateMemberRole`'s creator protection, and
+`organization/leave`'s last-owner count. ObjectStack's own readers all trim and
+lower-case (the #5942 grade ladder, `mapMembershipRole`). So on such a row the
+vendor never entered its owner branch at all and fell through to
+`hasPermission({ member: ['delete'] })` — **which an org admin passes**. An org
+admin could remove, demote, or count out an owner that every ObjectStack check
+treated as an owner.
+
+Not reachable through the ordinary invite/accept path (better-auth's own writes
+are canonical). Reachable through anything else that writes the column: an
+operator SQL fix-up, a data import, a SCIM group mapping, a script.
+
+**The fix normalises at the write**, so the disagreement is unrepresentable
+rather than adjudicated per reader:
+
+- ObjectQL `beforeInsert` / `beforeUpdate` hooks on `sys_member` canonicalise
+ `role` on every write path, in every context (system and better-auth adapter
+ writes included — those are the paths this exists for). They run at priority
+ 5, ahead of the ADR-0092 identity write guard and the ADR-0024 D5.2
+ break-glass guard, so both judge the value's normal form.
+- A **one-off convergent pass runs at boot** and canonicalises rows that already
+ exist. It is idempotent, safe to re-run, and logs a census of every distinct
+ non-canonical spelling it found with row counts.
+
+Canonicalisation is per token: a token that is a membership role (ADR-0108's
+closed vocabulary) is trimmed and lower-cased; any other token is preserved
+verbatim apart from trimming, because `mapMembershipRole` passes an unknown
+value through with its case and it becomes a position name a permission set may
+be bound to. A value carrying no known role at all is left completely untouched
+and only reported — it cannot produce the inversion.
+
+No API, schema or configuration change: `sys_member.role`'s option list is
+unchanged, and canonicalisation never moves a membership's grade, so no
+membership gains or loses authority as a result of this fix.

packages/plugins/plugin-auth/src/auth-plugin.ts

@@ -43,6 +43,7 @@ import {
  type SecondaryStorageLike,
 } from './identity-write-guard.js';
 import { registerLastAdminGuard } from './last-admin-guard.js';
+import { registerMemberRoleCanonicalization } from './member-role-canonical.js';
 import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js';
 import { MANAGED_EXTENSION_EDITABLE_FIELDS } from './managed-extension-fields.js';
 import { runSetInitialPassword } from './set-initial-password.js';
@@ -843,6 +844,26 @@ export class AuthPlugin implements Plugin {
  }
  });
 
+ // [#8317] The one-off half of the ruling: rows written BEFORE the
+ // canonicalisation hook above — or written outside ObjectQL entirely by an
+ // operator SQL fix-up, an import or a SCIM group remap — still carry a
+ // non-canonical `sys_member.role`, and each one is a live authorization
+ // inversion (owner to us, plain member to better-auth). The pass is
+ // convergent, idempotent and reports a census of every distinct spelling it
+ // found, so a database already canonical costs one query.
+ ctx.hook('kernel:ready', async () => {
+ try {
+ const ql = ctx.getService<IDataEngine>('objectql');
+ if (!ql) return;
+ const { canonicalizeStoredMemberRoles } = await import('./member-role-canonical.js');
+ await canonicalizeStoredMemberRoles(ql, { logger: ctx.logger });
+ } catch (e) {
+ ctx.logger.warn?.('[auth] sys_member.role canonicalisation pass failed', {
+ error: (e as Error).message,
+ });
+ }
+ });
+
  // ADR-0081 D1 — single-org default-organization bootstrap. Every WALLED
  // posture (`group` and `isolated`) keeps its existing owner: the enterprise
  // organizations package, which runs the same idempotent helper with the
@@ -1019,6 +1040,19 @@ export class AuthPlugin implements Plugin {
  if (object === SystemObjectName.USER) continue; // sys_user tiering above
  registerManagedUpdateWhitelist(object, fields);
  }
+ // [#8317] Canonicalise `sys_member.role` on every ObjectQL write, at
+ // priority 5 — AHEAD of both guards below. better-auth reads that
+ // column with a raw `split(',')` (no trim, no lower-case), so a stored
+ // `Owner` / `' owner'` is an owner to our grade ladder and a plain
+ // member to the vendor, and its "only an owner may remove an owner"
+ // branch never fires: an org admin could remove an owner. Normalising
+ // at the write makes that disagreement unrepresentable instead of
+ // adjudicated per-reader (maintainer ruling 2026-08-13, option A), and
+ // it runs first so both guards judge the value's normal form.
+ registerMemberRoleCanonicalization(engine, {
+ packageId: 'com.objectstack.plugin-auth.member-role-canonical',
+ logger: ctx.logger,
+ });
  registerIdentityWriteGuard(engine, {
  packageId: 'com.objectstack.plugin-auth.identity-write-guard',
  logger: ctx.logger,

packages/plugins/plugin-auth/src/index.ts

@@ -28,6 +28,13 @@ export * from './identity-write-guard.js';
 // has to be able to register the invariant itself rather than ship an
 // environment that can ban or delete its last administrator.
 export * from './last-admin-guard.js';
+// [#8317] `sys_member.role` canonicalisation — the write-path hooks and the
+// one-off convergent pass. Exported for the same reason the two guards above
+// are, plus one of its own: a host that upgrades outside this plugin's boot
+// path still has to converge its stored rows, because every non-canonical one
+// is an authorization inversion (an owner to ObjectStack, a plain member to
+// better-auth's raw `split(',')`).
+export * from './member-role-canonical.js';
 export * from './sys-user-writable-fields.js';
 export * from './otp-send-guard.js';
 // ADR-0069 D2 / #4772 — the cross-node rate-limit counter store (kernel cache,