diff --git a/.changeset/adr-0105-d6-unit-and-subordinates.md b/.changeset/adr-0105-d6-unit-and-subordinates.md new file mode 100644 index 0000000000..f07384b246 --- /dev/null +++ b/.changeset/adr-0105-d6-unit-and-subordinates.md @@ -0,0 +1,34 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): 扩 ADR-0105 D6 ② 的收件人词表至 ADR 原文范围 —— `unit_and_subordinates` +也判红 + +`org-axis-cross-org-bu-grant`(D6 ②)此前只对 `sharedWith.type === +'business_unit'` 判红,而授权面**更大**的另一个业务单元收件人 +`unit_and_subordinates`(一个 BU **加上其全部后代单元**,ADR-0057 D5 子树扩张) +直接放行。两者的缺陷完全相同:平台级对象(`tenancy.enabled: false` / +`systemFields.tenant: false`)没有 organization 列可供 Layer 0 收口,BU 子树没有 +任何 organization 可供解析,授权因而跨到库里每一个 organization —— 正是 ADR 拒绝 +的"跨 org BU 巨树",从后门到达。 + +漏掉的恰恰是 ADR-0105 D6 ② 自己点名的那一个: + +> Every BU mechanism — `unit_and_subordinates` sharing, `adminScope` +> delegation, depth scopes — operates within one organization. There is no +> cross-org tree. + +判定改为收件人类型 ∈ `{ business_unit, unit_and_subordinates }`,诊断信息里点名 +**实际写下的**类型并说明其触及范围(子树那一个额外写明 "AND every descendant +unit"),修复建议改为指向三个扁平收件人。 + +词表与 spec 枚举 `ShareRecipientType` 的差集不再是隐式的:规则里以表格逐条写明 +拦截二者、放行 `user` / `team` / `position` 的理由(它们的运行时展开都不经 +`BusinessUnitGraphService`,是 `tenancy.enabled: false` 平台级目录**被设计用来** +共享的方式),并附一条测试断言两半恰好划分 `ShareRecipientType` —— 将来枚举加成员 +会在词表处失败,而不是无声地落进没人选过的那一桶。#4991 正是这条断言缺席的产物。 + +这是 error 级门禁的扩张,因此复核了真实元数据:`examples/app-showcase` / +`app-crm` 是仓库里仅有的已声明 sharing rule(共 11 条),全仓无任何对象关掉 +tenancy,扩张后 org-axis 红线数为 **0** —— 不产生新红。 diff --git a/packages/lint/src/validate-org-axis-red-lines.test.ts b/packages/lint/src/validate-org-axis-red-lines.test.ts index b4f823c86d..e83f2391c2 100644 --- a/packages/lint/src/validate-org-axis-red-lines.test.ts +++ b/packages/lint/src/validate-org-axis-red-lines.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest'; import { ObjectSchema } from '@objectstack/spec/data'; -import { SharingRuleSchema, type SharingRuleInput } from '@objectstack/spec/security'; +import { ShareRecipientType, SharingRuleSchema, type SharingRuleInput } from '@objectstack/spec/security'; import { validateOrgAxisRedLines, @@ -284,8 +284,21 @@ describe('validateOrgAxisRedLines — ① no permission inheritance on the org a }); }); +/** + * ── Rule ②'s recipient word list ──────────────────────────────────────────── + * + * The two BU-tree recipients ② intercepts, and the three it deliberately lets + * past. Split out here because the drift guard below asserts the two halves + * partition `ShareRecipientType` exactly — the check whose absence is #4991. + */ +const BU_TREE_RECIPIENTS = ['business_unit', 'unit_and_subordinates'] as const; +const FLAT_RECIPIENTS = ['user', 'team', 'position'] as const; + describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal', () => { - const platformGlobalStack = (tenancy: unknown) => ({ + const platformGlobalStack = ( + tenancy: unknown, + recipientType: string = 'business_unit', + ) => ({ objects: [ objectFixture( tenancy === undefined @@ -298,24 +311,60 @@ describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal' name: 'catalog_to_plant', type: 'criteria', object: 'material_catalog', - sharedWith: { type: 'business_unit', value: 'bu_plant_a' }, + sharedWith: { type: recipientType, value: 'bu_plant_a' }, condition: 'true', - }), + } as unknown as SharingRuleInput), ], }); - it('flags a business-unit grant on a `tenancy.enabled: false` object', () => { - const findings = validateOrgAxisRedLines(platformGlobalStack({ enabled: false })); - expect(findings).toHaveLength(1); - expect(findings[0]).toMatchObject({ - severity: 'error', - rule: ORG_AXIS_CROSS_ORG_BU_GRANT, - path: 'sharingRules[0].sharedWith', - }); - expect(findings[0].message).toMatch(/spans EVERY organization/); + /** + * The word list ② enforces must PARTITION the authoring enum: every member of + * `ShareRecipientType` is either intercepted as a BU-tree recipient or named + * in the allowed half with a reason in the rule's own comment. No third + * bucket, no silent remainder. + * + * This is the guard #4991 is the absence of. ② shipped naming a single + * recipient, `business_unit`, while ADR-0105 D6 ②'s own sentence names + * `unit_and_subordinates` — the strictly WIDER grant (a BU plus every + * descendant unit) sailed past the gate that stopped the narrower one. A + * sixth enum member added tomorrow fails HERE, at the vocabulary, instead of + * quietly inheriting whichever bucket nobody chose for it. + */ + it('partitions `ShareRecipientType` — no recipient is unaccounted for (#4991)', () => { + const declared = [...ShareRecipientType.options].sort(); + const accounted = [...BU_TREE_RECIPIENTS, ...FLAT_RECIPIENTS].sort(); + expect(accounted).toEqual(declared); }); - it('flags the `systemFields.tenant: false` spelling of the same opt-out', () => { + it.each(BU_TREE_RECIPIENTS)( + 'flags a `%s` grant on a `tenancy.enabled: false` object', + (recipientType) => { + const findings = validateOrgAxisRedLines(platformGlobalStack({ enabled: false }, recipientType)); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: ORG_AXIS_CROSS_ORG_BU_GRANT, + path: 'sharingRules[0].sharedWith', + }); + expect(findings[0].message).toMatch(/spans EVERY organization/); + // The diagnostic names the recipient actually written, not a generic + // "business-unit rule" the author then has to go match up themselves. + expect(findings[0].message).toContain(`\`${recipientType}\``); + }, + ); + + it('spells out that `unit_and_subordinates` reaches the whole subtree', () => { + // The two recipients share a defect but not a blast radius: this one is the + // BU plus every descendant unit (ADR-0057 D5), so the message says so. + const [finding] = validateOrgAxisRedLines( + platformGlobalStack({ enabled: false }, 'unit_and_subordinates'), + ); + expect(finding.message).toMatch(/AND every descendant unit/); + const [narrow] = validateOrgAxisRedLines(platformGlobalStack({ enabled: false }, 'business_unit')); + expect(narrow.message).not.toMatch(/descendant/); + }); + + it('flags `unit_and_subordinates` under the `systemFields.tenant: false` spelling too', () => { expect( rules({ objects: [objectFixture({ name: 'material_catalog', systemFields: { tenant: false } })], @@ -324,7 +373,7 @@ describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal' name: 'r', type: 'criteria', object: 'material_catalog', - sharedWith: { type: 'business_unit', value: 'bu' }, + sharedWith: { type: 'unit_and_subordinates', value: 'bu_field_ops' }, condition: 'true', }), ], @@ -332,27 +381,60 @@ describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal' ).toEqual([ORG_AXIS_CROSS_ORG_BU_GRANT]); }); - it('allows a business-unit grant on an ORG-SCOPED object (the normal case)', () => { - expect(rules(platformGlobalStack({ enabled: true }))).toEqual([]); - expect(rules(platformGlobalStack(undefined))).toEqual([]); + it('allows `unit_and_subordinates` on an ORG-SCOPED object (the showcase shape)', () => { + // `share_new_inquiries_with_field_ops` → `showcase_inquiry` in + // examples/app-showcase: a real, correct subtree grant. The widened word + // list must not turn the sanctioned intra-org case red. + expect(rules(platformGlobalStack({ enabled: true }, 'unit_and_subordinates'))).toEqual([]); + expect(rules(platformGlobalStack(undefined, 'unit_and_subordinates'))).toEqual([]); }); - it('allows a non-BU audience on a platform-global object', () => { + it('flags the `systemFields.tenant: false` spelling of the same opt-out', () => { expect( rules({ - objects: [objectFixture({ name: 'material_catalog', tenancy: { enabled: false } })], + objects: [objectFixture({ name: 'material_catalog', systemFields: { tenant: false } })], sharingRules: [ sharingRule({ name: 'r', type: 'criteria', object: 'material_catalog', - sharedWith: { type: 'position', value: 'buyer' }, + sharedWith: { type: 'business_unit', value: 'bu' }, condition: 'true', }), ], }), - ).toEqual([]); + ).toEqual([ORG_AXIS_CROSS_ORG_BU_GRANT]); }); + + it('allows a business-unit grant on an ORG-SCOPED object (the normal case)', () => { + expect(rules(platformGlobalStack({ enabled: true }))).toEqual([]); + expect(rules(platformGlobalStack(undefined))).toEqual([]); + }); + + it.each(FLAT_RECIPIENTS)( + 'allows the flat `%s` audience on a platform-global object (the sanctioned path)', + (recipientType) => { + // These three expand with no business-unit tree involved — `user` not at + // all, `team` via `TeamGraphService`, `position` flat over holders + // (ADR-0090 D3). Sharing a platform-global catalog to them is what + // `tenancy.enabled: false` is FOR; ② forbids resolving a BU subtree with + // no organization to resolve it within, not sharing a global object. + expect( + rules({ + objects: [objectFixture({ name: 'material_catalog', tenancy: { enabled: false } })], + sharingRules: [ + sharingRule({ + name: 'r', + type: 'criteria', + object: 'material_catalog', + sharedWith: { type: recipientType, value: 'buyer' }, + condition: 'true', + } as unknown as SharingRuleInput), + ], + }), + ).toEqual([]); + }, + ); }); describe('validateOrgAxisRedLines — input tolerance', () => { diff --git a/packages/lint/src/validate-org-axis-red-lines.ts b/packages/lint/src/validate-org-axis-red-lines.ts index 90408acfd4..7d0fa5d052 100644 --- a/packages/lint/src/validate-org-axis-red-lines.ts +++ b/packages/lint/src/validate-org-axis-red-lines.ts @@ -29,7 +29,10 @@ * business-unit sharing rule on a PLATFORM-GLOBAL object (`tenancy.enabled: * false`) has no organization column to scope against, so the grant spans every * organization in the database — a cross-org BU grant by construction, and the - * "cross-org BU mega-tree" the ADR rejected, arrived at by accident. + * "cross-org BU mega-tree" the ADR rejected, arrived at by accident. It covers + * BOTH business-unit recipients, `business_unit` and `unit_and_subordinates` + * — see {@link BU_TREE_RECIPIENT_TYPES} for the word list and its deliberate + * complement. * * Both are `error`, per ADR-0049 discipline: each mirrors a real enforcement * property (the Layer 0 wall's independence; the org-predicated BU resolver), @@ -62,6 +65,41 @@ type AnyRec = Record; /** The org-axis grouping reference. Reporting only — never an authorization input. */ const ORG_PARENT_FIELD = 'parent_organization_id'; +/** + * The sharing-rule recipients rule ② intercepts: the ones whose runtime + * expansion IS the business-unit tree. + * + * Cross-checked word-for-word against the authoring enum `ShareRecipientType` + * (`@objectstack/spec/security`, `sharing.zod.ts`) — the only vocabulary an + * author can write, since `sharedWith` is `.strict()` and rejects everything + * else by name. That enum has FIVE members; this list intercepts two, and the + * difference is deliberate, not an oversight (it is exactly the oversight + * #4991 was filed for — ② shipped naming only `business_unit` while ADR-0105 + * D6 ②'s own text names `unit_and_subordinates`): + * + * | `ShareRecipientType` | ② | Why | + * |-------------------------|---|--------------------------------------------| + * | `business_unit` | ✅ | Members of exactly one BU — `BusinessUnitGraphService`, org-predicated. | + * | `unit_and_subordinates` | ✅ | That BU **plus every descendant unit** (ADR-0057 D5 subtree widening) — same resolver, strictly WIDER grant. | + * | `user` | — | A literal user id, no expansion at all. No tree to resolve, so no org to resolve it in. | + * | `team` | — | `sys_team` is a FLAT collaboration grouping (ADR-0090 D3 renamed `group` → `team`); `TeamGraphService`, not the BU graph. | + * | `position` | — | Flat holder expansion (ADR-0090 D3 finalized the retirement of the position hierarchy); `PositionGraphService`, not the BU graph. The BU *depth scopes* D6 ② also names are a SCOPE mechanism, not a sharing-rule recipient. | + * + * The three allowed recipients are the sanctioned way to share a + * platform-global object (ADR-0066): naming a user, a flat team, or a flat + * position audience grants those people the catalog, which is the entire point + * of `tenancy.enabled: false`. What ② forbids is not "sharing a global object" + * but "resolving a BU SUBTREE with no organization to resolve it within". + * + * The runtime contract `SharingRuleRecipientType` + * (`spec/contracts/sharing-service.ts`) additionally carries `queue`; it is + * deliberately NOT authorable ("no `sys_queue` yet") and `expandRecipient` + * returns `[]` for it, so no author can reach it and it grants nothing. If it + * ever becomes authorable it is a work-distribution list, not a BU node — but + * this table is the place to re-decide that, in `ShareRecipientType` order. + */ +const BU_TREE_RECIPIENT_TYPES = new Set(['business_unit', 'unit_and_subordinates']); + /** Coerce a collection (array or name-keyed map) to an array of records. */ function asArray(v: unknown): AnyRec[] { if (Array.isArray(v)) return v as AnyRec[]; @@ -199,8 +237,10 @@ export function validateOrgAxisRedLines(stack: unknown): OrgAxisFinding[] { // ── ② Business-unit trees remain org-internal ───────────────────────────── // - // A `business_unit` recipient on a platform-global object has no organization + // A business-unit recipient on a platform-global object has no organization // column to scope against, so the grant reaches every organization's rows. + // Both BU recipients count — see `BU_TREE_RECIPIENT_TYPES` for why that word + // list is two long and which three of `ShareRecipientType` it lets past. const tenancyDisabledObjects = new Set( objects.filter((o) => isTenancyDisabled(o)).map((o) => str(o.name)).filter(Boolean), ); @@ -211,21 +251,29 @@ export function validateOrgAxisRedLines(stack: unknown): OrgAxisFinding[] { // spelt out only in the schema's rejection message (#4984). const sharedWith = rule.sharedWith as AnyRec | undefined; const recipientType = str(sharedWith?.type); - if (recipientType !== 'business_unit') return; + if (!BU_TREE_RECIPIENT_TYPES.has(recipientType)) return; + // `unit_and_subordinates` is the strictly wider of the two — it is the BU + // named plus every descendant unit — so say which one was written rather + // than a generic "business-unit rule" the author has to go look up. + const reach = + recipientType === 'unit_and_subordinates' + ? 'a business unit AND every descendant unit' + : 'a business unit'; findings.push({ severity: 'error', rule: ORG_AXIS_CROSS_ORG_BU_GRANT, where: `sharing rule "${str(rule.name) || rIndex}" on object "${target}"`, path: `sharingRules[${rIndex}].sharedWith`, message: - `A business-unit sharing rule targets "${target}", which opted out of tenancy ` + - `(\`tenancy.enabled: false\`). Platform-global objects carry no organization column, so this ` + - `grant spans EVERY organization — a cross-organization business-unit grant, which ADR-0105 D6 ` + - `forbids (BU trees are org-internal).`, + `Sharing rule recipient \`${recipientType}\` (${reach}) targets "${target}", which opted out of ` + + `tenancy (\`tenancy.enabled: false\`). Platform-global objects carry no organization column, so ` + + `this grant spans EVERY organization — a cross-organization business-unit grant, which ADR-0105 ` + + `D6 forbids (BU trees are org-internal).`, hint: `Either scope the object to organizations (drop \`tenancy.enabled: false\` so Layer 0 walls it), ` + - `or share it to a position / permission-set audience instead of a business unit. A ` + - `platform-global catalog that everyone should read wants an OWD of \`public_read\`, not a BU grant.`, + `or share it to a \`user\` / \`team\` / \`position\` audience instead — those expand flat, with no ` + + `business-unit tree to resolve. A platform-global catalog that everyone should read wants an OWD ` + + `of \`public_read\`, not a BU grant.`, }); });