Skip to content

Commit f4e741b

Browse files
claude[bot]claude
andauthored
security(engine): exempt the referential FK-clear write from the object-level CRUD check (#12941)
* feat(security): marker-scoped CRUD exemption for the referential FK clear The engine's `set_null` reference cleanup already tags its UPDATE with the server-derived `__referentialFieldClear` marker (#3023). The marker reached only the ownership-anchor guard, so the cleanup write still had to pass the object-level CRUD check on the referencing object: a role holding full delete rights on A and no grant at all on B could delete an A only while B was empty. plugin-security now exempts a marked UPDATE from the object-level CRUD grant check, and from that check alone. Field-level security on the FK column, the RLS `using` row scope and the RLS post-image `check` all stay enforced -- each pinned -- and the `cascade` arm keeps the operator's own delete authority, untouched. The write is not elevated: it stays the operator's identity, so audit attribution (`user_id`/`actor`, `updated_by`) is unchanged. Ruled by the maintainer on 2026-08-28 (#12597, second round, option B). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry * test(security): ablation prediction for the #12597 CRUD exemption Committed BEFORE the mutation so the prediction cannot be written after the reading. Reverting the exemption (restoring the bare `if (permissionSets.length > 0)` at the object-level CRUD check) is predicted to turn SIX of the fourteen tests in `delete-reference-cleanup-system-identity.test.ts` RED: 1. `[#12597 - PIN INVERTED] a non-empty referencing table no longer needs ...` 2. `THE CONTRACT: full delete on A + NOTHING on B + a non-empty ...` 3. `GUARD 1 - field-level security on the FK column still refuses ...` 4. `GUARD 2 - the RLS \`using\` row scope ... still refuses ...` 5. `GUARD 2 CONTROL - the same policy admits the write when the row IS in scope` 6. `GUARD 3 - the RLS post-image \`check\` still refuses ...` Guards 1-3 are predicted red DESPITE still refusing, because each asserts its own gate's ADR-0112 envelope (`forbiddenFields`, `recordId` + "row-level security", "row-level CHECK") and the reverted CRUD check answers first with none of them. That is the whole reason those pins are written on envelopes rather than on the bare fact of a refusal. The other two new arms are predicted GREEN either way: the converse (no delete right on the target) and the ordinary unmarked update, neither of which the exemption is in a position to change. Note on the ablation's mechanics: the mutated file is `plugin-security/src/security-plugin.ts`, which this suite imports RELATIVELY (`./security-plugin.js`), so vitest reads the SOURCE in this checkout and no rebuild leg applies to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry * chore(changeset): minor bump for the referential FK-clear CRUD exemption MINOR by the maintainer's 2026-08-28 ruling: the change moves which deletes succeed, an observable behavioural contract change on the delete path rather than a patch-grade defect repair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry * docs(security): point the exemption comment at the suite that exists The comment cited `delete-reference-cleanup-crud-exemption.test.ts`, a filename that resolves to nothing: the pins live in this package's `delete-reference-cleanup-system-identity.test.ts`, in its `#12597` describe. Caught in review. A citation that resolves to nothing is the defect class the ADR-anchor gate exists to name, and it is worse in a comment explaining a permission boundary, where the next reader is looking for the evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c94be62 commit f4e741b

3 files changed

Lines changed: 378 additions & 16 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
---
4+
5+
feat(security): the referential FK-clear write is exempt from the object-level CRUD check (#12597)
6+
7+
**This changes which deletes succeed** — an observable behavioural contract
8+
change on the delete path, which is why it ships `minor` rather than as a
9+
patch-grade defect repair.
10+
11+
Deleting a record makes the engine clear every optional lookup that points at it
12+
(`deleteBehavior: 'set_null'`). That cleanup `UPDATE` is engine-owned referential
13+
integrity, and it has carried the server-derived `__referentialFieldClear` marker
14+
since #3023 — but the marker reached only the ownership-anchor guard, so the
15+
write still had to pass the **object-level CRUD check** on the referencing
16+
object. Consequence, measured on a real deployment across 17 role×object pairs: a
17+
role with full delete rights on A and no grant at all on B could delete an A only
18+
while B was **empty**. The moment a real row referenced it, the delete failed with
19+
one generic "you do not have permission", and nothing on any permission screen
20+
showed that deleting A also required write authority on B.
21+
22+
**What is exempt: the object-level CRUD grant check, and nothing else.** A marked
23+
`update` skips that one gate (both the caller's grant and the ADR-0090 D10
24+
delegator half of the same question). Everything else in the security middleware
25+
runs unchanged and is pinned test-by-test:
26+
27+
- field-level security on the FK column still refuses;
28+
- the RLS `using` row scope on the referencing object still refuses;
29+
- the RLS post-image `check` still refuses — so a deployment declaring
30+
`product != null` keeps getting a truthful refusal instead of a silent clear;
31+
- declared validation rules keep firing (they were never in this path);
32+
- a caller without delete rights on the target is still refused;
33+
- an ordinary, unmarked update on the referencing object is untouched.
34+
35+
⛔ Deliberately **not** `isSystem`: that bypass is total (see
36+
`content/docs/permissions/system-context.mdx` — "Elevation is total, and it is not
37+
granular"), and it would have switched off all three guards above. ⛔ The
38+
`cascade` arm — deleting whole referencing rows — is **unchanged** and still
39+
requires the caller's own delete authority on those rows.
40+
41+
The write is not elevated at all, so audit attribution is unchanged: the cleanup
42+
`UPDATE` still runs under the operator's identity and lands in the ledger as that
43+
operator (`user_id` / `actor`, and the `updated_by` stamp).
44+
45+
No authorable surface changes, and no metadata needs migrating: a deployment that
46+
was working around this by granting write access on referencing tables can narrow
47+
those grants, but nothing forces it to.

packages/plugins/plugin-security/src/delete-reference-cleanup-system-identity.test.ts

Lines changed: 278 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,93 @@ const READER: PermissionSet = {
146146
},
147147
} as unknown as PermissionSet;
148148

149+
// ---------------------------------------------------------------------------
150+
// #12597 fixtures — one per guard that must SURVIVE the CRUD exemption.
151+
//
152+
// Each is `LINE_LEAD` plus exactly one guard, so a refusal can only come from
153+
// that guard: the object-level CRUD check is exempted for this write, and every
154+
// arm asserts the ADR-0112 envelope of the gate it names rather than the bare
155+
// fact of a refusal (a suite that only asked "was it refused?" would stay green
156+
// if the exemption regressed and the CRUD check answered instead).
157+
// ---------------------------------------------------------------------------
158+
159+
/**
160+
* Guard 1 — FIELD-LEVEL security on the FK column itself. Nothing else about
161+
* `LINE_LEAD` changes: the caller still holds no object grant on B, so the CRUD
162+
* check is exempted exactly as in the positive case and FLS is the only gate
163+
* left that can speak.
164+
*/
165+
const FLS_LOCKED_LEAD: PermissionSet = {
166+
name: 'ehr_fls_lead',
167+
label: 'Line Lead (FK column locked by FLS)',
168+
objects: {
169+
os_ehr_product: {
170+
allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true,
171+
modifyAllRecords: true, viewAllRecords: true,
172+
},
173+
},
174+
fields: {
175+
'os_ehr_andon_record.product': { readable: true, editable: false },
176+
},
177+
} as unknown as PermissionSet;
178+
179+
/**
180+
* Guards 2 and 3 — the RLS pair. Both add READ on B, deliberately: the row
181+
* gates re-read the target row as the caller, so without a read grant the
182+
* refusal would be the read denial wearing the row gate's clothes — a phantom
183+
* pin that passes for the wrong reason. With read granted, the caller still
184+
* holds no EDIT bit, so the exemption is still what carries the write past
185+
* step 2 and the row gate is the only thing left that can refuse.
186+
*/
187+
const RLS_USING_LEAD: PermissionSet = {
188+
name: 'ehr_rls_using_lead',
189+
label: 'Line Lead (RLS `using` row scope on B)',
190+
objects: {
191+
os_ehr_product: {
192+
allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true,
193+
modifyAllRecords: true, viewAllRecords: true,
194+
},
195+
os_ehr_andon_record: { allowRead: true, viewAllRecords: true },
196+
},
197+
rowLevelSecurity: [
198+
{
199+
name: 'andon_own_rows_only',
200+
object: 'os_ehr_andon_record',
201+
operation: 'update',
202+
using: 'owner_id == current_user.id',
203+
},
204+
],
205+
} as unknown as PermissionSet;
206+
207+
/**
208+
* Guard 3 — the RLS POST-IMAGE `check`. This is the sharpest of the three and
209+
* the reason the ruling narrowed: `check` is a data-SHAPE constraint, not a
210+
* reach question, so an FK-clear that empties `product` is precisely the write
211+
* a deployment declaring `product != null` means to forbid. Honouring that
212+
* intent when it is spelled as a `validations` entry and ignoring it when it is
213+
* spelled as an RLS `check` is the declared-not-enforced split this project
214+
* prices highest.
215+
*/
216+
const RLS_CHECK_LEAD: PermissionSet = {
217+
name: 'ehr_rls_check_lead',
218+
label: 'Line Lead (RLS post-image `check` on B)',
219+
objects: {
220+
os_ehr_product: {
221+
allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true,
222+
modifyAllRecords: true, viewAllRecords: true,
223+
},
224+
os_ehr_andon_record: { allowRead: true, viewAllRecords: true },
225+
},
226+
rowLevelSecurity: [
227+
{
228+
name: 'andon_product_always_set',
229+
object: 'os_ehr_andon_record',
230+
operation: 'update',
231+
check: 'product != null',
232+
},
233+
],
234+
} as unknown as PermissionSet;
235+
149236
function makeStubDriver() {
150237
const stores = new Map<string, Map<string, Record<string, unknown>>>();
151238
const storeFor = (o: string) => {
@@ -306,25 +393,34 @@ describe('#12166 — pre-delete reference check runs as SYSTEM (ruling A)', () =
306393
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(true);
307394
});
308395

309-
it('the elevation is the CHECK only — a non-empty referencing table still needs the caller\'s own write authority (constraint 1)', async () => {
310-
// Ruling constraint 1: "Nothing else about the delete path changes
311-
// identity." The `set_null` UPDATE below still runs as the caller, so a
312-
// caller with no grant on the referencing object is refused here — the
313-
// reference CHECK was relaxed, the caller's authority over the dependent
314-
// rows was not. Pinned so the boundary is visible rather than discovered:
315-
// a later edit that elevated the cleanup WRITES too would turn this green.
396+
it('[#12597 — PIN INVERTED] a non-empty referencing table no longer needs the caller\'s own write authority on it', async () => {
397+
// ⚠️ THIS PIN WAS INVERTED, DELIBERATELY. It previously asserted the
398+
// opposite — `PERMISSION_DENIED` with `details.operation === 'update'` —
399+
// as #12166 ruling constraint 1's boundary ("nothing else about the delete
400+
// path changes identity"), and that assertion is what MEASURED the residue
401+
// this card was opened for: a role with full delete on A and no grant on B
402+
// could delete an A only while B was empty.
403+
//
404+
// The maintainer ruled that residue away on 2026-08-28 (#12597, second
405+
// round, option B): the FK-clear UPDATE is exempted from the object-level
406+
// CRUD check, scoped by the `__referentialFieldClear` marker. So the case
407+
// below now SUCCEEDS by ruling, and the old expectation is falsified rather
408+
// than merely stale. ⛔ The `cascade` arm is untouched and still requires
409+
// the caller's own delete authority on the child rows.
410+
//
411+
// The exemption's fences — FLS, the RLS `using` row scope and the RLS
412+
// post-image `check` all still refusing — are pinned one describe below.
316413
const h = await boot();
317414
const p = await h.seed('os_ehr_product', { name: 'Widget' });
318-
await h.seed('os_ehr_andon_record', { product: p.id });
415+
const a = await h.seed('os_ehr_andon_record', { product: p.id });
319416

320417
const err = await h.deleteAs('os_ehr_product', p.id, h.caller());
321418

322-
expect(err).not.toBe(null);
323-
expect(err.code).toBe('PERMISSION_DENIED');
324-
// The refusal now names the WRITE it could not perform, not the read the
325-
// check used to fail on.
326-
expect(err.details?.object).toBe('os_ehr_andon_record');
327-
expect(err.details?.operation).toBe('update');
419+
expect(err).toBe(null);
420+
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(false);
421+
// set_null, not cascade: the referencing ROW survives with a cleared FK.
422+
expect(h.stores.get('os_ehr_andon_record')?.has(a.id)).toBe(true);
423+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product ?? null).toBe(null);
328424
});
329425
});
330426

@@ -425,3 +521,171 @@ describe('#12166 constraint 3 — the ledger records BOTH halves', () => {
425521
)).toBe(true);
426522
});
427523
});
524+
525+
// ---------------------------------------------------------------------------
526+
// #12597 — the FK-clear write is exempt from the object-level CRUD check, and
527+
// from THAT CHECK ALONE (maintainer ruling 2026-08-28, second round, option B).
528+
//
529+
// The round-1 measurement is why the exemption is marker-scoped rather than
530+
// `isSystem`: `security-plugin.ts`'s `isSystem` short-circuit is TOTAL (the
531+
// repo's own page says so — `content/docs/permissions/system-context.mdx`:
532+
// "Elevation is total, and it is not granular"), and it would have switched off
533+
// three guards that answer questions referential integrity does not ask. Those
534+
// three are the pins below; each names the gate's own envelope, so a refusal
535+
// migrating between gates reddens instead of reading as "still refused".
536+
// ---------------------------------------------------------------------------
537+
538+
describe('#12597 — the referential FK clear is exempt from the object-level CRUD check', () => {
539+
it('THE CONTRACT: full delete on A + NOTHING on B + a non-empty referencing table ⇒ the delete succeeds and the FK is cleared', async () => {
540+
const h = await boot();
541+
const p = await h.seed('os_ehr_product', { name: 'Widget' });
542+
// Owned by somebody else and referencing the target: the reporting
543+
// deployment's shape, and the case that used to 403 on the UPDATE.
544+
const a = await h.seed('os_ehr_andon_record', { product: p.id, owner_id: 'u_other' });
545+
expect(h.stores.get('os_ehr_andon_record')?.size).toBe(1);
546+
547+
// Attribution is asserted on the CONTEXT the cleanup write actually carries
548+
// — the gate-(a) reading this card carries forward. The write must stay the
549+
// OPERATOR's own identity, because every attribution channel keys on
550+
// `session.userId`: `writeAudit`'s `user_id`/`actor` and the `updated_by`
551+
// stamp both read it, and a bare `isSystem` context was measured producing
552+
// `user_id: null, actor: null`. This exemption never touches identity, so
553+
// the pin is that the context reaching the engine is the caller's own.
554+
const cleanupContexts: any[] = [];
555+
h.engine.registerMiddleware(async (opCtx: any, next: () => Promise<void>) => {
556+
if (opCtx.object === 'os_ehr_andon_record' && opCtx.operation === 'update') {
557+
cleanupContexts.push(opCtx.context);
558+
}
559+
return next();
560+
});
561+
562+
const err = await h.deleteAs('os_ehr_product', p.id, h.caller());
563+
564+
expect(err).toBe(null);
565+
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(false);
566+
// `set_null`, not `cascade` — the referencing row survives, minus the FK.
567+
expect(h.stores.get('os_ehr_andon_record')?.has(a.id)).toBe(true);
568+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product ?? null).toBe(null);
569+
570+
expect(cleanupContexts.length).toBe(1);
571+
expect(cleanupContexts[0]?.userId).toBe('u_lead');
572+
expect(cleanupContexts[0]?.isSystem).not.toBe(true);
573+
// …and it is the server-derived marker, not an identity switch, that the
574+
// exemption keys on (#3023; stamped in `cascadeDeleteRelations`).
575+
expect(cleanupContexts[0]?.__referentialFieldClear).toBe(true);
576+
});
577+
578+
it('GUARD 1 — field-level security on the FK column still refuses, and the FK is unchanged', async () => {
579+
const h = await boot([FLS_LOCKED_LEAD]);
580+
const p = await h.seed('os_ehr_product', { name: 'Widget' });
581+
const a = await h.seed('os_ehr_andon_record', { product: p.id, owner_id: 'u_other' });
582+
583+
const err = await h.deleteAs('os_ehr_product', p.id, h.caller());
584+
585+
expect(err).not.toBe(null);
586+
expect(err.code).toBe('PERMISSION_DENIED');
587+
expect(err.statusCode ?? err.status).toBe(403);
588+
// The FLS gate's own envelope — it names the field it refused, which is what
589+
// distinguishes it from the CRUD denial the exemption removed.
590+
expect(err.details?.object).toBe('os_ehr_andon_record');
591+
expect(err.details?.forbiddenFields).toContain('product');
592+
// Nothing moved: neither the FK nor the delete it was blocking.
593+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product).toBe(p.id);
594+
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(true);
595+
});
596+
597+
it('GUARD 2 — the RLS `using` row scope on the referencing object still refuses, and the FK is unchanged', async () => {
598+
const h = await boot([RLS_USING_LEAD]);
599+
const p = await h.seed('os_ehr_product', { name: 'Widget' });
600+
// Out of the caller's row scope: `owner_id == current_user.id` does not hold.
601+
const a = await h.seed('os_ehr_andon_record', { product: p.id, owner_id: 'u_other' });
602+
603+
const err = await h.deleteAs('os_ehr_product', p.id, h.caller());
604+
605+
expect(err).not.toBe(null);
606+
expect(err.code).toBe('PERMISSION_DENIED');
607+
expect(err.statusCode ?? err.status).toBe(403);
608+
// The row gate's own envelope: it names the ROW, and its developer sentence
609+
// says row-level security — the CRUD denial says neither.
610+
expect(err.details?.object).toBe('os_ehr_andon_record');
611+
expect(err.details?.recordId).toBe(a.id);
612+
expect(err.developerMessage).toContain('row-level security');
613+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product).toBe(p.id);
614+
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(true);
615+
});
616+
617+
it('GUARD 2 CONTROL — the same policy admits the write when the row IS in scope', async () => {
618+
// Without this arm, GUARD 2 is also satisfied by the exemption never firing
619+
// at all: "refused" would be indistinguishable from "the CRUD check answered
620+
// first". Here the ONLY thing that changes is the row's owner.
621+
const h = await boot([RLS_USING_LEAD]);
622+
const p = await h.seed('os_ehr_product', { name: 'Widget' });
623+
const a = await h.seed('os_ehr_andon_record', { product: p.id, owner_id: 'u_lead' });
624+
625+
const err = await h.deleteAs('os_ehr_product', p.id, h.caller());
626+
627+
expect(err).toBe(null);
628+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product ?? null).toBe(null);
629+
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(false);
630+
});
631+
632+
it('GUARD 3 — the RLS post-image `check` still refuses, and the FK is unchanged', async () => {
633+
// The `check` is `product != null`, i.e. the deployment declared that this
634+
// FK may never be emptied. Under a blanket `isSystem` elevation the clear
635+
// went through and the declaration was silently ignored; under the ruled
636+
// narrowing the deployment gets a truthful refusal.
637+
const h = await boot([RLS_CHECK_LEAD]);
638+
const p = await h.seed('os_ehr_product', { name: 'Widget' });
639+
const a = await h.seed('os_ehr_andon_record', { product: p.id, owner_id: 'u_other' });
640+
641+
const err = await h.deleteAs('os_ehr_product', p.id, h.caller());
642+
643+
expect(err).not.toBe(null);
644+
expect(err.code).toBe('PERMISSION_DENIED');
645+
expect(err.statusCode ?? err.status).toBe(403);
646+
expect(err.details?.object).toBe('os_ehr_andon_record');
647+
// The check gate's own sentence — distinct from both the CRUD denial and
648+
// the row gate's "(row-level security)".
649+
expect(err.developerMessage).toContain('row-level CHECK');
650+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product).toBe(p.id);
651+
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(true);
652+
});
653+
654+
it('THE CONVERSE, again: the exemption is not a delete gate — a caller without delete rights on the TARGET is still refused', async () => {
655+
// The exemption widens which deletes SUCCEED; it must not widen who may ask.
656+
// This arm would go green if the exemption had been spelled anywhere that a
657+
// user-initiated write can reach.
658+
const h = await boot([READER]);
659+
const p = await h.seed('os_ehr_product', { name: 'Widget' });
660+
const a = await h.seed('os_ehr_andon_record', { product: p.id, owner_id: 'u_other' });
661+
662+
const err = await h.deleteAs('os_ehr_product', p.id, h.caller('u_reader'));
663+
664+
expect(err).not.toBe(null);
665+
expect(err.code).toBe('PERMISSION_DENIED');
666+
expect(err.details?.object).toBe('os_ehr_product');
667+
expect(err.details?.operation).toBe('delete');
668+
expect(h.stores.get('os_ehr_product')?.has(p.id)).toBe(true);
669+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product).toBe(p.id);
670+
});
671+
672+
it('an ORDINARY update on the referencing object is untouched by the exemption', async () => {
673+
// The exemption keys on a marker the engine stamps on its own cleanup write
674+
// and nothing else. A caller who edits the same object directly, with the
675+
// same grants, still meets the object-level CRUD check — the marker is the
676+
// whole difference, so this is what proves the exemption is scoped to it.
677+
const h = await boot();
678+
const p = await h.seed('os_ehr_product', { name: 'Widget' });
679+
const a = await h.seed('os_ehr_andon_record', { product: p.id, owner_id: 'u_other' });
680+
681+
const err = await h.engine
682+
.update('os_ehr_andon_record', { id: a.id, product: null }, { context: h.caller() } as any)
683+
.then(() => null, (e: any) => e);
684+
685+
expect(err).not.toBe(null);
686+
expect(err.code).toBe('PERMISSION_DENIED');
687+
expect(err.details?.object).toBe('os_ehr_andon_record');
688+
expect(err.details?.operation).toBe('update');
689+
expect(h.stores.get('os_ehr_andon_record')?.get(a.id)?.product).toBe(p.id);
690+
});
691+
});

0 commit comments

Comments
 (0)