From 37e97b5be420e8587518c8567cbe07b344d2a8a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 08:49:45 +0000 Subject: [PATCH 1/4] fix(plugin-auth): phone_number is better-auth's, and widen D7's plugin derivation WIP checkpoint --- .../src/managed-extension-fields.test.ts | 408 +++++++++++++++--- .../src/managed-extension-fields.ts | 23 +- 2 files changed, 382 insertions(+), 49 deletions(-) diff --git a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts index bc781140e4..5a7edfc50f 100644 --- a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts +++ b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts @@ -40,13 +40,39 @@ * better-auth object fails as stale, so the exemption list cannot rot into * documentation of nothing. * - * There is a SECOND axis of the same blindness that this file does not close: - * the `getAuthTables()` call below loads `organization` only, while the auth - * manager loads many more plugins behind feature flags, so a column an unloaded - * plugin owns is invisible to the comparison even for a MAPPED object. - * `sys_user.phone_number` is a live instance — filed as #7820, deliberately not - * fixed here, because widening the plugin set turns this guard red on an - * ownership question nobody has decided. + * ## The second axis: the plugins the derivation loads (#7820) + * + * The absence above was about OBJECTS the map skips. The same blindness has a + * second axis — COLUMNS the derivation skips because the plugin that owns them + * was never loaded. `getAuthTables()` used to be called here with `organization` + * alone while the auth manager assembles fourteen plugin factories, so for a + * fully MAPPED object like `sys_user` the guard was still comparing against a + * fraction of better-auth's real surface and answering green about the rest. + * + * It is now called with the auth manager's whole set, adopting the reason + * `better-auth-schema-parity.test.ts` already records for doing the same: + * + * > Plugins that are feature-flagged off in some deployments are still + * > included: the column has to exist before the flag can be turned on. + * + * A flag is a deployment choice, not a schema fact. `phoneNumber` is opt-in and + * owns `sys_user.phone_number` in every deployment that turns it on, so a guard + * that only looks at the default set is not answering the ownership question at + * all. Widening it produced exactly one live collision — `sys_user.phone_number`, + * declared as an ObjectStack extension field while `auth-schema-config.ts` had + * shipped the `phoneNumber → phone_number` mapping since #2766. The maintainer + * ruled the column better-auth's on 2026-08-12 and it left + * `MANAGED_EXTENSION_FIELDS` in the same change. + * + * Two things keep the widening from silently rotting back, because "green after + * the removal" alone would prove only that today's case passes: + * + * - the auth-manager DRIFT TRIPWIRE below scans `auth-manager.ts` for the + * plugin factories it imports and fails when one of them is not accounted + * for here, so a plugin added there cannot quietly stay outside the guard; + * - `findCollisions()` is a pure function, exercised against a SYNTHETIC + * registry that declares a column only a widened plugin contributes — so the + * red direction is pinned in-repo, not merely asserted in a PR description. */ import { describe, it, expect } from 'vitest'; @@ -54,7 +80,20 @@ import { readFileSync, readdirSync, statSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { getAuthTables } from 'better-auth/db'; -import { organization } from 'better-auth/plugins'; +import { jwt } from 'better-auth/plugins'; +import { admin } from 'better-auth/plugins/admin'; +import { bearer } from 'better-auth/plugins/bearer'; +import { customSession } from 'better-auth/plugins/custom-session'; +import { deviceAuthorization } from 'better-auth/plugins/device-authorization'; +import { genericOAuth } from 'better-auth/plugins/generic-oauth'; +import { haveIBeenPwned } from 'better-auth/plugins/haveibeenpwned'; +import { magicLink } from 'better-auth/plugins/magic-link'; +import { organization } from 'better-auth/plugins/organization'; +import { phoneNumber } from 'better-auth/plugins/phone-number'; +import { twoFactor } from 'better-auth/plugins/two-factor'; +import { oauthProvider } from '@better-auth/oauth-provider'; +import { scim } from '@better-auth/scim'; +import { sso } from '@better-auth/sso'; import { MANAGED_EXTENSION_FIELDS, @@ -118,41 +157,61 @@ const UNMAPPED_MANAGED_OBJECTS: Record = { noBetterAuthColumns: true, }, - // ── Opt-in core plugins this file's getAuthTables() call does not load ──── - // It loads `organization` only, which is the surface D7 has covered since - // #3624. Column-level parity for these three IS covered — by - // `better-auth-schema-parity.test.ts`, which loads their plugins. What is - // missing here is only the D7 collision direction, and it costs nothing - // while no extension field is declared on them. The moment one is, the - // "nothing is silently skipped" assertion below turns red and the mapping - // has to be done properly. + // ── Opt-in core plugins: derived, but deliberately not mapped ───────────── + // #7770 registered these three because this file's getAuthTables() call did + // not load their plugins. #7820 widened that call to the auth manager's whole + // set, so the models ARE derived now — the reasons below are restated to say + // so rather than keep claiming an absence that stopped being true. What did + // NOT change is the disposition: mapping them is a coverage EXPANSION #7770 + // deferred and the 2026-08-12 ruling kept unmoved, so `MODEL_TO_OBJECT` still + // does not name them and `betterAuthFieldsByObject()` still drops them. + // + // The safety note is the load-bearing half and is untouched: this costs + // nothing while no extension field is declared on them, and the moment one + // is, the "nothing is silently skipped" assertion below turns red and the + // mapping has to be done properly. Column-level parity for all three is + // covered meanwhile by `better-auth-schema-parity.test.ts`. sys_two_factor: { reason: - "Owned by better-auth's opt-in `twoFactor` plugin (model `twoFactor`), which this file's " - + 'getAuthTables() call does not load. No extension field is declared on it.', + "Owned by better-auth's opt-in `twoFactor` plugin (model `twoFactor`). The plugin is in this " + + "file's getAuthTables() call since #7820, but MODEL_TO_OBJECT deliberately does not map the " + + 'object — mapping it is a coverage expansion, not part of that fix. No extension field is ' + + 'declared on it.', }, sys_device_code: { reason: - "Owned by better-auth's opt-in `deviceAuthorization` plugin (model `deviceCode`), which this " - + "file's getAuthTables() call does not load. No extension field is declared on it.", + "Owned by better-auth's opt-in `deviceAuthorization` plugin (model `deviceCode`). The plugin " + + "is in this file's getAuthTables() call since #7820, but MODEL_TO_OBJECT deliberately does " + + 'not map the object — mapping it is a coverage expansion, not part of that fix. No extension ' + + 'field is declared on it.', }, sys_jwks: { reason: - "Owned by better-auth's opt-in `jwt` plugin (model `jwks`), which this file's getAuthTables() " - + 'call does not load. No extension field is declared on it.', + "Owned by better-auth's opt-in `jwt` plugin (model `jwks`). The plugin is in this file's " + + 'getAuthTables() call since #7820, but MODEL_TO_OBJECT deliberately does not map the object ' + + '— mapping it is a coverage expansion, not part of that fix. No extension field is declared ' + + 'on it.', }, - // ── Plugins getAuthTables() structurally cannot see (#3653) ─────────────── + // ── Plugins getAuthTables() cannot ADDRESS as an ObjectStack object (#3653) ─ + // Both plugins are now in the call (#7820), so their models do appear in the + // derived tables — under better-auth's own names (`ssoProvider`, + // `scimProvider`, …). What they accept no `schema` option for is the mapping: + // there is no way to tell getAuthTables() that `ssoProvider` materializes as + // `sys_sso_provider`, so MODEL_TO_OBJECT cannot key off anything the library + // reports and the derivation has nothing to compare against the object. sys_sso_provider: { reason: - '@better-auth/sso accepts no `schema` option, so getAuthTables() cannot see its models at all ' - + '(#3653). Its columns are bridged mechanically by objectql-adapter.ts and gated by the ' - + 'dedicated sso/scim block in better-auth-schema-parity.test.ts.', + '@better-auth/sso accepts no `schema` option, so getAuthTables() reports its models only under ' + + "better-auth's own names and they cannot be mapped onto this object (#3653). Its columns are " + + 'bridged mechanically by objectql-adapter.ts and gated by the dedicated sso/scim block in ' + + 'better-auth-schema-parity.test.ts.', }, sys_scim_provider: { reason: - '@better-auth/scim accepts no `schema` option, so getAuthTables() cannot see its models at all ' - + '(#3653). Same bridge and same dedicated gate as sys_sso_provider.', + '@better-auth/scim accepts no `schema` option, so getAuthTables() reports its models only under ' + + "better-auth's own names and they cannot be mapped onto this object (#3653). Same bridge and " + + 'same dedicated gate as sys_sso_provider.', }, // ── @better-auth/oauth-provider — separate package, dedicated gate ──────── @@ -253,19 +312,97 @@ function toSnakeCase(name: string): string { return name.replace(/([a-z0-9])([A-Z])/g, '$1_$2').toLowerCase(); } +/** + * Every better-auth plugin factory `auth-manager.ts` can assemble, keyed by the + * imported name, with how this file accounts for it. + * + * `loaded: true` means the factory is in {@link betterAuthPluginSet} below and + * its columns are therefore part of the derived surface. Anything else needs a + * reason, and "it is off by default" is not one — see the file header. + * + * Every entry is currently loaded. The map exists so a future exclusion has to + * be written down rather than simply omitted, and so the tripwire below can + * reconcile it against `auth-manager.ts` in both directions. + */ +const AUTH_MANAGER_PLUGINS: Record = { + bearer: { loaded: true }, + organization: { loaded: true }, + twoFactor: { loaded: true }, + haveIBeenPwned: { loaded: true }, + admin: { loaded: true }, + phoneNumber: { loaded: true }, + magicLink: { loaded: true }, + genericOAuth: { loaded: true }, + jwt: { loaded: true }, + oauthProvider: { loaded: true }, + sso: { loaded: true }, + scim: { loaded: true }, + deviceAuthorization: { loaded: true }, + customSession: { loaded: true }, +}; + +/** + * The plugin set the auth manager actually assembles (`buildPluginList()`), + * constructed here WITHOUT the `schema` overrides — see below for why that + * omission is load-bearing rather than laziness. + * + * Feature-flagged-off plugins are included on the reason + * `better-auth-schema-parity.test.ts` states for its own derivation: the column + * has to exist before the flag can be turned on, so a deployment-time flag + * cannot decide who owns a column. Several of these contribute no model surface + * at all today (`bearer`, `haveIBeenPwned`, `magicLink` — which reuses + * `verification` — `genericOAuth`, `customSession`); they are constructed + * anyway so the set is "what the auth manager loads" rather than "what someone + * judged relevant", and so a version bump that gives one of them a user column + * is seen here on the day it lands. + * + * ⛔ Do NOT add the `schema:` options from `auth-schema-config.ts` to make this + * match the parity gate's call. The two gates want different things from the + * same function: parity resolves the exact COLUMN better-auth writes, so it + * needs our `fields` mappings; D7 asks whose column it is, and our schema + * config carries `additionalFields` — the ADR-0105 D8 `businessUnitId` / + * `positions` on `invitation` are ObjectStack's, declared through better-auth's + * own extension seam. Feeding those in makes the derived "better-auth surface" + * include our extension fields and the guard reports us colliding with + * ourselves. Pinned by the `additionalFields` assertion below. The snake_case + * half the mappings would provide is already covered: `toSnakeCase` records + * both spellings. + */ +function betterAuthPluginSet(): unknown[] { + return [ + bearer(), + // `teams: { enabled: true }` mirrors the auth-manager default. Without it + // better-auth omits the team models entirely, so the sys_team / + // sys_team_member entries below would be absent and any extension field + // added to those objects would collide silently. (#3624) + organization({ teams: { enabled: true } }), + twoFactor(), + haveIBeenPwned(), + admin(), + // The callbacks are required by the constructors and never invoked: this + // file only reads the schema each plugin declares. + phoneNumber({ sendOTP: async () => {} }), + magicLink({ sendMagicLink: async () => {} }), + genericOAuth({ config: [] }), + jwt(), + deviceAuthorization(), + customSession(async ({ user, session }: { user: unknown; session: unknown }) => ({ + user, + session, + })), + sso(), + scim(), + oauthProvider({ loginPage: '/login' }), + ]; +} + /** * Every field better-auth owns, per ObjectStack object name, in BOTH spellings * — comparing only one would let `parentOrganizationId` slip past a check on * `parent_organization_id`. */ function betterAuthFieldsByObject(): Record> { - // `teams: { enabled: true }` mirrors the auth-manager default. Without it - // better-auth omits the team models entirely, so the sys_team / - // sys_team_member entries below would be absent and any extension field - // added to those objects would collide silently. (#3624) - const tables = getAuthTables({ - plugins: [organization({ teams: { enabled: true } })], - } as never); + const tables = getAuthTables({ plugins: betterAuthPluginSet() } as never); const out: Record> = {}; for (const [model, table] of Object.entries(tables ?? {})) { const object = MODEL_TO_OBJECT[model]; @@ -282,6 +419,59 @@ function betterAuthFieldsByObject(): Record> { return out; } +/** + * The collision rule itself, as a pure function of the two surfaces. + * + * Extracted so the RED direction can be pinned with a synthetic registry + * instead of only being observed once by whoever last ablated the file. The + * real assertion calls it with `MANAGED_EXTENSION_FIELDS`; the synthetic-overlap + * test calls it with a registry declaring a column that only a WIDENED plugin + * contributes, which is the future case #7820 exists to make catchable. + */ +function findCollisions( + registry: Readonly>>, + byObject: Record>, +): string[] { + const collisions: string[] = []; + for (const [object, fields] of Object.entries(registry)) { + const owned = byObject[object]; + // Accounted for by the "nothing is silently skipped" assertion — the loop + // never passes on an object it simply could not derive. + if (!owned) continue; + for (const field of fields) { + if (owned.has(field)) collisions.push(`${object}.${field}`); + } + } + return collisions; +} + +/** + * The better-auth plugin factories `auth-manager.ts` imports, scanned from its + * source. + * + * Scanned rather than imported for the same reason `declaredBetterAuthObjects()` + * scans: the auth manager builds its list behind feature flags inside an async + * method, so there is no value to import that names the SET. Reading the file is + * the only way to ask "which plugins can this process load" without booting one + * — and this file must not edit `auth-manager.ts` to make it exportable, which + * would put the answer under the control of the thing being audited. + * + * `@better-auth/core/*` is excluded: those are runtime utilities + * (`runWithRequestState`, `isPublicRoutableHost`), not plugin factories, and + * they declare no schema. + */ +function authManagerPluginFactories(): string[] { + const source = readFileSync(join(HERE, 'auth-manager.ts'), 'utf8'); + const found = new Set(); + const pattern = + /const\s*\{\s*([A-Za-z0-9_]+)\s*\}\s*=\s*await import\('((?:better-auth\/plugins|@better-auth\/)[^']*)'\)/g; + for (const [, name, specifier] of source.matchAll(pattern)) { + if (specifier.startsWith('@better-auth/core')) continue; + found.add(name); + } + return [...found].sort(); +} + describe('managed extension fields (ADR-0105 D7)', () => { const byObject = betterAuthFieldsByObject(); const managedObjects = declaredBetterAuthObjects(); @@ -331,10 +521,9 @@ describe('managed extension fields (ADR-0105 D7)', () => { `these objects declare managedBy: 'better-auth' but are absent from MODEL_TO_OBJECT, so this ` + `guard skips them entirely and any extension field declared on them gets ZERO collision ` + `coverage while reading as covered: ${unaccounted.join(', ')}. Pick one deliberately:\n` - + ` (a) MAP IT — and it takes both halves, in this order: 1. pass the plugin that owns the ` - + `model into the getAuthTables() call in betterAuthFieldsByObject(), because a map entry ` - + `alone derives no table and the object stays silently skipped; 2. add the ` - + `model: 'object_name' entry to MODEL_TO_OBJECT.\n` + + ` (a) MAP IT — and it takes both halves, in this order: 1. make sure the plugin that owns ` + + `the model is in betterAuthPluginSet(), because a map entry alone derives no table and the ` + + `object stays silently skipped; 2. add the model: 'object_name' entry to MODEL_TO_OBJECT.\n` + ` (b) REGISTER IT in UNMAPPED_MANAGED_OBJECTS with the reason better-auth owns no ` + `derivable surface for it here.\n` + ` Do NOT complete MANAGED_EXTENSION_FIELDS just to make the comparison look meaningful: ` @@ -382,15 +571,7 @@ describe('managed extension fields (ADR-0105 D7)', () => { }); it('no declared extension field collides with better-auth\'s own schema', () => { - const collisions: string[] = []; - for (const [object, fields] of Object.entries(MANAGED_EXTENSION_FIELDS)) { - const owned = byObject[object]; - // Accounted for by the assertion directly above — never a silent pass. - if (!owned) continue; - for (const field of fields) { - if (owned.has(field)) collisions.push(`${object}.${field}`); - } - } + const collisions = findCollisions(MANAGED_EXTENSION_FIELDS, byObject); expect( collisions, `these extension fields collide with better-auth's own schema at the pinned version: ` + @@ -400,6 +581,95 @@ describe('managed extension fields (ADR-0105 D7)', () => { ).toEqual([]); }); + it('derives the columns the WIDENED plugin set contributes (#7820)', () => { + // The half of the #7820 ruling that "green on today's removal" cannot + // prove. Every name here is contributed by a plugin the derivation did NOT + // load before this change, and every one is written as a LITERAL — not read + // back out of the plugin list — so narrowing the list again cannot shrink + // this expectation along with the surface it is meant to police. (That + // co-moving shape is exactly how a sibling coverage pin stayed green while + // a mapping was deleted underneath it.) + const expected: Array<[string, string, string]> = [ + ['sys_user', 'two_factor_enabled', 'twoFactor'], + ['sys_user', 'role', 'admin'], + ['sys_user', 'banned', 'admin'], + ['sys_user', 'ban_reason', 'admin'], + ['sys_user', 'ban_expires', 'admin'], + ['sys_user', 'phone_number', 'phoneNumber'], + ['sys_user', 'phone_number_verified', 'phoneNumber'], + ['sys_session', 'impersonated_by', 'admin'], + ]; + const missing = expected + .filter(([object, column]) => !byObject[object]?.has(column)) + .map(([object, column, plugin]) => `${object}.${column} (${plugin} plugin)`); + expect( + missing, + `the derived better-auth surface no longer contains columns the auth manager's plugin set ` + + `owns: ${missing.join(', ')}. The plugin list in betterAuthPluginSet() was narrowed, so ` + + `this guard is back to answering green about columns it is not looking at — which is the ` + + `#7820 defect, not a passing test. Restore the plugin, or, if better-auth genuinely moved ` + + `the column, update this list and say where it went.`, + ).toEqual([]); + }); + + it('derives better-auth\'s OWN surface, not our additionalFields (#7820)', () => { + // The tempting "improvement" this blocks: passing the `schema:` options + // from auth-schema-config.ts into betterAuthPluginSet() to match the parity + // gate's call. Those carry ADR-0105 D8's `additionalFields`, which are + // OURS, declared through better-auth's extension seam — derived that way, + // the guard reports `sys_invitation.business_unit_id` and `.positions` as + // better-auth-owned and demands we rename our own columns. + for (const field of ['business_unit_id', 'positions']) { + expect( + byObject.sys_invitation?.has(field), + `${field} is an ObjectStack extension field (ADR-0105 D8 placement intent) that appears in ` + + `getAuthTables() output only when our own schema overrides are passed in. It is in the ` + + `derived better-auth surface, so the derivation is no longer describing better-auth — ` + + `drop the schema: options from betterAuthPluginSet().`, + ).toBe(false); + } + // Same claim from the other side: the mapped-through fields better-auth + // really does own on that table are still derived. + expect(byObject.sys_invitation?.has('inviter_id')).toBe(true); + }); + + it('every plugin auth-manager.ts can load is accounted for here (#7820)', () => { + // The tripwire that keeps the widening wide. Without it the plugin list + // above is a snapshot of one afternoon's auth manager, and the next plugin + // added there re-opens the exact blind spot #7820 closed — silently, + // because a plugin nobody loaded owns columns nobody compared. + const imported = authManagerPluginFactories(); + const declared = Object.keys(AUTH_MANAGER_PLUGINS).sort(); + + const unaccounted = imported.filter((name) => AUTH_MANAGER_PLUGINS[name] === undefined); + expect( + unaccounted, + `auth-manager.ts imports better-auth plugin factories this guard does not account for: ` + + `${unaccounted.join(', ')}. A plugin the auth manager can assemble owns columns on the ` + + `tables this guard compares, so leaving it out means the derived surface is narrower than ` + + `the one a booted environment gets. Add it to betterAuthPluginSet() and to ` + + `AUTH_MANAGER_PLUGINS as { loaded: true } — or, if it genuinely declares no schema this ` + + `call can read, register it with loaded: false and the reason. "It is off by default" is ` + + `NOT a reason: the column has to exist before the flag can be turned on.`, + ).toEqual([]); + + const stale = declared.filter((name) => !imported.includes(name)); + expect( + stale, + `AUTH_MANAGER_PLUGINS names plugins auth-manager.ts no longer imports: ${stale.join(', ')}. ` + + `Either the plugin was dropped (remove it here and from betterAuthPluginSet()) or the ` + + `import shape changed and this scan is now blind — check authManagerPluginFactories() ` + + `against the real import sites before deleting anything.`, + ).toEqual([]); + + // Nothing may sit in the map unloaded without a stated reason. + for (const [name, entry] of Object.entries(AUTH_MANAGER_PLUGINS)) { + if (!entry.loaded) { + expect(entry.reason.length, `${name} is not loaded but carries no reason`).toBeGreaterThan(20); + } + } + }); + it('editable extension fields are a SUBSET of declared extension fields', () => { // Listing a field as ours must not be what makes it editable — the two // tiers are separate decisions (ADR-0092 D1). @@ -432,6 +702,48 @@ describe('managed extension fields (ADR-0105 D7)', () => { }); }); +describe('the collision rule goes red on a FUTURE overlap (#7820)', () => { + // The ruling that widened the derivation states its post-condition in two + // directions: green on the `sys_user.phone_number` removal, and genuinely red + // on any new overlap. The first is the suite above. This is the second, and + // it is a permanent pin rather than a one-off ablation someone ran once — + // a widening that only makes today's case pass has implemented half of it. + const byObject = betterAuthFieldsByObject(); + + it('reports a column contributed by a WIDENED plugin as a collision', () => { + // `two_factor_enabled` reaches the surface only through `twoFactor()`, + // which the pre-#7820 derivation did not load — so under the old plugin set + // this exact registry produced NO collision. That is the regression being + // pinned, in the direction that matters. + expect( + findCollisions({ sys_user: new Set(['two_factor_enabled']) }, byObject), + ).toEqual(['sys_user.two_factor_enabled']); + }); + + it('reports the camelCase spelling of the same column', () => { + // better-auth authors `twoFactorEnabled`; a registry that happened to spell + // it that way must not slip past a snake_case-only comparison. + expect( + findCollisions({ sys_user: new Set(['twoFactorEnabled']) }, byObject), + ).toEqual(['sys_user.twoFactorEnabled']); + }); + + it('reports `phone_number` if it is ever re-declared as an extension field', () => { + // The specific entry #7820 removed. Re-adding it must not be a quiet edit: + // better-auth owns the column via `AUTH_PHONE_NUMBER_USER_FIELDS`, and the + // real registry assertion above would go red — this states why. + expect( + findCollisions({ sys_user: new Set(['phone_number']) }, byObject), + ).toEqual(['sys_user.phone_number']); + }); + + it('stays silent on a genuine extension field, so the pin is not vacuous', () => { + // A rule that reported everything would satisfy the three tests above while + // making the real assertion useless. + expect(findCollisions({ sys_user: new Set(['manager_id']) }, byObject)).toEqual([]); + }); +}); + describe('sys_api_key exemption premise (#7770)', () => { it('better-auth still ships no apiKey plugin, so sys_api_key has no model to collide with', async () => { // This is the whole warrant for `noBetterAuthColumns: true` on diff --git a/packages/plugins/plugin-auth/src/managed-extension-fields.ts b/packages/plugins/plugin-auth/src/managed-extension-fields.ts index bdd4f99a5a..1d236a1506 100644 --- a/packages/plugins/plugin-auth/src/managed-extension-fields.ts +++ b/packages/plugins/plugin-auth/src/managed-extension-fields.ts @@ -19,6 +19,13 @@ * fields. better-auth never reads or writes these, so they can be edited * through the ordinary path under normal FLS / `requiredPermissions`. * + * A field is an extension field because **better-auth does not write it**, not + * because ObjectStack finds it useful. `auth-schema-config.ts` is where that is + * decided: a column named in one of its `AUTH_*` mappings is a column + * better-auth addresses, so it belongs to the protocol population no matter how + * platform-flavoured its name looks (#7820 removed `sys_user.phone_number` on + * exactly that evidence). + * * ADR-0092 made the *guard* registry-driven with a per-object update whitelist; * `sys_user → {name, image}` was its first and only entry. This module is the * home for the rest, so "which fields on a managed table are ours, and which of @@ -34,6 +41,11 @@ * version and fails the build on any overlap, so the upgrade that would cause * this is the moment someone finds out. * + * That derivation loads the plugin set the auth manager actually assembles — + * feature-flagged-off plugins included, because the column has to exist before + * the flag can be turned on. A narrower set answers green about columns it is + * not looking at (#7820). + * * D7 catches only the OVERLAP — a field on both sides. A better-auth upgrade * that adds a field we have nothing named like sails straight through it and * fails at runtime instead (`team.memberCount`, #3624). That complementary @@ -47,7 +59,16 @@ export const MANAGED_EXTENSION_FIELDS: Readonly Date: Wed, 12 Aug 2026 08:51:05 +0000 Subject: [PATCH 2/4] test(plugin-auth): derive the D7 plugin set from the accounting map itself --- .../src/managed-extension-fields.test.ts | 135 ++++++++---------- 1 file changed, 63 insertions(+), 72 deletions(-) diff --git a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts index 5a7edfc50f..e6995e6315 100644 --- a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts +++ b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts @@ -314,47 +314,24 @@ function toSnakeCase(name: string): string { /** * Every better-auth plugin factory `auth-manager.ts` can assemble, keyed by the - * imported name, with how this file accounts for it. + * name it imports, with how this guard accounts for it. * - * `loaded: true` means the factory is in {@link betterAuthPluginSet} below and - * its columns are therefore part of the derived surface. Anything else needs a - * reason, and "it is off by default" is not one — see the file header. + * The plugin SET is derived from this map rather than written beside it, which + * is what makes the accounting real: an entry carrying a `construct` IS in the + * derived surface, because that thunk is what builds it. There is no way to + * declare a plugin covered and not load it — the failure a hand-kept pair of + * lists invites. `skip` is the other disposition and costs a written reason; + * "it is off by default" is not one, see the file header. * - * Every entry is currently loaded. The map exists so a future exclusion has to - * be written down rather than simply omitted, and so the tripwire below can - * reconcile it against `auth-manager.ts` in both directions. - */ -const AUTH_MANAGER_PLUGINS: Record = { - bearer: { loaded: true }, - organization: { loaded: true }, - twoFactor: { loaded: true }, - haveIBeenPwned: { loaded: true }, - admin: { loaded: true }, - phoneNumber: { loaded: true }, - magicLink: { loaded: true }, - genericOAuth: { loaded: true }, - jwt: { loaded: true }, - oauthProvider: { loaded: true }, - sso: { loaded: true }, - scim: { loaded: true }, - deviceAuthorization: { loaded: true }, - customSession: { loaded: true }, -}; - -/** - * The plugin set the auth manager actually assembles (`buildPluginList()`), - * constructed here WITHOUT the `schema` overrides — see below for why that - * omission is load-bearing rather than laziness. - * - * Feature-flagged-off plugins are included on the reason + * Feature-flagged-off plugins are all constructed on the reason * `better-auth-schema-parity.test.ts` states for its own derivation: the column * has to exist before the flag can be turned on, so a deployment-time flag - * cannot decide who owns a column. Several of these contribute no model surface - * at all today (`bearer`, `haveIBeenPwned`, `magicLink` — which reuses - * `verification` — `genericOAuth`, `customSession`); they are constructed - * anyway so the set is "what the auth manager loads" rather than "what someone - * judged relevant", and so a version bump that gives one of them a user column - * is seen here on the day it lands. + * cannot decide who owns a column. Several contribute no model surface at all + * today (`bearer`, `haveIBeenPwned`, `magicLink` — which reuses `verification` + * — `genericOAuth`, `customSession`); they are constructed anyway so the set is + * "what the auth manager loads" rather than "what someone judged relevant", and + * so a version bump that gives one of them a user column is seen here the day + * it lands. * * ⛔ Do NOT add the `schema:` options from `auth-schema-config.ts` to make this * match the parity gate's call. The two gates want different things from the @@ -368,32 +345,40 @@ const AUTH_MANAGER_PLUGINS: Record unknown } | { skip: string }> = { + bearer: { construct: () => bearer() }, + // `teams: { enabled: true }` mirrors the auth-manager default. Without it + // better-auth omits the team models entirely, so the sys_team / + // sys_team_member entries below would be absent and any extension field added + // to those objects would collide silently. (#3624) + organization: { construct: () => organization({ teams: { enabled: true } }) }, + twoFactor: { construct: () => twoFactor() }, + haveIBeenPwned: { construct: () => haveIBeenPwned() }, + admin: { construct: () => admin() }, + // The callbacks below are required by their constructors and never invoked: + // this file only reads the schema each plugin declares. + phoneNumber: { construct: () => phoneNumber({ sendOTP: async () => undefined }) }, + magicLink: { construct: () => magicLink({ sendMagicLink: async () => undefined }) }, + genericOAuth: { construct: () => genericOAuth({ config: [] }) }, + jwt: { construct: () => jwt() }, + deviceAuthorization: { construct: () => deviceAuthorization() }, + customSession: { + construct: () => + customSession(async ({ user, session }: { user: unknown; session: unknown }) => ({ + user, + session, + })), + }, + sso: { construct: () => sso() }, + scim: { construct: () => scim() }, + oauthProvider: { construct: () => oauthProvider({ loginPage: '/login' }) }, +}; + +/** The plugin set the auth manager actually assembles (`buildPluginList()`). */ function betterAuthPluginSet(): unknown[] { - return [ - bearer(), - // `teams: { enabled: true }` mirrors the auth-manager default. Without it - // better-auth omits the team models entirely, so the sys_team / - // sys_team_member entries below would be absent and any extension field - // added to those objects would collide silently. (#3624) - organization({ teams: { enabled: true } }), - twoFactor(), - haveIBeenPwned(), - admin(), - // The callbacks are required by the constructors and never invoked: this - // file only reads the schema each plugin declares. - phoneNumber({ sendOTP: async () => {} }), - magicLink({ sendMagicLink: async () => {} }), - genericOAuth({ config: [] }), - jwt(), - deviceAuthorization(), - customSession(async ({ user, session }: { user: unknown; session: unknown }) => ({ - user, - session, - })), - sso(), - scim(), - oauthProvider({ loginPage: '/login' }), - ]; + return Object.values(AUTH_MANAGER_PLUGINS) + .filter((entry): entry is { construct: () => unknown } => 'construct' in entry) + .map((entry) => entry.construct()); } /** @@ -647,27 +632,33 @@ describe('managed extension fields (ADR-0105 D7)', () => { `auth-manager.ts imports better-auth plugin factories this guard does not account for: ` + `${unaccounted.join(', ')}. A plugin the auth manager can assemble owns columns on the ` + `tables this guard compares, so leaving it out means the derived surface is narrower than ` - + `the one a booted environment gets. Add it to betterAuthPluginSet() and to ` - + `AUTH_MANAGER_PLUGINS as { loaded: true } — or, if it genuinely declares no schema this ` - + `call can read, register it with loaded: false and the reason. "It is off by default" is ` - + `NOT a reason: the column has to exist before the flag can be turned on.`, + + `the one a booted environment gets. Add it to AUTH_MANAGER_PLUGINS with a construct thunk ` + + `— or, if it genuinely declares no schema this call can read, with a skip reason. "It is ` + + `off by default" is NOT a reason: the column has to exist before the flag can be turned on.`, ).toEqual([]); const stale = declared.filter((name) => !imported.includes(name)); expect( stale, `AUTH_MANAGER_PLUGINS names plugins auth-manager.ts no longer imports: ${stale.join(', ')}. ` - + `Either the plugin was dropped (remove it here and from betterAuthPluginSet()) or the ` - + `import shape changed and this scan is now blind — check authManagerPluginFactories() ` - + `against the real import sites before deleting anything.`, + + `Either the plugin was dropped (remove its entry) or the import shape changed and this scan ` + + `is now blind — check authManagerPluginFactories() against the real import sites before ` + + `deleting anything.`, ).toEqual([]); - // Nothing may sit in the map unloaded without a stated reason. + // A skip is a decision, so it costs a sentence. `construct` needs no such + // check: it cannot claim coverage it does not deliver, because the thunk IS + // what builds the derived surface. for (const [name, entry] of Object.entries(AUTH_MANAGER_PLUGINS)) { - if (!entry.loaded) { - expect(entry.reason.length, `${name} is not loaded but carries no reason`).toBeGreaterThan(20); + if ('skip' in entry) { + expect(entry.skip.length, `${name} is skipped but carries no reason`).toBeGreaterThan(20); } } + // The set really is built from the map — otherwise the reconciliation above + // would be auditing a list nothing reads. + expect(betterAuthPluginSet().length).toBe( + Object.values(AUTH_MANAGER_PLUGINS).filter((entry) => 'construct' in entry).length, + ); }); it('editable extension fields are a SUBSET of declared extension fields', () => { From 8dec72c3eba94fc9e85a880b44e1f78d4960f0ca Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 08:55:44 +0000 Subject: [PATCH 3/4] test(plugin-auth): satisfy oauthProvider's required consentPage in the D7 derivation --- .../plugin-auth/src/managed-extension-fields.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts index e6995e6315..7de7c06d62 100644 --- a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts +++ b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts @@ -371,7 +371,12 @@ const AUTH_MANAGER_PLUGINS: Record unknown } | { skip }, sso: { construct: () => sso() }, scim: { construct: () => scim() }, - oauthProvider: { construct: () => oauthProvider({ loginPage: '/login' }) }, + // `loginPage` / `consentPage` are required by the constructor and are URLs + // the auth manager resolves from the console mount point; nothing about the + // schema depends on their value. + oauthProvider: { + construct: () => oauthProvider({ loginPage: '/login', consentPage: '/oauth/consent' }), + }, }; /** The plugin set the auth manager actually assembles (`buildPluginList()`). */ From 3251cd905ff65789749eb217a6f02490321a14e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 09:17:01 +0000 Subject: [PATCH 4/4] chore(changeset): phone_number ownership + D7 plugin derivation widening --- .changeset/thick-eagles-fetch.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .changeset/thick-eagles-fetch.md diff --git a/.changeset/thick-eagles-fetch.md b/.changeset/thick-eagles-fetch.md new file mode 100644 index 0000000000..12207e2fc4 --- /dev/null +++ b/.changeset/thick-eagles-fetch.md @@ -0,0 +1,24 @@ +--- +'@objectstack/plugin-auth': patch +--- + +Correct `sys_user.phone_number`'s ownership and widen the ADR-0105 D7 collision guard's plugin derivation. + +`phone_number` was declared as an ObjectStack extension field in +`MANAGED_EXTENSION_FIELDS` while `auth-schema-config.ts` has shipped the explicit +`phoneNumber -> phone_number` mapping since better-auth's phone-number plugin was +wired in, so better-auth writes that exact column whenever the plugin is enabled. +The entry is removed: the mapping is the ownership evidence. No write surface +changes — the field was never in `MANAGED_EXTENSION_EDITABLE_FIELDS`, and the +admin bulk-import path that does upsert it runs under a system context off its +own field list. + +The reason D7 never reported this overlap is the second half: it derived +better-auth's owned columns from a single plugin (`organization`) while the auth +manager assembles fourteen. The derivation now loads the auth manager's whole +set, on the reason the sibling parity gate already records — a plugin that is +feature-flagged off in some deployments still owns its columns, because the +column has to exist before the flag can be turned on. A drift tripwire reconciles +the set against `auth-manager.ts`'s imports so a plugin added there cannot stay +outside the guard, and the collision rule is now pinned in the red direction +against a synthetic registry, not only the green one.