Skip to content

Commit 6613ad0

Browse files
os-zhuangclaude
andauthored
fix(security): exempt engine referential FK clears from the owner_id transfer guard (#3023) (#3048)
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. The cascade FK clear is engine-mandated referential integrity, 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. The marker can't be forged from client input, so it can never slip a real ownership transfer past the guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5286745 commit 6613ad0

5 files changed

Lines changed: 91 additions & 2 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
'@objectstack/objectql': patch
4+
---
5+
6+
fix(security): exempt engine referential FK clears from the owner_id transfer guard (#3023)
7+
8+
Follow-up to the #3004 ownership-anchor guard. `owner_id` is a lookup to `sys_user`
9+
with the default `deleteBehavior: 'set_null'`, so deleting a `sys_user` makes
10+
`cascadeDeleteRelations` null `owner_id` on every dependent row. That cascade write
11+
re-entered the write middleware under the deleter's context, where the #3004 guard
12+
read the `owner_id = null` as a user-initiated disown and denied it — aborting the
13+
cascade mid-way (no transaction, so partial state) for any deleter without the
14+
transfer grant on the child object (e.g. a member clearing a `public_read_write`
15+
child that RLS would otherwise have allowed).
16+
17+
The cascade FK clear is engine-mandated referential integrity consequent to an
18+
already-authorized parent delete, not a user ownership change. `cascadeDeleteRelations`
19+
now tags the `set_null` write with a server-derived `__referentialFieldClear` context
20+
marker (set by the engine, never built from a request — same trust model as
21+
`__expandRead`), and the ownership-anchor guard skips when that marker is present.
22+
Ordinary user writes are unaffected; the marker cannot be forged from client input,
23+
so it can never slip a real ownership transfer past the guard.

packages/objectql/src/engine-cascade-delete.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,4 +138,38 @@ describe('cascadeDeleteRelations — required FK escalates set_null → restrict
138138
expect(await engine.findOne('acct', { where: { id: a.id } })).toBeNull();
139139
expect(await engine.findOne('task', { where: { id: t.id } })).toBeNull();
140140
});
141+
142+
it('[#3023] tags the referential set_null write with __referentialFieldClear so the owner guard can exempt it', async () => {
143+
// The cascade FK clear is an engine-internal integrity write. It must
144+
// carry the server-set marker plugin-security's ownership-anchor guard
145+
// keys off — otherwise nulling an owner_id-style FK would trip the
146+
// #3004 transfer guard and abort the cascade. A user-driven update must
147+
// NOT carry the marker (control).
148+
const seen: Array<{ op: string; marker: unknown; where: unknown }> = [];
149+
engine.registerMiddleware(async (opCtx: any, next: () => Promise<void>) => {
150+
if (opCtx.operation === 'update') {
151+
seen.push({
152+
op: opCtx.operation,
153+
marker: opCtx.context?.__referentialFieldClear,
154+
where: opCtx.data,
155+
});
156+
}
157+
await next();
158+
});
159+
160+
const a = await engine.insert('acct', { name: 'Acme' });
161+
const n = await engine.insert('note', { body: 'hi', account: a.id });
162+
163+
// A normal user update — no marker.
164+
await engine.update('note', { id: n.id, body: 'edited' }, { context: { userId: 'u1' } } as any);
165+
// The cascade set_null when the parent is deleted — marked.
166+
await engine.delete('acct', { where: { id: a.id }, context: { userId: 'u1' } } as any);
167+
168+
const userUpdate = seen.find((s) => (s.where as any)?.body === 'edited');
169+
const cascadeClear = seen.find((s) => (s.where as any)?.account === null);
170+
expect(userUpdate?.marker, 'user update carries no referential marker').toBeUndefined();
171+
expect(cascadeClear?.marker, 'cascade FK clear carries the marker').toBe(true);
172+
// And the cascade actually nulled the FK.
173+
expect((await engine.findOne('note', { where: { id: n.id } }) as any).account).toBeNull();
174+
});
141175
});

packages/objectql/src/engine.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2631,7 +2631,16 @@ export class ObjectQL implements IDataEngine {
26312631
// hooks and events fire.
26322632
await this.delete(childName, { where: { id: depId }, context } as any);
26332633
} else {
2634-
await this.update(childName, { id: depId, [fieldName]: null }, { context } as any);
2634+
// [#3023] Clear the FK as an engine-internal referential-integrity
2635+
// write, tagged so plugin-security's ownership-anchor guard (#3004)
2636+
// treats an `owner_id = null` cascade as integrity maintenance, not
2637+
// a user-initiated disown — otherwise deleting the referenced record
2638+
// trips the transfer guard and aborts the cascade mid-way. Marker
2639+
// rides a server-DERIVED context (set here, never from client input
2640+
// — same trust model as `__expandRead`), so it cannot be forged from
2641+
// a request to bypass the guard on an ordinary write.
2642+
const referentialCtx = { ...(context ?? {}), __referentialFieldClear: true } as ExecutionContext;
2643+
await this.update(childName, { id: depId, [fieldName]: null }, { context: referentialCtx } as any);
26352644
}
26362645
}
26372646
}

packages/plugins/plugin-security/src/security-plugin.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,19 @@ describe('SecurityPlugin', () => {
369369
const opCtx: any = { object: 'task', operation: 'update', data, context: memberCtx() };
370370
await harness.run(opCtx); // must not throw — owner_id is not an own property
371371
});
372+
373+
it('[#3023] an engine referential FK clear (__referentialFieldClear) is exempt — owner_id:null cascade is allowed for a plain member', async () => {
374+
// cascadeDeleteRelations nulls owner_id on dependents when the referenced
375+
// sys_user is deleted; that integrity write must not be blocked by the
376+
// disown guard. The same payload WITHOUT the marker is denied (covered
377+
// by the disown test above).
378+
const harness = await boot([memberSet], () => ({ id: 't1', owner_id: 'someone' }));
379+
const opCtx: any = {
380+
object: 'task', operation: 'update', data: { id: 't1', owner_id: null },
381+
context: memberCtx({ __referentialFieldClear: true }),
382+
};
383+
await harness.run(opCtx); // must not throw — engine-internal referential clear
384+
});
372385
});
373386

374387
it('without org-scoping plugin — strips tenant_isolation RLS so find applies no tenant where', async () => {

packages/plugins/plugin-security/src/security-plugin.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1099,10 +1099,20 @@ export class SecurityPlugin implements Plugin {
10991099
//
11001100
// `organization_id` auto-injection has moved to
11011101
// `@objectstack/organizations`; its forge guard is step 3.7 below.
1102+
//
1103+
// [#3023] EXEMPTION — the engine's referential-integrity FK clear. When a
1104+
// referenced record is deleted, `cascadeDeleteRelations` nulls the FK on
1105+
// every dependent (owner_id included). That `owner_id = null` is an
1106+
// integrity-mandated consequence of an already-authorized parent delete,
1107+
// NOT a user disown, so the transfer guard must not fire and abort the
1108+
// cascade. The marker rides a server-DERIVED context (never client-built —
1109+
// same trust model as `__expandRead`), so it cannot be forged from a
1110+
// request to slip an ordinary ownership write past the guard.
11021111
if (
11031112
(opCtx.operation === 'insert' || opCtx.operation === 'update') &&
11041113
opCtx.data &&
1105-
typeof opCtx.data === 'object'
1114+
typeof opCtx.data === 'object' &&
1115+
!opCtx.context?.__referentialFieldClear
11061116
) {
11071117
const isInsert = opCtx.operation === 'insert';
11081118
const rows = (Array.isArray(opCtx.data) ? opCtx.data : [opCtx.data]) as Record<string, unknown>[];

0 commit comments

Comments
 (0)