Skip to content

Commit 1258dca

Browse files
os-elonclaude
andauthored
fix(objectql): restore the #4757 unscoped multi-delete refusal on sys_attachment through the wired engine (#9797)
* fix(objectql,service-storage): restore the #4757 unscoped multi-delete refusal through the wired engine The predicate path's per-row dispatch (#5038/#5574) binds input.id on every beforeDelete dispatch, so sys_attachment's declared #4757 refusal of a predicate-less multi: true delete always took the handler's by-id branch — and a zero-match predicate dispatched nothing at all. The refusal could not fire through ObjectQL.delete on exactly the shape it refuses. Engine: a new opt-in registration declaration, dispatchUnscopedMultiDelete (beforeDelete only, refused elsewhere), makes ObjectQL.delete's predicate branch dispatch the whole-operation context ONCE — before the matched-row read, zero-match included — to registrations that declared for it. Binding input.id on that context is refused (HookTargetRebindError, path 'unscoped-multi'), mirroring D4/#6752. Undeclared objects see no new dispatch. service-storage: the sys_attachment access-hook registration declares the flag, making the handler's own #4757 branch reachable again with its declared envelope (ATTACHMENT_DELETE_DENIED, 403). The unit block that pinned the whole-operation shape by direct handler call — green while the wired engine did the opposite — is re-pointed at a real ObjectQL engine. Fixes #9719 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM * chore: changeset for the #4757 unscoped multi-delete restoration Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM * test: keep the new suites out of the frozen TEST_DEBT ledger check:type-check-debt re-measured @objectstack/objectql at +3 over its frozen 355: two 1-arg registerObject calls and one unused loop variable, all in the new dispatch test. Fixed at the source (registerObject gets its packageId; the seed loop no longer declares an unused row) — the ledger number is back to exactly 355. The service-storage harness carried the same two idioms latently (its test layer is not ledger-measured today) — fixed the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 52fbba6 commit 1258dca

7 files changed

Lines changed: 825 additions & 65 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/objectql': minor
3+
'@objectstack/service-storage': patch
4+
---
5+
6+
Restore the #4757 unscoped multi-delete refusal on `sys_attachment` through the wired engine (#9719).
7+
8+
`ObjectQL.registerHook` gains an opt-in `dispatchUnscopedMultiDelete` declaration (valid on `beforeDelete` registrations only — anything else is refused at registration): when a `multi: true` delete arrives with no `where` at all (absent or `null`), the engine's predicate path dispatches the whole-operation context ONCE to declaring registrations — before any matched row is resolved, zero-match included — so a guard about the operation's shape can refuse it. Binding `input.id` on that context is refused (`HookTargetRebindError`, path `'unscoped-multi'`). Undeclared registrations, scoped deletes (including the match-all `where: {}`), and by-id deletes see no new dispatch.
9+
10+
The `sys_attachment` access guard declares the flag, so its documented refusal of a predicate-less multi-delete fires again with its declared envelope (`ATTACHMENT_DELETE_DENIED`, HTTP 403): since the per-row dispatch contract (#5038/#5574) that branch was unreachable, and a predicate-less `multi: true` delete quietly removed every row the caller happened to be entitled to. System-context and context-less programmatic deletes bypass the guard exactly as before.
Lines changed: 381 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,381 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#9719] The opt-in whole-operation `beforeDelete` dispatch for an UNSCOPED
5+
* predicate delete — `multi: true` carrying no caller `where` at all.
6+
*
7+
* Why the engine needs it, measured on #9719: the per-row contract
8+
* (#5038 / #5574) binds `input.id` on every predicate dispatch, so a handler
9+
* guarding the OPERATION SHAPE (the #4757 predicate-less multi-delete refusal
10+
* on `sys_attachment`) always took its by-id branch — and a zero-match
11+
* predicate dispatches nothing at all ([D1]), so the guard never ran on
12+
* exactly the shape it refuses. Both unreachability limbs need one dispatch
13+
* that happens BEFORE the matched-row read, keyed on the operation's shape.
14+
*
15+
* Pinned here, against the REAL engine:
16+
* 1. the dispatch CONDITION — `where` absent or `null` is unscoped;
17+
* `where: {}` and a real predicate are scoped and get NO extra dispatch;
18+
* 2. the dispatched SHAPE — whole-operation: `input.id` undefined, the
19+
* caller's raw `options` (the `hook.zod.ts` upper-bound read),
20+
* `dispatch.mode === 'record'`, the batch `scope` identity-shared;
21+
* 3. ORDERING — a refusal from the flagged handler rejects the delete
22+
* before the doomed-row read and before `deleteMany`: zero driver calls;
23+
* 4. the ZERO-MATCH limb — an unscoped delete of an EMPTY table still
24+
* dispatches (with a positive control proving the measurement is not
25+
* vacuous);
26+
* 5. NEUTRALITY — undeclared registrations and by-id deletes see no new
27+
* dispatch: today's behaviour for every other object is unchanged;
28+
* 6. the retired-lever rule — binding `input.id` on the whole-operation
29+
* context is refused (`HookTargetRebindError`, path `'unscoped-multi'`),
30+
* never silently ignored;
31+
* 7. the registration-time refusal of the flag on any event whose dispatch
32+
* never reads it (ADR-0078: no silently inert declaration).
33+
*
34+
* The consumer half — the #4757 refusal itself, wired end-to-end — is pinned
35+
* in `packages/services/service-storage/src/attachment-access-hooks.test.ts`.
36+
*/
37+
38+
import { describe, it, expect } from 'vitest';
39+
import { ObjectQL } from './engine.js';
40+
import { HOOK_TARGET_REBIND_ERROR_CODE } from './hook-target-rebind-errors.js';
41+
import type { HookContext } from '@objectstack/spec/data';
42+
43+
const FIELDS = {
44+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
45+
status: { name: 'status', label: 'Status', type: 'text' as const },
46+
owner: { name: 'owner', label: 'Owner', type: 'text' as const },
47+
};
48+
const attObject = { name: 'att', label: 'Att', fields: FIELDS };
49+
const taskObject = { name: 'task', label: 'Task', fields: FIELDS };
50+
51+
/**
52+
* Minimal in-memory driver. Its WHERE matcher REFUSES combinators and
53+
* operator objects by throwing (the conforming shape
54+
* `check-where-matcher-conformance.mjs` documents): a double that answers an
55+
* operator silently wrong makes a suite green on a different query.
56+
*/
57+
function makeStubDriver() {
58+
const stores = new Map<string, Map<string, Record<string, unknown>>>();
59+
const storeFor = (o: string) => {
60+
let s = stores.get(o);
61+
if (!s) { s = new Map(); stores.set(o, s); }
62+
return s;
63+
};
64+
const matches = (row: Record<string, unknown>, where: unknown): boolean => {
65+
if (!where || typeof where !== 'object') return true;
66+
for (const [k, v] of Object.entries(where)) {
67+
if (k.startsWith('$')) throw new Error(`stub driver: unsupported combinator ${k}`);
68+
if (v !== null && typeof v === 'object') throw new Error(`stub driver: unsupported operator value on ${k}`);
69+
if ((row[k] ?? null) !== (v ?? null)) return false;
70+
}
71+
return true;
72+
};
73+
const d: any = {
74+
name: 'memory', version: '0.0.0', supports: {},
75+
stores,
76+
/** Every read/write the engine issued, in order — the ordering pins read it. */
77+
calls: [] as string[],
78+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
79+
async execute() { return null; }, async syncSchema() {},
80+
async find(o: string, ast: any) {
81+
d.calls.push(`find:${o}`);
82+
return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
83+
},
84+
async findOne(o: string, ast: any) {
85+
d.calls.push(`findOne:${o}`);
86+
for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r;
87+
return null;
88+
},
89+
async create(o: string, data: Record<string, unknown>) {
90+
const id = String(data.id);
91+
const row = { ...data, id };
92+
storeFor(o).set(id, row);
93+
return row;
94+
},
95+
async update() { return null; },
96+
async delete(o: string, id: string) { d.calls.push(`delete:${o}`); return storeFor(o).delete(String(id)); },
97+
async count(o: string, ast: any) {
98+
return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)).length;
99+
},
100+
async deleteMany(o: string, ast: any) {
101+
d.calls.push(`deleteMany:${o}`);
102+
const doomed = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
103+
for (const r of doomed) storeFor(o).delete(String(r.id));
104+
return doomed.length;
105+
},
106+
async updateMany() { return 0; },
107+
};
108+
return d;
109+
}
110+
111+
async function boot() {
112+
const engine = new ObjectQL();
113+
const driver = makeStubDriver();
114+
engine.registerDriver(driver, true);
115+
await engine.init();
116+
engine.registry.registerObject(attObject as any, 'app:test');
117+
engine.registry.registerObject(taskObject as any, 'app:test');
118+
return { engine, driver };
119+
}
120+
121+
/** Seed rows straight into the driver store — no insert hooks involved. */
122+
function seed(driver: any, object: string, rows: Array<Record<string, unknown>>) {
123+
if (!driver.stores.get(object)) driver.stores.set(object, new Map());
124+
for (const row of rows) driver.stores.get(object)!.set(String(row.id), { ...row });
125+
}
126+
127+
const rowsIn = (driver: any, object: string): number => driver.stores.get(object)?.size ?? 0;
128+
129+
type Seen = { id: unknown; where: unknown; multi: unknown; mode: unknown; index: unknown };
130+
const snapshot = (ctx: HookContext): Seen => ({
131+
id: (ctx.input as any).id,
132+
where: (ctx.input as any).options?.where,
133+
multi: (ctx.input as any).options?.multi,
134+
mode: (ctx.dispatch as any)?.mode,
135+
index: (ctx.dispatch as any)?.index,
136+
});
137+
138+
describe('[#9719] registration-time validation of dispatchUnscopedMultiDelete', () => {
139+
it("refuses the flag on any event other than 'beforeDelete'", async () => {
140+
const { engine } = await boot();
141+
for (const event of ['beforeUpdate', 'afterDelete', 'beforeInsert']) {
142+
expect(() =>
143+
engine.registerHook(event, async () => {}, {
144+
object: 'att',
145+
dispatchUnscopedMultiDelete: true,
146+
}),
147+
).toThrow(/dispatchUnscopedMultiDelete/);
148+
}
149+
});
150+
151+
it("accepts the flag on 'beforeDelete'", async () => {
152+
const { engine } = await boot();
153+
expect(() =>
154+
engine.registerHook('beforeDelete', async () => {}, {
155+
object: 'att',
156+
dispatchUnscopedMultiDelete: true,
157+
}),
158+
).not.toThrow();
159+
});
160+
});
161+
162+
describe('[#9719] the whole-operation dispatch on an unscoped predicate delete', () => {
163+
it('dispatches ONCE, whole-operation-shaped, before the per-row fan-out', async () => {
164+
const { engine, driver } = await boot();
165+
seed(driver, 'att', [
166+
{ id: 'a1', status: 'x', owner: 'u1' },
167+
{ id: 'a2', status: 'y', owner: 'u1' },
168+
]);
169+
const seen: Seen[] = [];
170+
engine.registerHook(
171+
'beforeDelete',
172+
async (ctx: HookContext) => { seen.push(snapshot(ctx)); },
173+
{ object: 'att', dispatchUnscopedMultiDelete: true },
174+
);
175+
176+
await engine.delete('att', { multi: true, context: { userId: 'u1' } } as any);
177+
178+
// 1 whole-operation dispatch + 2 per-row dispatches, in that order.
179+
expect(seen).toHaveLength(3);
180+
expect(seen[0]).toMatchObject({ id: undefined, where: undefined, multi: true, mode: 'record', index: 0 });
181+
expect([seen[1]!.id, seen[2]!.id].sort()).toEqual(['a1', 'a2']);
182+
expect(seen[1]!.mode).toBe('per-row');
183+
// The handler that let it pass did not stop the wipe — engine policy stays
184+
// with the handler, not the dispatch.
185+
expect(rowsIn(driver, 'att')).toBe(0);
186+
});
187+
188+
it('a refusal from the flagged handler rejects the delete BEFORE any driver call', async () => {
189+
const { engine, driver } = await boot();
190+
seed(driver, 'att', [{ id: 'a1', status: 'x', owner: 'u1' }]);
191+
engine.registerHook(
192+
'beforeDelete',
193+
async (ctx: HookContext) => {
194+
if ((ctx.input as any).id === undefined) {
195+
const err: any = new Error('unscoped delete refused by test guard');
196+
err.code = 'TEST_UNSCOPED_REFUSED';
197+
err.status = 403;
198+
throw err;
199+
}
200+
},
201+
{ object: 'att', dispatchUnscopedMultiDelete: true },
202+
);
203+
204+
await expect(engine.delete('att', { multi: true, context: { userId: 'u1' } } as any))
205+
.rejects.toMatchObject({ code: 'TEST_UNSCOPED_REFUSED', status: 403 });
206+
// No doomed-row read, no deleteMany: the refusal cost nothing downstream.
207+
expect(driver.calls).toEqual([]);
208+
expect(rowsIn(driver, 'att')).toBe(1);
209+
});
210+
211+
it('the ZERO-MATCH limb: an unscoped delete of an EMPTY table still dispatches', async () => {
212+
const { engine, driver } = await boot();
213+
// No rows at all — the [D1] per-row gate would dispatch nothing.
214+
let dispatched = 0;
215+
engine.registerHook(
216+
'beforeDelete',
217+
async (ctx: HookContext) => {
218+
if ((ctx.input as any).id === undefined) {
219+
dispatched += 1;
220+
const err: any = new Error('unscoped delete refused by test guard');
221+
err.code = 'TEST_UNSCOPED_REFUSED';
222+
err.status = 403;
223+
throw err;
224+
}
225+
},
226+
{ object: 'att', dispatchUnscopedMultiDelete: true },
227+
);
228+
229+
await expect(engine.delete('att', { multi: true, context: { userId: 'u1' } } as any))
230+
.rejects.toMatchObject({ code: 'TEST_UNSCOPED_REFUSED' });
231+
expect(dispatched).toBe(1);
232+
expect(driver.calls).toEqual([]);
233+
});
234+
235+
it('positive control for the zero-match limb: WITHOUT the flag, the same shape dispatches nothing', async () => {
236+
const { engine, driver } = await boot();
237+
let dispatched = 0;
238+
engine.registerHook(
239+
'beforeDelete',
240+
async () => { dispatched += 1; },
241+
{ object: 'att' }, // same handler, same object — flag withheld
242+
);
243+
244+
// Resolves: zero rows matched, zero per-row dispatches ([D1]), and no
245+
// whole-operation dispatch without the declaration. This is exactly the
246+
// pre-#9719 wired behaviour, so the zero-match measurement above is a
247+
// measurement of the flag, not of the harness.
248+
await expect(engine.delete('att', { multi: true, context: { userId: 'u1' } } as any))
249+
.resolves.toBeDefined();
250+
expect(dispatched).toBe(0);
251+
expect(driver.calls).toContain('deleteMany:att');
252+
});
253+
254+
it("dispatches on `where: null` — the handler contract's other unscoped spelling", async () => {
255+
const { engine, driver } = await boot();
256+
seed(driver, 'att', [{ id: 'a1', status: 'x', owner: 'u1' }]);
257+
const seen: Seen[] = [];
258+
engine.registerHook(
259+
'beforeDelete',
260+
async (ctx: HookContext) => { seen.push(snapshot(ctx)); },
261+
{ object: 'att', dispatchUnscopedMultiDelete: true },
262+
);
263+
264+
await engine.delete('att', { multi: true, where: null, context: { userId: 'u1' } } as any);
265+
expect(seen[0]).toMatchObject({ id: undefined, where: null, multi: true, mode: 'record' });
266+
});
267+
268+
it('the whole-operation `scope` is identity-shared with the per-row contexts', async () => {
269+
const { engine, driver } = await boot();
270+
seed(driver, 'att', [{ id: 'a1', status: 'x', owner: 'u1' }]);
271+
const perRowSawMarker: unknown[] = [];
272+
engine.registerHook(
273+
'beforeDelete',
274+
async (ctx: HookContext) => {
275+
const scope = (ctx.dispatch as any)?.scope as Record<string, unknown>;
276+
if ((ctx.input as any).id === undefined) scope.marker = 'from-whole-op';
277+
else perRowSawMarker.push(scope.marker);
278+
},
279+
{ object: 'att', dispatchUnscopedMultiDelete: true },
280+
);
281+
282+
await engine.delete('att', { multi: true, context: { userId: 'u1' } } as any);
283+
expect(perRowSawMarker).toEqual(['from-whole-op']);
284+
});
285+
});
286+
287+
describe('[#9719] scoped deletes and undeclared registrations see NO new dispatch', () => {
288+
it('a real `where` predicate gets only the per-row fan-out', async () => {
289+
const { engine, driver } = await boot();
290+
seed(driver, 'att', [
291+
{ id: 'a1', status: 'x', owner: 'u1' },
292+
{ id: 'a2', status: 'y', owner: 'u1' },
293+
]);
294+
const seen: Seen[] = [];
295+
engine.registerHook(
296+
'beforeDelete',
297+
async (ctx: HookContext) => { seen.push(snapshot(ctx)); },
298+
{ object: 'att', dispatchUnscopedMultiDelete: true },
299+
);
300+
301+
await engine.delete('att', { multi: true, where: { status: 'x' }, context: { userId: 'u1' } } as any);
302+
expect(seen).toHaveLength(1);
303+
expect(seen[0]).toMatchObject({ id: 'a1', mode: 'per-row' });
304+
expect(rowsIn(driver, 'att')).toBe(1);
305+
});
306+
307+
it('`where: {}` is a REAL match-all query, not an unscoped delete', async () => {
308+
const { engine, driver } = await boot();
309+
seed(driver, 'att', [{ id: 'a1', status: 'x', owner: 'u1' }]);
310+
const seen: Seen[] = [];
311+
engine.registerHook(
312+
'beforeDelete',
313+
async (ctx: HookContext) => { seen.push(snapshot(ctx)); },
314+
{ object: 'att', dispatchUnscopedMultiDelete: true },
315+
);
316+
317+
await engine.delete('att', { multi: true, where: {}, context: { userId: 'u1' } } as any);
318+
// Per-row only: the guard's whole-operation branch is not summoned for a
319+
// query that really ran and really matched.
320+
expect(seen).toHaveLength(1);
321+
expect(seen[0]).toMatchObject({ id: 'a1', mode: 'per-row' });
322+
expect(rowsIn(driver, 'att')).toBe(0);
323+
});
324+
325+
it('a by-id delete is untouched', async () => {
326+
const { engine, driver } = await boot();
327+
seed(driver, 'att', [{ id: 'a1', status: 'x', owner: 'u1' }]);
328+
const seen: Seen[] = [];
329+
engine.registerHook(
330+
'beforeDelete',
331+
async (ctx: HookContext) => { seen.push(snapshot(ctx)); },
332+
{ object: 'att', dispatchUnscopedMultiDelete: true },
333+
);
334+
335+
await engine.delete('att', { where: { id: 'a1' }, context: { userId: 'u1' } } as any);
336+
expect(seen).toHaveLength(1);
337+
expect(seen[0]!.id).toBe('a1');
338+
expect(seen[0]!.mode).toBe('record');
339+
expect(rowsIn(driver, 'att')).toBe(0);
340+
});
341+
342+
it('an object whose registration does NOT declare the flag keeps exactly today\'s dispatches', async () => {
343+
const { engine, driver } = await boot();
344+
seed(driver, 'task', [
345+
{ id: 't1', status: 'x', owner: 'u1' },
346+
{ id: 't2', status: 'y', owner: 'u1' },
347+
]);
348+
const seen: Seen[] = [];
349+
engine.registerHook(
350+
'beforeDelete',
351+
async (ctx: HookContext) => { seen.push(snapshot(ctx)); },
352+
{ object: 'task' },
353+
);
354+
355+
await engine.delete('task', { multi: true, context: { userId: 'u1' } } as any);
356+
// Per-row dispatches only — no whole-operation call arrived, and the
357+
// unscoped wipe proceeds as it does today for undeclared objects.
358+
expect(seen).toHaveLength(2);
359+
expect(seen.every((s) => s.id !== undefined && s.mode === 'per-row')).toBe(true);
360+
expect(rowsIn(driver, 'task')).toBe(0);
361+
});
362+
});
363+
364+
describe('[#9719] the id slot is not a lever on the whole-operation context', () => {
365+
it('binding `input.id` is refused with HookTargetRebindError, nothing deleted', async () => {
366+
const { engine, driver } = await boot();
367+
seed(driver, 'att', [{ id: 'a1', status: 'x', owner: 'u1' }]);
368+
engine.registerHook(
369+
'beforeDelete',
370+
async (ctx: HookContext) => {
371+
if ((ctx.input as any).id === undefined) (ctx.input as any).id = 'a1';
372+
},
373+
{ object: 'att', dispatchUnscopedMultiDelete: true },
374+
);
375+
376+
await expect(engine.delete('att', { multi: true, context: { userId: 'u1' } } as any))
377+
.rejects.toMatchObject({ code: HOOK_TARGET_REBIND_ERROR_CODE, path: 'unscoped-multi' });
378+
expect(driver.calls).toEqual([]);
379+
expect(rowsIn(driver, 'att')).toBe(1);
380+
});
381+
});

0 commit comments

Comments
 (0)