You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Migrates the PowerShell (PSScriptAnalyzer) formatter hook into a repo-agnostic plugin. Consumer conventions from PSScriptAnalyzerSettings.psd1 (walk bounded to project dir); pwsh/module absent -> clean no-op; kill switch HOOK_POWERSHELL_FORMAT_ENABLED. Fixes an inherited case-insensitive write-back bug (-ne -> -cne so casing fixes persist). Settings-file trust model documented. 39 tests; validate clean. Clean-tier wave Phase 1.
Note
Medium Risk
The hook rewrites PowerShell files on edit and PSScriptAnalyzer may load and run custom rule modules referenced from the opt-in settings file; risk is mitigated by explicit opt-in, project-dir bounds, and advisory-only behavior.
Overview
Adds a new powershell-format marketplace plugin that runs on Write/Edit of .ps1, .psm1, and .psd1 files. It formats in place with PSScriptAnalyzer’s Invoke-Formatter and reports lint via Invoke-ScriptAnalyzer, using the consumer’s closest PSScriptAnalyzerSettings.psd1 (walk-up bounded by CLAUDE_PROJECT_DIR when set). Repos without that settings file are not touched—no built-in default rules.
Behavior matches sibling formatter hooks: advisory only (always exit 0, findings in additionalContext), HOOK_POWERSHELL_FORMAT_ENABLED kill switch, silent skip when pwsh or PSScriptAnalyzer is missing, optional telemetry, and a bundled hook-utils.sh copy. The PowerShell block fixes casing-only reformats by using -cne for write-back, preserves encoding/BOM, skips non–UTF-8 legacy files, anchors CustomRulePath at the settings directory, and documents the trust model for settings and custom rules.
marketplace.json and README.md catalog entries register the plugin. A large contract test suite covers opt-in, extensions, graceful degrade, ceiling, BOM/ANSI, CustomRulePath, and telemetry.
Reviewed by Cursor Bugbot for commit 3f57de3. Bugbot is set up for automated code reviews on this repo. Configure here.
Migrate medley's powershell-format hook into a repo-agnostic marketplace
plugin. Mirrors the ruff-format/biome-format tool-runner pattern:
- Opt-in on a consumer PSScriptAnalyzerSettings.psd1, discovered by walking
up from the edited file to the repo root (keeps the medley seam, adds
monorepo-safe per-module discovery).
- Graceful degrade: pwsh absent OR PSScriptAnalyzer module absent -> clean
silent no-op (exit 3 sentinel from the pwsh probe; exit 4 = analyzer throw
surfaces as advisory tool-break).
- Advisory only (always exit 0); kill switch HOOK_POWERSHELL_FORMAT_ENABLED;
opt-in telemetry envelope via HOOK_TELEMETRY_SINK.
- Byte-identical hook-utils.sh copy (sync-hook-utils SSOT).
Fixes a latent bug inherited from the medley baseline: PowerShell string -ne
is case-insensitive, so a casing-only reformat (PSUseCorrectCasing) never
persisted. The formatted output is now compared with case-sensitive -cne.
Self-contained contract tests (36 cases; skip cleanly without pwsh, with
deterministic pwsh-stub cases for the exit-3/exit-4 degrade paths).
…ath analyzer, document settings trust model
Security-review follow-up (ACCEPT-WITH-NOTES):
- Bound the PSScriptAnalyzerSettings.psd1 walk-up ceiling by CLAUDE_PROJECT_DIR
when set, so the settings ceiling matches the file-membership ceiling
hook::read_file_path already enforces; the git-root ceiling stays the fallback
when unset. A settings file above the project dir can no longer govern an edit.
- Analyzer call: Invoke-ScriptAnalyzer has no -LiteralPath (verified against
module 1.25.0 — only -Path, which treats [ ] * ? as wildcards). Realize the
literal-path intent via [WildcardPattern]::Escape($file) on -Path; a filename
containing wildcard metacharacters was otherwise silently skipped (verified:
raw -Path on weird[1].ps1 -> 0 findings; escaped -> 1).
- README: document the settings-file trust model. A PSScriptAnalyzerSettings.psd1
may declare CustomRulePath modules that PSScriptAnalyzer loads and executes
during analysis (Microsoft Learn, "Using PSScriptAnalyzer > Custom rules"), so
it is executed-adjacent config — trust it as you trust build/CI config.
Contract suite: 39 cases green (adds ceiling-bound + git-root-fallback contrast).
️✅ There are no secrets present in this pull request anymore.
If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
This is a well-engineered plugin migration. The security posture is sound, the -cne bug fix is correct and necessary, the settings-ceiling bound is properly implemented and tested, and the test suite is unusually thorough (stub-pwsh approach makes the graceful-degrade paths deterministic even on runners without PSScriptAnalyzer). The hook-utils.sh copy is confirmed byte-identical to lib/hook-utils.sh. Ready to merge with one security note to close out and a couple of minor observations.
This is not a secret. _cpd is a shell local that stores the canonical form of $CLAUDE_PROJECT_DIR (it's claude project dir). GitGuardian's generic-password pattern appears to have fired on the variable name _cpd + assignment structure. There is no credential or sensitive value anywhere in this file. Recommend marking the incident as a false positive in the GitGuardian dashboard and, optionally, renaming _cpd → _canonical_cpd to reduce noise on future scans (no functional impact either way).
Security Review (playbook checklist)
Surface
Status
Notes
Code execution
✅ Pass
Advisory (always exits 0), no eval/curl | sh, kill switch present, file path flows through jq parse then hook::read_file_path membership guard before use
Untrusted input in shell
✅ Pass
File and settings paths passed to pwsh via env vars (L164–165), not as CLI arguments — no word-splitting or glob expansion risk
Remote MCP
✅ N/A
No MCP server
userConfig secrets
✅ N/A
No userConfig (intentional; PSScriptAnalyzerSettings.psd1 is the config surface)
Cache isolation
✅ Pass
${CLAUDE_PLUGIN_ROOT} in hooks.json command; no ../ reach-outs
Data egress / telemetry
✅ Pass
HOOK_TELEMETRY_SINK is opt-in; unset = exact no-op; fire-and-forget to a local executable only; no outbound network
Provenance
✅ Pass
Author matches plugin.json; MIT license; no third-party SaaS wiring
The WildcardPattern::Escape workaround for Invoke-ScriptAnalyzer -Path (L192–193) is the right call — filenames with [, ], *, or ? would be treated as glob patterns by -Path without it. Good defensive code, and the comment documents the module version it was verified against.
Correctness
-cne fix is load-bearing. The comment at L178–180 explains it clearly: PowerShell's -ne is case-insensitive, so get-childitem vs Get-ChildItem compare as equal and the formatter write-back would silently no-op. The fix to -cne is correct, necessary, and the test suite proves it (Case 3 in the test file uses PSUseCorrectCasing specifically to exercise this path).
Settings walk ceiling (L111–133) is properly bounded: when CLAUDE_PROJECT_DIR is set, the walk stops at the project dir rather than the git root, matching the file-membership guard in hook::read_file_path. The contrast test (Case 1b in the test file) proves the ceiling is active by confirming a root-level settings file IS found when CLAUDE_PROJECT_DIR is unset.
PSSA_OUTPUT merges stdout+stderr (L2072>&1). This is intentional — the pwsh block routes findings to Console.Error.WriteLine and all diagnostic text should feed the advisory context. The merge is sound but a brief inline note clarifying the intent (that stderr findings are intentionally captured) would help the next reader. Minor.
Telemetry status semantics: Status "ok" on the exit-1 (findings) path (L236) means "the analyzer ran to judgment" — consistent with sibling plugins. The findings themselves live in data.findings, so a telemetry sink can distinguish clean vs violations without relying on status. This is well-aligned with the playbook's reintegration guidance on the observability contract.
Plugin Manifest & Structure
plugin.json is valid: $schema, name, version (0.1.0), description, author, keywords all present. No userConfig is correct given the consumer-owned settings file is the config surface (documented clearly in README).
hooks.json uses "${CLAUDE_PLUGIN_ROOT}"/hooks/powershell-format.sh — correct cache-isolation pattern, consistent with all other plugins in this marketplace.
hook-utils.sh is byte-identical to lib/hook-utils.sh (confirmed via diff). The SSOT copy is clean. The double-source guard at line 12 prevents re-sourcing issues.
The marketplace entry (powershell-format, category formatting, tags matching plugin.json keywords) is clean.
Test Coverage
The 39-test suite is the strongest aspect of this PR. Highlights worth noting:
Stub pwsh (exit 3 / exit 4) makes the module-absent and tool-break paths deterministic — tests run correctly even on runners without PSScriptAnalyzer installed.
Ceiling contrast (Case 1b) proves the ceiling was the active mechanism, not a missing file.
Telemetry envelope validation checks all 7 required fields, schema version, data.file is repo-relative, and duration_ms is a non-negative integer.
pwsh-absent PATH surgery is clever; the fallback echo "SKIP: ..." handles the case where pwsh shares a directory with jq/git gracefully.
One minor note: wait_for_sink polls at 20ms × 150 tries (3 s max) for the fire-and-forget background subshell. On a heavily loaded CI runner this is usually fine but could be flaky in extreme cases. Pre-existing pattern from sibling plugins, acceptable as-is.
Minor / Nits
_cpd rename (optional): Renaming to _canonical_cpd at L113–114 would make the GitGuardian false positive less likely to reoccur on future scans. Not required for correctness. Fix this →
2>&1 intent comment (optional): A one-line note above L2072>&1 explaining that stderr findings are intentionally captured would help future maintainers understand why stderr is merged rather than discarded.
Trust model section in README: The CustomRulePath warning is appropriately placed and well-worded. This is the right level of disclosure for an "executed-adjacent configuration" surface.
Verdict: Approve. Close the GitGuardian false positive, optionally rename _cpd, then merge.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Cursor Bugbot skipped (usage limit reached) with incomplete review results, so I did not approve. Human review is needed; no reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…at settings dir
Address Codex review findings on #39:
- P2: read with BOM detection (StreamReader) and write back with the
detected encoding — pwsh's Set-Content default (utf8NoBOM) stripped a
UTF-8 BOM and transcoded UTF-16 on first format. Files that are neither
BOM'd nor valid UTF-8 (legacy ANSI) skip cleanly (exit 5) instead of
being transcoded.
- P2: Set-Location to the settings directory before Invoke-Formatter /
Invoke-ScriptAnalyzer so repo-relative CustomRulePath entries resolve
from the settings file, not the agent's incidental cwd.
Finding:Set-Content rewrites with PowerShell's default encoding (utf8NoBOM on pwsh 7), silently stripping a UTF-8 BOM or transcoding UTF-16 on first format.
Uses System.IO.StreamReader with detectEncodingFromByteOrderMarks: $true and a strict-UTF-8 fallback (throwOnInvalidBytes: $true)
Captures the actual detected encoding via $sr.CurrentEncoding
Writes back with [System.IO.File]::WriteAllText($file, $formatted, $enc) — preserves whatever BOM/encoding the file had
Files that are neither BOM'd nor valid UTF-8 (legacy ANSI) hit a DecoderFallbackException → exit 5 → emit_skipped in bash — the file is left byte-identical rather than transcoded
Tests:
Case 8 — Writes a file with UTF-8 BOM (\xEF\xBB\xBF), reformats it, confirms (a) casing was fixed (get-childitem → Get-ChildItem) and (b) first six bytes are still efbbbf476574 (BOM + "Get") — proving the write-back preserved the encoding.
Case 9 — Writes a file with an invalid UTF-8 byte (\xE9), runs the hook, confirms exit 0 silent skip and hex dump is byte-identical.
The implementation is correct. The .NET stream APIs are the right level to operate at here — Get-Content/Set-Content would not give per-file encoding fidelity.
Codex P2 #2 — CustomRulePath Working Directory ✅ Resolved
Finding: If PSScriptAnalyzerSettings.psd1 contains a relative CustomRulePath (e.g. './rules/MyRules.psm1'), PSScriptAnalyzer resolves it from the current PowerShell location, which was the agent's incidental cwd — causing rule-not-found tool breaks.
Anchors PowerShell's cwd at the directory containing the settings file before both Invoke-Formatter and Invoke-ScriptAnalyzer run. Uses -LiteralPath (not -Path) so a settings directory with glob metacharacters in its name doesn't misroute. Since $file and $litFile are already absolute paths, the Set-Location only affects relative resolution within the settings file itself — exactly the intended scope.
Test (Case 10): Creates a test repo with PSScriptAnalyzerSettings.psd1 containing CustomRulePath = './rules/CleanRules.psm1', runs the hook from an unrelated cwd, and verifies (a) exit 0, no tool break and (b) get-childitem was reformatted to Get-ChildItem — confirming the relative rule path resolved and the formatter ran.
Correctness Notes
All correctness observations from the previous review stand and none are invalidated by the new commit:
2>&1 stderr merge: still intentional and sound (L231)
Minor Observation
The README doesn't mention encoding preservation behavior or the ANSI-skip exit path. Users with mixed-encoding repos would benefit from a note in the Behavior section that the hook preserves the original file encoding and skips files with non-UTF-8 bytes safely rather than transcoding them. Low priority — these are implementation details that don't affect basic usage, and the hook comment block at the top of the script explains it well.
Verdict: Approve
Both Codex P2 findings are resolved with correct implementation and solid test coverage (Cases 8, 9, 10). The security posture, graceful degrade, -cne fix, and settings ceiling from the previous review cycle all hold. No open correctness or security concerns.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Cursor Bugbot skipped (usage limit reached), so automated review results are incomplete and I did not approve. Human review is needed; no reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Cursor Bugbot skipped (usage limit reached), so automated review results are incomplete and I did not approve. Human review is needed; no reviewers were assigned because no non-author reviewers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Use hook::physical_path for FILE_DIR_POSIX and CEILING so symlinked
CLAUDE_PROJECT_DIR matches the resolved file directory during the
PSScriptAnalyzerSettings.psd1 walk-up.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Finding: When CLAUDE_PROJECT_DIR is a symlinked path, the walk-up ceiling was stored using logical pwd (cd … && pwd). The file's directory, however, is resolved by hook::read_file_path using realpath/readlink -f. The two representations differ across a symlink boundary, so "$dir" == "$CEILING" never matched — the walk continued to the git root, admitting settings files above the project boundary.
This is exactly correct. hook::physical_path calls realpath/readlink -f — the same resolver hook::read_file_path uses for its membership check — so all three walk anchors (FILE_DIR_POSIX, root, CEILING) now use the same physical representation. The ceiling comparison "$dir" == "$CEILING" is now apples-to-apples even through symlinked project dirs.
Incidentally, this also eliminates the _cpd local variable, closing out the first review's optional rename nit — GitGuardian false-positive surface is gone.
One subtle change: the old code guarded [[ -n "$_cpd" ]] && CEILING="$_cpd", keeping CEILING=$root if CLAUDE_PROJECT_DIR didn't resolve. The new code always assigns CEILING when CLAUDE_PROJECT_DIR is set. In practice, CLAUDE_PROJECT_DIR must be a valid directory or the hook would have already failed the membership check in hook::read_file_path. And hook::physical_path always returns its input as a fallback, so an unresolvable path yields the original string rather than empty — the walk still terminates at the git root in the degenerate case. The behavior is safe.
The caf prefix was being picked up by the typos CI gate as a word fragment adjacent to the high byte. The bare \xE9 byte is all the fixture needs to trigger the DecoderFallbackException path — the test still proves the hook skips rather than transcodes, and the exit 0 + byte-identical check is unchanged. Clean fix.
There is no dedicated test for the symlinked CLAUDE_PROJECT_DIR scenario that motivated the fix. In practice, writing a robust cross-platform symlink test in bash is non-trivial (requires mktemp -d + ln -s, and symlink creation can require elevated permissions on some environments). The fix itself is a targeted refactor onto hook::physical_path — a function already exercised by the shared hook-utils test suite — so the behavioral correctness of the resolver is already covered. Acceptable as-is.
Verdict: Approve. All three Codex P2 findings are resolved with correct implementations and the typos CI fix is clean. The hook is ready to merge.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Cursor Bugbot skipped (usage limit reached), so automated review results are incomplete and I did not approve. Human review is needed; no reviewers were assigned because no non-author reviewers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The hook probes pwsh only; claiming Windows PowerShell support was
inaccurate. PSScriptAnalyzer behavior also differs on 5.1, so the
supported engine is stated exactly.
Finding: The README previously claimed support for "PowerShell 7+ (cross-platform) or Windows PowerShell". The hook only probes pwsh, so powershell.exe (Windows PowerShell 5.1) was never used — the documentation overstated the supported runtime.
-**PowerShell** (`pwsh`) on `PATH` — PowerShell 7+ (cross-platform) or Windows- PowerShell. If absent, the hook is a silent no-op.+**PowerShell 7+** (`pwsh`) on `PATH` — the hook probes `pwsh` only; legacy+ Windows PowerShell 5.1 (`powershell.exe`) is not used. If absent, the hook is+ a silent no-op.
This is the right fix. The alternative (falling back to powershell.exe) would have introduced real compatibility risk: PSScriptAnalyzer output format and encoding defaults differ between 5.1 and 7+, and the BOM/StreamReader path relies on .NET APIs that behave differently on Windows PowerShell. The commit message states the rationale clearly ("PSScriptAnalyzer behavior also differs on 5.1"). The new wording is explicit and accurate.
All Prior Findings Still Hold
All correctness, security, and structural conclusions from the three previous review cycles are unchanged — this commit touched only README prose, not hook logic:
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Cursor Bugbot skipped (usage limit reached), so automated review results are incomplete and I did not approve. Human review is needed; no reviewers were assigned because no non-author reviewers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the PowerShell (PSScriptAnalyzer) formatter hook into a repo-agnostic plugin. Consumer conventions from PSScriptAnalyzerSettings.psd1 (walk bounded to project dir); pwsh/module absent -> clean no-op; kill switch HOOK_POWERSHELL_FORMAT_ENABLED. Fixes an inherited case-insensitive write-back bug (-ne -> -cne so casing fixes persist). Settings-file trust model documented. 39 tests; validate clean. Clean-tier wave Phase 1.
Note
Medium Risk
The hook rewrites PowerShell files on edit and PSScriptAnalyzer may load and run custom rule modules referenced from the opt-in settings file; risk is mitigated by explicit opt-in, project-dir bounds, and advisory-only behavior.
Overview
Adds a new
powershell-formatmarketplace plugin that runs on Write/Edit of.ps1,.psm1, and.psd1files. It formats in place with PSScriptAnalyzer’sInvoke-Formatterand reports lint viaInvoke-ScriptAnalyzer, using the consumer’s closestPSScriptAnalyzerSettings.psd1(walk-up bounded byCLAUDE_PROJECT_DIRwhen set). Repos without that settings file are not touched—no built-in default rules.Behavior matches sibling formatter hooks: advisory only (always exit 0, findings in
additionalContext),HOOK_POWERSHELL_FORMAT_ENABLEDkill switch, silent skip whenpwshor PSScriptAnalyzer is missing, optional telemetry, and a bundledhook-utils.shcopy. The PowerShell block fixes casing-only reformats by using-cnefor write-back, preserves encoding/BOM, skips non–UTF-8 legacy files, anchorsCustomRulePathat the settings directory, and documents the trust model for settings and custom rules.marketplace.jsonandREADME.mdcatalog entries register the plugin. A large contract test suite covers opt-in, extensions, graceful degrade, ceiling, BOM/ANSI, CustomRulePath, and telemetry.Reviewed by Cursor Bugbot for commit 3f57de3. Bugbot is set up for automated code reviews on this repo. Configure here.