From 5b9acf172c7c5cefaa94bf094e934b9b989063ab Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 14:06:49 +0000 Subject: [PATCH 1/4] fix(plugin-security): explain names read depth as the decider of a row only depth admits Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .../plugin-security/src/explain-engine.ts | 146 +++++++++++++++--- 1 file changed, 128 insertions(+), 18 deletions(-) diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 872b2d32d09..5a4b02f89a2 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -1147,13 +1147,20 @@ interface RecordAttributionContext { vamaSets: string[]; /** [#20431] The object's declared columns ({@link declaredColumnsOf}); absent → values only. */ declaredColumns: MatchesFilterOptions | undefined; + /** + * The effective depth the object-level `depth` layer published for this + * operation (ADR-0057 D1, already D10-intersected). The row story reads the + * layer's own value, never a second evaluator call, so the depth layer's + * object-level detail and its record block name one depth. + */ + depthScope: string; } /** * [C2 / ADR-0095] Fill the per-record row story onto the pipeline. Prepends the * `tenant_isolation` Layer 0, tags every layer with its `kernelTier`, attaches a - * `record` attribution to the four row-scoped layers (tenant / owd / sharing / - * rls), and returns the top-level `record` verdict. Every row-level judgement is + * `record` attribution to the row-scoped layers (tenant / owd / depth / sharing / + * vama_bypass / rls), and returns the top-level `record` verdict. Every row-level judgement is * evaluated with the SAME artifacts enforcement produces: * - Layer 0 / Layer 1 filters come from `computeLayeredRlsFilter` (the middleware's * own `Layer0(tenant) AND Layer1(business)` split); @@ -1168,7 +1175,7 @@ async function applyRecordAttribution( ): Promise<{ record: NonNullable; posture: AuthzPosture }> { const { deps, object, recordId, engineOp, context, sets, layers, owd, capsDeny, crudAllowed, vamaEffective, vamaSets, - declaredColumns, + declaredColumns, depthScope, } = ra; const isRead = engineOp === 'find'; const posture = derivePosture(context); @@ -1341,6 +1348,25 @@ async function applyRecordAttribution( 'writable (fail closed), never admitted.' : undefined); const anyShareAdmits = shareRules.some((r) => r.effect === 'admits'); + // ── read depth: did the depth-widened owner-match admit THIS row? ──────── + // The sharing read filter is ONE predicate carrying two grants: the + // owner-match, which the read depth widens (own → [me], unit → my unit's + // owners, org → no owner-match at all, ADR-0057 D1), OR-ed with the ids of the + // caller's own shares. So "the filter admits the row" does not say WHICH half + // admitted it. Read off facts this pass already holds: a filter match on a row + // the caller neither owns nor holds a share on came from the owner-match + // beyond the caller, which only a depth wider than `own` can reach. + // - An on-behalf-of read hands plugin-sharing the delegated context with no + // depth stamped, so its owner-match never widens there. + // - Below `org`, a `null` filter means the sharing service imposes no + // owner-match on this object at all (no owner column, a bypass object), + // which no depth produced. + const delegatedRead = context?.onBehalfOf?.userId != null; + const depthWidens = depthScope !== 'own'; + const depthAdmitsRow = isRead && recordExists && !sharingFaultDetail && owd.effect === 'private' + && !delegatedRead && depthWidens + && sharingMatches === true && !ownerIsMe && !anyShareAdmits + && (depthScope === 'org' || sharingFilter !== null); let sharingOutcome: ExplainRecordAttribution['outcome']; if (!recordExists) { sharingOutcome = 'not_evaluated'; @@ -1350,6 +1376,8 @@ async function applyRecordAttribution( sharingOutcome = 'not_evaluated'; // baseline already grants the rows sharing would add } else if (canEdit !== undefined) { sharingOutcome = canEdit ? 'admitted' : 'excluded'; + } else if (depthAdmitsRow) { + sharingOutcome = 'not_evaluated'; // read depth already admits the row sharing would add } else if (ownerIsMe || anyShareAdmits || sharingMatches === true) { sharingOutcome = 'admitted'; } else if (sharingFilter === undefined && shares.length === 0) { @@ -1383,15 +1411,90 @@ async function applyRecordAttribution( 'not ownership or a share (see the vama_bypass layer).' : 'The sharing service grants write on this record (ownership or an edit/full share).') : 'No ownership and no edit/full share grants write on this record.') - : sharingOutcome === 'admitted' - ? (ownerIsMe ? 'Caller owns the record — visible without a share.' : `${shareRules.length} share(s) attached; access is granted for this record.`) - : `${shareRules.length} share(s) attached; none grants the caller access to this record.` + - (vamaEffective - ? ` Superseded by the View/Modify All Data bypass [${vamaSets.join(', ')}] — see the vama_bypass layer.` - : '')), + : depthAdmitsRow + ? `${shareRules.length} share(s) attached; none grants the caller access to this record — read depth ` + + `'${depthScope}' already admits it, so sharing adds nothing here (see the depth layer).` + : sharingOutcome === 'admitted' + // A grant is named only where one exists: a share naming the + // caller. A filter that admits the row without one says which + // part of the sharing service let it through instead. + ? (ownerIsMe + ? 'Caller owns the record — visible without a share.' + : anyShareAdmits + ? `${shareRules.length} share(s) attached; access is granted for this record.` + : sharingFilter === null + ? `${shareRules.length} share(s) attached; none grants the caller access, but the sharing ` + + 'service imposes no owner-match on this object (rowFilter null), so the record is admitted without one.' + : `The sharing service's read filter admits this record (see rowFilter), but none of the ` + + `${shareRules.length} share(s) attached names the caller directly.`) + : `${shareRules.length} share(s) attached; none grants the caller access to this record.` + + (vamaEffective + ? ` Superseded by the View/Modify All Data bypass [${vamaSets.join(', ')}] — see the vama_bypass layer.` + : '')), }; } + // ── depth: what the read depth did to THIS row ─────────────────────────── + // Judged from the facts above (`depthAdmitsRow`), never from a second filter. + // A row a share admits as well keeps `sharing` as its decider, the last layer + // to admit it in pipeline order (`decidedBy`'s published contract); below + // `org`, the one predicate cannot say whether the depth reached its owner + // too, so that cell says so rather than guess. + const depthLayer = layers.find((l) => l.layer === 'depth'); + if (depthLayer) { + const ownerLabel = String(ownerRaw ?? 'nobody'); + const depthRecord = (outcome: ExplainRecordAttribution['outcome'], detail: string): ExplainRecordAttribution => + ({ outcome, rules: [], detail }); + depthLayer.record = !recordExists + ? depthRecord('not_evaluated', 'Record not found; read depth not evaluated.') + : !isRead + ? depthRecord('not_evaluated', + 'The write depth is judged inside the sharing service\'s per-record write gate, together with ownership ' + + 'and shares; this report does not separate it from them (see the sharing layer).') + : sharingFaultDetail + ? depthRecord('not_evaluated', + "Read depth widens the sharing service's owner-match, whose read filter could not be evaluated " + + '(see the sharing layer).') + : owd.effect !== 'private' + ? depthRecord('not_evaluated', 'Baseline is not private — read depth adds nothing beyond it for this record.') + : delegatedRead + ? depthRecord('not_evaluated', + 'On an on-behalf-of read the sharing filter is asked with the delegated context as it stands, so ' + + 'read depth is not separated from it for this record (see the sharing layer).') + : !depthWidens + ? depthRecord('not_evaluated', + "Effective read depth is 'own' — it reaches no owner beyond the caller, so it adds nothing to " + + 'the baseline for this record (see owd_baseline).') + : sharingFilter === undefined + ? depthRecord('not_evaluated', + "Read depth widens the sharing service's owner-match, which is unavailable on this deployment.") + : ownerIsMe + ? depthRecord('admitted', + `Caller owns the record — inside every read depth, including '${depthScope}'; the baseline ` + + 'already admits it (see owd_baseline).') + : depthAdmitsRow + ? depthRecord('admitted', depthScope === 'org' + ? "Effective read depth 'org' admits this record without a share: at org depth the sharing " + + 'service imposes no owner-match (ADR-0057 D1).' + : `The record's owner (${ownerLabel}) is inside the caller's '${depthScope}' read depth — ` + + "the sharing service's owner-match admits it without a share (ADR-0057 D1).") + : sharingMatches === false + ? depthRecord('excluded', + `The record's owner (${ownerLabel}) is outside the caller's '${depthScope}' read depth; ` + + 'a share may still widen access (see the sharing layer).') + : depthScope === 'org' + ? depthRecord('admitted', + "Effective read depth 'org' admits this record (no owner-match at org depth); a share " + + 'admits it as well (see the sharing layer).') + : sharingFilter === null + ? depthRecord('not_evaluated', + 'The sharing service imposes no owner-match on this object, so read depth has ' + + 'nothing to widen for this record (see the sharing layer).') + : depthRecord('not_evaluated', + `A share admits this record (see the sharing layer); whether the caller's ` + + `'${depthScope}' read depth also reaches its owner is not separated from that share here.`); + } + // ── vama_bypass: what the bypass did to THIS row ───────────────────────── // [#4647] The layer that claims "ownership and sharing checks are skipped" // now says so at row granularity too, and says it from the same verdict the @@ -1456,8 +1559,10 @@ async function applyRecordAttribution( // bypass where no gate answered (a deployment without plugin-sharing) and on // the read branch, whose row filter the bypass short-circuits identically. const vamaAdmitsRow = vamaEffective && recordExists; + // `depthAdmitsRow` is the part of the sharing filter's match the sharing + // layer no longer claims, so the disjunction is the one it always was. const businessRowAdmits = isRead - ? owd.effect !== 'private' || ownerIsMe || sharingOutcome === 'admitted' || vamaAdmitsRow + ? owd.effect !== 'private' || ownerIsMe || sharingOutcome === 'admitted' || depthAdmitsRow || vamaAdmitsRow : canEdit !== undefined ? canEdit : owd.effect === 'public' || ownerIsMe || sharingOutcome === 'admitted' || vamaAdmitsRow; @@ -1483,17 +1588,22 @@ async function applyRecordAttribution( else if (!businessRowAdmits) { visible = false; decidedBy = owd.effect === 'private' ? 'sharing' : 'owd_baseline'; } else { visible = true; + // `depth` ranks with `sharing`, the other layer that widens a private + // baseline past its owner: a row no share and no ownership admits, but the + // read depth does, is the depth layer's admission. decidedBy = bypassWasDecisive ? 'vama_bypass' : (owd.effect === 'private' && !ownerIsMe && sharingOutcome === 'admitted') ? 'sharing' - : layer1 != null - ? 'rls' - : owd.effect === 'private' && ownerIsMe - ? 'owd_baseline' - : layer0 != null - ? 'tenant_isolation' - : 'object_crud'; + : depthAdmitsRow + ? 'depth' + : layer1 != null + ? 'rls' + : owd.effect === 'private' && ownerIsMe + ? 'owd_baseline' + : layer0 != null + ? 'tenant_isolation' + : 'object_crud'; } return { @@ -1934,7 +2044,7 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput if (input.recordId) { const out = await applyRecordAttribution({ deps, object, recordId: input.recordId, engineOp: dataOp, context, sets, layers, owd, capsDeny, crudAllowed, - vamaEffective, vamaSets, declaredColumns, + vamaEffective, vamaSets, declaredColumns, depthScope: scope, }); recordVerdict = out.record; posture = out.posture; From edf58c1d239f6c70a9ae31b2792a3417cf56b751 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 14:15:35 +0000 Subject: [PATCH 2/4] test(plugin-security): pin the depth decider, the rule share, and every published decidedBy member Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .../src/explain-dependency-fault.test.ts | 10 +- .../src/explain-engine.test.ts | 174 +++++++++++++++++- .../src/explain-read-verdict-inputs.test.ts | 79 +++++++- 3 files changed, 256 insertions(+), 7 deletions(-) diff --git a/packages/plugins/plugin-security/src/explain-dependency-fault.test.ts b/packages/plugins/plugin-security/src/explain-dependency-fault.test.ts index de483b75086..7c3e69c218b 100644 --- a/packages/plugins/plugin-security/src/explain-dependency-fault.test.ts +++ b/packages/plugins/plugin-security/src/explain-dependency-fault.test.ts @@ -359,10 +359,16 @@ describe('healthy sharing layer — reported exactly as before', () => { const stack = await makeStack({ storeFaults: ['sys_record_share'] }); const decision = await stack.explain('read', 'unshared', HR_CTX); expect((await stack.request('read', 'unshared', HR_CTX)).ok).toBe(true); - expect(decision.record.visible).toBe(true); + // The org depth admits the row, so the report credits the depth layer; + // a fault would have answered `visible: false`, decided by sharing. + expect(decision.record).toEqual({ recordId: ROWS.unshared.id, visible: true, decidedBy: 'depth' }); const sharingRecord = layerOf(decision, 'sharing').record; - expect(sharingRecord.outcome).toBe('admitted'); + // An answer, not a fault: the filter is published (null, unrestricted) and + // judged against the row. A faulted layer publishes neither. expect(sharingRecord.rowFilter).toBeNull(); + expect(sharingRecord.matchesRecord).toBe(true); + expect(sharingRecord.detail).not.toContain('could not be evaluated'); + expect(layerOf(decision, 'depth').record.outcome).toBe('admitted'); }); }); diff --git a/packages/plugins/plugin-security/src/explain-engine.test.ts b/packages/plugins/plugin-security/src/explain-engine.test.ts index 2e3e7f5762e..c1e438fece2 100644 --- a/packages/plugins/plugin-security/src/explain-engine.test.ts +++ b/packages/plugins/plugin-security/src/explain-engine.test.ts @@ -3,7 +3,7 @@ import { describe, it, expect } from 'vitest'; import { resolveUserAuthzGrants, resetPlatformAdminEmailMemo } from '@objectstack/core'; -import { PermissionSetSchema } from '@objectstack/spec/security'; +import { ExplainDecisionSchema, PermissionSetSchema } from '@objectstack/spec/security'; import { PermissionEvaluator } from './permission-evaluator'; import { explainAccess, buildContextForUser, resolveDelegatorContext, type ExplainEngineDeps } from './explain-engine'; import { RLS_DENY_FILTER } from './rls-compiler'; @@ -594,6 +594,178 @@ describe('explainAccess — record-grained (C2 / ADR-0095)', () => { expect(d.record).toMatchObject({ visible: true, decidedBy: 'vama_bypass' }); }); + // ── read depth, at row granularity ────────────────────────────────────── + // The sharing read filter is one predicate carrying two grants: the + // owner-match the read depth widens, OR-ed with the caller's own shares. A + // row the filter admits with no share and no ownership behind it was admitted + // by the depth, and the report names the depth layer, not sharing. + describe('read depth admits a row that no share does', () => { + const ORG_READER = PermissionSetSchema.parse({ + name: 'org_reader', + objects: { leave_request: { allowRead: true, readScope: 'org' } }, + }); + const NOT_MINE = { id: 'r1', organization_id: 'org1', owner_id: 'u_other' }; + const RULE_SHARE = { + id: 'shr_rule', recipient_type: 'user', recipient_id: 'u1', access_level: 'read' as const, + source: 'rule', source_id: 'rule_open_requests', + }; + const layerRecord = (d: { layers: Array<{ layer: string; record?: any }> }, layer: string) => + d.layers.find((l) => l.layer === layer)!.record!; + const read = (deps: ExplainEngineDeps, context: any = REC_CTX) => + explainAccess(deps, { object: 'leave_request', operation: 'read', context, recordId: 'r1' }); + + it("org depth: the row explains as decidedBy 'depth', with a depth-layer record block", async () => { + // At org depth plugin-sharing composes no owner-match: the filter is null. + const d = await read(recDeps({ sets: [ORG_READER], record: NOT_MINE, shares: [], sharingFilter: null })); + expect(d.record).toMatchObject({ visible: true, decidedBy: 'depth' }); + const depth = layerRecord(d, 'depth'); + expect(depth.outcome).toBe('admitted'); + expect(depth.detail).toContain("read depth 'org'"); + const sharing = layerRecord(d, 'sharing'); + expect(sharing.outcome).not.toBe('admitted'); + expect(sharing.detail).not.toContain('access is granted'); + expect(sharing.detail).toContain('see the depth layer'); + }); + + it("unit depth: an owner inside the caller's unit explains as 'depth', naming the owner", async () => { + const d = await read(recDeps({ record: NOT_MINE, shares: [], sharingFilter: { owner_id: { $in: ['u1', 'u_other'] } } })); + expect(d.record).toMatchObject({ visible: true, decidedBy: 'depth' }); + expect(layerRecord(d, 'depth').outcome).toBe('admitted'); + expect(layerRecord(d, 'depth').detail).toContain("owner (u_other) is inside the caller's 'unit' read depth"); + expect(layerRecord(d, 'sharing').outcome).toBe('not_evaluated'); + }); + + it("unit depth: an owner outside the caller's unit is excluded by the depth layer too", async () => { + const d = await read(recDeps({ record: NOT_MINE, shares: [], sharingFilter: { owner_id: { $in: ['u1'] } } })); + expect(d.record).toMatchObject({ visible: false, decidedBy: 'sharing' }); + expect(layerRecord(d, 'depth').outcome).toBe('excluded'); + expect(layerRecord(d, 'sharing').outcome).toBe('excluded'); + }); + + it("a criteria rule's share still explains as 'sharing', with '1 share(s) attached'", async () => { + // A criteria sharing rule materialises as a `sys_record_share` row with + // `source: 'rule'` naming the matched user (sharing-rule-service.ts). + const d = await read(recDeps({ + record: NOT_MINE, + shares: [RULE_SHARE], + sharingFilter: { $or: [{ owner_id: { $in: ['u1'] } }, { id: { $in: ['r1'] } }] }, + })); + expect(d.record).toMatchObject({ visible: true, decidedBy: 'sharing' }); + const sharing = layerRecord(d, 'sharing'); + expect(sharing.outcome).toBe('admitted'); + expect(sharing.detail).toBe('1 share(s) attached; access is granted for this record.'); + expect(sharing.rules?.[0]).toMatchObject({ kind: 'sharing_rule', name: 'rule_open_requests', effect: 'admits' }); + }); + + it("a share AND org depth: 'sharing' decides, the last layer to admit; the depth layer admits as well", async () => { + const d = await read(recDeps({ sets: [ORG_READER], record: NOT_MINE, shares: [RULE_SHARE], sharingFilter: null })); + expect(d.record).toMatchObject({ visible: true, decidedBy: 'sharing' }); + expect(layerRecord(d, 'depth').outcome).toBe('admitted'); + }); + + it("a share AND unit depth: 'sharing' decides; the depth layer says it cannot separate the two", async () => { + const d = await read(recDeps({ + record: NOT_MINE, + shares: [RULE_SHARE], + sharingFilter: { $or: [{ owner_id: { $in: ['u1', 'u_other'] } }, { id: { $in: ['r1'] } }] }, + })); + expect(d.record).toMatchObject({ visible: true, decidedBy: 'sharing' }); + expect(layerRecord(d, 'depth').outcome).toBe('not_evaluated'); + expect(layerRecord(d, 'depth').detail).toContain('not separated'); + }); + + it('below org, a null filter is the sharing service imposing nothing — not the depth', async () => { + const d = await read(recDeps({ record: NOT_MINE, shares: [], sharingFilter: null })); + expect(d.record).toMatchObject({ visible: true, decidedBy: 'sharing' }); + const sharing = layerRecord(d, 'sharing'); + expect(sharing.detail).toContain('imposes no owner-match'); + expect(sharing.detail).not.toContain('access is granted'); + expect(layerRecord(d, 'depth').outcome).toBe('not_evaluated'); + }); + + it('an on-behalf-of read never credits the depth: its filter is asked with no depth stamped', async () => { + const d = await read( + { ...recDeps({ record: NOT_MINE, shares: [], sharingFilter: { owner_id: { $in: ['u1', 'u_other'] } } }), ql: DELEGATOR_QL }, + { ...REC_CTX, onBehalfOf: { userId: 'u_boss' } }, + ); + expect(d.record?.visible).toBe(true); + expect(d.record?.decidedBy).not.toBe('depth'); + expect(layerRecord(d, 'depth').outcome).toBe('not_evaluated'); + expect(layerRecord(d, 'sharing').detail).not.toContain('access is granted'); + }); + }); + + // ── every published decider is reachable ──────────────────────────────── + // The enum is read from the spec, never restated here: a member the spec + // gains, and no branch of the decision tree produces, fails by name until a + // scenario produces it or the ledger below says why none can. + describe('decision.record.decidedBy: each published member is produced by a branch, or ledgered', () => { + const DECIDED_BY: readonly string[] = + ExplainDecisionSchema.shape.record.unwrap().shape.decidedBy.unwrap().options; + const VIEW_ALL = PermissionSetSchema.parse({ + name: 'compliance_auditor', + objects: { '*': { allowRead: true, viewAllRecords: true } }, + }); + const CAPS_GATED = { + isPrivate: false, + requiredPermissions: { all: [], read: ['manage_metadata'], create: [], update: [], delete: [] }, + fieldRequiredPermissions: {}, + }; + const NOT_MINE = { id: 'r1', organization_id: 'org1', owner_id: 'u_other' }; + const ask = (deps: ExplainEngineDeps, operation: 'read' | 'delete' = 'read') => + explainAccess(deps, { object: 'leave_request', operation, context: REC_CTX, recordId: 'r1' }); + + const PRODUCERS: Record Promise<{ record?: { decidedBy?: string } }>> = { + tenant_isolation: () => ask(recDeps({ + layered: { layer0: { organization_id: 'org1' }, layer1: null }, + record: { ...NOT_MINE, organization_id: 'org2' }, + })), + required_permissions: () => ask({ ...recDeps(), getObjectSecurityMeta: async () => CAPS_GATED }), + object_crud: () => ask(recDeps(), 'delete'), + owd_baseline: () => ask(recDeps({ record: { id: 'r1', organization_id: 'org1', owner_id: 'u1' } })), + depth: () => ask(recDeps({ record: NOT_MINE, shares: [], sharingFilter: { owner_id: { $in: ['u1', 'u_other'] } } })), + sharing: () => ask(recDeps({ + record: NOT_MINE, + shares: [{ id: 'shr_1', recipient_type: 'user', recipient_id: 'u1', access_level: 'read', source: 'manual' }], + sharingFilter: { $or: [{ owner_id: { $in: ['u1'] } }, { id: { $in: ['r1'] } }] }, + })), + vama_bypass: () => ask(recDeps({ sets: [VIEW_ALL], record: NOT_MINE, shares: [], sharingFilter: { owner_id: 'u1' } })), + rls: () => ask(recDeps({ + layered: { layer0: null, layer1: { status: 'open' } }, + record: { ...NOT_MINE, owner_id: 'u1', status: 'closed' }, + })), + }; + const NOT_PRODUCED: Record = { + principal: + 'The principal layer resolves who is asking and which sets they hold; it never judges a row. A principal ' + + 'whose grants fail to resolve (a missing or unresolvable delegator) is refused at object_crud, which is ' + + 'the layer the tree names for it.', + fls: + 'Field-level security masks fields of a row that is served; it never admits or removes the row, so it ' + + 'cannot decide whether the row is visible.', + }; + + it('the producers and the ledger partition the enum exactly', () => { + for (const member of DECIDED_BY) { + const produced = member in PRODUCERS; + const ledgered = member in NOT_PRODUCED; + expect(produced || ledgered, `decidedBy member '${member}' is produced by no branch and carries no ledger reason`).toBe(true); + expect(produced && ledgered, `decidedBy member '${member}' is both produced and ledgered`).toBe(false); + } + for (const key of [...Object.keys(PRODUCERS), ...Object.keys(NOT_PRODUCED)]) { + expect(DECIDED_BY, `'${key}' is not a member of the published decidedBy enum`).toContain(key); + } + for (const [member, reason] of Object.entries(NOT_PRODUCED)) { + expect(reason.trim().length, `the ledger entry for '${member}' carries no reason`).toBeGreaterThan(0); + } + }); + + it.each(Object.keys(PRODUCERS))("'%s' is produced by the branch its scenario drives", async (member) => { + const d = await PRODUCERS[member](); + expect(d.record?.decidedBy).toBe(member); + }); + }); + it('degrades gracefully with no record-grained deps — object-level layers plus a best-effort verdict', async () => { // Only the base object-level deps: recordId is given but fetchRecord / // computeLayeredRlsFilter etc. are absent (e.g. no plugin-sharing). diff --git a/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts b/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts index 33d6cd25536..a904afc3cb8 100644 --- a/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts +++ b/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts @@ -88,7 +88,7 @@ const REPORTER_CTX = ctxFor(U_REPORTER, 'dept_reporter'); */ const UNIT_OWNERS = [U_LEAD, U_REPORTER]; -type RowKey = 'shared' | 'owned' | 'unshared'; +type RowKey = 'shared' | 'owned' | 'unshared' | 'ruled' | 'lead_shared'; const ROWS: Record> = { /** Owned by the admin; the reporter holds a `read` share on it. */ @@ -97,6 +97,10 @@ const ROWS: Record> = { owned: { id: 'kle_owned', name: 'Owned', value: 1, owner_id: U_REPORTER, created_by: U_ADMIN, organization_id: 'org1' }, /** Neither shared with nor owned by the reporter or the lead's unit. */ unshared: { id: 'kle_unshared', name: 'Unshared', value: 1, owner_id: U_ADMIN, created_by: U_ADMIN, organization_id: 'org1' }, + /** Owned by the admin; a criteria sharing rule grants the reporter `read` on it. */ + ruled: { id: 'kle_ruled', name: 'Ruled', value: 9, owner_id: U_ADMIN, created_by: U_ADMIN, organization_id: 'org1' }, + /** Owned by the reporter (inside the lead's unit) AND shared to the lead and to the HR reviewer. */ + lead_shared: { id: 'kle_lead_shared', name: 'LeadShared', value: 1, owner_id: U_REPORTER, created_by: U_ADMIN, organization_id: 'org1' }, }; const READ_SHARE = { @@ -104,12 +108,27 @@ const READ_SHARE = { recipient_type: 'user', recipient_id: U_REPORTER, access_level: 'read', source: 'manual', }; +/** + * The row a criteria sharing rule materialises for the user it matched: + * `source: 'rule'`, `source_id` naming the rule (sharing-rule-service.ts). + */ +const RULE_SHARE = { + id: 'shr_rule', object_name: OBJECT, record_id: ROWS.ruled.id, + recipient_type: 'user', recipient_id: U_REPORTER, access_level: 'read', source: 'rule', source_id: 'rule_high_value', +}; + +/** Two manual shares on a row whose owner is already inside the lead's unit. */ +const DEPTH_AND_SHARE = [U_LEAD, U_HR].map((recipient) => ({ + id: `shr_both_${recipient}`, object_name: OBJECT, record_id: ROWS.lead_shared.id, + recipient_type: 'user', recipient_id: recipient, access_level: 'read', source: 'manual', +})); + // ── in-memory engine ─────────────────────────────────────────────────────── function makeEngine(schema: Record) { const tables: Record = { [OBJECT]: (Object.keys(ROWS) as RowKey[]).map((k) => ({ ...ROWS[k] })), - sys_record_share: [{ ...READ_SHARE }], + sys_record_share: [{ ...READ_SHARE }, { ...RULE_SHARE }, ...DEPTH_AND_SHARE.map((r) => ({ ...r }))], }; // `$or` / `$and` conjoin WITH their sibling keys, the way a real driver ANDs // them (#7620). An operator this double does not know THROWS rather than @@ -288,13 +307,18 @@ describe('explain(read) agrees with the find, cell by cell (private OWD)', () => }); describe('the read depth: explain stamps it the way step 2.6 does', () => { - it('an org-depth reader is admitted by the sharing layer on an unshared, not-owned row', async () => { + it('an org-depth reader is admitted by the read depth on an unshared, not-owned row — not by a share', async () => { const decision = await expectCell(await makeStack(), 'unshared', HR_CTX, true); const sharingRecord = decision.layers.find((l: any) => l.layer === 'sharing').record; // `org` depth: plugin-sharing imposes no owner-match, which is what the // find saw (its AST carries only the id and tenant terms). - expect(sharingRecord.outcome).toBe('admitted'); expect(sharingRecord.rowFilter).toBeNull(); + // No share names the reviewer, so the sharing layer claims no admission + // and states no grant; the depth layer is the one that admitted the row. + expect(sharingRecord.outcome).toBe('not_evaluated'); + expect(sharingRecord.detail).not.toContain('access is granted'); + expect(decision.layers.find((l: any) => l.layer === 'depth').record.outcome).toBe('admitted'); + expect(decision.record.decidedBy).toBe('depth'); }); it('a depth the CALLER brings does not decide — explain computes it, as step 2.6 overwrites it (widening)', async () => { @@ -311,6 +335,53 @@ describe('the read depth: explain stamps it the way step 2.6 does', () => { }); }); +describe('the decider of a visible row: read depth or a share, through the real sharing service', () => { + // `visible` is unchanged from the cells above (each one still agrees with + // the find); what is pinned here is WHICH layer the report credits. + const layerRecord = (decision: any, layer: string) => decision.layers.find((l: any) => l.layer === layer).record; + const cells: Array<[string, RowKey, any, string]> = [ + ['hr_reviewer (org depth), unshared row', 'unshared', HR_CTX, 'depth'], + ['hr_reviewer (org depth), a row shared to someone else', 'shared', HR_CTX, 'depth'], + ['team_lead (unit depth), owned inside the unit', 'owned', LEAD_CTX, 'depth'], + ['dept_reporter (own depth) via a manual share', 'shared', REPORTER_CTX, 'sharing'], + ['dept_reporter (own depth) via a criteria rule', 'ruled', REPORTER_CTX, 'sharing'], + ['dept_reporter (own depth) as owner', 'owned', REPORTER_CTX, 'owd_baseline'], + // Both admit: the last layer to admit, in pipeline order, decides. + ['team_lead (unit depth) holding a share on a row inside the unit', 'lead_shared', LEAD_CTX, 'sharing'], + ['hr_reviewer (org depth) holding a share', 'lead_shared', HR_CTX, 'sharing'], + ]; + it.each(cells)('%s → %s row → decidedBy %s', async (_label, rowKey, context, decider) => { + const decision = await expectCell(await makeStack(), rowKey, context, true); + expect(decision.record.decidedBy).toBe(decider); + }); + + it('a depth-only row carries a depth-layer record block naming the owner and the depth', async () => { + const decision = await expectCell(await makeStack(), 'owned', LEAD_CTX, true); + const depth = layerRecord(decision, 'depth'); + expect(depth.outcome).toBe('admitted'); + expect(depth.detail).toContain(`owner (${U_REPORTER}) is inside the caller's 'unit' read depth`); + const sharing = layerRecord(decision, 'sharing'); + expect(sharing.outcome).toBe('not_evaluated'); + expect(sharing.detail).toContain('0 share(s) attached; none grants the caller access'); + expect(sharing.detail).not.toContain('access is granted'); + }); + + it("the criteria rule's row still reads '1 share(s) attached; access is granted'", async () => { + const decision = await expectCell(await makeStack(), 'ruled', REPORTER_CTX, true); + const sharing = layerRecord(decision, 'sharing'); + expect(sharing.outcome).toBe('admitted'); + expect(sharing.detail).toBe('1 share(s) attached; access is granted for this record.'); + expect(sharing.rules[0]).toMatchObject({ kind: 'sharing_rule', name: 'rule_high_value', effect: 'admits' }); + }); + + it('a row outside the unit is excluded by the depth layer and the sharing layer alike', async () => { + const decision = await expectCell(await makeStack(), 'unshared', LEAD_CTX, false); + expect(decision.record.decidedBy).toBe('sharing'); + expect(layerRecord(decision, 'depth').outcome).toBe('excluded'); + expect(layerRecord(decision, 'sharing').outcome).toBe('excluded'); + }); +}); + describe('the on-behalf-of path keeps its previous inputs', () => { it("explain hands plugin-sharing's read filter the delegated context unchanged", async () => { // The middleware's delegated read depth (the agent-leg intersection, plus From 256d6f97c3d09285ec045296a0698700799ad92d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 14:16:09 +0000 Subject: [PATCH 3/4] chore(changeset): plugin-security patch for the depth decider Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .changeset/21726-explain-decided-by-depth.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .changeset/21726-explain-decided-by-depth.md diff --git a/.changeset/21726-explain-decided-by-depth.md b/.changeset/21726-explain-decided-by-depth.md new file mode 100644 index 00000000000..381b29da7f0 --- /dev/null +++ b/.changeset/21726-explain-decided-by-depth.md @@ -0,0 +1,14 @@ +--- +'@objectstack/plugin-security': patch +--- + +`POST /api/v1/security/explain` with `{ object, operation: 'read', recordId }` now credits read depth for a row that only the caller's read depth admits. + +Clause-②: no + +- **Before.** The object's OWD is private. The caller's read depth is wider than `own` (`own_and_reports`, `unit`, `unit_and_below` or `org`). The caller reads a row they do not own and hold no share on. Explain answered `decidedBy: 'sharing'` for that row, with the sharing layer `admitted` and the detail "0 share(s) attached; access is granted for this record." No share granted anything; the read depth did. `depth` is a member of the published `decidedBy` enum, and no answer ever produced it. +- **Now.** That row answers `decidedBy: 'depth'`. The `depth` layer carries a `record` block: `admitted`, naming the depth and, below `org`, the owner it reached. The sharing layer is `not_evaluated`, and its detail says that no share grants the row and that read depth already admits it. +- **The sharing detail names a grant only when one exists**, meaning a share that names the caller. A filter that admits a row with no such share now says which part of the sharing service let the row through. +- **A row a share admits keeps `decidedBy: 'sharing'`** and its "N share(s) attached; access is granted for this record." detail, also when the read depth admits it too. Of the two layers that admit it, sharing comes last in the pipeline, and the `decidedBy` contract names the last layer to admit. +- **No access decision changes.** `visible` is the same for every record; only the layer the report credits moves. No key is added. `depth` and `not_evaluated` are existing values. +- **The `depth` layer's `record` block is new on every record-grained request.** On a write it is `not_evaluated`, because the write depth is judged inside the sharing service's per-record write gate together with ownership and shares, and this report does not separate them. From 8998a647a0fe86ceadb4bbbea7ab68d1646eec74 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 14:23:50 +0000 Subject: [PATCH 4/4] test(plugin-security): name the expected decider in the decider cells' titles Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .../src/explain-read-verdict-inputs.test.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts b/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts index a904afc3cb8..dd6a95a60a7 100644 --- a/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts +++ b/packages/plugins/plugin-security/src/explain-read-verdict-inputs.test.ts @@ -339,18 +339,18 @@ describe('the decider of a visible row: read depth or a share, through the real // `visible` is unchanged from the cells above (each one still agrees with // the find); what is pinned here is WHICH layer the report credits. const layerRecord = (decision: any, layer: string) => decision.layers.find((l: any) => l.layer === layer).record; - const cells: Array<[string, RowKey, any, string]> = [ - ['hr_reviewer (org depth), unshared row', 'unshared', HR_CTX, 'depth'], - ['hr_reviewer (org depth), a row shared to someone else', 'shared', HR_CTX, 'depth'], - ['team_lead (unit depth), owned inside the unit', 'owned', LEAD_CTX, 'depth'], - ['dept_reporter (own depth) via a manual share', 'shared', REPORTER_CTX, 'sharing'], - ['dept_reporter (own depth) via a criteria rule', 'ruled', REPORTER_CTX, 'sharing'], - ['dept_reporter (own depth) as owner', 'owned', REPORTER_CTX, 'owd_baseline'], + const cells: Array<[string, RowKey, string, any]> = [ + ['hr_reviewer (org depth), unshared row', 'unshared', 'depth', HR_CTX], + ['hr_reviewer (org depth), a row shared to someone else', 'shared', 'depth', HR_CTX], + ['team_lead (unit depth), owned inside the unit', 'owned', 'depth', LEAD_CTX], + ['dept_reporter (own depth) via a manual share', 'shared', 'sharing', REPORTER_CTX], + ['dept_reporter (own depth) via a criteria rule', 'ruled', 'sharing', REPORTER_CTX], + ['dept_reporter (own depth) as owner', 'owned', 'owd_baseline', REPORTER_CTX], // Both admit: the last layer to admit, in pipeline order, decides. - ['team_lead (unit depth) holding a share on a row inside the unit', 'lead_shared', LEAD_CTX, 'sharing'], - ['hr_reviewer (org depth) holding a share', 'lead_shared', HR_CTX, 'sharing'], + ['team_lead (unit depth) holding a share on a row inside the unit', 'lead_shared', 'sharing', LEAD_CTX], + ['hr_reviewer (org depth) holding a share', 'lead_shared', 'sharing', HR_CTX], ]; - it.each(cells)('%s → %s row → decidedBy %s', async (_label, rowKey, context, decider) => { + it.each(cells)('%s × %s row → decidedBy %s', async (_label, rowKey, decider, context) => { const decision = await expectCell(await makeStack(), rowKey, context, true); expect(decision.record.decidedBy).toBe(decider); });