Skip to content

Commit 2f3581f

Browse files
os-zhuangclaude
andauthored
feat(lint): warn when a master-detail child has no object-level CRUD grant (#2700) (#2721)
New ADR-0090 D7 security-posture rule `security-master-detail-ungranted` (advisory warning; does not gate the build). A master-detail detail object derives its RECORD-level access from the master (ADR-0055 controlled_by_parent, gate 2), but object-level CRUD is a SEPARATE gate 1 (checkObjectPermission) that is never derived — a permission set that grants the parent but forgets the child denies role-bound non-admin users a 403 before the parent-derived access is ever consulted, the silent "can't submit the subtable" trap (framework#2700, downstream os-tianshun-mtc#43). Flags a non-system detail (has a master_detail field) that NO authored permission set grants (explicit entry or '*' wildcard). Stays silent when the package authors no permission sets, when a package-declared '*' wildcard grant covers every object, or for sys_*/isSystem objects — keeping false positives near zero. The per-set gap and CRUD auto-inheritance are deliberately out of scope (secure-by-default, Salesforce parity). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6c22b12 commit 2f3581f

3 files changed

Lines changed: 237 additions & 1 deletion

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/lint': patch
3+
---
4+
5+
feat(lint): warn when a master-detail child has no object-level CRUD grant (ADR-0090 D7)
6+
7+
New security-posture rule `security-master-detail-ungranted` (advisory
8+
`warning`; it does not gate the build). A master-detail DETAIL object derives
9+
its RECORD-level access from the master (ADR-0055 `controlled_by_parent`,
10+
gate ②), but object-level CRUD is a SEPARATE gate ① (`checkObjectPermission`)
11+
that is never derived — a permission set that grants the parent but forgets the
12+
child denies role-bound non-admin users a 403 before the parent-derived access
13+
is ever consulted, surfacing as the silent "can't fill in / can't submit the
14+
subtable" trap (framework#2700, downstream os-tianshun-mtc#43).
15+
16+
The rule flags a non-system detail (has a `master_detail` field) that NO
17+
authored permission set grants (explicit entry or `'*'` wildcard). It stays
18+
silent when the package authors no permission sets, when a package-declared
19+
`'*'` wildcard grant covers every object, or for `sys_*` / `isSystem` objects —
20+
keeping the false-positive rate near zero. The residual per-set gap (one role
21+
grants it, another forgets it) is intentionally out of scope, and CRUD
22+
auto-inheritance is deliberately NOT adopted (secure-by-default, Salesforce
23+
parity).

packages/lint/src/validate-security-posture.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
SECURITY_ANCHOR_HIGH_PRIVILEGE,
1717
SECURITY_ROLE_WORD,
1818
SECURITY_PRIVATE_NO_READSCOPE,
19+
SECURITY_MASTER_DETAIL_UNGRANTED,
1920
} from './validate-security-posture.js';
2021

2122
const rulesOf = (stack: Record<string, unknown>) =>
@@ -205,3 +206,112 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => {
205206
).toEqual([]);
206207
});
207208
});
209+
210+
// ── Rule: security-master-detail-ungranted (framework#2700) ───────────
211+
describe('validateSecurityPosture · master-detail detail ungranted (framework#2700)', () => {
212+
const mdOnly = (stack: Record<string, unknown>) =>
213+
validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_MASTER_DETAIL_UNGRANTED);
214+
215+
/**
216+
* A work_order (master, granted) + work_order_item (detail). `childGrant`
217+
* grants the child in the same set; `extraPermission` appends another set
218+
* (e.g. an admin wildcard). Omit both → the incident shape: parent granted,
219+
* child forgotten. The master grant carries readScope so it never trips the
220+
* separate private-no-readscope info rule.
221+
*/
222+
const detailStack = (
223+
childGrant?: Record<string, unknown>,
224+
opts?: { extraPermission?: Record<string, unknown> },
225+
): Record<string, unknown> => ({
226+
objects: [
227+
{ name: 'work_order', label: 'Work Order', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } },
228+
{
229+
name: 'work_order_item',
230+
label: 'Work Order Item',
231+
sharingModel: 'controlled_by_parent',
232+
fields: { order: { name: 'order', type: 'master_detail', reference: 'work_order', required: true } },
233+
},
234+
],
235+
permissions: [
236+
{
237+
name: 'field_tech',
238+
label: 'Field Tech',
239+
objects: {
240+
work_order: { allowRead: true, allowCreate: true, allowEdit: true, readScope: 'unit' },
241+
...(childGrant ? { work_order_item: childGrant } : {}),
242+
},
243+
},
244+
...(opts?.extraPermission ? [opts.extraPermission] : []),
245+
],
246+
});
247+
248+
it('warns when the child is granted by no permission set — the incident shape', () => {
249+
const md = mdOnly(detailStack());
250+
expect(md).toHaveLength(1);
251+
expect(md[0]).toMatchObject({ severity: 'warning', where: 'object "work_order_item"' });
252+
expect(md[0].path).toBe('objects[1].fields.order');
253+
expect(md[0].message).toContain('controlled_by_parent');
254+
expect(md[0].message).toContain('403');
255+
expect(md[0].message).toContain('work_order'); // names the master
256+
expect(md[0].hint).toContain('permissions[i].objects.work_order_item');
257+
});
258+
259+
it.each([
260+
['allowRead', { allowRead: true }],
261+
['allowCreate', { allowCreate: true }],
262+
['allowEdit', { allowEdit: true }],
263+
['allowDelete', { allowDelete: true }],
264+
['viewAllRecords (VAMA)', { viewAllRecords: true }],
265+
['modifyAllRecords (VAMA)', { modifyAllRecords: true }],
266+
])('stays silent when the child is granted %s in a set', (_label, grant) => {
267+
expect(mdOnly(detailStack(grant))).toEqual([]);
268+
});
269+
270+
it("stays silent when a package-declared '*' wildcard grant covers every object", () => {
271+
expect(
272+
mdOnly(detailStack(undefined, { extraPermission: { name: 'app_admin', objects: { '*': { allowRead: true } } } })),
273+
).toEqual([]);
274+
});
275+
276+
it('does not fire when the package authors no permission sets (nothing to compare)', () => {
277+
const { permissions: _drop, ...noPerms } = detailStack();
278+
expect(mdOnly(noPerms)).toEqual([]);
279+
});
280+
281+
it('ignores a non-detail object that is merely ungranted (lookup, not master_detail)', () => {
282+
expect(
283+
mdOnly({
284+
objects: [
285+
{ name: 'work_order', label: 'Work Order', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } },
286+
{ name: 'lonely', label: 'Lonely', sharingModel: 'private', fields: { ref: { name: 'ref', type: 'lookup', reference: 'work_order' } } },
287+
],
288+
permissions: [{ name: 'u', objects: { work_order: { allowRead: true, readScope: 'unit' } } }],
289+
}),
290+
).toEqual([]);
291+
});
292+
293+
it('exempts system detail objects (sys_ prefix or isSystem:true)', () => {
294+
expect(
295+
mdOnly({
296+
objects: [
297+
{ name: 'thing', label: 'Thing', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } },
298+
{ name: 'sys_thing_audit', sharingModel: 'controlled_by_parent', fields: { thing: { name: 'thing', type: 'master_detail', reference: 'thing' } } },
299+
{ name: 'thing_internal', isSystem: true, sharingModel: 'controlled_by_parent', fields: { thing: { name: 'thing', type: 'master_detail', reference: 'thing' } } },
300+
],
301+
permissions: [{ name: 'u', objects: { thing: { allowRead: true, readScope: 'unit' } } }],
302+
}),
303+
).toEqual([]);
304+
});
305+
306+
it('detects detail objects declared with the array field form', () => {
307+
const md = mdOnly({
308+
objects: [
309+
{ name: 'work_order', label: 'Work Order', sharingModel: 'private', fields: [{ name: 'name', label: 'Name' }] },
310+
{ name: 'work_order_item', sharingModel: 'controlled_by_parent', fields: [{ name: 'order', type: 'master_detail', reference: 'work_order', required: true }] },
311+
],
312+
permissions: [{ name: 'u', objects: { work_order: { allowRead: true, readScope: 'unit' } } }],
313+
});
314+
expect(md).toHaveLength(1);
315+
expect(md[0].where).toBe('object "work_order_item"');
316+
});
317+
});

packages/lint/src/validate-security-posture.ts

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@
1515
* | security-anchor-high-privilege(error) | ADR-0090 D5/D9 anchors |
1616
* | security-role-word (error) | ADR-0090 D3 vocabulary freeze |
1717
* | security-private-no-readscope (info) | admin-intent mismatch class |
18+
* | security-master-detail-ungranted(warn) | framework#2700 os-tianshun-mtc#43|
1819
*
1920
* Per ADR-0049 discipline these are NOT advisory security: every `error` rule
2021
* mirrors a runtime enforcement point (D1 fail-closed OWD default, D4 zod
2122
* enum + fail-closed evaluator, D5/D9 anchor binding gate, D3 rename wave) —
22-
* the lint moves the failure from runtime-deny to author-time fix-it.
23+
* the lint moves the failure from runtime-deny to author-time fix-it. The lone
24+
* `warning` (master-detail-ungranted) likewise mirrors a runtime gate — the
25+
* object-level CRUD check (ADR-0055) — but stays advisory: it flags a *likely*
26+
* misconfiguration whose per-permission-set nuance it cannot fully adjudicate.
2327
*
2428
* Pure `(stack) => Finding[]`; accepts the NORMALIZED stack input (works both
2529
* pre- and post-zod-parse, so `os lint` catches what the zod gate would
@@ -35,6 +39,7 @@ export const SECURITY_WILDCARD_VAMA = 'security-wildcard-vama';
3539
export const SECURITY_ANCHOR_HIGH_PRIVILEGE = 'security-anchor-high-privilege';
3640
export const SECURITY_ROLE_WORD = 'security-role-word';
3741
export const SECURITY_PRIVATE_NO_READSCOPE = 'security-private-no-readscope';
42+
export const SECURITY_MASTER_DETAIL_UNGRANTED = 'security-master-detail-ungranted';
3843

3944
export type SecuritySeverity = 'error' | 'warning' | 'info';
4045

@@ -101,6 +106,44 @@ function labelHasRoleWord(label: unknown): boolean {
101106
return /\brole(s)?\b/i.test(label);
102107
}
103108

109+
/** The `reference`/`reference_to` target a relationship field points at. */
110+
function refOf(def: AnyRec): string | undefined {
111+
const r = (def.reference ?? def.reference_to) as unknown;
112+
return typeof r === 'string' && r ? r : undefined;
113+
}
114+
115+
/**
116+
* The first `master_detail` field on an object, if any — its presence is what
117+
* makes the object a DETAIL (the child side of a master-detail; ADR-0055).
118+
* Works for both the array and name-keyed-map field forms (`asArray` folds the
119+
* map key into `name`).
120+
*/
121+
function firstMasterDetailField(obj: AnyRec): { name: string; parent?: string } | undefined {
122+
for (const f of asArray(obj.fields)) {
123+
if (f.type === 'master_detail') {
124+
return { name: String(f.name ?? '?'), parent: refOf(f) };
125+
}
126+
}
127+
return undefined;
128+
}
129+
130+
/**
131+
* Does a per-object permission entry open the object-level CRUD gate at all?
132+
* Any of the four CRUD bits, or a super-user bypass (View/Modify All Data),
133+
* counts — this mirrors the runtime `checkObjectPermission` gate (ADR-0066 D2):
134+
* that gate returns true if ANY set contributes one of these for the object.
135+
*/
136+
function grantsObjectAccess(p: AnyRec): boolean {
137+
return (
138+
p.allowRead === true ||
139+
p.allowCreate === true ||
140+
p.allowEdit === true ||
141+
p.allowDelete === true ||
142+
p.viewAllRecords === true ||
143+
p.modifyAllRecords === true
144+
);
145+
}
146+
104147
/**
105148
* Validate the security posture of a stack. Returns findings (empty = clean).
106149
* `error` findings gate the build in `os compile`; `info` is advisory.
@@ -335,5 +378,65 @@ export function validateSecurityPosture(stack: AnyRec): SecurityFinding[] {
335378
}
336379
}
337380

381+
// ── ADR-0055: master-detail DETAIL object with no object-level CRUD ───
382+
// A master-detail CHILD derives its RECORD-level scope from the master
383+
// (`controlled_by_parent`) — but that is gate ②. Object-level CRUD is a
384+
// SEPARATE gate ① (`checkObjectPermission`) that is NEVER derived: a set that
385+
// lists the parent but forgets the child denies role-bound non-admin users a
386+
// 403 *before* the parent-derived access is ever consulted, surfacing as the
387+
// silent "can't fill in / can't submit the subtable" trap (framework#2700,
388+
// downstream os-tianshun-mtc#43). Statically detectable: a detail (has a
389+
// master_detail field) that NO authored permission set grants.
390+
//
391+
// Advisory `warning` — it does not gate the build. Two deliberate silences
392+
// keep the false-positive rate near zero: (a) if the package authors no
393+
// permission sets there is nothing to compare against, and (b) a package-
394+
// declared `'*'` wildcard grant is treated as covering every object (a broad
395+
// grant is an explicit choice — suppress rather than cry wolf). The residual
396+
// per-set gap (one role grants it, another forgets it) is intentionally out
397+
// of scope (issue #2700); the platform's own default admin set lives outside
398+
// the linted stack, so it never masks a package that forgot the child here.
399+
if (permissionSets.length > 0) {
400+
const wildcardGrantsAll = permissionSets.some((ps) =>
401+
grantsObjectAccess(((ps.objects as AnyRec | undefined)?.['*'] ?? {}) as AnyRec),
402+
);
403+
if (!wildcardGrantsAll) {
404+
const grantedObjects = new Set<string>();
405+
for (const ps of permissionSets) {
406+
const objectsMap = (ps.objects && typeof ps.objects === 'object' ? ps.objects : {}) as AnyRec;
407+
for (const [objName, rawPerm] of Object.entries(objectsMap)) {
408+
if (objName === '*') continue;
409+
if (grantsObjectAccess((rawPerm ?? {}) as AnyRec)) grantedObjects.add(objName);
410+
}
411+
}
412+
for (let i = 0; i < objects.length; i++) {
413+
const obj = objects[i];
414+
if (!obj || typeof obj !== 'object' || isSystemObject(obj)) continue;
415+
const objName = typeof obj.name === 'string' ? obj.name : '';
416+
if (!objName || grantedObjects.has(objName)) continue;
417+
const md = firstMasterDetailField(obj);
418+
if (!md) continue;
419+
const parentText = md.parent ? ` → "${md.parent}"` : '';
420+
findings.push({
421+
severity: 'warning',
422+
rule: SECURITY_MASTER_DETAIL_UNGRANTED,
423+
where: `object "${objName}"`,
424+
path: `objects[${i}].fields.${md.name}`,
425+
message:
426+
`detail object "${objName}" (master_detail "${md.name}"${parentText}) has no object-level ` +
427+
`CRUD grant in any permission set. A master-detail child derives its RECORD-level access ` +
428+
`from the master (ADR-0055 controlled_by_parent), but object-level CRUD is a SEPARATE gate ` +
429+
`that is never derived — role-bound non-admin users are denied (403) before the ` +
430+
`parent-derived access is ever consulted (the silent "can't submit the subtable" trap).`,
431+
hint:
432+
`Grant "${objName}" in at least one permission set that already grants its master` +
433+
`${md.parent ? ` "${md.parent}"` : ''} — e.g. permissions[i].objects.${objName} = ` +
434+
`{ allowRead: true, allowCreate: true, allowEdit: true }. If no role should ever touch ` +
435+
`it (a pure system/internal table), name it sys_* or set isSystem: true.`,
436+
});
437+
}
438+
}
439+
}
440+
338441
return findings;
339442
}

0 commit comments

Comments
 (0)