Skip to content

Commit f2b8ac9

Browse files
os-zhuangclaude
andauthored
feat(lint): navigation reachability vs. granted access (#3583) (#3698)
Assessment R5. Joins what an app's navigation exposes against buildAccessMatrix — the first lint consumer of the ADR-0090 D6 matrix, which until now only backed `os compile`'s snapshot gate. Navigation and permissions are separate metadata, each valid alone, so an app can ship a menu entry for an object no permission set grants read on. The entry renders and opening it fails permission-denied. The failure hides during development because the platform's built-in `admin_full_access` carries a wildcard grant, so it works while you browse as an administrator and breaks for exactly the users the app ships permission sets for. Advisory severity: a grant may come from a set another installed package ships, which a static check cannot see. Quiet by construction in three cases — platform-provided objects (granted by their own packages), a stack that declares no permission sets at all (permissions are managed elsewhere, so flagging every entry says nothing), and any stack whose set carries a wildcard `objects: { '*': … }` grant. That last one is a bug found while verifying against the real apps: the access matrix records the wildcard under the literal key `*`, so without it the rule would have fired hardest on the stacks that granted the most. Verification also corrected the finding text. An earlier draft claimed the entry fails "for every user, including admins" — untrue, since admin_full_access is a wildcard set. The message now says what actually happens, because a diagnostic that overstates its case teaches authors to discount it. Findings on the example apps: app-crm 0, app-todo 0 (declares no permission sets), app-showcase 7 — verified TRUE positives, not false ones: showcase binds `everyone` to `showcase_member_default`, which grants 7 objects while the navigation exposes ~20, so a member genuinely cannot open Teams / Categories / Field Zoo / Cascading Select / Expense Reports / Business Units / Preferences. Left for the showcase maintainers rather than silently rewriting an example app's permission model. Suites: lint 447, cli 642 passing; eslint, nul-bytes and api-surface clean. Claude-Session: https://claude.ai/code/session_01GBks5G7AkwrvgF2kL5rfkT Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3167e29 commit f2b8ac9

8 files changed

Lines changed: 445 additions & 0 deletions

File tree

.changeset/nav-access-lint.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/lint': minor
3+
'@objectstack/cli': minor
4+
---
5+
6+
Navigation reachability vs. granted access (issue #3583, assessment R5)
7+
8+
`validate-nav-access` joins what an app's navigation exposes against
9+
`buildAccessMatrix` — the first lint consumer of the ADR-0090 D6 matrix, which
10+
previously only backed `os compile`'s snapshot gate. An object in the menu that
11+
no permission set grants read on renders as an entry and then fails
12+
permission-denied when opened: it works while you browse as an administrator
13+
(the platform's built-in `admin_full_access` carries a wildcard grant) and
14+
breaks for exactly the users the app ships permission sets for.
15+
16+
Advisory severity — a grant can legitimately come from a permission set another
17+
installed package ships. Quiet by construction in three cases: platform-provided
18+
objects (their own packages grant them), stacks that declare no permission sets
19+
at all (permissions managed elsewhere, so flagging every entry says nothing),
20+
and any stack where a set carries a wildcard `objects: { '*': … }` grant — the
21+
shape `admin_full_access` itself uses, which the access matrix records under the
22+
literal key `*`.
23+
24+
Wired into `os validate`, `os lint`, and `os compile`.

content/docs/deployment/validating-metadata.mdx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,24 @@ An axis naming a measure the dataset declares but this chart does not *select*
159159
nothing. The react `<ObjectChart>` block is object-bound rather than
160160
dataset-bound and is not checked here.
161161

162+
### 7. Navigation exposing objects nobody can read
163+
164+
Navigation and permissions are separate metadata, each valid on its own — so an
165+
app can put an object in its menu that no permission set grants read on. The
166+
entry renders; opening it fails permission-denied for everyone except a holder
167+
of the platform's built-in wildcard admin set. It works while you browse as an
168+
administrator and breaks for the users the app ships permission sets for.
169+
170+
```ts
171+
navigation: [{ id: 'nav_forecast', type: 'object', objectName: 'crm_forecast' }]
172+
// …and no permission set lists `crm_forecast` under `objects` → warning
173+
```
174+
175+
Advisory: the grant may come from a permission set another installed package
176+
ships. Skipped for platform-provided objects (whose own packages grant them),
177+
for stacks that declare no permission sets at all, and when any set carries a
178+
wildcard (`objects: { '*': … }`) grant.
179+
162180
## The one gate, two entry points
163181

164182
`os validate` and `os build` (alias of `os compile`) run the **same** validator:
@@ -172,6 +190,7 @@ dataset-bound and is not checked here.
172190
| Object & action name references (#3583) |||
173191
| Page-component field bindings (#3583) |||
174192
| Chart bindings outside dashboards (#3583) |||
193+
| Navigation vs. granted access (ADR-0090 D6) |||
175194
| Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) |||
176195
| Emits `dist/objectstack.json` |||
177196

packages/cli/src/commands/compile.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { validateDashboardActionRefs } from '@objectstack/lint';
1515
import { validateFilterTokens } from '@objectstack/lint';
1616
import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint';
1717
import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint';
18+
import { validateNavAccess } from '@objectstack/lint';
1819
import { validateResponsiveStyles } from '@objectstack/lint';
1920
import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
2021
import { validateReadonlyFlowWrites } from '@objectstack/lint';
@@ -340,6 +341,7 @@ export default class Compile extends Command {
340341
...validateActionNameRefs(result.data as Record<string, unknown>),
341342
...validatePageFieldBindings(result.data as Record<string, unknown>),
342343
...validateChartBindings(result.data as Record<string, unknown>),
344+
...validateNavAccess(result.data as Record<string, unknown>),
343345
];
344346
const refErrors = refFindings.filter((f) => f.severity === 'error');
345347
const refWarnings = refFindings.filter((f) => f.severity === 'warning');

packages/cli/src/commands/lint.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { validateWidgetBindings } from '@objectstack/lint';
1212
import { validateRecordTitle, validateSemanticRoles, validateCapabilityReferences, validateSecurityPosture, validateOrgAxisRedLines, validateApprovalApprovers, validateSeedReplaySafety, validateSeedStateMachine } from '@objectstack/lint';
1313
import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint';
1414
import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint';
15+
import { validateNavAccess } from '@objectstack/lint';
1516
import { collectAndLintDocs } from '../utils/collect-docs.js';
1617
import { scoreMetadata } from '../lint/score.js';
1718
import { runMetadataEval } from '../lint/metadata-eval.js';
@@ -551,6 +552,21 @@ export function lintConfig(config: any): LintIssue[] {
551552
});
552553
}
553554

555+
// ── Navigation reachability vs. granted access (ADR-0090 D6, issue #3583) ──
556+
// Navigation and permissions are separate metadata, so an app can expose an
557+
// object no permission set grants read on: the entry renders and the click
558+
// fails permission-denied for every user, including admins. Advisory — the
559+
// grant may come from a set another installed package ships.
560+
for (const t of validateNavAccess(config)) {
561+
issues.push({
562+
severity: t.severity,
563+
rule: t.rule,
564+
message: `${t.where}: ${t.message}`,
565+
path: t.path,
566+
fix: t.hint,
567+
});
568+
}
569+
554570
return issues;
555571
}
556572

packages/cli/src/commands/validate.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { validateDashboardActionRefs } from '@objectstack/lint';
1616
import { validateFilterTokens } from '@objectstack/lint';
1717
import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint';
1818
import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint';
19+
import { validateNavAccess } from '@objectstack/lint';
1920
import { validateResponsiveStyles } from '@objectstack/lint';
2021
import { validateJsxPages, validateReactPages, validateReactPageProps, validatePageSourceStyling } from '@objectstack/lint';
2122
import { validateCapabilityReferences } from '@objectstack/lint';
@@ -302,6 +303,7 @@ export default class Validate extends Command {
302303
...validateActionNameRefs(result.data as Record<string, unknown>),
303304
...validatePageFieldBindings(result.data as Record<string, unknown>),
304305
...validateChartBindings(result.data as Record<string, unknown>),
306+
...validateNavAccess(result.data as Record<string, unknown>),
305307
];
306308
const refErrors = refFindings.filter((f) => f.severity === 'error');
307309
const refWarnings = refFindings.filter((f) => f.severity === 'warning');

packages/lint/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,4 +203,7 @@ export {
203203
} from './validate-chart-bindings.js';
204204
export type { ChartBindingFinding, ChartBindingSeverity } from './validate-chart-bindings.js';
205205

206+
export { validateNavAccess, NAV_OBJECT_UNGRANTED } from './validate-nav-access.js';
207+
export type { NavAccessFinding, NavAccessSeverity } from './validate-nav-access.js';
208+
206209
export { buildAccessMatrix, diffAccessMatrix } from './build-access-matrix.js';
Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { validateNavAccess, NAV_OBJECT_UNGRANTED } from './validate-nav-access.js';
5+
6+
const objects = [
7+
{ name: 'crm_lead', fields: { name: { type: 'text' } } },
8+
{ name: 'crm_forecast', fields: { name: { type: 'text' } } },
9+
];
10+
11+
const navApp = (items: unknown[]) => [{ name: 'crm', label: 'CRM', navigation: items }];
12+
13+
const objectNav = (id: string, objectName: string) => ({
14+
id,
15+
type: 'object',
16+
label: objectName,
17+
objectName,
18+
});
19+
20+
describe('validateNavAccess', () => {
21+
// The HotCRM instance: an object in the menu that no set grants.
22+
it('warns on a nav-exposed object no permission set grants read on', () => {
23+
const findings = validateNavAccess({
24+
objects,
25+
apps: navApp([objectNav('nav_leads', 'crm_lead'), objectNav('nav_forecast', 'crm_forecast')]),
26+
permissions: [
27+
{ name: 'crm_user', label: 'CRM User', objects: { crm_lead: { allowRead: true } } },
28+
],
29+
});
30+
expect(findings).toHaveLength(1);
31+
expect(findings[0].severity).toBe('warning');
32+
expect(findings[0].rule).toBe(NAV_OBJECT_UNGRANTED);
33+
expect(findings[0].path).toBe('apps[0].navigation[1].objectName');
34+
expect(findings[0].message).toContain('crm_forecast');
35+
});
36+
37+
// The platform's own `admin_full_access` uses this shape; a stack may too.
38+
// Without wildcard support the matrix records the literal key `*` and the
39+
// rule would fire on exactly the stacks that granted the most.
40+
it('accepts a wildcard read grant as covering every object', () => {
41+
const findings = validateNavAccess({
42+
objects,
43+
apps: navApp([objectNav('nav_forecast', 'crm_forecast')]),
44+
permissions: [
45+
{ name: 'full', label: 'Full', objects: { '*': { allowRead: true, viewAllRecords: true } } },
46+
],
47+
});
48+
expect(findings).toEqual([]);
49+
});
50+
51+
it('accepts read granted through viewAllRecords or modifyAllRecords', () => {
52+
for (const bit of ['viewAllRecords', 'modifyAllRecords']) {
53+
const findings = validateNavAccess({
54+
objects,
55+
apps: navApp([objectNav('nav_forecast', 'crm_forecast')]),
56+
permissions: [
57+
{ name: 'admin', label: 'Admin', objects: { crm_forecast: { [bit]: true } } },
58+
],
59+
});
60+
expect(findings, `${bit} should grant read`).toEqual([]);
61+
}
62+
});
63+
64+
it('does not treat a write-only grant as read', () => {
65+
const findings = validateNavAccess({
66+
objects,
67+
apps: navApp([objectNav('nav_forecast', 'crm_forecast')]),
68+
permissions: [
69+
{ name: 'writer', label: 'Writer', objects: { crm_forecast: { allowCreate: true, allowEdit: true } } },
70+
],
71+
});
72+
expect(findings).toHaveLength(1);
73+
});
74+
75+
it('reports an object once even when several nav entries expose it', () => {
76+
const findings = validateNavAccess({
77+
objects,
78+
apps: [
79+
{
80+
name: 'crm',
81+
navigation: [objectNav('nav_a', 'crm_forecast')],
82+
areas: [{ id: 'area', label: 'Area', navigation: [objectNav('nav_b', 'crm_forecast')] }],
83+
},
84+
],
85+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
86+
});
87+
expect(findings).toHaveLength(1);
88+
});
89+
90+
it('walks area navigation and nested children', () => {
91+
const findings = validateNavAccess({
92+
objects,
93+
apps: [
94+
{
95+
name: 'crm',
96+
areas: [
97+
{
98+
id: 'area_sales',
99+
label: 'Sales',
100+
navigation: [
101+
{
102+
id: 'grp',
103+
type: 'group',
104+
label: 'Group',
105+
children: [objectNav('nav_forecast', 'crm_forecast')],
106+
},
107+
],
108+
},
109+
],
110+
},
111+
],
112+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
113+
});
114+
expect(findings).toHaveLength(1);
115+
expect(findings[0].path).toBe('apps[0].areas[0].navigation[0].children[0].objectName');
116+
});
117+
});
118+
119+
describe('validateNavAccess — exemptions (false-positive floor)', () => {
120+
it('skips platform-provided objects — their own packages grant them', () => {
121+
const findings = validateNavAccess({
122+
objects,
123+
apps: navApp([
124+
objectNav('nav_users', 'sys_user'),
125+
objectNav('nav_approvals', 'sys_approval_request'),
126+
]),
127+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
128+
});
129+
expect(findings).toEqual([]);
130+
});
131+
132+
it('skips a stack that declares no permission sets at all', () => {
133+
const findings = validateNavAccess({
134+
objects,
135+
apps: navApp([objectNav('nav_forecast', 'crm_forecast')]),
136+
});
137+
expect(findings).toEqual([]);
138+
});
139+
140+
it('skips an object this stack does not define', () => {
141+
// A dangling nav target is `validate-object-references`' finding, not this
142+
// rule's — reporting it here would double-report one mistake.
143+
const findings = validateNavAccess({
144+
objects,
145+
apps: navApp([objectNav('nav_ghost', 'crm_ghost')]),
146+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
147+
});
148+
expect(findings).toEqual([]);
149+
});
150+
151+
it('ignores non-object nav entries', () => {
152+
const findings = validateNavAccess({
153+
objects,
154+
apps: navApp([
155+
{ id: 'nav_dash', type: 'dashboard', label: 'D', dashboardName: 'd' },
156+
{ id: 'nav_url', type: 'url', label: 'U', url: '/x' },
157+
{ id: 'nav_sep', type: 'separator' },
158+
]),
159+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
160+
});
161+
expect(findings).toEqual([]);
162+
});
163+
164+
it('is silent when every exposed object is granted, and tolerates empty input', () => {
165+
const findings = validateNavAccess({
166+
objects,
167+
apps: navApp([objectNav('nav_leads', 'crm_lead'), objectNav('nav_forecast', 'crm_forecast')]),
168+
permissions: [
169+
{
170+
name: 'p',
171+
label: 'P',
172+
objects: { crm_lead: { allowRead: true }, crm_forecast: { allowRead: true } },
173+
},
174+
],
175+
});
176+
expect(findings).toEqual([]);
177+
expect(validateNavAccess({})).toEqual([]);
178+
expect(validateNavAccess(null as unknown as Record<string, unknown>)).toEqual([]);
179+
});
180+
181+
it('accepts a grant coming from any one of several sets', () => {
182+
const findings = validateNavAccess({
183+
objects,
184+
apps: navApp([objectNav('nav_forecast', 'crm_forecast')]),
185+
permissions: [
186+
{ name: 'a', label: 'A', objects: { crm_lead: { allowRead: true } } },
187+
{ name: 'b', label: 'B', objects: { crm_forecast: { allowRead: true } } },
188+
],
189+
});
190+
expect(findings).toEqual([]);
191+
});
192+
});

0 commit comments

Comments
 (0)