From 4851dd925c4d9495f836a93d67f1ca5cada9209e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 08:27:13 +0000 Subject: [PATCH 1/3] fix(sharing): revoke every share on a deleted record, not just rule grants (#5103) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `sys_record_share` row says "principal P has level L on (object O, record R)". Delete R and the row describes nothing — yet it stayed in the table forever. #4779 (PR #5102) bound an `afterDelete` for this, but inside the sharing-RULE package, where two conditions fenced it in: it revokes only `source: 'rule'` rows, and `bindRuleHooks` binds only on objects that appear in `sys_sharing_rule`. So an object using nothing but MANUAL shares had no delete hook at all, and manual share + record delete = a permanent orphan. Harm is bounded today only because record ids are never reused — an assumption no gate enforces. A custom primary key, an import preserving ids, or any future recycling turns those rows into real escalation: a new record on a recycled id inherits the dead record's recipients. Maintainer ruling (2026-08-04, on the issue): option A. Option B (a platform polymorphic weak-reference cascade) is a separate engine-lane design card (#5180); when it lands these hooks collapse into it. - `record-share-cascade.ts` binds ONE global `beforeDelete`/`afterDelete` pair and judges the object's sharing posture from `sharingModel` metadata PER DELETE. Nothing is enumerated at boot, so nothing goes stale — an object that gains sharing at runtime is covered on its next delete with no rebind, which is a stronger answer to the ruling's hot-update requirement than a metadata subscription would have been. Bounded row sets are revoked synchronously and set-based; an unbounded delete queues an object-scoped orphan sweep rather than the rule path's revoke-then-regrant, which is unavailable here because nothing can re-create a manual share. System-context deletes cascade too. - `SharingService.sweepOrphanedRecordShares` is the record-existence twin of `sweepOrphanedRuleGrants` (#4433) — that one asks whether the RULE row still exists and therefore can never see a manual share. Runs on `kernel:bootstrapped`, keyset-paged with one batched existence probe per object per page and a scan cap that reports itself. An object whose probe FAILS keeps its rows: "could not ask" is not "the record is gone". - The `beforeDelete` row-set stash moves from `rule-hooks.ts` into `bulk-recompute.ts` beside its resolver, so both hook packages share one answer per write instead of resolving the same predicate twice. Rule recompute still never touches a manual share (#5102's pin, re-asserted in this branch's tests). Only the record's DELETION revokes it, and only because there is no longer anything to have access to. Fixes #5103 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015W6nhsDrz6zWQc8je12a1t --- .changeset/record-delete-share-cascade.md | 57 ++ .../plugin-sharing/src/bulk-recompute.ts | 64 ++ packages/plugins/plugin-sharing/src/index.ts | 14 + .../src/record-share-cascade.test.ts | 666 ++++++++++++++++++ .../src/record-share-cascade.ts | 262 +++++++ .../plugins/plugin-sharing/src/rule-hooks.ts | 52 +- .../plugin-sharing/src/sharing-plugin.ts | 53 ++ .../plugin-sharing/src/sharing-service.ts | 252 ++++++- 8 files changed, 1390 insertions(+), 30 deletions(-) create mode 100644 .changeset/record-delete-share-cascade.md create mode 100644 packages/plugins/plugin-sharing/src/record-share-cascade.test.ts create mode 100644 packages/plugins/plugin-sharing/src/record-share-cascade.ts diff --git a/.changeset/record-delete-share-cascade.md b/.changeset/record-delete-share-cascade.md new file mode 100644 index 0000000000..0aa2e7b5a6 --- /dev/null +++ b/.changeset/record-delete-share-cascade.md @@ -0,0 +1,57 @@ +--- +"@objectstack/plugin-sharing": patch +--- + +fix(sharing): deleting a record now revokes every `sys_record_share` row on it, whatever the source (#5103) + +A share row says "principal P has level L on (object O, record R)". Delete R and +the row describes nothing — yet until now it stayed in the table forever. + +#4779 (PR #5102) bound an `afterDelete` for this, but inside the sharing-RULE +package, where two conditions fenced it in: it revokes only `source: 'rule'` +rows, and it binds only on objects that appear in `sys_sharing_rule`. So an +object that uses nothing but MANUAL shares — a `sharingModel: 'private'` object +with no rule ever configured — had no delete hook at all, and **manual share + +record delete = a permanent orphan**. + +Today the harm is bounded, and only because record ids are never reused: the +`record_id IN (…)` predicate `buildReadFilter` emits matches nothing. Nothing +enforces that assumption. A custom primary key, an import that preserves ids, or +any future id recycling turns every one of those rows into a real privilege +escalation — a new record landing on a recycled id inherits the dead record's +recipients outright. Secondarily, `sys_record_share` grew without bound and +Setup's Record Shares list showed rows pointing at nothing. + +**What changed** + +- **A record-delete cascade on every sharing-capable object.** `plugin-sharing` + binds one `beforeDelete`/`afterDelete` pair with no object filter and judges + the object's sharing posture from its `sharingModel` metadata *per delete*. + Nothing is enumerated at boot, so nothing goes stale: an object that gains + `sharingModel` at runtime is covered on its very next delete, with no rebind. + Bounded deletes (a scalar id, an `$in` list, or a predicate matching at most + 1000 rows) are revoked synchronously and set-based; an unbounded one queues an + object-scoped orphan sweep instead. System-context deletes cascade too. +- **A boot-time orphan sweep keyed on record existence.** On + `kernel:bootstrapped`, share rows whose RECORD no longer exists are revoked — + historical orphans, rows a failed hook missed, and the one posture the cascade + deliberately skips (an unmarked system object). This is a different question + from the existing `sweepOrphanedRuleGrants`, which asks whether the RULE row + still exists and therefore can never see a manual share. Bounded per boot: + keyset pages, one batched existence probe per object per page, and a scan cap + that reports when it stopped early. An object whose existence probe FAILS has + its rows left in place — "could not ask" is never read as "the record is gone". + +**What did not change** + +Rule *recompute* still never touches a manual share. That boundary (#5102) is +the point: while the record exists, a manual grant is a human decision no rule +evaluation may overrule. Only the record's DELETION revokes it, and only because +there is no longer anything to have access to. + +New exports for hosts that compose the plugin by hand: +`bindRecordShareCascade` / `unbindRecordShareCascade`, +`objectCanCarryRecordShares`, `SharingService.revokeSharesForDeletedRecords`, +`SharingService.sweepOrphanedRecordShares`, and `effectiveSharingModel`. Nothing +was removed or renamed; the standard `SharingServicePlugin` composition needs no +changes. diff --git a/packages/plugins/plugin-sharing/src/bulk-recompute.ts b/packages/plugins/plugin-sharing/src/bulk-recompute.ts index f8c677a149..714cc49831 100644 --- a/packages/plugins/plugin-sharing/src/bulk-recompute.ts +++ b/packages/plugins/plugin-sharing/src/bulk-recompute.ts @@ -194,6 +194,70 @@ export async function resolveAffectedRows( } } +/** + * [#4779] Shared-`HookContext` key holding the row set the write is about to + * change, stashed by the `before` hook for the `after` hook to consume. + * + * The stash is necessary, not a convenience: an update that moves rows OUT of + * a rule's criteria makes them unfindable by the write's own predicate the + * instant it lands, and a delete removes them outright — so `afterUpdate` / + * `afterDelete` are structurally too late to ask "which rows was this?". + * `ObjectQL.update()` / `.delete()` reuse ONE `HookContext` instance across + * each before/after pair (they mutate `ctx.event` in place), which is the same + * seam `primary-bu-projection.ts`'s `__primaryBuUserId` rides on. + * + * [#5103] Lives HERE, next to the resolver, rather than in `rule-hooks.ts` + * where it started: two independent hook packages now need the same answer for + * the same write (the rule recompute, and the record-delete share cascade), + * and each resolving it separately would double the predicate query on every + * bulk write for no gain — the row set is a property of the WRITE, not of + * either subscriber. + */ +export const AFFECTED_ROWS_STASH_KEY = '__sharingAffectedRows'; + +/** + * Resolve (or reuse) the row set a `before` hook's write is about to change and + * park it on the shared `HookContext`. + * + * **Reuse is the point.** The first plugin-sharing `before` hook to run on a + * write resolves; every later one reads that answer back. Recomputing would be + * wasteful and — worse — could disagree, because a resolve issued after an + * earlier hook has already changed something is answering a different question. + * + * Never throws: `resolveAffectedRows` already fails safe to `unbounded`, and + * this adds the belt for a genuinely unexpected throw. "Unknown" must never + * degrade to "no rows" — that is the direction that silently skips cleanup. + */ +export async function stashAffectedRows( + engine: RecomputeEngine | { find?: RecomputeEngine['find'] }, + objectName: string, + hookCtx: any, + logger?: MinimalLogger, +): Promise { + const already = hookCtx?.[AFFECTED_ROWS_STASH_KEY] as AffectedRows | undefined; + if (already) return already; + let resolved: AffectedRows; + try { + resolved = typeof engine?.find === 'function' + ? await resolveAffectedRows(engine as RecomputeEngine, objectName, hookCtx, logger) + : { kind: 'unbounded', reason: 'resolve-failed', detail: 'engine has no find()' }; + } catch (err: any) { + resolved = { kind: 'unbounded', reason: 'resolve-failed', detail: err?.message }; + } + if (hookCtx && typeof hookCtx === 'object') hookCtx[AFFECTED_ROWS_STASH_KEY] = resolved; + return resolved; +} + +/** + * What an `after` hook should act on. A missing stash means no `before` hook of + * ours ran for this write, which is not "nothing changed" — it is "we do not + * know", and it reads as `unbounded` so the caller takes its safe branch. + */ +export function readAffectedRows(hookCtx: any): AffectedRows { + return (hookCtx?.[AFFECTED_ROWS_STASH_KEY] as AffectedRows | undefined) + ?? { kind: 'unbounded', reason: 'resolve-failed', detail: 'no before-hook stash' }; +} + /** * The asynchronous half of the ruling: re-grant after the synchronous revoke. * diff --git a/packages/plugins/plugin-sharing/src/index.ts b/packages/plugins/plugin-sharing/src/index.ts index eec30d137b..bda00f31c9 100644 --- a/packages/plugins/plugin-sharing/src/index.ts +++ b/packages/plugins/plugin-sharing/src/index.ts @@ -13,8 +13,11 @@ export { SysRecordShare, SysSharingRule, SysShareLink } from './objects/index.js export { SysBusinessUnit, SysBusinessUnitMember } from '@objectstack/platform-objects/identity'; export { SharingService, + effectiveSharingModel, type SharingEngine, type SharingServiceOptions, + type OrphanShareSweepOptions, + type OrphanShareSweepResult, } from './sharing-service.js'; export { SharingRuleService, @@ -43,10 +46,21 @@ export { RuleRegrantQueue, resolveAffectedRows, idsFromHookInput, + stashAffectedRows, + readAffectedRows, + AFFECTED_ROWS_STASH_KEY, type AffectedRows, type UnboundedReason, type RecomputeEngine, } from './bulk-recompute.js'; +export { + bindRecordShareCascade, + unbindRecordShareCascade, + objectCanCarryRecordShares, + orphanShareSweepQueue, + RECORD_SHARE_CASCADE_PACKAGE, + type CascadeEngine, +} from './record-share-cascade.js'; export { parseCriteria, isMatchAllCriteria, diff --git a/packages/plugins/plugin-sharing/src/record-share-cascade.test.ts b/packages/plugins/plugin-sharing/src/record-share-cascade.test.ts new file mode 100644 index 0000000000..d8d0cb2bcb --- /dev/null +++ b/packages/plugins/plugin-sharing/src/record-share-cascade.test.ts @@ -0,0 +1,666 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5103] Record delete ⇒ every `sys_record_share` row on that record goes, + * whatever its `source`. + * + * #4779 (PR #5102) added an `afterDelete`, but inside the sharing-RULE package: + * it revokes only `source: 'rule'` rows, and it binds only on objects that + * appear in `sys_sharing_rule`. So an object that uses nothing but manual + * shares had no delete hook at all, and its share rows outlived their records + * forever — a latent authorization defect the moment a record id is reused. + * + * Maintainer ruling (2026-08-04): option A — plugin-sharing binds `afterDelete` + * on all sharing-enabled objects (posture read from `sharingModel` METADATA, + * not from the rules table) and revokes ALL sources; plus a boot sweep keyed on + * "does the record still exist". + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; +import { SharingService } from './sharing-service.js'; +import { SharingRuleService } from './sharing-rule-service.js'; +import { + bindRecordShareCascade, + unbindRecordShareCascade, + objectCanCarryRecordShares, + orphanShareSweepQueue, + RECORD_SHARE_CASCADE_PACKAGE, +} from './record-share-cascade.js'; +import { bindRuleHooks, SHARING_RULE_HOOK_PACKAGE } from './rule-hooks.js'; +import { AFFECTED_ROWS_STASH_KEY, RULE_RECOMPUTE_ROW_CAP } from './bulk-recompute.js'; + +interface Row { [k: string]: any } + +const SYS = { isSystem: true, positions: [], permissions: [] } as any; +const ADMIN_SESSION = { isSystem: false, userId: 'admin' }; + +type HookEntry = { event: string; handler: (ctx: any) => any; options: Row }; + +/** + * A fake ObjectQL engine reproducing the parts of the write pipeline this fix + * depends on: + * + * - `before*` / `after*` of one write share ONE `HookContext` instance (the + * real engine mutates `ctx.event` in place); + * - a hook registered with no `object` option fires for EVERY object, which is + * `ObjectQLEngine.triggerHooks`' documented per-object matching and the + * seam the cascade's global bind rides on; + * - a predicate delete leaves `input.id` undefined; + * - `delete` refuses a predicate-shaped call without `multi: true`, pinned to + * the engine's own exported dispatch predicate (#4434, #4550). + */ +function makeEngine() { + const tables: Record = {}; + const schemas: Record = {}; + const hooks: HookEntry[] = []; + const ensure = (n: string) => (tables[n] ??= []); + const deleteCalls: Array<{ object: string; options: any }> = []; + const findCalls: Array<{ object: string; options: any }> = []; + + function matches(row: Row, f: any): boolean { + if (!f || typeof f !== 'object') return true; + if (Array.isArray(f.$or)) return f.$or.some((x: any) => matches(row, x)); + if (Array.isArray(f.$and)) return f.$and.every((x: any) => matches(row, x)); + for (const [k, v] of Object.entries(f)) { + if (k === '$or' || k === '$and') continue; + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (v != null && typeof v === 'object' && '$gt' in (v as any)) { + if (!(String(rv) > String((v as any).$gt))) return false; + continue; + } + if (rv !== v) return false; + } + return true; + } + + const engine = { + _tables: tables, + _schemas: schemas, + _deleteCalls: deleteCalls, + _findCalls: findCalls, + /** Set to make `find` throw for one object (the failed-probe branch). */ + failFindOn: null as string | null, + getSchema(name: string) { return schemas[name]; }, + registry: { + getObject(name: string) { return schemas[name]; }, + }, + async find(o: string, opts?: any) { + if (engine.failFindOn === o) throw new Error(`boom: ${o} unavailable`); + findCalls.push({ object: o, options: opts }); + const f = opts?.filter ?? opts?.where; + let rows = ensure(o).filter((r) => matches(r, f)); + const order = opts?.orderBy?.[0]; + if (order?.field) { + rows = [...rows].sort((a, b) => (String(a[order.field]) < String(b[order.field]) ? -1 : 1)); + } + return rows.slice(0, opts?.limit ?? 10000); + }, + async insert(o: string, data: any) { const row = { ...data }; ensure(o).push(row); return row; }, + async update(o: string, idOrData: any, dataOrOpts?: any) { + const data = typeof idOrData === 'object' ? idOrData : dataOrOpts; + const id = typeof idOrData === 'object' ? idOrData.id : idOrData; + const t = ensure(o); const i = t.findIndex((r) => r.id === id); + if (i >= 0) t[i] = { ...t[i], ...data }; + return t[i]; + }, + async delete(o: string, opts?: any) { + assertEngineDeleteDispatch(opts); + deleteCalls.push({ object: o, options: opts }); + const t = ensure(o); const where = opts?.where ?? {}; + for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); + return { ok: true }; + }, + registerHook(event: string, handler: (ctx: any) => any, options: Row = {}) { + hooks.push({ event, handler, options }); + hooks.sort((a, b) => (a.options.priority ?? 100) - (b.options.priority ?? 100)); + }, + unregisterHooksByPackage(packageId: string) { + let removed = 0; + for (let i = hooks.length - 1; i >= 0; i--) { + if (hooks[i].options.packageId === packageId) { hooks.splice(i, 1); removed++; } + } + return removed; + }, + boundFor(packageId: string) { return hooks.filter((h) => h.options.packageId === packageId); }, + + /** `triggerHooks`' matching: no `object` option ⇒ every object. */ + async fire(event: string, object: string, ctx: any) { + for (const h of [...hooks]) { + if (h.event !== event) continue; + const target = h.options.object; + if (target != null) { + const targets = Array.isArray(target) ? target : [target]; + if (!targets.includes('*') && !targets.includes(object)) continue; + } + await h.handler(ctx); + } + }, + + /** A single-id delete — `input.id` is populated, as the engine does. */ + async simulateDeleteById(object: string, id: string, session: any = ADMIN_SESSION) { + const ctx: any = { + object, + event: 'beforeDelete', + input: { id, options: { where: { id } } }, + session, + }; + await engine.fire('beforeDelete', object, ctx); + const t = ensure(object); + const i = t.findIndex((r) => r.id === id); + if (i >= 0) t.splice(i, 1); + ctx.event = 'afterDelete'; + ctx.result = { id }; + await engine.fire('afterDelete', object, ctx); + return ctx; + }, + + /** A predicate delete — no `input.id`, the #4779 precondition. */ + async simulateBulkDelete(object: string, where: any, session: any = ADMIN_SESSION) { + const ctx: any = { + object, + event: 'beforeDelete', + input: { id: undefined, options: { where, multi: true } }, + session, + }; + await engine.fire('beforeDelete', object, ctx); + const t = ensure(object); + for (let i = t.length - 1; i >= 0; i--) if (where == null || matches(t[i], where)) t.splice(i, 1); + ctx.event = 'afterDelete'; + await engine.fire('afterDelete', object, ctx); + return ctx; + }, + }; + return engine; +} + +type Engine = ReturnType; + +const shares = (engine: Engine) => engine._tables.sys_record_share ?? []; +const shareIds = (engine: Engine) => shares(engine).map((r) => String(r.id)).sort(); + +function manualShare(engine: Engine, object: string, recordId: string, recipient: string, id = `shr_${recordId}_${recipient}`) { + (engine._tables.sys_record_share ??= []).push({ + id, + object_name: object, + record_id: recordId, + recipient_type: 'user', + recipient_id: recipient, + access_level: 'read', + source: 'manual', + granted_by: 'admin', + }); + return id; +} + +describe('#5103 objectCanCarryRecordShares — the runtime, metadata-driven posture', () => { + it('accepts any object that DECLARES a sharing model', () => { + for (const sharingModel of ['private', 'public_read', 'public_read_write', 'controlled_by_parent']) { + expect(objectCanCarryRecordShares({ name: 'account', sharingModel })).toBe(true); + } + }); + + it('accepts a custom object with no declared model (ADR-0090 D1 secure default is private)', () => { + expect(objectCanCarryRecordShares({ name: 'inquiry', fields: {} })).toBe(true); + }); + + it('reads the nested `security.sharingModel` spelling too', () => { + expect(objectCanCarryRecordShares({ name: 'sys_thing', isSystem: true, security: { sharingModel: 'private' } })) + .toBe(true); + }); + + it('skips an UNMARKED system object — the documented boundary the boot sweep backstops', () => { + expect(objectCanCarryRecordShares({ name: 'sys_audit_log', isSystem: true })).toBe(false); + expect(objectCanCarryRecordShares({ name: 'sys_job_run' })).toBe(false); + }); + + it("never cascades on the sharing subsystem's own tables (no share-row delete firing a share-row delete)", () => { + expect(objectCanCarryRecordShares({ name: 'sys_record_share', sharingModel: 'private' })).toBe(false); + expect(objectCanCarryRecordShares({ name: 'sys_sharing_rule', sharingModel: 'private' })).toBe(false); + expect(objectCanCarryRecordShares({ name: 'sys_share_link' })).toBe(false); + }); + + it('falls toward cleanup when the schema cannot be resolved (unknown ≠ "leave the orphan")', () => { + expect(objectCanCarryRecordShares(undefined)).toBe(true); + expect(objectCanCarryRecordShares(null)).toBe(true); + }); +}); + +describe('#5103 record delete revokes every share on the record', () => { + let engine: Engine; + let sharing: SharingService; + let logger: any; + + beforeEach(() => { + logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + engine = makeEngine(); + // An object using ONLY manual shares — no sharing rule anywhere. This is + // the case #5102's rule-scoped afterDelete could not reach: `bindRuleHooks` + // enumerates `sys_sharing_rule.object_name`, so nothing was ever bound here. + engine._schemas.contract = { name: 'contract', sharingModel: 'private', fields: { owner_id: {} } }; + engine._tables.contract = [ + { id: 'ctr1', owner_id: 'boss' }, + { id: 'ctr2', owner_id: 'boss' }, + ]; + engine._tables.sys_record_share = []; + engine._tables.sys_sharing_rule = []; + sharing = new SharingService({ engine: engine as any, logger }); + bindRecordShareCascade(engine as any, sharing, logger); + }); + + it('binds the before/after pair globally — no object filter, so nothing goes stale', () => { + const bound = engine.boundFor(RECORD_SHARE_CASCADE_PACKAGE); + expect(bound.map((h) => h.event).sort()).toEqual(['afterDelete', 'beforeDelete']); + for (const h of bound) expect(h.options.object).toBeUndefined(); + }); + + /** + * THE REPRO — an object with NO rules at all, the case #5102 left uncovered. + * Revert-proof: unbind the cascade (or restore the `source: 'rule'` clause on + * the revoke) and the manual row survives its record, so this reads 1, not 0. + */ + it('revokes a MANUAL share when its record is deleted, on an object with no rules', async () => { + manualShare(engine, 'contract', 'ctr1', 'carol'); + manualShare(engine, 'contract', 'ctr2', 'dave'); + + await engine.simulateDeleteById('contract', 'ctr1'); + + expect(shareIds(engine)).toEqual(['shr_ctr2_dave']); + }); + + it('leaves the shares of records that were NOT deleted', async () => { + manualShare(engine, 'contract', 'ctr1', 'carol'); + manualShare(engine, 'contract', 'ctr2', 'carol', 'shr_keep'); + // Same record id on a DIFFERENT object — the revoke is keyed on both. + engine._schemas.invoice = { name: 'invoice', sharingModel: 'private', fields: { owner_id: {} } }; + manualShare(engine, 'invoice', 'ctr1', 'carol', 'shr_other_object'); + + await engine.simulateDeleteById('contract', 'ctr1'); + + expect(shareIds(engine)).toEqual(['shr_keep', 'shr_other_object']); + }); + + it('revokes on a SYSTEM-context delete too (the record is gone either way)', async () => { + manualShare(engine, 'contract', 'ctr1', 'carol'); + + await engine.simulateDeleteById('contract', 'ctr1', SYS); + + expect(shareIds(engine)).toEqual([]); + }); + + it('revokes every id of a BOUNDED predicate delete (`multi: true`, no input.id)', async () => { + manualShare(engine, 'contract', 'ctr1', 'carol'); + manualShare(engine, 'contract', 'ctr2', 'dave'); + engine._tables.contract.push({ id: 'ctr3', owner_id: 'other' }); + manualShare(engine, 'contract', 'ctr3', 'erin'); + + await engine.simulateBulkDelete('contract', { owner_id: 'boss' }); + + expect(shareIds(engine)).toEqual(['shr_ctr3_erin']); + }); + + it('issues a set-based revoke, not one delete per share row', async () => { + manualShare(engine, 'contract', 'ctr1', 'carol'); + manualShare(engine, 'contract', 'ctr1', 'dave'); + manualShare(engine, 'contract', 'ctr2', 'erin'); + engine._deleteCalls.length = 0; + + await engine.simulateBulkDelete('contract', { owner_id: 'boss' }); + + const shareDeletes = engine._deleteCalls.filter((c) => c.object === 'sys_record_share'); + expect(shareDeletes).toHaveLength(1); + expect(shareDeletes[0].options).toMatchObject({ + multi: true, + where: { object_name: 'contract', record_id: { $in: ['ctr1', 'ctr2'] } }, + }); + expect(shares(engine)).toEqual([]); + }); + + it('skips objects whose metadata puts them outside sharing (no query on the hot path)', async () => { + engine._schemas.sys_audit_log = { name: 'sys_audit_log', isSystem: true }; + engine._tables.sys_audit_log = [{ id: 'evt1' }]; + manualShare(engine, 'sys_audit_log', 'evt1', 'carol'); + engine._deleteCalls.length = 0; + + await engine.simulateDeleteById('sys_audit_log', 'evt1'); + + expect(engine._deleteCalls.filter((c) => c.object === 'sys_record_share')).toHaveLength(0); + expect(shareIds(engine)).toEqual(['shr_evt1_carol']); // the boot sweep's job + }); + + it('covers an object that gains `sharingModel` AFTER boot — no rebind needed', async () => { + engine._schemas.late = { name: 'late', isSystem: true }; // unmarked system object → skipped + engine._tables.late = [{ id: 'late1' }, { id: 'late2' }]; + manualShare(engine, 'late', 'late1', 'carol'); + await engine.simulateDeleteById('late', 'late1'); + expect(shareIds(engine)).toEqual(['shr_late1_carol']); + + // Metadata hot-update: the object turns on sharing. Nothing is re-bound. + engine._schemas.late = { name: 'late', isSystem: true, sharingModel: 'private', fields: { owner_id: {} } }; + manualShare(engine, 'late', 'late2', 'dave'); + + await engine.simulateDeleteById('late', 'late2'); + + expect(shareIds(engine)).toEqual(['shr_late1_carol']); // only the pre-flip orphan remains + }); + + it('never fails the write when the revoke throws — and names the repair', async () => { + manualShare(engine, 'contract', 'ctr1', 'carol'); + const boom = { + revokeSharesForDeletedRecords: vi.fn(async () => { throw new Error('driver down'); }), + sweepOrphanedRecordShares: vi.fn(async () => ({ scanned: 0, revoked: 0, unresolvedObjects: [], truncated: false })), + }; + unbindRecordShareCascade(engine as any); + bindRecordShareCascade(engine as any, boom as any, logger); + + await expect(engine.simulateDeleteById('contract', 'ctr1')).resolves.toBeDefined(); + + expect(boom.revokeSharesForDeletedRecords).toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('next '), + expect.objectContaining({ object: 'contract' }), + ); + }); + + it('unbinds cleanly (idempotent re-bind never doubles the hooks)', () => { + bindRecordShareCascade(engine as any, sharing, logger); + expect(engine.boundFor(RECORD_SHARE_CASCADE_PACKAGE)).toHaveLength(2); + expect(unbindRecordShareCascade(engine as any)).toBe(2); + expect(engine.boundFor(RECORD_SHARE_CASCADE_PACKAGE)).toHaveLength(0); + }); +}); + +describe('#5103 an UNBOUNDED delete reclaims by sweep, never by revoking the object', () => { + let engine: Engine; + let sharing: SharingService; + let logger: any; + + beforeEach(() => { + logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + engine = makeEngine(); + engine._schemas.contract = { name: 'contract', sharingModel: 'private', fields: { owner_id: {} } }; + engine._tables.contract = [{ id: 'ctr1', owner_id: 'boss' }, { id: 'ctr2', owner_id: 'boss' }]; + engine._tables.sys_record_share = []; + sharing = new SharingService({ engine: engine as any, logger }); + bindRecordShareCascade(engine as any, sharing, logger); + }); + + it('sweeps by record-existence when the delete names no predicate at all', async () => { + manualShare(engine, 'contract', 'ctr1', 'carol'); + manualShare(engine, 'contract', 'ctr2', 'dave'); + + await engine.simulateBulkDelete('contract', undefined); + await orphanShareSweepQueue.whenIdle(); + + expect(engine._tables.contract).toEqual([]); + expect(shareIds(engine)).toEqual([]); + }); + + it('spares shares whose record survived the unbounded delete', async () => { + // A survivor: the sweep asks each share row's record, so a row whose + // record is still there is untouched — the property that makes sweeping + // manual shares safe at all. + engine._tables.contract.push({ id: 'ctr3', owner_id: 'boss' }); + manualShare(engine, 'contract', 'ctr1', 'carol'); + manualShare(engine, 'contract', 'ctr3', 'erin', 'shr_survivor'); + const originalDelete = engine.delete.bind(engine); + // Simulate the engine's own row-scoping: the bulk delete removes only ctr1. + engine.delete = originalDelete; + + const ctx: any = { + object: 'contract', + event: 'beforeDelete', + input: { id: undefined, options: { where: undefined, multi: true } }, + session: ADMIN_SESSION, + }; + await engine.fire('beforeDelete', 'contract', ctx); + engine._tables.contract = engine._tables.contract.filter((r) => r.id !== 'ctr1'); + ctx.event = 'afterDelete'; + await engine.fire('afterDelete', 'contract', ctx); + await orphanShareSweepQueue.whenIdle(); + + expect(shareIds(engine)).toEqual(['shr_survivor']); + }); + + it('never revokes the object wholesale — the rule path\'s revoke-then-regrant is not available here', async () => { + manualShare(engine, 'contract', 'ctr2', 'dave'); + engine._deleteCalls.length = 0; + + await engine.simulateBulkDelete('contract', undefined); + + // Not a single `{ object_name: 'contract' }`-only delete: that shape would + // destroy manual grants nothing could ever re-create. + for (const call of engine._deleteCalls.filter((c) => c.object === 'sys_record_share')) { + expect(call.options.where).not.toEqual({ object_name: 'contract' }); + } + }); + + it('treats an over-cap predicate as unbounded and still converges', async () => { + engine._tables.contract = []; + for (let i = 0; i < RULE_RECOMPUTE_ROW_CAP + 1; i++) { + engine._tables.contract.push({ id: `big${i}`, owner_id: 'boss' }); + } + manualShare(engine, 'contract', 'big0', 'carol'); + + const ctx = await engine.simulateBulkDelete('contract', { owner_id: 'boss' }); + expect((ctx as any)[AFFECTED_ROWS_STASH_KEY]).toMatchObject({ kind: 'unbounded', reason: 'over-cap' }); + await orphanShareSweepQueue.whenIdle(); + + expect(shareIds(engine)).toEqual([]); + }); +}); + +describe('#5103 boot sweep — the record-existence predicate', () => { + let engine: Engine; + let sharing: SharingService; + let logger: any; + + beforeEach(() => { + logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + engine = makeEngine(); + engine._schemas.contract = { name: 'contract', sharingModel: 'private', fields: { owner_id: {} } }; + engine._tables.contract = [{ id: 'ctr_live', owner_id: 'boss' }]; + engine._tables.sys_record_share = []; + sharing = new SharingService({ engine: engine as any, logger }); + }); + + it('removes historical orphans of EVERY source and keeps the valid rows', async () => { + manualShare(engine, 'contract', 'ctr_live', 'carol', 'shr_live_manual'); + manualShare(engine, 'contract', 'ctr_gone', 'carol', 'shr_dead_manual'); + (engine._tables.sys_record_share ??= []).push( + { id: 'shr_live_rule', object_name: 'contract', record_id: 'ctr_live', recipient_type: 'user', recipient_id: 'wes', access_level: 'read', source: 'rule', source_id: 'srule_1' }, + { id: 'shr_dead_rule', object_name: 'contract', record_id: 'ctr_gone', recipient_type: 'user', recipient_id: 'wes', access_level: 'read', source: 'rule', source_id: 'srule_1' }, + ); + + const result = await sharing.sweepOrphanedRecordShares(); + + expect(result).toMatchObject({ scanned: 4, revoked: 2, unresolvedObjects: [], truncated: false }); + expect(shareIds(engine)).toEqual(['shr_live_manual', 'shr_live_rule']); + }); + + it('is idempotent — a second boot finds nothing to do', async () => { + manualShare(engine, 'contract', 'ctr_gone', 'carol'); + expect((await sharing.sweepOrphanedRecordShares()).revoked).toBe(1); + expect((await sharing.sweepOrphanedRecordShares()).revoked).toBe(0); + expect((await sharing.sweepOrphanedRecordShares()).revoked).toBe(0); + }); + + it('covers the posture the cascade skips (an unmarked system object)', async () => { + engine._schemas.sys_audit_log = { name: 'sys_audit_log', isSystem: true }; + engine._tables.sys_audit_log = []; + manualShare(engine, 'sys_audit_log', 'evt_gone', 'carol'); + + expect((await sharing.sweepOrphanedRecordShares()).revoked).toBe(1); + expect(shareIds(engine)).toEqual([]); + }); + + /** + * "Could not ask" is not "the record is gone". A probe failure that deleted + * would turn a transient driver error into permanent access loss — the + * inverse of the #4757 lesson ("nothing was queried" ≠ "nothing matched"). + */ + it('LEAVES rows alone when the existence probe fails, and reports the object', async () => { + manualShare(engine, 'contract', 'ctr_gone', 'carol'); + engine.failFindOn = 'contract'; + + const result = await sharing.sweepOrphanedRecordShares(); + + expect(result.revoked).toBe(0); + expect(result.unresolvedObjects).toEqual(['contract']); + expect(shareIds(engine)).toEqual(['shr_ctr_gone_carol']); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('could not check whether records still exist'), + expect.objectContaining({ object: 'contract' }), + ); + }); + + it('scopes to one object when asked (the unbounded-delete repair)', async () => { + engine._schemas.invoice = { name: 'invoice', sharingModel: 'private', fields: { owner_id: {} } }; + engine._tables.invoice = []; + manualShare(engine, 'contract', 'ctr_gone', 'carol', 'shr_contract_orphan'); + manualShare(engine, 'invoice', 'inv_gone', 'carol', 'shr_invoice_orphan'); + + const result = await sharing.sweepOrphanedRecordShares({ object: 'contract' }); + + expect(result).toMatchObject({ scanned: 1, revoked: 1 }); + expect(shareIds(engine)).toEqual(['shr_invoice_orphan']); + }); + + it('probes existence in ONE batched query per object per page, not one per share row', async () => { + for (let i = 0; i < 25; i++) manualShare(engine, 'contract', `ctr_gone_${i}`, 'carol', `shr_${i}`); + engine._findCalls.length = 0; + + await sharing.sweepOrphanedRecordShares({ batchSize: 100 }); + + const probes = engine._findCalls.filter((c) => c.object === 'contract'); + expect(probes).toHaveLength(1); + expect(probes[0].options.where).toMatchObject({ id: { $in: expect.any(Array) } }); + expect(shares(engine)).toEqual([]); + }); + + it('pages by keyset and REPORTS a scan that its cap cut short', async () => { + for (let i = 0; i < 12; i++) manualShare(engine, 'contract', `ctr_gone_${i}`, 'carol', `shr_${String(i).padStart(2, '0')}`); + + const result = await sharing.sweepOrphanedRecordShares({ batchSize: 5, max: 10 }); + + expect(result.scanned).toBe(10); + expect(result.truncated).toBe(true); + // A capped pass still cleans what it saw, and the rest survive for the next. + expect(result.revoked).toBe(10); + expect(shares(engine)).toHaveLength(2); + }); + + it('walks past rows it just deleted (a seek, never an OFFSET — #4363)', async () => { + // Every row on this page is an orphan and gets deleted; an offset-paged + // walk would then skip the next page's rows entirely. + for (let i = 0; i < 9; i++) manualShare(engine, 'contract', `ctr_gone_${i}`, 'carol', `shr_${i}`); + + const result = await sharing.sweepOrphanedRecordShares({ batchSize: 3 }); + + expect(result.scanned).toBe(9); + expect(result.revoked).toBe(9); + expect(shares(engine)).toEqual([]); + }); +}); + +describe('#5103 coexistence with the #5102 rule hooks', () => { + let engine: Engine; + let sharing: SharingService; + let rules: SharingRuleService; + let logger: any; + + beforeEach(async () => { + logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + engine = makeEngine(); + engine._schemas.opportunity = { name: 'opportunity', sharingModel: 'private', fields: { owner_id: {} } }; + engine._tables.opportunity = [ + { id: 'opp1', region: 'east', owner_id: 'boss' }, + { id: 'opp2', region: 'east', owner_id: 'boss' }, + ]; + engine._tables.sys_record_share = []; + engine._tables.sys_sharing_rule = [{ + id: 'srule_east', + name: 'east_to_alice', + label: 'East → Alice', + object_name: 'opportunity', + criteria_json: JSON.stringify({ region: 'east' }), + recipient_type: 'user', + recipient_id: 'alice', + access_level: 'edit', + active: true, + }]; + sharing = new SharingService({ engine: engine as any, logger }); + rules = new SharingRuleService({ engine: engine as any, sharing, logger }); + bindRuleHooks(engine as any, rules, await rules.listRules({ activeOnly: true }, SYS), logger); + bindRecordShareCascade(engine as any, sharing, logger); + }); + + it('revokes BOTH the rule grant and the manual share on a rules-bearing object', async () => { + await rules.evaluateRule('srule_east', SYS); + expect(shares(engine).filter((r) => r.source === 'rule')).toHaveLength(2); + manualShare(engine, 'opportunity', 'opp1', 'carol', 'shr_manual_opp1'); + manualShare(engine, 'opportunity', 'opp2', 'carol', 'shr_manual_opp2'); + + await engine.simulateDeleteById('opportunity', 'opp1'); + + const left = shares(engine); + expect(left.every((r) => r.record_id === 'opp2')).toBe(true); + expect(left.map((r) => r.source).sort()).toEqual(['manual', 'rule']); + }); + + /** + * #5102's pin, re-asserted from this branch: a rule RECOMPUTE (record still + * exists) must never touch a manual share. Only a record DELETE may, and + * only because the record is gone. + */ + it('rule recompute still never touches a manual share (#5102 stays pinned)', async () => { + await rules.evaluateRule('srule_east', SYS); + manualShare(engine, 'opportunity', 'opp1', 'carol', 'shr_manual_survivor'); + + // Move opp1 out of the rule's criteria — the rule grant goes, the manual + // share stays, because opp1 is still a record a human made a decision about. + const ctx: any = { + object: 'opportunity', + event: 'beforeUpdate', + input: { id: undefined, data: { region: 'west' }, options: { where: { id: 'opp1' }, multi: true } }, + session: ADMIN_SESSION, + }; + await engine.fire('beforeUpdate', 'opportunity', ctx); + engine._tables.opportunity[0].region = 'west'; + ctx.event = 'afterUpdate'; + await engine.fire('afterUpdate', 'opportunity', ctx); + + const left = shares(engine); + expect(left.find((r) => r.id === 'shr_manual_survivor')).toBeDefined(); + expect(left.filter((r) => r.source === 'rule' && r.record_id === 'opp1')).toEqual([]); + }); + + it('resolves the write\'s row set ONCE for both hook packages (shared stash)', async () => { + manualShare(engine, 'opportunity', 'opp1', 'carol'); + engine._findCalls.length = 0; + + const ctx = await engine.simulateBulkDelete('opportunity', { region: 'east' }); + + expect((ctx as any)[AFFECTED_ROWS_STASH_KEY]).toMatchObject({ kind: 'rows' }); + // Exactly one predicate resolve against the target object across both + // `beforeDelete` hooks — the stash is reused, not recomputed. + const resolves = engine._findCalls.filter( + (c) => c.object === 'opportunity' && c.options?.limit === RULE_RECOMPUTE_ROW_CAP + 1, + ); + expect(resolves).toHaveLength(1); + }); + + it('both packages bind, and neither unbind touches the other', () => { + expect(engine.boundFor(SHARING_RULE_HOOK_PACKAGE).length).toBeGreaterThan(0); + expect(engine.boundFor(RECORD_SHARE_CASCADE_PACKAGE)).toHaveLength(2); + + unbindRecordShareCascade(engine as any); + + expect(engine.boundFor(SHARING_RULE_HOOK_PACKAGE).length).toBeGreaterThan(0); + expect(engine.boundFor(RECORD_SHARE_CASCADE_PACKAGE)).toHaveLength(0); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/record-share-cascade.ts b/packages/plugins/plugin-sharing/src/record-share-cascade.ts new file mode 100644 index 0000000000..6ce28524d3 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/record-share-cascade.ts @@ -0,0 +1,262 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5103] The record-delete → share-revoke cascade. + * + * ## The invariant + * + * A `sys_record_share` row says "principal P has level L on (object O, record + * R)". Delete R and the row cannot describe any access at all — there is + * nothing left to have access to. So: **record gone ⇒ every share on it gone, + * whatever its `source`.** + * + * ## What was missing + * + * #4779 (PR #5102) bound `afterDelete` inside the sharing-RULE package, and + * two conditions fenced it in: + * + * 1. it revokes only `source: 'rule'` rows — correct for that package (rule + * recompute must never touch a human's manual decision, which #5102 + * pinned and this module does not regress: recompute still only ever sees + * rule rows); + * 2. it binds only on objects that appear in `sys_sharing_rule` — so an + * object using nothing but manual shares had no delete hook at all. + * + * Together: **manual share + record delete = a permanent orphan, on every + * object.** Harmless only while record ids are never reused — an assumption + * nothing in the platform enforces. Recycle an id (custom primary keys, an + * import that preserves ids) and the new record inherits the dead one's + * recipients. + * + * ## The binding model, and why there is nothing to rebind + * + * The maintainer ruling (2026-08-04) chose option A: plugin-sharing binds + * `afterDelete` on all sharing-enabled objects, "sharing-enabled" decided at + * runtime by `sharingModel` METADATA rather than by the rules table — which + * the ruling notes is a different enumeration model from the rule hooks', and + * asks for metadata hot-updates to be handled if a seam allows it. + * + * A seam does, and it is better than a rebind: bind ONE hook pair with no + * object filter and evaluate the metadata predicate **inside the handler**, at + * delete time. An object that gains `sharingModel` an hour after boot is + * covered on its very next delete, because the enumeration never happened — + * there is no bound set to go stale, and therefore no rebind to forget. (The + * same reason `plugin-pinyin-search` binds its companion hooks globally with a + * cheap early-out instead of enumerating provisioned objects.) The predicate + * is `objectCanCarryRecordShares`, one exported function, tested in isolation. + * + * Cost of the global bind: one registry lookup (an in-memory map read) per + * delete. Objects the predicate rejects return before any query runs. + * + * ## Bounded and unbounded deletes + * + * A predicate (`multi: true`) delete does not name its ids, and after it lands + * the rows are unfindable — the same problem #4779 solved with a `beforeDelete` + * stash, whose resolver and `HookContext` key this module now SHARES rather + * than duplicating (`bulk-recompute.ts`). Whichever of the two packages' + * `before` hooks runs first resolves; the other reads the same answer. + * + * - bounded row set → revoke every share of those ids, synchronously, + * set-based; + * - unbounded (no predicate at all, or over the cap) → queue an + * object-scoped orphan sweep. NOT the rule path's "revoke everything on the + * object and re-grant asynchronously": that trade is only sound where a + * reconcile can put the grants back, and nothing can ever re-create a + * manual share. The sweep instead asks, per share row, whether its record + * still exists — which needs no id list from the write. + * + * The `kernel:bootstrapped` sweep (unscoped) is the durable backstop for both: + * a hook that failed, a process that died mid-cascade, and every orphan that + * predates this code converge on the next boot. + */ + +import { + stashAffectedRows, + readAffectedRows, + RuleRegrantQueue, +} from './bulk-recompute.js'; +import { effectiveSharingModel, type SharingService } from './sharing-service.js'; + +export const RECORD_SHARE_CASCADE_PACKAGE = 'plugin-sharing:record-share-cascade'; + +/** + * The sharing subsystem's own tables. They are never the TARGET of a share + * (`sys_record_share` is on the enforcement bypass list precisely so the gates + * do not recurse through themselves), and cascading on `sys_record_share` + * itself would mean a share-row delete firing a share-row delete. + */ +const SHARING_OWN_OBJECTS = new Set([ + 'sys_record_share', + 'sys_sharing_rule', + 'sys_share_link', +]); + +/** + * Could `schema`'s object carry `sys_record_share` rows? The runtime, + * metadata-driven answer to "is sharing enabled here" — evaluated per delete, + * so it tracks metadata changes with no rebind. + * + * Deliberately WIDER than `assertSharingEnforced`, which gates whether a new + * MANUAL grant may be created. Two rows can exist that that gate would refuse + * today: one written before the object's `sharingModel` changed, and one + * materialised by the rule evaluator (which grants under system context and so + * skips the gate entirely). A cleanup predicate narrower than the set of rows + * that can exist is how orphans survive, and being wide costs only a delete + * statement against a record that is already gone. + * + * The one exclusion beyond the subsystem's own tables is a SYSTEM object that + * declares no `sharingModel`: it resolves to public (ADR-0090 D1 makes the + * secure default apply to custom objects, not platform ones), no gate consults + * shares on it, and sparing it keeps this hook off the platform's hottest + * delete paths. A rule pointed at such an object could still materialise rows + * there — that residue is the boot sweep's, and it is the documented boundary + * of this predicate rather than an unnoticed hole. + * + * An unresolvable schema returns `true`: "we cannot tell" must fall toward + * cleaning up, never toward the orphan this issue is about. + */ +export function objectCanCarryRecordShares(schema: unknown): boolean { + if (schema == null) return true; + const s = schema as any; + const name = s?.name == null ? '' : String(s.name); + if (SHARING_OWN_OBJECTS.has(name)) return false; + const declared = s?.sharingModel ?? s?.security?.sharingModel; + if (declared != null) return true; + return effectiveSharingModel(s) !== 'public'; +} + +/** The slice of the engine this module needs. */ +export interface CascadeEngine { + registerHook( + event: string, + handler: (ctx: any) => any | Promise, + options?: { object?: string | string[]; priority?: number; packageId?: string }, + ): void; + unregisterHooksByPackage(packageId: string): number; + find?(object: string, options?: any): Promise; + getSchema?(object: string): any; + registry?: { getObject?(name: string): any }; +} + +interface MinimalLogger { + info?: (msg: any, ...rest: any[]) => void; + warn?: (msg: any, ...rest: any[]) => void; +} + +/** + * The in-process executor for the unbounded-delete branch's object-scoped + * sweep. Module-scoped for the same reason `ruleRegrantQueue` is: a rebind + * must not orphan work already in flight. Serialized, so a burst of bulk + * deletes cannot fan out into parallel table walks. Exported for tests to + * await; production code never does. + */ +export const orphanShareSweepQueue = new RuleRegrantQueue(); + +function resolveSchema(engine: CascadeEngine, objectName: string): unknown { + try { + const fromRegistry = engine.registry?.getObject?.(objectName); + if (fromRegistry) return fromRegistry; + } catch { + /* fall through to getSchema */ + } + try { + return engine.getSchema?.(objectName); + } catch { + return undefined; + } +} + +/** + * Bind the record-delete → share-revoke cascade. Idempotent: unbinds its own + * package first, so a re-bind (hot reload, a second `kernel:ready`) never + * doubles the hooks. + * + * Runs for SYSTEM writes too — unlike the rule recompute, which leaves seed + * writes to the boot backfill. There is no equivalent backstop here: the + * record is gone either way, and a system-context delete (a seed reset, a + * platform cleanup job, an admin tool) orphans a manual share exactly as an + * interactive one does. + */ +export function bindRecordShareCascade( + engine: CascadeEngine, + sharing: Pick, + logger?: MinimalLogger, +): void { + if (typeof engine.registerHook !== 'function') return; + if (typeof engine.unregisterHooksByPackage === 'function') { + engine.unregisterHooksByPackage(RECORD_SHARE_CASCADE_PACKAGE); + } + // 190 — after the rule package's 180, so on a rules-bearing object the + // rule-scoped revoke runs first and this one sweeps up whatever it left + // (manual rows). Either order reaches the same state; this one keeps the + // narrower, subsystem-owned revoke first. + const opts = { packageId: RECORD_SHARE_CASCADE_PACKAGE, priority: 190 }; + + const applies = (objectName: string): boolean => { + if (!objectName) return false; + if (SHARING_OWN_OBJECTS.has(objectName)) return false; + return objectCanCarryRecordShares(resolveSchema(engine, objectName)); + }; + + engine.registerHook('beforeDelete', async (ctx: any) => { + const objectName = String(ctx?.object ?? ''); + if (!applies(objectName)) return; + // Must be `before`: the delete is what makes these rows unfindable. + // Shared stash — the rule package's own `beforeDelete` reads or writes the + // same answer, so a write resolves its row set once however many of our + // hook packages are bound to it. + await stashAffectedRows(engine as any, objectName, ctx, logger); + }, opts); + + engine.registerHook('afterDelete', async (ctx: any) => { + const objectName = String(ctx?.object ?? ''); + if (!applies(objectName)) return; + try { + const affected = readAffectedRows(ctx); + if (affected.kind === 'rows') { + if (affected.ids.length === 0) return; + await sharing.revokeSharesForDeletedRecords(objectName, affected.ids); + return; + } + // Unbounded: the ids are unknown, so ask the shares instead of the write. + // Queued, because the walk's cost is unrelated to this write's and a hook + // must not hold the caller. Under-cleaning for a moment is the safe + // direction; over-deleting a manual share would be unrecoverable. + logger?.warn?.( + '[sharing] a bulk delete touched more rows than could be enumerated — the shares of the ' + + 'deleted records are being reclaimed by a background orphan sweep instead ' + + '(a restart re-runs the same sweep)', + { object: objectName, reason: affected.reason }, + ); + orphanShareSweepQueue.enqueue( + () => sharing.sweepOrphanedRecordShares({ object: objectName }).then(() => undefined), + (err: any) => logger?.warn?.( + '[sharing] background orphan share sweep failed — share rows for the deleted records stay ' + + 'until the next sweep (any bulk delete on this object, or a restart)', + { object: objectName, error: err?.message }, + ), + ); + } catch (err: any) { + // The delete has already landed; failing here would not bring the record + // back, so never rethrow. `warn`, not `error`: the consequence is a stale + // share row that no live record matches, and the `kernel:bootstrapped` + // sweep repairs it on the next boot — a functional degradation with a + // named repair path, not silent durability loss. + logger?.warn?.( + '[sharing] could not revoke the shares of a deleted record — the rows stay until the next ' + + 'boot-time orphan sweep reclaims them', + { object: objectName, error: err?.message }, + ); + } + }, opts); + + logger?.info?.( + '[sharing] record-delete share cascade bound (all objects; sharing posture judged per delete)', + ); +} + +/** Unbind the cascade. Returns the number of hooks removed. */ +export function unbindRecordShareCascade(engine: CascadeEngine): number { + if (typeof engine.unregisterHooksByPackage !== 'function') return 0; + return engine.unregisterHooksByPackage(RECORD_SHARE_CASCADE_PACKAGE); +} diff --git a/packages/plugins/plugin-sharing/src/rule-hooks.ts b/packages/plugins/plugin-sharing/src/rule-hooks.ts index 3b5aff9577..9d42cee84a 100644 --- a/packages/plugins/plugin-sharing/src/rule-hooks.ts +++ b/packages/plugins/plugin-sharing/src/rule-hooks.ts @@ -6,7 +6,8 @@ import { isMatchAllCriteria, SharingCriteriaValidationError } from './rule-crite import { RULE_RECOMPUTE_ROW_CAP, RuleRegrantQueue, - resolveAffectedRows, + stashAffectedRows as stashAffectedRowsOnCtx, + readAffectedRows, type AffectedRows, } from './bulk-recompute.js'; @@ -14,20 +15,6 @@ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; export const SHARING_RULE_HOOK_PACKAGE = 'plugin-sharing:rules'; -/** - * [#4779] Shared-`HookContext` key holding the row set the write is about to - * change, stashed by the `before` hook for the `after` hook to consume. - * - * The stash is necessary, not a convenience: an update that moves rows OUT of - * a rule's criteria makes them unfindable by the write's own predicate the - * instant it lands, and a delete removes them outright — so `afterUpdate` / - * `afterDelete` are structurally too late to ask "which rows was this?". - * `ObjectQL.update()` / `.delete()` reuse ONE `HookContext` instance across - * each before/after pair (they mutate `ctx.event` in place), which is the same - * seam `primary-bu-projection.ts`'s `__primaryBuUserId` rides on. - */ -const STASH_KEY = '__sharingAffectedRows'; - /** * Package id for the `sys_sharing_rule` DATA-change triggers that re-run the * bind (#2592). Deliberately distinct from {@link SHARING_RULE_HOOK_PACKAGE} @@ -94,8 +81,17 @@ export const ruleRegrantQueue = new RuleRegrantQueue(); * - `afterDelete` — revoke the deleted rows' rule grants. Nothing can * re-grant them: `evaluateRule` iterates records that still exist, so a * grant whose record is gone is unreachable by every reconcile path and - * outlives restarts (the orphan noted at the tail of #4779). Harmless only - * while record ids are never reused — an assumption no gate enforces. + * outlives restarts (the orphan noted at the tail of #4779). + * + * [#5103] This package covers `source: 'rule'` rows only, and only on objects + * that HAVE a rule — which left a manual share on a deleted record orphaned + * forever. The general invariant ("the record is gone, so no share on it can + * be valid") is not the rule subsystem's to enforce and now lives in + * `record-share-cascade.ts`, bound on every sharing-capable object regardless + * of rules. The two are deliberately independent: this one keeps working if + * the cascade is unbound, and its unbounded-delete branch (revoke the object's + * rule grants, re-grant asynchronously) is a rule-only trade the cascade must + * never make on manual rows. * * [#4779] `if (!id) return` — the line these hooks used to open with — is * gone. It read as a cheap guard and was in fact the whole defect: predicate @@ -149,23 +145,21 @@ export function bindRuleHooks( ); }; + /** + * [#5103] Delegates to the shared stash: the record-delete cascade binds + * its own `beforeDelete` on the same objects, and whichever of the two runs + * first resolves the row set for both. Still skips system writes — the + * recompute half deliberately leaves seeds to the boot backfill — but the + * skip is now only about *this* subscriber; the cascade stashes for system + * writes on its own account. + */ const stashAffectedRows = async (ctx: any) => { if ((ctx?.session as any)?.isSystem) return; - try { - ctx[STASH_KEY] = typeof engine.find === 'function' - ? await resolveAffectedRows(engine as Required, objectName, ctx, logger) - : ({ kind: 'unbounded', reason: 'resolve-failed', detail: 'engine has no find()' } as AffectedRows); - } catch (err: any) { - // resolveAffectedRows already fails safe; this is the belt for a - // genuinely unexpected throw. Unknown must never degrade to "no rows". - ctx[STASH_KEY] = { kind: 'unbounded', reason: 'resolve-failed', detail: err?.message } as AffectedRows; - } + await stashAffectedRowsOnCtx(engine, objectName, ctx, logger); }; /** What the `after` hook should act on when no `before` hook ran. */ - const affectedFrom = (ctx: any): AffectedRows => - (ctx?.[STASH_KEY] as AffectedRows | undefined) - ?? ({ kind: 'unbounded', reason: 'resolve-failed', detail: 'no before-hook stash' } as AffectedRows); + const affectedFrom = (ctx: any): AffectedRows => readAffectedRows(ctx); engine.registerHook('afterInsert', async (ctx: any) => { if ((ctx?.session as any)?.isSystem) return; diff --git a/packages/plugins/plugin-sharing/src/sharing-plugin.ts b/packages/plugins/plugin-sharing/src/sharing-plugin.ts index 3094ec5719..0b312ce1c2 100644 --- a/packages/plugins/plugin-sharing/src/sharing-plugin.ts +++ b/packages/plugins/plugin-sharing/src/sharing-plugin.ts @@ -13,6 +13,7 @@ import { registerShareLinkRoutes } from './share-link-routes.js'; import { bindRuleHooks, unbindAllRuleHooks, bindRuleCriteriaGuard, RULE_REBIND_TRIGGER_PACKAGE } from './rule-hooks.js'; import { bindRuleProvenanceStamp, unbindRuleProvenanceStamp } from './sharing-rule-provenance.js'; import { bindPrimaryBuHooks, backfillPrimaryBu } from './primary-bu-projection.js'; +import { bindRecordShareCascade } from './record-share-cascade.js'; import { bootstrapDeclaredSharingRules } from './bootstrap-declared-sharing-rules.js'; export interface SharingPluginOptions { @@ -434,6 +435,7 @@ export class SharingServicePlugin implements Plugin { this.service = new SharingService({ engine: engine as SharingEngine, bypassObjects: this.options.bypassObjects, + logger: ctx.logger as any, // [ADR-0057] Late-bound lookup of the enterprise hierarchy resolver. // Open edition: not registered → hierarchy scopes fail closed to own. hierarchyResolver: () => { @@ -463,6 +465,31 @@ export class SharingServicePlugin implements Plugin { ctx.logger.warn('SharingServicePlugin: primary-bu projection not started', { error: err?.message }); } + // [#5103] Record delete ⇒ every share on that record is revoked, whatever + // its source. Bound REGARDLESS of `enforce`, and deliberately: with + // enforcement off the rows are not consulted, but they are still written + // (by rules, by the share REST surface in a host that mounts it) and + // still accumulate — and a deployment that flips `enforce` back on must + // not inherit a table of dangling grants. Same reasoning as the primary-BU + // projection above: this is data hygiene on a table this plugin owns, not + // an access-control surface. + // + // Not bound per object: the posture is judged per delete from live + // metadata, so an object that gains `sharingModel` after boot is covered + // without a rebind (see record-share-cascade.ts). + try { + if (typeof engine.registerHook === 'function' && typeof engine.unregisterHooksByPackage === 'function') { + bindRecordShareCascade(engine, this.service, ctx.logger as any); + } else { + ctx.logger.warn( + 'SharingServicePlugin: engine has no hook API — record deletes will NOT revoke their ' + + 'sys_record_share rows; the kernel:bootstrapped orphan sweep is the only reclaim', + ); + } + } catch (err: any) { + ctx.logger.warn('SharingServicePlugin: record-share delete cascade not bound', { error: err?.message }); + } + // Enforcement (read-filter middleware + sharing-rule hooks) is opt-out // via `enforce: false`. The share-link service below is registered // REGARDLESS — capability-token sharing does not depend on principal- @@ -632,6 +659,32 @@ export class SharingServicePlugin implements Plugin { ctx.logger.warn('SharingServicePlugin: access-level backfill (kernel:bootstrapped) failed', { error: err?.message }); } + // [#5103] Reclaim share rows whose RECORD no longer exists — the + // convergence path for orphans the cascade could not have caught: rows + // that predate it, a hook that failed, a process that died between the + // delete and the revoke, and deletes on the one posture the cascade + // deliberately skips (an unmarked system object). Runs BEFORE the + // rule-grant passes and outside the `ruleService` guard: this sweep is + // source-agnostic and must also run in the `enforce: false` posture, + // where there is no rule service at all. + // + // Bounded per boot (keyset pages + a scan cap that reports itself) so a + // table that only grows cannot make startup cost grow with it. + try { + if (this.service) { + const swept = await this.service.sweepOrphanedRecordShares(); + if (swept.truncated) { + ctx.logger.info( + 'SharingServicePlugin: orphaned share sweep hit its per-boot scan cap — the remaining ' + + 'rows are examined on the next boot', + { scanned: swept.scanned, revoked: swept.revoked }, + ); + } + } + } catch (err: any) { + ctx.logger.warn('SharingServicePlugin: orphaned record-share sweep (kernel:bootstrapped) failed', { error: err?.message }); + } + if (!this.ruleService) return; try { // [#4433] EVERY rule, not `activeOnly` — a deactivated rule's grants diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 6ff9b29e9d..5a0b84a140 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -8,6 +8,7 @@ import type { SharingExecutionContext, ShareAccessLevel, } from '@objectstack/spec/contracts'; +import { keysetWalk } from '@objectstack/types'; import { WRITE_ACCESS_LEVELS, normalizeAccessLevel } from './access-level.js'; /** @@ -66,7 +67,7 @@ const OWNER_FIELD = 'owner_id'; * parse at authoring. A stored value this function does not recognise * fails CLOSED to `private` (never silently public). */ -function effectiveSharingModel(schema: any): 'private' | 'read' | 'public' { +export function effectiveSharingModel(schema: any): 'private' | 'read' | 'public' { const m = schema?.sharingModel ?? schema?.security?.sharingModel; if (m === 'private') return 'private'; if (m === 'public_read') return 'read'; @@ -102,6 +103,54 @@ export interface SharingSecurityProbe { ): Promise<'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org'>; } +/** + * [#5103] Ids per `$in` on the record-delete cascade's revoke. Mirrors the + * chunk `SharingRuleService.revokeRuleGrantsForRecords` already uses: a single + * statement binding a thousand parameters is a portability trap (SQLite's + * default `SQLITE_MAX_VARIABLE_NUMBER` is 999 on older builds), and one number + * for both revoke paths keeps them from drifting. + */ +const RECORD_SHARE_REVOKE_CHUNK = 200; + +/** [#5103] Share rows read per page by the orphan sweep. */ +const ORPHAN_SWEEP_PAGE_SIZE = 500; + +/** + * [#5103] Share rows one sweep will scan before stopping and reporting + * truncation. The sweep runs on every boot, so it must cost a bounded amount + * on a table that only grows; the next boot resumes from the start and the + * rows it did not reach stay reachable by the object-scoped sweep. A cap is + * not a failure — but an unreported cap turns a partial scan into a false + * "nothing to clean", which is why {@link OrphanShareSweepResult} carries it. + */ +const ORPHAN_SWEEP_MAX_ROWS = 50_000; + +/** [#5103] Options for {@link SharingService.sweepOrphanedRecordShares}. */ +export interface OrphanShareSweepOptions { + /** Restrict the sweep to one object. Default: every object with share rows. */ + object?: string; + /** Share rows per page. Default {@link ORPHAN_SWEEP_PAGE_SIZE}. */ + batchSize?: number; + /** Stop after scanning this many rows. Default {@link ORPHAN_SWEEP_MAX_ROWS}. */ + max?: number; +} + +/** [#5103] What one {@link SharingService.sweepOrphanedRecordShares} pass did. */ +export interface OrphanShareSweepResult { + /** Share rows examined. */ + scanned: number; + /** Share rows revoked because their record no longer exists. */ + revoked: number; + /** + * Objects whose existence probe could not be run (unregistered object, + * driver error). Their rows were LEFT ALONE — "could not ask" is not + * "the record is gone", and only the second one may delete anything. + */ + unresolvedObjects: string[]; + /** True when {@link OrphanShareSweepOptions.max} stopped the scan early. */ + truncated: boolean; +} + export interface SharingServiceOptions { engine: SharingEngine; /** Object names that bypass sharing — typically platform internals. */ @@ -118,6 +167,8 @@ export interface SharingServiceOptions { * null → management authority fails CLOSED to owner-only. */ securityService?: () => SharingSecurityProbe | null | undefined; + /** [#5103] Optional logger for the record-delete cascade / orphan sweep. */ + logger?: { info?: Function; warn?: Function; error?: Function; debug?: Function }; } /** @@ -133,11 +184,13 @@ export class SharingService implements ISharingService { private readonly bypassObjects: Set; private readonly hierarchyResolver?: () => IHierarchyScopeResolver | null | undefined; private readonly securityService?: () => SharingSecurityProbe | null | undefined; + private readonly logger?: SharingServiceOptions['logger']; constructor(options: SharingServiceOptions) { this.engine = options.engine; this.hierarchyResolver = options.hierarchyResolver; this.securityService = options.securityService; + this.logger = options.logger; this.bypassObjects = new Set([ 'sys_record_share', 'sys_user', @@ -739,8 +792,205 @@ export class SharingService implements ISharingService { return Array.isArray(rows) ? (rows as RecordShare[]) : []; } + /** + * [#5103] Revoke EVERY `sys_record_share` row belonging to records that have + * just been deleted — regardless of `source`. + * + * This is the one place manual shares are swept, and the justification is + * exactly what makes it safe: the record is GONE. A share says "principal P + * has level L on (object O, record R)"; with R deleted the row cannot + * describe any access a human decided to give, so keeping it is not respect + * for the admin's decision, it is a dangling reference. Contrast rule + * RECOMPUTE, which must never touch a manual row (#5102 pinned that, and it + * stays pinned): there the record still exists and the human's decision is + * still about something. + * + * Why the orphan matters even though the record is gone: `buildReadFilter` + * emits `id IN ()`, which matches nothing today ONLY + * because record ids are never reused — an assumption no gate enforces. A + * new record landing on a recycled id would inherit the dead record's + * recipients outright (#5103). The rows are also unbounded growth and show + * up in Setup's Record Shares list pointing at nothing. + * + * Set-based and chunked, so its cost tracks the number of ids, not the + * number of share rows. Returns nothing: counting would need a read the hot + * delete path should not pay for, and callers that need a count (tests, the + * sweep) can read the table. + */ + async revokeSharesForDeletedRecords( + object: string, + recordIds: readonly string[], + ): Promise { + if (!object || recordIds.length === 0) return; + for (let i = 0; i < recordIds.length; i += RECORD_SHARE_REVOKE_CHUNK) { + const batch = recordIds.slice(i, i + RECORD_SHARE_REVOKE_CHUNK); + await this.engine.delete('sys_record_share', { + where: { object_name: object, record_id: { $in: batch } }, + multi: true, + context: SYSTEM_CTX, + } as any); + } + } + + /** + * [#5103] Revoke every share row whose RECORD no longer exists. + * + * The convergence half of the record-delete cascade, and the shape + * `SharingRuleService.sweepOrphanedRuleGrants` (#4433) established — with a + * different predicate, which is the whole point: that sweep asks "does the + * RULE row still exist", so it can never see a manual share, nor a rule + * grant whose rule is alive and whose record is not. This one asks "does the + * RECORD still exist", which is the question the invariant is actually made + * of, and it is source-agnostic. + * + * Two callers, one primitive: + * - `kernel:bootstrapped`, unscoped — historical orphans from before the + * cascade existed, plus anything a crashed hook missed, converge on the + * next boot; + * - the cascade's unbounded-delete branch, scoped to one object — a bulk + * delete whose row set could not be enumerated cannot name the ids to + * revoke, but the sweep does not need them: it reads the shares and asks + * about each record. This is deliberately NOT the rule path's + * "revoke everything on the object and re-grant asynchronously" — that + * trade is only available where a reconcile can put the grants back, and + * nothing can re-create a manual share. + * + * Bounded on both axes: rows are read by keyset page (never `OFFSET`, which + * skips rows in a walk that deletes as it goes — #4363), the scan stops at + * `max` and SAYS so, and existence is probed one batched `id IN (…)` per + * object per page rather than one query per share row. + * + * Fails SAFE per object: a probe that throws leaves that object's rows + * untouched and is reported in `unresolvedObjects`. "Nothing was queried" is + * not "nothing matched" — deleting on a failed probe would turn a transient + * driver error into permanent access loss. + */ + async sweepOrphanedRecordShares( + options?: OrphanShareSweepOptions, + ): Promise { + const result: OrphanShareSweepResult = { + scanned: 0, + revoked: 0, + unresolvedObjects: [], + truncated: false, + }; + const unresolved = new Set(); + const walk = keysetWalk( + (q) => this.engine.find('sys_record_share', { + ...q, + fields: ['id', 'object_name', 'record_id'], + context: SYSTEM_CTX, + }), + { + where: options?.object ? { object_name: options.object } : undefined, + pageSize: Math.max(1, options?.batchSize ?? ORPHAN_SWEEP_PAGE_SIZE), + max: options?.max ?? ORPHAN_SWEEP_MAX_ROWS, + }, + ); + + try { + for await (const page of walk.pages()) { + result.scanned += page.length; + + // Group the page by object so existence is one probe per object, not + // one per row. + const byObject = new Map>(); + for (const row of page) { + const objectName = row?.object_name == null ? '' : String(row.object_name); + const recordId = row?.record_id == null ? '' : String(row.record_id); + const shareId = row?.id == null ? '' : String(row.id); + if (!objectName || !recordId || !shareId) continue; + const perRecord = byObject.get(objectName) ?? new Map(); + const shareIds = perRecord.get(recordId) ?? []; + shareIds.push(shareId); + perRecord.set(recordId, shareIds); + byObject.set(objectName, perRecord); + } + + for (const [objectName, perRecord] of byObject) { + if (unresolved.has(objectName)) continue; + const recordIds = [...perRecord.keys()]; + let live: Set; + try { + live = await this.findLiveRecordIds(objectName, recordIds); + } catch (err: any) { + unresolved.add(objectName); + this.logger?.warn?.( + '[sharing] orphan share sweep could not check whether records still exist — ' + + 'its share rows were left in place (they are re-checked on the next sweep)', + { object: objectName, error: err?.message }, + ); + continue; + } + const orphanShareIds: string[] = []; + for (const [recordId, shareIds] of perRecord) { + if (live.has(recordId)) continue; + orphanShareIds.push(...shareIds); + } + if (orphanShareIds.length === 0) continue; + await this.deleteSharesByIds(orphanShareIds); + result.revoked += orphanShareIds.length; + } + } + } catch (err: any) { + this.logger?.warn?.( + '[sharing] orphan share sweep stopped early — remaining rows are re-checked on the next sweep', + { object: options?.object, error: err?.message, scanned: result.scanned }, + ); + result.truncated = true; + } + + result.unresolvedObjects = [...unresolved]; + result.truncated = result.truncated || walk.truncated; + if (result.revoked > 0) { + this.logger?.warn?.( + '[sharing] revoked share rows whose record no longer exists (#5103)', + { shares: result.revoked, scanned: result.scanned, object: options?.object }, + ); + } + return result; + } + // ── helpers ────────────────────────────────────────────────────── + /** + * [#5103] Which of `recordIds` still exist on `object`. Batched by + * {@link RECORD_SHARE_REVOKE_CHUNK} so the `$in` never outgrows a driver's + * bind-parameter limit. Throws on a query failure — the caller MUST treat + * that as "unknown", never as "none of them exist". + */ + private async findLiveRecordIds( + object: string, + recordIds: readonly string[], + ): Promise> { + const live = new Set(); + for (let i = 0; i < recordIds.length; i += RECORD_SHARE_REVOKE_CHUNK) { + const batch = recordIds.slice(i, i + RECORD_SHARE_REVOKE_CHUNK); + const rows = await this.engine.find(object, { + where: { id: { $in: batch } }, + fields: ['id'], + limit: batch.length, + context: SYSTEM_CTX, + }); + for (const row of (rows ?? [])) { + if ((row as any)?.id != null) live.add(String((row as any).id)); + } + } + return live; + } + + /** [#5103] Set-based delete of share rows by id, chunked like the revoke. */ + private async deleteSharesByIds(shareIds: readonly string[]): Promise { + for (let i = 0; i < shareIds.length; i += RECORD_SHARE_REVOKE_CHUNK) { + const batch = shareIds.slice(i, i + RECORD_SHARE_REVOKE_CHUNK); + await this.engine.delete('sys_record_share', { + where: { id: { $in: batch } }, + multi: true, + context: SYSTEM_CTX, + } as any); + } + } + /** * [ADR-0057] Resolve the owner-id set for a DEPTH scope. `own`/unset/`org` * resolve locally to the caller. HIERARCHY scopes (`unit` / `unit_and_below` From 5cc6183cf7f0055c9e4219cb96786c9be33f7b77 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 08:32:13 +0000 Subject: [PATCH 2/3] fix(sharing): the deferred-sweep breadcrumb is info, not warn (#5103) The rule path warns on its unbounded branch because recipients visibly lose access to records they still qualify for until the re-grant lands. Nothing equivalent happens here: the sweep only removes rows whose record is gone, so a deferred reclaim takes nothing from a surviving record and has no user-visible consequence. A warn on every predicate delete would only erode the level (AGENTS.md's own caution against over-applying it). The sweep still warns when it actually revokes rows. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015W6nhsDrz6zWQc8je12a1t --- .../plugins/plugin-sharing/src/record-share-cascade.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-sharing/src/record-share-cascade.ts b/packages/plugins/plugin-sharing/src/record-share-cascade.ts index 6ce28524d3..7662f3bf83 100644 --- a/packages/plugins/plugin-sharing/src/record-share-cascade.ts +++ b/packages/plugins/plugin-sharing/src/record-share-cascade.ts @@ -222,7 +222,14 @@ export function bindRecordShareCascade( // Queued, because the walk's cost is unrelated to this write's and a hook // must not hold the caller. Under-cleaning for a moment is the safe // direction; over-deleting a manual share would be unrecoverable. - logger?.warn?.( + // `info`, not `warn`: unlike the rule path's unbounded branch — which + // revokes grants that records still deserve and warns because recipients + // visibly lose access until the re-grant lands — nothing here is taken + // from a surviving record. The sweep only removes rows whose record is + // gone, so a deferred reclaim has no user-visible consequence to warn + // about, and a `warn` on every bulk delete would just erode the level. + // The sweep speaks up (at `warn`) if it actually revokes anything. + logger?.info?.( '[sharing] a bulk delete touched more rows than could be enumerated — the shares of the ' + 'deleted records are being reclaimed by a background orphan sweep instead ' + '(a restart re-runs the same sweep)', From 5863f032248cf759b544ce06e12c773451073a4c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 08:33:58 +0000 Subject: [PATCH 3/3] docs(sharing): record why referential cascades are already covered (#5103) `cascadeDeleteRelations` removes a `deleteBehavior: 'cascade'` child through the public `delete()` rather than the driver, so a detail record swept away with its master reaches this hook like any other delete. That is the fact behind treating `controlled_by_parent` as sharing-capable: manual grants are refused there, but the rule evaluator can still materialise rows under system context, and this is the path that reclaims them. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015W6nhsDrz6zWQc8je12a1t --- .../plugin-sharing/src/record-share-cascade.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/plugins/plugin-sharing/src/record-share-cascade.ts b/packages/plugins/plugin-sharing/src/record-share-cascade.ts index 7662f3bf83..4d67af242f 100644 --- a/packages/plugins/plugin-sharing/src/record-share-cascade.ts +++ b/packages/plugins/plugin-sharing/src/record-share-cascade.ts @@ -68,6 +68,17 @@ * The `kernel:bootstrapped` sweep (unscoped) is the durable backstop for both: * a hook that failed, a process that died mid-cascade, and every orphan that * predates this code converge on the next boot. + * + * ## Referential cascades are covered + * + * `ObjectQLEngine.cascadeDeleteRelations` removes a `deleteBehavior: 'cascade'` + * child by recursing through the PUBLIC `delete()` ("so the child's own + * cascade, hooks and events fire"), not by calling the driver directly — so a + * detail record swept away with its master reaches this hook like any other + * delete. That is why `controlled_by_parent` counts as sharing-capable in + * {@link objectCanCarryRecordShares} despite refusing manual grants: rows can + * exist there (the rule evaluator grants under system context), and this is + * the path that reclaims them. */ import {