Skip to content

Commit af96af6

Browse files
fix(lint): org-axis 红线读 spec 声明的 sharing-rule 键 —— D6 两条门禁此前从不触发 (#4984) (#4992)
`validateOrgAxisRedLines` 取 `rule.criteria ?? rule.filter` 与 `rule.sharedTo ?? rule.recipient`,而 `SharingRuleSchema` 是 `.strict()` 的、 声明的键是 `condition` / `sharedWith` —— 那四个名字只作为**被拒别名**存在于 `sharingRuleUnknownKeyError` 里。规则跑在 parse 之后(`input: 'parsed'`), 所以对任何 spec 合法的 stack 这四个属性恒为 undefined, `parent_organization_id` 的判定恒为 false:一条 error 级红线在 sharing-rule 路径上从不触发。 改为只读 canonical 键。别名不在 consumer 侧用 `??` 容忍 —— schema 的拒收 信息已经给出处方,parse 就是那道门。`condition` 是 `ExpressionInput`, 三种形状(裸串 / `{dialect,source}` / 编译后 `{dialect,ast}`)都要能扫到。 结构性的一半在 fixture:原测试用的正是那些被拒别名,所以**测试全绿而规则全死**。 现在每个 sharing-rule fixture 先过 `SharingRuleSchema`、每个 object fixture 先过 `ObjectSchema` —— fixture 与 spec 漂移即在 fixture 处判红,而不是去测一个 作者写不出来的形状。 Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent dca25e1 commit af96af6

3 files changed

Lines changed: 312 additions & 23 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
fix(lint): `validateOrgAxisRedLines` reads the sharing-rule keys the spec declares — the ADR-0105 D6 red lines fired on nothing before (#4984)
6+
7+
The two ADR-0105 D6 red lines are declared `error` and gate `os validate` /
8+
`os build` / `os lint`. On the sharing-rule path neither of them could fire.
9+
10+
`validateOrgAxisRedLines` read `rule.criteria ?? rule.filter` and
11+
`rule.sharedTo ?? rule.recipient`. `SharingRuleSchema` is `.strict()` and its
12+
declared keys are `condition` and `sharedWith`; all four names it was reading
13+
exist only as **rejected aliases** in `sharingRuleUnknownKeyError`, the
14+
prescription attached to the refusal message. The rule runs on the post-parse
15+
stack (`input: 'parsed'`), so for every spec-valid stack those four properties
16+
were `undefined`, `JSON.stringify(undefined ?? '')` was `'""'`, and the
17+
`parent_organization_id` test was constantly false.
18+
19+
**This is a behaviour change: the rule previously never triggered.** Both red
20+
lines are now live on the sharing-rule path:
21+
22+
| Authored shape | Before | After |
23+
|:--|:--|:--|
24+
| `condition` reading `parent_organization_id` | passed | `error` `org-axis-permission-inheritance` at `sharingRules[i].condition` |
25+
| `sharedWith` reading `parent_organization_id` | passed | `error` `org-axis-permission-inheritance` at `sharingRules[i].sharedWith` |
26+
| `sharedWith: { type: 'business_unit' }` on a `tenancy.enabled: false` object | passed | `error` `org-axis-cross-org-bu-grant` at `sharingRules[i].sharedWith` |
27+
28+
A stack that ships today keeps building unless it contains one of those three —
29+
the shapes D6 forbids and the gate was meant to have been refusing all along.
30+
The rejected aliases are deliberately **not** read: a rule spelling `criteria`
31+
or `sharedTo` is refused by the schema's own parse with the canonical key
32+
named, and a consumer must not tolerate what the producer's contract rejects.
33+
34+
`condition` is an `ExpressionInput`, so all three of its shapes are scanned —
35+
the authored bare string, the parsed `{ dialect, source }` envelope, and the
36+
compiled `{ dialect, ast }` form.
37+
38+
The rule's own tests were the reason this survived review: their fixtures used
39+
the same rejected aliases, so the suite was green while the gate was dead. Every
40+
sharing-rule fixture now goes through `SharingRuleSchema` before the lint sees
41+
it, and every object fixture through `ObjectSchema` — a fixture that drifts from
42+
the spec surface fails at the fixture instead of silently exercising a shape no
43+
author can write.

packages/lint/src/validate-org-axis-red-lines.test.ts

Lines changed: 221 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4+
import { ObjectSchema } from '@objectstack/spec/data';
5+
import { SharingRuleSchema, type SharingRuleInput } from '@objectstack/spec/security';
46

57
import {
68
validateOrgAxisRedLines,
@@ -10,6 +12,104 @@ import {
1012

1113
const rules = (stack: unknown) => validateOrgAxisRedLines(stack).map((f) => f.rule);
1214

15+
/**
16+
* ── The fixture/schema drift guard (#4984) ──────────────────────────────────
17+
*
18+
* Every sharing-rule fixture below is built through `sharingRule()`, which
19+
* PARSES it with the real `SharingRuleSchema` before the lint ever sees it. A
20+
* fixture that drifts from the spec surface therefore fails here, loudly, at
21+
* the fixture — not silently, by exercising a code path no author can reach.
22+
*
23+
* This guard exists because its absence cost a red line. The pre-#4984 fixtures
24+
* spelled the rule's keys `criteria` and `sharedTo`; `SharingRuleSchema` is
25+
* `.strict()` and knows those two only as REJECTED aliases of `condition` and
26+
* `sharedWith`. The lint read the aliases too, so the tests passed against a
27+
* shape no spec-valid stack can contain: green tests, dead gate. Renaming the
28+
* keys alone would fix today's bug and leave tomorrow's drift undetectable —
29+
* this helper is the structural half of the fix.
30+
*
31+
* It returns the PARSED rule, which is also what the registry feeds the lint
32+
* (`input: 'parsed'`): `condition` arrives as the `{ dialect, source }`
33+
* envelope, not the authored string.
34+
*/
35+
function sharingRule(input: SharingRuleInput): Record<string, unknown> {
36+
const result = SharingRuleSchema.safeParse(input);
37+
if (!result.success) {
38+
const detail = result.error.issues
39+
.map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`)
40+
.join('; ');
41+
throw new Error(
42+
`sharing-rule fixture is not spec-valid — the lint would be tested against a shape ` +
43+
`no author can write (#4984). Keys given: ${Object.keys(input as object).join(', ')}. ${detail}`,
44+
);
45+
}
46+
return result.data as unknown as Record<string, unknown>;
47+
}
48+
49+
/** Same guard for the object fixtures rule ② reads (`tenancy` / `systemFields`). */
50+
function objectFixture(input: Record<string, unknown>): Record<string, unknown> {
51+
const result = ObjectSchema.safeParse({
52+
label: 'Fixture',
53+
fields: { name: { type: 'text', label: 'Name' } },
54+
...input,
55+
});
56+
if (!result.success) {
57+
throw new Error(
58+
`object fixture is not spec-valid (#4984): ` +
59+
result.error.issues.map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`).join('; '),
60+
);
61+
}
62+
return result.data as unknown as Record<string, unknown>;
63+
}
64+
65+
/** A recipient that is beyond reproach, for fixtures whose subject is the predicate. */
66+
const HQ_TEAM = { type: 'team', value: 'hq' } as const;
67+
68+
describe('sharing-rule fixtures track the spec surface (meta-test, #4984)', () => {
69+
it('rejects a fixture spelled with the schema-rejected aliases', () => {
70+
// The exact pre-#4984 fixture shapes. Each must now fail at the fixture.
71+
expect(() =>
72+
sharingRule({
73+
name: 'hq_rollup',
74+
object: 'work_order',
75+
criteria: { parent_organization_id: 'org_hq' },
76+
} as unknown as SharingRuleInput),
77+
).toThrow(/not spec-valid/);
78+
expect(() =>
79+
sharingRule({
80+
name: 'plant_team',
81+
object: 'work_order',
82+
sharedTo: { type: 'business_unit', id: 'bu_plant_a' },
83+
} as unknown as SharingRuleInput),
84+
).toThrow(/not spec-valid/);
85+
});
86+
87+
it('rejects the rejected recipient alias `id` (the recipient shape is strict too)', () => {
88+
expect(() =>
89+
sharingRule({
90+
name: 'r',
91+
type: 'criteria',
92+
object: 'work_order',
93+
// `id` is an alias of `value`, rejected by `sharingRecipientUnknownKeyError`.
94+
sharedWith: { type: 'business_unit', id: 'bu_plant_a' },
95+
condition: 'true',
96+
} as unknown as SharingRuleInput),
97+
).toThrow(/not spec-valid/);
98+
});
99+
100+
it('accepts the canonical shape and hands the lint the PARSED envelope', () => {
101+
const rule = sharingRule({
102+
name: 'ok',
103+
type: 'criteria',
104+
object: 'work_order',
105+
sharedWith: HQ_TEAM,
106+
condition: "record.status == 'open'",
107+
});
108+
expect(rule.condition).toEqual({ dialect: 'cel', source: "record.status == 'open'" });
109+
expect(rule.sharedWith).toEqual({ type: 'team', value: 'hq' });
110+
});
111+
});
112+
13113
describe('validateOrgAxisRedLines — ① no permission inheritance on the org axis', () => {
14114
it('flags an RLS `using` on a permission set that walks the org parent', () => {
15115
const findings = validateOrgAxisRedLines({
@@ -62,16 +162,98 @@ describe('validateOrgAxisRedLines — ① no permission inheritance on the org a
62162
expect(findings[0].path).toBe('objects[0].rowLevelSecurity[0].using');
63163
});
64164

65-
it('flags a sharing rule whose criteria walk the org parent', () => {
165+
it('flags a spec-valid sharing rule whose `condition` walks the org parent', () => {
166+
// Exactly the stack #4984 showed passing `os validate` / `os build` / `os lint`.
167+
const findings = validateOrgAxisRedLines({
168+
sharingRules: [
169+
sharingRule({
170+
name: 'hq_sees_children',
171+
type: 'criteria',
172+
object: 'work_order',
173+
sharedWith: HQ_TEAM,
174+
condition: "record.parent_organization_id == 'org_hq'",
175+
}),
176+
],
177+
});
178+
expect(findings).toHaveLength(1);
179+
expect(findings[0]).toMatchObject({
180+
severity: 'error',
181+
rule: ORG_AXIS_PERMISSION_INHERITANCE,
182+
path: 'sharingRules[0].condition',
183+
});
184+
expect(findings[0].where).toBe('sharing rule "hq_sees_children"');
185+
expect(findings[0].message).toMatch(/parent_organization_id/);
186+
});
187+
188+
it('flags the pre-parse `condition` shape too (`os lint` runs on the normalized stack)', () => {
189+
// Bare-string shorthand — what the author typed, before `ExpressionInputSchema`
190+
// wraps it. The `parsed` tier falls back to `normalized` under `os lint`.
66191
expect(
67192
rules({
68193
sharingRules: [
69-
{ name: 'hq_rollup', object: 'work_order', criteria: { parent_organization_id: 'org_hq' } },
194+
{
195+
name: 'hq_sees_children',
196+
type: 'criteria',
197+
object: 'work_order',
198+
sharedWith: { type: 'team', value: 'hq' },
199+
condition: "record.parent_organization_id == 'org_hq'",
200+
},
70201
],
71202
}),
72203
).toEqual([ORG_AXIS_PERMISSION_INHERITANCE]);
73204
});
74205

206+
it('flags the compiled `{ dialect, ast }` condition shape', () => {
207+
expect(
208+
rules({
209+
sharingRules: [
210+
{
211+
name: 'hq_sees_children',
212+
object: 'work_order',
213+
sharedWith: { type: 'team', value: 'hq' },
214+
condition: {
215+
dialect: 'cel',
216+
ast: { type: 'binary', op: '==', left: { type: 'member', path: ['record', 'parent_organization_id'] } },
217+
},
218+
},
219+
],
220+
}),
221+
).toEqual([ORG_AXIS_PERMISSION_INHERITANCE]);
222+
});
223+
224+
it('flags a recipient that reaches for the org parent', () => {
225+
const findings = validateOrgAxisRedLines({
226+
sharingRules: [
227+
sharingRule({
228+
name: 'by_parent_org',
229+
type: 'criteria',
230+
object: 'work_order',
231+
sharedWith: { type: 'team', value: 'parent_organization_id' },
232+
condition: 'true',
233+
}),
234+
],
235+
});
236+
expect(findings).toHaveLength(1);
237+
expect(findings[0].path).toBe('sharingRules[0].sharedWith');
238+
});
239+
240+
it('does NOT resurrect the schema-rejected aliases — parse is that gate', () => {
241+
// `criteria` / `filter` / `sharedTo` / `recipient` are rejected by
242+
// `SharingRuleSchema` with a named fix-it. Reading them here as `??`
243+
// fallbacks is what made this rule inert (#4984); a consumer must not
244+
// tolerate what the producer's contract refuses (Prime Directive #12).
245+
expect(
246+
rules({
247+
sharingRules: [
248+
{ name: 'a', object: 'work_order', criteria: { parent_organization_id: 'org_hq' } },
249+
{ name: 'b', object: 'work_order', filter: "record.parent_organization_id == 'x'" },
250+
{ name: 'c', object: 'work_order', sharedTo: { type: 'team', id: 'parent_organization_id' } },
251+
{ name: 'd', object: 'work_order', recipient: { type: 'team', id: 'parent_organization_id' } },
252+
],
253+
}),
254+
).toEqual([]);
255+
});
256+
75257
it('stays silent on membership-based and business-unit scoping (the sanctioned paths)', () => {
76258
expect(
77259
rules({
@@ -88,23 +270,37 @@ describe('validateOrgAxisRedLines — ① no permission inheritance on the org a
88270
},
89271
],
90272
sharingRules: [
91-
{ name: 'plant_team', object: 'work_order', sharedTo: { type: 'business_unit', id: 'bu_plant_a' } },
273+
sharingRule({
274+
name: 'plant_team',
275+
type: 'criteria',
276+
object: 'work_order',
277+
sharedWith: { type: 'business_unit', value: 'bu_plant_a' },
278+
condition: "record.status == 'open'",
279+
}),
92280
],
93-
objects: [{ name: 'work_order' }],
281+
objects: [objectFixture({ name: 'work_order' })],
94282
}),
95283
).toEqual([]);
96284
});
97285
});
98286

99287
describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal', () => {
100288
const platformGlobalStack = (tenancy: unknown) => ({
101-
objects: [{ name: 'material_catalog', tenancy }],
289+
objects: [
290+
objectFixture(
291+
tenancy === undefined
292+
? { name: 'material_catalog' }
293+
: { name: 'material_catalog', tenancy },
294+
),
295+
],
102296
sharingRules: [
103-
{
297+
sharingRule({
104298
name: 'catalog_to_plant',
299+
type: 'criteria',
105300
object: 'material_catalog',
106-
sharedTo: { type: 'business_unit', id: 'bu_plant_a' },
107-
},
301+
sharedWith: { type: 'business_unit', value: 'bu_plant_a' },
302+
condition: 'true',
303+
}),
108304
],
109305
});
110306

@@ -114,17 +310,23 @@ describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal'
114310
expect(findings[0]).toMatchObject({
115311
severity: 'error',
116312
rule: ORG_AXIS_CROSS_ORG_BU_GRANT,
117-
path: 'sharingRules[0].sharedTo',
313+
path: 'sharingRules[0].sharedWith',
118314
});
119315
expect(findings[0].message).toMatch(/spans EVERY organization/);
120316
});
121317

122318
it('flags the `systemFields.tenant: false` spelling of the same opt-out', () => {
123319
expect(
124320
rules({
125-
objects: [{ name: 'material_catalog', systemFields: { tenant: false } }],
321+
objects: [objectFixture({ name: 'material_catalog', systemFields: { tenant: false } })],
126322
sharingRules: [
127-
{ name: 'r', object: 'material_catalog', sharedTo: { type: 'business_unit', id: 'bu' } },
323+
sharingRule({
324+
name: 'r',
325+
type: 'criteria',
326+
object: 'material_catalog',
327+
sharedWith: { type: 'business_unit', value: 'bu' },
328+
condition: 'true',
329+
}),
128330
],
129331
}),
130332
).toEqual([ORG_AXIS_CROSS_ORG_BU_GRANT]);
@@ -138,9 +340,15 @@ describe('validateOrgAxisRedLines — ② business-unit trees stay org-internal'
138340
it('allows a non-BU audience on a platform-global object', () => {
139341
expect(
140342
rules({
141-
objects: [{ name: 'material_catalog', tenancy: { enabled: false } }],
343+
objects: [objectFixture({ name: 'material_catalog', tenancy: { enabled: false } })],
142344
sharingRules: [
143-
{ name: 'r', object: 'material_catalog', sharedTo: { type: 'position', name: 'buyer' } },
345+
sharingRule({
346+
name: 'r',
347+
type: 'criteria',
348+
object: 'material_catalog',
349+
sharedWith: { type: 'position', value: 'buyer' },
350+
condition: 'true',
351+
}),
144352
],
145353
}),
146354
).toEqual([]);

0 commit comments

Comments
 (0)