fix(lint): nav-object-ungranted hint drops the gating remedy it cannot honour (#16436)

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.


Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8

Co-authored-by: Claude <noreply@anthropic.com>

Author: baozhoutao

.changeset/nav-object-ungranted-hint-drops-gating.md

@@ -0,0 +1,5 @@
+---
+"@objectstack/lint": patch
+---
+
+`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

@@ -190,3 +190,61 @@ describe('validateNavAccess — exemptions (false-positive floor)', () => {
     expect(findings).toEqual([]);
   });
 });
+
+// #16065: the hint used to prescribe a third remedy — "gate the entry with
+// requiredPermissions/visible" — that this rule cannot honour: neither key is
+// read anywhere in this file, so a gated-but-ungranted entry kept warning
+// forever. The fix corrects the hint (and the doc-block) to the two remedies
+// the rule can actually prove; it does NOT make the rule start reading those
+// keys. These controls pin that: gating must not silence a genuinely
+// ungranted object, and must not spuriously flag a genuinely granted one.
+describe('validateNavAccess — gating keys are not read (#16065)', () => {
+  it('hint prescribes only the two remedies it can prove, not gating', () => {
+    const findings = validateNavAccess({
+      objects,
+      apps: navApp([objectNav('nav_forecast', 'crm_forecast')]),
+      permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
+    });
+    expect(findings).toHaveLength(1);
+    const { hint } = findings[0];
+    // The two remedies the rule can prove.
+    expect(hint).toContain('allowRead: true');
+    expect(hint).toContain('viewAllRecords');
+    // The retired third remedy must not return.
+    expect(hint).not.toContain('gate the entry');
+    expect(hint).not.toMatch(/requiredPermissions.*if it is meant for admins only/);
+  });
+
+  it('a requiredPermissions-gated but ungranted object still fires (gating is not a remedy)', () => {
+    const findings = validateNavAccess({
+      objects,
+      apps: navApp([
+        { ...objectNav('nav_forecast', 'crm_forecast'), requiredPermissions: ['view_forecast'] },
+      ]),
+      permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
+    });
+    expect(findings).toHaveLength(1);
+    expect(findings[0].rule).toBe(NAV_OBJECT_UNGRANTED);
+  });
+
+  it('a visible:false but ungranted object still fires (gating is not a remedy)', () => {
+    const findings = validateNavAccess({
+      objects,
+      apps: navApp([{ ...objectNav('nav_forecast', 'crm_forecast'), visible: false }]),
+      permissions: [{ name: 'p', label: 'P', objects: { crm_lead: { allowRead: true } } }],
+    });
+    expect(findings).toHaveLength(1);
+    expect(findings[0].rule).toBe(NAV_OBJECT_UNGRANTED);
+  });
+
+  it('a gated AND granted object stays silent — gating causes no false positive either', () => {
+    const findings = validateNavAccess({
+      objects,
+      apps: navApp([
+        { ...objectNav('nav_forecast', 'crm_forecast'), requiredPermissions: ['view_forecast'] },
+      ]),
+      permissions: [{ name: 'p', label: 'P', objects: { crm_forecast: { allowRead: true } } }],
+    });
+    expect(findings).toEqual([]);
+  });
+});

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

@@ -25,6 +25,13 @@
  * entry — it is only *suspicious*, and the ceiling for a static check is a
  * warning (the same posture `validate-capability-references` takes).
  *
+ * The only remedies that clear this finding are granting read (a permission set's
+ * `objects` entry with `allowRead: true` or `viewAllRecords`) or dropping the nav
+ * entry. Gating the item with `requiredPermissions` or `visible` does not: those
+ * gate on capabilities/visibility, not the object grant, so a holder who clears
+ * the gate can still lack read and hit permission-denied — honouring either key
+ * here would silence the rule on an object that is genuinely reachable.
+ *
  * Two exemptions keep it quiet:
  *   - **Platform-provided objects** (`sys_user`, `sys_approval_request`, …) are
  *     skipped: the packages that register them ship their own permission sets,
@@ -170,9 +177,10 @@ export function validateNavAccess(stack: AnyRec): NavAccessFinding[] {
         `and breaks for the users the app ships permission sets for.`,
       hint:
         `Add "${objectName}" to a permission set's \`objects\` with \`allowRead: true\` ` +
-        `(or \`viewAllRecords\`), gate the entry with \`requiredPermissions\`/\`visible\` ` +
-        `if it is meant for admins only, or drop it. Ignore this if a permission set ` +
-        `from another installed package grants it.`,
+        `(or \`viewAllRecords\`), or drop the entry. Gating it with \`requiredPermissions\`/` +
+        `\`visible\` does not clear this: those gate visibility, not the object grant, so a ` +
+        `holder can still open the entry and hit permission-denied. Ignore this if a ` +
+        `permission set from another installed package grants it.`,
     });
   }