Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/29610-explain-modify-all-conditioned-field.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@objectstack/plugin-security': patch
---

fix(plugin-security): `os explain` no longer tells a Modify All holder that a row-conditioned field lock binds

Clause-②: yes (widening)
Metadata-protocol: none

The access explanation's field-level layer listed every field permission that carries a `when` as "judged per row by the write gate, applied as the lock everywhere else". For a caller whose permission sets hold Modify All Data on the object, that was untrue: the field merge applies the Modify All vote after the per-entry votes, under every `when` verdict, so such a lock binds on no row (ruling of record 6573563853, letter A). The explanation now lists those entries separately, as superseded by the Modify All Data field vote and naming the sets that hold it. A View All holder, and a caller who holds neither, keeps the row-conditioned sentence as before, because their edit lock still binds per row.

Enforcement is unchanged. `PermissionEvaluator.conditionedFieldEntries` now names each entry's superseding sets in a new `supersededBy` member, read from the same `superuserFieldVote` that `getFieldPermissions` applies.
63 changes: 63 additions & 0 deletions packages/plugins/plugin-security/src/explain-engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,69 @@ describe('explainAccess — fls reports partial masking (#9127)', () => {
expect(fls.detail).toContain(`amount (locked when status == 'approved', set "hr_user")`);
});

// [#29610 / ruling of record 6573563853, letter A] A set holding Modify All
// on the object votes editable on every field after the per-entry vote, under
// every `when` verdict, so for that caller a conditioned lock binds on no row.
// The report reads the SAME vote (`superuserFieldVote` through
// `conditionedFieldEntries`), never a second copy of it.
const CONDITIONED_AMOUNT = {
'leave_request.amount': { readable: true, editable: false, when: "status == 'approved'" },
};

it('[#29610] a Modify All set supersedes a row-conditioned lock: the entry is reported as binding on no row', async () => {
const fls = await flsOf({
sets: [
{
name: 'hr_user',
objects: { leave_request: { allowRead: true, allowEdit: true } },
fields: CONDITIONED_AMOUNT,
},
{
name: 'hr_admin',
objects: { leave_request: { allowRead: true, allowEdit: true, viewAllRecords: true, modifyAllRecords: true } },
},
] as any,
});
expect(fls.verdict).toBe('not_applicable');
expect(fls.detail).not.toContain('judged per row by the write gate');
expect(fls.detail).toContain('1 row-conditioned field permission(s) are superseded by the Modify All Data field vote [hr_admin]');
expect(fls.detail).toContain('binds on no row');
expect(fls.detail).toContain(`amount (locked when status == 'approved', set "hr_user")`);
});

it('[#29610] a View All set does NOT supersede it: the edit lock still binds per row (read-side control)', async () => {
const fls = await flsOf({
sets: [
{
name: 'hr_user',
objects: { leave_request: { allowRead: true, allowEdit: true } },
fields: CONDITIONED_AMOUNT,
},
{
name: 'hr_auditor',
objects: { leave_request: { allowRead: true, viewAllRecords: true } },
},
] as any,
});
expect(fls.detail).toContain('1 field permission(s) carry a row condition — judged per row by the write gate');
expect(fls.detail).toContain(`amount (locked when status == 'approved', set "hr_user")`);
expect(fls.detail).not.toContain('superseded');
});

it('[#29610] a caller without the vote keeps the row-conditioned sentence as before (control)', async () => {
const fls = await flsOf({
sets: [
{
name: 'hr_user',
objects: { leave_request: { allowRead: true, allowEdit: true } },
fields: CONDITIONED_AMOUNT,
},
] as any,
});
expect(fls.detail).toContain('1 field permission(s) carry a row condition — judged per row by the write gate');
expect(fls.detail).not.toContain('superseded');
});

it('[#25866] an entry without a `when` adds no row-condition sentence', async () => {
const fls = await flsOf({
sets: [
Expand Down
31 changes: 24 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2959,18 +2959,35 @@ async function explainObjectPass(deps: ExplainEngineDeps, input: ExplainInput, a
// stored row before an update); this report holds no row, so each such entry
// is reported AS a row-conditioned lock — which is also how every row-blind
// reader applies it — rather than folded silently into the mask above.
//
// [#29610 / ruling of record 6573563853, letter A] …unless the principal's
// Modify All vote supersedes it: `getFieldPermissions` applies that vote after
// the per-entry votes under every `when` verdict, so such an entry's lock
// binds on no row. The evaluator names the superseding sets from the SAME
// vote (`ConditionedFieldEntry.supersededBy`); this layer only words it, in
// the "Superseded by …" form the sharing attribution already uses. Each
// principal's entries are judged against its own sets: under D10 the masks
// intersect, so a delegator's lock still binds when only the agent holds the
// vote.
const conditioned = [
...deps.evaluator.conditionedFieldEntries(object, sets),
...(delegatorSets ? deps.evaluator.conditionedFieldEntries(object, delegatorSets) : []),
];
const describeConditioned = (e: (typeof conditioned)[number]): string =>
`${e.field} (${e.editable ? 'editable' : 'locked'} when ${e.when}, set "${e.set}")`;
const binding = conditioned.filter((e) => e.supersededBy.length === 0);
const superseded = conditioned.filter((e) => e.supersededBy.length > 0);
const supersedingSets = [...new Set(superseded.flatMap((e) => e.supersededBy))];
const conditionedSentence =
conditioned.length > 0
? ` ${conditioned.length} field permission(s) carry a row condition — judged per row by the write gate, ` +
`applied as the lock everywhere else: ${listFields(
conditioned.map((e) => `${e.field} (${e.editable ? 'editable' : 'locked'} when ${e.when}, set "${e.set}")`),
(f) => f,
)}.`
: '';
(binding.length > 0
? ` ${binding.length} field permission(s) carry a row condition — judged per row by the write gate, ` +
`applied as the lock everywhere else: ${listFields(binding.map(describeConditioned), (f) => f)}.`
: '') +
(superseded.length > 0
? ` ${superseded.length} row-conditioned field permission(s) are superseded by the Modify All Data field vote ` +
`[${supersedingSets.join(', ')}], which votes every field editable on every row, so the lock binds on no row: ` +
`${listFields(superseded.map(describeConditioned), (f) => f)}.`
: '');
layers.push({
layer: 'fls',
verdict: maskNarrows ? 'narrows' : 'not_applicable',
Expand Down
22 changes: 21 additions & 1 deletion packages/plugins/plugin-security/src/permission-evaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ export interface ConditionedFieldEntry {
when: string;
/** The entry's edit vote on a matching row. */
editable: boolean;
/**
* [#29610 / ruling of record 6573563853, letter A] The sets whose Modify All
* vote supersedes the entry — empty when none does. Read from
* `superuserFieldVote` (`@objectstack/core`), the vote
* {@link PermissionEvaluator.getFieldPermissions} applies after the
* per-entry votes under every `when` verdict: when it votes `editable`, every
* field of the object is readable and editable on every row, so the entry's
* lock binds on no row for this principal. A View All vote supersedes
* nothing here: it votes only the read bit, which a conditioned entry already
* casts (the spec refuses `readable: false` beside a `when`).
*/
supersededBy: string[];
}

/**
Expand Down Expand Up @@ -525,13 +537,20 @@ export class PermissionEvaluator {
* [#25866] The row-conditioned entries the sets declare on `objectName`,
* for a reader that holds no row (`os explain`): each is reported AS a
* row-conditioned lock — judged per row by the write gate, applied as its
* lock everywhere else — rather than folded silently into the mask.
* lock everywhere else — rather than folded silently into the mask. Each
* entry names the sets whose Modify All vote supersedes it (`supersededBy`,
* #29610), so the report never calls a lock binding that the merge overrides.
*/
conditionedFieldEntries(
objectName: string,
permissionSets: PermissionSet[],
): ConditionedFieldEntry[] {
const out: ConditionedFieldEntry[] = [];
// [#29610] The same super-user vote `getFieldPermissions` applies — never a
// second reading of the bits. An `editable` vote overrides every entry.
const supersededBy = superuserFieldVote(permissionSets, objectName)?.editable
? this.superuserBypassSets(objectName, permissionSets, { bit: 'modify' })
: [];
for (const ps of permissionSets) {
if (!ps.fields) continue;
for (const [key, perm] of Object.entries(ps.fields)) {
Expand All @@ -543,6 +562,7 @@ export class PermissionEvaluator {
set: ps.name,
when,
editable: perm.editable === true,
supersededBy: [...supersededBy],
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/plugins/plugin-security/src/security-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2839,10 +2839,10 @@ describe('PermissionEvaluator', () => {
'task.title': { readable: true, editable: true, when: 'done == false' } as any,
});
expect(evaluator.conditionedFieldEntries('contact', [ps])).toEqual([
{ field: 'email', set: 'hr', when: "status == 'approved'", editable: false },
{ field: 'email', set: 'hr', when: "status == 'approved'", editable: false, supersededBy: [] },
]);
expect(evaluator.conditionedFieldEntries('task', [ps])).toEqual([
{ field: 'title', set: 'hr', when: 'done == false', editable: true },
{ field: 'title', set: 'hr', when: 'done == false', editable: true, supersededBy: [] },
]);
});

Expand Down
Loading