Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions packages/objectql/src/registry.test.ts
Original file line number Diff line number Diff line change
@@ -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, RESERVED_NAMESPACES } from './registry';

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import RESERVED_NAMESPACES.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed

describe('SchemaRegistry', () => {
let registry: SchemaRegistry;
Expand Down Expand Up @@ -721,3 +721,75 @@
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']);
});
});
87 changes: 86 additions & 1 deletion packages/objectql/src/registry.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion packages/platform-objects/src/identity/sys-account.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
4 changes: 3 additions & 1 deletion packages/platform-objects/src/identity/sys-api-key.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
4 changes: 3 additions & 1 deletion packages/platform-objects/src/identity/sys-member.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
4 changes: 3 additions & 1 deletion packages/platform-objects/src/identity/sys-session.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
7 changes: 6 additions & 1 deletion packages/platform-objects/src/identity/sys-team.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
8 changes: 7 additions & 1 deletion packages/platform-objects/src/identity/sys-user.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand Down