From 098e0905abaa11e93240fae67d59dfe3e35867ea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:19:09 +0000 Subject: [PATCH 1/4] fix(auth): canonicalise sys_member.role at the write (#8317) better-auth reads sys_member.role with a raw split(',') -- no trim, no lower-case -- so a row stored as 'Owner' or ' owner' is an owner to the #5942 grade ladder and a plain member to the vendor. Its 'only an owner may remove an owner' branch therefore never fires and the request falls through to hasPermission({ member: ['delete'] }), which an org admin passes: an org admin could remove an owner. Maintainer ruling 2026-08-13, option A -- normalise at the write: beforeInsert/beforeUpdate hooks on sys_member plus a one-off convergent boot pass for existing rows. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --- .../plugins/plugin-auth/src/auth-plugin.ts | 34 ++ packages/plugins/plugin-auth/src/index.ts | 7 + .../plugin-auth/src/member-role-canonical.ts | 477 ++++++++++++++++++ 3 files changed, 518 insertions(+) create mode 100644 packages/plugins/plugin-auth/src/member-role-canonical.ts diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 4e49d73339..80de208846 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/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('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, diff --git a/packages/plugins/plugin-auth/src/index.ts b/packages/plugins/plugin-auth/src/index.ts index abccb1abb3..042bcf5753 100644 --- a/packages/plugins/plugin-auth/src/index.ts +++ b/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, diff --git a/packages/plugins/plugin-auth/src/member-role-canonical.ts b/packages/plugins/plugin-auth/src/member-role-canonical.ts new file mode 100644 index 0000000000..34aeaf1bc0 --- /dev/null +++ b/packages/plugins/plugin-auth/src/member-role-canonical.ts @@ -0,0 +1,477 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8317] `sys_member.role` has ONE spelling — canonical, lower-case, trimmed. + * + * ## The disagreement this abolishes + * + * Three readers answer "is this membership an owner", and until this module + * two of them could disagree about the same row: + * + * 1. the #5942 grade ladder (`orgRoleGrade` / `isOrgAdminGrade`, + * `invitation-role-cap.ts`) — `split(',')` then `trim().toLowerCase()`; + * 2. `mapMembershipRole` (`@objectstack/spec/identity`) — `trim().toLowerCase()`; + * 3. **better-auth itself** — `better-auth@1.7.0-rc.2`, + * `dist/plugins/organization/routes/crud-members.mjs`, a raw + * `role.split(",")` with NO trim and NO lower-casing, in three branches: + * `removeMember`'s "only an owner may remove an owner", `updateMemberRole`'s + * creator protection, and `organization/leave`'s last-owner count. + * + * For a row stored as `Owner` (or `' owner'`), (1) and (2) say owner and (3) + * says plain member. The vendor therefore skips its owner branch entirely and + * falls through to `hasPermission({ member: ['delete'] })`, **which an org + * admin passes** — so an admin could remove an owner, an authorization + * inversion, while every ObjectStack-side check treated that same row as an + * owner. + * + * ## The remedy, as ruled (maintainer, 2026-08-13 — option A) + * + * Normalise at the WRITE, not at the read. Reading through a normalising seam + * (option B) means intercepting every vendor read forever; documenting the + * invariant (option C) leaves declared-not-enforced standing. Canonicalising + * the stored value makes the disagreement **unrepresentable** instead of + * adjudicated per-reader, and it needs exactly two pieces: + * + * - {@link registerMemberRoleCanonicalization} — engine `beforeInsert` / + * `beforeUpdate` hooks on `sys_member`, so no ObjectQL write path can mint a + * divergent row again; + * - {@link canonicalizeStoredMemberRoles} — a one-off convergent pass over + * rows that already exist, run at boot, idempotent, safe to re-run. + * + * ⚠️ Deliberately NOT in scope: the #8289 remove-member envelope guard + * (`remove-member-permission-guard.ts`), which reproduces the vendor's + * predicate byte-for-byte **on purpose** — including the asymmetry where the + * target's roles are split without `trim()` and the caller's with it. After + * this module lands, that guard's refusal population and the vendor's agree by + * construction, which is the point of leaving it alone; "cleaning it up" would + * change who is refused under cover of a normalisation fix. + * + * ## What "canonical" means here, exactly — and why it is per TOKEN + * + * A `sys_member.role` value is a comma-separated list. Canonicalisation is + * applied to each token independently: + * + * - a token that IS a known membership role (ADR-0108's closed vocabulary, + * {@link BUILTIN_MEMBERSHIP_ROLES}) once trimmed and lower-cased becomes + * exactly that canonical spelling; + * - any other token is preserved VERBATIM apart from trimming; + * - tokens that are empty after trimming are dropped. + * + * The second clause is not timidity, it is a measured consumer fact. + * `resolve-authz-context.ts` projects every token through `mapMembershipRole`, + * whose `default:` arm returns `raw.trim()` — case preserved. So for a token + * outside the vocabulary the CASE is meaningful: it becomes a position name + * that a `sys_position_permission_set` row may be bound to. Lower-casing it + * would silently re-point that binding. Trimming it cannot: every consumer in + * the repo trims before it looks (`resolve-authz-context`, `mapMembershipRole`, + * `parseOrgRoles`), and the vendor only ever asks `includes(creatorRole)` with + * `creatorRole` defaulting to `'owner'` — which no foreign token can be. + * + * A value whose tokens are ALL foreign is therefore left completely untouched + * and merely reported: it carries no known role, so no reader can read it as an + * owner, so it cannot produce the inversion, so there is nothing to buy by + * rewriting it. + * + * ## The security invariant this buys, stated so it can be tested + * + * For any canonicalised value `v` and any known membership role `R`: + * + * ``` + * v.split(',').includes(R) // the vendor's raw read + * === + * parseOrgRoles(v).includes(R) // the #5942 ladder's read + * ``` + * + * That is the whole defect, closed: the two readers can no longer disagree + * about whether a row carries `owner`. `member-role-canonical.test.ts` pins it + * against predicates EXTRACTED FROM THE INSTALLED VENDOR FILE, so a vendor + * upgrade that moves the predicate reddens the pin instead of silently + * un-verifying it. + * + * ## The one hole that stays, named rather than hidden + * + * A write that never reaches ObjectQL — raw SQL against the database, a driver + * fixture loaded out of band — can still store a divergent row after boot. The + * hooks cannot see it and the boot pass has already run. It converges at the + * next restart. Closing that would mean a database-level constraint, which is + * a different (and larger) decision than the one ruled here. + */ + +import { BUILTIN_MEMBERSHIP_ROLES } from '@objectstack/spec/identity'; +import { SystemObjectName } from '@objectstack/spec/system'; + +/** The closed membership-role vocabulary (ADR-0108), indexed for lookup. */ +const KNOWN_ROLES: ReadonlySet = new Set(BUILTIN_MEMBERSHIP_ROLES); + +/** `sys_member` — the one object this module speaks about. */ +export const MEMBER_OBJECT = SystemObjectName.MEMBER; + +/** + * Is this token, once trimmed and lower-cased, one of the framework's + * membership roles? + */ +export function isKnownMembershipRole(token: string): boolean { + return KNOWN_ROLES.has(token.trim().toLowerCase()); +} + +/** + * Flatten a stored/incoming role value to its comma-separated string form. + * Mirrors `parseOrgRoles`'s tolerance for the array spelling; anything that is + * neither a string nor an array is not a role value at all. + */ +function flatten(raw: unknown): string | null { + if (Array.isArray(raw)) return raw.join(','); + return typeof raw === 'string' ? raw : null; +} + +/** + * The canonical form of a `sys_member.role` value, or `null` when there is + * nothing to canonicalise. + * + * `null` is returned for three genuinely different situations, all of which + * mean "leave the stored value exactly as it is": + * + * - the value is not a role value (not a string, not an array); + * - the value carries NO known membership role, so it cannot produce the + * inversion and its case may be load-bearing elsewhere (see the module doc); + * - the value is already canonical. + * + * Collapsing the three into one `null` is deliberate: every caller's next move + * is the same — do not write. The census in + * {@link canonicalizeStoredMemberRoles} is where they are told apart, because + * that is the one place a human reads them. + */ +export function canonicalMemberRole(raw: unknown): string | null { + const flat = flatten(raw); + if (flat === null) return null; + + const tokens = flat.split(','); + let carriesKnownRole = false; + const canonicalTokens: string[] = []; + for (const token of tokens) { + const trimmed = token.trim(); + if (trimmed.length === 0) continue; + const lowered = trimmed.toLowerCase(); + if (KNOWN_ROLES.has(lowered)) { + carriesKnownRole = true; + canonicalTokens.push(lowered); + } else { + // Foreign token: trimmed only, case preserved — it is a position name to + // `mapMembershipRole`, and lower-casing it would re-point a binding. + canonicalTokens.push(trimmed); + } + } + + // No known role anywhere in the value: no reader can grade it as an owner or + // an admin, so there is no disagreement to abolish and nothing to gain from + // touching it. + if (!carriesKnownRole) return null; + + const canonical = canonicalTokens.join(','); + return canonical === flat ? null : canonical; +} + +/** + * Is this value already in canonical form — i.e. would + * {@link canonicalMemberRole} leave it alone? + * + * ⚠️ `true` therefore also covers "not a role value" and "carries no known + * role". It answers "is there a rewrite to do here", not "is this a valid + * membership role" — that second question is the `sys_member.role` select's, + * and the record validator already answers it on every ObjectQL write. + */ +export function isCanonicalMemberRole(raw: unknown): boolean { + return canonicalMemberRole(raw) === null; +} + +// --------------------------------------------------------------------------- +// Write path — the hooks +// --------------------------------------------------------------------------- + +type LoggerLike = { + info?(msg: string, meta?: Record): void; + warn?(msg: string, meta?: Record): void; + error?(msg: string, meta?: Record): void; + debug?(msg: string, meta?: Record): void; +}; + +export interface MemberRoleCanonicalizationOptions { + packageId: string; + logger?: LoggerLike; +} + +/** + * Register the write-path canonicalisation on an ObjectQL engine. + * + * **Priority 5 — ahead of every other `sys_member` before-hook**, and that + * placement is the contract rather than a detail. The ADR-0092 identity write + * guard sits at 10 and the ADR-0024 D5.2 break-glass guard at 20; both JUDGE + * the payload, and a judgement should be made on the value's normal form, not + * on whichever spelling the caller happened to send. Canonicalisation decides + * nothing and performs no I/O, so running it first costs a string compare on + * writes that need no rewrite. + * + * Covers both dispatch shapes of each event by construction: `beforeInsert` + * fires per row for a batch insert, and `beforeUpdate`'s predicate path + * dispatches per matched row over THE shared payload (ADR-0058 D3), so a + * payload rewritten here binds the whole batch. + * + * ⚠️ It fires for EVERY context, `isSystem` included — better-auth's own + * adapter writes, SCIM group remaps and import scripts are precisely the write + * paths this exists for, and every one of them is a system context. + */ +export function registerMemberRoleCanonicalization( + engine: any, + opts: MemberRoleCanonicalizationOptions, +): void { + const { packageId, logger } = opts; + + const canonicalize = (event: 'insert' | 'update') => async (ctx: any): Promise => { + const data = ctx?.input?.data; + if (!data || typeof data !== 'object') return; + if (!Object.prototype.hasOwnProperty.call(data, 'role')) return; + const canonical = canonicalMemberRole((data as Record).role); + if (canonical === null) return; + const before = (data as Record).role; + (data as Record).role = canonical; + // Not a degradation — the write proceeds, and it proceeds with the value + // every reader agrees about. `debug` because it is per-write and a bulk + // import can produce a great many of them; the boot pass's summary is + // where an operator is told the population exists. + logger?.debug?.( + `[MemberRoleCanonical] normalised sys_member.role on ${event}: ` + + `${JSON.stringify(before)} -> ${JSON.stringify(canonical)} (#8317)`, + ); + }; + + engine.registerHook('beforeInsert', canonicalize('insert'), { + object: MEMBER_OBJECT, + priority: 5, + packageId, + }); + engine.registerHook('beforeUpdate', canonicalize('update'), { + object: MEMBER_OBJECT, + priority: 5, + packageId, + }); +} + +// --------------------------------------------------------------------------- +// The one-off migration +// --------------------------------------------------------------------------- + +/** One distinct stored spelling, as the census reports it. */ +export interface MemberRoleSpellingCensusEntry { + /** The spelling exactly as it is stored. */ + stored: string; + /** What it would become, or `null` when it is left alone. */ + canonical: string | null; + /** How many rows carry this spelling. */ + count: number; + /** Does it carry at least one role from the closed vocabulary? */ + carriesKnownRole: boolean; + /** + * Would the ladder and the vendor disagree about this spelling as stored? + * This is the inversion, counted rather than assumed. + */ + divergent: boolean; + /** Rows actually rewritten (0 when the spelling is left alone). */ + rewritten: number; +} + +export interface CanonicalizeStoredMemberRolesResult { + /** Rows examined. */ + scanned: number; + /** Rows whose stored spelling is not canonical. */ + nonCanonical: number; + /** Rows rewritten to the canonical spelling. */ + normalized: number; + /** + * Rows left alone although non-canonical — spellings carrying no known + * membership role, where the case may be load-bearing as a position name. + */ + declined: number; + /** Rows whose rewrite threw. */ + failed: number; + /** Every distinct non-canonical spelling found, with counts. */ + census: MemberRoleSpellingCensusEntry[]; +} + +export interface CanonicalizeStoredMemberRolesOptions { + logger?: LoggerLike; + /** Safety valve for very large tables; rows beyond it wait for the next boot. */ + limit?: number; +} + +const SYSTEM_CTX = { isSystem: true }; + +/** + * Does the vendor's raw read and the #5942 ladder's read disagree about any + * known role in this value? Reported per spelling so the boot log states the + * inversion it found rather than implying one from a rewrite count. + * + * The vendor's half is `split(',')` with no trim and no lower-case — the + * predicate quoted in the module doc, applied here to every known role rather + * than to `creatorRole` alone, because `admin` drives our own ladder. + */ +function spellingIsDivergent(stored: string): boolean { + const vendorTokens = stored.split(','); + const ladderTokens = stored + .split(',') + .map((t) => t.trim().toLowerCase()) + .filter((t) => t.length > 0); + for (const role of KNOWN_ROLES) { + if (vendorTokens.includes(role) !== ladderTokens.includes(role)) return true; + } + return false; +} + +/** + * The one-off convergent pass (maintainer ruling, 2026-08-13): rewrite every + * existing `sys_member.role` that is not canonical. + * + * **Reports before it rewrites.** The census names every distinct non-canonical + * spelling with its row count, so the operator sees exactly what the pass + * touched (and what it declined) instead of a bare number — the ruling asked + * for counts, not for a guess. + * + * Idempotent and convergent: a second run finds nothing, and a partial run + * leaves the rest for the next boot. It writes through `ql.update` under a + * system context, so it passes back through the ADR-0024 D5.2 break-glass guard + * — which is correct and deliberate: `Owner` and `owner` are the same grade to + * `isOrgAdminGrade`, so a canonicalisation can never be the write that revokes + * the last administrator's standing, and if it somehow were, being refused is + * the right outcome. + */ +export async function canonicalizeStoredMemberRoles( + ql: any, + options: CanonicalizeStoredMemberRolesOptions = {}, +): Promise { + const limit = options.limit ?? 5000; + const logger = options.logger; + const result: CanonicalizeStoredMemberRolesResult = { + scanned: 0, + nonCanonical: 0, + normalized: 0, + declined: 0, + failed: 0, + census: [], + }; + if (!ql || typeof ql.find !== 'function' || typeof ql.update !== 'function') return result; + + let rows: any[] = []; + try { + const found = await ql.find(MEMBER_OBJECT, { limit }, { context: SYSTEM_CTX }); + rows = Array.isArray(found) ? found : Array.isArray(found?.records) ? found.records : []; + } catch (e: any) { + // No membership table yet (fresh boot, mock mode) — nothing to converge. + logger?.debug?.('[MemberRoleCanonical] sys_member not readable — skipping the pass', { + error: e?.message ?? String(e), + }); + return result; + } + + result.scanned = rows.length; + + // Census first, rewrite second. The two loops are separate so the counts + // report the population as it was FOUND: a census accumulated while writing + // describes a table half-way through its own migration. + const census = new Map(); + const pending: Array<{ id: unknown; stored: string; canonical: string }> = []; + for (const row of rows) { + const stored = row?.role; + if (typeof stored !== 'string') continue; + const canonical = canonicalMemberRole(stored); + // The value is already canonical (or carries nothing known) AND unchanged: + // `canonicalMemberRole` folds both into `null`, so tell them apart here, + // where the census needs the distinction. + const carriesKnownRole = stored.split(',').some((t) => isKnownMembershipRole(t)); + const wouldChange = + canonical !== null || + // A spelling that carries no known role can still be non-canonical + // (stray whitespace) — it is simply left alone. Count it as found. + stored !== + stored + .split(',') + .map((t) => t.trim()) + .filter((t) => t.length > 0) + .join(','); + if (!wouldChange) continue; + + result.nonCanonical += 1; + const entry = census.get(stored) ?? { + stored, + canonical, + count: 0, + carriesKnownRole, + divergent: spellingIsDivergent(stored), + rewritten: 0, + }; + entry.count += 1; + census.set(stored, entry); + + if (canonical === null) { + result.declined += 1; + continue; + } + if (row?.id === undefined || row?.id === null) { + result.declined += 1; + continue; + } + pending.push({ id: row.id, stored, canonical }); + } + + result.census = [...census.values()]; + + for (const { id, stored, canonical } of pending) { + try { + await ql.update(MEMBER_OBJECT, { id, role: canonical }, { context: SYSTEM_CTX }); + result.normalized += 1; + const entry = census.get(stored); + if (entry) entry.rewritten += 1; + } catch (e: any) { + result.failed += 1; + logger?.warn?.('[MemberRoleCanonical] could not canonicalise a sys_member.role row', { + memberId: id, + stored, + canonical, + error: e?.message ?? String(e), + }); + } + } + + if (result.normalized > 0) { + logger?.info?.( + `[MemberRoleCanonical] canonicalised sys_member.role on ${result.normalized} row(s) of ` + + `${result.scanned} — better-auth reads this column with a raw split(','), so a ` + + `non-canonical spelling read as an owner to ObjectStack and as a plain member to the ` + + `vendor (#8317).`, + { spellings: result.census.filter((c) => c.rewritten > 0).map((c) => ({ stored: c.stored, canonical: c.canonical, rows: c.rewritten })) }, + ); + } + if (result.declined > 0) { + logger?.warn?.( + `[MemberRoleCanonical] left ${result.declined} non-canonical sys_member.role row(s) ` + + `untouched: they carry no role from the closed membership vocabulary, so their case is ` + + `a position name (mapMembershipRole passes unknown values through with their case) and ` + + `rewriting it could re-point a sys_position_permission_set binding. They cannot produce ` + + `the #8317 inversion; decide them by hand if they are not intended.`, + { spellings: result.census.filter((c) => c.canonical === null).map((c) => ({ stored: c.stored, rows: c.count })) }, + ); + } + if (result.failed > 0) { + // Durability-class: the boot looks healthy, the row stays divergent, and + // the divergence is an authorization inversion — an org admin can remove + // that owner. Consequence and fix, both, in the first line. + logger?.error?.( + `[MemberRoleCanonical] ${result.failed} sys_member.role row(s) could NOT be canonicalised. ` + + `Those memberships stay readable as an owner by ObjectStack and as a plain member by ` + + `better-auth, so an org admin can remove or demote them (#8317). Fix: correct the row ` + + `(lower-case and trim the role value) and restart, or re-run the boot pass — it is ` + + `idempotent and converges.`, + { spellings: result.census.filter((c) => c.rewritten < c.count && c.canonical !== null).map((c) => ({ stored: c.stored, rows: c.count - c.rewritten })) }, + ); + } + + return result; +} From 0cba233c61df536d0a1bced351a715b2d8ef43f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:28:18 +0000 Subject: [PATCH 2/4] test(auth): pin the #8317 inversion against better-auth's own extracted predicates Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --- .../src/member-role-canonical.test.ts | 519 ++++++++++++++++++ 1 file changed, 519 insertions(+) create mode 100644 packages/plugins/plugin-auth/src/member-role-canonical.test.ts diff --git a/packages/plugins/plugin-auth/src/member-role-canonical.test.ts b/packages/plugins/plugin-auth/src/member-role-canonical.test.ts new file mode 100644 index 0000000000..89743a95f6 --- /dev/null +++ b/packages/plugins/plugin-auth/src/member-role-canonical.test.ts @@ -0,0 +1,519 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8317] `sys_member.role` canonicalisation. + * + * ## Why the vendor's predicates are EXTRACTED rather than restated + * + * The defect is a disagreement between our grade ladder and better-auth's own + * reading of the same column. A test that restates the vendor's predicate in + * TypeScript proves the restatement agrees with the ladder — which is a fact + * about this file, not about the vendor. Both sides of the comparison would + * then derive from the same source, and the assertion could not fail for the + * reason it exists. + * + * So the three owner-tests are read out of the INSTALLED vendor file at test + * time, and the predicate is built from the bytes that were extracted. Two + * things follow, both wanted: + * + * - the expectation is the vendor's, so a pin that goes green means the + * vendor really does read the canonicalised row as an owner; + * - a vendor upgrade that moves, renames or corrects any of the three + * branches fails the extraction and reddens the suite, instead of silently + * leaving a pin that verifies nothing. + * + * The three branches (better-auth 1.7.0-rc.2, + * `dist/plugins/organization/routes/crud-members.mjs`): + * + * 1. `removeMember` — `const roles = toBeRemovedMember.role.split(",");` + * … `if (roles.includes(creatorRole))` + * 2. `updateMemberRole` — `const isUpdatingCreator = + * toBeUpdatedMember.role.split(",").includes(creatorRole);` + * 3. `organization/leave` — `if (member.role.split(",").includes(creatorRole))` + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; +import { assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { + canonicalMemberRole, + isCanonicalMemberRole, + isKnownMembershipRole, + registerMemberRoleCanonicalization, + canonicalizeStoredMemberRoles, +} from './member-role-canonical.js'; +import { orgRoleGrade, isOrgAdminGrade, parseOrgRoles } from './invitation-role-cap.js'; +import { + targetCarriesCreatorRole, + callerCarriesCreatorRole, +} from './remove-member-permission-guard.js'; +import { BUILTIN_MEMBERSHIP_ROLES } from '@objectstack/spec/identity'; + +// --------------------------------------------------------------------------- +// The vendor's three owner-tests, extracted from the installed package +// --------------------------------------------------------------------------- + +const require_ = createRequire(import.meta.url); +/** `…/better-auth/dist/index.mjs` → `…/better-auth/dist`. */ +const VENDOR_DIST = dirname(require_.resolve('better-auth')); +const CRUD_MEMBERS = join(VENDOR_DIST, 'plugins', 'organization', 'routes', 'crud-members.mjs'); +const VENDOR_SOURCE = readFileSync(CRUD_MEMBERS, 'utf8'); + +interface VendorBranch { + /** The route whose owner-test this is. */ + branch: string; + /** + * Matches the branch's owner-test in the vendor source. Capture groups + * concatenate, after a leading `role`, into a JS expression over one role + * value — so the predicate under test is assembled from vendor BYTES. + */ + pattern: RegExp; +} + +const VENDOR_BRANCHES: readonly VendorBranch[] = [ + { + branch: 'removeMember — only an owner may remove an owner', + // `const roles = toBeRemovedMember.role.split(",");` … `if (roles.includes(creatorRole)) {` + pattern: + /const roles = toBeRemovedMember\.role(\.split\(","\));[\s\S]{0,200}?if \(roles(\.includes\(creatorRole\))\)/, + }, + { + branch: 'updateMemberRole — creator protection', + pattern: + /const isUpdatingCreator = toBeUpdatedMember\.role(\.split\(","\)\.includes\(creatorRole\));()/, + }, + { + branch: 'organization/leave — last-owner count', + pattern: /\n\tif \(member\.role(\.split\(","\)\.includes\(creatorRole\))\) \{()/, + }, +]; + +/** + * Build the branch's predicate out of the vendor's own source text. + * + * Throws when the branch cannot be found or is not unique — which is the drift + * tripwire: on a vendor upgrade this fails loudly rather than testing a + * predicate the vendor no longer runs. + */ +function vendorOwnerTest(branch: VendorBranch): (role: unknown, creatorRole: string) => boolean { + const match = VENDOR_SOURCE.match(branch.pattern); + if (!match) { + throw new Error( + `[#8317] could not find the '${branch.branch}' owner-test in ${CRUD_MEMBERS}. ` + + `better-auth changed it — re-read the route and update the pattern (and re-decide ` + + `whether the canonicalisation still closes the disagreement).`, + ); + } + const occurrences = [ + ...VENDOR_SOURCE.matchAll(new RegExp(branch.pattern.source, 'g')), + ].length; + if (occurrences !== 1) { + throw new Error( + `[#8317] the '${branch.branch}' pattern matched ${occurrences} times in ${CRUD_MEMBERS}; ` + + `it must identify exactly one branch.`, + ); + } + const expression = `role${match[1] ?? ''}${match[2] ?? ''}`; + // eslint-disable-next-line no-new-func + return new Function('role', 'creatorRole', `return ${expression};`) as ( + role: unknown, + creatorRole: string, + ) => boolean; +} + +const CREATOR_ROLE = 'owner'; + +describe('#8317 — the vendor predicates this card is about', () => { + it('extracts all three owner-tests from the installed better-auth', () => { + for (const branch of VENDOR_BRANCHES) { + const test = vendorOwnerTest(branch); + // Sanity: the extracted predicate behaves like an owner-test on the + // canonical value. If this fails the extraction grabbed the wrong text. + expect(test('owner', CREATOR_ROLE), branch.branch).toBe(true); + expect(test('member', CREATOR_ROLE), branch.branch).toBe(false); + } + }); + + it('still reads the column raw — no trim, no lower-case (the defect itself)', () => { + for (const branch of VENDOR_BRANCHES) { + const source = VENDOR_SOURCE.match(branch.pattern)?.[0] ?? ''; + expect(source, branch.branch).toContain('split(",")'); + expect(source.toLowerCase(), branch.branch).not.toContain('tolowercase'); + } + }); +}); + +/** The spellings that produce the inversion, and the canonical one. */ +const NON_CANONICAL = ['Owner', ' owner', 'OWNER', 'owner ', ' Owner '] as const; + +describe('#8317 — the inversion, reproduced against the real vendor predicates', () => { + it.each([...NON_CANONICAL])( + 'stored %o: our ladder says owner, all three vendor branches say plain member', + (stored) => { + // ObjectStack's side — the #5942 grade ladder. + expect(orgRoleGrade(stored)).toBe(3); // GRADE_OWNER + expect(isOrgAdminGrade(stored)).toBe(true); + + // better-auth's side, from the vendor's own bytes. + for (const branch of VENDOR_BRANCHES) { + expect(vendorOwnerTest(branch)(stored, CREATOR_ROLE), branch.branch).toBe(false); + } + }, + ); + + it.each([...NON_CANONICAL])( + 'after canonicalisation, stored %o reads as an owner to BOTH sides, on all three branches', + (stored) => { + const canonical = canonicalMemberRole(stored); + expect(canonical).toBe('owner'); + + expect(orgRoleGrade(canonical)).toBe(3); + expect(isOrgAdminGrade(canonical)).toBe(true); + for (const branch of VENDOR_BRANCHES) { + expect(vendorOwnerTest(branch)(canonical, CREATOR_ROLE), branch.branch).toBe(true); + } + }, + ); + + it('closes the disagreement for every known role, not just `owner`', () => { + const spellings = [ + 'Owner', + ' owner ', + 'ADMIN', + ' Admin', + 'Delegated_Admin', + 'MEMBER', + 'Owner,Admin', + 'owner , admin', + 'admin,', + ]; + for (const stored of spellings) { + const canonical = canonicalMemberRole(stored) ?? stored; + for (const role of BUILTIN_MEMBERSHIP_ROLES) { + // The invariant the module doc states: for a canonical value the + // vendor's raw `split(',')` and the ladder's parse agree about every + // known role. + expect( + canonical.split(',').includes(role), + `${JSON.stringify(canonical)} / ${role}`, + ).toBe(parseOrgRoles(canonical).includes(role)); + } + } + }); +}); + +describe('canonicalMemberRole', () => { + it('lower-cases and trims each known-role token', () => { + expect(canonicalMemberRole('Owner')).toBe('owner'); + expect(canonicalMemberRole(' owner')).toBe('owner'); + expect(canonicalMemberRole('OWNER , Admin')).toBe('owner,admin'); + expect(canonicalMemberRole('Delegated_Admin')).toBe('delegated_admin'); + }); + + it('drops tokens that are empty after trimming', () => { + expect(canonicalMemberRole('owner,')).toBe('owner'); + expect(canonicalMemberRole('owner, ,admin')).toBe('owner,admin'); + }); + + it('accepts the array spelling `parseOrgRoles` tolerates', () => { + expect(canonicalMemberRole(['Owner', ' Admin'])).toBe('owner,admin'); + }); + + it('returns null for a value that is already canonical', () => { + expect(canonicalMemberRole('owner')).toBeNull(); + expect(canonicalMemberRole('owner,admin')).toBeNull(); + expect(isCanonicalMemberRole('owner,admin')).toBe(true); + }); + + it('returns null for a value that is not a role value at all', () => { + expect(canonicalMemberRole(null)).toBeNull(); + expect(canonicalMemberRole(undefined)).toBeNull(); + expect(canonicalMemberRole(42)).toBeNull(); + expect(canonicalMemberRole({})).toBeNull(); + }); + + it('leaves a value carrying NO known role completely alone, case included', () => { + // `mapMembershipRole`'s default arm returns `raw.trim()` — case preserved — + // so an unknown token is a POSITION NAME a permission set may be bound to. + // Lower-casing it would re-point that binding, and no reader can read it + // as an owner, so there is no disagreement to abolish either. + expect(canonicalMemberRole('Sales_Manager')).toBeNull(); + expect(canonicalMemberRole(' ACME-Rep ')).toBeNull(); + expect(isKnownMembershipRole('Sales_Manager')).toBe(false); + }); + + it('canonicalises the KNOWN token of a mixed value and preserves the foreign one', () => { + // The hole a value-level "all tokens known" rule would leave: this row IS + // an owner to the ladder and is not to the vendor. + expect(vendorOwnerTest(VENDOR_BRANCHES[0])('Owner,Sales_Manager', CREATOR_ROLE)).toBe(false); + expect(isOrgAdminGrade('Owner,Sales_Manager')).toBe(true); + + expect(canonicalMemberRole('Owner,Sales_Manager')).toBe('owner,Sales_Manager'); + for (const branch of VENDOR_BRANCHES) { + expect(vendorOwnerTest(branch)('owner,Sales_Manager', CREATOR_ROLE), branch.branch).toBe(true); + } + }); +}); + +// --------------------------------------------------------------------------- +// The write path +// --------------------------------------------------------------------------- + +type Handler = (ctx: any) => Promise; + +/** Fake engine capturing hook registrations (the identity-write-guard shape). */ +function makeHookEngine() { + const handlers: Record> = {}; + return { + handlers, + registerHook: (event: string, handler: Handler, options: any) => { + (handlers[event] ??= []).push({ handler, options }); + }, + }; +} + +function hookOn(engine: ReturnType, event: string) { + const entry = engine.handlers[event]?.find((h) => + h.options?.packageId?.includes('member-role-canonical'), + ); + if (!entry) throw new Error(`no canonicalisation handler registered for ${event}`); + return entry; +} + +describe('#8317 — write-path canonicalisation hooks', () => { + let engine: ReturnType; + beforeEach(() => { + engine = makeHookEngine(); + registerMemberRoleCanonicalization(engine, { packageId: 'test.member-role-canonical' }); + }); + + it('registers on sys_member only, at priority 5 — ahead of both guards', () => { + for (const event of ['beforeInsert', 'beforeUpdate']) { + const { options } = hookOn(engine, event); + expect(options.object).toBe('sys_member'); + // ADR-0092 identity write guard is 10; ADR-0024 D5.2 break-glass is 20. + expect(options.priority).toBe(5); + expect(options.priority).toBeLessThan(10); + } + }); + + it.each(['beforeInsert', 'beforeUpdate'])( + '%s rewrites a non-canonical payload in place', + async (event) => { + for (const stored of NON_CANONICAL) { + const data: Record = { user_id: 'usr_1', role: stored }; + await hookOn(engine, event).handler({ object: 'sys_member', input: { data } }); + expect(data.role).toBe('owner'); + } + }, + ); + + it('fires for EVERY context — system writes are the ones this exists for', async () => { + for (const session of [undefined, { userId: 'usr_1' }, { userId: 'usr_1', isSystem: true }]) { + const data: Record = { role: ' Owner ' }; + await hookOn(engine, 'beforeInsert').handler({ object: 'sys_member', input: { data }, session }); + expect(data.role).toBe('owner'); + } + }); + + it('leaves a canonical payload byte-identical', async () => { + const data: Record = { role: 'owner,admin' }; + await hookOn(engine, 'beforeUpdate').handler({ object: 'sys_member', input: { data } }); + expect(data.role).toBe('owner,admin'); + }); + + it('does not invent a role on a payload that carries none', async () => { + const data: Record = { user_id: 'usr_1' }; + await hookOn(engine, 'beforeUpdate').handler({ object: 'sys_member', input: { data } }); + expect(Object.prototype.hasOwnProperty.call(data, 'role')).toBe(false); + }); + + it('tolerates the shapes a hook context can legitimately arrive in', async () => { + const handler = hookOn(engine, 'beforeUpdate').handler; + await expect(handler({ object: 'sys_member', input: {} })).resolves.toBeUndefined(); + await expect(handler({ object: 'sys_member' })).resolves.toBeUndefined(); + await expect(handler({})).resolves.toBeUndefined(); + }); + + it('rewrites the SHARED payload a predicate update dispatches per row (ADR-0058 D3)', async () => { + // The batch payload is THE payload, not a copy — one rewrite binds every + // matched row, which is what makes a bulk `multi: true` role write safe. + const data: Record = { role: 'Owner' }; + const batchInput = { data, options: { multi: true } }; + for (const id of ['mem_1', 'mem_2', 'mem_3']) { + await hookOn(engine, 'beforeUpdate').handler({ + object: 'sys_member', + input: { id, data: batchInput.data, options: batchInput.options }, + }); + } + expect(data.role).toBe('owner'); + }); +}); + +// --------------------------------------------------------------------------- +// The one-off pass +// --------------------------------------------------------------------------- + +/** + * Memory engine for the migration. `update` is pinned to ObjectQL's own + * dispatch predicate (#4550/#5480): a fake looser than the real engine turns a + * green suite into no suite at all on exactly the write this pass performs. + */ +function makeMemoryEngine(rows: Array>) { + const calls: Array<{ object: string; patch: any; options: any }> = []; + return { + rows, + calls, + async find(object: string, q: any = {}, _opts?: any) { + if (object !== 'sys_member') return []; + const out = rows.map((r) => ({ ...r })); + return q?.limit ? out.slice(0, q.limit) : out; + }, + async update(object: string, patch: any, options?: any) { + assertEngineUpdateDispatch(patch, options); + calls.push({ object, patch, options }); + const row = rows.find((r) => r.id === patch.id); + if (!row) return null; + Object.assign(row, patch); + return { ...row }; + }, + }; +} + +describe('#8317 — the one-off convergent pass', () => { + it('canonicalises the divergent rows and reports a census of what it found', async () => { + const engine = makeMemoryEngine([ + { id: 'mem_1', user_id: 'u1', role: 'Owner' }, + { id: 'mem_2', user_id: 'u2', role: ' owner' }, + { id: 'mem_3', user_id: 'u3', role: 'Owner' }, + { id: 'mem_4', user_id: 'u4', role: 'owner' }, // already canonical + { id: 'mem_5', user_id: 'u5', role: 'admin' }, // already canonical + { id: 'mem_6', user_id: 'u6', role: ' ACME-Rep ' }, // no known role + ]); + + const result = await canonicalizeStoredMemberRoles(engine); + + expect(result.scanned).toBe(6); + expect(result.nonCanonical).toBe(4); // three owner spellings + the foreign one + expect(result.normalized).toBe(3); + expect(result.declined).toBe(1); + expect(result.failed).toBe(0); + + expect(engine.rows.map((r) => r.role)).toEqual([ + 'owner', + 'owner', + 'owner', + 'owner', + 'admin', + ' ACME-Rep ', // untouched, case AND whitespace preserved + ]); + + const census = Object.fromEntries(result.census.map((c) => [c.stored, c])); + expect(census['Owner']).toMatchObject({ count: 2, rewritten: 2, canonical: 'owner', divergent: true }); + expect(census[' owner']).toMatchObject({ count: 1, rewritten: 1, canonical: 'owner', divergent: true }); + expect(census[' ACME-Rep ']).toMatchObject({ + count: 1, + rewritten: 0, + canonical: null, + carriesKnownRole: false, + // No known role in the value, so both readers agree it is not an owner — + // this row was never part of the inversion. + divergent: false, + }); + }); + + it('is idempotent — a second pass finds nothing and writes nothing', async () => { + const engine = makeMemoryEngine([{ id: 'mem_1', user_id: 'u1', role: 'Owner' }]); + + const first = await canonicalizeStoredMemberRoles(engine); + expect(first.normalized).toBe(1); + + const second = await canonicalizeStoredMemberRoles(engine); + expect(second.nonCanonical).toBe(0); + expect(second.normalized).toBe(0); + expect(second.census).toEqual([]); + expect(engine.calls).toHaveLength(1); // no second write + }); + + it('writes by SCALAR id under a system context (the engine dispatch it claims)', async () => { + const engine = makeMemoryEngine([{ id: 'mem_1', user_id: 'u1', role: 'Owner' }]); + await canonicalizeStoredMemberRoles(engine); + expect(engine.calls).toHaveLength(1); + expect(engine.calls[0]).toMatchObject({ + object: 'sys_member', + patch: { id: 'mem_1', role: 'owner' }, + options: { context: { isSystem: true } }, + }); + }); + + it('counts a row whose rewrite throws instead of reporting it converged', async () => { + const engine = makeMemoryEngine([ + { id: 'mem_1', user_id: 'u1', role: 'Owner' }, + { id: 'mem_2', user_id: 'u2', role: 'Admin' }, + ]); + const realUpdate = engine.update.bind(engine); + engine.update = async (object: string, patch: any, options?: any) => { + if (patch.id === 'mem_2') throw new Error('refused by a guard'); + return realUpdate(object, patch, options); + }; + + const result = await canonicalizeStoredMemberRoles(engine); + expect(result.normalized).toBe(1); + expect(result.failed).toBe(1); + expect(engine.rows[1].role).toBe('Admin'); // still divergent, and said so + }); + + it('degrades to a no-op when there is no engine or no membership table', async () => { + await expect(canonicalizeStoredMemberRoles(undefined)).resolves.toMatchObject({ scanned: 0 }); + await expect(canonicalizeStoredMemberRoles({})).resolves.toMatchObject({ scanned: 0 }); + const throwing = { + async find() { + throw new Error('no such table: sys_member'); + }, + async update() { + throw new Error('unreachable'); + }, + }; + await expect(canonicalizeStoredMemberRoles(throwing)).resolves.toMatchObject({ + scanned: 0, + normalized: 0, + }); + }); + + it('a canonicalisation never changes a row GRADE, so it can never revoke standing', async () => { + // Why the pass may safely write through the ADR-0024 D5.2 break-glass guard + // under a system context: the guard counts administrators with + // `isOrgAdminGrade`, which already trims and lower-cases. + for (const stored of [...NON_CANONICAL, 'ADMIN', ' Delegated_Admin ', 'MEMBER']) { + const canonical = canonicalMemberRole(stored); + expect(canonical, stored).not.toBeNull(); + expect(orgRoleGrade(canonical), stored).toBe(orgRoleGrade(stored)); + expect(isOrgAdminGrade(canonical), stored).toBe(isOrgAdminGrade(stored)); + } + }); +}); + +// --------------------------------------------------------------------------- +// #8289 stays exactly where it is +// --------------------------------------------------------------------------- + +describe('#8317 keeps #8289 decoupled', () => { + it('leaves the remove-member guard reproducing the vendor asymmetry byte-for-byte', () => { + // #8289's guard splits the TARGET's roles without `trim()` and the + // CALLER's with it, on purpose, so its refusal set is the vendor's. This + // card does not correct it: after canonicalisation the two populations + // agree by construction, which is the point. If someone "cleans it up", + // this pin says so. + expect(targetCarriesCreatorRole(' owner', 'owner')).toBe(false); + expect(callerCarriesCreatorRole(' owner', 'owner')).toBe(true); + // …and the guard's target half still matches the vendor's, value for value. + const vendorRemoveMember = vendorOwnerTest(VENDOR_BRANCHES[0]); + for (const stored of ['owner', 'Owner', ' owner', 'owner,admin', 'member']) { + expect(targetCarriesCreatorRole(stored, 'owner'), stored).toBe( + vendorRemoveMember(stored, 'owner'), + ); + } + }); +}); From cbc2582164824e0be16f3fdc542baab2924389a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:55:22 +0000 Subject: [PATCH 3/4] fix(auth): match the kernel Logger's error() arity; add the changeset Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --- .changeset/member-role-canonical.md | 47 +++++++++++++++++++ .../src/member-role-canonical.test.ts | 1 - .../plugin-auth/src/member-role-canonical.ts | 15 ++++-- 3 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 .changeset/member-role-canonical.md diff --git a/.changeset/member-role-canonical.md b/.changeset/member-role-canonical.md new file mode 100644 index 0000000000..84de5c6aac --- /dev/null +++ b/.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. diff --git a/packages/plugins/plugin-auth/src/member-role-canonical.test.ts b/packages/plugins/plugin-auth/src/member-role-canonical.test.ts index 89743a95f6..9a6eba1e1c 100644 --- a/packages/plugins/plugin-auth/src/member-role-canonical.test.ts +++ b/packages/plugins/plugin-auth/src/member-role-canonical.test.ts @@ -116,7 +116,6 @@ function vendorOwnerTest(branch: VendorBranch): (role: unknown, creatorRole: str ); } const expression = `role${match[1] ?? ''}${match[2] ?? ''}`; - // eslint-disable-next-line no-new-func return new Function('role', 'creatorRole', `return ${expression};`) as ( role: unknown, creatorRole: string, diff --git a/packages/plugins/plugin-auth/src/member-role-canonical.ts b/packages/plugins/plugin-auth/src/member-role-canonical.ts index 34aeaf1bc0..4e5067b2f1 100644 --- a/packages/plugins/plugin-auth/src/member-role-canonical.ts +++ b/packages/plugins/plugin-auth/src/member-role-canonical.ts @@ -188,11 +188,17 @@ export function isCanonicalMemberRole(raw: unknown): boolean { // Write path — the hooks // --------------------------------------------------------------------------- +/** + * The kernel `Logger` surface this module uses, structurally — including + * `error`'s three-parameter shape (`message, error?, meta?`), which is what the + * kernel's own contract declares. Spelling it any other way makes `ctx.logger` + * unassignable at the call site. + */ type LoggerLike = { - info?(msg: string, meta?: Record): void; - warn?(msg: string, meta?: Record): void; - error?(msg: string, meta?: Record): void; - debug?(msg: string, meta?: Record): void; + info?(msg: string, meta?: Record): void; + warn?(msg: string, meta?: Record): void; + error?(msg: string, error?: Error, meta?: Record): void; + debug?(msg: string, meta?: Record): void; }; export interface MemberRoleCanonicalizationOptions { @@ -469,6 +475,7 @@ export async function canonicalizeStoredMemberRoles( `better-auth, so an org admin can remove or demote them (#8317). Fix: correct the row ` + `(lower-case and trim the role value) and restart, or re-run the boot pass — it is ` + `idempotent and converges.`, + undefined, { spellings: result.census.filter((c) => c.rewritten < c.count && c.canonical !== null).map((c) => ({ stored: c.stored, rows: c.count - c.rewritten })) }, ); } From d7e8c1a03e54acf52c4c54bea4b48860862c3ab4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 13:30:37 +0000 Subject: [PATCH 4/4] test(auth): drop import.meta from the vendor-source pin (TS1470 in this CJS-typed package) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plugin-auth publishes CommonJS, so under module: NodeNext any import.meta is a TS1470 — which drifted the package's frozen TEST_DEBT ledger entry 111 -> 112. Fixed the type rather than the ledger: reuse the findUp-from-CWD idiom rate-limit-storage-isolation.test.ts already established here, and seed createRequire from the package root so the better-auth file read is the one THIS package is pinned to. Re-measure is back at 111. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --- .../src/member-role-canonical.test.ts | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-auth/src/member-role-canonical.test.ts b/packages/plugins/plugin-auth/src/member-role-canonical.test.ts index 9a6eba1e1c..62893b9d72 100644 --- a/packages/plugins/plugin-auth/src/member-role-canonical.test.ts +++ b/packages/plugins/plugin-auth/src/member-role-canonical.test.ts @@ -33,7 +33,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; -import { readFileSync } from 'node:fs'; +import { readFileSync, existsSync } from 'node:fs'; import { createRequire } from 'node:module'; import { dirname, join } from 'node:path'; import { assertEngineUpdateDispatch } from '@objectstack/objectql'; @@ -55,7 +55,33 @@ import { BUILTIN_MEMBERSHIP_ROLES } from '@objectstack/spec/identity'; // The vendor's three owner-tests, extracted from the installed package // --------------------------------------------------------------------------- -const require_ = createRequire(import.meta.url); +/** + * Locate this package by walking up from the CWD — the idiom + * `rate-limit-storage-isolation.test.ts` established here and states the reason + * for: plugin-auth is CJS-typed (no `"type": "module"`, it publishes + * `dist/index.js` as CommonJS), so under `module: NodeNext` `import.meta` is a + * TS1470 in this package however well it runs under vitest. + */ +function findUp(predicate: (dir: string) => boolean, what: string): string { + let dir = process.cwd(); + for (;;) { + if (predicate(dir)) return dir; + const parent = dirname(dir); + if (parent === dir) throw new Error(`could not locate ${what}`); + dir = parent; + } +} + +const PKG = findUp((dir) => { + const manifest = join(dir, 'package.json'); + if (!existsSync(manifest)) return false; + const { name } = JSON.parse(readFileSync(manifest, 'utf8')) as { name?: string }; + return name === '@objectstack/plugin-auth'; +}, 'the @objectstack/plugin-auth package root'); + +// Resolved from THIS package, so the file read is the better-auth this package +// is pinned to — not whatever a hoist happens to put at the repo root. +const require_ = createRequire(join(PKG, 'probe.js')); /** `…/better-auth/dist/index.mjs` → `…/better-auth/dist`. */ const VENDOR_DIST = dirname(require_.resolve('better-auth')); const CRUD_MEMBERS = join(VENDOR_DIST, 'plugins', 'organization', 'routes', 'crud-members.mjs');