[REVIEW] Permissions-Policy: `recommendations` always populated — spurious "Fix:" output when status is `good`

[BodenMcHale]

Summary

checkPermissionsPolicy in src/rules.ts unconditionally populates the recommendations array with a generic remediation string, even when isGood === true — i.e. when the header already restricts camera, microphone, and geolocation. A developer who has correctly set Permissions-Policy: camera=(), microphone=(), geolocation=() receives a status: 'good' result with an empty findings[], but also a non-empty recommendations[] telling them to set the very policy they already have. Every other rule in the codebase returns recommendations: [] when its status is 'good'; this is the only exception.

Investigation Path

1. Read all six source files: src/rules.ts, src/analyzer.ts, src/fetch.ts, src/index.ts, src/cli.ts, src/types.ts.
2. Audited every return statement in each check* function for structural consistency — specifically that findings and recommendations are both empty arrays when status is 'good'.
3. All five other rules are consistent:
   • checkHSTS — findings and recommendations are only pushed when a condition fails.
   • checkCSP — starts with const recommendations: string[] = []; pushes conditionally.
   • checkXFrameOptions — findings: [], recommendations: [] on the good path.
   • checkXContentTypeOptions — recommendations: score < 10 ? [...] : [].
   • checkReferrerPolicy — recommendations: isStrong ? [] : [...].
4. Found the single exception at src/rules.ts line 141: checkPermissionsPolicy uses findings: isGood ? [] : [...] (correct) but recommendations: ["Set Permissions-Policy to..."] (unconditional — no ternary guard).
5. Traced the effect through src/cli.ts line 61: for (const rec of h.recommendations) console.log(...) — confirms the "Fix:" line prints regardless of status.
6. Ran all three deduplication searches against open and recently-closed issues (searched: "Permissions-Policy recommendations", "good status fix recommendation", "spurious Fix recommendation false positive output") — zero matches.

Evidence

• File: src/rules.ts lines 121–143, commit 81f8735d167b597ed7a7a88cc7f89ba0b440b07d

export function checkPermissionsPolicy(headers: RawHeaders): HeaderFinding {
  const raw = getHeader(headers, 'permissions-policy') ?? getHeader(headers, 'feature-policy');
  if (!raw) return { /* ... score: 0, status: 'missing' ... */ };

  const lc = raw.toLowerCase();
  const hasCam = lc.includes("camera=()");
  const hasMic = lc.includes("microphone=()");
  const hasGeo = lc.includes("geolocation=()");
  const score = (hasCam && hasMic && hasGeo) ? 10 : 5;
  const isGood = score === 10;

  return {
    header: "Permissions-Policy",
    score,
    maxScore: 10,
    status: isGood ? "good" : "warning",
    raw,
    findings: isGood ? [] : ["Permissions-Policy does not restrict at least camera, microphone, and geolocation"],
    recommendations: ["Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app"]
    //                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    //                 No ternary guard — always populated, even when isGood === true
  };
}

• Current behavior: A correctly configured header returns:

checkPermissionsPolicy({
  'permissions-policy': 'camera=(), microphone=(), geolocation=()',
})
// =>
{
  header: 'Permissions-Policy',
  score: 10,
  maxScore: 10,
  status: 'good',       // ← correct
  findings: [],         // ← correct
  recommendations: [    // ← INCORRECT: should be []
    'Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app'
  ]
}

• Expected behavior: recommendations: [] when isGood === true, matching the contract of every other check function.

• CLI impact (src/cli.ts line 61):

✓ Permissions-Policy (10/10)
  camera=(), microphone=(), geolocation=()
  Fix: Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app

A developer sees a green ✓ immediately followed by an apparently required "Fix:" — contradictory output.

• Reference: The structural contract is implicit in every other rule and explicit in the HeaderFinding interface (src/types.ts), where findings and recommendations are described as per-header guidance; both should be empty when the header is correctly configured.

Impact

Developers who have correctly configured Permissions-Policy receive contradictory, noise-introducing output. Two concrete consequences:

1. CLI confusion. The color-coded ✓ (good) and the Fix: recommendation appear on consecutive lines for the same header. A developer reading the output quickly may either (a) wonder if they need to change something that is already correct, potentially regressing their configuration, or (b) dismiss all Fix: lines as low-signal, undermining trust in genuine warnings from other headers.

2. False positives for API consumers. Any library user filtering by h.recommendations.length > 0 to build automated remediation pipelines, CI annotations, or dashboard alerts will receive spurious Permissions-Policy alerts for every site that has already implemented the recommended policy — exactly the sites that should be silent.

This is the most common failure mode for "passed our header audit" workflows: the tool flags something as needing fixing that has already been fixed.

Suggested Remediation

Add the same ternary guard that checkReferrerPolicy already uses (src/rules.ts line 117):

// src/rules.ts — line 141
// Before:
recommendations: ["Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app"]

// After:
recommendations: isGood ? [] : ["Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app"]

This is a one-line, zero-risk change with no logic impact on the warning or missing paths.

Acceptance Criteria

• checkPermissionsPolicy({ 'permissions-policy': 'camera=(), microphone=(), geolocation=()' }) returns recommendations: []
• checkPermissionsPolicy({ 'permissions-policy': 'camera=()' }) continues to return a non-empty recommendations (warning path is unaffected)
• checkPermissionsPolicy({}) continues to return status: 'missing' with a non-empty recommendations (missing path is unaffected)
• A new test assertion is added: expect(checkPermissionsPolicy({ 'permissions-policy': 'camera=(), microphone=(), geolocation=()' }).recommendations).toEqual([])
• CLI output for a fully-restricted Permissions-Policy shows no Fix: line

---

Claude Code review routine · commit 81f8735d167b597ed7a7a88cc7f89ba0b440b07d · 2026-05-26T00:00:00Z

[BodenMcHale]

Triage Review

Reviewed at: 2026-05-26T12:00:00Z
Commit checked: 81f8735
Version: 1.0.1

Verification

Confirmed. The bug is present at HEAD with no fix in any branch.

src/rules.ts:140–141 — the exact asymmetry:

findings:        isGood ? [] : ["Permissions-Policy does not restrict at least camera, microphone, and geolocation"],
recommendations: ["Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app"]
//               ^^^ no ternary guard — unconditionally populated

findings correctly guards on isGood; recommendations does not. Every other check function in the file returns recommendations: [] on its good path — confirmed by reading all six check* functions in src/rules.ts.

Test gap confirmed. The existing test at test/analyzer.test.ts:269–272 checks only score and status for the good path — there is no assertion on recommendations:

it('present returns score 10', () => {
  const r = checkPermissionsPolicy({ 'permissions-policy': 'camera=(), microphone=(), geolocation=()' });
  expect(r.score).toBe(10);
  expect(r.status).toBe('good');
  // recommendations is never asserted
});

CLI impact confirmed. src/cli.ts:61:

for (const rec of h.recommendations) console.log(`  ${D}  Fix: ${rec}${R}`);

This loop has no status guard — it prints every recommendations entry regardless of whether the header is good, warning, or missing. A developer with a correctly configured Permissions-Policy: camera=(), microphone=(), geolocation=() sees:

✓ Permissions-Policy (10/10)
  camera=(), microphone=(), geolocation=()
  Fix: Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app

The green ✓ and the Fix: are contradictory on consecutive lines.

Root Cause

The checkPermissionsPolicy function was written as part of the strictness update in commit 8d29a8c. The findings field was given the isGood ternary, but the corresponding ternary on recommendations was omitted — likely an oversight during the same edit. No commit since 8d29a8c has touched this return statement.

Suggested Fix

One-character-class change at src/rules.ts:141:

// Before
recommendations: ["Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app"]

// After
recommendations: isGood ? [] : ["Set Permissions-Policy to camera=(), microphone=(), geolocation=(), and any other features needed by your app"]

Risk: zero. The isGood variable is already in scope from line 133. The warning and missing paths are structurally identical after this change.

Add one assertion to the existing 'present returns score 10' test:

expect(r.recommendations).toEqual([]);

No other test changes needed — the warning and missing paths already behave correctly and their tests will continue to pass.

Reference

• src/rules.ts:117 — checkReferrerPolicy uses the identical ternary pattern that this function is missing: recommendations: isStrong ? [] : ['Use: strict-origin-when-cross-origin']
• src/types.ts — HeaderFinding.recommendations is described as per-header guidance; an empty array is the correct value when the header requires no action

---

Claude Code triage routine · auto-generated

---

Generated by Claude Code