Skip to content

Commit 49ce224

Browse files
committed
wip(plugin-security): explain's record write verdict composes the by-id write gate's authored-row-write deferral
Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 045dae8 commit 49ce224

3 files changed

Lines changed: 203 additions & 6 deletions

File tree

‎packages/plugins/plugin-security/src/explain-enforce-parity.test.ts‎

Lines changed: 141 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -559,15 +559,26 @@ const SHARING_READER = { userId: 'u_reader', positions: ['qa_pos'], permissions:
559559
const SHARING_WRITER = { userId: 'u_writer', positions: ['qa_pos'], permissions: ['qa_line_writer'], posture: 'MEMBER' };
560560
/** Read and write depth `own`: the control. */
561561
const SHARING_OWN = { userId: 'u_own', positions: ['qa_pos'], permissions: ['qa_line_own'], posture: 'MEMBER' };
562+
/**
563+
* Read depth `org`, write depth `own`, and an app-authored row-level `update`
564+
* and `delete` policy admitting every line not named `unnamed`
565+
* ({@link bootSharing}'s `authored` rows): record sharing refuses this writer
566+
* every line owned by someone else, and the by-id write gate defers that
567+
* refusal to the policy.
568+
*/
569+
const AUTHORED_WRITER = { userId: 'u_authored', positions: ['qa_pos'], permissions: ['qa_line_authored'], posture: 'MEMBER' };
562570

563571
/**
564572
* A `private`-OWD object with an `owner_id`, the real `SharingService` as the
565573
* kernel's `sharing` service and its middleware on the engine after this
566574
* plugin's, as the platform boots them. Row `l_other` is owned by nobody the
567575
* rig explains. `faultReadFilter` makes the sharing service's read filter
568-
* reject, as a share store that cannot be read does.
576+
* reject, as a share store that cannot be read does. `authored` adds
577+
* {@link AUTHORED_WRITER}'s set and three lines: `l_theirs` (owned by
578+
* someone else, admitted by the policy), `l_unnamed` (owned by someone else,
579+
* not admitted) and `l_mine` (owned by the writer).
569580
*/
570-
async function bootSharing(opts: { faultReadFilter?: boolean } = {}) {
581+
async function bootSharing(opts: { faultReadFilter?: boolean; authored?: boolean } = {}) {
571582
const LINE = `qa_parity_line_${next()}`;
572583
const engine = new ObjectQL();
573584
engine.registerDriver(
@@ -601,6 +612,13 @@ async function bootSharing(opts: { faultReadFilter?: boolean } = {}) {
601612
{ id: 'l_other', name: 'owned by someone else', owner_id: 'u_somebody' },
602613
{ id: 'l_own', name: 'owned by the control', owner_id: 'u_own' },
603614
], SYS);
615+
if (opts.authored) {
616+
await engine.insert(LINE, [
617+
{ id: 'l_theirs', name: 'theirs', owner_id: 'u_somebody' },
618+
{ id: 'l_unnamed', name: 'unnamed', owner_id: 'u_somebody' },
619+
{ id: 'l_mine', name: 'mine', owner_id: AUTHORED_WRITER.userId },
620+
], SYS);
621+
}
604622

605623
const sets = [
606624
MEMBER_DEFAULT,
@@ -613,6 +631,18 @@ async function bootSharing(opts: { faultReadFilter?: boolean } = {}) {
613631
name: 'qa_line_own',
614632
objects: { [LINE]: { allowRead: true, allowEdit: true, readScope: 'own', writeScope: 'own' } },
615633
}),
634+
...(opts.authored
635+
? [PermissionSetSchema.parse({
636+
name: 'qa_line_authored',
637+
objects: {
638+
[LINE]: { allowRead: true, allowEdit: true, allowDelete: true, allowTransfer: true, readScope: 'org', writeScope: 'own' },
639+
},
640+
rowLevelSecurity: [
641+
{ name: 'qa_line_named_updates', object: LINE, operation: 'update', using: "name != 'unnamed'" },
642+
{ name: 'qa_line_named_deletes', object: LINE, operation: 'delete', using: "name != 'unnamed'" },
643+
],
644+
})]
645+
: []),
616646
];
617647
let security: unknown;
618648
let sharing: SharingService | undefined;
@@ -645,11 +675,21 @@ async function bootSharing(opts: { faultReadFilter?: boolean } = {}) {
645675
engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' });
646676

647677
return {
648-
explain: (caller: object, operation: 'read' | 'update', recordId: string) =>
678+
explain: (caller: object, operation: 'read' | 'update' | 'delete' | 'transfer', recordId: string) =>
649679
explained(plugin.explainAccessForCaller({ object: LINE, operation, recordId }, { ...caller })),
650680
find: (caller: object, id: string) => rowsOf(engine.find(LINE, { where: { id }, context: { ...caller } } as never)),
651681
update: (caller: object, id: string) =>
652682
landed(engine.update(LINE, { name: 'renamed' }, { where: { id }, context: { ...caller } } as never)),
683+
remove: (caller: object, id: string) => landed(engine.delete(LINE, { where: { id }, context: { ...caller } } as never)),
684+
/** A transfer is a by-id update of `owner_id`, gated by the transfer grant. */
685+
transfer: (caller: object, id: string) =>
686+
landed(engine.update(LINE, { owner_id: 'u_recipient' }, { where: { id }, context: { ...caller } } as never)),
687+
/**
688+
* Record sharing's tri-state update verdict, asked as the `sys_attachment`
689+
* and `sys_comment` parent gates ask it before anything else: the caller's
690+
* envelope with no operation-private key.
691+
*/
692+
checkEdit: (caller: object, id: string) => sharing!.checkEdit(LINE, id, { ...caller } as never),
653693
teardown: async () => {
654694
try { await engine.destroy(); } catch { /* noop */ }
655695
releaseAsyncContexts(engine, plugin);
@@ -700,7 +740,7 @@ const withRls = (predicate: string, f: (rig: RlsRig) => Promise<RowFaces>, opts:
700740
try { return { ...(await f(rig)), teardown: rig.teardown }; } catch (e) { await rig.teardown(); throw e; }
701741
};
702742
const withSharing = (
703-
opts: { faultReadFilter?: boolean },
743+
opts: { faultReadFilter?: boolean; authored?: boolean },
704744
f: (rig: Awaited<ReturnType<typeof bootSharing>>) => Promise<RowFaces>,
705745
): RowRun => async () => {
706746
const rig = await bootSharing(opts);
@@ -935,6 +975,48 @@ const TABLE: Row[] = [
935975
explain: await r.explain(SHARING_OWN, 'update', 'l_other'), enforce: await r.update(SHARING_OWN, 'l_other'),
936976
})),
937977
},
978+
// #24647: the by-id write gate defers record sharing's refusal to an
979+
// app-authored row-level write policy that admits the row (ADR-0105 D3).
980+
{
981+
card: '#24647', shape: 'private OWD, an `own` writer whose authored update policy admits a row owned by someone else, update',
982+
position: 'record.visible',
983+
enforced: 'admitted',
984+
run: withSharing({ authored: true }, async (r) => ({
985+
explain: await r.explain(AUTHORED_WRITER, 'update', 'l_theirs'), enforce: await r.update(AUTHORED_WRITER, 'l_theirs'),
986+
})),
987+
},
988+
{
989+
card: '#24647', shape: 'private OWD, an `own` writer whose authored delete policy admits a row owned by someone else, delete',
990+
position: 'record.visible',
991+
enforced: 'admitted',
992+
run: withSharing({ authored: true }, async (r) => ({
993+
explain: await r.explain(AUTHORED_WRITER, 'delete', 'l_theirs'), enforce: await r.remove(AUTHORED_WRITER, 'l_theirs'),
994+
})),
995+
},
996+
{
997+
card: '#24647 control', shape: 'private OWD, the same writer, a row owned by someone else that no authored policy admits, update',
998+
position: 'record.visible',
999+
enforced: REFUSED_DENIED,
1000+
run: withSharing({ authored: true }, async (r) => ({
1001+
explain: await r.explain(AUTHORED_WRITER, 'update', 'l_unnamed'), enforce: await r.update(AUTHORED_WRITER, 'l_unnamed'),
1002+
})),
1003+
},
1004+
{
1005+
card: '#24647 control', shape: 'private OWD, the same writer, a row owned by someone else that no authored policy admits, delete',
1006+
position: 'record.visible',
1007+
enforced: REFUSED_DENIED,
1008+
run: withSharing({ authored: true }, async (r) => ({
1009+
explain: await r.explain(AUTHORED_WRITER, 'delete', 'l_unnamed'), enforce: await r.remove(AUTHORED_WRITER, 'l_unnamed'),
1010+
})),
1011+
},
1012+
{
1013+
card: '#24647 probe', shape: 'private OWD, the same writer, a row owned by someone else the policy admits, transfer',
1014+
position: 'record.visible',
1015+
enforced: 'admitted',
1016+
run: withSharing({ authored: true }, async (r) => ({
1017+
explain: await r.explain(AUTHORED_WRITER, 'transfer', 'l_theirs'), enforce: await r.transfer(AUTHORED_WRITER, 'l_theirs'),
1018+
})),
1019+
},
9381020
// #20002: a dependency enforcement shares with explain throws.
9391021
{
9401022
card: '#20002', shape: 'the sharing read filter throws, the caller\'s own row, read', position: 'record.visible',
@@ -999,3 +1081,58 @@ describe('security.explain answers what enforcement does — the enumeration', (
9991081
}, 60_000);
10001082
}
10011083
});
1084+
1085+
/**
1086+
* #24647's row, beyond the verdict the table compares: what the record story
1087+
* names, and what the `sys_attachment` and `sys_comment` parent gates answer.
1088+
*
1089+
* Both gates ask record sharing's `checkEdit` FIRST, with the caller's envelope
1090+
* and no operation-private key, refuse on its `deny` without asking anything
1091+
* further, and ask explain's record-grained `update` verdict only once that
1092+
* step admits (each gate's own parity table pins it: its `sharing deny` row
1093+
* refuses with explain never called, though the double would admit). So on a
1094+
* row the deferral admits, record sharing's own `deny` still decides both
1095+
* gates: explain admitting it cannot widen either one.
1096+
*/
1097+
describe('an app-authored row-level write policy the by-id write gate defers to (ADR-0105 D3)', () => {
1098+
it('the update story names the deferral: record sharing excludes the row, row-level security admits it', async () => {
1099+
const rig = await bootSharing({ authored: true });
1100+
try {
1101+
const x = await rig.explain(AUTHORED_WRITER, 'update', 'l_theirs');
1102+
expect(x.kind, describeExplained(x)).toBe('decision');
1103+
if (x.kind !== 'decision') return;
1104+
expect(x.decision.record).toEqual({ recordId: 'l_theirs', visible: true, decidedBy: 'rls' });
1105+
const sharingRecord = x.decision.layers.find((l) => l.layer === 'sharing')?.record;
1106+
expect(sharingRecord?.outcome).toBe('excluded');
1107+
expect(sharingRecord?.detail).toMatch(/app-authored row-level update policy admits this record/);
1108+
expect(x.decision.layers.find((l) => l.layer === 'rls')?.record?.outcome).toBe('admitted');
1109+
} finally {
1110+
await rig.teardown();
1111+
}
1112+
}, 60_000);
1113+
1114+
it("the parent gates' first question, record sharing, still refuses the row explain now admits", async () => {
1115+
const rig = await bootSharing({ authored: true });
1116+
try {
1117+
expect(await rig.checkEdit(AUTHORED_WRITER, 'l_theirs')).toBe('deny');
1118+
const x = await rig.explain(AUTHORED_WRITER, 'update', 'l_theirs');
1119+
expect(x.kind === 'decision' ? x.decision.record?.visible : x.kind, describeExplained(x)).toBe(true);
1120+
expect(await rig.update(AUTHORED_WRITER, 'l_theirs')).toEqual({ kind: 'admitted' });
1121+
} finally {
1122+
await rig.teardown();
1123+
}
1124+
}, 60_000);
1125+
1126+
it('the positive control: a row record sharing admits is admitted by the first question, by explain and by the door', async () => {
1127+
const rig = await bootSharing({ authored: true });
1128+
try {
1129+
expect(await rig.checkEdit(AUTHORED_WRITER, 'l_mine')).toBe('allow');
1130+
const x = await rig.explain(AUTHORED_WRITER, 'update', 'l_mine');
1131+
expect(x.kind === 'decision' ? x.decision.record : x.kind, describeExplained(x))
1132+
.toEqual({ recordId: 'l_mine', visible: true, decidedBy: 'rls' });
1133+
expect(await rig.update(AUTHORED_WRITER, 'l_mine')).toEqual({ kind: 'admitted' });
1134+
} finally {
1135+
await rig.teardown();
1136+
}
1137+
}, 60_000);
1138+
});

‎packages/plugins/plugin-security/src/explain-engine.ts‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import {
3636
} from '@objectstack/formula';
3737
import { ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec';
3838
import type {
39+
AuthoredRowWriteOperation,
40+
AuthoredRowWriteVerdict,
3941
ControlledByParentWriteDenialLeg,
4042
ControlledByParentWriteOutcome,
4143
ControlledByParentWriteUnresolvedReason,
@@ -380,6 +382,31 @@ export interface ExplainEngineDeps {
380382
* explanation for a `delete` must consult this rather than the update gate.
381383
*/
382384
canDeleteRecord?: (object: string, recordId: string, context: any) => Promise<boolean>;
385+
/**
386+
* [#5493 / ADR-0105 D3] The by-id write gate's DEFERRAL: whether an
387+
* APP-AUTHORED row-level policy admits the row for the refusing gate's verb
388+
* (`ISecurityService.checkAuthoredRowWrite`, reached through the sharing
389+
* service's own probe). The sharing middleware asks it only after its
390+
* per-record gate refused the row, and only the literal `admit` lifts that
391+
* refusal: the row then goes on to the pre-image gate, which is the
392+
* row-level security this report's `rls` layer explains.
393+
*
394+
* Asked on the same condition here: for a by-id `update` or `delete` whose
395+
* gate above ({@link canEditRecord} / {@link canDeleteRecord}) answered
396+
* `false`, with that gate's verb and the context being EXPLAINED. Every
397+
* other answer leaves the refusal standing, and so does a rejection: the
398+
* door's probe catches its own failure and keeps its refusal, so a failing
399+
* probe is not a failing request and is not reported as one.
400+
*
401+
* Optional: a deps bag wired without it reports the gate's refusal as the
402+
* write verdict, as before.
403+
*/
404+
probeAuthoredRowWrite?: (
405+
object: string,
406+
recordId: string,
407+
operation: AuthoredRowWriteOperation,
408+
context: any,
409+
) => Promise<AuthoredRowWriteVerdict | string>;
383410
/**
384411
* [ADR-0055] The master-detail write check's own answer for a by-id UPDATE
385412
* of `recordId` by `context`: `ISecurityService.checkControlledByParentWrite`,
@@ -1558,6 +1585,17 @@ async function applyRecordAttribution(
15581585
: undefined;
15591586
const writeGateFault = canEditOrFault === DEPENDENCY_FAULT;
15601587
const canEdit = canEditOrFault === DEPENDENCY_FAULT ? undefined : canEditOrFault;
1588+
// [#5493 / ADR-0105 D3] The by-id write gate DEFERS before it refuses: when
1589+
// its per-record gate refuses an `update` or a `delete`, the sharing
1590+
// middleware asks whether an app-authored row-level policy admits the row
1591+
// for that verb, and only `admit` lifts the refusal, handing the row to the
1592+
// pre-image gate (the `rls` layer below). Asked on that condition alone, so
1593+
// a row the gate admits, abstains on or faulted on is explained as before;
1594+
// every answer but `admit`, and a rejection, keeps the refusal, as at the door.
1595+
const authoredRowWriteAdmits = canEdit === false && (engineOp === 'update' || engineOp === 'delete')
1596+
&& deps.probeAuthoredRowWrite
1597+
? (await deps.probeAuthoredRowWrite(object, recordId, engineOp, context).catch(() => 'abstain')) === 'admit'
1598+
: false;
15611599
// [ADR-0055] A by-id write of a `controlled_by_parent` record (an update, a
15621600
// delete, a transfer: {@link MASTER_CHECKED_BY_ID_WRITES}) is judged by the
15631601
// master-detail write check (step 2.8), which the sharing gate above cannot
@@ -1697,7 +1735,11 @@ async function applyRecordAttribution(
16971735
? `The write gate admits this record via the Modify All Data bypass [${vamaSets.join(', ')}], ` +
16981736
'not ownership or a share (see the vama_bypass layer).'
16991737
: 'The sharing service grants write on this record (ownership or an edit/full share).')
1700-
: 'No ownership and no edit/full share grants write on this record.')
1738+
: 'No ownership and no edit/full share grants write on this record.' +
1739+
(authoredRowWriteAdmits
1740+
? ` The by-id ${engineOp} does not stop at that refusal: an app-authored row-level ${engineOp} ` +
1741+
'policy admits this record, so the write gate defers to row-level security (see the rls layer).'
1742+
: ''))
17011743
: depthAdmitsRow
17021744
? `${shareRules.length} share(s) attached; none grants the caller access to this record — read depth ` +
17031745
`'${depthScope}' already admits it, so sharing adds nothing here (see the depth layer).`
@@ -1865,7 +1907,7 @@ async function applyRecordAttribution(
18651907
const businessRowAdmits = isRead
18661908
? owd.effect !== 'private' || ownerIsMe || sharingOutcome === 'admitted' || depthAdmitsRow || vamaAdmitsRow
18671909
: canEdit !== undefined
1868-
? canEdit
1910+
? canEdit || authoredRowWriteAdmits
18691911
: owd.effect === 'public' || ownerIsMe || sharingOutcome === 'admitted' || vamaAdmitsRow;
18701912

18711913
// [#4647] Was the bypass DECISIVE? Only where the baseline and the concrete
@@ -1893,6 +1935,9 @@ async function applyRecordAttribution(
18931935
// the same layer.
18941936
else if (masterLegRefusal) { visible = false; decidedBy = 'sharing'; }
18951937
else if (!businessRowAdmits) { visible = false; decidedBy = owd.effect === 'private' ? 'sharing' : 'owd_baseline'; }
1938+
// [#5493 / ADR-0105 D3] A row the write gate refused and its deferral
1939+
// admitted was last admitted by the app-authored row-level policy.
1940+
else if (authoredRowWriteAdmits) { visible = true; decidedBy = 'rls'; }
18961941
else {
18971942
visible = true;
18981943
// `depth` ranks with `sharing`, the other layer that widens a private

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4946,6 +4946,9 @@ export class SecurityPlugin implements Plugin {
49464946
// - plugin-sharing's per-record gate is asked with `__writeScope` stamped
49474947
// as the middleware stamps it (step 2.6), always overwritten, so an
49484948
// `org` / unit writer is not judged owner-only.
4949+
// - [#5493 / ADR-0105 D3] A refusal of that gate is deferred as the
4950+
// middleware defers it: to an app-authored row-level policy that admits
4951+
// the row for the gate's verb (`probeAuthoredRowWrite`, wired below).
49494952
// [#19986] The READ verdict's twin: plugin-sharing's read filter is asked
49504953
// with `__readScope` stamped as step 2.6 stamps it for a find, so
49514954
// `record.visible` for `read` is what the caller's find returns.
@@ -5053,6 +5056,18 @@ export class SecurityPlugin implements Plugin {
50535056
...(sharing && typeof sharing.canDelete === 'function'
50545057
? { canDeleteRecord: async (o: string, rid: string, c: any) => sharing.canDelete(o, rid, await withWriteScope(o, c)) }
50555058
: {}),
5059+
// [#5493 / ADR-0105 D3] The by-id write gate's deferral, asked through
5060+
// the sharing service's own probe, the member its middleware asks
5061+
// after the gate above refuses, with the context that gate is asked
5062+
// with. The probe passes through to {@link checkAuthoredRowWrite} and
5063+
// abstains on every failure; a sharing service without it defers
5064+
// nothing, so explain reports the gate's refusal as before.
5065+
...(sharing && typeof sharing.probeAuthoredRowWrite === 'function'
5066+
? {
5067+
probeAuthoredRowWrite: async (o: string, rid: string, op: AuthoredRowWriteOperation, c: any) =>
5068+
sharing.probeAuthoredRowWrite(o, rid, op, await withWriteScope(o, c)),
5069+
}
5070+
: {}),
50565071
// [#21771] The by-id write path's read question, asked the same way:
50575072
// the caller-context by-id read, every data middleware included.
50585073
recordAbsentToCaller: (o: string, rid: string, c: any) =>

0 commit comments

Comments
 (0)