diff --git a/.changeset/lint-master-detail-ungranted-crud.md b/.changeset/lint-master-detail-ungranted-crud.md new file mode 100644 index 0000000000..412c84f68c --- /dev/null +++ b/.changeset/lint-master-detail-ungranted-crud.md @@ -0,0 +1,23 @@ +--- +'@objectstack/lint': patch +--- + +feat(lint): warn when a master-detail child has no object-level CRUD grant (ADR-0090 D7) + +New security-posture rule `security-master-detail-ungranted` (advisory +`warning`; it does not gate the build). A master-detail DETAIL object derives +its RECORD-level access from the master (ADR-0055 `controlled_by_parent`, +gate ②), but object-level CRUD is a SEPARATE gate ① (`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, surfacing as the silent "can't fill in / can't submit the +subtable" trap (framework#2700, downstream os-tianshun-mtc#43). + +The rule flags a non-system detail (has a `master_detail` field) that NO +authored permission set grants (explicit entry or `'*'` wildcard). It stays +silent when the package authors no permission sets, when a package-declared +`'*'` wildcard grant covers every object, or for `sys_*` / `isSystem` objects — +keeping the false-positive rate near zero. The residual per-set gap (one role +grants it, another forgets it) is intentionally out of scope, and CRUD +auto-inheritance is deliberately NOT adopted (secure-by-default, Salesforce +parity). diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index 240bed8429..042c77d65d 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -16,6 +16,7 @@ import { SECURITY_ANCHOR_HIGH_PRIVILEGE, SECURITY_ROLE_WORD, SECURITY_PRIVATE_NO_READSCOPE, + SECURITY_MASTER_DETAIL_UNGRANTED, } from './validate-security-posture.js'; const rulesOf = (stack: Record) => @@ -205,3 +206,112 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => { ).toEqual([]); }); }); + +// ── Rule: security-master-detail-ungranted (framework#2700) ─────────── +describe('validateSecurityPosture · master-detail detail ungranted (framework#2700)', () => { + const mdOnly = (stack: Record) => + validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_MASTER_DETAIL_UNGRANTED); + + /** + * A work_order (master, granted) + work_order_item (detail). `childGrant` + * grants the child in the same set; `extraPermission` appends another set + * (e.g. an admin wildcard). Omit both → the incident shape: parent granted, + * child forgotten. The master grant carries readScope so it never trips the + * separate private-no-readscope info rule. + */ + const detailStack = ( + childGrant?: Record, + opts?: { extraPermission?: Record }, + ): Record => ({ + objects: [ + { name: 'work_order', label: 'Work Order', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } }, + { + name: 'work_order_item', + label: 'Work Order Item', + sharingModel: 'controlled_by_parent', + fields: { order: { name: 'order', type: 'master_detail', reference: 'work_order', required: true } }, + }, + ], + permissions: [ + { + name: 'field_tech', + label: 'Field Tech', + objects: { + work_order: { allowRead: true, allowCreate: true, allowEdit: true, readScope: 'unit' }, + ...(childGrant ? { work_order_item: childGrant } : {}), + }, + }, + ...(opts?.extraPermission ? [opts.extraPermission] : []), + ], + }); + + it('warns when the child is granted by no permission set — the incident shape', () => { + const md = mdOnly(detailStack()); + expect(md).toHaveLength(1); + expect(md[0]).toMatchObject({ severity: 'warning', where: 'object "work_order_item"' }); + expect(md[0].path).toBe('objects[1].fields.order'); + expect(md[0].message).toContain('controlled_by_parent'); + expect(md[0].message).toContain('403'); + expect(md[0].message).toContain('work_order'); // names the master + expect(md[0].hint).toContain('permissions[i].objects.work_order_item'); + }); + + it.each([ + ['allowRead', { allowRead: true }], + ['allowCreate', { allowCreate: true }], + ['allowEdit', { allowEdit: true }], + ['allowDelete', { allowDelete: true }], + ['viewAllRecords (VAMA)', { viewAllRecords: true }], + ['modifyAllRecords (VAMA)', { modifyAllRecords: true }], + ])('stays silent when the child is granted %s in a set', (_label, grant) => { + expect(mdOnly(detailStack(grant))).toEqual([]); + }); + + it("stays silent when a package-declared '*' wildcard grant covers every object", () => { + expect( + mdOnly(detailStack(undefined, { extraPermission: { name: 'app_admin', objects: { '*': { allowRead: true } } } })), + ).toEqual([]); + }); + + it('does not fire when the package authors no permission sets (nothing to compare)', () => { + const { permissions: _drop, ...noPerms } = detailStack(); + expect(mdOnly(noPerms)).toEqual([]); + }); + + it('ignores a non-detail object that is merely ungranted (lookup, not master_detail)', () => { + expect( + mdOnly({ + objects: [ + { name: 'work_order', label: 'Work Order', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } }, + { name: 'lonely', label: 'Lonely', sharingModel: 'private', fields: { ref: { name: 'ref', type: 'lookup', reference: 'work_order' } } }, + ], + permissions: [{ name: 'u', objects: { work_order: { allowRead: true, readScope: 'unit' } } }], + }), + ).toEqual([]); + }); + + it('exempts system detail objects (sys_ prefix or isSystem:true)', () => { + expect( + mdOnly({ + objects: [ + { name: 'thing', label: 'Thing', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } }, + { name: 'sys_thing_audit', sharingModel: 'controlled_by_parent', fields: { thing: { name: 'thing', type: 'master_detail', reference: 'thing' } } }, + { name: 'thing_internal', isSystem: true, sharingModel: 'controlled_by_parent', fields: { thing: { name: 'thing', type: 'master_detail', reference: 'thing' } } }, + ], + permissions: [{ name: 'u', objects: { thing: { allowRead: true, readScope: 'unit' } } }], + }), + ).toEqual([]); + }); + + it('detects detail objects declared with the array field form', () => { + const md = mdOnly({ + objects: [ + { name: 'work_order', label: 'Work Order', sharingModel: 'private', fields: [{ name: 'name', label: 'Name' }] }, + { name: 'work_order_item', sharingModel: 'controlled_by_parent', fields: [{ name: 'order', type: 'master_detail', reference: 'work_order', required: true }] }, + ], + permissions: [{ name: 'u', objects: { work_order: { allowRead: true, readScope: 'unit' } } }], + }); + expect(md).toHaveLength(1); + expect(md[0].where).toBe('object "work_order_item"'); + }); +}); diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index 27172f89de..9c997dd299 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -15,11 +15,15 @@ * | security-anchor-high-privilege(error) | ADR-0090 D5/D9 anchors | * | security-role-word (error) | ADR-0090 D3 vocabulary freeze | * | security-private-no-readscope (info) | admin-intent mismatch class | + * | security-master-detail-ungranted(warn) | framework#2700 os-tianshun-mtc#43| * * Per ADR-0049 discipline these are NOT advisory security: every `error` rule * mirrors a runtime enforcement point (D1 fail-closed OWD default, D4 zod * enum + fail-closed evaluator, D5/D9 anchor binding gate, D3 rename wave) — - * the lint moves the failure from runtime-deny to author-time fix-it. + * the lint moves the failure from runtime-deny to author-time fix-it. The lone + * `warning` (master-detail-ungranted) likewise mirrors a runtime gate — the + * object-level CRUD check (ADR-0055) — but stays advisory: it flags a *likely* + * misconfiguration whose per-permission-set nuance it cannot fully adjudicate. * * Pure `(stack) => Finding[]`; accepts the NORMALIZED stack input (works both * 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'; export const SECURITY_ANCHOR_HIGH_PRIVILEGE = 'security-anchor-high-privilege'; export const SECURITY_ROLE_WORD = 'security-role-word'; export const SECURITY_PRIVATE_NO_READSCOPE = 'security-private-no-readscope'; +export const SECURITY_MASTER_DETAIL_UNGRANTED = 'security-master-detail-ungranted'; export type SecuritySeverity = 'error' | 'warning' | 'info'; @@ -101,6 +106,44 @@ function labelHasRoleWord(label: unknown): boolean { return /\brole(s)?\b/i.test(label); } +/** The `reference`/`reference_to` target a relationship field points at. */ +function refOf(def: AnyRec): string | undefined { + const r = (def.reference ?? def.reference_to) as unknown; + return typeof r === 'string' && r ? r : undefined; +} + +/** + * The first `master_detail` field on an object, if any — its presence is what + * makes the object a DETAIL (the child side of a master-detail; ADR-0055). + * Works for both the array and name-keyed-map field forms (`asArray` folds the + * map key into `name`). + */ +function firstMasterDetailField(obj: AnyRec): { name: string; parent?: string } | undefined { + for (const f of asArray(obj.fields)) { + if (f.type === 'master_detail') { + return { name: String(f.name ?? '?'), parent: refOf(f) }; + } + } + return undefined; +} + +/** + * Does a per-object permission entry open the object-level CRUD gate at all? + * Any of the four CRUD bits, or a super-user bypass (View/Modify All Data), + * counts — this mirrors the runtime `checkObjectPermission` gate (ADR-0066 D2): + * that gate returns true if ANY set contributes one of these for the object. + */ +function grantsObjectAccess(p: AnyRec): boolean { + return ( + p.allowRead === true || + p.allowCreate === true || + p.allowEdit === true || + p.allowDelete === true || + p.viewAllRecords === true || + p.modifyAllRecords === true + ); +} + /** * Validate the security posture of a stack. Returns findings (empty = clean). * `error` findings gate the build in `os compile`; `info` is advisory. @@ -335,5 +378,65 @@ export function validateSecurityPosture(stack: AnyRec): SecurityFinding[] { } } + // ── ADR-0055: master-detail DETAIL object with no object-level CRUD ─── + // A master-detail CHILD derives its RECORD-level scope from the master + // (`controlled_by_parent`) — but that is gate ②. Object-level CRUD is a + // SEPARATE gate ① (`checkObjectPermission`) that is NEVER derived: a set that + // lists the parent but forgets the child denies role-bound non-admin users a + // 403 *before* the parent-derived access is ever consulted, surfacing as the + // silent "can't fill in / can't submit the subtable" trap (framework#2700, + // downstream os-tianshun-mtc#43). Statically detectable: a detail (has a + // master_detail field) that NO authored permission set grants. + // + // Advisory `warning` — it does not gate the build. Two deliberate silences + // keep the false-positive rate near zero: (a) if the package authors no + // permission sets there is nothing to compare against, and (b) a package- + // declared `'*'` wildcard grant is treated as covering every object (a broad + // grant is an explicit choice — suppress rather than cry wolf). The residual + // per-set gap (one role grants it, another forgets it) is intentionally out + // of scope (issue #2700); the platform's own default admin set lives outside + // the linted stack, so it never masks a package that forgot the child here. + if (permissionSets.length > 0) { + const wildcardGrantsAll = permissionSets.some((ps) => + grantsObjectAccess(((ps.objects as AnyRec | undefined)?.['*'] ?? {}) as AnyRec), + ); + if (!wildcardGrantsAll) { + const grantedObjects = new Set(); + for (const ps of permissionSets) { + const objectsMap = (ps.objects && typeof ps.objects === 'object' ? ps.objects : {}) as AnyRec; + for (const [objName, rawPerm] of Object.entries(objectsMap)) { + if (objName === '*') continue; + if (grantsObjectAccess((rawPerm ?? {}) as AnyRec)) grantedObjects.add(objName); + } + } + for (let i = 0; i < objects.length; i++) { + const obj = objects[i]; + if (!obj || typeof obj !== 'object' || isSystemObject(obj)) continue; + const objName = typeof obj.name === 'string' ? obj.name : ''; + if (!objName || grantedObjects.has(objName)) continue; + const md = firstMasterDetailField(obj); + if (!md) continue; + const parentText = md.parent ? ` → "${md.parent}"` : ''; + findings.push({ + severity: 'warning', + rule: SECURITY_MASTER_DETAIL_UNGRANTED, + where: `object "${objName}"`, + path: `objects[${i}].fields.${md.name}`, + message: + `detail object "${objName}" (master_detail "${md.name}"${parentText}) has no object-level ` + + `CRUD grant in any permission set. A master-detail child derives its RECORD-level access ` + + `from the master (ADR-0055 controlled_by_parent), but object-level CRUD is a SEPARATE gate ` + + `that is never derived — role-bound non-admin users are denied (403) before the ` + + `parent-derived access is ever consulted (the silent "can't submit the subtable" trap).`, + hint: + `Grant "${objName}" in at least one permission set that already grants its master` + + `${md.parent ? ` "${md.parent}"` : ''} — e.g. permissions[i].objects.${objName} = ` + + `{ allowRead: true, allowCreate: true, allowEdit: true }. If no role should ever touch ` + + `it (a pure system/internal table), name it sys_* or set isSystem: true.`, + }); + } + } + } + return findings; }