fix(identity): reconcile managedBy:'better-auth' apiMethods with the write guard (#1591) (#3213)

os-zhuang · claude · web-flow · commit 865ccb92fd56 · 2026-07-18T21:48:08.000+08:00
* fix(identity): reconcile managedBy:'better-auth' apiMethods with the write guard (#1591) Phase 1 — tighten `apiMethods` on better-auth-managed identity objects. `managedBy: 'better-auth'` promises generic CRUD is suppressed (writes flow through better-auth's endpoints; the plugin-auth identity write guard rejects direct user-context create/update/delete), yet these schemas still advertised the write verbs in `enable.apiMethods`. That made the HTTP exposure gate admit the request and let it 403 at the engine instead of answering a clean 405, and the metadata contradicted its own doc comments. Each better-auth object is now read-only (`get`/`list`), except `sys_user`, which keeps `update` — the one generic write opened on an identity table (ADR-0092 D4), server-side clamped to the profile-field whitelist ({name, image}) and declared via `userActions.edit`. Phase 2 — registration-time consistency backstop. `reconcileManagedApiMethods` runs in `SchemaRegistry.registerObject` and strips any generic write verb a better-auth object advertises but does not grant (per `resolveCrudAffordances`: the managedBy bucket default plus `userActions` overrides), with a warning. Reads are never touched; non-better-auth objects are untouched. This makes the contradiction impossible to reintroduce — even if a schema re-adds a write verb, the effective exposed set is corrected at registration. Generalizing to other `managedBy` buckets (`externallyManaged`/`writeVia`) stays with #1878. Proven: registry unit tests for the derivation; the identity-create dogfood suite updated to assert `POST /data/sys_team` now returns 405 (OBJECT_API_METHOD_NOT_ALLOWED) at the exposure gate, before the engine's 403 backstop — system-context inserts still succeed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GwY68hKVysP98BX7i5eUoE * test(objectql): drop unused RESERVED_NAMESPACES import (CodeQL) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GwY68hKVysP98BX7i5eUoE --------- Co-authored-by: Claude <noreply@anthropic.com>
diff --git a/packages/objectql/src/registry.test.ts b/packages/objectql/src/registry.test.ts
@@ -1,5 +1,5 @@
-import { describe, it, expect, beforeEach } from 'vitest';
-import { SchemaRegistry, applySystemFields, computeFQN, parseFQN, RESERVED_NAMESPACES } from './registry';
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { SchemaRegistry, applySystemFields, reconcileManagedApiMethods, computeFQN, parseFQN } from './registry';
 
 describe('SchemaRegistry', () => {
     let registry: SchemaRegistry;
@@ -721,3 +721,75 @@ describe('applySystemFields', () => {
         expect(stored.fields.updated_by).toMatchObject({ system: true, readonly: true });
     });
 });
+
+// ==========================================
+// reconcileManagedApiMethods — #1591 / ADR-0092
+// Registration-time consistency: a better-auth-managed object may not
+// advertise generic write verbs it doesn't grant.
+// ==========================================
+describe('reconcileManagedApiMethods', () => {
+    const managed = (extra: any = {}): any => ({
+        name: 'sys_thing',
+        managedBy: 'better-auth',
+        enable: { apiEnabled: true, apiMethods: ['get', 'list', 'create', 'update', 'delete'] },
+        ...extra,
+    });
+
+    it('strips create/update/delete from a better-auth object with no write affordances', () => {
+        const warn = vi.fn();
+        const out = reconcileManagedApiMethods(managed(), { warn });
+        expect(out).not.toBe(managed()); // new object
+        expect(out.enable.apiMethods).toEqual(['get', 'list']);
+        // Warning names the object and the stripped verbs.
+        expect(warn).toHaveBeenCalledTimes(1);
+        expect(warn.mock.calls[0][0]).toContain('sys_thing');
+        expect(warn.mock.calls[0][0]).toContain('create');
+    });
+
+    it('keeps update when userActions.edit grants the edit affordance (sys_user case)', () => {
+        const warn = vi.fn();
+        const out = reconcileManagedApiMethods(
+            managed({ userActions: { edit: true }, enable: { apiEnabled: true, apiMethods: ['get', 'list', 'create', 'update', 'delete'] } }),
+            { warn },
+        );
+        // update survives (edit affordance granted); create/delete still stripped.
+        expect(out.enable.apiMethods).toEqual(['get', 'list', 'update']);
+        expect(warn).toHaveBeenCalledTimes(1);
+    });
+
+    it('is a no-op (same reference, no warning) when nothing needs stripping', () => {
+        const warn = vi.fn();
+        const already: any = managed({ enable: { apiEnabled: true, apiMethods: ['get', 'list'] } });
+        const out = reconcileManagedApiMethods(already, { warn });
+        expect(out).toBe(already);
+        expect(warn).not.toHaveBeenCalled();
+    });
+
+    it('never touches read verbs', () => {
+        const out = reconcileManagedApiMethods(
+            managed({ enable: { apiEnabled: true, apiMethods: ['get', 'delete'] } }),
+            { warn: vi.fn() },
+        );
+        expect(out.enable.apiMethods).toEqual(['get']);
+    });
+
+    it('leaves non-better-auth objects untouched (platform bucket keeps full CRUD)', () => {
+        const warn = vi.fn();
+        const platform: any = {
+            name: 'sys_business_unit',
+            managedBy: 'platform',
+            enable: { apiEnabled: true, apiMethods: ['get', 'list', 'create', 'update', 'delete'] },
+        };
+        const out = reconcileManagedApiMethods(platform, { warn });
+        expect(out).toBe(platform);
+        expect(out.enable.apiMethods).toEqual(['get', 'list', 'create', 'update', 'delete']);
+        expect(warn).not.toHaveBeenCalled();
+    });
+
+    it('applies on registerObject (the contradiction cannot be stored)', () => {
+        const reg = new SchemaRegistry({ multiTenant: false });
+        reg.registerObject(managed(), 'sys', 'sys', 'own');
+        const stored = (reg as any).objectContributors.get('sys_thing')[0].definition;
+        expect(stored.enable.apiMethods).toEqual(['get', 'list']);
+    });
+});
diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts
@@ -1,6 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
-import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary } from '@objectstack/spec/data';
+import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveCrudAffordances } from '@objectstack/spec/data';
 import { resolveMultiOrgEnabled, resolveSearchPinyinEnabled } from '@objectstack/types';
 import { provisionSearchCompanion } from './search-companion.js';
 import { ObjectStackManifest, ManifestSchema, InstalledPackage, InstalledPackageSchema } from '@objectstack/spec/kernel';
@@ -367,6 +367,84 @@ export function applySystemFields(
   };
 }
 
+/**
+ * Generic-write `apiMethods` verbs mapped to the {@link resolveCrudAffordances}
+ * flag each one needs. Read verbs (`get`/`list`/`search`/`history`/…) are
+ * always permitted, so they are absent here and never stripped.
+ */
+const MANAGED_WRITE_VERB_AFFORDANCE: Record<string, 'create' | 'edit' | 'delete'> = {
+  create: 'create',
+  update: 'edit',
+  upsert: 'edit',
+  delete: 'delete',
+  purge: 'delete',
+};
+
+/**
+ * Reconcile a better-auth-managed object's `enable.apiMethods` against the
+ * generic-write affordances it actually grants (ADR-0092 / #1591).
+ *
+ * `managedBy: 'better-auth'` promises generic CRUD is suppressed — identity
+ * writes flow through better-auth's own endpoints, and the plugin-auth
+ * identity write guard fail-closed rejects direct create/update/delete from a
+ * user context. An object that *also* advertises those verbs in
+ * `enable.apiMethods` is internally contradictory: the HTTP exposure gate
+ * (ADR-0049) would admit the request and let it 403 at the engine instead of
+ * answering a clean 405, and the metadata misrepresents what the API offers.
+ *
+ * This is the registration-time backstop that makes the contradiction
+ * impossible to *ship*: any write verb whose CRUD affordance the object does
+ * not grant — via the `managedBy` bucket default plus `userActions` overrides,
+ * exactly as {@link resolveCrudAffordances} computes for the UI — is stripped,
+ * with a warning. Reads are never touched. `sys_user` keeps `update` because
+ * `userActions.edit: true` grants the edit affordance (its writes are clamped
+ * to a field whitelist by the guard); every other identity table derives down
+ * to reads only.
+ *
+ * Scope is deliberately limited to `better-auth`, the only bucket the write
+ * guard enforces today — generalizing to an `externallyManaged`/`writeVia`
+ * capability for the other buckets is ADR-0049 / #1878 (a separate phase),
+ * not this reconciliation.
+ *
+ * Returns the input unchanged when nothing is stripped; otherwise a new schema
+ * with a rewritten `enable.apiMethods` (immutable, like {@link applySystemFields}).
+ */
+export function reconcileManagedApiMethods(
+  schema: ServiceObject,
+  opts?: { warn?: (msg: string) => void },
+): ServiceObject {
+  if ((schema as any).managedBy !== 'better-auth') return schema;
+
+  const methods = (schema as any).enable?.apiMethods;
+  if (!Array.isArray(methods) || methods.length === 0) return schema;
+
+  const affordances = resolveCrudAffordances(schema);
+  const stripped: string[] = [];
+  const kept = methods.filter((m: string) => {
+    const need = MANAGED_WRITE_VERB_AFFORDANCE[m];
+    if (need && !affordances[need]) {
+      stripped.push(m);
+      return false;
+    }
+    return true;
+  });
+
+  if (stripped.length === 0) return schema;
+
+  const warn = opts?.warn ?? ((msg: string) => console.warn(msg));
+  warn(
+    `[Registry] Object "${schema.name}" is managedBy:'better-auth' but advertised ` +
+      `generic write verb(s) [${stripped.join(', ')}] in enable.apiMethods it does not ` +
+      `permit — stripping them (ADR-0092/#1591). Writes on better-auth-managed tables go ` +
+      `through better-auth's endpoints, not the generic data API. Kept: [${kept.join(', ')}].`,
+  );
+
+  return {
+    ...schema,
+    enable: { ...(schema as any).enable, apiMethods: kept },
+  };
+}
+
 /**
  * Platform namespaces that multiple packages may legitimately share, so the
  * install-time namespace-uniqueness gate (ADR-0048 Phase 1) must never fire on
@@ -590,6 +668,13 @@ export class SchemaRegistry {
     // applySystemFields().
     schema = applySystemFields(schema, { multiTenant: this.multiTenant });
 
+    // [ADR-0092 / #1591] Reconcile generic-write `apiMethods` against the CRUD
+    // affordances a better-auth-managed object actually grants — strip verbs
+    // the identity write guard would reject anyway, so the HTTP exposure gate
+    // answers a clean 405 and the metadata can't contradict itself. No-op for
+    // every non-`better-auth` object.
+    schema = reconcileManagedApiMethods(schema);
+
     // [ADR-0079] Object-materialization seam — DESIGNATE-ONLY primary-title
     // provisioning. Runs AFTER `applySystemFields` (so any designated field
     // co-exists with the injected system columns) and ONLY for owned objects
diff --git a/packages/platform-objects/src/identity/sys-account.object.ts b/packages/platform-objects/src/identity/sys-account.object.ts
@@ -214,7 +214,9 @@ export const SysAccount = ObjectSchema.create({
     trackHistory: false,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'update', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get', 'list'],
     trash: true,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-api-key.object.ts b/packages/platform-objects/src/identity/sys-api-key.object.ts
@@ -214,7 +214,9 @@ export const SysApiKey = ObjectSchema.create({
     trackHistory: true,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'update', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get', 'list'],
     trash: false,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-device-code.object.ts b/packages/platform-objects/src/identity/sys-device-code.object.ts
@@ -139,7 +139,9 @@ export const SysDeviceCode = ObjectSchema.create({
     trackHistory: false,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'create', 'update', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get'],
     trash: false,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-invitation.object.ts b/packages/platform-objects/src/identity/sys-invitation.object.ts
@@ -242,7 +242,9 @@ export const SysInvitation = ObjectSchema.create({
     trackHistory: true,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'update', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get', 'list'],
     trash: false,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-member.object.ts b/packages/platform-objects/src/identity/sys-member.object.ts
@@ -187,7 +187,9 @@ export const SysMember = ObjectSchema.create({
     trackHistory: true,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'update', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get', 'list'],
     trash: false,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-organization.object.ts b/packages/platform-objects/src/identity/sys-organization.object.ts
@@ -247,7 +247,9 @@ export const SysOrganization = ObjectSchema.create({
     trackHistory: true,
     searchable: true,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'update', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get', 'list'],
     trash: true,
     mru: true,
   },
diff --git a/packages/platform-objects/src/identity/sys-session.object.ts b/packages/platform-objects/src/identity/sys-session.object.ts
@@ -210,7 +210,9 @@ export const SysSession = ObjectSchema.create({
     trackHistory: false,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get', 'list'],
     trash: false,
     mru: false,
     clone: false,
diff --git a/packages/platform-objects/src/identity/sys-team-member.object.ts b/packages/platform-objects/src/identity/sys-team-member.object.ts
@@ -110,7 +110,9 @@ export const SysTeamMember = ObjectSchema.create({
     trackHistory: true,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get', 'list'],
     trash: false,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-team.object.ts b/packages/platform-objects/src/identity/sys-team.object.ts
@@ -171,7 +171,12 @@ export const SysTeam = ObjectSchema.create({
     trackHistory: true,
     searchable: true,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'update', 'delete'],
+    // Generic writes are refused by the plugin-auth identity write guard
+    // (ADR-0092 D2) and better-auth owns create/update/delete via the
+    // team actions above — so the generic data API exposes reads only.
+    // The HTTP layer now answers 405 (api-exposure) before the engine's
+    // 403 backstop. See #1591.
+    apiMethods: ['get', 'list'],
     trash: true,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-two-factor.object.ts b/packages/platform-objects/src/identity/sys-two-factor.object.ts
@@ -174,7 +174,9 @@ export const SysTwoFactor = ObjectSchema.create({
     trackHistory: false,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'create', 'update', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get'],
     trash: false,
     mru: false,
   },
diff --git a/packages/platform-objects/src/identity/sys-user.object.ts b/packages/platform-objects/src/identity/sys-user.object.ts
@@ -749,7 +749,13 @@ export const SysUser = ObjectSchema.create({
     trackHistory: true,
     searchable: true,
     apiEnabled: true,
-    apiMethods: ['get', 'list', 'create', 'update', 'delete'],
+    // #1591 — create/delete are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth (Invite / Create User / admin
+    // actions), so they are not exposed. `update` stays: it is the ONE
+    // generic write opened on an identity table (ADR-0092 D4), server-side
+    // clamped to the profile-field whitelist ({name, image}) by the guard —
+    // `userActions.edit: true` above declares the affordance.
+    apiMethods: ['get', 'list', 'update'],
     trash: true,
     mru: true,
   },
diff --git a/packages/platform-objects/src/identity/sys-verification.object.ts b/packages/platform-objects/src/identity/sys-verification.object.ts
@@ -88,7 +88,9 @@ export const SysVerification = ObjectSchema.create({
     trackHistory: false,
     searchable: false,
     apiEnabled: true,
-    apiMethods: ['get', 'create', 'delete'],
+    // #1591 — reads only: writes are refused by the identity write guard
+    // (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
+    apiMethods: ['get'],
     trash: false,
     mru: false,
   },
diff --git a/packages/qa/dogfood/test/single-tenant-identity-create.dogfood.test.ts b/packages/qa/dogfood/test/single-tenant-identity-create.dogfood.test.ts
@@ -9,14 +9,16 @@
  * 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.
+ * data-API insert is now REJECTED fail-closed for user contexts. As of #1591
+ * `sys_team.enable.apiMethods` no longer advertises `create`, so the rejection
+ * lands at the HTTP exposure gate (ADR-0049) as a clean 405 — before the
+ * engine's identity-write-guard 403 backstop even runs. 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';
@@ -42,14 +44,16 @@ describe('ADR-0057: org-scoped identity creatable single-tenant', () => {
   });
 
   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).
+    // #1591 — sys_team is managedBy:'better-auth' and no longer advertises
+    // `create` in enable.apiMethods, so a USER-context insert through the
+    // generic data API is refused at the HTTP exposure gate (ADR-0049) with a
+    // clean 405, before the engine's identity-write-guard 403 backstop runs.
+    // 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);
+    expect(direct.status).toBe(405);
     const denied: any = await direct.json();
-    expect(denied.code).toBe('PERMISSION_DENIED');
+    expect(denied.code).toBe('OBJECT_API_METHOD_NOT_ALLOWED');
 
     // 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
