From 20eaff7ea71e6da852fae17461e5b825773ff600 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 22:51:55 +0000 Subject: [PATCH 01/18] wip(objectql,plugin-auth): organization delete refuses while records remain Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- packages/objectql/src/engine.ts | 151 +++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 102 ++++++++- .../plugin-auth/src/organization-delete.ts | 202 ++++++++++++++++++ 3 files changed, 454 insertions(+), 1 deletion(-) create mode 100644 packages/plugins/plugin-auth/src/organization-delete.ts diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index b857b0a3d3e..b73dee98d7b 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -19498,6 +19498,11 @@ export class ObjectQL implements IObjectQLEngine { ): Promise { // Entered before the first nested `delete()` can come back here. cascadeSetAdd(cascadeSet.entered, object, id); + // [#24948] An organization that still owns rows is refused here, whole, + // before the first relation below is cascaded or cleared. + if (object === ORGANIZATION_OBJECT) { + await this.refuseOrganizationDeleteWhileOwned(id, context, objects, cascadeSet); + } for (const child of objects) { const childName = (child as any)?.name as string | undefined; const fields = (child as any)?.fields as Record | undefined; @@ -19505,6 +19510,10 @@ export class ObjectQL implements IObjectQLEngine { for (const [fieldName, fdef] of Object.entries(fields)) { const resolvedBehavior = this.cascadeRelationBehavior(object, child, fieldName, fdef); if (resolvedBehavior === undefined) continue; + // [#24948] Judged by `refuseOrganizationDeleteWhileOwned` above, which + // found no row outside the cascade set: there is nothing to clear, and + // clearing is not on offer for this column anyway. + if (this.isTenantColumnSetNull(object, child, fieldName, resolvedBehavior)) continue; let behavior: string = resolvedBehavior; // [#9689] (maintainer ruling 2026-08-19, Q3 = B): the judgement the @@ -19856,6 +19865,148 @@ export class ObjectQL implements IObjectQLEngine { } } + /** + * [#24948] Is `fieldName` on `child` the TENANT column of `child`, pointing at + * the organization being deleted with the `set_null` behaviour? + * + * The tenant column is the one `resolveTenantFieldName` names: the column + * the engine stamps and scopes by, whether the registry injected it + * (`TENANT_SCOPE_FIELD_DEF`) or the object declared its own, as the identity + * tables do. Every such lookup resolves to `set_null` unless its author wrote + * something else, and ADR-0131 D1 leaves NULL nothing to mean: a cleared + * tenant column is an orphan, and on a store whose column is already NOT + * NULL the clear is a constraint failure halfway through a delete. So the + * engine never clears one. {@link refuseOrganizationDeleteWhileOwned} refuses + * the delete instead. An authored `cascade` or `restrict` keeps its own + * meaning. + */ + private isTenantColumnSetNull( + object: string, + child: ServiceObject, + fieldName: string, + behavior: string | undefined, + ): boolean { + return ( + object === ORGANIZATION_OBJECT && + behavior === 'set_null' && + resolveTenantFieldName(child) === fieldName + ); + } + + /** + * [#24948] The engine half of organization delete: refuse while any row + * outside the cascade set still names the organization in its tenant column. + * + * Runs before the walk touches the first relation, and reads every tenant + * relation before it throws, so the one refusal names every table that holds + * rows, each with its count. It is the safety net under plugin-auth's own + * refusal, which answers the same question before better-auth deletes the + * organization's members: whatever path reaches a `sys_organization` delete, + * no row is orphaned to NULL, on a store born with a nullable tenant column + * or anywhere else. + * + * The probe is the walk's own ({@link probeReferencingRows}: system + * identity, its elevation filed first, a missing table read as no rows). A + * count reaches the refusal only when the caller could see every row it + * counts ({@link dependentCountIsDisclosable}); otherwise the table is named + * without one. The wire code is the standard catalog's `DELETE_RESTRICTED`, + * because that is the condition: a delete refused while dependents exist. + */ + private async refuseOrganizationDeleteWhileOwned( + id: string | number, + context: ExecutionContext | undefined, + objects: ServiceObject[], + cascadeSet: CascadeDeleteSet, + ): Promise { + const owned: Array<{ object: string; count?: number }> = []; + for (const child of objects) { + const childName = (child as any)?.name as string | undefined; + const fields = (child as any)?.fields as Record | undefined; + if (!childName || !fields) continue; + for (const [fieldName, fdef] of Object.entries(fields)) { + const behavior = this.cascadeRelationBehavior(ORGANIZATION_OBJECT, child, fieldName, fdef); + if (!this.isTenantColumnSetNull(ORGANIZATION_OBJECT, child, fieldName, behavior)) continue; + const probeWhere = this.referenceProbeFilter(fieldName, fdef, id); + this.fileReferenceCheckElevation(cascadeSet, ORGANIZATION_OBJECT, id, childName, fieldName, context); + const probed = await this.probeReferencingRows(childName, fieldName, fdef, id, probeWhere, context); + const outside = probed.filter((row) => !cascadeSetHas(cascadeSet.members, childName, row?.id)); + if (outside.length === 0) continue; + const disclose = await this.dependentCountIsDisclosable( + childName, probeWhere, probed.map((row: any) => String(row?.id)), fieldName, false, id, context, + ); + owned.push(disclose ? { object: childName, count: outside.length } : { object: childName }); + } + } + if (owned.length === 0) return; + const named = owned + .map((o) => (o.count === undefined ? o.object : `${o.object} (${o.count})`)) + .join(', '); + const err: any = new Error( + `Cannot delete this organization: it still owns records in ${named}. ` + + 'Delete or move those records first, then delete the organization.', + ); + err.developerMessage = + `Cannot delete ${ORGANIZATION_OBJECT} (${id}): rows still reference it through their tenant column ` + + `in ${named}. The engine never clears a tenant column (ADR-0131 D1), so the delete is refused ` + + 'until those rows are deleted or moved to another organization.'; + err.code = 'DELETE_RESTRICTED'; + err.status = 409; + err.httpStatus = 409; + err.object = ORGANIZATION_OBJECT; + // The first table in the `DELETE_RESTRICTED` envelope's own two keys, and + // every table in `dependents`. + err.dependentObject = owned[0].object; + if (owned[0].count !== undefined) err.dependentCount = owned[0].count; + err.dependents = owned; + throw err; + } + + /** + * [#24948] How many rows each object holds for `organizationId` in its tenant + * column: the question plugin-auth's organization delete asks before + * anything is deleted. Only objects holding at least one row are listed. + * + * The relations read are the ones {@link refuseOrganizationDeleteWhileOwned} + * judges, through the same reader ({@link cascadeRelationBehavior}: a + * federated object's unprovisioned injected column is no relation), widened + * to every behaviour: a tenant column its author declared `restrict` blocks + * the delete too, and one declared `cascade` would delete records, which is + * not the organization's own structure either. A count, never a row: the + * read is a system-identity `count`, and a table never provisioned holds no + * row. + * + * `exclude` names the objects the caller removes as part of the delete + * itself. + */ + async organizationOwnedRowCounts( + organizationId: string, + options?: { exclude?: Iterable; context?: ExecutionContext }, + ): Promise> { + const exclude = new Set(options?.exclude ?? []); + const counts: Array<{ object: string; count: number }> = []; + for (const child of this._registry.getAllObjects()) { + const childName = (child as any)?.name as string | undefined; + const fields = (child as any)?.fields as Record | undefined; + if (!childName || !fields || exclude.has(childName)) continue; + const tenantField = resolveTenantFieldName(child); + if (!tenantField || !fields[tenantField]) continue; + const fdef = fields[tenantField]; + if (this.cascadeRelationBehavior(ORGANIZATION_OBJECT, child, tenantField, fdef) === undefined) continue; + let count: number; + try { + count = await this.count(childName, { + where: this.referenceProbeFilter(tenantField, fdef, organizationId), + context: ObjectQL.referenceCheckContext(options?.context), + }); + } catch (error) { + if (isMissingTableError(error, childName)) continue; + throw error; + } + if (count > 0) counts.push({ object: childName, count }); + } + return counts; + } + async delete(object: string, options?: EngineDeleteOptions): Promise { object = this.resolveObjectName(object); // [#22782] An aborting `afterDelete` rolls this write back — see `opensAfterAbortUnit`. diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 87392c6086b..37bce1d5d84 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -209,6 +209,13 @@ import { retireHeldKeyrings, UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS, } from './signing-key-read-cache.js'; +import { + ORGANIZATION_DELETE_PATH, + isDefaultOrganizationTarget, + organizationDeleteRefusal, + organizationNotEmptyBody, + removeOrganizationTeams, +} from './organization-delete.js'; /** * Detect WebContainer (StackBlitz) environment. @@ -2424,6 +2431,16 @@ export class AuthManager { // fall through — the vendor still re-decides everything it owns } + // ── #24948: organization delete refuses while records remain ── + // Before the vendor's handler, so a refused delete writes nothing: + // the vendor clears the caller's active organization and deletes the + // members before its own `beforeDeleteOrganization` hook runs. + // `organization-delete.ts` carries the contract. + if (ctx?.path === ORGANIZATION_DELETE_PATH) { + await this.assertOrganizationDeletable(ctx); + // fall through — the vendor still authorizes and deletes + } + // ── #9714: a revoke that identifies NO record must not report ── // success. better-auth 1.7.1's `revoke-session` handler skips the // delete when the token matches zero rows (or another user's row) @@ -3854,6 +3871,20 @@ export class AuthManager { }); } }, + // [#24948] The teams and their members are the organization's own + // auth structure, and better-auth's delete leaves them behind. They + // go here, inside the request's transaction (`handleRequest`), after + // the `hooks.before` refusal has found no records left. The Default + // Organization is the last-administrator guard's to answer, so its + // teams are not touched: when that guard admits the delete, the + // engine's own refusal still holds while anything names it. + beforeDeleteOrganization: async ({ organization }: any) => { + const engine = this.getDataEngine(); + const organizationId = organization?.id; + if (!engine || typeof organizationId !== 'string' || !organizationId) return; + if (await isDefaultOrganizationTarget(engine, organization)) return; + await removeOrganizationTeams(engine, organizationId); + }, }, // No mailer is wired in framework yet — log the accept URL so // operators / UI can fall back to copy-paste flows. Replace this @@ -6674,8 +6705,14 @@ export class AuthManager { return scimRequestScope.run({ scim: true }, runHandler); } : runHandler; + // [#24948] An organization delete is one unit of work on the same terms: + // better-auth clears the caller's active organization and deletes the + // members and invitations before the organization row, with no + // transaction, and the engine's refusal on that last row used to leave the + // organization standing with no owner. const vendorResponse = - endpointPath !== undefined && SESSION_ERASURE_PATHS.has(endpointPath) + endpointPath !== undefined && + (SESSION_ERASURE_PATHS.has(endpointPath) || endpointPath === ORGANIZATION_DELETE_PATH) ? await this.runSubjectErasureAtomically(runRequest) : await runRequest(); @@ -6959,6 +6996,11 @@ export class AuthManager { * - side effects outside the datasource (a sent email, secondary-storage * session state) are not transactional and are not undone by a rollback. * `/admin/remove-user` sends nothing, so no path here relies on it. + * + * [#24948] `/organization/delete` runs here too, on the same terms and for + * the same reason: the vendor's member, invitation and organization deletes + * are separate adapter calls, and the teams removed by + * `beforeDeleteOrganization` must roll back with them. */ private async runSubjectErasureAtomically( run: () => Promise, @@ -8035,6 +8077,64 @@ export class AuthManager { * the same roles map we hand the org plugin, so this never becomes a second * spelling of the authorization question. */ + /** + * [#24948] `/organization/delete` admission: refuse 409 `DELETE_RESTRICTED`, + * naming every table and its row count, while the organization still owns + * records (`organization-delete.ts` carries the contract and the exclusion + * set). + * + * Only for a caller the vendor itself admits — a member whose role holds + * `organization: ['delete']`, asked of the vendor's own `hasPermission`, as + * {@link assertRemoveMemberPermitted} does — so a caller it refuses gets its + * refusal, and no count. Everything this cannot decide falls through to the + * vendor: no session, no organization id, no membership, a lookup that + * failed. The COUNT does not fall through: it is the guard, and a guard that + * could not be evaluated must not pass, so its failure propagates. The + * Default Organization stays the last-administrator guard's to answer. + */ + private async assertOrganizationDeletable(ctx: any): Promise { + const engine = this.getDataEngine(); + if (!engine) return; + const organizationId = typeof ctx?.body?.organizationId === 'string' ? ctx.body.organizationId : ''; + if (!organizationId) return; + + let organization: any; + try { + const actor = await this.resolveActor(ctx); + if (!actor?.userId) return; // the vendor answers 401 + const sys = withSystemReadContext(engine); + const callerRow: any = await sys.findOne('sys_member', { + where: { organization_id: organizationId, user_id: actor.userId }, + }); + if (!callerRow) return; // the vendor answers USER_IS_NOT_A_MEMBER_OF_THE_ORGANIZATION + const { hasPermission } = await import('better-auth/plugins/organization'); + const permitted = await hasPermission( + { + role: callerRow.role, + options: (this.orgRolesMap ? { roles: this.orgRolesMap } : {}) as any, + permissions: { organization: ['delete'] }, + organizationId, + } as any, + ctx, + ); + if (!permitted) return; // the vendor answers YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_ORGANIZATION + organization = await sys.findOne('sys_organization', { where: { id: organizationId } }); + if (!organization) return; // the vendor answers 400 + if (await isDefaultOrganizationTarget(engine, organization)) return; + } catch { + // A lookup that did not complete hands the request back to the vendor, + // which re-reads everything it decides; the engine's own refusal still + // stands under the delete. + return; + } + + const owned = await organizationDeleteRefusal(engine, organizationId); + if (owned && owned.length > 0) { + const { APIError } = await import('better-auth/api'); + throw new APIError('CONFLICT', organizationNotEmptyBody(owned)); + } + } + private async assertRemoveMemberPermitted(ctx: any): Promise { const engine = this.getDataEngine(); if (!engine) return; diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts new file mode 100644 index 00000000000..eecd662584f --- /dev/null +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -0,0 +1,202 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24948] Organization delete: refused while the organization still owns + * records, and otherwise one unit of work that removes the organization's own + * auth structure with it. + * + * ## What better-auth does, and what it leaves + * + * `POST /organization/delete` (better-auth 1.7.3, + * `plugins/organization/routes/crud-org.mjs`) authorizes the caller, clears the + * caller's active organization, calls `organizationHooks.beforeDeleteOrganization`, + * and then `adapter.deleteOrganization`, which deletes `member`, then + * `invitation`, then the organization row. It deletes no team, no team member + * and nothing it does not own. The engine's referential cascade then ran on + * the organization row and applied the tenant column's `set_null` default to + * every row the organization still owned: a 500 halfway through on a store + * whose tenant column is NOT NULL (members and invitations already gone, the + * organization left with no owner), or a 200 that orphaned the rows to NULL on + * a store born nullable. Every organization created through the door owns a + * default team, so on a NOT NULL store even an empty one could not be deleted. + * + * ## The contract (maintainer ruling on #24948, letter B) + * + * 1. **Refused while the organization owns records.** {@link + * organizationDeleteRefusal} counts, per table, the rows that name the + * organization in their tenant column, leaving out the organization's own + * auth structure. Any count above zero refuses with 409 + * `DELETE_RESTRICTED` and `dependents: [{ object, count }]`: counts, never a + * row value. It runs in `hooks.before`, ahead of the vendor's handler, and + * only for a caller the vendor itself would admit (a member holding the + * `organization: ['delete']` permission), so a refused delete writes + * nothing at all — not even the caller's active organization — and a caller + * the vendor would refuse learns no count. + * 2. **The organization's own auth structure goes with it.** That is + * {@link ORGANIZATION_AUTH_STRUCTURE}: members and invitations (better-auth + * deletes them), teams and team members ({@link removeOrganizationTeams}, + * from `beforeDeleteOrganization`), and the organization-admin grants its + * owners and admins hold (plugin-security's member-delete hook revokes them + * when better-auth deletes the members). Any other grant the organization + * holds is counted as a record. + * 3. **One unit of work.** `AuthManager.handleRequest` runs the whole request + * inside one engine transaction and rolls it back on any 4xx/5xx answer, the + * subject-erasure wrapper's posture. On a driver without transactions the + * refusal above is the guarantee, since it runs before the first write. + * 4. **The engine never clears the tenant column.** Under all of this, + * `ObjectQL.cascadeDeleteRelations` refuses a `sys_organization` delete + * while any row still names it, whatever path reaches it. + * + * The Default Organization is not judged here: the last-administrator guard + * keeps answering for it exactly as before ({@link isDefaultOrganizationTarget}). + */ + +import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { GRANT_SET_NAME_FIELD } from './grant-set-name.js'; + +/** The better-auth endpoint this module guards. */ +export const ORGANIZATION_DELETE_PATH = '/organization/delete'; + +/** The grant table; partly the organization's own structure (see {@link ORGANIZATION_AUTH_STRUCTURE}). */ +const GRANT_OBJECT = 'sys_user_permission_set'; + +/** + * The objects an organization delete removes as the organization's OWN auth + * structure, and which therefore are not records it "still owns". Each is + * removed by the delete itself: + * + * - `sys_member`, `sys_invitation`: better-auth's `adapter.deleteOrganization`; + * - `sys_team`, `sys_team_member`: {@link removeOrganizationTeams}; + * - `sys_user_permission_set`, ONLY the grants of the organization-admin sets + * ({@link ORGANIZATION_ADMIN_SET_NAMES}): plugin-security's member-delete + * hook revokes the grant a membership grade conferred once the membership is + * gone. Every other grant scoped to the organization is counted. + */ +export const ORGANIZATION_AUTH_STRUCTURE: readonly string[] = [ + 'sys_member', + 'sys_invitation', + 'sys_team', + 'sys_team_member', + GRANT_OBJECT, +]; + +/** The permission sets a membership grade grants, by name (ADR-0131 D4). */ +export const ORGANIZATION_ADMIN_SET_NAMES: readonly string[] = [ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS]; + +/** One table the organization still owns rows in, and how many. */ +export interface OrganizationOwnedRows { + object: string; + count: number; +} + +/** The engine reads this module needs; `organizationOwnedRowCounts` is ObjectQL's. */ +type CountingEngine = IDataEngine & { + organizationOwnedRowCounts?: ( + organizationId: string, + options?: { exclude?: Iterable }, + ) => Promise; +}; + +const SYSTEM_CTX = { isSystem: true } as const; + +function rowsOf(result: unknown): any[] { + if (Array.isArray(result)) return result; + const records = (result as { records?: unknown } | null | undefined)?.records; + return Array.isArray(records) ? records : []; +} + +/** + * The tables in which `organizationId` still owns records, each with its row + * count; empty when the organization holds nothing but its own auth + * structure. `undefined` when the engine cannot answer the question (an engine + * that is not ObjectQL): the caller then leaves the decision to the engine's + * own cascade, which refuses on its own. + */ +export async function organizationDeleteRefusal( + engine: IDataEngine, + organizationId: string, +): Promise { + const counting = engine as CountingEngine; + if (typeof counting.organizationOwnedRowCounts !== 'function') return undefined; + const owned = await counting.organizationOwnedRowCounts(organizationId, { exclude: ORGANIZATION_AUTH_STRUCTURE }); + + // The grants: every grant scoped to the organization, less those of the + // organization-admin sets. Two counts rather than one `$nin`, so a grant + // whose set name is NULL (written before the column existed) is counted + // too: a NULL never matches `NOT IN`. + const tenantGrants = await engine.count(GRANT_OBJECT, { + where: { organization_id: organizationId }, + context: SYSTEM_CTX, + } as any); + if (tenantGrants > 0) { + const adminGrants = await engine.count(GRANT_OBJECT, { + where: { organization_id: organizationId, [GRANT_SET_NAME_FIELD]: { $in: [...ORGANIZATION_ADMIN_SET_NAMES] } }, + context: SYSTEM_CTX, + } as any); + const others = tenantGrants - adminGrants; + if (others > 0) owned.push({ object: GRANT_OBJECT, count: others }); + } + return owned; +} + +/** + * The 409 body for a refused delete: the standard catalog's + * `DELETE_RESTRICTED`, every table with its count, and the remedy. + */ +export function organizationNotEmptyBody(owned: readonly OrganizationOwnedRows[]): { + message: string; + code: 'DELETE_RESTRICTED'; + dependents: OrganizationOwnedRows[]; +} { + const named = owned.map((o) => `${o.object} (${o.count})`).join(', '); + return { + message: + `This organization still owns records in ${named}. ` + + 'Delete or move those records first, then delete the organization.', + code: 'DELETE_RESTRICTED', + dependents: owned.map((o) => ({ object: o.object, count: o.count })), + }; +} + +/** + * Is `organizationId` the Default Organization: the one with `slug = 'default'`, + * else the deployment's only organization? The last-administrator guard's own + * reading of the anchor (`last-admin-guard.ts`). Its delete is that guard's to + * answer, as before, so the refusal above does not run for it. + */ +export async function isDefaultOrganizationTarget( + engine: IDataEngine, + organization: { id?: unknown; slug?: unknown } | undefined, +): Promise { + if (!organization) return false; + if (organization.slug === 'default') return true; + const total = await engine.count('sys_organization', { context: SYSTEM_CTX } as any); + return total <= 1; +} + +/** + * Delete the organization's teams and their members: the part of its auth + * structure better-auth leaves behind. Called from `beforeDeleteOrganization`, + * inside the request's transaction, so a later failure rolls it back. + * + * Team members first, by their team: `sys_team_member` reaches the + * organization only through `team_id`, and a multi-row delete runs no cascade. + * Then each team by id, so the engine's cascade clears what still points at it + * (a session's active team). + */ +export async function removeOrganizationTeams(engine: IDataEngine, organizationId: string): Promise { + const teams = rowsOf( + await engine.find('sys_team', { where: { organization_id: organizationId }, fields: ['id'], context: SYSTEM_CTX } as any), + ); + const teamIds = teams.map((t) => t?.id).filter((id): id is string | number => id != null); + if (teamIds.length === 0) return; + await engine.delete('sys_team_member', { + where: { team_id: { $in: teamIds } }, + multi: true, + context: SYSTEM_CTX, + } as any); + for (const id of teamIds) { + await engine.delete('sys_team', { where: { id }, context: SYSTEM_CTX } as any); + } +} From de6b997e266f541b0e1292315b60d87aa746a9d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:11:28 +0000 Subject: [PATCH 02/18] wip: dogfood pins, org-admin grants removed with the organization, posture-aware Default Organization reading Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- ...engine-organization-delete-refusal.test.ts | 262 ++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 25 +- .../src/organization-delete.test.ts | 177 ++++++++++++ .../plugin-auth/src/organization-delete.ts | 64 +++-- .../org-record-admin-reach.dogfood.test.ts | 25 ++ ...n-delete-federated-fixture.dogfood.test.ts | 56 +++- ...ion-delete-refuse-nonempty.dogfood.test.ts | 223 +++++++++++++++ 7 files changed, 793 insertions(+), 39 deletions(-) create mode 100644 packages/objectql/src/engine-organization-delete-refusal.test.ts create mode 100644 packages/plugins/plugin-auth/src/organization-delete.test.ts create mode 100644 packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts diff --git a/packages/objectql/src/engine-organization-delete-refusal.test.ts b/packages/objectql/src/engine-organization-delete-refusal.test.ts new file mode 100644 index 00000000000..c10368f262b --- /dev/null +++ b/packages/objectql/src/engine-organization-delete-refusal.test.ts @@ -0,0 +1,262 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24948] The engine never clears a tenant column: deleting a + * `sys_organization` row that rows still name in their tenant column is + * refused, whole, before the cascade writes anything. + * + * Before, the tenant column's `set_null` default ran on every such row. On a + * store whose column is NOT NULL that was a constraint failure in the middle + * of the delete; on a store born nullable it orphaned the rows to NULL. Here + * the same delete answers 409 `DELETE_RESTRICTED` naming every table with its + * count, and every row is as it was. The door half (plugin-auth's refusal + * before better-auth deletes the members) is pinned in + * `packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +type Row = Record; + +const PACKAGE_ID = 'test-24948'; +const ORG_ID = 'org_24948'; +const OTHER_ORG = 'org_other'; + +/** An in-memory driver that records every write, so "nothing changed" is observable. */ +function makeDriver() { + const tables: Record = {}; + const writes: string[] = []; + const missing = new Set(); + const rowsOf = (o: string): Row[] => (tables[o] ??= []); + const matches = (row: Row, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + const exp = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; + if ((row[k] ?? null) !== (exp ?? null)) return false; + } + return true; + }; + const missingTable = (o: string) => + Object.assign(new Error(`SQLITE_ERROR: no such table: ${o}`), { code: 'SQLITE_ERROR' }); + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async syncSchema() {}, + async find(o: string, ast: any) { + if (missing.has(o)) throw missingTable(o); + const hit = rowsOf(o).filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' ? hit.slice(0, ast.limit) : hit; + }, + async findOne(o: string, ast: any) { + const [first] = await this.find(o, { ...ast, limit: 1 }); + return first ?? null; + }, + async count(o: string, ast: any) { + return (await this.find(o, { where: ast?.where })).length; + }, + async create(o: string, data: Row) { + const row = { ...data, id: String(data.id) }; + rowsOf(o).push(row); + writes.push(`create ${o}`); + return row; + }, + async update(o: string, id: string, data: Row) { + const rows = rowsOf(o); + const at = rows.findIndex((r) => r.id === String(id)); + if (at < 0) throw new Error(`not found ${o}/${id}`); + rows[at] = { ...rows[at], ...data, id: String(id) }; + writes.push(`update ${o}/${id}`); + return rows[at]; + }, + async upsert(o: string, data: Row) { return this.create(o, data); }, + async delete(o: string, id: string) { + const rows = rowsOf(o); + const at = rows.findIndex((r) => r.id === String(id)); + if (at < 0) return false; + rows.splice(at, 1); + writes.push(`delete ${o}/${id}`); + return true; + }, + async bulkCreate() { return []; }, async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { id: 'trx' }; }, + async commit() {}, async rollback() {}, + }; + return { + driver, + writes, + missing, + rows: (o: string) => rowsOf(o).map((r) => ({ ...r })), + seed: (o: string, row: Row) => void rowsOf(o).push(row), + }; +} + +const ORGANIZATION = { + name: 'sys_organization', + label: 'Organization', + fields: { name: { name: 'name', label: 'Name', type: 'text' as const } }, +}; + +/** Business objects: the registry injects the tenant column `organization_id`. */ +const ACCOUNT = { + name: 'acct', + label: 'Account', + fields: { name: { name: 'name', label: 'Name', type: 'text' as const } }, +}; +const CONTACT = { + name: 'contact', + label: 'Contact', + fields: { name: { name: 'name', label: 'Name', type: 'text' as const } }, +}; + +/** The identity tables' shape: the object DECLARES its own tenant column, `set_null` written out. */ +const TEAM = { + name: 'team', + label: 'Team', + fields: { + name: { name: 'name', label: 'Name', type: 'text' as const }, + organization_id: { + name: 'organization_id', label: 'Organization', type: 'lookup' as const, + reference: 'sys_organization', deleteBehavior: 'set_null' as const, + }, + }, +}; + +/** A cascading child of the organization: it would delete rows if the walk reached it first. */ +const ORG_NOTE = { + name: 'org_note', + label: 'Organization Note', + fields: { + body: { name: 'body', label: 'Body', type: 'text' as const }, + org: { name: 'org', label: 'Organization', type: 'master_detail' as const, reference: 'sys_organization' }, + }, +}; + +/** A lookup to the organization that is NOT a tenant column: its `set_null` is untouched. */ +const SESSION = { + name: 'session_like', + label: 'Session', + tenancy: { enabled: false }, + fields: { + active_organization_id: { + name: 'active_organization_id', label: 'Active Organization', type: 'lookup' as const, + reference: 'sys_organization', + }, + }, +}; + +async function makeEngine(objects: any[]) { + const engine = new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {} } } as any); + const store = makeDriver(); + engine.registerDriver(store.driver, true); + await engine.init(); + for (const o of objects) engine.registry.registerObject(o, PACKAGE_ID); + store.seed('sys_organization', { id: ORG_ID, name: 'Doomed Org' }); + store.seed('sys_organization', { id: OTHER_ORG, name: 'Other Org' }); + return { engine, store }; +} + +const deleteOrg = (engine: ObjectQL) => + engine.delete('sys_organization', { where: { id: ORG_ID } } as any).then(() => null, (e: any) => e); + +describe('[#24948] deleting an organization that rows still name in their tenant column is refused', () => { + it('refuses with 409 DELETE_RESTRICTED, naming every table with its count', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, CONTACT]); + store.seed('acct', { id: 'a1', name: 'A1', organization_id: ORG_ID }); + store.seed('acct', { id: 'a2', name: 'A2', organization_id: ORG_ID }); + store.seed('acct', { id: 'a3', name: 'Elsewhere', organization_id: OTHER_ORG }); + store.seed('contact', { id: 'c1', name: 'C1', organization_id: ORG_ID }); + + const err = await deleteOrg(engine); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(err?.status).toBe(409); + expect(err?.object).toBe('sys_organization'); + expect(err?.dependents).toEqual([ + { object: 'acct', count: 2 }, + { object: 'contact', count: 1 }, + ]); + // The envelope's own two keys carry the first table. + expect(err?.dependentObject).toBe('acct'); + expect(err?.dependentCount).toBe(2); + }); + + it('changes nothing: the organization, every row and its tenant column are as they were', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, ORG_NOTE]); + store.seed('acct', { id: 'a1', name: 'A1', organization_id: ORG_ID }); + // A cascading child the walk would delete if it ran before the refusal. + store.seed('org_note', { id: 'n1', body: 'keep me', org: ORG_ID, organization_id: ORG_ID }); + const before = { + org: store.rows('sys_organization'), acct: store.rows('acct'), note: store.rows('org_note'), + }; + + const err = await deleteOrg(engine); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(store.writes).toEqual([]); + expect(store.rows('sys_organization')).toEqual(before.org); + expect(store.rows('acct')).toEqual(before.acct); + expect(store.rows('acct')[0].organization_id).toBe(ORG_ID); + expect(store.rows('org_note')).toEqual(before.note); + }); + + it('refuses a declared tenant column that writes `set_null` out, as the identity tables do', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, TEAM]); + store.seed('team', { id: 't1', name: 'Default Team', organization_id: ORG_ID }); + + const err = await deleteOrg(engine); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(err?.dependents).toEqual([{ object: 'team', count: 1 }]); + expect(store.rows('team')[0].organization_id).toBe(ORG_ID); + }); + + it('CONTROL: an empty organization deletes, and a lookup that is not a tenant column is still cleared', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, TEAM, SESSION]); + store.seed('acct', { id: 'a3', name: 'Elsewhere', organization_id: OTHER_ORG }); + store.seed('session_like', { id: 's1', active_organization_id: ORG_ID }); + + expect(await deleteOrg(engine)).toBeNull(); + + expect(store.rows('sys_organization').map((r) => r.id)).toEqual([OTHER_ORG]); + expect(store.rows('session_like')[0].active_organization_id).toBeNull(); + expect(store.rows('acct')[0].organization_id).toBe(OTHER_ORG); + }); + + it('CONTROL: a table never provisioned holds no row, so it refuses nothing', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); + store.missing.add('acct'); + + expect(await deleteOrg(engine)).toBeNull(); + expect(store.rows('sys_organization').map((r) => r.id)).toEqual([OTHER_ORG]); + }); +}); + +describe('[#24948] organizationOwnedRowCounts: the question plugin-auth asks before anything is deleted', () => { + it('counts each table holding rows for the organization, and skips excluded and empty ones', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, CONTACT, TEAM, SESSION]); + store.seed('acct', { id: 'a1', organization_id: ORG_ID }); + store.seed('acct', { id: 'a2', organization_id: ORG_ID }); + store.seed('acct', { id: 'a3', organization_id: OTHER_ORG }); + store.seed('team', { id: 't1', organization_id: ORG_ID }); + store.seed('session_like', { id: 's1', active_organization_id: ORG_ID }); + + expect(await engine.organizationOwnedRowCounts(ORG_ID)).toEqual([ + { object: 'acct', count: 2 }, + { object: 'team', count: 1 }, + ]); + expect(await engine.organizationOwnedRowCounts(ORG_ID, { exclude: ['team'] })).toEqual([ + { object: 'acct', count: 2 }, + ]); + expect(await engine.organizationOwnedRowCounts(OTHER_ORG)).toEqual([{ object: 'acct', count: 1 }]); + // Reads only: the count wrote nothing. + expect(store.writes).toEqual([]); + }); + + it('reads a table never provisioned as holding no row', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); + store.missing.add('acct'); + expect(await engine.organizationOwnedRowCounts(ORG_ID)).toEqual([]); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 37bce1d5d84..dc54aee88f7 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -214,7 +214,7 @@ import { isDefaultOrganizationTarget, organizationDeleteRefusal, organizationNotEmptyBody, - removeOrganizationTeams, + removeOrganizationAuthStructure, } from './organization-delete.js'; /** @@ -3871,19 +3871,20 @@ export class AuthManager { }); } }, - // [#24948] The teams and their members are the organization's own - // auth structure, and better-auth's delete leaves them behind. They - // go here, inside the request's transaction (`handleRequest`), after - // the `hooks.before` refusal has found no records left. The Default - // Organization is the last-administrator guard's to answer, so its - // teams are not touched: when that guard admits the delete, the - // engine's own refusal still holds while anything names it. + // [#24948] The teams, their members and the organization-admin + // grants are the organization's own auth structure, and better-auth's + // delete leaves them behind. They go here, inside the request's + // transaction (`handleRequest`), after the `hooks.before` refusal has + // found no records left. The Default Organization is the + // last-administrator guard's to answer, so its structure is not + // touched: when that guard admits the delete, the engine's own + // refusal still holds while anything names it. beforeDeleteOrganization: async ({ organization }: any) => { const engine = this.getDataEngine(); const organizationId = organization?.id; if (!engine || typeof organizationId !== 'string' || !organizationId) return; - if (await isDefaultOrganizationTarget(engine, organization)) return; - await removeOrganizationTeams(engine, organizationId); + if (await isDefaultOrganizationTarget(engine, organization, postureEnforcesWall(resolveTenancyPosture()))) return; + await removeOrganizationAuthStructure(engine, organizationId); }, }, // No mailer is wired in framework yet — log the accept URL so @@ -6999,7 +7000,7 @@ export class AuthManager { * * [#24948] `/organization/delete` runs here too, on the same terms and for * the same reason: the vendor's member, invitation and organization deletes - * are separate adapter calls, and the teams removed by + * are separate adapter calls, and the teams and grants removed by * `beforeDeleteOrganization` must roll back with them. */ private async runSubjectErasureAtomically( @@ -8120,7 +8121,7 @@ export class AuthManager { if (!permitted) return; // the vendor answers YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_ORGANIZATION organization = await sys.findOne('sys_organization', { where: { id: organizationId } }); if (!organization) return; // the vendor answers 400 - if (await isDefaultOrganizationTarget(engine, organization)) return; + if (await isDefaultOrganizationTarget(engine, organization, postureEnforcesWall(resolveTenancyPosture()))) return; } catch { // A lookup that did not complete hands the request back to the vendor, // which re-reads everything it decides; the engine's own refusal still diff --git a/packages/plugins/plugin-auth/src/organization-delete.test.ts b/packages/plugins/plugin-auth/src/organization-delete.test.ts new file mode 100644 index 00000000000..4878e9e7657 --- /dev/null +++ b/packages/plugins/plugin-auth/src/organization-delete.test.ts @@ -0,0 +1,177 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24948] The organization-delete refusal and the auth-structure removal, at + * the seam: what is counted as a record, what is the organization's own + * structure, and in which order the structure goes. + * + * The door, booted on SQLite with the real better-auth endpoint, is pinned in + * `packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts`; + * the engine's own refusal in `packages/objectql/src/engine-organization-delete-refusal.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; +import { + ORGANIZATION_AUTH_STRUCTURE, + isDefaultOrganizationTarget, + organizationDeleteRefusal, + organizationNotEmptyBody, + removeOrganizationAuthStructure, +} from './organization-delete.js'; + +type Row = Record; + +const ORG = 'org_1'; + +/** A data engine over in-memory tables that records every call. */ +function makeEngine(tables: Record, owned: Array<{ object: string; count: number }> = []) { + const calls: string[] = []; + const asked: Array<{ organizationId: string; exclude: string[] }> = []; + const matches = (row: Row, where: Row = {}): boolean => + Object.entries(where).every(([k, v]) => { + if (v && typeof v === 'object' && Array.isArray(v.$in)) return v.$in.includes(row[k]); + return (row[k] ?? null) === (v ?? null); + }); + const engine: any = { + async find(object: string, q: any) { + calls.push(`find ${object}`); + return (tables[object] ?? []).filter((r) => matches(r, q?.where)); + }, + async count(object: string, q: any) { + calls.push(`count ${object}`); + return (tables[object] ?? []).filter((r) => matches(r, q?.where)).length; + }, + async delete(object: string, q: any) { + calls.push(`delete ${object}${q?.multi ? ' multi' : ''}`); + expect(q?.context?.isSystem, `${object} is deleted as the system`).toBe(true); + tables[object] = (tables[object] ?? []).filter((r) => !matches(r, q?.where)); + return true; + }, + async organizationOwnedRowCounts(organizationId: string, options?: { exclude?: Iterable }) { + asked.push({ organizationId, exclude: [...(options?.exclude ?? [])] }); + return owned.map((o) => ({ ...o })); + }, + }; + return { engine, calls, asked, tables }; +} + +describe('[#24948] organizationDeleteRefusal: the records the organization still owns', () => { + it('asks the engine with the organization\'s own auth structure excluded', async () => { + const { engine, asked } = makeEngine({}, [{ object: 'crm_account', count: 3 }]); + expect(await organizationDeleteRefusal(engine, ORG)).toEqual([{ object: 'crm_account', count: 3 }]); + expect(asked).toEqual([{ organizationId: ORG, exclude: [...ORGANIZATION_AUTH_STRUCTURE] }]); + expect([...ORGANIZATION_AUTH_STRUCTURE].sort()).toEqual( + ['sys_invitation', 'sys_member', 'sys_team', 'sys_team_member', 'sys_user_permission_set'], + ); + }); + + it('counts every grant scoped to the organization except the organization-admin ones, NULL-named grants included', async () => { + const { engine } = makeEngine({ + sys_user_permission_set: [ + { organization_id: ORG, permission_set: ORGANIZATION_ADMIN }, + { organization_id: ORG, permission_set: ORGANIZATION_ADMIN_NO_BYPASS }, + { organization_id: ORG, permission_set: 'sales_rep' }, + { organization_id: ORG, permission_set: null }, + { organization_id: 'org_other', permission_set: 'sales_rep' }, + ], + }); + expect(await organizationDeleteRefusal(engine, ORG)).toEqual([{ object: 'sys_user_permission_set', count: 2 }]); + }); + + it('is empty for an organization holding only its organization-admin grants', async () => { + const { engine } = makeEngine({ + sys_user_permission_set: [{ organization_id: ORG, permission_set: ORGANIZATION_ADMIN }], + }); + expect(await organizationDeleteRefusal(engine, ORG)).toEqual([]); + }); + + it('answers undefined for an engine that cannot count, leaving the decision to the engine\'s cascade', async () => { + const { engine } = makeEngine({}); + delete engine.organizationOwnedRowCounts; + expect(await organizationDeleteRefusal(engine, ORG)).toBeUndefined(); + }); + + it('propagates a count that fails: a guard that could not be evaluated does not pass', async () => { + const { engine } = makeEngine({}); + const fault = new Error('connection lost'); + engine.organizationOwnedRowCounts = async () => { throw fault; }; + await expect(organizationDeleteRefusal(engine, ORG)).rejects.toBe(fault); + }); +}); + +describe('[#24948] organizationNotEmptyBody: the 409 body', () => { + it('is the standard DELETE_RESTRICTED code with every table and its count, and the remedy', () => { + const body = organizationNotEmptyBody([ + { object: 'crm_account', count: 3 }, + { object: 'crm_contact', count: 1 }, + ]); + expect(body.code).toBe('DELETE_RESTRICTED'); + expect(body.dependents).toEqual([ + { object: 'crm_account', count: 3 }, + { object: 'crm_contact', count: 1 }, + ]); + expect(body.message).toContain('crm_account (3)'); + expect(body.message).toContain('Delete or move those records first'); + }); +}); + +describe('[#24948] isDefaultOrganizationTarget: the last-administrator guard\'s reading of the anchor', () => { + it('is the `default` slug, whatever else exists', async () => { + const { engine } = makeEngine({ sys_organization: [{ id: 'a' }, { id: 'b' }] }); + expect(await isDefaultOrganizationTarget(engine, { id: 'a', slug: 'default' }, false)).toBe(true); + }); + + it('is the only organization, whatever its slug', async () => { + const { engine } = makeEngine({ sys_organization: [{ id: 'a', slug: 'acme' }] }); + expect(await isDefaultOrganizationTarget(engine, { id: 'a', slug: 'acme' }, false)).toBe(true); + }); + + it('CONTROL: another organization, beside others, is not', async () => { + const { engine } = makeEngine({ sys_organization: [{ id: 'a' }, { id: 'b' }] }); + expect(await isDefaultOrganizationTarget(engine, { id: 'b', slug: 'acme' }, false)).toBe(false); + }); + + it('is never one under a walled posture, where no organization anchors platform standing', async () => { + const { engine } = makeEngine({ sys_organization: [{ id: 'a', slug: 'default' }] }); + expect(await isDefaultOrganizationTarget(engine, { id: 'a', slug: 'default' }, true)).toBe(false); + }); +}); + +describe('[#24948] removeOrganizationAuthStructure: what better-auth leaves behind', () => { + it('deletes the team members, then each team by id, then the organization-admin grants, and nothing else', async () => { + const { engine, calls, tables } = makeEngine({ + sys_team: [ + { id: 't1', organization_id: ORG }, + { id: 't2', organization_id: 'org_other' }, + ], + sys_team_member: [ + { id: 'tm1', team_id: 't1' }, + { id: 'tm2', team_id: 't2' }, + ], + sys_user_permission_set: [ + { id: 'g1', organization_id: ORG, permission_set: ORGANIZATION_ADMIN }, + { id: 'g2', organization_id: ORG, permission_set: 'sales_rep' }, + { id: 'g3', organization_id: 'org_other', permission_set: ORGANIZATION_ADMIN }, + ], + }); + + await removeOrganizationAuthStructure(engine, ORG); + + expect(calls.filter((c) => c.startsWith('delete'))).toEqual([ + 'delete sys_team_member multi', + 'delete sys_team', + 'delete sys_user_permission_set multi', + ]); + expect(tables.sys_team.map((r) => r.id)).toEqual(['t2']); + expect(tables.sys_team_member.map((r) => r.id)).toEqual(['tm2']); + // A grant that is not the organization's own structure stays: the refusal counted it. + expect(tables.sys_user_permission_set.map((r) => r.id)).toEqual(['g2', 'g3']); + }); + + it('deletes no team member when the organization has no team', async () => { + const { engine, calls } = makeEngine({ sys_team: [], sys_team_member: [{ id: 'tm1', team_id: 't9' }] }); + await removeOrganizationAuthStructure(engine, ORG); + expect(calls).not.toContain('delete sys_team_member multi'); + }); +}); diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts index eecd662584f..f44c26c7399 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -34,11 +34,10 @@ * the vendor would refuse learns no count. * 2. **The organization's own auth structure goes with it.** That is * {@link ORGANIZATION_AUTH_STRUCTURE}: members and invitations (better-auth - * deletes them), teams and team members ({@link removeOrganizationTeams}, - * from `beforeDeleteOrganization`), and the organization-admin grants its - * owners and admins hold (plugin-security's member-delete hook revokes them - * when better-auth deletes the members). Any other grant the organization - * holds is counted as a record. + * deletes them), and teams, team members and the organization-admin grants + * its owners and admins hold ({@link removeOrganizationAuthStructure}, from + * `beforeDeleteOrganization`). Any other grant the organization holds is + * counted as a record. * 3. **One unit of work.** `AuthManager.handleRequest` runs the whole request * inside one engine transaction and rolls it back on any 4xx/5xx answer, the * subject-erasure wrapper's posture. On a driver without transactions the @@ -67,11 +66,10 @@ const GRANT_OBJECT = 'sys_user_permission_set'; * removed by the delete itself: * * - `sys_member`, `sys_invitation`: better-auth's `adapter.deleteOrganization`; - * - `sys_team`, `sys_team_member`: {@link removeOrganizationTeams}; - * - `sys_user_permission_set`, ONLY the grants of the organization-admin sets - * ({@link ORGANIZATION_ADMIN_SET_NAMES}): plugin-security's member-delete - * hook revokes the grant a membership grade conferred once the membership is - * gone. Every other grant scoped to the organization is counted. + * - `sys_team`, `sys_team_member`, and in `sys_user_permission_set` ONLY the + * grants of the organization-admin sets ({@link ORGANIZATION_ADMIN_SET_NAMES}), + * the ones a membership grade confers: {@link removeOrganizationAuthStructure}. + * Every other grant scoped to the organization is counted. */ export const ORGANIZATION_AUTH_STRUCTURE: readonly string[] = [ 'sys_member', @@ -160,43 +158,57 @@ export function organizationNotEmptyBody(owned: readonly OrganizationOwnedRows[] } /** - * Is `organizationId` the Default Organization: the one with `slug = 'default'`, - * else the deployment's only organization? The last-administrator guard's own - * reading of the anchor (`last-admin-guard.ts`). Its delete is that guard's to - * answer, as before, so the refusal above does not run for it. + * Is `organization` the Default Organization: on a posture with no + * organization wall, the one with `slug = 'default'`, else the deployment's + * only organization? The last-administrator guard's own reading of the anchor + * (`last-admin-guard.ts`, `defaultOrganizationId`): under a walled posture no + * organization anchors platform standing, so none is the Default one here + * either. Its delete is that guard's to answer, as before, so the refusal + * above does not run for it. `walled` is the guard's own expression, + * `postureEnforcesWall(resolveTenancyPosture())`. */ export async function isDefaultOrganizationTarget( engine: IDataEngine, organization: { id?: unknown; slug?: unknown } | undefined, + walled: boolean, ): Promise { - if (!organization) return false; + if (!organization || walled) return false; if (organization.slug === 'default') return true; const total = await engine.count('sys_organization', { context: SYSTEM_CTX } as any); return total <= 1; } /** - * Delete the organization's teams and their members: the part of its auth - * structure better-auth leaves behind. Called from `beforeDeleteOrganization`, - * inside the request's transaction, so a later failure rolls it back. + * Delete the part of the organization's auth structure better-auth leaves + * behind: its teams with their members, and the organization-admin grants its + * owners and admins hold. Called from `beforeDeleteOrganization`, inside the + * request's transaction, so a later failure rolls it back. * * Team members first, by their team: `sys_team_member` reaches the * organization only through `team_id`, and a multi-row delete runs no cascade. * Then each team by id, so the engine's cascade clears what still points at it - * (a session's active team). + * (a session's active team). The grants go explicitly, here, rather than + * waiting on plugin-security's member-delete reconcile: measured on a booted + * showcase, they were still present when the organization row was deleted. */ -export async function removeOrganizationTeams(engine: IDataEngine, organizationId: string): Promise { +export async function removeOrganizationAuthStructure(engine: IDataEngine, organizationId: string): Promise { const teams = rowsOf( await engine.find('sys_team', { where: { organization_id: organizationId }, fields: ['id'], context: SYSTEM_CTX } as any), ); const teamIds = teams.map((t) => t?.id).filter((id): id is string | number => id != null); - if (teamIds.length === 0) return; - await engine.delete('sys_team_member', { - where: { team_id: { $in: teamIds } }, + if (teamIds.length > 0) { + await engine.delete('sys_team_member', { + where: { team_id: { $in: teamIds } }, + multi: true, + context: SYSTEM_CTX, + } as any); + for (const id of teamIds) { + await engine.delete('sys_team', { where: { id }, context: SYSTEM_CTX } as any); + } + } + await engine.delete(GRANT_OBJECT, { + where: { organization_id: organizationId, [GRANT_SET_NAME_FIELD]: { $in: [...ORGANIZATION_ADMIN_SET_NAMES] } }, multi: true, context: SYSTEM_CTX, } as any); - for (const id of teamIds) { - await engine.delete('sys_team', { where: { id }, context: SYSTEM_CTX } as any); - } } diff --git a/packages/qa/dogfood/test/org-record-admin-reach.dogfood.test.ts b/packages/qa/dogfood/test/org-record-admin-reach.dogfood.test.ts index 1573fc8c981..de931556860 100644 --- a/packages/qa/dogfood/test/org-record-admin-reach.dogfood.test.ts +++ b/packages/qa/dogfood/test/org-record-admin-reach.dogfood.test.ts @@ -207,9 +207,34 @@ describe('sys_organization admin actions follow the membership grade, and every ); } expect(await findRows(ql, 'sys_organization', { id: org.doomed.id }, 1)).toHaveLength(1); + // [#24948] The doomed organization was created through the door, so it owns + // its default team, the team member and the owner's organization-admin + // grant besides three members. That is its own auth structure, and the + // delete removes all of it: none of it is a record that refuses the delete. + const teamIds = (await findRows(ql, 'sys_team', { organization_id: org.doomed.id })).map((t) => t.id); + expect(teamIds, 'PREMISE: the door gave the organization its default team').toHaveLength(1); + expect(await findRows(ql, 'sys_team_member', { team_id: { $in: teamIds } })).toHaveLength(1); const deleted = await stack.apiAs(tokens.owner, 'POST', '/auth/organization/delete', { organizationId: org.doomed.id }); expect(deleted.status, await deleted.clone().text()).toBe(200); expect(await findRows(ql, 'sys_organization', { id: org.doomed.id }, 1)).toEqual([]); + for (const object of ['sys_member', 'sys_team', 'sys_user_permission_set']) { + expect(await findRows(ql, object, { organization_id: org.doomed.id }), object).toEqual([]); + } + expect(await findRows(ql, 'sys_team_member', { team_id: { $in: teamIds } })).toEqual([]); + // Nothing was orphaned to NULL on the way. + expect(await findRows(ql, 'sys_team', { organization_id: null })).toEqual([]); + + // CONTROL: the organization the grades are measured in owns a record, and + // the same owner's delete of it is refused with the count, changing nothing. + await ql.insert('showcase_category', { name: 'Reach', organization_id: org.main.id }, { context: SYSTEM_CTX }); + const membersBefore = await findRows(ql, 'sys_member', { organization_id: org.main.id }); + const held = await stack.apiAs(tokens.owner, 'POST', '/auth/organization/delete', { organizationId: org.main.id }); + expect(held.status, await held.clone().text()).toBe(409); + const body = (await held.json()) as { code?: string; dependents?: unknown }; + expect(body.code).toBe('DELETE_RESTRICTED'); + expect(body.dependents).toEqual([{ object: 'showcase_category', count: 1 }]); + expect(await findRows(ql, 'sys_organization', { id: org.main.id }, 1)).toHaveLength(1); + expect(await findRows(ql, 'sys_member', { organization_id: org.main.id })).toEqual(membersBefore); }, 60_000); it('every api action the record serves reaches a door this composition mounts — none answers ENDPOINT_NOT_FOUND', async () => { diff --git a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts index 6966c18c0eb..6a1f3d9387a 100644 --- a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts @@ -44,6 +44,13 @@ * reference to `sys_organization` on ANY organization's delete, so the * second organization reaches the same scan with nothing else attached. * + * [#24948] The same boot also pins the organization-delete contract where the + * federated fixture can interfere with it: the refusal's per-table count reads + * the cascade's own relations, so it skips the injected anchor too; a deleted + * organization leaves nothing naming it; and the Default Organization keeps + * the last-administrator guard's 403, now with every row as it was (main + * answered that 403 after better-auth had already deleted the members). + * * The working directory is a temporary one. The showcase's external datasource * and its fixture both name a cwd-relative SQLite file, so this file's remote * database is its own, never one another file left in the package directory. @@ -131,13 +138,60 @@ describe('[#21910] an organization delete with the showcase federated fixture pr expect(refused.code).toBe('INVALID_FILTER'); }); - it('the owner deletes the organization: 200, the row is gone, and the federated rows are untouched', async () => { + it('the owner deletes the organization: 200, nothing names it any more, and the federated rows are untouched', async () => { const before = await findRows(ql, FEDERATED, {}, 500); const deleted = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: orgId }); expect(deleted.status, await deleted.clone().text()).toBe(200); expect(await findRows(ql, 'sys_organization', { id: orgId }, 1)).toHaveLength(0); + // [#24948] Its own auth structure went with it, and no row was orphaned to it. + for (const object of ['sys_member', 'sys_team', 'sys_user_permission_set', 'showcase_category']) { + expect(await findRows(ql, object, { organization_id: orgId }, 5), object).toEqual([]); + } expect((await findRows(ql, FEDERATED, {}, 500)).length).toBe(before.length); }); + + // Ordered here on purpose: the guard counts an organization owner as an + // administrator, so it refuses only while the admin owns no organization + // but the Default one (the second organization above is gone by now). + it('[#24948] (e) the Default Organization keeps the last-administrator guard\'s 403, and nothing changes', async () => { + const [defaultOrg] = await findRows(ql, 'sys_organization', { slug: 'default' }, 1); + const defaultId = String(defaultOrg.id); + const membersBefore = await findRows(ql, 'sys_member', { organization_id: defaultId }, 50); + const teamsBefore = await findRows(ql, 'sys_team', { organization_id: defaultId }, 50); + + const res = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: defaultId }); + + expect(res.status, await res.clone().text()).toBe(403); + expect(((await res.json()) as { code?: string }).code).toBe('PERMISSION_DENIED'); + // Main answered the same 403 after better-auth had already deleted the + // members; the request is one unit of work now, so they are back. + expect(await findRows(ql, 'sys_organization', { id: defaultId }, 1)).toHaveLength(1); + expect(await findRows(ql, 'sys_member', { organization_id: defaultId }, 50)).toEqual(membersBefore); + expect(await findRows(ql, 'sys_team', { organization_id: defaultId }, 50)).toEqual(teamsBefore); + }); + it('[#24948] an organization that owns a record is refused 409 with its count, and the scan still never probes the federated table', async () => { + // A second organization the admin owns, holding one record: the refusal's + // count reads the same relations the cascade does, so it must skip the + // federated object's injected anchor too, or it would answer the remote's + // INVALID_FILTER instead of a count. + const held = await ql.insert('sys_organization', { name: 'Org 24948', slug: 'org-24948' }, { context: SYSTEM_CTX }); + const heldId = String(held.id); + const [admin] = await findRows(ql, 'sys_user', { email: 'admin@objectos.ai' }, 1); + await ql.insert('sys_member', { user_id: String(admin?.id), organization_id: heldId, role: 'owner' }, { context: SYSTEM_CTX }); + await ql.insert('showcase_category', { name: 'Held', organization_id: heldId }, { context: SYSTEM_CTX }); + const membersBefore = await findRows(ql, 'sys_member', { organization_id: heldId }, 5); + + const refused = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: heldId }); + + expect(refused.status, await refused.clone().text()).toBe(409); + const body = (await refused.json()) as { code?: string; dependents?: unknown }; + expect(body.code).toBe('DELETE_RESTRICTED'); + expect(body.dependents).toEqual([{ object: 'showcase_category', count: 1 }]); + // Refused before anything was deleted: the owner is still there. + expect(await findRows(ql, 'sys_organization', { id: heldId }, 1)).toHaveLength(1); + expect(await findRows(ql, 'sys_member', { organization_id: heldId }, 5)).toEqual(membersBefore); + }); + }); diff --git a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts new file mode 100644 index 00000000000..a0a30278161 --- /dev/null +++ b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts @@ -0,0 +1,223 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24948] Deleting an organization, on the wire, on a booted multi-organization + * showcase (SQLite). + * + * ## The defect + * + * `POST /auth/organization/delete` runs better-auth's `deleteOrganization`, + * which deletes the members, then the invitations, then the organization row, + * with no transaction. The engine's cascade then applied the tenant column's + * `set_null` default to every row the organization still owned. On a store + * whose tenant column is NOT NULL (a ceremonied one) that failed on + * `sys_team.organization_id` with a 500, after the members and invitations + * were already gone, leaving an organization with no owner. On a store born + * nullable (18.0.0) it answered 200 and orphaned the teams and records to + * NULL. Every organization created through the door owns a default team, so + * on a NOT NULL store not even an empty one could be deleted. + * + * ## The contract (maintainer ruling on #24948, letter B) + * + * - (a)(c) An organization that still owns records is refused with 409 + * `DELETE_RESTRICTED`, naming each table and its count, before anything is + * deleted: every row is exactly as it was, the caller's active organization + * included. + * - (b) An empty organization (only its own auth structure: members, + * invitations, the default team and its members, the organization-admin + * grant) deletes fully, and nothing is left naming it. + * - (d) The engine refuses the same delete on its own, whatever path reaches + * it: the tenant column is never cleared. + * - (e) The Default Organization keeps the last-administrator guard's answer: + * pinned in `organization-delete-federated-fixture.dogfood.test.ts`, whose + * `single` boot has one (this `posture-only` boot has none). + * + * The seam pins are `packages/objectql/src/engine-organization-delete-refusal.test.ts` + * (the engine) and `packages/plugins/plugin-auth/src/organization-delete.test.ts`. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { type VerifyStack } from '@objectstack/verify'; +import { bootShowcase } from './showcase-boot.js'; + +const SYSTEM_CTX = { isSystem: true }; +const PASSWORD = 'OrgDelete!Pass123'; + +async function findRows(ql: any, object: string, where: Record, limit = 500): Promise { + const rows = await ql.find(object, { where, limit, context: SYSTEM_CTX }); + return Array.isArray(rows) ? rows : (rows?.records ?? []); +} + +describe('[#24948] organization delete: refused while it owns records, whole when it does not', () => { + let stack: VerifyStack; + let ql: any; + let ownerToken: string; + let adminToken: string; + let ownerId: string; + const org = { empty: '', full: '' }; + + /** Every count the delete may touch, for one organization. */ + const snapshot = async (organizationId: string) => { + const teams = await findRows(ql, 'sys_team', { organization_id: organizationId }); + const teamIds = teams.map((t) => t.id); + return { + sys_organization: (await findRows(ql, 'sys_organization', { id: organizationId })).length, + sys_member: (await findRows(ql, 'sys_member', { organization_id: organizationId })).length, + sys_invitation: (await findRows(ql, 'sys_invitation', { organization_id: organizationId })).length, + sys_team: teams.length, + sys_team_member: teamIds.length + ? (await findRows(ql, 'sys_team_member', { team_id: { $in: teamIds } })).length + : 0, + sys_user_permission_set: (await findRows(ql, 'sys_user_permission_set', { organization_id: organizationId })).length, + showcase_category: (await findRows(ql, 'showcase_category', { organization_id: organizationId })).length, + showcase_preference: (await findRows(ql, 'showcase_preference', { organization_id: organizationId })).length, + }; + }; + /** Rows orphaned to NULL in the tables an organization delete used to clear. */ + const orphans = async () => ({ + sys_team: (await findRows(ql, 'sys_team', { organization_id: null })).length, + showcase_category: (await findRows(ql, 'showcase_category', { organization_id: null })).length, + }); + const activeOrganizationOf = async (userId: string) => + (await findRows(ql, 'sys_session', { user_id: userId })).map((s) => s.active_organization_id ?? null); + const createOrg = async (name: string, slug: string): Promise => { + const res = await stack.apiAs(ownerToken, 'POST', '/auth/organization/create', { name, slug }); + expect(res.status, await res.clone().text()).toBe(200); + return String(((await res.json()) as { id: string }).id); + }; + // Written in system context rather than through `invite-member`: the door + // also queues the invitation EMAIL, a `sys_email` row naming the + // organization, and whether that row is the organization's own structure is + // an open question on #24948, so the refusal counts it today. + const invite = async (organizationId: string, email: string) => { + await ql.insert('sys_invitation', { + organization_id: organizationId, email, role: 'member', status: 'pending', + inviter_id: ownerId, expires_at: new Date(Date.now() + 86_400_000).toISOString(), + }, { context: SYSTEM_CTX }); + }; + const deleteOrg = (token: string, organizationId: string) => + stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId }); + + beforeAll(async () => { + stack = await bootShowcase({ multiTenant: 'posture-only' }); + ql = await stack.kernel.getServiceAsync('objectql'); + ownerToken = await stack.signUp('org-delete.owner@verify.test', PASSWORD, 'Org delete owner'); + adminToken = await stack.signUp('org-delete.admin@verify.test', PASSWORD, 'Org delete admin'); + [{ id: ownerId }] = await findRows(ql, 'sys_user', { email: 'org-delete.owner@verify.test' }, 1); + const [{ id: adminId }] = await findRows(ql, 'sys_user', { email: 'org-delete.admin@verify.test' }, 1); + + // Both through the door: each is born with its default team, the team + // member and the owner's organization-admin grant. + org.empty = await createOrg('Org Delete Empty', 'org-delete-empty'); + org.full = await createOrg('Org Delete Full', 'org-delete-full'); + for (const id of [org.empty, org.full]) { + await invite(id, `invitee.${id}@verify.test`); + // An `admin` member: refused by the vendor, so never told a count. + await ql.insert('sys_member', { user_id: String(adminId), organization_id: id, role: 'admin' }, { context: SYSTEM_CTX }); + } + // The records the full organization owns: two tables. + for (const name of ['Cat A', 'Cat B']) { + await ql.insert('showcase_category', { name, organization_id: org.full }, { context: SYSTEM_CTX }); + } + await ql.insert('showcase_preference', { name: 'Pref A', organization_id: org.full }, { context: SYSTEM_CTX }); + // The caller's active organization is a write the vendor makes before its + // delete hook; a refused delete must not make it. + const active = await stack.apiAs(ownerToken, 'POST', '/auth/organization/set-active', { organizationId: org.full }); + expect(active.status, await active.clone().text()).toBe(200); + }, 240_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + it('PREMISE: a door-created organization owns its auth structure, and the full one owns records', async () => { + for (const id of [org.empty, org.full]) { + const counts = await snapshot(id); + expect(counts.sys_organization).toBe(1); + expect(counts.sys_member).toBe(2); + expect(counts.sys_invitation).toBe(1); + expect(counts.sys_team).toBe(1); + expect(counts.sys_team_member).toBe(1); + expect(counts.sys_user_permission_set).toBeGreaterThanOrEqual(1); + } + expect((await snapshot(org.full)).showcase_category).toBe(2); + expect((await snapshot(org.full)).showcase_preference).toBe(1); + }); + + it('(a)(c) an organization that owns records is refused 409 with each table and its count, and nothing changes', async () => { + const before = await snapshot(org.full); + const orphansBefore = await orphans(); + const activeBefore = await activeOrganizationOf(ownerId); + expect(activeBefore, 'PREMISE: the owner has the organization active').toContain(org.full); + + const res = await deleteOrg(ownerToken, org.full); + + expect(res.status, await res.clone().text()).toBe(409); + const body = (await res.json()) as { code?: string; dependents?: Array<{ object: string; count: number }> }; + expect(body.code).toBe('DELETE_RESTRICTED'); + expect([...(body.dependents ?? [])].sort((x, y) => x.object.localeCompare(y.object))).toEqual([ + { object: 'showcase_category', count: 2 }, + { object: 'showcase_preference', count: 1 }, + ]); + // Counts, never values: no record's name rides the refusal. + expect(JSON.stringify(body)).not.toContain('Cat A'); + + expect(await snapshot(org.full)).toEqual(before); + expect(await orphans()).toEqual(orphansBefore); + expect(await activeOrganizationOf(ownerId)).toEqual(activeBefore); + }); + + it('a caller the vendor refuses gets the vendor refusal, never a count', async () => { + const res = await deleteOrg(adminToken, org.full); + expect(res.status, await res.clone().text()).toBe(403); + const body = (await res.json()) as { code?: string; dependents?: unknown }; + expect(body.code).toBe('YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_ORGANIZATION'); + expect(body.dependents).toBeUndefined(); + }); + + it('(b) an empty organization deletes fully: its auth structure goes with it, and nothing is orphaned', async () => { + const orphansBefore = await orphans(); + + const res = await deleteOrg(ownerToken, org.empty); + + expect(res.status, await res.clone().text()).toBe(200); + expect(await snapshot(org.empty)).toEqual({ + sys_organization: 0, + sys_member: 0, + sys_invitation: 0, + sys_team: 0, + sys_team_member: 0, + sys_user_permission_set: 0, + showcase_category: 0, + showcase_preference: 0, + }); + expect(await orphans()).toEqual(orphansBefore); + }); + + it('(d) the engine refuses the same delete on its own, and clears no tenant column', async () => { + const before = await snapshot(org.full); + const err: any = await ql + .delete('sys_organization', { where: { id: org.full }, context: SYSTEM_CTX }) + .then(() => null, (e: unknown) => e); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(err?.status).toBe(409); + const named = (err?.dependents ?? []).map((d: { object: string }) => d.object); + expect(named).toEqual(expect.arrayContaining(['showcase_category', 'showcase_preference', 'sys_team'])); + expect(await snapshot(org.full)).toEqual(before); + }); + + it('the operator\'s remedy: once its records are deleted, the organization deletes', async () => { + for (const object of ['showcase_category', 'showcase_preference']) { + for (const row of await findRows(ql, object, { organization_id: org.full })) { + await ql.delete(object, { where: { id: row.id }, context: SYSTEM_CTX }); + } + } + + const res = await deleteOrg(ownerToken, org.full); + + expect(res.status, await res.clone().text()).toBe(200); + const after = await snapshot(org.full); + expect(Object.values(after).every((n) => n === 0), JSON.stringify(after)).toBe(true); + }); +}); From ffa920b084c77233635d16496bac33089ee1930f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:20:35 +0000 Subject: [PATCH 03/18] wip: changeset Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- ...948-organization-delete-refuse-nonempty.md | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .changeset/24948-organization-delete-refuse-nonempty.md diff --git a/.changeset/24948-organization-delete-refuse-nonempty.md b/.changeset/24948-organization-delete-refuse-nonempty.md new file mode 100644 index 00000000000..f4e9c472093 --- /dev/null +++ b/.changeset/24948-organization-delete-refuse-nonempty.md @@ -0,0 +1,27 @@ +--- +'@objectstack/objectql': minor +'@objectstack/plugin-auth': minor +--- + +fix(objectql, plugin-auth)!: deleting an organization that still owns records is refused with 409 and a per-table count, before anything is deleted; an empty one is deleted whole, its teams and organization-admin grants with it; the engine never clears a tenant column + +**The 18.0.0 defect this fixes.** `POST /api/v1/auth/organization/delete` ran better-auth's delete, which removes the organization's members, then its invitations, then the organization row, as three separate writes. The engine's delete cascade then applied the tenant column's `set_null` default to every row the organization still owned. On a database whose tenant column is NOT NULL (one that took the v18 organization-ownership ceremony), the delete answered 500 on `sys_team.organization_id` after the members and invitations were already gone, so the organization survived with no owner and nobody could delete it. On a database 18.0.0 created, where the column is still nullable, the delete answered 200 and left the organization's teams and records with `organization_id = NULL`, owned by nobody and visible to nobody. Every organization created through the door owns a default team, so on a NOT NULL database not even an empty organization could be deleted. Deployments that run `single` are not reached: their one organization keeps the last-administrator guard's 403, and they cannot create another. + +**What happens now.** + +- An organization that still owns records is refused with `409` and the standard `DELETE_RESTRICTED` code. The body names each table and its row count, `dependents: [{ object, count }]`, and never a row's values. The refusal runs before better-auth's handler, so nothing is written, the caller's active organization included. A caller the organization does not let delete it still gets better-auth's own `403`, with no count. +- An organization that owns nothing but its own auth structure is deleted whole: its members and invitations (better-auth), and its teams, their members and the organization-admin grants its owners and admins hold (plugin-auth, from `beforeDeleteOrganization`). Any other grant scoped to the organization counts as a record. +- The request runs as one engine transaction and rolls back on any error answer, so a delete either removes all of that or none of it. On a datasource whose driver has no transactions, the refusal above is the guarantee, because it runs before the first write. +- Under all of this, the engine itself refuses to delete a `sys_organization` row while any row still names it in its tenant column, whatever path reaches the delete: `409 DELETE_RESTRICTED` with the same `dependents` list. A tenant column is never set to NULL. Other lookups to `sys_organization`, such as a session's active organization, are cleared as before. +- `ObjectQL.organizationOwnedRowCounts(organizationId, { exclude })` answers, per object, how many rows name an organization in their tenant column. It is the question the refusal asks. + +Clause-②: yes (narrowing) + + + +**BREAKING** (the organization delete's accept set narrows), shipped as `minor` under the launch-window convention for breaking changes. + +- **FROM:** deleting an organization that still owned records answered `500` after deleting its members and invitations (tenant column NOT NULL), or `200` with its teams and records orphaned to `organization_id = NULL` (tenant column nullable, an 18.0.0-born database). A direct engine delete of such a `sys_organization` row did the same. +- **TO:** both answer `409 DELETE_RESTRICTED`, with `dependents: [{ object, count }]` naming every table that still holds rows. Nothing is deleted or changed. +- **Fix:** delete the organization's records, or move them to another organization, then delete the organization. The refusal names the tables to look at. +- **Already orphaned on 18.0.0?** Rows an earlier organization delete set to `organization_id = NULL` are not touched by this release. Organization-less rows are what the v18 organization-ownership ceremony (`os migrate organization-ownership`) attributes or reports. From a454cbcc459ff131a6401a7578fd078a07945fb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:43:00 +0000 Subject: [PATCH 04/18] wip: federated-reader census rows for the organization-delete count Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- .../src/federated-injected-column-readers.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/objectql/src/federated-injected-column-readers.test.ts b/packages/objectql/src/federated-injected-column-readers.test.ts index 83dc53d56fb..1938e4597aa 100644 --- a/packages/objectql/src/federated-injected-column-readers.test.ts +++ b/packages/objectql/src/federated-injected-column-readers.test.ts @@ -144,6 +144,17 @@ const READERS: Record = { disposition: 'skips', why: 'finds the participants the scan would probe; the same skip follows the reference match', }, + // [#24948] The organization-delete refusal reads the tenant column of every + // relation the cascade follows into `sys_organization`. + 'engine.ts#organizationOwnedRowCounts :: resolveTenantFieldName()': { + disposition: 'skips', + via: 'cascadeRelationBehavior', + why: "counts a tenant column only once the cascade's own relation reading admits it, so a federated object's unprovisioned injected column is never counted", + }, + 'engine.ts#isTenantColumnSetNull :: resolveTenantFieldName()': { + disposition: 'not-a-read', + why: "compares a field name with the object's tenant column, on a relation its callers already read through cascadeRelationBehavior; it reads no row", + }, 'lifecycle/lifecycle-service.ts#tenantWindowsFor :: isFederatedUnprovisionedInjectedColumn()': { disposition: 'skips', why: 'a federated object whose organization_id is the injection has no tenant partition', @@ -563,7 +574,7 @@ describe('[#21918] every engine reader of an injected column has a disposition t } }, SCAN_BUDGET_MS); - it('the sites that skip are exactly the cascade scan, its plan, and the lifecycle tenant partition', () => { + it('the sites that skip are exactly the cascade scan, its plan, the organization-delete count, and the lifecycle tenant partition', () => { const skipping = new Set( Object.entries(READERS) .filter(([, row]) => row.disposition === 'skips') @@ -571,6 +582,7 @@ describe('[#21918] every engine reader of an injected column has a disposition t ); expect([...skipping].sort()).toEqual([ 'engine.ts#cascadeRelationBehavior', + 'engine.ts#organizationOwnedRowCounts', 'engine.ts#planCascadeAtomicity', 'lifecycle/lifecycle-service.ts#tenantWindowsFor', ]); From 4889c2400d42f511732de600109c6d281d0e6bb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:58:07 +0000 Subject: [PATCH 05/18] fix(plugin-auth): spell the organization-delete refusal code at its throw site, and read the engine options without casts Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- .../plugins/plugin-auth/src/auth-manager.ts | 3 +- .../src/organization-delete.test.ts | 5 ++-- .../plugin-auth/src/organization-delete.ts | 29 ++++++++++++------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index dc54aee88f7..4707d3c23a5 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -211,6 +211,7 @@ import { } from './signing-key-read-cache.js'; import { ORGANIZATION_DELETE_PATH, + ORGANIZATION_DELETE_REFUSAL_CODE, isDefaultOrganizationTarget, organizationDeleteRefusal, organizationNotEmptyBody, @@ -8132,7 +8133,7 @@ export class AuthManager { const owned = await organizationDeleteRefusal(engine, organizationId); if (owned && owned.length > 0) { const { APIError } = await import('better-auth/api'); - throw new APIError('CONFLICT', organizationNotEmptyBody(owned)); + throw new APIError('CONFLICT', { code: ORGANIZATION_DELETE_REFUSAL_CODE, ...organizationNotEmptyBody(owned) }); } } diff --git a/packages/plugins/plugin-auth/src/organization-delete.test.ts b/packages/plugins/plugin-auth/src/organization-delete.test.ts index 4878e9e7657..148d3eaa5fc 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.test.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.test.ts @@ -14,6 +14,7 @@ import { describe, it, expect } from 'vitest'; import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; import { ORGANIZATION_AUTH_STRUCTURE, + ORGANIZATION_DELETE_REFUSAL_CODE, isDefaultOrganizationTarget, organizationDeleteRefusal, organizationNotEmptyBody, @@ -101,12 +102,12 @@ describe('[#24948] organizationDeleteRefusal: the records the organization still }); describe('[#24948] organizationNotEmptyBody: the 409 body', () => { - it('is the standard DELETE_RESTRICTED code with every table and its count, and the remedy', () => { + it('names every table with its count and the remedy, beside the standard DELETE_RESTRICTED code', () => { const body = organizationNotEmptyBody([ { object: 'crm_account', count: 3 }, { object: 'crm_contact', count: 1 }, ]); - expect(body.code).toBe('DELETE_RESTRICTED'); + expect(ORGANIZATION_DELETE_REFUSAL_CODE).toBe('DELETE_RESTRICTED'); expect(body.dependents).toEqual([ { object: 'crm_account', count: 3 }, { object: 'crm_contact', count: 1 }, diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts index f44c26c7399..957394f63b8 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -51,12 +51,20 @@ */ import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; +import type { StandardErrorCode } from '@objectstack/spec/api'; import type { IDataEngine } from '@objectstack/spec/contracts'; import { GRANT_SET_NAME_FIELD } from './grant-set-name.js'; /** The better-auth endpoint this module guards. */ export const ORGANIZATION_DELETE_PATH = '/organization/delete'; +/** + * The refusal's wire code: the standard catalog's member for "a delete refused + * while dependents exist" (ADR-0112). Not a registered synonym of it: the + * ledger refuses one that merely re-spells a standard member. + */ +export const ORGANIZATION_DELETE_REFUSAL_CODE: StandardErrorCode = 'DELETE_RESTRICTED'; + /** The grant table; partly the organization's own structure (see {@link ORGANIZATION_AUTH_STRUCTURE}). */ const GRANT_OBJECT = 'sys_user_permission_set'; @@ -126,12 +134,12 @@ export async function organizationDeleteRefusal( const tenantGrants = await engine.count(GRANT_OBJECT, { where: { organization_id: organizationId }, context: SYSTEM_CTX, - } as any); + }); if (tenantGrants > 0) { const adminGrants = await engine.count(GRANT_OBJECT, { where: { organization_id: organizationId, [GRANT_SET_NAME_FIELD]: { $in: [...ORGANIZATION_ADMIN_SET_NAMES] } }, context: SYSTEM_CTX, - } as any); + }); const others = tenantGrants - adminGrants; if (others > 0) owned.push({ object: GRANT_OBJECT, count: others }); } @@ -139,12 +147,12 @@ export async function organizationDeleteRefusal( } /** - * The 409 body for a refused delete: the standard catalog's - * `DELETE_RESTRICTED`, every table with its count, and the remedy. + * The 409 body for a refused delete, beside its code + * ({@link ORGANIZATION_DELETE_REFUSAL_CODE}, spelled at the throw site): every + * table with its count, and the remedy. */ export function organizationNotEmptyBody(owned: readonly OrganizationOwnedRows[]): { message: string; - code: 'DELETE_RESTRICTED'; dependents: OrganizationOwnedRows[]; } { const named = owned.map((o) => `${o.object} (${o.count})`).join(', '); @@ -152,7 +160,6 @@ export function organizationNotEmptyBody(owned: readonly OrganizationOwnedRows[] message: `This organization still owns records in ${named}. ` + 'Delete or move those records first, then delete the organization.', - code: 'DELETE_RESTRICTED', dependents: owned.map((o) => ({ object: o.object, count: o.count })), }; } @@ -174,7 +181,7 @@ export async function isDefaultOrganizationTarget( ): Promise { if (!organization || walled) return false; if (organization.slug === 'default') return true; - const total = await engine.count('sys_organization', { context: SYSTEM_CTX } as any); + const total = await engine.count('sys_organization', { context: SYSTEM_CTX }); return total <= 1; } @@ -193,7 +200,7 @@ export async function isDefaultOrganizationTarget( */ export async function removeOrganizationAuthStructure(engine: IDataEngine, organizationId: string): Promise { const teams = rowsOf( - await engine.find('sys_team', { where: { organization_id: organizationId }, fields: ['id'], context: SYSTEM_CTX } as any), + await engine.find('sys_team', { where: { organization_id: organizationId }, fields: ['id'], context: SYSTEM_CTX }), ); const teamIds = teams.map((t) => t?.id).filter((id): id is string | number => id != null); if (teamIds.length > 0) { @@ -201,14 +208,14 @@ export async function removeOrganizationAuthStructure(engine: IDataEngine, organ where: { team_id: { $in: teamIds } }, multi: true, context: SYSTEM_CTX, - } as any); + }); for (const id of teamIds) { - await engine.delete('sys_team', { where: { id }, context: SYSTEM_CTX } as any); + await engine.delete('sys_team', { where: { id }, context: SYSTEM_CTX }); } } await engine.delete(GRANT_OBJECT, { where: { organization_id: organizationId, [GRANT_SET_NAME_FIELD]: { $in: [...ORGANIZATION_ADMIN_SET_NAMES] } }, multi: true, context: SYSTEM_CTX, - } as any); + }); } From 037461e591dea8a7dac5a7f8aee219db963eced5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:58:08 +0000 Subject: [PATCH 06/18] docs(permissions): regenerate the tenant-audit census for the three organization-delete write sites Generated by node scripts/tenant-audit-census.mjs --write, and the page's census-derived prose numbers restated to match (233 to 236 sites, 131 to 134 decidably elevated, 49 to 50 const-named). Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- .../docs/permissions/tenant-audit-census.mdx | 34 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 17 ++++++---- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index afe0f643350..96e2a95752a 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **51 of the 233 sites are spelled that way**. A +forwarding shim cannot, and **51 of the 236 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **233** | +| 175 write call sites | quoted in the merged changeset | **236** | | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **29** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **156 of 233** decidable, **77** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 131 decidably elevated, 0 decidably not, 94 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **159 of 236** decidable, **77** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 134 decidably elevated, 0 decidably not, 94 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -201,17 +201,17 @@ at any commit. Two structural facts do plausibly widen this reading against any hand or regex one, and both are counted in the generated tables below: the 41 sites reached -through an erased (`any`) receiver, and the 49 that name their object through a +through an erased (`any`) receiver, and the 50 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -131 of 233 (56%) as decidably elevated, with 94 more whose elevation is a +134 of 236 (57%) as decidably elevated, with 94 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `2 / 233`, and say what it is**: the sites whose options argument was +⇒ **Cite `2 / 236`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **29 further sites** have an options argument this @@ -223,29 +223,29 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **233** | -| …whose object name is statically decidable | 156 | +| write call sites on the application surface | **236** | +| …whose object name is statically decidable | 159 | | …whose object name is chosen at run time | 77 | -| …against an object with tenancy ENABLED | 155 | +| …against an object with tenancy ENABLED | 158 | | …against an object that declares tenancy off | 1 | -| threading a tenant context | 174 | +| threading a tenant context | 177 | | PROVABLY carrying none (options read, no context key) | **8** | | …of those, against a decidably tenancy-enabled object | **2** | | options argument UNREADABLE — may or may not carry one | 51 | | …of those, against a decidably tenancy-enabled object | 29 | -| threading a decidably ELEVATED (`isSystem`) context | 131 | +| threading a decidably ELEVATED (`isSystem`) context | 134 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 94 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 192 | +| receiver carried a readable engine type | 195 | | receiver erased, placed by the object NAME | 31 | | receiver erased, placed by an `object: string` PARAMETER | 5 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 | -| object name spelled inline | 107 | -| object name spelled through a `const` | 49 | +| object name spelled inline | 109 | +| object name spelled through a `const` | 50 | | object name is an `object: string` parameter | 7 | | object name is some other run-time expression | 70 | @@ -297,11 +297,11 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-20 at `39fc914f7`. +Measured on 2026-10-20 at `a454cbcc4`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 681 | +| tracked non-test sources scanned | 684 | | engine-shaped types recognised | 82 | | declared objects in the registry | 118 | | same-named calls subtracted as non-engine | 169 | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 51bf6dc291e..b5ee0ccd14c 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 233 | -| Object name statically decidable | 156 | +| Write call sites | 236 | +| Object name statically decidable | 159 | | Object name chosen at run time | 77 | -| Against a tenancy-enabled object | 155 | +| Against a tenancy-enabled object | 158 | | Against an object declaring tenancy off | 1 | -| Threading a tenant context | 174 | +| Threading a tenant context | 177 | | Provably carrying none | 8 | | …and decidably tenancy-enabled | 2 | | Options argument unreadable | 51 | | …and decidably tenancy-enabled | 29 | -| Threading a decidably elevated context | 131 | +| Threading a decidably elevated context | 134 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 94 | @@ -90,11 +90,11 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-20 at `39fc914f7`. +Measured on 2026-10-20 at `a454cbcc4`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 681 | +| tracked non-test sources scanned | 684 | | engine-shaped types recognised | 82 | | declared objects in the registry | 118 | | same-named calls subtracted as non-engine | 169 | @@ -155,6 +155,9 @@ Measured on 2026-10-20 at `39fc914f7`. | `packages/plugins/plugin-auth/src/objectql-adapter.ts` | `delete` | `objectName` | undecidable | PROVABLY NONE | 5 | | `packages/plugins/plugin-auth/src/objectql-adapter.ts` | `insert` | `objectName` | undecidable | options unreadable | 2 | | `packages/plugins/plugin-auth/src/objectql-adapter.ts` | `update` | `objectName` | undecidable | options unreadable | 5 | +| `packages/plugins/plugin-auth/src/organization-delete.ts` | `delete` | `sys_team` | enabled | elevated | 1 | +| `packages/plugins/plugin-auth/src/organization-delete.ts` | `delete` | `sys_team_member` | enabled | elevated | 1 | +| `packages/plugins/plugin-auth/src/organization-delete.ts` | `delete` | `sys_user_permission_set` | enabled | elevated | 1 | | `packages/plugins/plugin-auth/src/reconcile-membership.ts` | `insert` | `sys_member` | enabled | context, elevation undecidable | 1 | | `packages/plugins/plugin-auth/src/scim-connection-service.ts` | `insert` | `sys_scim_connection_credential` | enabled | PROVABLY NONE | 1 | | `packages/plugins/plugin-auth/src/session-tombstone.ts` | `update` | `objectName` | undecidable | options unreadable | 1 | From 676a8b84d3dba1dcee617521c026021d943f1bf5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:08:56 +0000 Subject: [PATCH 07/18] test(plugin-auth): the organization-delete engine double routes delete through assertEngineDeleteDispatch and honours limit; pin it in the double ledger Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- packages/plugins/plugin-auth/src/organization-delete.test.ts | 5 ++++- scripts/engine-double-contract.pinned.json | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-auth/src/organization-delete.test.ts b/packages/plugins/plugin-auth/src/organization-delete.test.ts index 148d3eaa5fc..a1de7ee8f6b 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.test.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.test.ts @@ -11,6 +11,7 @@ */ import { describe, it, expect } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; import { ORGANIZATION_AUTH_STRUCTURE, @@ -37,13 +38,15 @@ function makeEngine(tables: Record, owned: Array<{ object: string const engine: any = { async find(object: string, q: any) { calls.push(`find ${object}`); - return (tables[object] ?? []).filter((r) => matches(r, q?.where)); + const rows = (tables[object] ?? []).filter((r) => matches(r, q?.where)); + return typeof q?.limit === 'number' ? rows.slice(0, q.limit) : rows; }, async count(object: string, q: any) { calls.push(`count ${object}`); return (tables[object] ?? []).filter((r) => matches(r, q?.where)).length; }, async delete(object: string, q: any) { + assertEngineDeleteDispatch(q); calls.push(`delete ${object}${q?.multi ? ' multi' : ''}`); expect(q?.context?.isSystem, `${object} is deleted as the system`).toBe(true); tables[object] = (tables[object] ?? []).filter((r) => !matches(r, q?.where)); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 80609a29c0e..cb7c960c7ad 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3111,6 +3111,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-auth/src/organization-delete.test.ts", + "verb": "delete", + "pinned": 1 + }, { "file": "packages/plugins/plugin-auth/src/organization-metadata-decode.test.ts", "verb": "update", From d1c373ce80eb385f7d0035fa809a85f299e1fc37 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:10:45 +0000 Subject: [PATCH 08/18] test(plugin-auth): the organization-delete double's WHERE matcher refuses combinators it does not implement Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- packages/plugins/plugin-auth/src/organization-delete.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/plugins/plugin-auth/src/organization-delete.test.ts b/packages/plugins/plugin-auth/src/organization-delete.test.ts index a1de7ee8f6b..03f49148c96 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.test.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.test.ts @@ -32,6 +32,7 @@ function makeEngine(tables: Record, owned: Array<{ object: string const asked: Array<{ organizationId: string; exclude: string[] }> = []; const matches = (row: Row, where: Row = {}): boolean => Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`test double: combinator ${k} is not implemented`); if (v && typeof v === 'object' && Array.isArray(v.$in)) return v.$in.includes(row[k]); return (row[k] ?? null) === (v ?? null); }); From 0573a968239deabada94926ad6748cae5d764ea8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:52:59 +0000 Subject: [PATCH 09/18] fix(objectql, plugin-auth): append-only ledgers neither refuse an organization delete nor lose their rows Objects declared managedBy: 'append-only' (sys_activity, sys_audit_log, sys_email and their siblings) are history: both refusals leave them out, and their rows keep naming the deleted organization, never NULL and never deleted. Pinned with the audit plugin composed, plus a business-unit refusal. Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- ...948-organization-delete-refuse-nonempty.md | 3 +- ...engine-organization-delete-refusal.test.ts | 44 +++++++++++++++++++ packages/objectql/src/engine.ts | 31 +++++++++++-- .../plugin-auth/src/organization-delete.ts | 5 ++- ...ion-delete-refuse-nonempty.dogfood.test.ts | 40 +++++++++++++++-- 5 files changed, 114 insertions(+), 9 deletions(-) diff --git a/.changeset/24948-organization-delete-refuse-nonempty.md b/.changeset/24948-organization-delete-refuse-nonempty.md index f4e9c472093..29072d35deb 100644 --- a/.changeset/24948-organization-delete-refuse-nonempty.md +++ b/.changeset/24948-organization-delete-refuse-nonempty.md @@ -10,6 +10,7 @@ fix(objectql, plugin-auth)!: deleting an organization that still owns records is **What happens now.** - An organization that still owns records is refused with `409` and the standard `DELETE_RESTRICTED` code. The body names each table and its row count, `dependents: [{ object, count }]`, and never a row's values. The refusal runs before better-auth's handler, so nothing is written, the caller's active organization included. A caller the organization does not let delete it still gets better-auth's own `403`, with no count. +- Append-only ledgers (objects declared `managedBy: 'append-only'`: `sys_activity`, `sys_audit_log`, `sys_email` and their siblings) are history, not records. They never refuse the delete, and their rows are kept: their `organization_id` goes on naming the deleted organization, never NULL and never deleted. The delete writes activity rows of its own, so counting them would refuse every delete. - An organization that owns nothing but its own auth structure is deleted whole: its members and invitations (better-auth), and its teams, their members and the organization-admin grants its owners and admins hold (plugin-auth, from `beforeDeleteOrganization`). Any other grant scoped to the organization counts as a record. - The request runs as one engine transaction and rolls back on any error answer, so a delete either removes all of that or none of it. On a datasource whose driver has no transactions, the refusal above is the guarantee, because it runs before the first write. - Under all of this, the engine itself refuses to delete a `sys_organization` row while any row still names it in its tenant column, whatever path reaches the delete: `409 DELETE_RESTRICTED` with the same `dependents` list. A tenant column is never set to NULL. Other lookups to `sys_organization`, such as a session's active organization, are cleared as before. @@ -23,5 +24,5 @@ Clause-②: yes (narrowing) - **FROM:** deleting an organization that still owned records answered `500` after deleting its members and invitations (tenant column NOT NULL), or `200` with its teams and records orphaned to `organization_id = NULL` (tenant column nullable, an 18.0.0-born database). A direct engine delete of such a `sys_organization` row did the same. - **TO:** both answer `409 DELETE_RESTRICTED`, with `dependents: [{ object, count }]` naming every table that still holds rows. Nothing is deleted or changed. -- **Fix:** delete the organization's records, or move them to another organization, then delete the organization. The refusal names the tables to look at. +- **Fix:** delete the organization's records, or move them to another organization, then delete the organization. The refusal names the tables to look at. Business units, positions, sharing rules and organization-level settings count as records; activity, audit and mail-log rows do not. - **Already orphaned on 18.0.0?** Rows an earlier organization delete set to `organization_id = NULL` are not touched by this release. Organization-less rows are what the v18 organization-ownership ceremony (`os migrate organization-ownership`) attributes or reports. diff --git a/packages/objectql/src/engine-organization-delete-refusal.test.ts b/packages/objectql/src/engine-organization-delete-refusal.test.ts index c10368f262b..21f69dd3a1d 100644 --- a/packages/objectql/src/engine-organization-delete-refusal.test.ts +++ b/packages/objectql/src/engine-organization-delete-refusal.test.ts @@ -146,6 +146,14 @@ const SESSION = { }, }; +/** An append-only ledger (the audit bucket): its rows outlive the organization they name. */ +const LEDGER = { + name: 'activity_like', + label: 'Activity', + managedBy: 'append-only' as const, + fields: { summary: { name: 'summary', label: 'Summary', type: 'text' as const } }, +}; + async function makeEngine(objects: any[]) { const engine = new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {} } } as any); const store = makeDriver(); @@ -233,6 +241,42 @@ describe('[#24948] deleting an organization that rows still name in their tenant }); }); +describe('[#24948] an append-only ledger neither refuses the delete nor is cleared by it (seat decision 6305179819)', () => { + it('deletes an organization only a ledger still names, and keeps the ledger rows naming it', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, LEDGER]); + store.seed('activity_like', { id: 'l1', summary: 'organization created', organization_id: ORG_ID }); + store.seed('activity_like', { id: 'l2', summary: 'team created', organization_id: ORG_ID }); + + expect(await deleteOrg(engine)).toBeNull(); + + expect(store.rows('sys_organization').map((r) => r.id)).toEqual([OTHER_ORG]); + // Kept, and still naming the deleted organization: never NULL, never deleted. + expect(store.rows('activity_like').map((r) => [r.id, r.organization_id])).toEqual([ + ['l1', ORG_ID], + ['l2', ORG_ID], + ]); + expect(store.writes.filter((w) => w.includes('activity_like'))).toEqual([]); + }); + + it('CONTROL: beside a ledger, a record still refuses, and only the record is named', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, LEDGER]); + store.seed('activity_like', { id: 'l1', organization_id: ORG_ID }); + store.seed('acct', { id: 'a1', organization_id: ORG_ID }); + + const err = await deleteOrg(engine); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(err?.dependents).toEqual([{ object: 'acct', count: 1 }]); + expect(store.writes).toEqual([]); + }); + + it('organizationOwnedRowCounts never counts a ledger', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, LEDGER]); + store.seed('activity_like', { id: 'l1', organization_id: ORG_ID }); + expect(await engine.organizationOwnedRowCounts(ORG_ID)).toEqual([]); + }); +}); + describe('[#24948] organizationOwnedRowCounts: the question plugin-auth asks before anything is deleted', () => { it('counts each table holding rows for the organization, and skips excluded and empty ones', async () => { const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, CONTACT, TEAM, SESSION]); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index ae114b9bf6d..69d673d2c5a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -19991,9 +19991,30 @@ export class ObjectQL implements IObjectQLEngine { ); } + /** + * [#24948] Is `child` an append-only ledger, whose rows outlive the + * organization they name? Read from the declaration the object already + * carries, `managedBy: 'append-only'` (the audit bucket: `sys_activity`, + * `sys_audit_log`, `sys_email` and their siblings), never from a list of + * names. + * + * Such a row is history, not a record an administrator manages: it ages out + * on its own retention, or is the delivery log of something that already + * happened. The organization delete itself writes `sys_activity` rows, so a + * ledger that counted would refuse every organization delete, the empty ones + * included. Neither refusal counts a ledger. Its rows are kept, and so is + * their tenant column: it goes on naming the deleted organization, never + * NULL (ADR-0131 D1) and never deleted (ADR-0131 §8), and under tenant + * scoping the rows read as no organization's (seat decision 6305179819). + */ + private static isAppendOnlyLedger(child: ServiceObject): boolean { + return (child as { managedBy?: unknown })?.managedBy === 'append-only'; + } + /** * [#24948] The engine half of organization delete: refuse while any row - * outside the cascade set still names the organization in its tenant column. + * outside the cascade set still names the organization in its tenant column, + * an append-only ledger's excepted ({@link isAppendOnlyLedger}). * * Runs before the walk touches the first relation, and reads every tenant * relation before it throws, so the one refusal names every table that holds @@ -20020,7 +20041,8 @@ export class ObjectQL implements IObjectQLEngine { for (const child of objects) { const childName = (child as any)?.name as string | undefined; const fields = (child as any)?.fields as Record | undefined; - if (!childName || !fields) continue; + // An append-only ledger's rows outlive the organization: not counted, kept as they are. + if (!childName || !fields || ObjectQL.isAppendOnlyLedger(child)) continue; for (const [fieldName, fdef] of Object.entries(fields)) { const behavior = this.cascadeRelationBehavior(ORGANIZATION_OBJECT, child, fieldName, fdef); if (!this.isTenantColumnSetNull(ORGANIZATION_OBJECT, child, fieldName, behavior)) continue; @@ -20074,7 +20096,8 @@ export class ObjectQL implements IObjectQLEngine { * row. * * `exclude` names the objects the caller removes as part of the delete - * itself. + * itself. An append-only ledger is never counted ({@link isAppendOnlyLedger}), + * the same set the engine's own refusal leaves out. */ async organizationOwnedRowCounts( organizationId: string, @@ -20085,7 +20108,7 @@ export class ObjectQL implements IObjectQLEngine { for (const child of this._registry.getAllObjects()) { const childName = (child as any)?.name as string | undefined; const fields = (child as any)?.fields as Record | undefined; - if (!childName || !fields || exclude.has(childName)) continue; + if (!childName || !fields || exclude.has(childName) || ObjectQL.isAppendOnlyLedger(child)) continue; const tenantField = resolveTenantFieldName(child); if (!tenantField || !fields[tenantField]) continue; const fdef = fields[tenantField]; diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts index 957394f63b8..c4823b77f11 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -25,7 +25,10 @@ * 1. **Refused while the organization owns records.** {@link * organizationDeleteRefusal} counts, per table, the rows that name the * organization in their tenant column, leaving out the organization's own - * auth structure. Any count above zero refuses with 409 + * auth structure and every append-only ledger (`managedBy: 'append-only'`: + * `sys_activity`, `sys_email` and their siblings), whose rows are history, + * are kept, and go on naming the deleted organization (seat decision + * 6305179819; the engine reads the same set). Any count above zero refuses with 409 * `DELETE_RESTRICTED` and `dependents: [{ object, count }]`: counts, never a * row value. It runs in `hooks.before`, ahead of the vendor's handler, and * only for a caller the vendor itself would admit (a member holding the diff --git a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts index a0a30278161..8749a125a88 100644 --- a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts @@ -25,7 +25,11 @@ * included. * - (b) An empty organization (only its own auth structure: members, * invitations, the default team and its members, the organization-admin - * grant) deletes fully, and nothing is left naming it. + * grant) deletes fully, and nothing is left naming it but the append-only + * `sys_activity` ledger, whose rows are kept (seat decision 6305179819). The + * boot composes the audit plugin, as `os serve` does, so that ledger is live. + * - An organization holding a business unit is refused like one holding any + * other record. * - (d) The engine refuses the same delete on its own, whatever path reaches * it: the tenant column is never cleared. * - (e) The Default Organization keeps the last-administrator guard's answer: @@ -37,6 +41,7 @@ */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { AuditPlugin } from '@objectstack/plugin-audit'; import { type VerifyStack } from '@objectstack/verify'; import { bootShowcase } from './showcase-boot.js'; @@ -54,7 +59,7 @@ describe('[#24948] organization delete: refused while it owns records, whole whe let ownerToken: string; let adminToken: string; let ownerId: string; - const org = { empty: '', full: '' }; + const org = { empty: '', full: '', unit: '' }; /** Every count the delete may touch, for one organization. */ const snapshot = async (organizationId: string) => { @@ -99,7 +104,10 @@ describe('[#24948] organization delete: refused while it owns records, whole whe stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId }); beforeAll(async () => { - stack = await bootShowcase({ multiTenant: 'posture-only' }); + // The audit plugin is composed, as `os serve` composes it: its + // `sys_activity` ledger names every organization from its creation on, + // and the delete writes rows to it itself. + stack = await bootShowcase({ multiTenant: 'posture-only', extraPlugins: [new AuditPlugin()] }); ql = await stack.kernel.getServiceAsync('objectql'); ownerToken = await stack.signUp('org-delete.owner@verify.test', PASSWORD, 'Org delete owner'); adminToken = await stack.signUp('org-delete.admin@verify.test', PASSWORD, 'Org delete admin'); @@ -110,6 +118,9 @@ describe('[#24948] organization delete: refused while it owns records, whole whe // member and the owner's organization-admin grant. org.empty = await createOrg('Org Delete Empty', 'org-delete-empty'); org.full = await createOrg('Org Delete Full', 'org-delete-full'); + org.unit = await createOrg('Org Delete Unit', 'org-delete-unit'); + // A business unit is a record its administrator created, not structure. + await ql.insert('sys_business_unit', { name: 'Field Ops', kind: 'department', organization_id: org.unit }, { context: SYSTEM_CTX }); for (const id of [org.empty, org.full]) { await invite(id, `invitee.${id}@verify.test`); // An `admin` member: refused by the vendor, so never told a count. @@ -177,10 +188,19 @@ describe('[#24948] organization delete: refused while it owns records, whole whe it('(b) an empty organization deletes fully: its auth structure goes with it, and nothing is orphaned', async () => { const orphansBefore = await orphans(); + const ledgerBefore = await findRows(ql, 'sys_activity', { organization_id: org.empty }); + expect(ledgerBefore.length, 'PREMISE: the activity ledger names the organization from its creation').toBeGreaterThan(0); const res = await deleteOrg(ownerToken, org.empty); expect(res.status, await res.clone().text()).toBe(200); + // [seat decision 6305179819] The append-only ledger refused nothing, and + // its rows are kept, still naming the deleted organization: none deleted, + // none cleared (the delete adds rows of its own). + const ledgerAfter = await findRows(ql, 'sys_activity', { organization_id: org.empty }); + const keptIds = new Set(ledgerAfter.map((r) => r.id)); + expect(ledgerBefore.every((r) => keptIds.has(r.id)), 'every ledger row is kept').toBe(true); + expect(await findRows(ql, 'sys_activity', { organization_id: null })).toEqual([]); expect(await snapshot(org.empty)).toEqual({ sys_organization: 0, sys_member: 0, @@ -194,6 +214,20 @@ describe('[#24948] organization delete: refused while it owns records, whole whe expect(await orphans()).toEqual(orphansBefore); }); + it('an organization holding only a business unit is refused 409 with its count, and nothing changes', async () => { + const before = await snapshot(org.unit); + const units = await findRows(ql, 'sys_business_unit', { organization_id: org.unit }); + + const res = await deleteOrg(ownerToken, org.unit); + + expect(res.status, await res.clone().text()).toBe(409); + const body = (await res.json()) as { code?: string; dependents?: unknown }; + expect(body.code).toBe('DELETE_RESTRICTED'); + expect(body.dependents).toEqual([{ object: 'sys_business_unit', count: 1 }]); + expect(await snapshot(org.unit)).toEqual(before); + expect(await findRows(ql, 'sys_business_unit', { organization_id: org.unit })).toEqual(units); + }); + it('(d) the engine refuses the same delete on its own, and clears no tenant column', async () => { const before = await snapshot(org.full); const err: any = await ql From 3889a6f1717bb0630dd05b0b0ba96d4ba57c01e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 01:57:20 +0000 Subject: [PATCH 10/18] wip: organization delete review round (B1 B2 B4 B5 B9) Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- ...engine-organization-delete-refusal.test.ts | 91 +++++++++++- packages/objectql/src/engine.ts | 133 ++++++++++++++++-- .../federated-injected-column-readers.test.ts | 8 ++ ...siness-unit-parent-delete-restrict.test.ts | 22 ++- ...ult-organization-statements.golden.test.ts | 35 ++++- 5 files changed, 268 insertions(+), 21 deletions(-) diff --git a/packages/objectql/src/engine-organization-delete-refusal.test.ts b/packages/objectql/src/engine-organization-delete-refusal.test.ts index 21f69dd3a1d..b231cde808c 100644 --- a/packages/objectql/src/engine-organization-delete-refusal.test.ts +++ b/packages/objectql/src/engine-organization-delete-refusal.test.ts @@ -27,6 +27,7 @@ const OTHER_ORG = 'org_other'; function makeDriver() { const tables: Record = {}; const writes: string[] = []; + const reads: string[] = []; const missing = new Set(); const rowsOf = (o: string): Row[] => (tables[o] ??= []); const matches = (row: Row, where: any): boolean => { @@ -45,6 +46,7 @@ function makeDriver() { async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, async syncSchema() {}, async find(o: string, ast: any) { + reads.push(o); if (missing.has(o)) throw missingTable(o); const hit = rowsOf(o).filter((r) => matches(r, ast?.where)); return typeof ast?.limit === 'number' ? hit.slice(0, ast.limit) : hit; @@ -54,7 +56,8 @@ function makeDriver() { return first ?? null; }, async count(o: string, ast: any) { - return (await this.find(o, { where: ast?.where })).length; + if (missing.has(o)) throw missingTable(o); + return rowsOf(o).filter((r) => matches(r, ast?.where)).length; }, async create(o: string, data: Row) { const row = { ...data, id: String(data.id) }; @@ -86,6 +89,7 @@ function makeDriver() { return { driver, writes, + reads, missing, rows: (o: string) => rowsOf(o).map((r) => ({ ...r })), seed: (o: string, row: Row) => void rowsOf(o).push(row), @@ -154,6 +158,29 @@ const LEDGER = { fields: { summary: { name: 'summary', label: 'Summary', type: 'text' as const } }, }; +/** An append-only ledger whose organization lookup is its ATTRIBUTION, not a tenant column (`sys_audit_log.tenant_id`). */ +const AUDIT_LIKE = { + name: 'audit_like', + label: 'Audit', + managedBy: 'append-only' as const, + systemFields: { tenant: false }, + fields: { + action: { name: 'action', label: 'Action', type: 'text' as const }, + tenant_id: { name: 'tenant_id', label: 'Tenant', type: 'lookup' as const, reference: 'sys_organization', readonly: true }, + }, +}; + +/** Tenancy disabled, with an `organization_id` attribution column (`sys_approval_request`). */ +const REQUEST_LIKE = { + name: 'request_like', + label: 'Request', + tenancy: { enabled: false }, + fields: { + subject: { name: 'subject', label: 'Subject', type: 'text' as const }, + organization_id: { name: 'organization_id', label: 'Organization', type: 'lookup' as const, reference: 'sys_organization' }, + }, +}; + async function makeEngine(objects: any[]) { const engine = new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {} } } as any); const store = makeDriver(); @@ -277,6 +304,68 @@ describe('[#24948] an append-only ledger neither refuses the delete nor is clear }); }); +describe('[#24948] references the delete keeps as they are (seat decision 6305996369)', () => { + it("an append-only ledger's non-tenant lookup keeps naming the deleted organization", async () => { + const { engine, store } = await makeEngine([ORGANIZATION, AUDIT_LIKE]); + store.seed('audit_like', { id: 'au1', action: 'org.created', tenant_id: ORG_ID }); + + expect(await deleteOrg(engine)).toBeNull(); + + expect(store.rows('audit_like')).toEqual([{ id: 'au1', action: 'org.created', tenant_id: ORG_ID }]); + expect(store.writes.filter((w) => w.includes('audit_like'))).toEqual([]); + }); + + it('an organization_id on a tenancy-disabled object is kept, naming it, and is not counted', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, REQUEST_LIKE, SESSION]); + store.seed('request_like', { id: 'r1', subject: 'Expense', organization_id: ORG_ID }); + store.seed('session_like', { id: 's1', active_organization_id: ORG_ID }); + expect(await engine.organizationOwnedRowCounts(ORG_ID)).toEqual([]); + + expect(await deleteOrg(engine)).toBeNull(); + + expect(store.rows('request_like')[0].organization_id).toBe(ORG_ID); + // CONTROL: a pointer is still cleared. + expect(store.rows('session_like')[0].active_organization_id).toBeNull(); + }); +}); + +describe('[#24948] the predicate path is refused for an organization', () => { + it('a multi delete of sys_organization is refused, by id only, before anything is written', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); + const err: any = await engine + .delete('sys_organization', { where: { name: 'Doomed Org' }, multi: true } as any) + .then(() => null, (e: any) => e); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(err?.status).toBe(409); + expect(err?.message).toContain('by id'); + expect(store.writes).toEqual([]); + expect(store.rows('sys_organization').map((r) => r.id)).toEqual([ORG_ID, OTHER_ORG]); + }); + + it('CONTROL: a multi delete of another object is not refused by this rule', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); + store.seed('acct', { id: 'a1', name: 'gone', organization_id: OTHER_ORG }); + const err = await engine + .delete('acct', { where: { name: 'gone' }, multi: true } as any) + .then(() => null, (e: any) => e); + expect(err?.code).not.toBe('DELETE_RESTRICTED'); + }); +}); + +describe('[#24948] the engine refusal counts, it does not read rows', () => { + it('refusing an organization that owns rows issues no row read of the owning table', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); + for (let i = 0; i < 5; i++) store.seed('acct', { id: `a${i}`, organization_id: ORG_ID }); + store.reads.length = 0; + + const err = await deleteOrg(engine); + + expect(err?.dependents).toEqual([{ object: 'acct', count: 5 }]); + expect(store.reads.filter((o) => o === 'acct')).toEqual([]); + }); +}); + describe('[#24948] organizationOwnedRowCounts: the question plugin-auth asks before anything is deleted', () => { it('counts each table holding rows for the organization, and skips excluded and empty ones', async () => { const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, CONTACT, TEAM, SESSION]); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 69d673d2c5a..5cf9e96e4f8 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -745,6 +745,28 @@ function writeFailureLogMeta( : { ...(logged as Record | undefined), ...meta }; } +/** + * [#24948] The refusal of a predicate (`multi`) delete of `sys_organization`: + * the standard `DELETE_RESTRICTED`, saying to delete by id. Only the by-id + * delete runs the referential pass that refuses an organization still owning + * rows and keeps every row's reference to it. + */ +function organizationPredicateDeleteRefusal(): Error { + const err: any = new Error( + 'Organizations are deleted one at a time, by id: a delete matching organizations by a filter is refused. ' + + 'Delete each organization by its id.', + ); + err.developerMessage = + `A predicate delete of ${ORGANIZATION_OBJECT} is refused: it runs no referential pass, so it could neither ` + + 'refuse an organization that still owns rows nor keep the rows naming it. Delete each organization by id ' + + `(engine.delete('${ORGANIZATION_OBJECT}', { where: { id } })).`; + err.code = 'DELETE_RESTRICTED'; + err.status = 409; + err.httpStatus = 409; + err.object = ORGANIZATION_OBJECT; + return err; +} + /** * Is this failure the security layer refusing the CALLER's read — the * ADR-0112 `PERMISSION_DENIED` code — rather than the read failing? @@ -19612,6 +19634,10 @@ export class ObjectQL implements IObjectQLEngine { // found no row outside the cascade set: there is nothing to clear, and // clearing is not on offer for this column anyway. if (this.isTenantColumnSetNull(object, child, fieldName, resolvedBehavior)) continue; + // [#24948] A reference the organization's delete keeps as it is, + // naming the deleted organization: never cleared, never counted. + if (object === ORGANIZATION_OBJECT && resolvedBehavior === 'set_null' && + ObjectQL.keepsOrganizationReference(child, fieldName)) continue; let behavior: string = resolvedBehavior; // [#9689] (maintainer ruling 2026-08-19, Q3 = B): the judgement the @@ -20011,6 +20037,82 @@ export class ObjectQL implements IObjectQLEngine { return (child as { managedBy?: unknown })?.managedBy === 'append-only'; } + /** + * [#24948] Does an organization delete keep `child.fieldName`, a `set_null` + * lookup into `sys_organization` that is not `child`'s tenant column, as it + * is? Two kinds of reference, and only these (seat decisions 6305179819 and + * 6305996369): + * + * - every such lookup on an append-only ledger ({@link isAppendOnlyLedger}): + * `sys_audit_log.tenant_id` is the audit log's attribution, not its tenant + * column, and history outlives what it audits; + * - an attribution column named `organization_id` on an object whose + * tenancy is disabled (`resolveTenantFieldName` answers null), such as + * `sys_approval_request`: an engine-owned row an administrator cannot + * delete, so it is neither counted nor cleared. + * + * Everything else keeps its `set_null`: a pointer such as a session's or an + * API key's `active_organization_id`, or a parent-organization lookup, is + * cleared exactly as before. + */ + private static keepsOrganizationReference(child: ServiceObject, fieldName: string): boolean { + if (ObjectQL.isAppendOnlyLedger(child)) return true; + return fieldName === SystemFieldName.ORGANIZATION_ID && resolveTenantFieldName(child) === null; + } + + /** + * [#24948] How many rows of `childName` name the organization `id` through + * `fieldName`, leaving out the rows the cascade itself deletes. A system + * `count`, never a row read, for every table the cascade deletes no row of, + * which is every table but the cascading children of the organization (none + * on the platform). Only for such a table are the rows probed + * ({@link probeReferencingRows}), so the cascade's own members can be left + * out exactly. A table never provisioned holds none. + */ + private async countReferencingRowsOutside( + childName: string, + fieldName: string, + fdef: any, + id: string | number, + cascadeSet: CascadeDeleteSet, + context: ExecutionContext | undefined, + ): Promise<{ where: Record; count: number }> { + const where = this.referenceProbeFilter(fieldName, fdef, id); + if ((cascadeSet.members.get(childName)?.size ?? 0) > 0) { + const probed = await this.probeReferencingRows(childName, fieldName, fdef, id, where, context); + return { where, count: probed.filter((row) => !cascadeSetHas(cascadeSet.members, childName, row?.id)).length }; + } + try { + return { where, count: await this.count(childName, { where, context: ObjectQL.referenceCheckContext(context) }) }; + } catch (error) { + if (isMissingTableError(error, childName)) return { where, count: 0 }; + throw error; + } + } + + /** + * [#24948] May the refusal disclose `count` to the caller? Only when the + * caller's own identity counts the same rows: the elevated count read rows + * the caller may not see. The system is always told. + */ + private async organizationCountIsDisclosable( + childName: string, + where: Record, + count: number, + context: ExecutionContext | undefined, + ): Promise { + if (context?.isSystem === true) return true; + try { + return (await this.count(childName, { where, context })) === count; + } catch (error) { + // The refusal stands either way; only the number is withheld. + this.logger.warn('[cascade-delete] disclosure count failed; the refusal names the table without a count', { + object: childName, error: (error as Error)?.message ?? String(error), + }); + return false; + } + } + /** * [#24948] The engine half of organization delete: refuse while any row * outside the cascade set still names the organization in its tenant column, @@ -20024,11 +20126,12 @@ export class ObjectQL implements IObjectQLEngine { * no row is orphaned to NULL, on a store born with a nullable tenant column * or anywhere else. * - * The probe is the walk's own ({@link probeReferencingRows}: system - * identity, its elevation filed first, a missing table read as no rows). A - * count reaches the refusal only when the caller could see every row it - * counts ({@link dependentCountIsDisclosable}); otherwise the table is named - * without one. The wire code is the standard catalog's `DELETE_RESTRICTED`, + * Each relation is COUNTED ({@link countReferencingRowsOutside}: system + * identity, its elevation filed first, a missing table read as no rows), + * never read row by row, so a large organization costs one `count` per + * table. A count reaches the refusal only when the caller's own identity + * counts the same ({@link organizationCountIsDisclosable}); otherwise the + * table is named without one. The wire code is the standard catalog's `DELETE_RESTRICTED`, * because that is the condition: a delete refused while dependents exist. */ private async refuseOrganizationDeleteWhileOwned( @@ -20046,15 +20149,11 @@ export class ObjectQL implements IObjectQLEngine { for (const [fieldName, fdef] of Object.entries(fields)) { const behavior = this.cascadeRelationBehavior(ORGANIZATION_OBJECT, child, fieldName, fdef); if (!this.isTenantColumnSetNull(ORGANIZATION_OBJECT, child, fieldName, behavior)) continue; - const probeWhere = this.referenceProbeFilter(fieldName, fdef, id); this.fileReferenceCheckElevation(cascadeSet, ORGANIZATION_OBJECT, id, childName, fieldName, context); - const probed = await this.probeReferencingRows(childName, fieldName, fdef, id, probeWhere, context); - const outside = probed.filter((row) => !cascadeSetHas(cascadeSet.members, childName, row?.id)); - if (outside.length === 0) continue; - const disclose = await this.dependentCountIsDisclosable( - childName, probeWhere, probed.map((row: any) => String(row?.id)), fieldName, false, id, context, - ); - owned.push(disclose ? { object: childName, count: outside.length } : { object: childName }); + const { where, count } = await this.countReferencingRowsOutside(childName, fieldName, fdef, id, cascadeSet, context); + if (count === 0) continue; + const disclose = await this.organizationCountIsDisclosable(childName, where, count, context); + owned.push(disclose ? { object: childName, count } : { object: childName }); } } if (owned.length === 0) return; @@ -20378,6 +20477,14 @@ export class ObjectQL implements IObjectQLEngine { // update twin says why; for every pre-#11009 input the two spellings // agree. const isPredicatePath = !isByIdDelete && dispatch.kind === 'multi' && typeof driver.deleteMany === 'function'; + // [#24948] An organization is deleted by id or not at all. The predicate + // arm runs no referential cascade, so it could neither refuse an + // organization that still owns rows nor keep a single row from naming a + // deleted one; any delete not by id is refused here, whatever the driver + // offers, before any hook or write. + if (!isByIdDelete && object === ORGANIZATION_OBJECT) { + throw organizationPredicateDeleteRefusal(); + } if (!isByIdDelete && !isPredicatePath) { // [#4550] The `reject` verdict of resolveEngineDeleteDispatch. It used // to be re-asked after the before phase because a hook could still bind diff --git a/packages/objectql/src/federated-injected-column-readers.test.ts b/packages/objectql/src/federated-injected-column-readers.test.ts index 1938e4597aa..7720b4f443c 100644 --- a/packages/objectql/src/federated-injected-column-readers.test.ts +++ b/packages/objectql/src/federated-injected-column-readers.test.ts @@ -151,6 +151,14 @@ const READERS: Record = { via: 'cascadeRelationBehavior', why: "counts a tenant column only once the cascade's own relation reading admits it, so a federated object's unprovisioned injected column is never counted", }, + 'engine.ts#keepsOrganizationReference :: organization_id': { + disposition: 'not-a-read', + why: "names the attribution column a tenancy-disabled object keeps through an organization delete; the walk reads its relation through cascadeRelationBehavior; it reads no row", + }, + 'engine.ts#keepsOrganizationReference :: resolveTenantFieldName()': { + disposition: 'not-a-read', + why: 'asks whether the object has a tenant column at all, on a relation the walk already read; it reads no row', + }, 'engine.ts#isTenantColumnSetNull :: resolveTenantFieldName()': { disposition: 'not-a-read', why: "compares a field name with the object's tenant column, on a relation its callers already read through cascadeRelationBehavior; it reads no row", diff --git a/packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts b/packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts index 4146339089b..481d8b8cd83 100644 --- a/packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts +++ b/packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts @@ -23,8 +23,10 @@ * - in bulk (`deleteManyData` and `batchData`'s delete case, a per-id loop in * the caller's order, its `atomic` arm one transaction): leaf to root * completes, root first refuses the root alone; - * - an organization's delete: `organization_id` is a plain lookup, so the - * units stay and keep their tree. + * - an organization's delete: [#24948] a business unit is a record the + * organization owns, so the organization is refused while its units remain + * (`DELETE_RESTRICTED`, every unit counted) and the tree is left whole; + * once the units are deleted, leaf to root, the organization deletes. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -158,11 +160,23 @@ describe('#24939 a business unit with child units refuses its delete (real engin expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' }); }); - it("an organization's delete completes on a tree: its units stay, and keep their parents", async () => { + it("[#24948] an organization's delete is refused while it owns units, and the tree is left whole", async () => { const engine = await boot(); await seedTree(engine); - await expect(engine.delete('sys_organization', { where: { id: ORG }, context: SYS })).resolves.toBe(true); + const err: any = await engine.delete('sys_organization', { where: { id: ORG }, context: SYS }).catch((e) => e); + expect(err.code).toBe('DELETE_RESTRICTED'); + expect(err.status).toBe(409); + expect(err.dependents).toEqual([{ object: 'sys_business_unit', count: 3 }]); expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' }); }); + + it('[#24948] CONTROL: once its units are deleted, leaf to root, the organization deletes', async () => { + const engine = await boot(); + await seedTree(engine); + + for (const id of ['bu_desk', 'bu_office', 'bu_emea']) await expect(del(engine, id)).resolves.toBe(true); + await expect(engine.delete('sys_organization', { where: { id: ORG }, context: SYS })).resolves.toBe(true); + expect(await tree(engine)).toEqual({}); + }); }); diff --git a/packages/runtime/src/default-organization-statements.golden.test.ts b/packages/runtime/src/default-organization-statements.golden.test.ts index d83e2f33962..39155fc2c23 100644 --- a/packages/runtime/src/default-organization-statements.golden.test.ts +++ b/packages/runtime/src/default-organization-statements.golden.test.ts @@ -195,12 +195,41 @@ describe('[#24523 / #21868] a sys_organization write retires the memoised Defaul await standing(); // the control above wrote the organization: read it once expect(await standing()).toEqual({ standing: true, organizationReads: 0 }); + // [#24948] CONTROL: an organization rows still name is not deleted, and + // standing survives the refusal. (The delete drops the memo before it is + // judged, so the next request re-reads once; the one after is warm.) + const refused = await engine.delete('sys_organization', { where: { id: old.id }, context: SYS }).then(() => null, (e: any) => e); + expect(refused?.code).toBe('DELETE_RESTRICTED'); + expect(refused?.status).toBe(409); + expect(await find('sys_organization', { id: old.id })).toHaveLength(1); + expect((await standing()).standing).toBe(true); + expect(await standing()).toEqual({ standing: true, organizationReads: 0 }); + + // The ruled remedy: move what the organization owns, then delete it. The + // rows wait in a holding organization while the Default Organization is + // deleted and recreated. + const owned: Array<{ object: string }> = await engine.organizationOwnedRowCounts(old.id); + expect(owned.length, 'PREMISE: rows name the Default Organization').toBeGreaterThan(0); + const holding = await engine.insert('sys_organization', { name: 'Holding', slug: 'holding' }, { context: SYS }); + const moved: Array<{ object: string; id: string }> = []; + // Grants first: moving a membership makes plugin-security grant its + // organization-admin set in the target, which a later grant move would duplicate. + const grantsFirst = [...owned].sort((a, b) => + Number(b.object === 'sys_user_permission_set') - Number(a.object === 'sys_user_permission_set')); + for (const { object } of grantsFirst) { + for (const row of await find(object, { organization_id: old.id })) { + await engine.update(object, { id: row.id, organization_id: holding.id }, { context: SYS }); + moved.push({ object, id: row.id }); + } + } await engine.delete('sys_organization', { where: { id: old.id }, context: SYS }); const created = await engine.insert('sys_organization', { name: 'Default Organization', slug: 'default' }, { context: SYS }); expect(created.id).not.toBe(old.id); - // The promotion's grant now belongs to the recreated organization. - const grant = await adminGrant(); - await engine.update('sys_user_permission_set', { id: grant.id, organization_id: created.id }, { context: SYS }); + // Everything the old one owned, the promotion's grant included, now belongs to the recreated one. + for (const { object, id } of moved) { + await engine.update(object, { id, organization_id: created.id }, { context: SYS }); + } + await engine.delete('sys_organization', { where: { id: holding.id }, context: SYS }); // A memo still answering the deleted id would withhold standing here. expect(await standing()).toEqual({ standing: true, organizationReads: 1 }); From dbc61eff0c8f14fe8258f54e2c9eea16e181142d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:01:09 +0000 Subject: [PATCH 11/18] wip: review round (B3 B6 B8 and the dogfood pins) Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- ...948-organization-delete-refuse-nonempty.md | 8 ++-- .../plugins/plugin-auth/src/auth-manager.ts | 37 ++++++++++++++++++- .../plugins/plugin-auth/src/auth-plugin.ts | 5 ++- .../plugin-auth/src/last-admin-guard.ts | 20 +++++++++- .../plugin-auth/src/objectql-adapter.test.ts | 19 ++++++++++ .../plugin-auth/src/objectql-adapter.ts | 20 ++++++++++ .../plugin-auth/src/organization-delete.ts | 9 +++-- ...n-delete-federated-fixture.dogfood.test.ts | 20 +++++++++- ...ion-delete-refuse-nonempty.dogfood.test.ts | 13 +++++++ 9 files changed, 140 insertions(+), 11 deletions(-) diff --git a/.changeset/24948-organization-delete-refuse-nonempty.md b/.changeset/24948-organization-delete-refuse-nonempty.md index 29072d35deb..cadc0576eec 100644 --- a/.changeset/24948-organization-delete-refuse-nonempty.md +++ b/.changeset/24948-organization-delete-refuse-nonempty.md @@ -10,10 +10,11 @@ fix(objectql, plugin-auth)!: deleting an organization that still owns records is **What happens now.** - An organization that still owns records is refused with `409` and the standard `DELETE_RESTRICTED` code. The body names each table and its row count, `dependents: [{ object, count }]`, and never a row's values. The refusal runs before better-auth's handler, so nothing is written, the caller's active organization included. A caller the organization does not let delete it still gets better-auth's own `403`, with no count. -- Append-only ledgers (objects declared `managedBy: 'append-only'`: `sys_activity`, `sys_audit_log`, `sys_email` and their siblings) are history, not records. They never refuse the delete, and their rows are kept: their `organization_id` goes on naming the deleted organization, never NULL and never deleted. The delete writes activity rows of its own, so counting them would refuse every delete. +- Append-only ledgers (objects declared `managedBy: 'append-only'`, such as `sys_activity`, `sys_audit_log` and `sys_email`) are history, not records. They never refuse the delete, and their rows are kept: every reference they hold to the organization (`organization_id` on `sys_activity` and `sys_email`, the attribution `tenant_id` on `sys_audit_log`) goes on naming the deleted organization, never NULL and never deleted. The delete writes activity rows of its own, so counting them would refuse every delete. +- An attribution column `organization_id` on an object whose tenancy is disabled (such as `sys_approval_request`, an engine-owned row an administrator cannot delete) is kept the same way: not counted, not cleared. A pointer to the organization, such as a session's or an API key's `active_organization_id`, is cleared as before. - An organization that owns nothing but its own auth structure is deleted whole: its members and invitations (better-auth), and its teams, their members and the organization-admin grants its owners and admins hold (plugin-auth, from `beforeDeleteOrganization`). Any other grant scoped to the organization counts as a record. -- The request runs as one engine transaction and rolls back on any error answer, so a delete either removes all of that or none of it. On a datasource whose driver has no transactions, the refusal above is the guarantee, because it runs before the first write. -- Under all of this, the engine itself refuses to delete a `sys_organization` row while any row still names it in its tenant column, whatever path reaches the delete: `409 DELETE_RESTRICTED` with the same `dependents` list. A tenant column is never set to NULL. Other lookups to `sys_organization`, such as a session's active organization, are cleared as before. +- The request runs as one engine transaction and rolls back on any error answer, so a delete either removes all of that or none of it. On a datasource whose driver has no transactions, the refusals are the guarantee, because both run before the first write: the record count above, and, for the Default Organization, the last-administrator guard, whose `403` is now asked before better-auth deletes any member (it used to arrive after the members and invitations were gone). +- Under all of this, the engine itself refuses to delete a `sys_organization` row while any row still names it in its tenant column, whatever path reaches the delete: `409 DELETE_RESTRICTED` with the same `dependents` list, which better-auth's door now carries to the wire too. It counts each table rather than reading its rows. An organization is deleted by id only: a delete matching organizations by a filter (`multi: true`) is refused with `409 DELETE_RESTRICTED` naming the by-id route, because only the by-id delete runs this referential pass. A tenant column is never set to NULL. - `ObjectQL.organizationOwnedRowCounts(organizationId, { exclude })` answers, per object, how many rows name an organization in their tenant column. It is the question the refusal asks. Clause-②: yes (narrowing) @@ -25,4 +26,5 @@ Clause-②: yes (narrowing) - **FROM:** deleting an organization that still owned records answered `500` after deleting its members and invitations (tenant column NOT NULL), or `200` with its teams and records orphaned to `organization_id = NULL` (tenant column nullable, an 18.0.0-born database). A direct engine delete of such a `sys_organization` row did the same. - **TO:** both answer `409 DELETE_RESTRICTED`, with `dependents: [{ object, count }]` naming every table that still holds rows. Nothing is deleted or changed. - **Fix:** delete the organization's records, or move them to another organization, then delete the organization. The refusal names the tables to look at. Business units, positions, sharing rules and organization-level settings count as records; activity, audit and mail-log rows do not. +- **Seed datasets:** on a composition with the organizations runtime (`@objectstack/organizations`) and an app that ships seed datasets, a new organization receives that seed replay at creation, so it owns records from its first moment and its delete is refused until they are removed. The refusal lists them. Delete them (or move them) like any other records, then delete the organization. Measured on the showcase: a fresh organization owns 132 seeded rows in 19 tables. - **Already orphaned on 18.0.0?** Rows an earlier organization delete set to `organization_id = NULL` are not touched by this release. Organization-less rows are what the v18 organization-ownership ceremony (`os migrate organization-ownership`) attributes or reports. diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 4707d3c23a5..82c98bf79ba 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -1645,6 +1645,18 @@ export class AuthManager { * into scope — so the map is not reachable from `ctx` at that point. */ private orgRolesMap: Record | undefined; + /** + * [#24948] The last-administrator guard's question about an organization + * delete (`LastAdminGuardHandle.judgeOrganizationDelete`), handed over by + * `AuthPlugin` once the guard is registered. Absent on a composition without + * the guard, where the Default Organization has no such protection to move. + */ + private organizationDeleteStandingJudge?: (organizationId: string) => Promise; + + /** [#24948] See {@link organizationDeleteStandingJudge}. */ + setOrganizationDeleteStandingJudge(judge: (organizationId: string) => Promise): void { + this.organizationDeleteStandingJudge = judge; + } // ADR-0069 — cached "does any org require MFA" flag (per-org tightening). // Refreshed lazily with a TTL so isAuthGateActive() stays synchronous + cheap. private _orgMfaCache: { value: boolean; at: number } = { value: false, at: 0 }; @@ -8092,7 +8104,8 @@ export class AuthManager { * vendor: no session, no organization id, no membership, a lookup that * failed. The COUNT does not fall through: it is the guard, and a guard that * could not be evaluated must not pass, so its failure propagates. The - * Default Organization stays the last-administrator guard's to answer. + * Default Organization stays the last-administrator guard's to answer: the + * guard's own question is asked here, so its 403 lands before the first write. */ private async assertOrganizationDeletable(ctx: any): Promise { const engine = this.getDataEngine(); @@ -8101,6 +8114,7 @@ export class AuthManager { if (!organizationId) return; let organization: any; + let defaultOrganization = false; try { const actor = await this.resolveActor(ctx); if (!actor?.userId) return; // the vendor answers 401 @@ -8122,7 +8136,7 @@ export class AuthManager { if (!permitted) return; // the vendor answers YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_ORGANIZATION organization = await sys.findOne('sys_organization', { where: { id: organizationId } }); if (!organization) return; // the vendor answers 400 - if (await isDefaultOrganizationTarget(engine, organization, postureEnforcesWall(resolveTenancyPosture()))) return; + defaultOrganization = await isDefaultOrganizationTarget(engine, organization, postureEnforcesWall(resolveTenancyPosture())); } catch { // A lookup that did not complete hands the request back to the vendor, // which re-reads everything it decides; the engine's own refusal still @@ -8130,6 +8144,25 @@ export class AuthManager { return; } + // The Default Organization is the last-administrator guard's to answer, + // and it answers HERE, before the vendor's first write, so its refusal + // leaves every member and invitation as it was on any driver, with or + // without transactions. The guard fails closed on a lookup it cannot + // complete, so this never passes for want of an answer. When it admits + // the delete, the vendor proceeds and the engine's own refusal still holds + // while anything names the organization. + if (defaultOrganization) { + if (!this.organizationDeleteStandingJudge) return; + try { + await this.organizationDeleteStandingJudge(organizationId); + } catch (err) { + if ((err as { code?: unknown } | null)?.code !== 'PERMISSION_DENIED') throw err; + const { APIError } = await import('better-auth/api'); + throw new APIError('FORBIDDEN', { message: (err as Error).message, code: 'PERMISSION_DENIED' }); + } + return; + } + const owned = await organizationDeleteRefusal(engine, organizationId); if (owned && owned.length > 0) { const { APIError } = await import('better-auth/api'); diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 01004ce1674..b6b1c05bdb5 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -1692,10 +1692,13 @@ export class AuthPlugin implements Plugin { // then switch the guard off for all of the above. Every hook registers // at priority 20 so the ADR-0092 checks above (10) still answer first // for user-context callers. See last-admin-guard.ts. - registerLastAdminGuard(engine, { + const lastAdminGuard = registerLastAdminGuard(engine, { packageId: 'com.objectstack.plugin-auth.last-admin-guard', logger: ctx.logger, }); + // [#24948] The same question, asked before an organization delete's + // first write (`AuthManager.assertOrganizationDeletable`). + this.authManager?.setOrganizationDeleteStandingJudge(lastAdminGuard.judgeOrganizationDelete); // [#15784] The COURTESY half of #15409's ruling: when a membership // ends, the session's claim on THAT organization ends with it — // re-pointed if the user still belongs somewhere, revoked if not. diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.ts b/packages/plugins/plugin-auth/src/last-admin-guard.ts index f8ba9876f6b..d7cc153befd 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.ts @@ -869,10 +869,23 @@ function touchesAny(data: Record, keys: readonly string[]): boo * re-binding after a hot reload runs `engine.unregisterHooksByPackage(packageId)` * first. */ +/** + * [#24948] The guard's question about a `sys_organization` delete, askable + * BEFORE the delete starts. better-auth's organization delete removes the + * members and invitations before it deletes the organization row, so the + * `beforeDelete` hook's answer arrives after those writes; plugin-auth asks + * this first, from the request's `hooks.before`, so a refused Default + * Organization delete writes nothing on any driver. The same judgement as the + * hook, `enforceStanding('organization-delete', …)`, never a second spelling. + */ +export interface LastAdminGuardHandle { + judgeOrganizationDelete(organizationId: string): Promise; +} + export function registerLastAdminGuard( engine: LastAdminGuardEngine, opts: LastAdminGuardOptions, -): void { +): LastAdminGuardHandle { const { packageId, logger } = opts; const maxScan = opts.maxScan ?? DEFAULT_MAX_SCAN; @@ -1848,4 +1861,9 @@ export function registerLastAdminGuard( 'Organization the grant anchor reads) and sys_metadata_activation ' + '(switching admin_full_access off) — ADR-0135 D5.2', ); + + return { + judgeOrganizationDelete: (organizationId: string) => + enforceStanding('organization-delete', SystemObjectName.ORGANIZATION, { id: organizationId }), + }; } diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.test.ts b/packages/plugins/plugin-auth/src/objectql-adapter.test.ts index 64842481fd2..c11c7c78467 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.test.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.test.ts @@ -553,6 +553,25 @@ describe('withValidationErrorMapping – ObjectQL ValidationError → better-aut expect(caught.body).not.toHaveProperty('dependentObject'); expect(caught.body).not.toHaveProperty('dependentCount'); expect(caught.body).not.toHaveProperty('developerMessage'); + expect(caught.body).not.toHaveProperty('dependents'); + }); + + it('[#24948] carries an organization refusal\'s dependents list to the wire, a withheld count staying absent', async () => { + const org: any = new Error('Cannot delete this organization: it still owns records in crm_account (2), crm_note.'); + org.code = 'DELETE_RESTRICTED'; + org.status = 409; + org.dependentObject = 'crm_account'; + org.dependentCount = 2; + org.dependents = [{ object: 'crm_account', count: 2 }, { object: 'crm_note' }]; + const adapter = withValidationErrorMapping({ + delete: async () => { + throw org; + }, + }); + + const caught: any = await adapter.delete().catch((e: unknown) => e); + expect(caught.statusCode).toBe(409); + expect(caught.body.dependents).toEqual([{ object: 'crm_account', count: 2 }, { object: 'crm_note' }]); }); }); diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.ts b/packages/plugins/plugin-auth/src/objectql-adapter.ts index 9dd2e48c850..5ed0db89608 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.ts @@ -581,11 +581,30 @@ function isReferentialDeleteRestriction( developerMessage?: string; dependentObject?: string; dependentCount?: number; + dependents?: unknown; } { if (!err || typeof err !== 'object') return false; return (err as { code?: unknown }).code === 'DELETE_RESTRICTED'; } +/** + * [#24948] The per-table list an organization-delete refusal carries + * (`dependents: [{ object, count }]`, the count absent where it is withheld), + * re-read from the engine error so the wire 409 names every table, not only the + * first. `undefined` for any other refusal, which carries no list. + */ +function wireDependents(raw: unknown): Array<{ object: string; count?: number }> | undefined { + if (!Array.isArray(raw)) return undefined; + const out: Array<{ object: string; count?: number }> = []; + for (const d of raw) { + const object = (d as { object?: unknown } | null)?.object; + if (typeof object !== 'string' || object === '') continue; + const count = (d as { count?: unknown }).count; + out.push(typeof count === 'number' ? { object, count } : { object }); + } + return out.length > 0 ? out : undefined; +} + /** * Re-throw `err` as a better-auth `APIError` when it is an ObjectQL validation * failure (400), an engine policy refusal (403) or a referential delete @@ -628,6 +647,7 @@ async function rethrowAsBetterAuthError(err: unknown): Promise { : {}), ...(err.dependentObject ? { dependentObject: err.dependentObject } : {}), ...(typeof err.dependentCount === 'number' ? { dependentCount: err.dependentCount } : {}), + ...(wireDependents(err.dependents) ? { dependents: wireDependents(err.dependents) } : {}), }); } throw err; diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts index c4823b77f11..c0e70605145 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -44,13 +44,16 @@ * 3. **One unit of work.** `AuthManager.handleRequest` runs the whole request * inside one engine transaction and rolls it back on any 4xx/5xx answer, the * subject-erasure wrapper's posture. On a driver without transactions the - * refusal above is the guarantee, since it runs before the first write. + * refusals are the guarantee, since both run before the first write: the + * count above, and the Default Organization's guard question below. * 4. **The engine never clears the tenant column.** Under all of this, * `ObjectQL.cascadeDeleteRelations` refuses a `sys_organization` delete * while any row still names it, whatever path reaches it. * - * The Default Organization is not judged here: the last-administrator guard - * keeps answering for it exactly as before ({@link isDefaultOrganizationTarget}). + * The Default Organization is not counted here ({@link isDefaultOrganizationTarget}): + * the last-administrator guard answers for it, as before, but its question is + * now asked from the same `hooks.before` (`LastAdminGuardHandle`), so its 403 + * also lands before better-auth deletes any member. */ import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; diff --git a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts index 6a1f3d9387a..219a55c905f 100644 --- a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts @@ -140,9 +140,14 @@ describe('[#21910] an organization delete with the showcase federated fixture pr it('the owner deletes the organization: 200, nothing names it any more, and the federated rows are untouched', async () => { const before = await findRows(ql, FEDERATED, {}, 500); + // CONTROL for the recorder (e) relies on: a delete that goes ahead is seen deleting its members. + const attempted: string[] = []; + ql.registerHook('beforeDelete', async () => { attempted.push('sys_member'); }, { object: 'sys_member', packageId: 'dogfood.24948.control' }); const deleted = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: orgId }); + ql.unregisterHooksByPackage('dogfood.24948.control'); expect(deleted.status, await deleted.clone().text()).toBe(200); + expect(attempted.length).toBeGreaterThan(0); expect(await findRows(ql, 'sys_organization', { id: orgId }, 1)).toHaveLength(0); // [#24948] Its own auth structure went with it, and no row was orphaned to it. @@ -159,14 +164,27 @@ describe('[#21910] an organization delete with the showcase federated fixture pr const [defaultOrg] = await findRows(ql, 'sys_organization', { slug: 'default' }, 1); const defaultId = String(defaultOrg.id); const membersBefore = await findRows(ql, 'sys_member', { organization_id: defaultId }, 50); + const invitationsBefore = await findRows(ql, 'sys_invitation', { organization_id: defaultId }, 50); const teamsBefore = await findRows(ql, 'sys_team', { organization_id: defaultId }, 50); + // Every delete the request ATTEMPTS on a member or an invitation, recorded + // at the engine, so the pin does not rest on the request's transaction: + // on a driver without one, an attempted delete would stay deleted. + const attempted: string[] = []; + const RECORDER = 'dogfood.24948.delete-attempts'; + for (const object of ['sys_member', 'sys_invitation']) { + ql.registerHook('beforeDelete', async () => { attempted.push(object); }, { object, packageId: RECORDER }); + } const res = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: defaultId }); + ql.unregisterHooksByPackage(RECORDER); expect(res.status, await res.clone().text()).toBe(403); expect(((await res.json()) as { code?: string }).code).toBe('PERMISSION_DENIED'); // Main answered the same 403 after better-auth had already deleted the - // members; the request is one unit of work now, so they are back. + // members. The guard's question is now asked before the vendor's first + // write, so no member or invitation delete was even attempted. + expect(attempted).toEqual([]); + expect(await findRows(ql, 'sys_invitation', { organization_id: defaultId }, 50)).toEqual(invitationsBefore); expect(await findRows(ql, 'sys_organization', { id: defaultId }, 1)).toHaveLength(1); expect(await findRows(ql, 'sys_member', { organization_id: defaultId }, 50)).toEqual(membersBefore); expect(await findRows(ql, 'sys_team', { organization_id: defaultId }, 50)).toEqual(teamsBefore); diff --git a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts index 8749a125a88..b76b72aa65c 100644 --- a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts @@ -190,6 +190,15 @@ describe('[#24948] organization delete: refused while it owns records, whole whe const orphansBefore = await orphans(); const ledgerBefore = await findRows(ql, 'sys_activity', { organization_id: org.empty }); expect(ledgerBefore.length, 'PREMISE: the activity ledger names the organization from its creation').toBeGreaterThan(0); + // [seat decision 6305996369] The audit log's attribution, `tenant_id`, is no + // tenant column; it is kept too. + const auditBefore = await findRows(ql, 'sys_audit_log', { tenant_id: org.empty }); + expect(auditBefore.length, 'PREMISE: the audit log attributes rows to the organization').toBeGreaterThan(0); + // An engine-owned row on a tenancy-disabled object, attributed by `organization_id`: kept, not counted. + const request = await ql.insert('sys_approval_request', { + process_name: 'expense_approval', object_name: 'showcase_category', record_id: 'rec_24948', + status: 'approved', created_at: new Date().toISOString(), organization_id: org.empty, + }, { context: SYSTEM_CTX }); const res = await deleteOrg(ownerToken, org.empty); @@ -201,6 +210,10 @@ describe('[#24948] organization delete: refused while it owns records, whole whe const keptIds = new Set(ledgerAfter.map((r) => r.id)); expect(ledgerBefore.every((r) => keptIds.has(r.id)), 'every ledger row is kept').toBe(true); expect(await findRows(ql, 'sys_activity', { organization_id: null })).toEqual([]); + const auditAfter = new Set((await findRows(ql, 'sys_audit_log', { tenant_id: org.empty })).map((r) => r.id)); + expect(auditBefore.every((r) => auditAfter.has(r.id)), 'every audit row keeps naming the organization').toBe(true); + const [requestAfter] = await findRows(ql, 'sys_approval_request', { id: request.id }); + expect(requestAfter?.organization_id).toBe(org.empty); expect(await snapshot(org.empty)).toEqual({ sys_organization: 0, sys_member: 0, From 6635c3c672eb6dfad5323bcb37a522315736c5d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:11:04 +0000 Subject: [PATCH 12/18] fix(plugin-auth): the Default Organization's last-administrator question is asked before better-auth deletes any member, judged with the members that leave with it Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- ...t-admin-guard.default-organization.test.ts | 42 +++++++++++++++- .../plugin-auth/src/last-admin-guard.ts | 49 ++++++++++++++++--- ...ion-delete-refuse-nonempty.dogfood.test.ts | 8 +-- 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts b/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts index fb993174e57..ba8c0d9eadc 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts @@ -25,7 +25,7 @@ import { SqlDriver } from '@objectstack/driver-sql'; import { ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; import { resetPlatformAdminEmailMemo } from '@objectstack/core'; -import { registerLastAdminGuard, type LastAdminGuardEngine } from './last-admin-guard.js'; +import { registerLastAdminGuard, type LastAdminGuardEngine, type LastAdminGuardHandle } from './last-admin-guard.js'; import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js'; const POSTURE_ENV = 'OS_TENANCY_POSTURE'; @@ -74,6 +74,8 @@ const OBJECTS = [ ]; let engines: ObjectQL[] = []; +/** The handle the most recent `promotedSingle` registered. */ +let guard: LastAdminGuardHandle; const ambient: Record = {}; beforeEach(() => { @@ -113,7 +115,7 @@ async function promotedSingle(opts: { secondOrganization?: boolean; grantOwner?: for (const o of OBJECTS) engine.registry.registerObject(o as never); await engine.syncSchemas(); bindTestSecurityCatalog(engine, { permissions: [{ name: ADMIN_FULL_ACCESS }] }); - registerLastAdminGuard(engine as unknown as LastAdminGuardEngine, { packageId: 'test.last-admin-guard.default-org' }); + guard = registerLastAdminGuard(engine as unknown as LastAdminGuardEngine, { packageId: 'test.last-admin-guard.default-org' }); await engine.insert('sys_organization', { id: 'org_default', name: 'Default Organization', slug: 'default' }, SYSTEM); if (opts.secondOrganization) { @@ -172,3 +174,39 @@ describe('[ADR-0131 D5] the organization writes that move the anchor off that gr await expect(ban(engine, 'usr_admin')).rejects.toMatchObject(refusal); }); }); + +describe('[#24948] the same question about deleting the Default Organization, asked before anything is deleted', () => { + const members = (engine: ObjectQL) => + engine.find('sys_member', { where: {}, ...SYSTEM } as any) as Promise>>; + + it('refuses when the grant the Default Organization owns is the last standing', async () => { + const engine = await promotedSingle(); + await expect(guard.judgeOrganizationDelete('org_default')).rejects.toMatchObject(refusal); + }); + + it('judges the members that leave with the organization: an owner membership of it is no standing that survives', async () => { + // The administrator's only standing is owning the Default Organization + // (the grant belongs to another organization and anchors nothing). Asked + // before better-auth deletes the members, a question that simulated only + // the organization row would still count that membership and admit. A + // third organization keeps org_b from becoming the only one, which would + // make it the Default Organization and its grant an anchor. + const engine = await promotedSingle({ secondOrganization: true, grantOwner: 'org_b' }); + await engine.insert('sys_organization', { id: 'org_c', name: 'C', slug: 'c' }, SYSTEM); + await engine.insert('sys_member', { id: 'mem_owner', user_id: 'usr_admin', organization_id: 'org_default', role: 'owner' }, SYSTEM); + await expect(ban(engine, 'usr_admin')).rejects.toMatchObject(refusal); // PREMISE: the membership is standing + + await expect(guard.judgeOrganizationDelete('org_default')).rejects.toMatchObject(refusal); + // A question, not a write: the membership is still there. + expect((await members(engine)).map((m) => m.id)).toEqual(['mem_owner']); + }); + + it('CONTROL: with another administrator standing elsewhere, the delete is admitted', async () => { + const engine = await promotedSingle({ secondOrganization: true, grantOwner: 'org_b' }); + await engine.insert('sys_organization', { id: 'org_c', name: 'C', slug: 'c' }, SYSTEM); + await engine.insert('sys_member', { id: 'mem_owner', user_id: 'usr_admin', organization_id: 'org_default', role: 'owner' }, SYSTEM); + await engine.insert('sys_member', { id: 'mem_other', user_id: 'usr_other', organization_id: 'org_b', role: 'owner' }, SYSTEM); + + await expect(guard.judgeOrganizationDelete('org_default')).resolves.toBeUndefined(); + }); +}); diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.ts b/packages/plugins/plugin-auth/src/last-admin-guard.ts index d7cc153befd..a1ac4429611 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.ts @@ -626,6 +626,24 @@ interface PendingStandingWrite { * standing write that addresses no existing row (`ids` is then empty). */ inserted?: Record; + /** + * [#24948] Writes to OTHER tables that land with this one, as one change: + * better-auth's organization delete removes the organization's members in + * the same request that removes the organization row, and the guard asked + * before that request writes anything must judge both. At most one write per + * table; {@link pendingFor} reads the one for a table. + */ + with?: readonly PendingStandingWrite[]; +} + +/** [#24948] The part of `pending` that addresses `table`, if any. */ +function pendingFor( + pending: PendingStandingWrite | undefined, + table: string, +): PendingStandingWrite | undefined { + if (!pending) return undefined; + if (pending.table === table) return pending; + return pending.with?.find((w) => w.table === table); } /** @@ -647,11 +665,12 @@ function applyPending( pending: PendingStandingWrite | undefined, table: string, ): Record | undefined { - if (!pending || pending.table !== table) return row; + const write = pendingFor(pending, table); + if (!write) return row; const id = toId(row.id); - if (!id || !pending.ids.has(id)) return row; - if (!pending.patch) return undefined; // deleted - return { ...row, ...pending.patch }; + if (!id || !write.ids.has(id)) return row; + if (!write.patch) return undefined; // deleted + return { ...row, ...write.patch }; } @@ -958,7 +977,7 @@ export function registerLastAdminGuard( const readAdminSetSwitchedOff = async (op: GuardedOp, pending?: PendingStandingWrite): Promise => { const fields = ['id', 'metadata_type', 'name', 'active']; const rows = await scanLedger(op, { where: { metadata_type: 'permission', name: ADMIN_FULL_ACCESS }, fields }); - const ledgerWrite = pending?.table === METADATA_ACTIVATION ? pending : undefined; + const ledgerWrite = pendingFor(pending, METADATA_ACTIVATION); if (ledgerWrite && ledgerWrite.ids.size > 0) { const seen = new Set(rows.map((row) => toId(row.id))); const missing = [...ledgerWrite.ids].filter((id) => !seen.has(id)); @@ -992,7 +1011,7 @@ export function registerLastAdminGuard( return undefined; } const fields = ['id', 'slug']; - const orgWrite = pending?.table === SystemObjectName.ORGANIZATION ? pending : undefined; + const orgWrite = pendingFor(pending, SystemObjectName.ORGANIZATION); const addressed = async (rows: Array>) => { if (!orgWrite || orgWrite.ids.size === 0) return rows; const seen = new Set(rows.map((row) => toId(row.id))); @@ -1557,6 +1576,7 @@ export function registerLastAdminGuard( | undefined, patch?: Record, inserted?: Record, + companions?: readonly PendingStandingWrite[], ): Promise => { const words = OP_WORDS[op]; // [ADR-0131 D3] An insert addresses no existing row: its pending write is @@ -1569,6 +1589,7 @@ export function registerLastAdminGuard( ids, ...(patch ? { patch } : {}), ...(inserted ? { inserted } : {}), + ...(companions && companions.length > 0 ? { with: companions } : {}), }); await failClosed(op, async () => { const before = await resolveAdminUserIds(op); @@ -1863,7 +1884,21 @@ export function registerLastAdminGuard( ); return { + // Judged as the request performs it: the organization's members leave with + // the row. Asked before either write, the membership half must be in the + // simulation too, or an owner's membership of the very organization being + // deleted would still read as standing. A read that fails refuses, like + // every other lookup the guard makes. judgeOrganizationDelete: (organizationId: string) => - enforceStanding('organization-delete', SystemObjectName.ORGANIZATION, { id: organizationId }), + failClosed('organization-delete', async () => { + const members = await scan('organization-delete', SystemObjectName.MEMBER, { + where: { organization_id: organizationId }, + fields: ['id'], + }); + const memberIds = new Set(members.map((row) => toId(row.id)).filter((id): id is string => !!id)); + await enforceStanding('organization-delete', SystemObjectName.ORGANIZATION, { id: organizationId }, undefined, undefined, [ + { table: SystemObjectName.MEMBER, ids: memberIds }, + ]); + }), }; } diff --git a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts index b76b72aa65c..94ac2915ddf 100644 --- a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts @@ -194,11 +194,6 @@ describe('[#24948] organization delete: refused while it owns records, whole whe // tenant column; it is kept too. const auditBefore = await findRows(ql, 'sys_audit_log', { tenant_id: org.empty }); expect(auditBefore.length, 'PREMISE: the audit log attributes rows to the organization').toBeGreaterThan(0); - // An engine-owned row on a tenancy-disabled object, attributed by `organization_id`: kept, not counted. - const request = await ql.insert('sys_approval_request', { - process_name: 'expense_approval', object_name: 'showcase_category', record_id: 'rec_24948', - status: 'approved', created_at: new Date().toISOString(), organization_id: org.empty, - }, { context: SYSTEM_CTX }); const res = await deleteOrg(ownerToken, org.empty); @@ -212,8 +207,7 @@ describe('[#24948] organization delete: refused while it owns records, whole whe expect(await findRows(ql, 'sys_activity', { organization_id: null })).toEqual([]); const auditAfter = new Set((await findRows(ql, 'sys_audit_log', { tenant_id: org.empty })).map((r) => r.id)); expect(auditBefore.every((r) => auditAfter.has(r.id)), 'every audit row keeps naming the organization').toBe(true); - const [requestAfter] = await findRows(ql, 'sys_approval_request', { id: request.id }); - expect(requestAfter?.organization_id).toBe(org.empty); + expect(await snapshot(org.empty)).toEqual({ sys_organization: 0, sys_member: 0, From c1dd7023983a689eb03b2be319b99a8e657e5b01 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:13:56 +0000 Subject: [PATCH 13/18] test(plugin-auth): drop an unused binding in the Default Organization guard pin Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- .../src/last-admin-guard.default-organization.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts b/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts index ba8c0d9eadc..ac0e8667c98 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts @@ -180,7 +180,7 @@ describe('[#24948] the same question about deleting the Default Organization, as engine.find('sys_member', { where: {}, ...SYSTEM } as any) as Promise>>; it('refuses when the grant the Default Organization owns is the last standing', async () => { - const engine = await promotedSingle(); + await promotedSingle(); await expect(guard.judgeOrganizationDelete('org_default')).rejects.toMatchObject(refusal); }); From ba33df88dacfdfe9e4b572949c928fefe761168f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:29:18 +0000 Subject: [PATCH 14/18] fix(objectql): the organization refusal's disclosure count asks the caller's own count, with no isSystem shortcut Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf Co-authored-by: Claude --- packages/objectql/src/engine.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 5cf9e96e4f8..9b01efb9c64 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -20093,7 +20093,7 @@ export class ObjectQL implements IObjectQLEngine { /** * [#24948] May the refusal disclose `count` to the caller? Only when the * caller's own identity counts the same rows: the elevated count read rows - * the caller may not see. The system is always told. + * the caller may not see. (A system caller's own count is the elevated one.) */ private async organizationCountIsDisclosable( childName: string, @@ -20101,7 +20101,6 @@ export class ObjectQL implements IObjectQLEngine { count: number, context: ExecutionContext | undefined, ): Promise { - if (context?.isSystem === true) return true; try { return (await this.count(childName, { where, context })) === count; } catch (error) { From 979fb8a63c9de73e5f3884dc312d1a1213a0201f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 03:11:21 +0000 Subject: [PATCH 15/18] test(plugin-auth): type the member scan's query options in the #24948 judge pins Keeps the query-options-erasure test surface at its 236 ceiling. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf --- .../src/last-admin-guard.default-organization.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts b/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts index ac0e8667c98..3f4cf65d711 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.default-organization.test.ts @@ -177,7 +177,7 @@ describe('[ADR-0131 D5] the organization writes that move the anchor off that gr describe('[#24948] the same question about deleting the Default Organization, asked before anything is deleted', () => { const members = (engine: ObjectQL) => - engine.find('sys_member', { where: {}, ...SYSTEM } as any) as Promise>>; + engine.find('sys_member', { where: {}, ...SYSTEM }) as Promise>>; it('refuses when the grant the Default Organization owns is the last standing', async () => { await promotedSingle(); From e184b513c426f05a3e764e0500f7afd19cfb22e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 03:36:21 +0000 Subject: [PATCH 16/18] fix(objectql, plugin-auth): engine-owned rows outlive the organization, like the ledgers (#24948) Seat decision 6307135388 (option B): an object declared managedBy 'engine-owned' is neither counted by the organization delete's refusal (engine and door) nor cleared by its walk. Its rows are kept, naming the deleted organization. Pinned in objectql (an approval request and its actions; a record beside them still refuses) and in dogfood. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf --- ...948-organization-delete-refuse-nonempty.md | 6 +- ...engine-organization-delete-refusal.test.ts | 61 ++++++++++++++++- packages/objectql/src/engine.ts | 67 ++++++++++--------- .../plugin-auth/src/organization-delete.ts | 10 +-- ...ion-delete-refuse-nonempty.dogfood.test.ts | 36 +++++++++- 5 files changed, 142 insertions(+), 38 deletions(-) diff --git a/.changeset/24948-organization-delete-refuse-nonempty.md b/.changeset/24948-organization-delete-refuse-nonempty.md index cadc0576eec..205906a7b0a 100644 --- a/.changeset/24948-organization-delete-refuse-nonempty.md +++ b/.changeset/24948-organization-delete-refuse-nonempty.md @@ -11,7 +11,9 @@ fix(objectql, plugin-auth)!: deleting an organization that still owns records is - An organization that still owns records is refused with `409` and the standard `DELETE_RESTRICTED` code. The body names each table and its row count, `dependents: [{ object, count }]`, and never a row's values. The refusal runs before better-auth's handler, so nothing is written, the caller's active organization included. A caller the organization does not let delete it still gets better-auth's own `403`, with no count. - Append-only ledgers (objects declared `managedBy: 'append-only'`, such as `sys_activity`, `sys_audit_log` and `sys_email`) are history, not records. They never refuse the delete, and their rows are kept: every reference they hold to the organization (`organization_id` on `sys_activity` and `sys_email`, the attribution `tenant_id` on `sys_audit_log`) goes on naming the deleted organization, never NULL and never deleted. The delete writes activity rows of its own, so counting them would refuse every delete. -- An attribution column `organization_id` on an object whose tenancy is disabled (such as `sys_approval_request`, an engine-owned row an administrator cannot delete) is kept the same way: not counted, not cleared. A pointer to the organization, such as a session's or an API key's `active_organization_id`, is cleared as before. +- Engine-owned rows (objects declared `managedBy: 'engine-owned'`) are treated the same way. The platform writes them as a consequence of records or activity, and an administrator cannot delete them over the data API: approval requests, approvers and tokens; automation runs; notifications, their receipts and deliveries; outgoing HTTP deliveries; mailboxes and mirrored mail; record shares and share links; import jobs; audience-binding suggestions; and organization-level settings values and their audit. They never refuse the delete, and they are kept, still naming the deleted organization. +- So engine-owned and append-only rows outlive the organization, and go on naming it. Under tenant scoping no organization reads them. +- An attribution column `organization_id` on an object whose tenancy is disabled is kept the same way: not counted, not cleared. A pointer to the organization, such as a session's or an API key's `active_organization_id`, is cleared as before. - An organization that owns nothing but its own auth structure is deleted whole: its members and invitations (better-auth), and its teams, their members and the organization-admin grants its owners and admins hold (plugin-auth, from `beforeDeleteOrganization`). Any other grant scoped to the organization counts as a record. - The request runs as one engine transaction and rolls back on any error answer, so a delete either removes all of that or none of it. On a datasource whose driver has no transactions, the refusals are the guarantee, because both run before the first write: the record count above, and, for the Default Organization, the last-administrator guard, whose `403` is now asked before better-auth deletes any member (it used to arrive after the members and invitations were gone). - Under all of this, the engine itself refuses to delete a `sys_organization` row while any row still names it in its tenant column, whatever path reaches the delete: `409 DELETE_RESTRICTED` with the same `dependents` list, which better-auth's door now carries to the wire too. It counts each table rather than reading its rows. An organization is deleted by id only: a delete matching organizations by a filter (`multi: true`) is refused with `409 DELETE_RESTRICTED` naming the by-id route, because only the by-id delete runs this referential pass. A tenant column is never set to NULL. @@ -25,6 +27,6 @@ Clause-②: yes (narrowing) - **FROM:** deleting an organization that still owned records answered `500` after deleting its members and invitations (tenant column NOT NULL), or `200` with its teams and records orphaned to `organization_id = NULL` (tenant column nullable, an 18.0.0-born database). A direct engine delete of such a `sys_organization` row did the same. - **TO:** both answer `409 DELETE_RESTRICTED`, with `dependents: [{ object, count }]` naming every table that still holds rows. Nothing is deleted or changed. -- **Fix:** delete the organization's records, or move them to another organization, then delete the organization. The refusal names the tables to look at. Business units, positions, sharing rules and organization-level settings count as records; activity, audit and mail-log rows do not. +- **Fix:** delete the organization's records, or move them to another organization, then delete the organization. The refusal names the tables to look at. Business units, positions and sharing rules count as records. Activity, audit and mail-log rows do not, nor do engine-owned rows (approval state, runs, deliveries, shares, settings values). - **Seed datasets:** on a composition with the organizations runtime (`@objectstack/organizations`) and an app that ships seed datasets, a new organization receives that seed replay at creation, so it owns records from its first moment and its delete is refused until they are removed. The refusal lists them. Delete them (or move them) like any other records, then delete the organization. Measured on the showcase: a fresh organization owns 132 seeded rows in 19 tables. - **Already orphaned on 18.0.0?** Rows an earlier organization delete set to `organization_id = NULL` are not touched by this release. Organization-less rows are what the v18 organization-ownership ceremony (`os migrate organization-ownership`) attributes or reports. diff --git a/packages/objectql/src/engine-organization-delete-refusal.test.ts b/packages/objectql/src/engine-organization-delete-refusal.test.ts index b231cde808c..818c9cd0473 100644 --- a/packages/objectql/src/engine-organization-delete-refusal.test.ts +++ b/packages/objectql/src/engine-organization-delete-refusal.test.ts @@ -170,7 +170,7 @@ const AUDIT_LIKE = { }, }; -/** Tenancy disabled, with an `organization_id` attribution column (`sys_approval_request`). */ +/** Tenancy disabled, with an `organization_id` attribution column (the closed rule of seat decision 6305996369, B5). */ const REQUEST_LIKE = { name: 'request_like', label: 'Request', @@ -181,6 +181,28 @@ const REQUEST_LIKE = { }, }; +/** + * An engine-owned object with a tenant column (`sys_approval_request`): the + * platform writes its rows, an administrator cannot delete them, and they + * outlive the organization (seat decision 6307135388). + */ +const APPROVAL_LIKE = { + name: 'approval_like', + label: 'Approval request', + managedBy: 'engine-owned' as const, + fields: { status: { name: 'status', label: 'Status', type: 'text' as const } }, +}; +/** Its actions: an append-only ledger (`sys_approval_action`). */ +const APPROVAL_ACTION_LIKE = { + name: 'approval_action_like', + label: 'Approval action', + managedBy: 'append-only' as const, + fields: { + request_id: { name: 'request_id', label: 'Request', type: 'lookup' as const, reference: 'approval_like' }, + action: { name: 'action', label: 'Action', type: 'text' as const }, + }, +}; + async function makeEngine(objects: any[]) { const engine = new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {} } } as any); const store = makeDriver(); @@ -304,6 +326,43 @@ describe('[#24948] an append-only ledger neither refuses the delete nor is clear }); }); +describe('[#24948] an engine-owned object neither refuses the delete nor is cleared by it (seat decision 6307135388)', () => { + const seedApproval = (store: ReturnType) => { + store.seed('approval_like', { id: 'ar1', status: 'approved', organization_id: ORG_ID }); + store.seed('approval_action_like', { id: 'aa1', request_id: 'ar1', action: 'submit', organization_id: ORG_ID }); + store.seed('approval_action_like', { id: 'aa2', request_id: 'ar1', action: 'approve', organization_id: ORG_ID }); + }; + + it('deletes an organization whose only rows are an approval request and its actions, and keeps them naming it', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, APPROVAL_LIKE, APPROVAL_ACTION_LIKE]); + seedApproval(store); + expect(await engine.organizationOwnedRowCounts(ORG_ID)).toEqual([]); + + expect(await deleteOrg(engine)).toBeNull(); + + expect(store.rows('sys_organization').map((r) => r.id)).toEqual([OTHER_ORG]); + // Kept, and still naming the deleted organization: never NULL, never deleted. + expect(store.rows('approval_like').map((r) => [r.id, r.organization_id])).toEqual([['ar1', ORG_ID]]); + expect(store.rows('approval_action_like').map((r) => [r.id, r.request_id, r.organization_id])).toEqual([ + ['aa1', 'ar1', ORG_ID], + ['aa2', 'ar1', ORG_ID], + ]); + expect(store.writes.filter((w) => w.includes('approval_'))).toEqual([]); + }); + + it('CONTROL: beside them, a record still refuses, and only the record is named', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT, APPROVAL_LIKE, APPROVAL_ACTION_LIKE]); + seedApproval(store); + store.seed('acct', { id: 'a1', organization_id: ORG_ID }); + + const err = await deleteOrg(engine); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(err?.dependents).toEqual([{ object: 'acct', count: 1 }]); + expect(store.writes).toEqual([]); + }); +}); + describe('[#24948] references the delete keeps as they are (seat decision 6305996369)', () => { it("an append-only ledger's non-tenant lookup keeps naming the deleted organization", async () => { const { engine, store } = await makeEngine([ORGANIZATION, AUDIT_LIKE]); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 9b01efb9c64..b121a6654f9 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -20018,45 +20018,52 @@ export class ObjectQL implements IObjectQLEngine { } /** - * [#24948] Is `child` an append-only ledger, whose rows outlive the - * organization they name? Read from the declaration the object already - * carries, `managedBy: 'append-only'` (the audit bucket: `sys_activity`, - * `sys_audit_log`, `sys_email` and their siblings), never from a list of - * names. + * [#24948] Do `child`'s rows outlive the organization they name? Read from + * the declaration the object already carries, never from a list of names: * - * Such a row is history, not a record an administrator manages: it ages out - * on its own retention, or is the delivery log of something that already - * happened. The organization delete itself writes `sys_activity` rows, so a - * ledger that counted would refuse every organization delete, the empty ones - * included. Neither refusal counts a ledger. Its rows are kept, and so is - * their tenant column: it goes on naming the deleted organization, never - * NULL (ADR-0131 D1) and never deleted (ADR-0131 §8), and under tenant - * scoping the rows read as no organization's (seat decision 6305179819). + * - `managedBy: 'append-only'`, a ledger (the audit bucket: `sys_activity`, + * `sys_audit_log`, `sys_email` and their siblings). Such a row is history: + * it ages out on its own retention, or is the delivery log of something + * that already happened. The organization delete itself writes + * `sys_activity` rows, so a ledger that counted would refuse every + * organization delete, the empty ones included (seat decision 6305179819). + * - `managedBy: 'engine-owned'`, a row the platform writes as a consequence + * of records or activity (approval state, automation runs, notification + * and HTTP deliveries, mirrored mail, record shares, import jobs, + * settings values), which an administrator cannot delete over the data + * API. Counting it would make any organization that has been used + * undeletable through every door (seat decision 6307135388). + * + * Neither refusal counts such a row. It is kept, and so is every reference it + * holds to the organization: it goes on naming the deleted organization, + * never NULL (ADR-0131 D1) and never deleted (ADR-0131 §8), and under tenant + * scoping the row reads as no organization's. */ - private static isAppendOnlyLedger(child: ServiceObject): boolean { - return (child as { managedBy?: unknown })?.managedBy === 'append-only'; + private static outlivesOrganization(child: ServiceObject): boolean { + const managedBy = (child as { managedBy?: unknown })?.managedBy; + return managedBy === 'append-only' || managedBy === 'engine-owned'; } /** * [#24948] Does an organization delete keep `child.fieldName`, a `set_null` * lookup into `sys_organization` that is not `child`'s tenant column, as it - * is? Two kinds of reference, and only these (seat decisions 6305179819 and - * 6305996369): + * is? Two kinds of reference, and only these (seat decisions 6305179819, + * 6305996369 and 6307135388): * - * - every such lookup on an append-only ledger ({@link isAppendOnlyLedger}): - * `sys_audit_log.tenant_id` is the audit log's attribution, not its tenant - * column, and history outlives what it audits; + * - every such lookup on an object whose rows outlive the organization + * ({@link outlivesOrganization}): `sys_audit_log.tenant_id` is the audit + * log's attribution, not its tenant column, and history outlives what it + * audits; * - an attribution column named `organization_id` on an object whose - * tenancy is disabled (`resolveTenantFieldName` answers null), such as - * `sys_approval_request`: an engine-owned row an administrator cannot - * delete, so it is neither counted nor cleared. + * tenancy is disabled (`resolveTenantFieldName` answers null): neither + * counted nor cleared. No object in this tree declares one today. * * Everything else keeps its `set_null`: a pointer such as a session's or an * API key's `active_organization_id`, or a parent-organization lookup, is * cleared exactly as before. */ private static keepsOrganizationReference(child: ServiceObject, fieldName: string): boolean { - if (ObjectQL.isAppendOnlyLedger(child)) return true; + if (ObjectQL.outlivesOrganization(child)) return true; return fieldName === SystemFieldName.ORGANIZATION_ID && resolveTenantFieldName(child) === null; } @@ -20115,7 +20122,7 @@ export class ObjectQL implements IObjectQLEngine { /** * [#24948] The engine half of organization delete: refuse while any row * outside the cascade set still names the organization in its tenant column, - * an append-only ledger's excepted ({@link isAppendOnlyLedger}). + * a row that outlives the organization excepted ({@link outlivesOrganization}). * * Runs before the walk touches the first relation, and reads every tenant * relation before it throws, so the one refusal names every table that holds @@ -20143,8 +20150,8 @@ export class ObjectQL implements IObjectQLEngine { for (const child of objects) { const childName = (child as any)?.name as string | undefined; const fields = (child as any)?.fields as Record | undefined; - // An append-only ledger's rows outlive the organization: not counted, kept as they are. - if (!childName || !fields || ObjectQL.isAppendOnlyLedger(child)) continue; + // An append-only or engine-owned object's rows outlive the organization: not counted, kept as they are. + if (!childName || !fields || ObjectQL.outlivesOrganization(child)) continue; for (const [fieldName, fdef] of Object.entries(fields)) { const behavior = this.cascadeRelationBehavior(ORGANIZATION_OBJECT, child, fieldName, fdef); if (!this.isTenantColumnSetNull(ORGANIZATION_OBJECT, child, fieldName, behavior)) continue; @@ -20194,8 +20201,8 @@ export class ObjectQL implements IObjectQLEngine { * row. * * `exclude` names the objects the caller removes as part of the delete - * itself. An append-only ledger is never counted ({@link isAppendOnlyLedger}), - * the same set the engine's own refusal leaves out. + * itself. An append-only or engine-owned object is never counted ({@link + * outlivesOrganization}), the same set the engine's own refusal leaves out. */ async organizationOwnedRowCounts( organizationId: string, @@ -20206,7 +20213,7 @@ export class ObjectQL implements IObjectQLEngine { for (const child of this._registry.getAllObjects()) { const childName = (child as any)?.name as string | undefined; const fields = (child as any)?.fields as Record | undefined; - if (!childName || !fields || exclude.has(childName) || ObjectQL.isAppendOnlyLedger(child)) continue; + if (!childName || !fields || exclude.has(childName) || ObjectQL.outlivesOrganization(child)) continue; const tenantField = resolveTenantFieldName(child); if (!tenantField || !fields[tenantField]) continue; const fdef = fields[tenantField]; diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts index c0e70605145..f7df57672a8 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -25,10 +25,12 @@ * 1. **Refused while the organization owns records.** {@link * organizationDeleteRefusal} counts, per table, the rows that name the * organization in their tenant column, leaving out the organization's own - * auth structure and every append-only ledger (`managedBy: 'append-only'`: - * `sys_activity`, `sys_email` and their siblings), whose rows are history, - * are kept, and go on naming the deleted organization (seat decision - * 6305179819; the engine reads the same set). Any count above zero refuses with 409 + * auth structure and every object declared `managedBy: 'append-only'` + * (ledgers: `sys_activity`, `sys_email` and their siblings) or + * `managedBy: 'engine-owned'` (rows the platform writes: approval state, + * runs, deliveries, shares), whose rows are kept and go on naming the + * deleted organization (seat decisions 6305179819 and 6307135388; the + * engine reads the same set). Any count above zero refuses with 409 * `DELETE_RESTRICTED` and `dependents: [{ object, count }]`: counts, never a * row value. It runs in `hooks.before`, ahead of the vendor's handler, and * only for a caller the vendor itself would admit (a member holding the diff --git a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts index 94ac2915ddf..e07b8d37e55 100644 --- a/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-refuse-nonempty.dogfood.test.ts @@ -28,6 +28,9 @@ * grant) deletes fully, and nothing is left naming it but the append-only * `sys_activity` ledger, whose rows are kept (seat decision 6305179819). The * boot composes the audit plugin, as `os serve` does, so that ledger is live. + * - An organization whose only rows are an approval request (engine-owned) + * and its actions (append-only) deletes too, and those rows are kept, still + * naming it (seat decision 6307135388). * - An organization holding a business unit is refused like one holding any * other record. * - (d) The engine refuses the same delete on its own, whatever path reaches @@ -59,7 +62,7 @@ describe('[#24948] organization delete: refused while it owns records, whole whe let ownerToken: string; let adminToken: string; let ownerId: string; - const org = { empty: '', full: '', unit: '' }; + const org = { empty: '', full: '', unit: '', approval: '' }; /** Every count the delete may touch, for one organization. */ const snapshot = async (organizationId: string) => { @@ -119,6 +122,19 @@ describe('[#24948] organization delete: refused while it owns records, whole whe org.empty = await createOrg('Org Delete Empty', 'org-delete-empty'); org.full = await createOrg('Org Delete Full', 'org-delete-full'); org.unit = await createOrg('Org Delete Unit', 'org-delete-unit'); + org.approval = await createOrg('Org Delete Approval', 'org-delete-approval'); + // [seat decision 6307135388] What an approval leaves behind: the request + // (engine-owned) and its actions (append-only). The platform writes both. + const now = new Date().toISOString(); + await ql.insert('sys_approval_request', { + id: 'apr_org_delete', organization_id: org.approval, process_name: 'expense_approval', + object_name: 'showcase_category', record_id: 'cat_gone', status: 'approved', created_at: now, + }, { context: SYSTEM_CTX }); + for (const [id, action] of [['apa_org_delete_1', 'submit'], ['apa_org_delete_2', 'approve']]) { + await ql.insert('sys_approval_action', { + id, organization_id: org.approval, request_id: 'apr_org_delete', action, created_at: now, + }, { context: SYSTEM_CTX }); + } // A business unit is a record its administrator created, not structure. await ql.insert('sys_business_unit', { name: 'Field Ops', kind: 'department', organization_id: org.unit }, { context: SYSTEM_CTX }); for (const id of [org.empty, org.full]) { @@ -221,6 +237,24 @@ describe('[#24948] organization delete: refused while it owns records, whole whe expect(await orphans()).toEqual(orphansBefore); }); + it('an organization whose only rows are an approval request and its actions deletes, and keeps them naming it', async () => { + const requestsBefore = await findRows(ql, 'sys_approval_request', { organization_id: org.approval }); + const actionsBefore = await findRows(ql, 'sys_approval_action', { organization_id: org.approval }); + expect(requestsBefore.map((r) => r.id), 'PREMISE: the request names the organization').toEqual(['apr_org_delete']); + expect(actionsBefore.length, 'PREMISE: its actions name the organization').toBe(2); + + const res = await deleteOrg(ownerToken, org.approval); + + // [seat decision 6307135388] Engine-owned and append-only rows refuse + // nothing, and are kept, still naming the deleted organization. + expect(res.status, await res.clone().text()).toBe(200); + expect(await findRows(ql, 'sys_organization', { id: org.approval })).toEqual([]); + expect(await findRows(ql, 'sys_approval_request', { organization_id: org.approval })).toEqual(requestsBefore); + expect(await findRows(ql, 'sys_approval_action', { organization_id: org.approval })).toEqual(actionsBefore); + expect(await findRows(ql, 'sys_approval_request', { organization_id: null })).toEqual([]); + expect(await findRows(ql, 'sys_approval_action', { organization_id: null })).toEqual([]); + }); + it('an organization holding only a business unit is refused 409 with its count, and nothing changes', async () => { const before = await snapshot(org.unit); const units = await findRows(ql, 'sys_business_unit', { organization_id: org.unit }); From e40aea862649a71529159c72f38ab2423cb15e21 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 04:35:13 +0000 Subject: [PATCH 17/18] fix(objectql, plugin-auth): the Default Organization's delete is counted before the first write (#24948) Delta review 6308041940: - R1: once the last-administrator guard admits the Default Organization's delete, its record count runs in hooks.before too, leaving out only the members and invitations better-auth removes; a refusal is the same 409. - R2: the organization row counts run as the system with none of the caller's tenant scope, so a foreign tenantId cannot undercount. - R3: deciding whether the target is the Default Organization is outside the fall-through, so a failed lookup refuses. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf --- ...948-organization-delete-refuse-nonempty.md | 5 +- ...engine-organization-delete-refusal.test.ts | 42 ++++++++ packages/objectql/src/engine.ts | 21 +++- .../plugins/plugin-auth/src/auth-manager.ts | 36 ++++--- .../src/organization-delete.test.ts | 24 +++++ .../plugin-auth/src/organization-delete.ts | 47 +++++++-- ...n-delete-federated-fixture.dogfood.test.ts | 95 +++++++++++++++++++ 7 files changed, 245 insertions(+), 25 deletions(-) diff --git a/.changeset/24948-organization-delete-refuse-nonempty.md b/.changeset/24948-organization-delete-refuse-nonempty.md index 205906a7b0a..9934930750e 100644 --- a/.changeset/24948-organization-delete-refuse-nonempty.md +++ b/.changeset/24948-organization-delete-refuse-nonempty.md @@ -5,7 +5,7 @@ fix(objectql, plugin-auth)!: deleting an organization that still owns records is refused with 409 and a per-table count, before anything is deleted; an empty one is deleted whole, its teams and organization-admin grants with it; the engine never clears a tenant column -**The 18.0.0 defect this fixes.** `POST /api/v1/auth/organization/delete` ran better-auth's delete, which removes the organization's members, then its invitations, then the organization row, as three separate writes. The engine's delete cascade then applied the tenant column's `set_null` default to every row the organization still owned. On a database whose tenant column is NOT NULL (one that took the v18 organization-ownership ceremony), the delete answered 500 on `sys_team.organization_id` after the members and invitations were already gone, so the organization survived with no owner and nobody could delete it. On a database 18.0.0 created, where the column is still nullable, the delete answered 200 and left the organization's teams and records with `organization_id = NULL`, owned by nobody and visible to nobody. Every organization created through the door owns a default team, so on a NOT NULL database not even an empty organization could be deleted. Deployments that run `single` are not reached: their one organization keeps the last-administrator guard's 403, and they cannot create another. +**The 18.0.0 defect this fixes.** `POST /api/v1/auth/organization/delete` ran better-auth's delete, which removes the organization's members, then its invitations, then the organization row, as three separate writes. The engine's delete cascade then applied the tenant column's `set_null` default to every row the organization still owned. On a database whose tenant column is NOT NULL (one that took the v18 organization-ownership ceremony), the delete answered 500 on `sys_team.organization_id` after the members and invitations were already gone, so the organization survived with no owner and nobody could delete it. On a database 18.0.0 created, where the column is still nullable, the delete answered 200 and left the organization's teams and records with `organization_id = NULL`, owned by nobody and visible to nobody. Every organization created through the door owns a default team, so on a NOT NULL database not even an empty organization could be deleted. Deployments that run `single` are not reached the same way: their one organization is the Default Organization, which the last-administrator guard answers first, and they cannot create another. **What happens now.** @@ -14,8 +14,9 @@ fix(objectql, plugin-auth)!: deleting an organization that still owns records is - Engine-owned rows (objects declared `managedBy: 'engine-owned'`) are treated the same way. The platform writes them as a consequence of records or activity, and an administrator cannot delete them over the data API: approval requests, approvers and tokens; automation runs; notifications, their receipts and deliveries; outgoing HTTP deliveries; mailboxes and mirrored mail; record shares and share links; import jobs; audience-binding suggestions; and organization-level settings values and their audit. They never refuse the delete, and they are kept, still naming the deleted organization. - So engine-owned and append-only rows outlive the organization, and go on naming it. Under tenant scoping no organization reads them. - An attribution column `organization_id` on an object whose tenancy is disabled is kept the same way: not counted, not cleared. A pointer to the organization, such as a session's or an API key's `active_organization_id`, is cleared as before. +- The Default Organization (slug `default`, or the deployment's only organization, never under an organization wall) is asked the last-administrator guard's question first, and answers `403 PERMISSION_DENIED` while deleting it would leave no platform administrator. Once the guard admits, it is counted like any other organization, before better-auth deletes anything, except that only its members and invitations are left out: its teams and grants are not removed, so they count, and the delete is refused `409 DELETE_RESTRICTED` while it holds them. - An organization that owns nothing but its own auth structure is deleted whole: its members and invitations (better-auth), and its teams, their members and the organization-admin grants its owners and admins hold (plugin-auth, from `beforeDeleteOrganization`). Any other grant scoped to the organization counts as a record. -- The request runs as one engine transaction and rolls back on any error answer, so a delete either removes all of that or none of it. On a datasource whose driver has no transactions, the refusals are the guarantee, because both run before the first write: the record count above, and, for the Default Organization, the last-administrator guard, whose `403` is now asked before better-auth deletes any member (it used to arrive after the members and invitations were gone). +- The request runs as one engine transaction and rolls back on any error answer, so a delete either removes all of that or none of it. On a datasource whose driver has no transactions, the refusals are the guarantee, because all of them run before the first write: the record count above, and, for the Default Organization, the last-administrator guard, whose `403` is now asked before better-auth deletes any member (it used to arrive after the members and invitations were gone). - Under all of this, the engine itself refuses to delete a `sys_organization` row while any row still names it in its tenant column, whatever path reaches the delete: `409 DELETE_RESTRICTED` with the same `dependents` list, which better-auth's door now carries to the wire too. It counts each table rather than reading its rows. An organization is deleted by id only: a delete matching organizations by a filter (`multi: true`) is refused with `409 DELETE_RESTRICTED` naming the by-id route, because only the by-id delete runs this referential pass. A tenant column is never set to NULL. - `ObjectQL.organizationOwnedRowCounts(organizationId, { exclude })` answers, per object, how many rows name an organization in their tenant column. It is the question the refusal asks. diff --git a/packages/objectql/src/engine-organization-delete-refusal.test.ts b/packages/objectql/src/engine-organization-delete-refusal.test.ts index 818c9cd0473..b00ecacd679 100644 --- a/packages/objectql/src/engine-organization-delete-refusal.test.ts +++ b/packages/objectql/src/engine-organization-delete-refusal.test.ts @@ -388,6 +388,48 @@ describe('[#24948] references the delete keeps as they are (seat decision 630599 }); }); +describe('[#24948] the organization counts carry none of the caller\'s tenant scope (delta review R2)', () => { + /** + * A tenant layer: scopes every `acct` read whose context carries a + * `tenantId` to that organization, as a tenant-scoping middleware does. A caller acting in + * OTHER_ORG must not read 0 rows for ORG_ID and pass the refusal. + */ + const scopeByTenant = (engine: ObjectQL) => + engine.registerMiddleware(async (opCtx: any, next: () => Promise) => { + const tenantId = opCtx.context?.tenantId; + if (tenantId && opCtx.object === 'acct' && opCtx.ast) { + opCtx.ast.where = { ...(opCtx.ast.where ?? {}), organization_id: tenantId }; + } + await next(); + }); + const foreign = { userId: 'u1', tenantId: OTHER_ORG } as any; + + it('organizationOwnedRowCounts counts the organization\'s rows under a foreign tenant scope', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); + scopeByTenant(engine); + store.seed('acct', { id: 'a1', organization_id: ORG_ID }); + store.seed('acct', { id: 'a2', organization_id: ORG_ID }); + // CONTROL: the tenant layer is live, and scopes this caller's own count away. + expect(await engine.count('acct', { where: { organization_id: ORG_ID }, context: foreign })).toBe(0); + + expect(await engine.organizationOwnedRowCounts(ORG_ID, { context: foreign })).toEqual([{ object: 'acct', count: 2 }]); + }); + + it('the engine refusal still refuses a caller scoped to another organization', async () => { + const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); + scopeByTenant(engine); + store.seed('acct', { id: 'a1', organization_id: ORG_ID }); + + const err: any = await engine + .delete('sys_organization', { where: { id: ORG_ID }, context: { ...foreign, isSystem: true } } as any) + .then(() => null, (e: any) => e); + + expect(err?.code).toBe('DELETE_RESTRICTED'); + expect(err?.dependents?.map((d: { object: string }) => d.object)).toEqual(['acct']); + expect(store.writes).toEqual([]); + }); +}); + describe('[#24948] the predicate path is refused for an organization', () => { it('a multi delete of sys_organization is refused, by id only, before anything is written', async () => { const { engine, store } = await makeEngine([ORGANIZATION, ACCOUNT]); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index b121a6654f9..9f2a413df73 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -20090,13 +20090,30 @@ export class ObjectQL implements IObjectQLEngine { return { where, count: probed.filter((row) => !cascadeSetHas(cascadeSet.members, childName, row?.id)).length }; } try { - return { where, count: await this.count(childName, { where, context: ObjectQL.referenceCheckContext(context) }) }; + return { where, count: await this.count(childName, { where, context: ObjectQL.organizationCountContext(context) }) }; } catch (error) { if (isMissingTableError(error, childName)) return { where, count: 0 }; throw error; } } + /** + * [#24948] The context an organization's row counts run under: system + * identity, and nothing of the caller's scope. A `tenantId` carried in from + * a caller acting in another organization would scope the count to THAT + * organization and read 0 rows naming this one, so the refusal would pass on + * an undercount. Only the request's transaction and trace are kept: the + * count must see the request's own uncommitted writes. The ordinary + * reference probes keep the caller's scope ({@link referenceCheckContext}). + */ + private static organizationCountContext(context?: ExecutionContext): ExecutionContext { + return { + isSystem: true, + ...(context?.transaction !== undefined ? { transaction: context.transaction } : {}), + ...(context?.traceId !== undefined ? { traceId: context.traceId } : {}), + } as ExecutionContext; + } + /** * [#24948] May the refusal disclose `count` to the caller? Only when the * caller's own identity counts the same rows: the elevated count read rows @@ -20222,7 +20239,7 @@ export class ObjectQL implements IObjectQLEngine { try { count = await this.count(childName, { where: this.referenceProbeFilter(tenantField, fdef, organizationId), - context: ObjectQL.referenceCheckContext(options?.context), + context: ObjectQL.organizationCountContext(options?.context), }); } catch (error) { if (isMissingTableError(error, childName)) continue; diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 2174ac144b5..9bf5c29d981 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -212,6 +212,7 @@ import { import { ORGANIZATION_DELETE_PATH, ORGANIZATION_DELETE_REFUSAL_CODE, + defaultOrganizationDeleteRefusal, isDefaultOrganizationTarget, organizationDeleteRefusal, organizationNotEmptyBody, @@ -8112,9 +8113,11 @@ export class AuthManager { * refusal, and no count. Everything this cannot decide falls through to the * vendor: no session, no organization id, no membership, a lookup that * failed. The COUNT does not fall through: it is the guard, and a guard that - * could not be evaluated must not pass, so its failure propagates. The - * Default Organization stays the last-administrator guard's to answer: the - * guard's own question is asked here, so its 403 lands before the first write. + * could not be evaluated must not pass, so its failure propagates, and so + * does a failure deciding whether the target is the Default Organization. + * The Default Organization is asked the last-administrator guard's question + * first (a 403), then its own record count (the same 409), both here, before + * the first write. */ private async assertOrganizationDeletable(ctx: any): Promise { const engine = this.getDataEngine(); @@ -8123,7 +8126,6 @@ export class AuthManager { if (!organizationId) return; let organization: any; - let defaultOrganization = false; try { const actor = await this.resolveActor(ctx); if (!actor?.userId) return; // the vendor answers 401 @@ -8145,7 +8147,6 @@ export class AuthManager { if (!permitted) return; // the vendor answers YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_ORGANIZATION organization = await sys.findOne('sys_organization', { where: { id: organizationId } }); if (!organization) return; // the vendor answers 400 - defaultOrganization = await isDefaultOrganizationTarget(engine, organization, postureEnforcesWall(resolveTenancyPosture())); } catch { // A lookup that did not complete hands the request back to the vendor, // which re-reads everything it decides; the engine's own refusal still @@ -8153,15 +8154,23 @@ export class AuthManager { return; } - // The Default Organization is the last-administrator guard's to answer, - // and it answers HERE, before the vendor's first write, so its refusal + // Which of the two refusals below applies is part of the guard, so it is + // decided outside the fall-through above: a lookup that fails here + // refuses, never hands the request to the vendor unasked. + const defaultOrganization = await isDefaultOrganizationTarget( + engine, organization, postureEnforcesWall(resolveTenancyPosture()), + ); + + // The Default Organization is asked the last-administrator guard's + // question first, HERE, before the vendor's first write, so its refusal // leaves every member and invitation as it was on any driver, with or // without transactions. The guard fails closed on a lookup it cannot // complete, so this never passes for want of an answer. When it admits - // the delete, the vendor proceeds and the engine's own refusal still holds - // while anything names the organization. - if (defaultOrganization) { - if (!this.organizationDeleteStandingJudge) return; + // the delete, the record count below is asked before the vendor's first + // write too, leaving out only what the vendor removes (members and + // invitations): plugin-auth removes none of the Default Organization's + // structure, so its teams and grants count. + if (defaultOrganization && this.organizationDeleteStandingJudge) { try { await this.organizationDeleteStandingJudge(organizationId); } catch (err) { @@ -8169,10 +8178,11 @@ export class AuthManager { const { APIError } = await import('better-auth/api'); throw new APIError('FORBIDDEN', { message: (err as Error).message, code: 'PERMISSION_DENIED' }); } - return; } - const owned = await organizationDeleteRefusal(engine, organizationId); + const owned = defaultOrganization + ? await defaultOrganizationDeleteRefusal(engine, organizationId) + : await organizationDeleteRefusal(engine, organizationId); if (owned && owned.length > 0) { const { APIError } = await import('better-auth/api'); throw new APIError('CONFLICT', { code: ORGANIZATION_DELETE_REFUSAL_CODE, ...organizationNotEmptyBody(owned) }); diff --git a/packages/plugins/plugin-auth/src/organization-delete.test.ts b/packages/plugins/plugin-auth/src/organization-delete.test.ts index 03f49148c96..4c5f5afb803 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.test.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.test.ts @@ -16,6 +16,8 @@ import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/s import { ORGANIZATION_AUTH_STRUCTURE, ORGANIZATION_DELETE_REFUSAL_CODE, + VENDOR_REMOVED_ON_DELETE, + defaultOrganizationDeleteRefusal, isDefaultOrganizationTarget, organizationDeleteRefusal, organizationNotEmptyBody, @@ -105,6 +107,28 @@ describe('[#24948] organizationDeleteRefusal: the records the organization still }); }); +describe('[#24948] defaultOrganizationDeleteRefusal: the Default Organization\'s count, once the guard admits', () => { + it('leaves out only what better-auth removes, so its teams and every grant count', async () => { + const owned = [{ object: 'sys_team', count: 1 }, { object: 'sys_user_permission_set', count: 2 }]; + const { engine, asked, calls } = makeEngine({ sys_user_permission_set: [{ organization_id: ORG }] }, owned); + expect(await defaultOrganizationDeleteRefusal(engine, ORG)).toEqual(owned); + expect(asked).toEqual([{ organizationId: ORG, exclude: ['sys_member', 'sys_invitation'] }]); + expect([...VENDOR_REMOVED_ON_DELETE]).toEqual(['sys_member', 'sys_invitation']); + // No organization-admin grant is subtracted: plugin-auth removes none of them here. + expect(calls).toEqual([]); + }); + + it('answers undefined for an engine that cannot count, and propagates a count that fails', async () => { + const { engine } = makeEngine({}); + delete engine.organizationOwnedRowCounts; + expect(await defaultOrganizationDeleteRefusal(engine, ORG)).toBeUndefined(); + const failing = makeEngine({}).engine; + const fault = new Error('connection lost'); + failing.organizationOwnedRowCounts = async () => { throw fault; }; + await expect(defaultOrganizationDeleteRefusal(failing, ORG)).rejects.toBe(fault); + }); +}); + describe('[#24948] organizationNotEmptyBody: the 409 body', () => { it('names every table with its count and the remedy, beside the standard DELETE_RESTRICTED code', () => { const body = organizationNotEmptyBody([ diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts index f7df57672a8..8aff7beba35 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -46,16 +46,20 @@ * 3. **One unit of work.** `AuthManager.handleRequest` runs the whole request * inside one engine transaction and rolls it back on any 4xx/5xx answer, the * subject-erasure wrapper's posture. On a driver without transactions the - * refusals are the guarantee, since both run before the first write: the - * count above, and the Default Organization's guard question below. + * refusals are the guarantee, since every one runs before the first write: + * the count above, and the Default Organization's guard question and count + * below. * 4. **The engine never clears the tenant column.** Under all of this, * `ObjectQL.cascadeDeleteRelations` refuses a `sys_organization` delete * while any row still names it, whatever path reaches it. * - * The Default Organization is not counted here ({@link isDefaultOrganizationTarget}): - * the last-administrator guard answers for it, as before, but its question is - * now asked from the same `hooks.before` (`LastAdminGuardHandle`), so its 403 - * also lands before better-auth deletes any member. + * The Default Organization ({@link isDefaultOrganizationTarget}) is asked two + * questions instead, both from the same `hooks.before`, so both land before + * better-auth deletes any member: first the last-administrator guard's + * (`LastAdminGuardHandle`, a 403), then, once the guard admits, its own record + * count ({@link defaultOrganizationDeleteRefusal}, the same 409). Its auth + * structure beyond members and invitations is not removed, so its teams and + * grants count. */ import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; @@ -154,6 +158,32 @@ export async function organizationDeleteRefusal( return owned; } +/** + * The tables better-auth itself removes when it deletes an organization: its + * members and its invitations. For the Default Organization they are all + * that is removed (plugin-auth removes none of its structure), so every other + * row that names it counts, its teams and grants included. + */ +export const VENDOR_REMOVED_ON_DELETE: readonly string[] = ['sys_member', 'sys_invitation']; + +/** + * The Default Organization's record count, asked after the last-administrator + * guard admits its delete and before better-auth's first write: every table + * holding rows that name it, but for its members and invitations + * ({@link VENDOR_REMOVED_ON_DELETE}). Its teams and grants are not removed, so + * they count: without this, an admitted delete would remove the members and + * invitations and only then meet the engine's refusal, a partial delete on a + * driver without transactions. `undefined` when the engine cannot answer. + */ +export async function defaultOrganizationDeleteRefusal( + engine: IDataEngine, + organizationId: string, +): Promise { + const counting = engine as CountingEngine; + if (typeof counting.organizationOwnedRowCounts !== 'function') return undefined; + return counting.organizationOwnedRowCounts(organizationId, { exclude: VENDOR_REMOVED_ON_DELETE }); +} + /** * The 409 body for a refused delete, beside its code * ({@link ORGANIZATION_DELETE_REFUSAL_CODE}, spelled at the throw site): every @@ -178,8 +208,9 @@ export function organizationNotEmptyBody(owned: readonly OrganizationOwnedRows[] * only organization? The last-administrator guard's own reading of the anchor * (`last-admin-guard.ts`, `defaultOrganizationId`): under a walled posture no * organization anchors platform standing, so none is the Default one here - * either. Its delete is that guard's to answer, as before, so the refusal - * above does not run for it. `walled` is the guard's own expression, + * either. Its delete is that guard's to answer first, and then + * {@link defaultOrganizationDeleteRefusal}'s rather than + * {@link organizationDeleteRefusal}'s. `walled` is the guard's own expression, * `postureEnforcesWall(resolveTenancyPosture())`. */ export async function isDefaultOrganizationTarget( diff --git a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts index 219a55c905f..58941180d48 100644 --- a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts @@ -61,6 +61,7 @@ import showcaseStack, { onEnable } from '@objectstack/example-showcase'; import { type VerifyStack } from '@objectstack/verify'; import { bootShowcase } from './showcase-boot.js'; import { resolveInjectedColumnProvenance } from '@objectstack/metadata-core'; +import { resetPlatformAdminEmailMemo } from '@objectstack/core'; import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -189,6 +190,100 @@ describe('[#21910] an organization delete with the showcase federated fixture pr expect(await findRows(ql, 'sys_member', { organization_id: defaultId }, 50)).toEqual(membersBefore); expect(await findRows(ql, 'sys_team', { organization_id: defaultId }, 50)).toEqual(teamsBefore); }); + it('[#24948] the Default Organization, the guard admitting on a config anchor, is refused 409 before any member or invitation delete', async () => { + const [defaultOrg] = await findRows(ql, 'sys_organization', { slug: 'default' }, 1); + const defaultId = String(defaultOrg.id); + // The config anchor: the admin is named in OS_PLATFORM_OWNER_EMAIL and + // verified, so platform standing survives the Default Organization and + // the last-administrator guard admits its delete. + const [admin] = await findRows(ql, 'sys_user', { email: 'admin@objectos.ai' }, 1); + const verifiedBefore = admin.email_verified; + if (!verifiedBefore) await ql.update('sys_user', { id: admin.id, email_verified: true }, { context: SYSTEM_CTX }); + const ownerEnvBefore = process.env.OS_PLATFORM_OWNER_EMAIL; + process.env.OS_PLATFORM_OWNER_EMAIL = 'admin@objectos.ai'; + resetPlatformAdminEmailMemo(); + + const membersBefore = await findRows(ql, 'sys_member', { organization_id: defaultId }, 50); + const invitationsBefore = await findRows(ql, 'sys_invitation', { organization_id: defaultId }, 50); + const teamsBefore = await findRows(ql, 'sys_team', { organization_id: defaultId }, 50); + const attempted: string[] = []; + const RECORDER = 'dogfood.24948.default-admitted'; + for (const object of ['sys_member', 'sys_invitation']) { + ql.registerHook('beforeDelete', async () => { attempted.push(object); }, { object, packageId: RECORDER }); + } + let res: Response; + try { + res = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: defaultId }); + } finally { + ql.unregisterHooksByPackage(RECORDER); + if (ownerEnvBefore === undefined) delete process.env.OS_PLATFORM_OWNER_EMAIL; + else process.env.OS_PLATFORM_OWNER_EMAIL = ownerEnvBefore; + resetPlatformAdminEmailMemo(); + if (!verifiedBefore) await ql.update('sys_user', { id: admin.id, email_verified: verifiedBefore }, { context: SYSTEM_CTX }); + } + + // Not the guard's 403: the guard admitted. The Default Organization's own + // record count refused, before better-auth's first write. + expect(res.status, await res.clone().text()).toBe(409); + const body = (await res.json()) as { code?: string; dependents?: Array<{ object: string }> }; + expect(body.code).toBe('DELETE_RESTRICTED'); + const named = (body.dependents ?? []).map((d) => d.object); + // The records it owns are named; its members and invitations are the + // vendor's to remove, so they are not. + expect(named).toEqual(expect.arrayContaining(['showcase_account'])); + expect(named).not.toContain('sys_member'); + expect(named).not.toContain('sys_invitation'); + expect(attempted).toEqual([]); + expect(await findRows(ql, 'sys_organization', { id: defaultId }, 1)).toHaveLength(1); + expect(await findRows(ql, 'sys_member', { organization_id: defaultId }, 50)).toEqual(membersBefore); + expect(await findRows(ql, 'sys_invitation', { organization_id: defaultId }, 50)).toEqual(invitationsBefore); + expect(await findRows(ql, 'sys_team', { organization_id: defaultId }, 50)).toEqual(teamsBefore); + }); + + let lookupFails = false; + let lookupMiddlewareRegistered = false; + it('[#24948] a failed Default Organization lookup refuses: it never hands the delete to better-auth unasked', async () => { + // An empty organization the admin owns: with the lookup working, it would + // delete. The lookup asks how many organizations exist (its slug is not + // `default`), and that count fails. + const lone = await ql.insert('sys_organization', { name: 'Org 24948 lookup', slug: 'org-24948-lookup' }, { context: SYSTEM_CTX }); + const loneId = String(lone.id); + const [admin] = await findRows(ql, 'sys_user', { email: 'admin@objectos.ai' }, 1); + await ql.insert('sys_member', { user_id: String(admin?.id), organization_id: loneId, role: 'owner' }, { context: SYSTEM_CTX }); + + const attempted: string[] = []; + const RECORDER = 'dogfood.24948.lookup-fails'; + // `count` runs the middleware chain only (no hook), and a middleware has + // no unregister, so this one is switched by a flag. + lookupFails = true; + if (!lookupMiddlewareRegistered) { + ql.registerMiddleware(async (opCtx: any, next: () => Promise) => { + if (lookupFails && opCtx.operation === 'count') throw new Error('organization lookup unavailable'); + await next(); + }, { object: 'sys_organization' }); + lookupMiddlewareRegistered = true; + } + for (const object of ['sys_member', 'sys_invitation', 'sys_organization']) { + ql.registerHook('beforeDelete', async () => { attempted.push(object); }, { object, packageId: RECORDER }); + } + let res: Response; + try { + res = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: loneId }); + } finally { + lookupFails = false; + ql.unregisterHooksByPackage(RECORDER); + } + + expect(res.status, await res.clone().text()).toBeGreaterThanOrEqual(400); + expect(attempted).toEqual([]); + expect(await findRows(ql, 'sys_organization', { id: loneId }, 1)).toHaveLength(1); + expect(await findRows(ql, 'sys_member', { organization_id: loneId }, 5)).toHaveLength(1); + + // CONTROL: with the lookup back, the same empty organization deletes. + const ok = await stack.apiAs(token, 'POST', '/auth/organization/delete', { organizationId: loneId }); + expect(ok.status, await ok.clone().text()).toBe(200); + }); + it('[#24948] an organization that owns a record is refused 409 with its count, and the scan still never probes the federated table', async () => { // A second organization the admin owns, holding one record: the refusal's // count reads the same relations the cascade does, so it must skip the From 139abc45a68ca76907c1a8d3da08774aa6b19e9a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 04:40:58 +0000 Subject: [PATCH 18/18] test(dogfood): pin that a failed Default Organization lookup never reaches better-auth's first write (#24948) The recorder now also watches the session's active-organization write, the vendor handler's first, so the pin goes red when the lookup falls through (ablated). Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016gmigttoA9DyRNtz6SepWf --- .../organization-delete-federated-fixture.dogfood.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts index 58941180d48..a4f529ddbe5 100644 --- a/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts +++ b/packages/qa/dogfood/test/organization-delete-federated-fixture.dogfood.test.ts @@ -250,9 +250,14 @@ describe('[#21910] an organization delete with the showcase federated fixture pr const loneId = String(lone.id); const [admin] = await findRows(ql, 'sys_user', { email: 'admin@objectos.ai' }, 1); await ql.insert('sys_member', { user_id: String(admin?.id), organization_id: loneId, role: 'owner' }, { context: SYSTEM_CTX }); + // Active, so better-auth's handler would write first to the session + // (clearing it) if the request reached it: the write recorded below. + const active = await stack.apiAs(token, 'POST', '/auth/organization/set-active', { organizationId: loneId }); + expect(active.status, await active.clone().text()).toBe(200); const attempted: string[] = []; const RECORDER = 'dogfood.24948.lookup-fails'; + ql.registerHook('beforeUpdate', async () => { attempted.push('sys_session'); }, { object: 'sys_session', packageId: RECORDER }); // `count` runs the middleware chain only (no hook), and a middleware has // no unregister, so this one is switched by a flag. lookupFails = true; @@ -275,6 +280,8 @@ describe('[#21910] an organization delete with the showcase federated fixture pr } expect(res.status, await res.clone().text()).toBeGreaterThanOrEqual(400); + // better-auth's handler never ran: not even its first write, the session's + // active organization, was attempted. expect(attempted).toEqual([]); expect(await findRows(ql, 'sys_organization', { id: loneId }, 1)).toHaveLength(1); expect(await findRows(ql, 'sys_member', { organization_id: loneId }, 5)).toHaveLength(1);