fix(plugin-auth): run better-auth adapter writes as system context (#3164)

.changeset/betterauth-adapter-system-writes.md

@@ -0,0 +1,27 @@
+---
+"@objectstack/plugin-auth": patch
+---
+
+fix(plugin-auth): run better-auth adapter WRITES as system context so #2948 doesn't strip readonly identity columns (#3164)
+
+The better-auth ObjectQL adapter wrapped the engine so its READS carried
+`isSystem` (to bypass the control-plane org-scope read hook), but its WRITES
+passed through with no context. The static-`readonly` UPDATE strip (#2948) runs
+on any non-system update — and since the adapter carries no caller context,
+`!ctx?.isSystem` was `true`, so the strip silently DROPPED better-auth's own
+writes to readonly `sys_user` columns: `email` (change-email), `banned` /
+`ban_reason` / `ban_expires` (admin ban). Those operations returned success but
+never persisted.
+
+`withSystemReadContext` is renamed to `withSystemContext` (a deprecated alias is
+kept for one release) and now injects `isSystem` on `insert` / `update` /
+`delete` as well as reads. This is correct because these are the identity
+authority's own writes — user-context writes to `managedBy: 'better-auth'` tables
+are already rejected upstream by the ADR-0092 identity write guard, so the
+adapter path only ever carries better-auth's internal writes.
+
+Found while implementing #3043 (the INSERT-side readonly strip). This is its
+UPDATE-side dual: #3043 relocated the insert strip to the external ingress
+precisely because internal writers (this adapter included) don't declare
+`isSystem`; the pre-existing engine-level UPDATE strip has no such relocation, so
+the adapter had to declare its writes system.

content/docs/data-modeling/fields.mdx

@@ -284,7 +284,7 @@ These properties are available on all field types:
 | `description` | `string` | — | Developer documentation |
 | `inlineHelpText` | `string` | — | Help text shown in UI |
 | `hidden` | `boolean` | `false` | Hide from default views |
-| `readonly` | `boolean` | `false` | Prevent editing — hidden from create/edit forms AND server-enforced: non-system `UPDATE` writes to the field are silently dropped (insert may still seed it) |
+| `readonly` | `boolean` | `false` | Prevent editing — hidden from create/edit forms AND server-enforced on both write paths: a non-system write to the field is silently dropped on `UPDATE` (in the engine) and on `INSERT` through the data API (REST/GraphQL/MCP/import, at the DataProtocol ingress). A stripped field still falls back to its `defaultValue`; **seeding a `readonly` column at create requires a system context** (import/migration/programmatic seed). Platform (`sys_`/`managedBy`) objects are governed by their own guards instead. |
 | `sortable` | `boolean` | `true` | Allow sorting by this field |
 | `group` | `string` | — | Group name for organizing in forms (e.g. `'billing'`) |
 

packages/plugins/plugin-auth/src/objectql-adapter.test.ts

@@ -4,7 +4,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import {
   createObjectQLAdapter,
   createObjectQLAdapterFactory,
-  withSystemReadContext,
+  withSystemContext,
   AUTH_MODEL_TO_PROTOCOL,
   resolveProtocolName,
 } from './objectql-adapter';
@@ -226,7 +226,9 @@ describe('createObjectQLAdapter – legacy model name mapping', () => {
   it('create: should call dataEngine.insert with sys_ protocol name', async () => {
     const adapter = createObjectQLAdapter(mockEngine);
     await adapter.create({ model: 'user', data: { email: 'a@b.com' } });
-    expect(mockEngine.insert).toHaveBeenCalledWith('sys_user', { email: 'a@b.com' });
+    // #3164 — adapter writes run as system so the #2948 readonly strip doesn't
+    // drop better-auth's own writes to readonly identity columns.
+    expect(mockEngine.insert).toHaveBeenCalledWith('sys_user', { email: 'a@b.com' }, { context: { isSystem: true } });
   });
 
   it('findOne: should call dataEngine.findOne with sys_ protocol name', async () => {
@@ -262,7 +264,8 @@ describe('createObjectQLAdapter – legacy model name mapping', () => {
       update: { name: 'New' },
     });
     expect(mockEngine.findOne).toHaveBeenCalledWith('sys_user', expect.anything());
-    expect(mockEngine.update).toHaveBeenCalledWith('sys_user', expect.objectContaining({ name: 'New', id: '1' }));
+    // #3164 — the update carries system context (readonly identity writes survive).
+    expect(mockEngine.update).toHaveBeenCalledWith('sys_user', expect.objectContaining({ name: 'New', id: '1' }), { context: { isSystem: true } });
   });
 
   it('delete: should call dataEngine with sys_ protocol name', async () => {
@@ -278,7 +281,7 @@ describe('createObjectQLAdapter – legacy model name mapping', () => {
   it('should pass through unknown model names unchanged', async () => {
     const adapter = createObjectQLAdapter(mockEngine);
     await adapter.create({ model: 'organization', data: { name: 'Acme' } });
-    expect(mockEngine.insert).toHaveBeenCalledWith('organization', { name: 'Acme' });
+    expect(mockEngine.insert).toHaveBeenCalledWith('organization', { name: 'Acme' }, { context: { isSystem: true } });
   });
 });
 
@@ -325,7 +328,7 @@ describe('createObjectQLAdapterFactory – schema-less plugin bridging (@better-
   });
 });
 
-describe('withSystemReadContext – system-scoped reads (org-scope bypass)', () => {
+describe('withSystemContext – system-scoped reads AND writes', () => {
   let mockEngine: IDataEngine;
 
   beforeEach(() => {
@@ -340,7 +343,7 @@ describe('withSystemReadContext – system-scoped reads (org-scope bypass)', ()
   });
 
   it('injects context.isSystem into find / findOne / count', async () => {
-    const e = withSystemReadContext(mockEngine);
+    const e = withSystemContext(mockEngine);
     await e.find('sys_member', { where: { user_id: 'u1' } } as any);
     await e.findOne('sys_organization', { where: { id: 'o1' } } as any);
     await e.count('sys_member', { where: { user_id: 'u1' } } as any);
@@ -350,19 +353,21 @@ describe('withSystemReadContext – system-scoped reads (org-scope bypass)', ()
   });
 
   it('merges isSystem with a caller-supplied context', async () => {
-    const e = withSystemReadContext(mockEngine);
+    const e = withSystemContext(mockEngine);
     await e.find('sys_member', { where: {}, context: { transaction: 'tx1' } } as any);
     expect(mockEngine.find).toHaveBeenCalledWith('sys_member', expect.objectContaining({ context: { transaction: 'tx1', isSystem: true } }));
   });
 
-  it('does NOT alter writes (insert / update / delete pass straight through)', async () => {
-    const e = withSystemReadContext(mockEngine);
+  it('runs WRITES as system too — insert/update carry isSystem, delete merges it into the query (#3164)', async () => {
+    const e = withSystemContext(mockEngine);
     await e.insert('sys_member', { id: 'm1' } as any);
     await e.update('sys_member', { id: 'm1' } as any);
     await e.delete('sys_member', { where: { id: 'm1' } } as any);
-    expect(mockEngine.insert).toHaveBeenCalledWith('sys_member', { id: 'm1' });
-    expect(mockEngine.update).toHaveBeenCalledWith('sys_member', { id: 'm1' });
-    expect(mockEngine.delete).toHaveBeenCalledWith('sys_member', { where: { id: 'm1' } });
+    // Without this the #2948 readonly-UPDATE strip silently drops better-auth's
+    // own writes to readonly identity columns (sys_user.email, banned, …).
+    expect(mockEngine.insert).toHaveBeenCalledWith('sys_member', { id: 'm1' }, { context: { isSystem: true } });
+    expect(mockEngine.update).toHaveBeenCalledWith('sys_member', { id: 'm1' }, { context: { isSystem: true } });
+    expect(mockEngine.delete).toHaveBeenCalledWith('sys_member', { where: { id: 'm1' }, context: { isSystem: true } });
   });
 });
 

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

@@ -145,29 +145,49 @@ function convertWhere(where: CleanedWhere[]): Record<string, any> {
 // ---------------------------------------------------------------------------
 
 /**
- * Wrap a data engine so its READ operations (find / findOne / count) run as
- * SYSTEM reads — injecting `context.isSystem: true` (merged; any caller-supplied
- * context still wins on other keys). better-auth has already authenticated the
- * session and scopes every query by its OWN where-clauses (e.g. member.userId =
- * session.user). A deployment's control-plane org-scope read hook, however, keys
- * off the CALLER's user id, and these adapter reads carry no caller context — so
- * without isSystem that hook filters sys_member / sys_organization reads down to
- * zero and `organization.list()` returns no orgs for a real member. Writes pass
- * through untouched (org-scope is a read-only hook).
+ * Wrap a data engine so its operations run as SYSTEM against the identity
+ * tables — injecting `context.isSystem: true` (merged; any caller-supplied
+ * context still wins on other keys). better-auth is the identity AUTHORITY: it
+ * has already authenticated the session and scopes every query/write by its OWN
+ * where-clauses (e.g. member.userId = session.user).
+ *
+ * READS run as system so a deployment's control-plane org-scope read hook —
+ * which keys off the CALLER's user id — doesn't filter these caller-context-less
+ * adapter reads of sys_member / sys_organization down to zero (which would make
+ * `organization.list()` return no orgs for a real member).
+ *
+ * WRITES (`update` / `insert` / `delete`) also run as system (#3164). Several
+ * identity columns are declared `readonly` on their schema — `sys_user.email`
+ * (change-email), `banned` / `ban_reason` / `ban_expires` (admin ban) — and the
+ * static-`readonly` UPDATE strip (#2948) runs on any NON-system update. Since
+ * the adapter carries no caller context, `!ctx?.isSystem` was TRUE and the strip
+ * silently DROPPED better-auth's own writes to those columns (change-email /
+ * ban would return success but never persist). Marking the adapter's writes
+ * system exempts them — correct, because these ARE the identity authority's own
+ * writes; user-context writes to `managedBy: 'better-auth'` tables are already
+ * rejected upstream by the identity write guard (ADR-0092 D2), so this path only
+ * ever carries better-auth's internal writes.
  */
-export function withSystemReadContext(engine: IDataEngine): IDataEngine {
+export function withSystemContext(engine: IDataEngine): IDataEngine {
   const e = engine as any;
   const asSystem = (q: any) => ({ ...(q ?? {}), context: { isSystem: true, ...(q?.context ?? {}) } });
   return {
-    insert: (m: string, d: any) => e.insert(m, d),
-    update: (m: string, d: any) => e.update(m, d),
-    delete: (m: string, q?: any) => e.delete(m, q),
+    insert: (m: string, d: any, o?: any) => e.insert(m, d, asSystem(o)),
+    update: (m: string, d: any, o?: any) => e.update(m, d, asSystem(o)),
+    delete: (m: string, q?: any) => e.delete(m, asSystem(q)),
     find: (m: string, q?: any) => e.find(m, asSystem(q)),
     findOne: (m: string, q?: any) => e.findOne(m, asSystem(q)),
     count: (m: string, q?: any) => e.count(m, asSystem(q)),
   } as unknown as IDataEngine;
 }
 
+/**
+ * @deprecated Renamed to {@link withSystemContext} (#3164) now that writes are
+ * system-scoped too, not only reads. Kept as an alias for one release so
+ * external callers / in-flight imports don't break.
+ */
+export const withSystemReadContext = withSystemContext;
+
 /**
  * Create an ObjectQL adapter **factory** for better-auth.
  *
@@ -185,7 +205,7 @@ export function withSystemReadContext(engine: IDataEngine): IDataEngine {
  * @returns better-auth AdapterFactory
  */
 export function createObjectQLAdapterFactory(rawDataEngine: IDataEngine) {
-  const dataEngine = withSystemReadContext(rawDataEngine);
+  const dataEngine = withSystemContext(rawDataEngine);
   // Field-name bridging for better-auth plugins that expose NO `schema` option
   // (e.g. @better-auth/sso): when a model is remapped via AUTH_MODEL_TO_PROTOCOL,
   // its camelCase model fields are also converted to snake_case columns on the
@@ -406,7 +426,7 @@ export function createObjectQLAdapterFactory(rawDataEngine: IDataEngine) {
  * @returns better-auth CustomAdapter (raw, without factory wrapping)
  */
 export function createObjectQLAdapter(rawDataEngine: IDataEngine) {
-  const dataEngine = withSystemReadContext(rawDataEngine);
+  const dataEngine = withSystemContext(rawDataEngine);
   return {
     create: async <T extends Record<string, any>>({ model, data, select: _select }: { model: string; data: T; select?: string[] }): Promise<T> => {
       const objectName = resolveProtocolName(model);