From a47681bea3fc198db311c8d9f684647bcf3746ce Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 03:33:07 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(auth):=20identity=20write=20guard=20?= =?UTF-8?q?=E2=80=94=20enforce=20managedBy:'better-auth'=20at=20the=20engi?= =?UTF-8?q?ne=20(#2816)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0092 D2/D3/D6 implementation: - identity-write-guard.ts: before{Insert,Update,Delete} hooks (priority 10) registered by plugin-auth at kernel:ready. User-context writes to any object whose schema declares managedBy:'better-auth' are rejected fail-closed (403 PERMISSION_DENIED with a pointer to the dedicated surfaces); better-auth adapter (no context) and isSystem writes bypass. Per-object update whitelist registry is the only opening; engine-stamped lifecycle columns (updated_at/updated_by) pass through but never satisfy the whitelist alone. - sys-user-writable-fields.ts: shared field tiers — profile edit whitelist {name, image} and the import superset {+phone_number, role} (subset-by-construction, ADR-0092 D3); admin-import-users.ts now derives UPDATE_ALLOWED_FIELDS from it. - D6: afterUpdate hook mirrors better-auth's refreshUserSessions — cached {session, user} snapshots in secondary storage are re-written (same TTL, never deleted) with the changed profile fields after a guarded edit. Verified against a live --fresh CRM backend: profile PATCH persists with fresh updated_at; email-only PATCH 403s; role/must_change_password are stripped and not persisted; insert/delete 403; create-user / set-user-password / self-service update-user / import upsert all work unchanged. Call-site sweep: every existing identity-table write already runs under SYSTEM_CTX or the adapter path. Closes #2816 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YGAN5VvvBi4YRGwpNT6e21 --- .changeset/identity-write-guard.md | 37 +++ .../plugin-auth/src/admin-import-users.ts | 10 +- .../plugins/plugin-auth/src/auth-plugin.ts | 36 +++ .../src/identity-write-guard.test.ts | 299 ++++++++++++++++++ .../plugin-auth/src/identity-write-guard.ts | 280 ++++++++++++++++ packages/plugins/plugin-auth/src/index.ts | 2 + .../src/sys-user-writable-fields.ts | 32 ++ 7 files changed, 694 insertions(+), 2 deletions(-) create mode 100644 .changeset/identity-write-guard.md create mode 100644 packages/plugins/plugin-auth/src/identity-write-guard.test.ts create mode 100644 packages/plugins/plugin-auth/src/identity-write-guard.ts create mode 100644 packages/plugins/plugin-auth/src/sys-user-writable-fields.ts diff --git a/.changeset/identity-write-guard.md b/.changeset/identity-write-guard.md new file mode 100644 index 0000000000..c3c2cdedb2 --- /dev/null +++ b/.changeset/identity-write-guard.md @@ -0,0 +1,37 @@ +--- +"@objectstack/plugin-auth": minor +--- + +feat(auth): identity write guard — `managedBy: 'better-auth'` is now enforced at the engine (ADR-0092 D2/D3/D6) + +Every object whose schema declares `managedBy: 'better-auth'` (`sys_user`, +`sys_member`, `sys_session`, `sys_api_key`, …) is now protected by engine +`beforeInsert` / `beforeUpdate` / `beforeDelete` hooks registered by +plugin-auth: **user-context** writes through the generic data path are +rejected fail-closed with `403 PERMISSION_DENIED`, closing the hole where a +wildcard admin permission set could raw-write any identity column (including +`email` and credential stamps) via the data API. Internal writes are +unaffected — the better-auth adapter, `isSystem` plugin/system contexts, and +the identity import keep working unchanged. + +The only opening is a per-object update whitelist +(`registerManagedUpdateWhitelist(object, fields)`): non-whitelisted fields are +stripped from the payload, and a payload that strips to nothing throws. The +first registration ships here: `sys_user → { name, image }` (pure profile +fields), backed by the new shared `SYS_USER_PROFILE_EDIT_FIELDS` / +`SYS_USER_IMPORT_UPDATE_FIELDS` constants — the import upsert's field +discipline is now derived from the same module (subset-by-construction, no +drift). + +After a guarded profile edit, an `afterUpdate` companion hook re-writes the +user's cached `{session, user}` snapshots in better-auth's secondary storage +(same TTL, mirror of better-auth's own `refreshUserSessions`) so session +reads stay coherent; it rewrites rather than deletes, and no-ops when no +secondary storage is wired. + +Migration note: server-side scripts that previously updated identity tables +with a **user** execution context must either run with a system context +(`{ isSystem: true }`) if they are genuinely internal, or move to the +dedicated auth endpoints (invite / create-user / set-user-password / ban / +better-auth APIs). Flows and automations that wrote non-profile `sys_user` +columns under a user identity are now filtered the same way. diff --git a/packages/plugins/plugin-auth/src/admin-import-users.ts b/packages/plugins/plugin-auth/src/admin-import-users.ts index 2277481a42..0adaab013f 100644 --- a/packages/plugins/plugin-auth/src/admin-import-users.ts +++ b/packages/plugins/plugin-auth/src/admin-import-users.ts @@ -57,11 +57,17 @@ import type { import { prepareImportRequest, runImport } from '@objectstack/rest'; import { generatePlaceholderEmail, isPlaceholderEmail } from './placeholder-email.js'; import { generateTemporaryPassword, normalizePhoneNumber, isLikelyEmail, type AdminActor, type EndpointResult } from './admin-user-endpoints.js'; +import { SYS_USER_IMPORT_UPDATE_FIELDS } from './sys-user-writable-fields.js'; export const IMPORT_USERS_MAX_ROWS = 500; -/** Profile fields an upsert row may modify on an EXISTING user. */ -const UPDATE_ALLOWED_FIELDS = new Set(['name', 'phone_number', 'role']); +/** + * Profile fields an upsert row may modify on an EXISTING user — shared with + * the identity write guard's Tier-1 whitelist via sys-user-writable-fields.ts + * (ADR-0092 D3: one file, one derivation; the import surface is a strict + * superset that additionally allows `phone_number` / `role`). + */ +const UPDATE_ALLOWED_FIELDS = SYS_USER_IMPORT_UPDATE_FIELDS; export interface IdentityImportEngine { find(objectName: string, query?: any): Promise; diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 21c579c096..14464a55d3 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -18,6 +18,12 @@ import { type AuthManagerOptions, } from './auth-manager.js'; import { ensureDefaultOrganization } from './ensure-default-organization.js'; +import { + registerIdentityWriteGuard, + registerManagedUpdateWhitelist, + type SecondaryStorageLike, +} from './identity-write-guard.js'; +import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js'; import { runSetInitialPassword } from './set-initial-password.js'; import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain } from './register-sso-provider.js'; import { runResendVerificationEmail } from './send-verification-email.js'; @@ -125,6 +131,10 @@ export class AuthPlugin implements Plugin { private options: AuthPluginOptions; private authManager: AuthManager | null = null; private configuredSocialProviders: SocialProviderConfig | undefined; + // ADR-0092 D6 — the EFFECTIVE better-auth secondaryStorage (host-supplied or + // the kernel-cache adapter wired in init). The identity write guard's + // session-snapshot refresh reads through this; undefined = refresh no-ops. + private effectiveSecondaryStorage: AuthManagerOptions['secondaryStorage']; constructor(options: AuthPluginOptions = {}) { this.options = { @@ -221,6 +231,9 @@ export class AuthPlugin implements Plugin { // Initialize auth manager with data engine this.authManager = new AuthManager(authConfig); + // ADR-0092 D6 — remember the storage better-auth will actually use so the + // identity write guard can keep cached session snapshots coherent. + this.effectiveSecondaryStorage = authConfig.secondaryStorage; // Register auth service ctx.registerService('auth', this.authManager); @@ -535,6 +548,29 @@ export class AuthPlugin implements Plugin { } }); + // ADR-0092 D2/D6 — generic identity write guard. Every object whose + // schema declares `managedBy: 'better-auth'` gets fail-closed protection + // against USER-CONTEXT writes through the generic data path; the only + // opening is the per-object update whitelist (sys_user → profile fields). + // Internal writes (better-auth adapter, isSystem plugin/system contexts) + // bypass — see identity-write-guard.ts for the full contract. + ctx.hook('kernel:ready', async () => { + try { + const engine: any = ctx.getService('objectql'); + if (!engine || typeof engine.registerHook !== 'function') return; + registerManagedUpdateWhitelist(SystemObjectName.USER, SYS_USER_PROFILE_EDIT_FIELDS); + registerIdentityWriteGuard(engine, { + packageId: 'com.objectstack.plugin-auth.identity-write-guard', + logger: ctx.logger, + getSecondaryStorage: () => + this.effectiveSecondaryStorage as SecondaryStorageLike | undefined, + }); + } catch { + // Engine not available (mock mode) — permission-set defaults remain + // the only gate, exactly the pre-guard status quo. + } + }); + // Register auth middleware on ObjectQL engine (if available) try { const ql = ctx.getService('objectql'); diff --git a/packages/plugins/plugin-auth/src/identity-write-guard.test.ts b/packages/plugins/plugin-auth/src/identity-write-guard.test.ts new file mode 100644 index 0000000000..58b5e3a173 --- /dev/null +++ b/packages/plugins/plugin-auth/src/identity-write-guard.test.ts @@ -0,0 +1,299 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0092 D2/D6 — identity write guard. + * + * The guard is exercised through a fake engine that records registerHook + * calls, so each registered handler is driven directly with synthetic + * HookContext shapes matching what ObjectQLEngine builds (session from + * buildSession, input.{id,data,options}). + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + registerIdentityWriteGuard, + registerManagedUpdateWhitelist, + getManagedUpdateWhitelist, +} from './identity-write-guard.js'; +import { + SYS_USER_PROFILE_EDIT_FIELDS, + SYS_USER_IMPORT_UPDATE_FIELDS, +} from './sys-user-writable-fields.js'; + +type Handler = (ctx: any) => Promise; + +/** Fake engine capturing hook registrations, with a static schema registry. */ +function makeEngine(schemas: Record) { + const handlers: Record> = {}; + return { + handlers, + getSchema: (name: string) => schemas[name], + registerHook: (event: string, handler: Handler, options: any) => { + (handlers[event] ??= []).push({ handler, options }); + }, + }; +} + +const SCHEMAS = { + sys_user: { name: 'sys_user', managedBy: 'better-auth' }, + sys_member: { name: 'sys_member', managedBy: 'better-auth' }, + sys_session: { name: 'sys_session', managedBy: 'better-auth' }, + crm_lead: { name: 'crm_lead' }, + sys_automation_run: { name: 'sys_automation_run', managedBy: 'system' }, +}; + +/** Session shapes as ObjectQLEngine.buildSession produces them. */ +const USER_SESSION = { userId: 'usr_1', positions: [] }; +const SYSTEM_SESSION = { userId: 'usr_1', isSystem: true }; + +function guardOn(engine: ReturnType, event: string): Handler { + const entry = engine.handlers[event]?.find( + (h) => h.options?.packageId?.includes('identity-write-guard'), + ); + if (!entry) throw new Error(`no guard handler registered for ${event}`); + return entry.handler; +} + +function freshEngine() { + const engine = makeEngine(SCHEMAS); + registerManagedUpdateWhitelist('sys_user', SYS_USER_PROFILE_EDIT_FIELDS); + registerIdentityWriteGuard(engine, { packageId: 'test.identity-write-guard' }); + return engine; +} + +describe('identity write guard — insert/delete (ADR-0092 D2)', () => { + let engine: ReturnType; + beforeEach(() => { + engine = freshEngine(); + }); + + it('rejects a user-context insert on every managedBy:better-auth table', async () => { + for (const object of ['sys_user', 'sys_member', 'sys_session']) { + await expect( + guardOn(engine, 'beforeInsert')({ object, session: USER_SESSION, input: { data: {} } }), + ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object }); + } + }); + + it('rejects a user-context delete, with a message pointing at the dedicated surfaces', async () => { + await expect( + guardOn(engine, 'beforeDelete')({ object: 'sys_member', session: USER_SESSION, input: { id: 'm1' } }), + ).rejects.toThrow(/managed by better-auth.*dedicated auth surface/s); + }); + + it('bypasses system-context and context-less (better-auth adapter) writes', async () => { + for (const event of ['beforeInsert', 'beforeDelete']) { + await expect( + guardOn(engine, event)({ object: 'sys_user', session: SYSTEM_SESSION, input: {} }), + ).resolves.toBeUndefined(); + await expect( + guardOn(engine, event)({ object: 'sys_user', session: undefined, input: {} }), + ).resolves.toBeUndefined(); + } + }); + + it('ignores objects that are not managed by better-auth (incl. other managedBy buckets)', async () => { + for (const object of ['crm_lead', 'sys_automation_run', 'not_registered']) { + await expect( + guardOn(engine, 'beforeInsert')({ object, session: USER_SESSION, input: { data: {} } }), + ).resolves.toBeUndefined(); + } + }); +}); + +describe('identity write guard — update whitelist (ADR-0092 D2)', () => { + let engine: ReturnType; + beforeEach(() => { + engine = freshEngine(); + }); + + it('strips non-whitelisted fields in place and lets whitelisted ones through', async () => { + const data: any = { id: 'u1', name: 'New Name', image: 'https://x/a.png', email: 'evil@x', role: 'admin' }; + await guardOn(engine, 'beforeUpdate')({ + object: 'sys_user', + session: USER_SESSION, + input: { id: 'u1', data }, + }); + expect(data).toEqual({ id: 'u1', name: 'New Name', image: 'https://x/a.png' }); + }); + + it('throws when every submitted field is non-whitelisted (loud failure, not a silent no-op)', async () => { + const data: any = { id: 'u1', email: 'evil@x', must_change_password: false }; + await expect( + guardOn(engine, 'beforeUpdate')({ object: 'sys_user', session: USER_SESSION, input: { id: 'u1', data } }), + ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + // The error names what IS editable so the caller can fix the payload. + await expect( + guardOn(engine, 'beforeUpdate')({ object: 'sys_user', session: USER_SESSION, input: { id: 'u1', data: { email: 'e@x' } } }), + ).rejects.toThrow(/Editable fields: name, image/); + }); + + it('passes engine-stamped lifecycle columns through, but they never satisfy the whitelist alone', async () => { + // The REST data routes stamp updated_at on every update — a legit + // profile edit must keep it (audit freshness)… + const ok: any = { id: 'u1', name: 'N', updated_at: '2026-07-11T00:00:00Z' }; + await guardOn(engine, 'beforeUpdate')({ object: 'sys_user', session: USER_SESSION, input: { id: 'u1', data: ok } }); + expect(ok).toEqual({ id: 'u1', name: 'N', updated_at: '2026-07-11T00:00:00Z' }); + // …but an email-only PATCH must still fail loudly, not degrade into a + // timestamp touch. + await expect( + guardOn(engine, 'beforeUpdate')({ + object: 'sys_user', + session: USER_SESSION, + input: { id: 'u1', data: { email: 'evil@x', updated_at: '2026-07-11T00:00:00Z' } }, + }), + ).rejects.toMatchObject({ code: 'PERMISSION_DENIED' }); + }); + + it('rejects updates to managed tables with NO registered whitelist (default-deny)', async () => { + await expect( + guardOn(engine, 'beforeUpdate')({ + object: 'sys_member', + session: USER_SESSION, + input: { id: 'm1', data: { role: 'owner' } }, + }), + ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', object: 'sys_member' }); + }); + + it('filters the payload of multi-row updates too (input.id undefined)', async () => { + const data: any = { banned: true, name: 'Bulk Rename' }; + await guardOn(engine, 'beforeUpdate')({ + object: 'sys_user', + session: USER_SESSION, + input: { id: undefined, data, options: { multi: true } }, + }); + expect(data).toEqual({ name: 'Bulk Rename' }); + }); + + it('bypasses system-context and context-less updates entirely (no stripping)', async () => { + const data: any = { id: 'u1', must_change_password: true }; + await guardOn(engine, 'beforeUpdate')({ object: 'sys_user', session: SYSTEM_SESSION, input: { id: 'u1', data } }); + expect(data).toEqual({ id: 'u1', must_change_password: true }); + await guardOn(engine, 'beforeUpdate')({ object: 'sys_user', session: undefined, input: { id: 'u1', data } }); + expect(data).toEqual({ id: 'u1', must_change_password: true }); + }); + + it('exposes the registered whitelist for introspection', () => { + expect(getManagedUpdateWhitelist('sys_user')).toEqual(new Set(['name', 'image'])); + expect(getManagedUpdateWhitelist('sys_session')).toBeUndefined(); + }); +}); + +describe('identity write guard — session snapshot refresh (ADR-0092 D6)', () => { + const NOW = Date.now(); + const EXPIRES = new Date(NOW + 3600_000).toISOString(); + + function makeStorage(userId: string, tokens: string[]) { + const store = new Map(); + store.set( + `active-sessions-${userId}`, + JSON.stringify(tokens.map((token) => ({ token, expiresAt: NOW + 3600_000 }))), + ); + for (const token of tokens) { + store.set( + token, + JSON.stringify({ + session: { token, userId, expiresAt: EXPIRES }, + user: { id: userId, name: 'Old Name', image: null, email: 'a@b.c' }, + }), + ); + } + const ttls: Record = {}; + return { + store, + ttls, + get: vi.fn(async (k: string) => store.get(k) ?? null), + set: vi.fn(async (k: string, v: string, ttl?: number) => { + store.set(k, v); + ttls[k] = ttl; + }), + delete: vi.fn(async (k: string) => void store.delete(k)), + }; + } + + function engineWithStorage(storage: any) { + const engine = makeEngine(SCHEMAS); + registerManagedUpdateWhitelist('sys_user', SYS_USER_PROFILE_EDIT_FIELDS); + registerIdentityWriteGuard(engine, { + packageId: 'test.identity-write-guard', + getSecondaryStorage: () => storage, + }); + return engine; + } + + it('re-writes every live cached session with the changed profile fields (same user, keeps TTL, never deletes)', async () => { + const storage = makeStorage('u1', ['tok-a', 'tok-b']); + const engine = engineWithStorage(storage); + await guardOn(engine, 'afterUpdate')({ + object: 'sys_user', + session: USER_SESSION, + input: { id: 'u1', data: { name: 'New Name' } }, + }); + for (const token of ['tok-a', 'tok-b']) { + const entry = JSON.parse(storage.store.get(token)!); + expect(entry.user).toMatchObject({ id: 'u1', name: 'New Name', email: 'a@b.c' }); + expect(entry.session.token).toBe(token); + expect(storage.ttls[token]).toBeGreaterThan(0); + } + expect(storage.delete).not.toHaveBeenCalled(); + }); + + it('no-ops without secondary storage, without a whitelisted change, or for system writes', async () => { + const storage = makeStorage('u1', ['tok-a']); + // System write — better-auth's own paths already refresh. + let engine = engineWithStorage(storage); + await guardOn(engine, 'afterUpdate')({ + object: 'sys_user', + session: SYSTEM_SESSION, + input: { id: 'u1', data: { name: 'X' } }, + }); + expect(storage.set).not.toHaveBeenCalled(); + // Non-whitelisted change only (nothing survived the guard anyway). + await guardOn(engine, 'afterUpdate')({ + object: 'sys_user', + session: USER_SESSION, + input: { id: 'u1', data: { last_login_ip: '1.2.3.4' } }, + }); + expect(storage.set).not.toHaveBeenCalled(); + // No storage wired. + engine = engineWithStorage(undefined); + await expect( + guardOn(engine, 'afterUpdate')({ + object: 'sys_user', + session: USER_SESSION, + input: { id: 'u1', data: { name: 'X' } }, + }), + ).resolves.toBeUndefined(); + }); + + it('survives storage failures without breaking the write', async () => { + const storage = { + get: vi.fn(async () => { + throw new Error('redis down'); + }), + set: vi.fn(), + delete: vi.fn(), + }; + const engine = engineWithStorage(storage); + await expect( + guardOn(engine, 'afterUpdate')({ + object: 'sys_user', + session: USER_SESSION, + input: { id: 'u1', data: { name: 'X' } }, + }), + ).resolves.toBeUndefined(); + }); +}); + +describe('sys-user writable-field tiers (ADR-0092 D3)', () => { + it('import whitelist is a strict superset of the profile whitelist', () => { + for (const f of SYS_USER_PROFILE_EDIT_FIELDS) { + expect(SYS_USER_IMPORT_UPDATE_FIELDS.has(f)).toBe(true); + } + expect(SYS_USER_IMPORT_UPDATE_FIELDS.has('phone_number')).toBe(true); + expect(SYS_USER_IMPORT_UPDATE_FIELDS.has('role')).toBe(true); + // The profile tier stays profile-only. + expect(SYS_USER_PROFILE_EDIT_FIELDS.has('role')).toBe(false); + expect(SYS_USER_PROFILE_EDIT_FIELDS.has('email')).toBe(false); + }); +}); diff --git a/packages/plugins/plugin-auth/src/identity-write-guard.ts b/packages/plugins/plugin-auth/src/identity-write-guard.ts new file mode 100644 index 0000000000..3ee538ab29 --- /dev/null +++ b/packages/plugins/plugin-auth/src/identity-write-guard.ts @@ -0,0 +1,280 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0092 D2 — generic identity write guard. + * + * `managedBy: 'better-auth'` promises that identity tables are only written + * through the better-auth pipeline, but until this guard that promise was + * enforced by nothing except UI affordances and default permission sets — + * `admin_full_access` (wildcard, no RLS) could raw-write any column of any + * identity table through the generic data API (ADR-0049 violation). + * + * The guard registers engine `beforeInsert` / `beforeUpdate` / `beforeDelete` + * hooks that fail-closed reject USER-CONTEXT writes to every object whose + * registered schema declares `managedBy: 'better-auth'`. The flag is read + * from the schema registry at evaluation time — there is no hardcoded table + * list to drift from the schemas. + * + * What passes untouched: + * - the better-auth adapter (its engine calls carry no caller context); + * - plugin / system writes (`isSystem` contexts, e.g. import, sign-in + * stamps, provenance hooks). + * + * The only opening is a per-object UPDATE whitelist + * ({@link registerManagedUpdateWhitelist}); non-whitelisted keys are + * stripped, and a payload that strips to nothing throws — a loud failure, + * not a silent no-op. First (and currently only) registration: + * `sys_user → SYS_USER_PROFILE_EDIT_FIELDS` (name, image). + * + * Rejections use `code: 'PERMISSION_DENIED'` + `status: 403`, which the REST + * layer's `mapDataError` / `sendError` already translate — same pattern as + * plugin-audit's FEEDS_DISABLED / FILES_DISABLED capability gates. + * + * ADR-0092 D6 — a companion `afterUpdate` hook keeps better-auth's + * secondary-storage session snapshots coherent after a guarded profile edit, + * mirroring better-auth's own `refreshUserSessions`: each cached + * `{session, user}` entry for the affected user is re-written (same TTL) + * with the changed fields merged in. It rewrites, never deletes — deleting + * would sign the user out when sessions live only in the cache. + */ + +type LoggerLike = { + info(msg: string): void; + warn(msg: string): void; + debug?(msg: string): void; +}; + +/** Shape of better-auth's `secondaryStorage` (see secondary-storage.ts). */ +export interface SecondaryStorageLike { + get(key: string): Promise; + set(key: string, value: string, ttl?: number): Promise; + delete(key: string): Promise; +} + +export interface IdentityWriteGuardOptions { + packageId: string; + logger?: LoggerLike; + /** + * Resolves the EFFECTIVE better-auth secondaryStorage (kernel cache + * adapter or host-supplied). Undefined / throwing = session caching not + * wired = the D6 refresh is a no-op (single-node memory cache TTLs the + * stale snapshot out). + */ + getSecondaryStorage?: () => SecondaryStorageLike | undefined; +} + +// --------------------------------------------------------------------------- +// Update whitelist registry (ADR-0092 D2) +// --------------------------------------------------------------------------- + +const updateWhitelists = new Map>(); + +/** + * Open a per-object UPDATE whitelist on a better-auth-managed table. The + * listed fields become editable through the generic data path for callers + * the permission layer already admits; everything else stays stripped. + * Registering is the ONLY way to open a managed table — absence = deny. + */ +export function registerManagedUpdateWhitelist(object: string, fields: Iterable): void { + updateWhitelists.set(object, new Set(fields)); +} + +/** Test seam / introspection. */ +export function getManagedUpdateWhitelist(object: string): ReadonlySet | undefined { + return updateWhitelists.get(object); +} + +// --------------------------------------------------------------------------- +// Guard internals +// --------------------------------------------------------------------------- + +/** + * A write is user-context when the operation context carries a real user and + * is not system-elevated. better-auth's own adapter calls pass no context at + * all (no session → not user-context), and plugin/system writes stamp + * `isSystem` — both bypass by construction. + */ +function isUserContextWrite(session: any): boolean { + return Boolean(session?.userId) && session?.isSystem !== true; +} + +function forbidden(object: string, message: string): Error { + const err: any = new Error(`PERMISSION_DENIED: ${message}`); + err.code = 'PERMISSION_DENIED'; + err.status = 403; + err.object = object; + return err; +} + +const DEDICATED_SURFACE_HINT = + 'use the dedicated auth surface instead (invite / create-user / admin endpoints, or the better-auth API)'; + +/** + * Engine-owned lifecycle stamps the WRITE PATH itself injects (the REST data + * routes stamp `updated_at`/`updated_by` on every update, for every object). + * They pass through the whitelist filter — stripping them would freeze the + * row's audit freshness on guarded edits — but they do NOT count as "a field + * the caller may edit": an update whose only surviving keys are lifecycle + * stamps is still rejected, otherwise an email-only PATCH would silently + * degrade into a timestamp touch instead of failing loudly. + */ +const LIFECYCLE_PASSTHROUGH = new Set(['updated_at', 'updated_by']); + +/** + * Register the identity write guard on an ObjectQL engine. Idempotent per + * package: callers re-binding after hot reload should first run + * `engine.unregisterHooksByPackage(packageId)` (the engine's standard + * re-bind contract). + * + * Priority 10 (< default 100) so a rejection fires before plugin-audit's + * `beforeUpdate` prior-snapshot fetch and any other default-priority hooks + * spend work on a doomed write. + */ +export function registerIdentityWriteGuard(engine: any, opts: IdentityWriteGuardOptions): void { + const { packageId, logger } = opts; + + const isManaged = (object: string): boolean => { + try { + return engine.getSchema?.(object)?.managedBy === 'better-auth'; + } catch { + return false; + } + }; + + const rejectWrite = (operation: 'create' | 'delete') => async (ctx: any) => { + if (!isManaged(ctx.object) || !isUserContextWrite(ctx.session)) return; + throw forbidden( + ctx.object, + `Identity table '${ctx.object}' is managed by better-auth (ADR-0092): ` + + `direct ${operation} via the data API is disabled — ${DEDICATED_SURFACE_HINT}.`, + ); + }; + + const guardUpdate = async (ctx: any) => { + if (!isManaged(ctx.object) || !isUserContextWrite(ctx.session)) return; + + const whitelist = updateWhitelists.get(ctx.object); + if (!whitelist) { + throw forbidden( + ctx.object, + `Identity table '${ctx.object}' is managed by better-auth (ADR-0092): ` + + `direct update via the data API is disabled — ${DEDICATED_SURFACE_HINT}.`, + ); + } + + const data: Record = (ctx.input?.data ?? {}) as Record; + const stripped: string[] = []; + let editableRemaining = 0; + for (const key of Object.keys(data)) { + // `id` is the row address, not a field write — the engine has already + // extracted it into `input.id`; leaving it in place changes nothing. + if (key === 'id') continue; + if (LIFECYCLE_PASSTHROUGH.has(key)) continue; + if (whitelist.has(key)) { + editableRemaining += 1; + } else { + delete data[key]; + stripped.push(key); + } + } + + if (editableRemaining === 0) { + throw forbidden( + ctx.object, + `None of the submitted fields (${stripped.join(', ') || '—'}) are editable on ` + + `'${ctx.object}' via the data API (ADR-0092). Editable fields: ` + + `${[...whitelist].join(', ')}. For anything else, ${DEDICATED_SURFACE_HINT}.`, + ); + } + if (stripped.length > 0) { + logger?.warn( + `[IdentityWriteGuard] stripped non-whitelisted field(s) from user-context update to ` + + `'${ctx.object}': ${stripped.join(', ')} (ADR-0092)`, + ); + } + }; + + engine.registerHook('beforeInsert', rejectWrite('create'), { priority: 10, packageId }); + engine.registerHook('beforeUpdate', guardUpdate, { priority: 10, packageId }); + engine.registerHook('beforeDelete', rejectWrite('delete'), { priority: 10, packageId }); + + // ── ADR-0092 D6 — session-snapshot coherence after guarded profile edits ── + // + // better-auth caches `{session, user}` per session token in secondary + // storage, plus an `active-sessions-${userId}` index. Its OWN update paths + // re-write those snapshots (internal-adapter `refreshUserSessions`); a + // guarded engine write bypasses that, so we mirror it here for the fields + // the guard let through. sys_user Tier-1 columns map 1:1 onto better-auth + // user-model field names (name → name, image → image); anything that would + // need a snake_case → camelCase translation is not whitelisted today, and + // widening the whitelist must extend this mapping deliberately. + const refreshSessionSnapshots = async (ctx: any) => { + try { + if (ctx.object !== 'sys_user' || !isUserContextWrite(ctx.session)) return; + const storage = opts.getSecondaryStorage?.(); + if (!storage) return; + + const data: Record = (ctx.input?.data ?? {}) as Record; + const whitelist = updateWhitelists.get('sys_user'); + const changed: Record = {}; + for (const key of Object.keys(data)) { + if (key !== 'id' && whitelist?.has(key)) changed[key] = data[key]; + } + if (Object.keys(changed).length === 0) return; + + const userId = ctx.input?.id ?? data.id; + if (!userId) { + // Multi-row user-context update — no single user to refresh. Rare + // (bulk profile edits); snapshots then age out on their TTL. + logger?.warn( + '[IdentityWriteGuard] multi-row sys_user update: session snapshots not refreshed (TTL will age them out)', + ); + return; + } + + const listRaw = await storage.get(`active-sessions-${String(userId)}`); + if (!listRaw) return; + let list: Array<{ token: string; expiresAt: number }> = []; + try { + list = JSON.parse(listRaw) ?? []; + } catch { + return; + } + const now = Date.now(); + await Promise.all( + list + .filter((s) => s && typeof s.token === 'string' && s.expiresAt > now) + .map(async ({ token }) => { + const cached = await storage.get(token); + if (!cached) return; + let parsed: any; + try { + parsed = JSON.parse(cached); + } catch { + return; + } + if (!parsed?.session || !parsed?.user) return; + const ttl = Math.floor((new Date(parsed.session.expiresAt).getTime() - now) / 1000); + if (!Number.isFinite(ttl) || ttl <= 0) return; + await storage.set( + token, + JSON.stringify({ session: parsed.session, user: { ...parsed.user, ...changed } }), + ttl, + ); + }), + ); + } catch (e) { + // Snapshot refresh must never fail the write — worst case the cached + // profile is stale until the session entry's TTL expires. + logger?.warn( + `[IdentityWriteGuard] session snapshot refresh failed: ${(e as Error)?.message ?? e}`, + ); + } + }; + + engine.registerHook('afterUpdate', refreshSessionSnapshots, { object: 'sys_user', packageId }); + + logger?.info( + '[IdentityWriteGuard] managedBy:better-auth write guard registered (ADR-0092 D2/D6)', + ); +} diff --git a/packages/plugins/plugin-auth/src/index.ts b/packages/plugins/plugin-auth/src/index.ts index bd3544e070..edbdba0a82 100644 --- a/packages/plugins/plugin-auth/src/index.ts +++ b/packages/plugins/plugin-auth/src/index.ts @@ -15,6 +15,8 @@ export * from './set-initial-password.js'; export * from './admin-user-endpoints.js'; export * from './placeholder-email.js'; export * from './admin-import-users.js'; +export * from './identity-write-guard.js'; +export * from './sys-user-writable-fields.js'; export * from './otp-send-guard.js'; export * from './register-sso-provider.js'; export * from './send-verification-email.js'; diff --git a/packages/plugins/plugin-auth/src/sys-user-writable-fields.ts b/packages/plugins/plugin-auth/src/sys-user-writable-fields.ts new file mode 100644 index 0000000000..081895fc5d --- /dev/null +++ b/packages/plugins/plugin-auth/src/sys-user-writable-fields.ts @@ -0,0 +1,32 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0092 D1/D3 — the single source of truth for which `sys_user` columns + * generic write surfaces may touch. + * + * Two tiers, subset-by-construction (a spread, not two hand-maintained + * lists), because the two surfaces intentionally differ: + * + * - the standard edit form / data API may only touch pure profile fields + * (enforced server-side by the identity write guard, ADR-0092 D2); + * - the admin bulk-identity import may additionally upsert `phone_number` + * (sign-in identifier — bulk identity onboarding is that surface's + * purpose) and `role`. Import runs under a system context, so it passes + * the guard by context, not by whitelist — this constant is its own + * field discipline. + * + * Everything not listed here is either admin-surface-only (role/ban columns, + * `manager_id`, `ai_access`, …) or never-direct (email, credentials, every + * system-managed stamp). See ADR-0092 D1 for the full tier table. Adding a + * field to `sys_user` never silently opens it — absence means denied. + */ + +/** Tier 1 — standard form / data-API editable (identity write guard whitelist). */ +export const SYS_USER_PROFILE_EDIT_FIELDS: ReadonlySet = new Set(['name', 'image']); + +/** Import-upsert may additionally touch these (admin bulk-identity surface). */ +export const SYS_USER_IMPORT_UPDATE_FIELDS: ReadonlySet = new Set([ + ...SYS_USER_PROFILE_EDIT_FIELDS, + 'phone_number', + 'role', +]); From 7298e732535231d9f72e31fe9eeb4dc6670d7722 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 03:49:22 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(dogfood):=20sys=5Fteam=20single-tenant?= =?UTF-8?q?=20create=20=E2=80=94=20align=20with=20ADR-0092=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old test asserted POST /data/sys_team returns 201 — a raw data-API path the product UI never exposes (sys_team's own schema gates every team-mutation affordance to multi-org mode, and its actions name the better-auth team endpoints as the canonical entry points). The identity write guard now rejects that user-context insert by design. The ADR-0057 regression target — organization_id optional at the schema level so single-tenant writes don't VALIDATION_FAIL — is kept, asserted through the writers that remain legitimate post-ADR-0092: system-context engine writes. The user-context 403 (PERMISSION_DENIED) becomes explicit coverage of the guard. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YGAN5VvvBi4YRGwpNT6e21 --- ...gle-tenant-identity-create.dogfood.test.ts | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/dogfood/test/single-tenant-identity-create.dogfood.test.ts b/packages/dogfood/test/single-tenant-identity-create.dogfood.test.ts index f62cb35eba..bbf8df5cfe 100644 --- a/packages/dogfood/test/single-tenant-identity-create.dogfood.test.ts +++ b/packages/dogfood/test/single-tenant-identity-create.dogfood.test.ts @@ -3,10 +3,20 @@ /** * ADR-0057 — org-scoped identity objects must be creatable in SINGLE-TENANT. * - * Single-tenant deployments have no `sys_organization` row and no auto-stamp - * (OrgScopingPlugin is multi-tenant-only), so a `required` `organization_id` - * made sys_business_unit / sys_team uncreatable (VALIDATION_FAILED). The field - * is now optional; this proves the create path works with no org. + * Single-tenant deployments have no auto-stamp (OrgScopingPlugin is + * multi-tenant-only), so a `required` `organization_id` made + * sys_business_unit / sys_team uncreatable (VALIDATION_FAILED). The field is + * now optional; this proves the create path works single-tenant. + * + * ADR-0092 update: `sys_team` is `managedBy: 'better-auth'`, so its generic + * data-API insert is now REJECTED fail-closed for user contexts by the + * identity write guard. The canonical user surfaces are better-auth's team + * endpoints (see sys_team.actions), which the schema itself gates to + * multi-org mode — single-org hides every team-mutation affordance. The + * ADR-0057 property (optional `organization_id`) therefore matters for the + * writers that remain legitimate single-tenant: SYSTEM-context writes. + * sys_business_unit is plugin-security's table (not better-auth-managed) and + * keeps the generic create path. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -31,8 +41,27 @@ describe('ADR-0057: org-scoped identity creatable single-tenant', () => { expect(body.record?.organization_id ?? null).toBeNull(); }); - it('creates a sys_team with no organization_id', async () => { - const res = await stack.apiAs(token, 'POST', '/data/sys_team', { name: 'Tiger Team' }); - expect(res.status).toBe(201); + it('sys_team: generic insert is guarded for users; org_id stays optional for system writes', async () => { + // ADR-0092 D2 — sys_team is managedBy:'better-auth', so a USER-context + // insert through the generic data API is rejected fail-closed (the + // canonical surfaces are better-auth's team endpoints, which the schema + // gates to multi-org mode — in single-org the affordances are hidden). + const direct = await stack.apiAs(token, 'POST', '/data/sys_team', { name: 'Tiger Team' }); + expect(direct.status).toBe(403); + const denied: any = await direct.json(); + expect(denied.code).toBe('PERMISSION_DENIED'); + + // ADR-0057's actual regression target — `organization_id` is OPTIONAL at + // the schema level, so a single-tenant (no org row) write does not die + // with VALIDATION_FAILED. System-context writes (org-structure sync, + // seeding) are the writers that remain legitimate post-ADR-0092. + const ql = await stack.kernel.getServiceAsync('objectql'); + const row = await ql.insert( + 'sys_team', + { name: 'Tiger Team' }, + { context: { isSystem: true } }, + ); + expect(row?.id).toBeTruthy(); + expect(row?.organization_id ?? null).toBeNull(); }); });