Skip to content

Commit 3bd1d1b

Browse files
committed
fix(lint): nav-object-ungranted hint drops the gating remedy it cannot honour
The hint (and module doc-block) prescribed gating a nav entry with requiredPermissions/visible as a way to clear the finding. Neither key is read by the rule, so following that advice left the warning firing forever. Corrected to the two remedies the rule can prove: grant read on the object, or drop the entry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
1 parent 693fbcb commit 3bd1d1b

3 files changed

Lines changed: 74 additions & 3 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
`nav-object-ungranted`'s hint no longer tells you to gate the nav entry with `requiredPermissions`/`visible` — that never cleared the finding, because the rule never reads either key. Gating restricts who can see the entry; it doesn't grant the object read, so a holder who clears the gate could still hit permission-denied, and the warning kept firing anyway. The hint (and the module doc-block) now name the two remedies that actually clear it: grant read on the object in a permission set (`allowRead: true` or `viewAllRecords`), or drop the nav entry. No behavior change — the rule fires and stays silent on exactly the same inputs as before; only the wording of the hint moved.

packages/lint/src/validate-nav-access.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,3 +190,61 @@ describe('validateNavAccess — exemptions (false-positive floor)', () => {
190190
expect(findings).toEqual([]);
191191
});
192192
});
193+
194+
// #16065: the hint used to prescribe a third remedy — "gate the entry with
195+
// requiredPermissions/visible" — that this rule cannot honour: neither key is
196+
// read anywhere in this file, so a gated-but-ungranted entry kept warning
197+
// forever. The fix corrects the hint (and the doc-block) to the two remedies
198+
// the rule can actually prove; it does NOT make the rule start reading those
199+
// keys. These controls pin that: gating must not silence a genuinely
200+
// ungranted object, and must not spuriously flag a genuinely granted one.
201+
describe('validateNavAccess — gating keys are not read (#16065)', () => {
202+
it('hint prescribes only the two remedies it can prove, not gating', () => {
203+
const findings = validateNavAccess({
204+
objects,
205+
apps: navApp([objectNav('nav_forecast', 'crm_forecast')]),
206+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
207+
});
208+
expect(findings).toHaveLength(1);
209+
const { hint } = findings[0];
210+
// The two remedies the rule can prove.
211+
expect(hint).toContain('allowRead: true');
212+
expect(hint).toContain('viewAllRecords');
213+
// The retired third remedy must not return.
214+
expect(hint).not.toContain('gate the entry');
215+
expect(hint).not.toMatch(/requiredPermissions.*if it is meant for admins only/);
216+
});
217+
218+
it('a requiredPermissions-gated but ungranted object still fires (gating is not a remedy)', () => {
219+
const findings = validateNavAccess({
220+
objects,
221+
apps: navApp([
222+
{ ...objectNav('nav_forecast', 'crm_forecast'), requiredPermissions: ['view_forecast'] },
223+
]),
224+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
225+
});
226+
expect(findings).toHaveLength(1);
227+
expect(findings[0].rule).toBe(NAV_OBJECT_UNGRANTED);
228+
});
229+
230+
it('a visible:false but ungranted object still fires (gating is not a remedy)', () => {
231+
const findings = validateNavAccess({
232+
objects,
233+
apps: navApp([{ ...objectNav('nav_forecast', 'crm_forecast'), visible: false }]),
234+
permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
235+
});
236+
expect(findings).toHaveLength(1);
237+
expect(findings[0].rule).toBe(NAV_OBJECT_UNGRANTED);
238+
});
239+
240+
it('a gated AND granted object stays silent — gating causes no false positive either', () => {
241+
const findings = validateNavAccess({
242+
objects,
243+
apps: navApp([
244+
{ ...objectNav('nav_forecast', 'crm_forecast'), requiredPermissions: ['view_forecast'] },
245+
]),
246+
permissions: [{ name: 'p', label: 'P', objects: { crm_forecast: { allowRead: true } } }],
247+
});
248+
expect(findings).toEqual([]);
249+
});
250+
});

packages/lint/src/validate-nav-access.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@
2525
* entry — it is only *suspicious*, and the ceiling for a static check is a
2626
* warning (the same posture `validate-capability-references` takes).
2727
*
28+
* The only remedies that clear this finding are granting read (a permission set's
29+
* `objects` entry with `allowRead: true` or `viewAllRecords`) or dropping the nav
30+
* entry. Gating the item with `requiredPermissions` or `visible` does not: those
31+
* gate on capabilities/visibility, not the object grant, so a holder who clears
32+
* the gate can still lack read and hit permission-denied — honouring either key
33+
* here would silence the rule on an object that is genuinely reachable.
34+
*
2835
* Two exemptions keep it quiet:
2936
* - **Platform-provided objects** (`sys_user`, `sys_approval_request`, …) are
3037
* skipped: the packages that register them ship their own permission sets,
@@ -170,9 +177,10 @@ export function validateNavAccess(stack: AnyRec): NavAccessFinding[] {
170177
`and breaks for the users the app ships permission sets for.`,
171178
hint:
172179
`Add "${objectName}" to a permission set's \`objects\` with \`allowRead: true\` ` +
173-
`(or \`viewAllRecords\`), gate the entry with \`requiredPermissions\`/\`visible\` ` +
174-
`if it is meant for admins only, or drop it. Ignore this if a permission set ` +
175-
`from another installed package grants it.`,
180+
`(or \`viewAllRecords\`), or drop the entry. Gating it with \`requiredPermissions\`/` +
181+
`\`visible\` does not clear this: those gate visibility, not the object grant, so a ` +
182+
`holder can still open the entry and hit permission-denied. Ignore this if a ` +
183+
`permission set from another installed package grants it.`,
176184
});
177185
}
178186

0 commit comments

Comments
 (0)