diff --git a/.changeset/org-axis-dead-alias-branches.md b/.changeset/org-axis-dead-alias-branches.md new file mode 100644 index 0000000000..6145fe7889 --- /dev/null +++ b/.changeset/org-axis-dead-alias-branches.md @@ -0,0 +1,39 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): 清除 `validateOrgAxisRedLines` 里 spec 合法 stack 永远到不了的四条分支 (#5009) + +`validate-org-axis-red-lines.ts` 是 `input: 'parsed'` 规则 —— 它看到的是 +`ObjectStackSchema` 解析后的产物。#4984 修掉了 sharing rule 字段那一层的 `??` +别名读法,但同一文件里还留着四条同形分支,每一条读的键 spec 都不声明: + +| 原读法 | spec 事实 | 处置 | +|:--|:--|:--| +| `cfg.permissions ?? cfg.permissionSets` | stack 根 **strip** 未声明键,`permissionSets` 解析后必为 `undefined` | 收敛为 `cfg.permissions` | +| `cfg.sharingRules ?? cfg.sharing`(两处) | 同上 | 收敛为 `cfg.sharingRules` | +| `str(rule.object ?? rule.objectName)` | `SharingRuleSchema` 是 `.strict()`,按名拒绝 `objectName`;`object` 又是必填 | 收敛为 `rule.object` | +| `asArray(object.rowLevelSecurity ?? object.rls)` 整段(约 20 行) | **`ObjectSchema` 两个键都不声明**,且 `.strict()` —— 带对象级 RLS 的 stack 在 `os validate` / `os build` 直接被拒("Unrecognized key(s) on this object") | **删除** | + +对任何 spec 合法的 stack,判定结果不变:这些分支本来就永远不执行(反向验证 —— +新测试跑在改动前的实现上,29 条由 `safeParse` fixture 驱动的断言全绿)。真正的 +代价从来不是漏报,而是误导:对象级 RLS **根本不是授权面**(`authorable-surface.json` +里只有 `security/PermissionSet:rowLevelSecurity` 一条),而那段死代码连 +`objects[N].rowLevelSecurity[M].using` 的诊断 path 都写好了,足以让下一位作者 +(人或 AI)相信它是真的并照着写更多代码 —— #5008 差点就这么做了。 + +行为上唯一的差别落在 `os lint`(不 parse,跑 normalized 层):把别名拼法写进 +stack 的作者,不再从这条红线拿到诊断,而是从 schema 那里拿到一条指名道姓的 +拒绝。别名容忍属于 producer 的拒绝,不属于 consumer(Prime Directive #12)。 + +同时补上一层结构性 meta-guard(#4992 模式),让下一条死分支在 review 前就红: + +- **declared-key guard** —— 规则源码里从 stack / permission set / RLS policy / + object / sharing rule 上读的每一个键,都必须出现在对应 schema 自己的 `.shape` + 里。扫源码而不是扫行为是刻意的:不可达分支根本没有行为可断言。 +- **reachability guard** —— 每个 `findings.push` 调用点都必须被至少一条过 + `safeParse` 的 fixture 触达;走不到的分支不允许存在。 +- 规则 ① 的 fixture 现在也走 `PermissionSetSchema.safeParse`(此前只有 sharing + rule 和 object fixture 有这层保护)。 + +四条分支各自被变异测试验证过:把任意一条加回去,都至少有两条测试转红。 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 e83f2391c2..ef11666181 100644 --- a/packages/lint/src/validate-org-axis-red-lines.test.ts +++ b/packages/lint/src/validate-org-axis-red-lines.test.ts @@ -1,8 +1,18 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import { readFileSync } from 'node:fs'; + import { describe, it, expect } from 'vitest'; +import { ObjectStackSchema } from '@objectstack/spec'; import { ObjectSchema } from '@objectstack/spec/data'; -import { ShareRecipientType, SharingRuleSchema, type SharingRuleInput } from '@objectstack/spec/security'; +import { + PermissionSetSchema, + RowLevelSecurityPolicySchema, + ShareRecipientType, + SharingRuleSchema, + type PermissionSetInput, + type SharingRuleInput, +} from '@objectstack/spec/security'; import { validateOrgAxisRedLines, @@ -46,6 +56,27 @@ function sharingRule(input: SharingRuleInput): Record { return result.data as unknown as Record; } +/** + * Same guard for the permission-set fixtures rule ① reads (#5009). + * + * `permissions[].rowLevelSecurity` is the ONLY authorable RLS surface — + * `ObjectSchema` declares no `rowLevelSecurity` (nor `rls`) and is `.strict()`, + * so the object-level traversal this rule used to carry could not run against a + * stack that parses. Building rule ①'s fixtures through the real schema is what + * keeps that from quietly coming back: a fixture for a surface the spec does + * not have now fails HERE. + */ +function permissionSet(input: PermissionSetInput): Record { + const result = PermissionSetSchema.safeParse(input); + if (!result.success) { + throw new Error( + `permission-set fixture is not spec-valid (#5009): ` + + result.error.issues.map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`).join('; '), + ); + } + return result.data as unknown as Record; +} + /** Same guard for the object fixtures rule ② reads (`tenancy` / `systemFields`). */ function objectFixture(input: Record): Record { const result = ObjectSchema.safeParse({ @@ -114,16 +145,19 @@ describe('validateOrgAxisRedLines — ① no permission inheritance on the org a it('flags an RLS `using` on a permission set that walks the org parent', () => { const findings = validateOrgAxisRedLines({ permissions: [ - { + permissionSet({ name: 'group_hq_reader', + label: 'Group HQ Reader', + objects: {}, rowLevelSecurity: [ { name: 'child_orgs', object: 'work_order', + operation: 'select', using: 'organization_id IN (current_user.parent_organization_id)', }, ], - }, + }), ], }); expect(findings).toHaveLength(1); @@ -140,28 +174,19 @@ describe('validateOrgAxisRedLines — ① no permission inheritance on the org a expect( rules({ permissions: [ - { - name: 'p', - rowLevelSecurity: [{ name: 'r', check: "parent_organization_id = 'org_hq'" }], - }, + permissionSet({ + name: 'pset', + label: 'P', + objects: {}, + rowLevelSecurity: [ + { name: 'r', object: 'work_order', operation: 'all', check: "parent_organization_id = 'org_hq'" }, + ], + }), ], }), ).toEqual([ORG_AXIS_PERMISSION_INHERITANCE]); }); - it('flags an object-authored RLS policy', () => { - const findings = validateOrgAxisRedLines({ - objects: [ - { - name: 'work_order', - rowLevelSecurity: [{ name: 'rollup', using: 'parent_organization_id = current_user.organization_id' }], - }, - ], - }); - expect(findings).toHaveLength(1); - expect(findings[0].path).toBe('objects[0].rowLevelSecurity[0].using'); - }); - it('flags a spec-valid sharing rule whose `condition` walks the org parent', () => { // Exactly the stack #4984 showed passing `os validate` / `os build` / `os lint`. const findings = validateOrgAxisRedLines({ @@ -258,16 +283,28 @@ describe('validateOrgAxisRedLines — ① no permission inheritance on the org a expect( rules({ permissions: [ - { + permissionSet({ name: 'plant_reader', + label: 'Plant Reader', + objects: {}, rowLevelSecurity: [ // ADR-0105 D2 — the engine's own union wall vocabulary. - { name: 'my_orgs', using: 'organization_id IN (current_user.accessible_org_ids)' }, + { + name: 'my_orgs', + object: 'work_order', + operation: 'select', + using: 'organization_id IN (current_user.accessible_org_ids)', + }, // Intra-org hierarchy — the business-unit tree, not the org tree. - { name: 'my_unit', using: 'business_unit_id IN (current_user.unit_ids)' }, - { name: 'mine', using: 'owner_id = current_user.id' }, + { + name: 'my_unit', + object: 'work_order', + operation: 'select', + using: 'business_unit_id IN (current_user.unit_ids)', + }, + { name: 'mine', object: 'work_order', operation: 'select', using: 'owner_id = current_user.id' }, ], - }, + }), ], sharingRules: [ sharingRule({ @@ -284,6 +321,110 @@ describe('validateOrgAxisRedLines — ① no permission inheritance on the org a }); }); +/** + * ── The surfaces this rule deliberately does NOT read (#5009) ─────────────── + * + * #4984 removed the `??` alias reads from the sharing rule's FIELDS and left + * three more of the same shape one level up. Each is pinned here against the + * schema fact that makes it unreachable, so "put the fallback back, just in + * case" fails a test with the evidence attached rather than passing quietly. + */ +const MANIFEST = { id: 'org_axis_probe', name: 'org_axis_probe', version: '1.0.0', type: 'app' } as const; + +/** The violating RLS policy shape, spelled for the object-level key that does not exist. */ +const ORG_WALKING_POLICY = { name: 'rollup', using: 'parent_organization_id = current_user.organization_id' }; + +describe('validateOrgAxisRedLines — undeclared keys are the schema’s job, not this rule’s (#5009)', () => { + it('object-level RLS is not an authoring surface: `ObjectSchema` declares neither `rowLevelSecurity` nor `rls`', () => { + const objectKeys = Object.keys(ObjectSchema.shape); + expect(objectKeys).not.toContain('rowLevelSecurity'); + expect(objectKeys).not.toContain('rls'); + // The only declared home for RLS policies is the permission set. + expect(Object.keys(PermissionSetSchema.shape)).toContain('rowLevelSecurity'); + + // And `ObjectSchema` is `.strict()`, so this is not a silent strip: a stack + // carrying an object-level policy is REFUSED by `os validate` / `os build`, + // by name. The traversal deleted in #5009 could therefore never run against + // a stack anyone can ship — it only ever described a surface that isn't. + const refused = ObjectStackSchema.safeParse({ + manifest: MANIFEST, + objects: [ + { + name: 'work_order', + label: 'Work Order', + fields: { name: { type: 'text', label: 'Name' } }, + rowLevelSecurity: [ORG_WALKING_POLICY], + }, + ], + }); + expect(refused.success).toBe(false); + expect(refused.error?.issues.map((i) => i.message).join(' ')).toMatch( + /Unrecognized key\(s\) on this object: `rowLevelSecurity`/, + ); + + // The lint stays silent on both spellings — as it did BEFORE the deletion + // for every stack that parses. Removing dead code changed no verdict. + expect(rules({ objects: [{ name: 'work_order', rowLevelSecurity: [ORG_WALKING_POLICY] }] })).toEqual([]); + expect(rules({ objects: [{ name: 'work_order', rls: [ORG_WALKING_POLICY] }] })).toEqual([]); + }); + + it('`permissionSets` / `sharing` are not stack-root keys — the root STRIPS them before any rule runs', () => { + const rootKeys = Object.keys(ObjectStackSchema.shape); + expect(rootKeys).toEqual(expect.arrayContaining(['permissions', 'sharingRules'])); + expect(rootKeys).not.toContain('permissionSets'); + expect(rootKeys).not.toContain('sharing'); + + // Unlike the `.strict()` sub-schemas this one strips rather than rejects, + // which is precisely why the dead branch was invisible: the stack parses, + // and the key the rule reached for is simply gone. + const parsed = ObjectStackSchema.safeParse({ + manifest: MANIFEST, + permissionSets: [{ name: 'pset', label: 'P', objects: {} }], + sharing: [{ name: 'r', type: 'criteria', object: 'o', sharedWith: HQ_TEAM, condition: 'true' }], + }); + expect(parsed.success).toBe(true); + expect(parsed.data).not.toHaveProperty('permissionSets'); + expect(parsed.data).not.toHaveProperty('sharing'); + + // So the rule reads the declared spellings only. A stack that spells them + // the other way gets its diagnostic from the schema, not from a red line + // that would fire on a shape `os validate` never lets through. + expect( + rules({ + permissionSets: [ + { name: 'p', rowLevelSecurity: [{ name: 'r', using: "parent_organization_id = 'org_hq'" }] }, + ], + sharing: [ + { name: 's', object: 'work_order', condition: "record.parent_organization_id == 'x'" }, + ], + }), + ).toEqual([]); + }); + + it('`sharingRules[].objectName` is rejected by name, and `object` is required on any rule that parsed', () => { + expect(Object.keys(SharingRuleSchema.shape)).toContain('object'); + expect(Object.keys(SharingRuleSchema.shape)).not.toContain('objectName'); + expect(() => + sharingRule({ + name: 'r', + type: 'criteria', + objectName: 'material_catalog', + sharedWith: { type: 'business_unit', value: 'bu' }, + condition: 'true', + } as unknown as SharingRuleInput), + ).toThrow(/not spec-valid/); + // ② therefore never needs a fallback for the rule's target object. + expect( + rules({ + objects: [objectFixture({ name: 'material_catalog', tenancy: { enabled: false } })], + sharingRules: [ + { name: 'r', objectName: 'material_catalog', sharedWith: { type: 'business_unit', value: 'bu' } }, + ], + }), + ).toEqual([]); + }); +}); + /** * ── Rule ②'s recipient word list ──────────────────────────────────────────── * @@ -437,6 +578,266 @@ describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal' ); }); +/** + * ── The structural meta-guard (#4992 pattern, #5009) ──────────────────────── + * + * The two tests above pin the three branches #5009 removed. These two pin the + * PROPERTY that made them removable, so the next one is caught before review: + * + * 1. **Declared-key guard.** Every key this rule reads off a stack, permission + * set, RLS policy, object or sharing rule must appear in that surface's own + * Zod `.shape`. A rule registered `input: 'parsed'` can only ever see + * declared keys, so a read of anything else is dead on arrival — this is the + * check whose absence cost #4984 a red line and #5009 three more branches. + * Scanning the source (not the behaviour) is deliberate: an unreachable + * branch has no behaviour to assert on, which is exactly the problem. + * + * 2. **Reachability guard.** Every `findings.push` site must be reached by at + * least one fixture that passed `safeParse`. A gate no spec-valid stack can + * trip is not a gate; it is documentation of a surface that does not exist, + * and it reads as an invitation to write more code against it. + */ +const RULE_SOURCE = readFileSync(new URL('./validate-org-axis-red-lines.ts', import.meta.url), 'utf8'); + +/** The rule's CODE — comments stripped, since the guards below scan reads, not prose. */ +const RULE_CODE = RULE_SOURCE.replace(/\/\*[\s\S]*?\*\//g, '').replace(/\/\/[^\n]*/g, ''); + +/** Distinct property names read off `receiver` in the rule's code. */ +function keysReadOff(receiver: string): string[] { + const re = new RegExp(`\\b${receiver}\\??\\.([A-Za-z_$][\\w$]*)`, 'g'); + return [...new Set([...RULE_CODE.matchAll(re)].map((m) => m[1]))].sort(); +} + +const shapeKeys = (schema: unknown): string[] => + Object.keys((schema as { shape: Record }).shape); + +/** + * Each receiver in the rule's body, the keys it is EXPECTED to read, and the + * schema that has to declare every one of them. The expected list is spelled + * out so that adding a read (or renaming a parameter, which would silently + * disarm the scan) forces a deliberate visit to this table. + */ +const READ_SURFACES: Array<{ + receiver: string; + expected: string[]; + declaredBy: string; + keys: () => string[]; +}> = [ + { + receiver: 'cfg', + expected: ['objects', 'permissions', 'sharingRules'], + declaredBy: 'ObjectStackSchema', + keys: () => shapeKeys(ObjectStackSchema), + }, + { + receiver: 'ps', + expected: ['name', 'rowLevelSecurity'], + declaredBy: 'PermissionSetSchema', + keys: () => shapeKeys(PermissionSetSchema), + }, + { + receiver: 'policy', + expected: ['name'], + declaredBy: 'RowLevelSecurityPolicySchema', + keys: () => shapeKeys(RowLevelSecurityPolicySchema), + }, + { + receiver: 'object', + expected: ['systemFields', 'tenancy'], + declaredBy: 'ObjectSchema', + keys: () => shapeKeys(ObjectSchema), + }, + { + receiver: 'rule', + expected: ['condition', 'name', 'object', 'sharedWith'], + declaredBy: 'SharingRuleSchema', + keys: () => shapeKeys(SharingRuleSchema), + }, + { + receiver: 'sharedWith', + expected: ['type'], + declaredBy: 'SharingRuleSchema.sharedWith', + keys: () => shapeKeys((SharingRuleSchema as unknown as { shape: { sharedWith: unknown } }).shape.sharedWith), + }, +]; + +describe('validateOrgAxisRedLines — reads only keys the spec declares (meta-test, #5009)', () => { + it.each(READ_SURFACES)('every key read off `$receiver` is declared by $declaredBy', (surface) => { + const read = keysReadOff(surface.receiver); + expect(read).toEqual(surface.expected); + const declared = surface.keys(); + expect(read.filter((k) => !declared.includes(k))).toEqual([]); + }); + + it('the RLS clause list is spelled from `RowLevelSecurityPolicySchema` keys', () => { + // `policy[clause]` is a COMPUTED read, so the scan above cannot see it; the + // word list it indexes with is checked here instead. + const match = /for \(const clause of \[([^\]]*)\] as const\)/.exec(RULE_CODE); + expect(match, 'the clause loop moved — update this guard').not.toBeNull(); + const clauses = [...match![1].matchAll(/'([^']+)'/g)].map((m) => m[1]); + expect(clauses).toEqual(['using', 'check']); + const declared = shapeKeys(RowLevelSecurityPolicySchema); + expect(clauses.filter((c) => !declared.includes(c))).toEqual([]); + }); +}); + +/** A `findings.push` call site, as the rule's source declares it. */ +interface PushSite { + rule: string; + pathTemplate: string; +} + +const RULE_IDS: Record = { + ORG_AXIS_PERMISSION_INHERITANCE, + ORG_AXIS_CROSS_ORG_BU_GRANT, +}; + +function pushSites(): PushSite[] { + return RULE_CODE.split('findings.push({') + .slice(1) + .map((block, i) => { + const ruleConst = /rule:\s*([A-Z_][A-Z0-9_]*)/.exec(block)?.[1]; + const pathTemplate = /path:\s*`([^`]*)`/.exec(block)?.[1]; + if (!ruleConst || !pathTemplate) { + throw new Error(`findings.push site #${i} has no literal \`rule:\` / \`path:\` — the guard cannot map it`); + } + const rule = RULE_IDS[ruleConst]; + if (!rule) throw new Error(`findings.push site #${i} emits unknown rule id \`${ruleConst}\``); + return { rule, pathTemplate }; + }); +} + +/** `permissions[${i}].rowLevelSecurity[${j}].${clause}` → a matcher for a concrete path. */ +function templateToRegex(template: string): RegExp { + const literals = template.split(/\$\{[^}]*\}/g).map((part) => part.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); + return new RegExp(`^${literals.join('[^.\\[\\]]+')}$`); +} + +/** + * One spec-valid stack per branch the rule still carries. EVERY fixture here + * goes through `safeParse` (via the helpers at the top of this file), so a + * branch is only "covered" if a stack an author can actually ship reaches it. + */ +const REACHABILITY_CORPUS: Array<{ label: string; stack: unknown }> = [ + { + label: '① permission-set RLS `using`', + stack: { + permissions: [ + permissionSet({ + name: 'group_hq_reader', + label: 'Group HQ Reader', + objects: {}, + rowLevelSecurity: [ + { + name: 'child_orgs', + object: 'work_order', + operation: 'select', + using: 'organization_id IN (current_user.parent_organization_id)', + }, + ], + }), + ], + }, + }, + { + label: '① permission-set RLS `check`', + stack: { + permissions: [ + permissionSet({ + name: 'group_hq_writer', + label: 'Group HQ Writer', + objects: {}, + rowLevelSecurity: [ + { + name: 'child_orgs', + object: 'work_order', + operation: 'insert', + check: "parent_organization_id = 'org_hq'", + }, + ], + }), + ], + }, + }, + { + label: '① sharing-rule `condition`', + stack: { + sharingRules: [ + sharingRule({ + name: 'hq_sees_children', + type: 'criteria', + object: 'work_order', + sharedWith: HQ_TEAM, + condition: "record.parent_organization_id == 'org_hq'", + }), + ], + }, + }, + { + label: '① sharing-rule `sharedWith`', + stack: { + sharingRules: [ + sharingRule({ + name: 'by_parent_org', + type: 'criteria', + object: 'work_order', + sharedWith: { type: 'team', value: 'parent_organization_id' }, + condition: 'true', + }), + ], + }, + }, + { + label: '② BU grant on a platform-global object', + stack: { + objects: [objectFixture({ name: 'material_catalog', tenancy: { enabled: false } })], + sharingRules: [ + sharingRule({ + name: 'catalog_to_plant', + type: 'criteria', + object: 'material_catalog', + sharedWith: { type: 'unit_and_subordinates', value: 'bu_plant_a' }, + condition: 'true', + }), + ], + }, + }, +]; + +describe('validateOrgAxisRedLines — every branch is reachable by a spec-valid stack (meta-test, #5009)', () => { + it('maps every `findings.push` site in the source', () => { + const sites = pushSites(); + // ① permission-set RLS, ① sharing rule, ② cross-org BU grant. The fourth — + // `objects[].rowLevelSecurity[].${clause}` — is gone: no such surface. + expect(sites).toHaveLength(3); + expect(sites.map((s) => s.pathTemplate)).not.toContain( + 'objects[${oIndex}].rowLevelSecurity[${pIndex}].${clause}', + ); + }); + + it('reaches every site from a fixture that passed `safeParse`', () => { + const emitted = REACHABILITY_CORPUS.flatMap(({ stack }) => validateOrgAxisRedLines(stack)); + expect(emitted.length).toBeGreaterThanOrEqual(pushSites().length); + + const unreached = pushSites().filter( + (site) => + !emitted.some((f) => f.rule === site.rule && templateToRegex(site.pathTemplate).test(f.path)), + ); + expect( + unreached.map((s) => `${s.rule} @ ${s.pathTemplate}`), + 'a branch no spec-valid stack can reach must be deleted, not kept "just in case" (#5009)', + ).toEqual([]); + }); + + it('emits no path the source does not declare', () => { + const matchers = pushSites().map((s) => ({ rule: s.rule, re: templateToRegex(s.pathTemplate) })); + const emitted = REACHABILITY_CORPUS.flatMap(({ stack }) => validateOrgAxisRedLines(stack)); + for (const finding of emitted) { + expect(matchers.some((m) => m.rule === finding.rule && m.re.test(finding.path))).toBe(true); + } + }); +}); + describe('validateOrgAxisRedLines — input tolerance', () => { it('returns no findings for empty / malformed input instead of throwing', () => { expect(validateOrgAxisRedLines(undefined)).toEqual([]); diff --git a/packages/lint/src/validate-org-axis-red-lines.ts b/packages/lint/src/validate-org-axis-red-lines.ts index 7d0fa5d052..65f4394cdf 100644 --- a/packages/lint/src/validate-org-axis-red-lines.ts +++ b/packages/lint/src/validate-org-axis-red-lines.ts @@ -39,6 +39,52 @@ * so the lint moves the failure from silent-wrong-answer to author-time fix-it. * * Pure `(stack) => Finding[]`; accepts the NORMALIZED stack input. + * + * ## Scope — the keys this rule reads, and the ones it deliberately does not + * + * Every key below is one `@objectstack/spec` DECLARES. That is a contract, not + * a style preference: the rule is registered `input: 'parsed'`, so what it sees + * is what `ObjectStackSchema` returned. An undeclared key never survives to be + * read — the stack root strips it, and the `.strict()` sub-schemas reject the + * whole stack outright — so a branch keyed on one is inert for every stack an + * author can actually ship (#4984, #5009). + * + * | Read | Declared by | + * |---------------------------------------|------------------------------------| + * | `permissions[]` | `ObjectStackSchema` | + * | `permissions[].rowLevelSecurity[]` | `PermissionSetSchema` | + * | `…[].using` / `…[].check` | `RowLevelSecurityPolicySchema` | + * | `sharingRules[]` | `ObjectStackSchema` | + * | `sharingRules[].condition` / `.sharedWith` / `.object` | `SharingRuleSchema` | + * | `objects[].tenancy` / `.systemFields` | `ObjectSchema` | + * + * NOT read, and each for a reason that is a schema fact: + * + * - `permissionSets` / `sharing` — not declared on the stack root. The root + * STRIPS them, so after parse they are `undefined` no matter what the author + * wrote. The declared spellings are `permissions` and `sharingRules`. + * - `sharingRules[].objectName` — `SharingRuleSchema` is `.strict()` and knows + * it only as a rejected name; `object` is required, so the canonical read can + * never be missing on a rule that parsed. + * - `objects[].rowLevelSecurity` / `objects[].rls` — **object-level RLS is not + * an authoring surface at all.** `ObjectSchema` declares neither key (nor + * does `authorable-surface.json` list one: the sole entry is + * `security/PermissionSet:rowLevelSecurity`), and `ObjectSchema` is + * `.strict()`, so a stack carrying one does not parse — it is refused with + * "Unrecognized key(s) on this object". Until #5009 this file walked that + * non-existent surface for ~20 lines, complete with an `objects[N]. + * rowLevelSecurity[M].using` diagnostic path. Nothing could reach it, and the + * cost was never the missed finding: the next author to read this rule (human + * or AI) came away believing object-level RLS was a real authorization + * surface and wrote more code against it (#5008 nearly did). RLS policies + * live on a permission set; that is the branch above. + * + * Alias tolerance belongs at the schema's refusal, not in a consumer (Prime + * Directive #12) — where it also converts a loud, named rejection into a + * silently-inert gate. `validate-org-axis-red-lines.test.ts` pins all of this + * structurally: every key read here is checked against the declaring schema's + * own `.shape`, and every `findings.push` site must be reachable by a fixture + * that passed `safeParse`. */ export const ORG_AXIS_PERMISSION_INHERITANCE = 'org-axis-permission-inheritance'; @@ -162,9 +208,11 @@ export function validateOrgAxisRedLines(stack: unknown): OrgAxisFinding[] { // ── ① No permission inheritance along the org axis ──────────────────────── // - // RLS policies may live on a permission set (`rowLevelSecurity`) or be - // authored per object; both reach the same compiler, so both are checked. - const permissionSets = asArray(cfg.permissions ?? cfg.permissionSets); + // RLS policies live on a PERMISSION SET (`permissions[].rowLevelSecurity`) — + // the one place `ObjectSchema` does not offer and `PermissionSetSchema` does. + // See the `## Scope` table above for the object-level surface that looked + // like a second home for them and never was (#5009). + const permissionSets = asArray(cfg.permissions); permissionSets.forEach((ps, psIndex) => { asArray(ps.rowLevelSecurity).forEach((policy, pIndex) => { for (const clause of ['using', 'check'] as const) { @@ -183,27 +231,6 @@ export function validateOrgAxisRedLines(stack: unknown): OrgAxisFinding[] { }); }); - const objects = asArray(cfg.objects); - objects.forEach((object, oIndex) => { - const objectName = str(object.name) || String(oIndex); - - asArray(object.rowLevelSecurity ?? object.rls).forEach((policy, pIndex) => { - for (const clause of ['using', 'check'] as const) { - if (!str(policy[clause]).includes(ORG_PARENT_FIELD)) continue; - findings.push({ - severity: 'error', - rule: ORG_AXIS_PERMISSION_INHERITANCE, - where: `object "${objectName}" policy "${str(policy.name) || pIndex}"`, - path: `objects[${oIndex}].rowLevelSecurity[${pIndex}].${clause}`, - message: - `RLS ${clause} reads \`${ORG_PARENT_FIELD}\`, which builds a permission hierarchy along the ` + - `organization axis. ADR-0105 D6 forbids it: the org tree is a REPORTING dimension only.`, - hint: INHERITANCE_HINT, - }); - } - }); - }); - // Sharing rules — the predicate and the recipient may both reach for the org // parent, so both slots are scanned. // @@ -215,7 +242,9 @@ export function validateOrgAxisRedLines(stack: unknown): OrgAxisFinding[] { // made this red line inert for every spec-valid stack (#4984): after parse // they are always `undefined`, so the gate never fired. Alias tolerance // belongs at the schema's refusal, not in a consumer (Prime Directive #12). - asArray(cfg.sharingRules ?? cfg.sharing).forEach((rule, rIndex) => { + // The collection itself is `sharingRules`; `sharing` was the same mistake one + // level up, and is gone with the rest of them (#5009). + asArray(cfg.sharingRules).forEach((rule, rIndex) => { const slots: Array<{ key: string; text: string }> = [ { key: 'condition', text: expressionText(rule.condition) }, { key: 'sharedWith', text: JSON.stringify(rule.sharedWith ?? '') ?? '' }, @@ -242,10 +271,12 @@ export function validateOrgAxisRedLines(stack: unknown): OrgAxisFinding[] { // 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), + asArray(cfg.objects).filter((o) => isTenancyDisabled(o)).map((o) => str(o.name)).filter(Boolean), ); - asArray(cfg.sharingRules ?? cfg.sharing).forEach((rule, rIndex) => { - const target = str(rule.object ?? rule.objectName); + asArray(cfg.sharingRules).forEach((rule, rIndex) => { + // `object` is REQUIRED by `SharingRuleSchema`, so a rule that parsed always + // has it; `objectName` is not a spelling the schema accepts (#5009). + const target = str(rule.object); if (!target || !tenancyDisabledObjects.has(target)) return; // `sharedWith` is the declared recipient key; `sharedTo` / `recipient` are // spelt out only in the schema's rejection message (#4984).