From f37aba69277663118ab38294f30233cad2277e92 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 06:25:46 +0000 Subject: [PATCH] fix(plugin-auth): run better-auth adapter writes as system context (#3164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Rename `withSystemReadContext` → `withSystemContext` (deprecated alias kept one release) and inject `isSystem` on insert/update/delete as well as reads. 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 this path only ever carries better-auth's internal writes. Found while implementing #3043 (the INSERT-side readonly strip) — this is its UPDATE-side dual. Also corrects content/docs/data-modeling/fields.mdx, which still said "insert may still seed it" (stale after #3043 / PR #3162): a readonly column is now server-enforced on INSERT too, and seeding one at create requires a system context. Verified: plugin-auth suite 460 passed (adapter writes now assert isSystem); dogfood auth/identity/permission regression green (sign-in, org/member reads, permission seeding, two-doors provenance). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01P5fRY4ctobCeFpBba1oGrz --- .../betterauth-adapter-system-writes.md | 27 ++++++++++ content/docs/data-modeling/fields.mdx | 2 +- .../plugin-auth/src/objectql-adapter.test.ts | 29 ++++++----- .../plugin-auth/src/objectql-adapter.ts | 50 +++++++++++++------ 4 files changed, 80 insertions(+), 28 deletions(-) create mode 100644 .changeset/betterauth-adapter-system-writes.md diff --git a/.changeset/betterauth-adapter-system-writes.md b/.changeset/betterauth-adapter-system-writes.md new file mode 100644 index 0000000000..d9371813ca --- /dev/null +++ b/.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. diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index 1ae9f3237c..18fbd99268 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/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'`) | diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.test.ts b/packages/plugins/plugin-auth/src/objectql-adapter.test.ts index 571e4e1ae7..8f43f83b72 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.test.ts +++ b/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 } }); }); }); diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.ts b/packages/plugins/plugin-auth/src/objectql-adapter.ts index daabb0a94b..24d8d2a57c 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.ts @@ -145,29 +145,49 @@ function convertWhere(where: CleanedWhere[]): Record { // --------------------------------------------------------------------------- /** - * 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 >({ model, data, select: _select }: { model: string; data: T; select?: string[] }): Promise => { const objectName = resolveProtocolName(model);