From 5ce0fc7c993dedbb7fc4c41d686285d2a0584b93 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:03:38 +0000 Subject: [PATCH 1/4] test(runtime): pin a restrict lookup to the parent a child also cascades from (red on main) Claude-Session: https://claude.ai/code/session_01FEkQ5mmLAr61euRoHc183L Co-authored-by: Claude --- ...ame-parent-real-driver.integration.test.ts | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 packages/runtime/src/cascade-delete-restrict-same-parent-real-driver.integration.test.ts diff --git a/packages/runtime/src/cascade-delete-restrict-same-parent-real-driver.integration.test.ts b/packages/runtime/src/cascade-delete-restrict-same-parent-real-driver.integration.test.ts new file mode 100644 index 00000000000..7d41be3bab2 --- /dev/null +++ b/packages/runtime/src/cascade-delete-restrict-same-parent-real-driver.integration.test.ts @@ -0,0 +1,209 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#26718] A `deleteBehavior: 'restrict'` lookup refuses the delete of its + * parent even when the same child ALSO cascades from that parent — on the real + * stack: a real `ObjectQL` over a real `SqlDriver` (better-sqlite3, on disk), + * driven through the real `ObjectStackProtocolImplementation`'s data-plane + * deletes (`deleteData` serves `DELETE /api/v1/data/:object/:id`, + * `deleteManyData` the bulk door). + * + * ## The defect + * + * A child holding a `master_detail` AND an authored `restrict` lookup to the + * same parent row: deleting the parent answered success and the child was + * gone. The by-id cascade collects its cascade set first (#22305) and judged + * every `restrict` only against rows OUTSIDE that set. The child is in the set, + * through its own master_detail, so its restrict was never asked; the cascade + * then deleted it. The record the author declared undeletable was destroyed. + * + * ## The rule (triage on #26718) + * + * Every restrict is evaluated before any cascade, so a restrict on any path + * refuses the delete with 409 `DELETE_RESTRICTED` naming the restricting + * object, and nothing is deleted. The engine now judges, before its first + * write, every AUTHORED restrict held by a row the delete reaches only through + * the record that restrict protects; see + * `ObjectQL.refuseRestrictsInsideOwnCascade`. The #22305 rule for a row the + * cascade reaches through another record (a sibling under a common parent) is + * unchanged, and pinned in `packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts`. + * + * ## Controls + * + * - the two fields pointing at DIFFERENT parents still refuse; + * - a plain master_detail with no restrict still cascades; + * - a grandchild reached through a cascade, holding a restrict to the root, + * refuses the root and leaves every row in place; + * - the bulk door reaches the same walk per id, so it refuses the same row. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SqlDriver } from '@objectstack/driver-sql'; + +const PART = { name: 'zz_widget_part', fields: { name: { type: 'text' } } }; + +/** The card's shape: master_detail first, then the restrict lookup. */ +const CHILD = { + name: 'zz_widget_child', + fields: { + name: { type: 'text' }, + f_master_detail: { type: 'master_detail', reference: 'zz_widget_part', required: true }, + f_lookup: { type: 'lookup', reference: 'zz_widget_part', deleteBehavior: 'restrict' }, + }, +}; + +/** The same two fields in the other declaration order. */ +const CHILD_LOOKUP_FIRST = { + name: 'zz_widget_child', + fields: { + name: { type: 'text' }, + f_lookup: { type: 'lookup', reference: 'zz_widget_part', deleteBehavior: 'restrict' }, + f_master_detail: { type: 'master_detail', reference: 'zz_widget_part', required: true }, + }, +}; + +/** A plain detail: cascades, holds no restrict. */ +const PLAIN = { + name: 'zz_widget_plain', + fields: { + name: { type: 'text' }, + part: { type: 'master_detail', reference: 'zz_widget_part', required: true }, + }, +}; + +/** A detail of the part, and a detail of THAT, holding a restrict to the part. */ +const MID = { + name: 'zz_widget_mid', + fields: { + name: { type: 'text' }, + part: { type: 'master_detail', reference: 'zz_widget_part', required: true }, + }, +}; +const LEAF = { + name: 'zz_widget_leaf', + fields: { + name: { type: 'text' }, + mid: { type: 'master_detail', reference: 'zz_widget_mid', required: true }, + guard: { type: 'lookup', reference: 'zz_widget_part', deleteBehavior: 'restrict' }, + }, +}; + +describe('[#26718] a restrict lookup to the parent a child also cascades from — real driver (better-sqlite3)', () => { + let dir: string | null = null; + let engine: ObjectQL | null = null; + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + engine = null; + if (dir) { rmSync(dir, { recursive: true, force: true }); dir = null; } + }); + + async function rig(objects: unknown[]) { + dir = mkdtempSync(join(tmpdir(), 'os-26718-real-')); + const real = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: join(dir, 'data.sqlite') }, + useNullAsDefault: true, + }); + await real.initObjects(objects as any); + engine = new ObjectQL(); + engine.registerDriver(real as any, true); + await engine.init(); + for (const o of objects) engine.registry.registerObject(o as any, 'com.objectstack.test.26718'); + const protocol: any = new ObjectStackProtocolImplementation(engine as any); + return { protocol, real }; + } + + it.each([ + ['master_detail declared first', CHILD], + ['restrict lookup declared first', CHILD_LOOKUP_FIRST], + ] as Array<[string, unknown]>)('refuses the parent delete 409 and deletes nothing (%s)', async (_label, child) => { + const { protocol, real } = await rig([PART, child]); + const p: any = await engine!.insert('zz_widget_part', { name: 'P' }); + await engine!.insert('zz_widget_child', { name: 'c', f_master_detail: p.id, f_lookup: p.id }); + + const err: any = await protocol.deleteData({ object: 'zz_widget_part', id: p.id }).catch((e: any) => e); + + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('DELETE_RESTRICTED'); + expect(err.status).toBe(409); + expect(err.dependentObject).toBe('zz_widget_child'); + expect(err.developerMessage).toContain('via f_lookup'); + // Counted out of the DRIVER, not through the engine: both rows are there. + expect(await real.count('zz_widget_part')).toBe(1); + expect(await real.count('zz_widget_child')).toBe(1); + }); + + it('the bulk door refuses the same row and deletes nothing', async () => { + const { protocol, real } = await rig([PART, CHILD]); + const p: any = await engine!.insert('zz_widget_part', { name: 'P' }); + await engine!.insert('zz_widget_child', { name: 'c', f_master_detail: p.id, f_lookup: p.id }); + + const res: any = await protocol.deleteManyData({ object: 'zz_widget_part', ids: [p.id] }); + + expect(res).toMatchObject({ success: false, succeeded: 0, failed: 1 }); + expect(res.results[0].success).toBe(false); + expect(await real.count('zz_widget_part')).toBe(1); + expect(await real.count('zz_widget_child')).toBe(1); + }); + + it('refuses the root when a grandchild the cascade reaches holds the restrict, and deletes nothing', async () => { + const { protocol, real } = await rig([PART, MID, LEAF]); + const p: any = await engine!.insert('zz_widget_part', { name: 'P' }); + const m: any = await engine!.insert('zz_widget_mid', { name: 'm', part: p.id }); + await engine!.insert('zz_widget_leaf', { name: 'l', mid: m.id, guard: p.id }); + + const err: any = await protocol.deleteData({ object: 'zz_widget_part', id: p.id }).catch((e: any) => e); + + expect(err.code).toBe('DELETE_RESTRICTED'); + expect(err.status).toBe(409); + expect(err.dependentObject).toBe('zz_widget_leaf'); + expect(await real.count('zz_widget_part')).toBe(1); + expect(await real.count('zz_widget_mid')).toBe(1); + expect(await real.count('zz_widget_leaf')).toBe(1); + }); + + it('control: the two fields at DIFFERENT parents still refuse the lookup parent', async () => { + const { protocol, real } = await rig([PART, CHILD]); + const owner: any = await engine!.insert('zz_widget_part', { name: 'owner' }); + const guarded: any = await engine!.insert('zz_widget_part', { name: 'guarded' }); + await engine!.insert('zz_widget_child', { name: 'c', f_master_detail: owner.id, f_lookup: guarded.id }); + + const err: any = await protocol.deleteData({ object: 'zz_widget_part', id: guarded.id }).catch((e: any) => e); + + expect(err.code).toBe('DELETE_RESTRICTED'); + expect(err.status).toBe(409); + expect(err.dependentObject).toBe('zz_widget_child'); + expect(await real.count('zz_widget_part')).toBe(2); + expect(await real.count('zz_widget_child')).toBe(1); + }); + + it('control: a plain master_detail with no restrict still cascades', async () => { + const { protocol, real } = await rig([PART, PLAIN]); + const p: any = await engine!.insert('zz_widget_part', { name: 'P' }); + await engine!.insert('zz_widget_plain', { name: 'd', part: p.id }); + + const res = await protocol.deleteData({ object: 'zz_widget_part', id: p.id }); + + expect(res).toMatchObject({ object: 'zz_widget_part', id: p.id, success: true }); + expect(await real.count('zz_widget_part')).toBe(0); + expect(await real.count('zz_widget_plain')).toBe(0); + }); + + it('control: once the child is gone the parent deletes', async () => { + const { protocol, real } = await rig([PART, CHILD]); + const p: any = await engine!.insert('zz_widget_part', { name: 'P' }); + const c: any = await engine!.insert('zz_widget_child', { name: 'c', f_master_detail: p.id, f_lookup: p.id }); + + await protocol.deleteData({ object: 'zz_widget_child', id: c.id }); + const res = await protocol.deleteData({ object: 'zz_widget_part', id: p.id }); + + expect(res).toMatchObject({ success: true }); + expect(await real.count('zz_widget_part')).toBe(0); + }); +}); From a7cff10cd2f40e901eae4608a62d1eb12a1e5d34 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:12:01 +0000 Subject: [PATCH 2/4] fix(objectql): judge a restrict the cascade would override before the first write Claude-Session: https://claude.ai/code/session_01FEkQ5mmLAr61euRoHc183L Co-authored-by: Claude --- ...ne-cascade-delete-sibling-restrict.test.ts | 105 ++++++ packages/objectql/src/engine.ts | 334 +++++++++++++----- 2 files changed, 359 insertions(+), 80 deletions(-) diff --git a/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts b/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts index 43710f3fe2c..736b9660f32 100644 --- a/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts +++ b/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts @@ -485,3 +485,108 @@ describe('[#22305] a cycle in the data terminates', () => { expect(driver.writes.filter((w: Write) => w.op === 'delete' && w.object === 'zz_item')).toHaveLength(2); }); }); + +// ── [#26718] A restrict held by a row the cascade reaches THROUGH its target ─ +// +// The rule above exempts a member from refusing a delete of ANOTHER member. +// It never exempted a row the cascade reaches only because the record it +// restricts is deleted: a child holding a master_detail AND an authored +// restrict lookup to the same parent was in the parent's set by its own +// master_detail, so its restrict was never asked and the cascade deleted it. +// Every such restrict is now judged before the first write (triage on +// #26718); the real-driver, protocol-door face of the same pins is +// `packages/runtime/src/cascade-delete-restrict-same-parent-real-driver.integration.test.ts`. + +/** A detail of the account that also restricts the account itself. */ +const pledge = (mdFirst: boolean) => { + const md = { name: 'account', type: 'master_detail' as const, reference: 'zz_account' }; + const guard = { name: 'guard', type: 'lookup' as const, reference: 'zz_account', deleteBehavior: 'restrict' }; + return { + name: 'zz_pledge', + label: 'Pledge', + fields: mdFirst ? { id, name, account: md, guard } : { id, name, guard, account: md }, + }; +}; +/** A detail of a contact, restricting the account or the contact above it. */ +const note = { + name: 'zz_note', + label: 'Note', + fields: { + id, name, + contact: { name: 'contact', type: 'master_detail' as const, reference: 'zz_contact' }, + guard_account: { name: 'guard_account', type: 'lookup' as const, reference: 'zz_account', deleteBehavior: 'restrict' }, + guard_contact: { name: 'guard_contact', type: 'lookup' as const, reference: 'zz_contact', deleteBehavior: 'restrict' }, + }, +}; + +describe('[#26718] a restrict on a row the cascade reaches through the record it protects refuses', () => { + it.each([ + ['master_detail declared first', true], + ['restrict lookup declared first', false], + ] as Array<[string, boolean]>)('a child restricting its own master refuses, and nothing is written (%s)', async (_l, mdFirst) => { + const { engine, driver } = await makeEngine([account, pledge(mdFirst)]); + const acc = await engine.insert('zz_account', { name: 'Acme' }); + await engine.insert('zz_pledge', { name: 'p', account: acc.id, guard: acc.id }); + driver.writes.length = 0; + + await expect(engine.delete('zz_account', { where: { id: acc.id } } as any)) + .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, object: 'zz_account', dependentObject: 'zz_pledge' }); + expect(driver.writes).toEqual([]); + expect(driver.rowsOf('zz_account')).toHaveLength(1); + expect(driver.rowsOf('zz_pledge')).toHaveLength(1); + }); + + it('a grandchild restricting the root refuses the root, and nothing is written', async () => { + const { engine, driver } = await makeEngine([account, contact, note]); + const acc = await engine.insert('zz_account', { name: 'Acme' }); + const c = await engine.insert('zz_contact', { name: 'c', account: acc.id }); + await engine.insert('zz_note', { name: 'n', contact: c.id, guard_account: acc.id }); + driver.writes.length = 0; + + await expect(engine.delete('zz_account', { where: { id: acc.id } } as any)) + .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, object: 'zz_account', dependentObject: 'zz_note' }); + expect(driver.writes).toEqual([]); + expect(driver.rowsOf('zz_contact')).toHaveLength(1); + expect(driver.rowsOf('zz_note')).toHaveLength(1); + }); + + it('a grandchild restricting the member above it refuses the root delete that reaches both', async () => { + const { engine, driver } = await makeEngine([account, contact, note]); + const acc = await engine.insert('zz_account', { name: 'Acme' }); + const c = await engine.insert('zz_contact', { name: 'c', account: acc.id }); + await engine.insert('zz_note', { name: 'n', contact: c.id, guard_contact: c.id }); + driver.writes.length = 0; + + await expect(engine.delete('zz_account', { where: { id: acc.id } } as any)) + .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, object: 'zz_contact', dependentObject: 'zz_note' }); + expect(driver.writes).toEqual([]); + // …the same answer deleting that member directly gives. + await expect(engine.delete('zz_contact', { where: { id: c.id } } as any)) + .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'zz_note' }); + expect(driver.rowsOf('zz_note')).toHaveLength(1); + }); + + it('control: the same restrict pointing at a record the delete does not reach is not asked', async () => { + const { engine, driver } = await makeEngine([account, pledge(true)]); + const acc = await engine.insert('zz_account', { name: 'Acme' }); + const other = await engine.insert('zz_account', { name: 'Other' }); + await engine.insert('zz_pledge', { name: 'p', account: acc.id, guard: other.id }); + + await engine.delete('zz_account', { where: { id: acc.id } } as any); + + expect(driver.rowsOf('zz_pledge')).toHaveLength(0); + expect(driver.rowsOf('zz_account').map((r) => r.id)).toEqual([other.id]); + }); + + it('control: deleting the restricted record of a pledge owned elsewhere still refuses', async () => { + const { engine, driver } = await makeEngine([account, pledge(true)]); + const owner = await engine.insert('zz_account', { name: 'Owner' }); + const guarded = await engine.insert('zz_account', { name: 'Guarded' }); + await engine.insert('zz_pledge', { name: 'p', account: owner.id, guard: guarded.id }); + + await expect(engine.delete('zz_account', { where: { id: guarded.id } } as any)) + .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'zz_pledge' }); + expect(driver.rowsOf('zz_account')).toHaveLength(2); + expect(driver.rowsOf('zz_pledge')).toHaveLength(1); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 8116d440cd7..024eb5bf8ca 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -4518,6 +4518,36 @@ interface CascadeDeleteSet { * read the same relations, file each record once between them. */ readonly elevationsFiled: Set; + /** + * [#26718] Member key ({@link cascadeMemberKey}) → the members a + * CASCADING relation reaches from it directly, every edge the collection + * walk read, a row reached by two paths included. The reach of one member + * inside the set ({@link cascadeReachOf}) is read from it: which rows the + * delete removes only BECAUSE that member goes. + */ + readonly cascadeEdges: Map>; +} + +/** [#26718] One member of a {@link CascadeDeleteSet}, as a single key. */ +function cascadeMemberKey(object: string, id: unknown): string { + return JSON.stringify([object, String(id)]); +} + +/** + * [#26718] Every member a cascade reaches from `object`/`id` through the + * set's own {@link CascadeDeleteSet.cascadeEdges}: its details, theirs, and so + * on, indexed like {@link CascadeDeleteSet.members}. The member itself is in + * it only when a cycle in the data leads back. + */ +function cascadeReachOf(set: CascadeDeleteSet, object: string, id: unknown): Map> { + const reach = new Map>(); + const queue: Array<{ object: string; id: unknown }> = [{ object, id }]; + for (const from of queue) { + for (const next of set.cascadeEdges.get(cascadeMemberKey(from.object, from.id)) ?? []) { + if (cascadeSetAdd(reach, next.object, next.id)) queue.push(next); + } + } + return reach; } /** Is `id` of `object` in this index of a {@link CascadeDeleteSet}? */ @@ -18680,6 +18710,13 @@ export class ObjectQL implements IObjectQLEngine { * OUTSIDE the set. A row in the set is about to be deleted by this same * cascade: it cannot be left dangling, so it refuses nothing, and * clearing its reference would be a write against a row about to go. + * [#26718] Between the two, {@link refuseRestrictsInsideOwnCascade}, also + * read-only: an AUTHORED `restrict` held by a member the cascade reaches + * only THROUGH the record that restrict protects (a child holding a + * master_detail and a restrict lookup to one parent; a grandchild under it) + * refuses the delete before the first write. The row is in the set, so phase + * 2 alone would let the cascade delete it. + * * Before this, the outcome depended on registration order: with the * contacts registered ahead of the contracts, deleting an account refused * on a contract's required lookup to a contact, a contract the same delete @@ -18727,6 +18764,9 @@ export class ObjectQL implements IObjectQLEngine { return; } const cascadeSet = await this.collectCascadeDeleteSet(object, id, context, objects); + // [#26718] Every restrict a member's own cascade would override is judged + // here, before the first write. + await this.refuseRestrictsInsideOwnCascade(cascadeSet, { object, id }, context, objects); await this.cascadeDeleteSets.run(cascadeSet, () => this.walkReferencingRelations(object, id, context, objects, cascadeSet), ); @@ -18763,7 +18803,9 @@ export class ObjectQL implements IObjectQLEngine { objects: ServiceObject[], opts: { fileElevations: boolean } = { fileElevations: true }, ): Promise { - const set: CascadeDeleteSet = { members: new Map(), entered: new Map(), elevationsFiled: new Set() }; + const set: CascadeDeleteSet = { + members: new Map(), entered: new Map(), elevationsFiled: new Set(), cascadeEdges: new Map(), + }; cascadeSetAdd(set.members, object, id); // The cascading relations pointing at each object, scanned once per object // rather than once per record: a wide cascade has many records of few @@ -18800,7 +18842,14 @@ export class ObjectQL implements IObjectQLEngine { ); for (const row of rows) { const depId = row?.id; - if (depId != null && cascadeSetAdd(set.members, childName, depId)) { + if (depId == null) continue; + // [#26718] The edge is kept even for a row already in the set: a + // row reached by two paths is in the reach of both. + const from = cascadeMemberKey(target.object, target.id); + let edges = set.cascadeEdges.get(from); + if (!edges) { edges = []; set.cascadeEdges.set(from, edges); } + edges.push({ object: childName, id: depId }); + if (cascadeSetAdd(set.members, childName, depId)) { queue.push({ object: childName, id: depId }); } } @@ -19204,6 +19253,11 @@ export class ObjectQL implements IObjectQLEngine { // is done. `cascade` keeps every row: its rows ARE the set, and the // walk deletes them below. // + // [#26718] …except an AUTHORED restrict held by a row the cascade reaches + // only through the record that restrict protects: that one was judged + // before the cascade wrote anything ({@link refuseRestrictsInsideOwnCascade}), + // so a row in the set reaching here refuses nothing. + // // After `probedIds`, on purpose: the disclosure check still compares // the caller's own view against every row the elevated probe read, so a // count is disclosed only to a caller who can see the whole relation, @@ -19244,84 +19298,10 @@ export class ObjectQL implements IObjectQLEngine { } if (behavior === 'restrict') { - // [#7307] TWO messages, two audiences — because this error has two - // and they were sharing one string. - // - // `message` is what a BUSINESS USER reads: REST ships it verbatim as - // `body.error` in the flat 409 envelope (`mapDataError`), and Console - // renders that as-is in a toast. It was composed English-only with the - // API names concatenated in and a metadata-authoring instruction on - // the end, so an operator deleting a 部门 in a fully Chinese app got an - // English sentence naming two tables and a column they have never - // seen, ending in advice only a developer can act on. It is now - // rendered through the operation-message catalog in the caller's - // locale against resolved LABELS — the same fix #3957 made one layer - // down for field constraints, reached from the operation side. - // - // `developerMessage` is what the DEVELOPER reads, and is the previous - // sentence unchanged, byte for byte: English, API names, and the - // `deleteBehavior:'cascade'` hint — which is correct and useful, and - // is not lost, it is addressed. It rides the structured half of the - // envelope alongside `dependentObject` / `dependentCount`, which - // already carry API names, so it discloses nothing new; no - // user-facing surface reads it. - // - // The wire code does NOT split: one `DELETE_RESTRICTED` (ADR-0112), - // two SENTENCES, exactly as the field catalog splits a message key - // without splitting `FieldErrorCode`. - const required = fdef.deleteBehavior !== 'restrict' && fdef.required === true; - const msgCtx = this.validationMessageContext(object, context); - const parent = objects.find((o) => (o as any)?.name === object); - // [#12166, ruling constraint 2] Whether this refusal may carry the - // row COUNT — see `dependentCountIsDisclosable` for why the elevation - // above is what makes the question exist, and why the answer is "only - // if the caller's own identity would have produced the same rows". - // The OBJECT is named either way: that half is what the ruling - // requires the caller to learn. - const discloseCount = await this.dependentCountIsDisclosable( - childName, probeWhere, probedIds, fieldName, multiValued, id, context, - ); - const err: any = new Error( - renderOperationMessage( - { - messageKey: discloseCount - ? (required ? 'delete_restricted_required' : 'delete_restricted') - : (required ? 'delete_restricted_required_opaque' : 'delete_restricted_opaque'), - params: { - object: this.objectDisplayLabel(object, (parent as any)?.label, msgCtx), - dependentObject: this.objectDisplayLabel(childName, (child as any)?.label, msgCtx), - field: resolveFieldLabel(fieldName, fdef, { ...msgCtx, objectName: childName }), - ...(discloseCount ? { count: dependents.length } : {}), - }, - }, - { locale: msgCtx.locale, translate: msgCtx.translate }, - ), - ); - // The DEVELOPER half is subject to the same rule, and the reason is - // measured rather than assumed: for `DELETE_RESTRICTED` (unlike the - // #7414 permission denial) this sentence RIDES THE ENVELOPE — REST's - // `mapDataError` ships it — so a count withheld from `message` and - // left here would not be withheld at all. - err.developerMessage = - `Cannot delete ${object} (${id}): ` + - `${discloseCount ? `${dependents.length} dependent ${childName} record(s) reference it` : `dependent ${childName} record(s) reference it`} via ${fieldName}` + - `${required ? ` (${fieldName} is required, so it cannot be cleared)` : ''}. ` + - `Delete or reassign them first, or set deleteBehavior:'cascade' on ${childName}.${fieldName}.`; - err.code = 'DELETE_RESTRICTED'; - err.status = 409; - err.httpStatus = 409; - err.object = object; - // Constraint 2's REQUIRED half — the referenced object is named - // unconditionally, because "which table is blocking me" is the one - // thing the reporting deployment's admins could not self-diagnose. - err.dependentObject = childName; - // …and its withheld half. Absent rather than zeroed: `0` would be a - // false statement about the rows (there is at least one, or this - // branch would not have been reached), and REST's envelope builder - // already omits the key for a non-number, so an absent count reaches - // the client as an absent key rather than as a lie. - if (discloseCount) err.dependentCount = dependents.length; - throw err; + throw await this.deleteRestrictedError({ + object, id, child, childName, fieldName, fdef, dependents, probeWhere, probedIds, multiValued, + objects, context, + }); } for (const dep of dependents) { @@ -19416,6 +19396,200 @@ export class ObjectQL implements IObjectQLEngine { } } + /** + * [#26718] Before the cascade writes anything: refuse the delete when an + * AUTHORED `restrict` is held by a row the cascade would remove only because + * the record that restrict protects is removed. + * + * The deleting walk judges a `restrict` against rows OUTSIDE the cascade set + * alone (#22305), and a row inside it was exempt whatever put it there. That + * exemption is right for a row the cascade reaches through ANOTHER record (a + * contract and the contact it restricts, both details of the account being + * deleted: the account's delete removes both, and neither is left dangling). + * It was wrong for a row reached THROUGH the very record it protects: a child + * holding a `master_detail` and a `restrict` lookup to the same parent is in + * that parent's set by its own master_detail, so the restrict was never + * asked and the cascade deleted the child — the record the author declared + * undeletable. A grandchild under that parent holding the restrict is the + * same case one level down. + * + * So for every member, every relation whose RESOLVED behaviour is `restrict` + * ({@link cascadeRelationBehavior}, an authored `restrict` on a lookup, + * tree or master_detail) is probed here when its object has a member in the + * reach of that member ({@link cascadeReachOf}); a referencing row in the + * reach refuses with the walk's own `DELETE_RESTRICTED` + * ({@link deleteRestrictedError}). Every restrict is judged before the + * first cascade, so the outcome no longer depends on the order the fields or + * objects are declared in, and nothing has been written when it refuses. + * + * ⛔ The required-lookup escalation (`set_null` on a required field read as + * `restrict`) is not judged here: it restricts because a required reference + * cannot be CLEARED, and a row the cascade deletes is never cleared. It keeps + * the #22305 rule. Neither does this replace the walk's judgement of rows + * outside the set, which is unchanged. + */ + private async refuseRestrictsInsideOwnCascade( + cascadeSet: CascadeDeleteSet, + root: { object: string; id: string | number }, + context: ExecutionContext | undefined, + objects: ServiceObject[], + ): Promise { + // The members with the ids as the rows carried them (the set's own index + // holds strings), so each probe below spells its filter as the walk does. + const members: Array<{ object: string; id: string | number }> = []; + const seen = new Set(); + for (const member of [root, ...[...cascadeSet.cascadeEdges.values()].flat()]) { + const key = cascadeMemberKey(member.object, member.id); + if (seen.has(key)) continue; + seen.add(key); + members.push(member); + } + // The restrict relations pointing at each object whose own object has a + // member, scanned once per object. + const restricting = new Map>(); + const restrictingInto = (target: string) => { + let relations = restricting.get(target); + if (!relations) { + relations = []; + 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 || !cascadeSet.members.has(childName)) continue; + for (const [fieldName, fdef] of Object.entries(fields)) { + if (this.cascadeRelationBehavior(target, child, fieldName, fdef) === 'restrict') { + relations.push({ child, childName, fieldName, fdef }); + } + } + } + restricting.set(target, relations); + } + return relations; + }; + for (const { object, id } of members) { + const relations = restrictingInto(object); + if (relations.length === 0) continue; + const reach = cascadeReachOf(cascadeSet, object, id); + for (const { child, childName, fieldName, fdef } of relations) { + if (!reach.has(childName)) continue; + const probeWhere = this.referenceProbeFilter(fieldName, fdef, id); + this.fileReferenceCheckElevation(cascadeSet, object, id, childName, fieldName, context); + const rows = await this.probeReferencingRows(childName, fieldName, fdef, id, probeWhere, context); + const dependents = rows.filter((row) => cascadeSetHas(reach, childName, row?.id)); + if (dependents.length === 0) continue; + throw await this.deleteRestrictedError({ + object, id, child, childName, fieldName, fdef, dependents, probeWhere, + probedIds: rows.map((r: any) => String(r?.id)), + multiValued: declaredMultiValued(fdef), + objects, context, + }); + } + } + } + + /** + * The `DELETE_RESTRICTED` refusal of deleting `object`/`id` because + * `dependents` of `childName` reference it via `fieldName`. One builder for + * the deleting walk ({@link walkReferencingRelations}) and the judgement + * before it ({@link refuseRestrictsInsideOwnCascade}), so the two answer one + * envelope. + */ + private async deleteRestrictedError(args: { + object: string; + id: string | number; + child: ServiceObject; + childName: string; + fieldName: string; + fdef: any; + dependents: readonly any[]; + probeWhere: Record; + probedIds: readonly string[]; + multiValued: boolean; + objects: ServiceObject[]; + context: ExecutionContext | undefined; + }): Promise { + const { + object, id, child, childName, fieldName, fdef, dependents, probeWhere, probedIds, multiValued, objects, context, + } = args; + // [#7307] TWO messages, two audiences — because this error has two + // and they were sharing one string. + // + // `message` is what a BUSINESS USER reads: REST ships it verbatim as + // `body.error` in the flat 409 envelope (`mapDataError`), and Console + // renders that as-is in a toast. It was composed English-only with the + // API names concatenated in and a metadata-authoring instruction on + // the end, so an operator deleting a 部门 in a fully Chinese app got an + // English sentence naming two tables and a column they have never + // seen, ending in advice only a developer can act on. It is now + // rendered through the operation-message catalog in the caller's + // locale against resolved LABELS — the same fix #3957 made one layer + // down for field constraints, reached from the operation side. + // + // `developerMessage` is what the DEVELOPER reads, and is the previous + // sentence unchanged, byte for byte: English, API names, and the + // `deleteBehavior:'cascade'` hint — which is correct and useful, and + // is not lost, it is addressed. It rides the structured half of the + // envelope alongside `dependentObject` / `dependentCount`, which + // already carry API names, so it discloses nothing new; no + // user-facing surface reads it. + // + // The wire code does NOT split: one `DELETE_RESTRICTED` (ADR-0112), + // two SENTENCES, exactly as the field catalog splits a message key + // without splitting `FieldErrorCode`. + const required = fdef.deleteBehavior !== 'restrict' && fdef.required === true; + const msgCtx = this.validationMessageContext(object, context); + const parent = objects.find((o) => (o as any)?.name === object); + // [#12166, ruling constraint 2] Whether this refusal may carry the + // row COUNT — see `dependentCountIsDisclosable` for why the elevation + // above is what makes the question exist, and why the answer is "only + // if the caller's own identity would have produced the same rows". + // The OBJECT is named either way: that half is what the ruling + // requires the caller to learn. + const discloseCount = await this.dependentCountIsDisclosable( + childName, probeWhere, probedIds, fieldName, multiValued, id, context, + ); + const err: any = new Error( + renderOperationMessage( + { + messageKey: discloseCount + ? (required ? 'delete_restricted_required' : 'delete_restricted') + : (required ? 'delete_restricted_required_opaque' : 'delete_restricted_opaque'), + params: { + object: this.objectDisplayLabel(object, (parent as any)?.label, msgCtx), + dependentObject: this.objectDisplayLabel(childName, (child as any)?.label, msgCtx), + field: resolveFieldLabel(fieldName, fdef, { ...msgCtx, objectName: childName }), + ...(discloseCount ? { count: dependents.length } : {}), + }, + }, + { locale: msgCtx.locale, translate: msgCtx.translate }, + ), + ); + // The DEVELOPER half is subject to the same rule, and the reason is + // measured rather than assumed: for `DELETE_RESTRICTED` (unlike the + // #7414 permission denial) this sentence RIDES THE ENVELOPE — REST's + // `mapDataError` ships it — so a count withheld from `message` and + // left here would not be withheld at all. + err.developerMessage = + `Cannot delete ${object} (${id}): ` + + `${discloseCount ? `${dependents.length} dependent ${childName} record(s) reference it` : `dependent ${childName} record(s) reference it`} via ${fieldName}` + + `${required ? ` (${fieldName} is required, so it cannot be cleared)` : ''}. ` + + `Delete or reassign them first, or set deleteBehavior:'cascade' on ${childName}.${fieldName}.`; + err.code = 'DELETE_RESTRICTED'; + err.status = 409; + err.httpStatus = 409; + err.object = object; + // Constraint 2's REQUIRED half — the referenced object is named + // unconditionally, because "which table is blocking me" is the one + // thing the reporting deployment's admins could not self-diagnose. + err.dependentObject = childName; + // …and its withheld half. Absent rather than zeroed: `0` would be a + // false statement about the rows (there is at least one, or this + // branch would not have been reached), and REST's envelope builder + // already omits the key for a non-number, so an absent count reaches + // the client as an absent key rather than as a lie. + if (discloseCount) err.dependentCount = dependents.length; + return err; + } + /** * [#24948] Is `fieldName` on `child` the TENANT column of `child`, pointing at * the organization being deleted with the `set_null` behaviour? From 30ce86010d13b821def915a1d5c64cc36121e012 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:19:35 +0000 Subject: [PATCH 3/4] chore(changeset): objectql patch for the restrict judged before the cascade Claude-Session: https://claude.ai/code/session_01FEkQ5mmLAr61euRoHc183L Co-authored-by: Claude --- .changeset/26718-restrict-over-own-cascade.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .changeset/26718-restrict-over-own-cascade.md diff --git a/.changeset/26718-restrict-over-own-cascade.md b/.changeset/26718-restrict-over-own-cascade.md new file mode 100644 index 00000000000..31039413451 --- /dev/null +++ b/.changeset/26718-restrict-over-own-cascade.md @@ -0,0 +1,19 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: a `deleteBehavior: 'restrict'` reference refuses the delete of the record it protects even when the referencing row would be cascade-deleted by that same delete (#26718) + +Clause-②: no (narrowing) — a by-id delete (`delete` by id, `DELETE /api/v1/data/:object/:id`, and the bulk `deleteManyData` door, which deletes by id row by row) refuses with 409 `DELETE_RESTRICTED` where it succeeded: when a row the delete would cascade-delete only through the record it restricts holds an authored `restrict` reference to that record. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the by-id delete (`@objectstack/objectql`), served by every delete door that deletes by id. + +- **Before.** A child holding a `master_detail` and a `deleteBehavior: 'restrict'` lookup to the same parent row: deleting the parent answered success, the cascade deleted the child, and the restrict was never asked. The same held for a grandchild under that parent holding the restrict. The delete collects every record its cascade removes first, and judged a `restrict` only against rows outside that set; the child is in the set by its own master_detail. +- **Now.** Before the first write, the delete judges every authored `restrict` (on a `lookup`, a `tree` or a `master_detail`) held by a row it would remove only because the record that restrict protects is removed. Such a row refuses the delete with the existing `409 DELETE_RESTRICTED`, naming the restricting object (`dependentObject`) and field, and nothing is deleted. The answer no longer depends on the order the two fields are declared in. +- **Unchanged.** A restrict held by a row outside the delete's cascade still refuses, as before. A row the cascade reaches through another record is still exempt: deleting an account still deletes a contract and the contact the contract restricts when both are details of that account. A required lookup's `set_null`, which is refused as `restrict` because it cannot be cleared, still steps aside for a row the cascade deletes. A plain `master_detail` with no restrict still cascades, and `set_null` still clears. +- **Who sees it.** An app with a detail that also restricts its own master (directly or from further down the cascade): deleting that master used to destroy the detail, and is now refused while the detail references it. + +**The remedy.** Delete the restricting detail (or clear its restrict reference) first, then delete the parent. If the detail is meant to go with its parent, drop the `restrict` from the second reference. From 40aace604ddb2ac656d23efaf72dfe3fd2476289 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:23:43 +0000 Subject: [PATCH 4/4] test(objectql): type the row in the #26718 control Claude-Session: https://claude.ai/code/session_01FEkQ5mmLAr61euRoHc183L Co-authored-by: Claude --- .../objectql/src/engine-cascade-delete-sibling-restrict.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts b/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts index 736b9660f32..516a3fcf1df 100644 --- a/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts +++ b/packages/objectql/src/engine-cascade-delete-sibling-restrict.test.ts @@ -575,7 +575,7 @@ describe('[#26718] a restrict on a row the cascade reaches through the record it await engine.delete('zz_account', { where: { id: acc.id } } as any); expect(driver.rowsOf('zz_pledge')).toHaveLength(0); - expect(driver.rowsOf('zz_account').map((r) => r.id)).toEqual([other.id]); + expect(driver.rowsOf('zz_account').map((r: Row) => r.id)).toEqual([other.id]); }); it('control: deleting the restricted record of a pledge owned elsewhere still refuses', async () => {