fix(claude-config): rename scan-root override env var

[kyle-sexton]

Fixes #2283 (row A11).

Introduces $PERMISSION_HYGIENE_SCAN_ROOT as the operator-facing scan-root override. $PERMISSION_HYGIENE_FIXTURE_DIR remains accepted as a deprecated alias.

Verification

• permission-rule-check.test.sh: 87 checks passed

Related

• fix(claude-config): audit-permission-grants reports a clean bill with no denominator, and four amplifiers around it #2283 — A5/A8/A15/A16 remain open

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 38s —— View job

---

Security Review

• Gather context (PR diff, changed files)
• Review plugin.json version bump
• Review permission-rule-check.sh env var rename/alias logic
• Review SKILL.md and test script changes
• Post findings

Scope: This PR only touches plugins/claude-config/ — a version bump, changelog entry, doc wording, and a local shell audit script (no GitHub Actions workflow files are modified).

Summary: No security issues found.

The change renames the operator-facing override from $PERMISSION_HYGIENE_FIXTURE_DIR to $PERMISSION_HYGIENE_SCAN_ROOT, keeping the old name as a deprecated fallback alias:

if [[ -n "${PERMISSION_HYGIENE_SCAN_ROOT:-${PERMISSION_HYGIENE_FIXTURE_DIR:-}}" ]]; then
  ROOT="${PERMISSION_HYGIENE_SCAN_ROOT:-$PERMISSION_HYGIENE_FIXTURE_DIR}"

permission-rule-check.sh#L98-L99

This is a behavior-preserving rename:

• Both variables feed the same ROOT variable through the same code path that already existed pre-PR (quoted throughout, used only as a find/jq directory argument — no new shell-injection or path-traversal surface introduced).
• The env var is operator-supplied (a local CLI tool run by the user against their own filesystem), not attacker-controlled input crossing a trust boundary.
• No change to permissions, no CI/workflow files touched, no secrets or credentials involved.
• The added test case (permission-rule-check.test.sh Case 8f) exercises the new variable name against a narrow Bash(npm test) rule and correctly asserts 0 findings.

Nothing in this diff falls into CRITICAL/IMPORTANT/SUGGESTION territory from a security standpoint.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe3476f369

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 28s —— View job

---

Code review complete

• Gather context (diff, CLAUDE.md, REVIEW.md)
• Review plugin.json version bump
• Review CHANGELOG.md
• Review SKILL.md docs update
• Review permission-rule-check.sh logic changes
• Review permission-rule-check.test.sh new tests
• Post findings

Scope: correctness, design, error handling, tests, docs (this repo has a claude-security-review.yml workflow, so security review is left to that lane per REVIEW.md).

Summary

This is a clean, well-scoped rename: $PERMISSION_HYGIENE_SCAN_ROOT becomes the operator-facing override, with $PERMISSION_HYGIENE_FIXTURE_DIR kept as a deprecated alias. All four prose surfaces (script header comment, --help text, refusal message, SKILL.md) were updated consistently, and I found no other file in the repo referencing the old name that was missed.

The fallback logic in permission-rule-check.sh:98-99 —

if [[ -n "${PERMISSION_HYGIENE_SCAN_ROOT:-${PERMISSION_HYGIENE_FIXTURE_DIR:-}}" ]]; then
  ROOT="${PERMISSION_HYGIENE_SCAN_ROOT:-$PERMISSION_HYGIENE_FIXTURE_DIR}"

correctly gives the new name precedence over the deprecated one when both are set, and the condition/assignment stay in sync (both use the same :- chain), so there's no drift risk between the "should I use the fallback" check and the actual value used. Backward compatibility is exercised implicitly throughout the existing suite, since every pre-existing test helper (run, run_with_home, run_with_config_dir) still drives the script via PERMISSION_HYGIENE_FIXTURE_DIR.

I left two minor, non-blocking notes inline on the test file — a case-label ordering nit, and a suggestion to add explicit coverage for the new precedence rule (new var wins when both old and new are set simultaneously), since this file otherwise tests precedence rules like this explicitly (e.g. the CLAUDE_CONFIG_DIR vs $HOME case).

Not independently verified: I did not run permission-rule-check.test.sh myself (no tool access to execute it in this environment), so the PR description's "87 checks passed" is author-claimed and unverified by this review. The logic reads correct on inspection.
· branch cursor/permission-scan-root-alias-63e1

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 1m 8s —— View job

---

Security Review

• Gather context (gh pr diff / git diff origin/main...HEAD)
• Review permission-rule-check.sh env var rename/alias logic
• Review SKILL.md, CHANGELOG.md, plugin.json changes
• Review permission-rule-check.test.sh changes
• Post findings

Scope: plugins/claude-config/ only — a version bump, changelog entry, doc wording, a local shell audit script, and its test file. No GitHub Actions workflow files are touched.

Findings: none.

This is a behavior-preserving rename of the operator-facing scan-root override from $PERMISSION_HYGIENE_FIXTURE_DIR to $PERMISSION_HYGIENE_SCAN_ROOT, with the old name kept as a deprecated fallback alias:

if [[ -n "${PERMISSION_HYGIENE_SCAN_ROOT:-${PERMISSION_HYGIENE_FIXTURE_DIR:-}}" ]]; then
  ROOT="${PERMISSION_HYGIENE_SCAN_ROOT:-$PERMISSION_HYGIENE_FIXTURE_DIR}"

permission-rule-check.sh#L98-L99

I traced every use of $ROOT downstream of this assignment (L102, L108, L125-126, L264, L269, L291-292, L314, L335, L338) — it is consistently double-quoted in every find/jq/string-slice use, both before and after this PR, so the new variable name feeds the exact same guarded code path with no new command-injection or path-traversal surface.

Other considerations:

• The value is operator-supplied (a local CLI tool run against the operator's own filesystem), not input crossing a trust boundary from an untrusted party.
• No permissions, settings schema, or CI/workflow files are touched — the diff is limited to a version bump, changelog/doc prose, and the shell script + its test file.
• The --count/exit-2 refusal semantics for an unresolved root are unchanged; the new variable only widens what can populate ROOT, it doesn't relax the existing "refuse rather than sweep an unknown tree" guard.

Nothing in this diff rises to CRITICAL/IMPORTANT/SUGGESTION on security grounds. (Note: this review is scoped to security only, per this lane's instructions — it doesn't cover the correctness/style feedback already left by other reviewers on this PR.)
· branch cursor/permission-scan-root-alias-63e1

[github-actions]

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/31589556794
• Failure class: other
• Last SDK result: (no execution file was produced)

The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging.

Re-run the job to retry the review; a new push also retries it only if the caller's pull_request triggers include synchronize (the canonical security caller keeps it). An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator.

Re-running does NOT help for every class:

• rate-limit that persists across re-runs, or auth — the credential or usage budget needs an operator; retrying will not clear it.
• a run that exhausted its turn budget ("subtype":"error_max_turns" above) will exhaust it again. As the PR author, split the change into smaller PRs; raising --max-turns is a change to the caller workflow, not something you can set on this PR.

[github-actions]

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/31589556689
• Failure class: other
• Last SDK result: (no execution file was produced)

Re-run the job to retry the review. A new push re-triggers this lane only if the caller's pull_request triggers include synchronize (the canonical caller omits it).
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).