From bb16143dc8d8687ea971bd4052d4ec27832ee026 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 11:22:39 +0000 Subject: [PATCH] fix(security): exempt engine referential FK clears from the owner_id transfer guard (#3023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit owner_id is a lookup to sys_user with default deleteBehavior 'set_null', so deleting a sys_user makes cascadeDeleteRelations null owner_id on every dependent. That cascade write re-entered the write middleware under the deleter's context, where the #3004 ownership-anchor guard read owner_id=null as a user disown and denied it — aborting the cascade mid-way (no transaction → partial state) for any deleter lacking the transfer grant on the child (e.g. a member clearing a public_read_write child that RLS would otherwise allow). The cascade FK clear is engine-mandated referential integrity consequent to an already-authorized parent delete, not a user ownership change. cascadeDeleteRelations now tags the set_null write with a server-derived __referentialFieldClear context marker (set by the engine, never built from a request — same trust model as __expandRead), and the ownership-anchor guard skips when it is present. Ordinary user writes are unaffected; the marker can't be forged from client input, so it can never slip a real ownership transfer past the guard. Tests: engine-cascade-delete proves the set_null write carries the marker (and a user update does not); plugin-security proves the guard honors it (a member's owner_id=null with the marker is allowed, denied without). objectql 899 / plugin-security 462 / related dogfood green. --- ...ner-guard-referential-cascade-exemption.md | 23 +++++++++++++ .../src/engine-cascade-delete.test.ts | 34 +++++++++++++++++++ packages/objectql/src/engine.ts | 11 +++++- .../src/security-plugin.test.ts | 13 +++++++ .../plugin-security/src/security-plugin.ts | 12 ++++++- 5 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 .changeset/owner-guard-referential-cascade-exemption.md diff --git a/.changeset/owner-guard-referential-cascade-exemption.md b/.changeset/owner-guard-referential-cascade-exemption.md new file mode 100644 index 0000000000..e02aae1c90 --- /dev/null +++ b/.changeset/owner-guard-referential-cascade-exemption.md @@ -0,0 +1,23 @@ +--- +'@objectstack/plugin-security': patch +'@objectstack/objectql': patch +--- + +fix(security): exempt engine referential FK clears from the owner_id transfer guard (#3023) + +Follow-up to the #3004 ownership-anchor guard. `owner_id` is a lookup to `sys_user` +with the default `deleteBehavior: 'set_null'`, so deleting a `sys_user` makes +`cascadeDeleteRelations` null `owner_id` on every dependent row. That cascade write +re-entered the write middleware under the deleter's context, where the #3004 guard +read the `owner_id = null` as a user-initiated disown and denied it — aborting the +cascade mid-way (no transaction, so partial state) for any deleter without the +transfer grant on the child object (e.g. a member clearing a `public_read_write` +child that RLS would otherwise have allowed). + +The cascade FK clear is engine-mandated referential integrity consequent to an +already-authorized parent delete, not a user ownership change. `cascadeDeleteRelations` +now tags the `set_null` write with a server-derived `__referentialFieldClear` context +marker (set by the engine, never built from a request — same trust model as +`__expandRead`), and the ownership-anchor guard skips when that marker is present. +Ordinary user writes are unaffected; the marker cannot be forged from client input, +so it can never slip a real ownership transfer past the guard. diff --git a/packages/objectql/src/engine-cascade-delete.test.ts b/packages/objectql/src/engine-cascade-delete.test.ts index 2699651704..62fe7f2b8a 100644 --- a/packages/objectql/src/engine-cascade-delete.test.ts +++ b/packages/objectql/src/engine-cascade-delete.test.ts @@ -138,4 +138,38 @@ describe('cascadeDeleteRelations — required FK escalates set_null → restrict expect(await engine.findOne('acct', { where: { id: a.id } })).toBeNull(); expect(await engine.findOne('task', { where: { id: t.id } })).toBeNull(); }); + + it('[#3023] tags the referential set_null write with __referentialFieldClear so the owner guard can exempt it', async () => { + // The cascade FK clear is an engine-internal integrity write. It must + // carry the server-set marker plugin-security's ownership-anchor guard + // keys off — otherwise nulling an owner_id-style FK would trip the + // #3004 transfer guard and abort the cascade. A user-driven update must + // NOT carry the marker (control). + const seen: Array<{ op: string; marker: unknown; where: unknown }> = []; + engine.registerMiddleware(async (opCtx: any, next: () => Promise) => { + if (opCtx.operation === 'update') { + seen.push({ + op: opCtx.operation, + marker: opCtx.context?.__referentialFieldClear, + where: opCtx.data, + }); + } + await next(); + }); + + const a = await engine.insert('acct', { name: 'Acme' }); + const n = await engine.insert('note', { body: 'hi', account: a.id }); + + // A normal user update — no marker. + await engine.update('note', { id: n.id, body: 'edited' }, { context: { userId: 'u1' } } as any); + // The cascade set_null when the parent is deleted — marked. + await engine.delete('acct', { where: { id: a.id }, context: { userId: 'u1' } } as any); + + const userUpdate = seen.find((s) => (s.where as any)?.body === 'edited'); + const cascadeClear = seen.find((s) => (s.where as any)?.account === null); + expect(userUpdate?.marker, 'user update carries no referential marker').toBeUndefined(); + expect(cascadeClear?.marker, 'cascade FK clear carries the marker').toBe(true); + // And the cascade actually nulled the FK. + expect((await engine.findOne('note', { where: { id: n.id } }) as any).account).toBeNull(); + }); }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 474a970663..42e155af62 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -2631,7 +2631,16 @@ export class ObjectQL implements IDataEngine { // hooks and events fire. await this.delete(childName, { where: { id: depId }, context } as any); } else { - await this.update(childName, { id: depId, [fieldName]: null }, { context } as any); + // [#3023] Clear the FK as an engine-internal referential-integrity + // write, tagged so plugin-security's ownership-anchor guard (#3004) + // treats an `owner_id = null` cascade as integrity maintenance, not + // a user-initiated disown — otherwise deleting the referenced record + // trips the transfer guard and aborts the cascade mid-way. Marker + // rides a server-DERIVED context (set here, never from client input + // — same trust model as `__expandRead`), so it cannot be forged from + // a request to bypass the guard on an ordinary write. + const referentialCtx = { ...(context ?? {}), __referentialFieldClear: true } as ExecutionContext; + await this.update(childName, { id: depId, [fieldName]: null }, { context: referentialCtx } as any); } } } diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 8faa2946a0..597e324997 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -369,6 +369,19 @@ describe('SecurityPlugin', () => { const opCtx: any = { object: 'task', operation: 'update', data, context: memberCtx() }; await harness.run(opCtx); // must not throw — owner_id is not an own property }); + + it('[#3023] an engine referential FK clear (__referentialFieldClear) is exempt — owner_id:null cascade is allowed for a plain member', async () => { + // cascadeDeleteRelations nulls owner_id on dependents when the referenced + // sys_user is deleted; that integrity write must not be blocked by the + // disown guard. The same payload WITHOUT the marker is denied (covered + // by the disown test above). + const harness = await boot([memberSet], () => ({ id: 't1', owner_id: 'someone' })); + const opCtx: any = { + object: 'task', operation: 'update', data: { id: 't1', owner_id: null }, + context: memberCtx({ __referentialFieldClear: true }), + }; + await harness.run(opCtx); // must not throw — engine-internal referential clear + }); }); it('without org-scoping plugin — strips tenant_isolation RLS so find applies no tenant where', async () => { diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index d886a394e1..75244b8d04 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -1099,10 +1099,20 @@ export class SecurityPlugin implements Plugin { // // `organization_id` auto-injection has moved to // `@objectstack/organizations`; its forge guard is step 3.7 below. + // + // [#3023] EXEMPTION — the engine's referential-integrity FK clear. When a + // referenced record is deleted, `cascadeDeleteRelations` nulls the FK on + // every dependent (owner_id included). That `owner_id = null` is an + // integrity-mandated consequence of an already-authorized parent delete, + // NOT a user disown, so the transfer guard must not fire and abort the + // cascade. The marker rides a server-DERIVED context (never client-built — + // same trust model as `__expandRead`), so it cannot be forged from a + // request to slip an ordinary ownership write past the guard. if ( (opCtx.operation === 'insert' || opCtx.operation === 'update') && opCtx.data && - typeof opCtx.data === 'object' + typeof opCtx.data === 'object' && + !opCtx.context?.__referentialFieldClear ) { const isInsert = opCtx.operation === 'insert'; const rows = (Array.isArray(opCtx.data) ? opCtx.data : [opCtx.data]) as Record[];