diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 81c3f775..450400b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -65,12 +65,32 @@ jobs: args: --check nvim/ shellcheck: - name: shellcheck (setup.sh) + name: shellcheck (active shell) runs-on: ubuntu-latest - continue-on-error: true steps: - uses: actions/checkout@v7 - - run: shellcheck setup.sh + + # An explicit list, not a glob: most of the repo's shell is out of scope, so the + # exclusions would outnumber the inclusions and read worse. Deliberately NOT covered + # (each carries pre-existing findings in code that works - the same carve-out the + # PSScriptAnalyzer job makes for legacy/vendored files): + # - the legacy Linux snapshot per CLAUDE.md: scripts/, config/, nvim/install.sh + # - bash/*.sh: sourced fragments with no shebang, never executed as scripts + # - claude/skills/**: vendored skill payloads, not ours to lint + # - git/templates/hooks/{ctags,post-*}: the upstream git-ctags recipe. Its SC1007 is + # a false positive (`CDPATH= cd --` is a valid env-var prefix, not a botched + # assignment), so gating on it would mean suppressing, not fixing. + # Add new first-party shell files here - nothing else picks them up. + - run: | + shellcheck \ + setup.sh \ + claude/no-claude-session-trailer.sh \ + claude/statusline-command.sh \ + git/templates/hooks/pre-commit \ + git/templates/hooks/prepare-commit-msg \ + git/templates/hooks/prepare-commit-msg.sh \ + git/work-hooks/policy \ + git/work-hooks/policy.sh gitleaks: name: gitleaks (secret scan) diff --git a/CLAUDE.md b/CLAUDE.md index 57c47ed6..19a98903 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -89,7 +89,7 @@ Codex CLI is wired in as a **read-only second-opinion reviewer** for Claude Code - **Deterministic guardrails, not only the skill.** `block-destructive-vcs.ps1` denies destructive **git** (`push --force` — allows `--force-with-lease` —, `reset --hard`, `clean -f`, `branch -D`) as a non-bypassable `PreToolUse` deny, where the `git-guardrails` skill only *advises*. **jj is intentionally ungated** (its op-log makes rewrites recoverable). `block-pwsh-in-bash.ps1` denies PowerShell sent to the Bash tool, enforcing the pwsh-native rule. - **Both deny hooks strip quoted substrings before matching**, so a destructive phrase quoted in a commit message (`git commit -m "reset --hard …"`) or a separator inside a quoted arg does not mis-trigger. VCS matching is **case-sensitive** on purpose: `git branch -d` (safe) and `-D` (force) differ only by case, so a case-insensitive match would deny the safe form too. Cmdlet detection is command-position + capitalized `Verb-Noun` only (lowercase cmdlets are *not* caught — that would false-block executables like `start-stop-daemon`). -- **Lint is a per-file inner-loop nudge, not CI.** `lint-powershell.ps1` runs PSScriptAnalyzer on the single edited `.ps1/.psm1/.psd1` (using the nearest `.vscode/PSScriptAnalyzerSettings.psd1` when present) and feeds findings back via `additionalContext` — non-blocking. It does **not** replace the full `-Recurse` run CI does over the whole source tree (a per-file pass can miss violations in untouched files). +- **Lint is a per-file inner-loop nudge, not CI.** `lint-powershell.ps1` runs PSScriptAnalyzer on the single edited `.ps1/.psm1/.psd1` and feeds findings back via `additionalContext` — non-blocking. It does **not** replace the full `-Recurse` run CI does over the whole source tree (a per-file pass can miss violations in untouched files). **It checks both ruleset layouts at each level walking up — `.vscode/PSScriptAnalyzerSettings.psd1` and the bare `PSScriptAnalyzerSettings.psd1`** — because this repo keeps its ruleset at the **root**, which is the file CI passes (`ci.yml`: `-Settings ./PSScriptAnalyzerSettings.psd1`). A `.vscode`-only lookup silently fell back to PSSA defaults here, so every finding was a rule the repo deliberately excludes (`Write-Host`, BOM, `ShouldProcess`) — a false positive the agent would then "fix". **The walk stops at the project boundary (`.git`/`.jj`)**, checked *after* the ruleset so one sitting at the root still wins: unbounded, it escapes into `$HOME` or the drive root, where a single stray `PSScriptAnalyzerSettings.psd1` would silently govern linting in **every** repo on the machine (verified — the hook went quiet on a project that had no ruleset of its own). `.jj` is needed because a non-colocated Jujutsu repo has no `.git`, and the check must not use `-PathType`: in this repo's own bare-worktree layout `.git` is a *file*, not a directory. Pinned by `tests/lint-powershell.Tests.ps1` — including the worktree-file boundary — and its first test asserts the no-ruleset case still *reports*, so the suite can't green on an inert hook. - **The two `warn-*` hooks replace the `hookify` plugin** (disabled in `enabledPlugins`), which was broken on Windows — its hooks shell out to `python3`, which resolves to the Microsoft Store stub. `warn-legacy-files.ps1` (`PreToolUse` Edit|Write) emits an **`ask`** decision before editing a legacy/do-not-touch dotfile (root `pwsh_profile.ps1`, `bootstrap.sh`, `Makefile`, `config/bspwm|sxhkd/`); it is **scoped to `$env:DOTFILES`** so the globally-wired hook never fires on same-named files in other repos, and no-ops if `$env:DOTFILES` is unset. `warn-hardcoded-secrets.ps1` (`PostToolUse` Edit|Write) warns via `additionalContext` when written content looks like a secret (assigned api-key/secret/token/password, private-key block, AWS access-key id); **global** on purpose, and it reports only rule names, never the matched value. Both are advisory, not hard denies. - **`warn-reasoning-extraction.ps1` guards the Fable-5 `reasoning_extraction` fallback (lever 5).** Reasoning-extraction phrasing (`'explain your reasoning step by step'`, chain-of-thought demands) can trip Claude Fable 5's refusal → fallback to Opus (Claude Code shows a transcript notice; the raw API returns `stop_reason: refusal`) — costly because subagents often run Fable under an Opus main loop. One script, two wirings: on **`UserPromptSubmit`** it warns via `additionalContext` (never denies a user prompt); on **`PreToolUse` Edit|Write** it emits an **`ask`** only when the phrase is being written into a config/prompt-bearing file (`CLAUDE.md`, `AGENTS.md`, `*SKILL.md`, `agents/*.md`, `codex/**`) — the worst case, a *standing* instruction persisted into config. Like the deny hooks it **strips quoted substrings before matching** (so docs that quote the phrase to ban it don't self-trigger) and applies a negation guard (`never`/`don't`/…). Advisory + ask, never a hard deny; reports rule names only. The full 12-lever set it enforces lives in `claude/AGENTS.md` → "Prompting downstream models". diff --git a/claude/AGENTS.md b/claude/AGENTS.md index 33ea696d..ccaa4f45 100644 --- a/claude/AGENTS.md +++ b/claude/AGENTS.md @@ -88,7 +88,7 @@ Applies to every prompt you author for a downstream model — subagent prompts, ## Linting - Run PSScriptAnalyzer with `-Recurse` over the whole source tree (e.g. `/src`), not per changed file — CI does, so a per-file run can pass while CI fails on a pre-existing violation in an untouched file. -- Always pass the project's settings file (`-Settings .vscode/PSScriptAnalyzerSettings.psd1`) so the local ruleset matches CI exactly. Never assume the local ruleset; verify against the file CI uses. +- Always pass the project's settings file (`-Settings `) so the local ruleset matches CI exactly. **Locate it, never assume it** — the path differs per repo (`.vscode/PSScriptAnalyzerSettings.psd1` and a bare `PSScriptAnalyzerSettings.psd1` at the root are both common), so read the CI workflow and pass the file it passes. Linting with the wrong ruleset — or none — silently reports rules the repo deliberately excludes, and every one of those is a false positive that costs a real fix. ## Terminal Usage diff --git a/claude/README.md b/claude/README.md index c96a69e6..27088278 100644 --- a/claude/README.md +++ b/claude/README.md @@ -75,7 +75,7 @@ self-skips if absent). All allow silently and only emit JSON when they act. | `PreToolUse` (Bash\|PowerShell) | `block-destructive-vcs.ps1` | Denies destructive **git** — `push --force` (allows `--force-with-lease`), `reset --hard`, `clean -f`, `branch -D`. jj is left ungated (its op-log makes rewrites recoverable). The git-guardrails *skill* only advises; this hook is non-bypassable | | `PreToolUse` (Bash) | `block-pwsh-in-bash.ps1` | Denies PowerShell sent to the Bash tool (a segment that *starts* with `pwsh`/`powershell`, `$env:`, or a `Verb-Noun` cmdlet), pointing at the PowerShell tool. Command-position only, so a cmdlet quoted as a search string is allowed | | `PreToolUse` (Edit\|Write) | `warn-legacy-files.ps1` | Emits an `ask` decision (pause-and-confirm) before editing a legacy/do-not-touch dotfile — root `pwsh_profile.ps1`, `bootstrap.sh`, `Makefile`, `config/bspwm\|sxhkd/`. Scoped to `$env:DOTFILES` so it never fires on same-named files in other repos; no-ops if `$env:DOTFILES` is unset | -| `PostToolUse` (Edit\|Write) | `lint-powershell.ps1` | Runs PSScriptAnalyzer on an edited `.ps1/.psm1/.psd1` (using the nearest `.vscode/PSScriptAnalyzerSettings.psd1` when present) and feeds findings back via `additionalContext` | +| `PostToolUse` (Edit\|Write) | `lint-powershell.ps1` | Runs PSScriptAnalyzer on an edited `.ps1/.psm1/.psd1` (using the nearest ruleset — `.vscode/PSScriptAnalyzerSettings.psd1` or the project root's, whichever it hits first walking up; the walk stops at the project boundary `.git`/`.jj`, so a stray ruleset in `$HOME` can't govern every repo) and feeds findings back via `additionalContext` | | `PostToolUse` (Edit\|Write) | `warn-hardcoded-secrets.ps1` | Scans written content (Write `content` / Edit `new_string`) for secret-shaped assignments (api-key/secret/token/password with a quoted literal, private-key blocks, AWS access-key ids) and warns via `additionalContext`. Global (secrets are bad anywhere); reports only rule names, never values | | `UserPromptSubmit` | `warn-reasoning-extraction.ps1` | Warns via `additionalContext` (never denies) when the submitted prompt asks for step-by-step reasoning / chain-of-thought — phrasing that can trip Claude Fable 5's `reasoning_extraction` refusal → Opus fallback (Claude Code shows a transcript notice). Nudges Claude to read it as a request for short rationale + assumptions + evidence | | `PreToolUse` (Edit\|Write) | `warn-reasoning-extraction.ps1` | Same script, config-write guard: emits an `ask` only when a reasoning-extraction phrase is being written into a config/prompt-bearing file (`CLAUDE.md`, `AGENTS.md`, `*SKILL.md`, `agents/*.md`, `codex/**`) — a *standing* instruction persisted into config. Strips quoted substrings before matching (so docs that quote the phrase to ban it don't self-trigger) plus a negation guard; reports rule names only | diff --git a/claude/lint-powershell.ps1 b/claude/lint-powershell.ps1 index 8638df13..488db237 100644 --- a/claude/lint-powershell.ps1 +++ b/claude/lint-powershell.ps1 @@ -21,12 +21,28 @@ if (-not (Get-Module -ListAvailable -Name PSScriptAnalyzer)) { exit 0 } Import-Module PSScriptAnalyzer -ErrorAction Stop # Use the project's ruleset when present (mirrors CI) by walking up from the edited file -# to the nearest `.vscode/PSScriptAnalyzerSettings.psd1`; fall back to PSSA defaults. +# to the nearest settings file; fall back to PSSA defaults. +# +# Both layouts are checked at each level: repos differ on where the ruleset lives, and +# THIS repo keeps it at the root - which is the file CI passes (`-Settings +# ./PSScriptAnalyzerSettings.psd1`). Looking only under `.vscode/` meant falling back to +# defaults here, reporting rules the repo deliberately excludes, so every finding was a +# false positive. $analyzerArgs = @{ Path = $filePath } $dir = Split-Path -Parent (Resolve-Path -LiteralPath $filePath) while ($dir) { - $candidate = Join-Path $dir '.vscode/PSScriptAnalyzerSettings.psd1' - if (Test-Path -LiteralPath $candidate) { $analyzerArgs['Settings'] = $candidate; break } + $candidate = '.vscode/PSScriptAnalyzerSettings.psd1', 'PSScriptAnalyzerSettings.psd1' | + ForEach-Object { Join-Path $dir $_ } | + Where-Object { Test-Path -LiteralPath $_ } | + Select-Object -First 1 + if ($candidate) { $analyzerArgs['Settings'] = $candidate; break } + # Stop at the project boundary. A ruleset further up belongs to another project - or to + # the user's home, where one stray file would silently govern linting in every repo on + # the machine. Checked AFTER the ruleset so a ruleset sitting AT the root still wins. + # `.jj` as well as `.git` because a non-colocated Jujutsu repo has no `.git` (same + # reason nvim/lua/config/autocmds.lua roots on both); no -PathType, since `.git` is a + # file rather than a directory in a worktree. + if ((Test-Path -LiteralPath (Join-Path $dir '.git')) -or (Test-Path -LiteralPath (Join-Path $dir '.jj'))) { break } $parent = Split-Path -Parent $dir if ($parent -eq $dir) { break } $dir = $parent diff --git a/claude/statusline-command.sh b/claude/statusline-command.sh index 1f33ba1a..40b8fa5c 100644 --- a/claude/statusline-command.sh +++ b/claude/statusline-command.sh @@ -106,8 +106,10 @@ if [ -n "$dir" ] && git -C "$dir" rev-parse --git-dir >/dev/null 2>&1; then git_seg="${branch_color}±${branch}${RESET}" fi - # Ahead/behind vs upstream (left=behind, right=ahead) - if ab=$(git -C "$dir" rev-list --left-right --count @{u}...HEAD 2>/dev/null); then + # Ahead/behind vs upstream (left=behind, right=ahead). + # `@{u}` is quoted because the braces are literal (git's upstream shorthand, not a + # brace expansion) — unquoted it trips shellcheck SC1083. + if ab=$(git -C "$dir" rev-list --left-right --count "@{u}...HEAD" 2>/dev/null); then behind=$(echo "$ab" | awk '{print $1}') ahead=$(echo "$ab" | awk '{print $2}') if [ "${ahead:-0}" -gt 0 ] && [ "${behind:-0}" -gt 0 ]; then diff --git a/tests/lint-powershell.Tests.ps1 b/tests/lint-powershell.Tests.ps1 new file mode 100644 index 00000000..008f07e1 --- /dev/null +++ b/tests/lint-powershell.Tests.ps1 @@ -0,0 +1,162 @@ +#Requires -Version 7 +# Behavioural tests for claude/lint-powershell.ps1 — the PostToolUse hook that lints an +# edited PowerShell file and feeds violations back as `additionalContext`. +# +# What matters here is RULESET DISCOVERY. The hook is only useful if it lints with the +# same ruleset CI uses; when it falls back to PSSA defaults it reports rules the repo +# deliberately excludes, and every one of those is a false positive the agent then +# "fixes". So each test drives the real hook over a throwaway tree and asserts on what +# the hook emits, not on how it searched. +# +# Needs PSScriptAnalyzer (the hook no-ops without it), so the suite skips rather than +# false-green. Computed at DISCOVERY time — Describe's -Skip: is evaluated then, so this +# cannot live in BeforeAll (which runs later, leaving the flag unset and silently +# skipping every test). +$script:HasPSSA = [bool](Get-Module -ListAvailable -Name PSScriptAnalyzer) + +BeforeAll { + $script:RepoRoot = Split-Path $PSScriptRoot -Parent + $script:Hook = Join-Path $script:RepoRoot 'claude/lint-powershell.ps1' + + # A ruleset excluding exactly the rule the fixture script trips. Purpose-built rather + # than a copy of the repo's own: the contract under test is "did the hook find and + # apply the nearest settings file", not which rules this repo happens to exclude. + $script:Settings = "@{ Severity = @('Error','Warning'); ExcludeRules = @('PSAvoidUsingWriteHost') }" + + # Builds /sub/edited.ps1, which trips PSAvoidUsingWriteHost. Returns its path. + # Dropping a ruleset is Add-Ruleset's job, deliberately kept separate: a $SettingsDir + # parameter here would bind $null to '' and silently place the ruleset at the root, + # turning the no-ruleset case into a root-ruleset case. + function New-LintTree { + param([string] $Root) + + $sub = Join-Path $Root 'sub' + New-Item -ItemType Directory -Path $sub -Force | Out-Null + + $edited = Join-Path $sub 'edited.ps1' + Set-Content -LiteralPath $edited -Value "Write-Host 'hi'" -Encoding UTF8 + return $edited + } + + function Add-Ruleset { + param([string] $Dir) + + New-Item -ItemType Directory -Path $Dir -Force | Out-Null + Set-Content -LiteralPath (Join-Path $Dir 'PSScriptAnalyzerSettings.psd1') ` + -Value $script:Settings -Encoding UTF8 + } + + # Marks $Dir as a project boundary. Every test sets one: without it the walk-up escapes + # into $env:TEMP's ancestors and the result depends on whether the machine happens to + # have a stray ruleset in a parent dir - so these tests would pass or fail on + # environment, not behaviour. + function Add-VcsRoot { + param([string] $Dir) + + New-Item -ItemType Directory -Path (Join-Path $Dir '.git') -Force | Out-Null + } + + # Drives the hook exactly as Claude Code does: tool-call JSON on stdin. + function Invoke-LintHook { + param([string] $FilePath) + $json = @{ tool_input = @{ file_path = $FilePath } } | ConvertTo-Json -Compress + return ($json | & pwsh -NoProfile -File $script:Hook 2>&1 | Out-String) + } + + function New-TempRoot { + $root = Join-Path ([IO.Path]::GetTempPath()) ('lintbook-' + [guid]::NewGuid()) + New-Item -ItemType Directory -Path $root -Force | Out-Null + return $root + } +} + +Describe 'claude/lint-powershell.ps1 ruleset discovery' -Skip:(-not $script:HasPSSA) { + It 'reports a violation when no ruleset is present' { + # Guards the other two tests from passing for the wrong reason: proves the fixture + # really does trip a rule, so silence below means "ruleset applied", not "hook + # never ran". Without this an inert hook would green the whole suite. + $root = New-TempRoot + try { + $edited = New-LintTree -Root $root + Add-VcsRoot -Dir $root + $out = Invoke-LintHook -FilePath $edited + + $out | Should -Match 'PSAvoidUsingWriteHost' -Because 'PSSA defaults flag Write-Host, so the fixture is a real violation' + } finally { + Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue + } + } + + It 'honours a ruleset in .vscode/' { + $root = New-TempRoot + try { + $edited = New-LintTree -Root $root + Add-VcsRoot -Dir $root + Add-Ruleset -Dir (Join-Path $root '.vscode') + $out = Invoke-LintHook -FilePath $edited + + $out.Trim() | Should -BeNullOrEmpty -Because 'the ruleset excludes the only rule the fixture trips' + } finally { + Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue + } + } + + It 'honours a ruleset at the project root' { + # THE BUG: this repo keeps PSScriptAnalyzerSettings.psd1 at the root — that is the + # file CI passes (ci.yml: -Settings ./PSScriptAnalyzerSettings.psd1). The hook only + # ever looked for `.vscode/PSScriptAnalyzerSettings.psd1`, so it silently linted + # with defaults and false-flagged rules this repo excludes (Write-Host on + # git/work-hooks/policy.ps1 was the recurring one). + $root = New-TempRoot + try { + $edited = New-LintTree -Root $root + Add-VcsRoot -Dir $root + Add-Ruleset -Dir $root + $out = Invoke-LintHook -FilePath $edited + + $out.Trim() | Should -BeNullOrEmpty -Because 'a root ruleset is what CI uses and the hook must match it' + } finally { + Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue + } + } + + It 'ignores a ruleset above the project root' { + # The walk must stop at the project boundary. A ruleset further up belongs to some + # other project - or to the user's home dir, where a stray file would silently + # govern the linting of every repo on the machine. Checking the ruleset BEFORE the + # boundary is what still allows a ruleset sitting AT the root (the case above). + $root = New-TempRoot + try { + $project = Join-Path $root 'project' + $edited = New-LintTree -Root $project + Add-VcsRoot -Dir $project + Add-Ruleset -Dir $root # outside the project + $out = Invoke-LintHook -FilePath $edited + + $out | Should -Match 'PSAvoidUsingWriteHost' -Because 'a ruleset outside the project must not silently apply to it' + } finally { + Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue + } + } + + It 'treats a worktree .git FILE as the project boundary' { + # THIS repo's own layout (bare repo + one worktree per branch): `.git` is a hidden + # POINTER FILE, not a directory. A `-PathType Container` check would miss it and the + # walk would escape the project, so pin the file form - it is the form that actually + # ships here, and the failure would be silent. + $root = New-TempRoot + try { + $project = Join-Path $root 'project' + $edited = New-LintTree -Root $project + Set-Content -LiteralPath (Join-Path $project '.git') ` + -Value 'gitdir: /elsewhere/.bare/worktrees/project' -Encoding UTF8 + Add-Ruleset -Dir $root # outside the project + + $out = Invoke-LintHook -FilePath $edited + + $out | Should -Match 'PSAvoidUsingWriteHost' -Because 'a worktree .git is a file and must still bound the walk' + } finally { + Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue + } + } +} diff --git a/winget/packages.json b/winget/packages.json index 7134b89e..95e02a09 100644 --- a/winget/packages.json +++ b/winget/packages.json @@ -41,6 +41,7 @@ { "PackageIdentifier": "jqlang.jq" }, { "PackageIdentifier": "junegunn.fzf" }, { "PackageIdentifier": "k3d.k3d" }, + { "PackageIdentifier": "koalaman.shellcheck" }, { "PackageIdentifier": "Kubernetes.kind" }, { "PackageIdentifier": "Kubernetes.kubectl" }, { "PackageIdentifier": "Kubernetes.minikube" },