diff --git a/plugins/claude-config/.claude-plugin/plugin.json b/plugins/claude-config/.claude-plugin/plugin.json index fce27ab3c..39da438bb 100644 --- a/plugins/claude-config/.claude-plugin/plugin.json +++ b/plugins/claude-config/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "claude-config", - "version": "0.21.9", + "version": "0.22.0", "description": "Five audit skills for a repo's Claude Code configuration: audit (settings.json / .mcp.json / hooks / plugins / permissions drift), audit-automation-gaps (evidence-gated verdicts on automation gaps), audit-permission-grants (allow-rule / allowed-tools grants for auto-mode durability and portability), audit-instructions (locally-owned instruction surfaces vs current model capability — proposes removals/rewrites of instructions the model no longer needs, and detects cross-surface instruction conflicts), and audit-pass (one coordinated, ordered, resumable pass over a named target — three-scope inventory, run-time-derived exclusion set, stable finding identity, suppression memory, resume, one human gate — delegating every check to the plugin that owns it).", "author": { "name": "Melodic Software", diff --git a/plugins/claude-config/CHANGELOG.md b/plugins/claude-config/CHANGELOG.md index 743c8a7de..235c11d5c 100644 --- a/plugins/claude-config/CHANGELOG.md +++ b/plugins/claude-config/CHANGELOG.md @@ -3,6 +3,58 @@ All notable changes to the `claude-config` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.22.0] + +### Changed + +- **`audit`: Category D no longer prescribes shell form with "no `args`" for hook commands.** The + row rested on a rationale — that the `"command":"bash"` + `args` variant "backslash-mangles + `${CLAUDE_PROJECT_DIR}` on native Windows" — that the [hooks + reference](https://code.claude.com/docs/en/hooks) contradicts: in exec form "path placeholders + like `${CLAUDE_PLUGIN_ROOT}` are substituted into `command` and into each `args` element as plain + strings", and "No shell tokenization happens on any platform." Mangling requires a shell, and exec + form has none. The real Windows defect behind the observation is narrower and is now its own row: + exec form "requires `command` to resolve to a real executable such as a `.exe`", so `"command": + "bash"` finds the WSL relay `System32\bash.exe` and the launch fails — the failure this repo hit + in #1006, where a fail-open guard enforced nothing. That is a defect in naming `bash` as the + executable, not in exec form, and the fix is a real binary plus the script path in `args`. + + Category D now follows the page's own guidance — "Prefer exec form for any hook that references a + path placeholder. In shell form, wrap each placeholder in double quotes" — and flags only the + unquoted placeholder, never shell form itself. Quoted shell form stays a correct spelling, which + it must: the page endorses omitting `args` for pipes, `&&`, redirects, and `.cmd`/`.bat` shims, + and this repository's own hooks use it. The replacement warns without swinging into the + mirror-image false positive the old row produced. The executable-resolution row is scoped to + Windows-targeting repos, since `bash` and `sh` are ordinary executables elsewhere, and it names + `"shell": "bash"` alongside the `node`-plus-`args` pattern as a documented fix. + + **A PowerShell bare-`$CLAUDE_PROJECT_DIR` row was drafted and then dropped**, because it could not + clear this repository's own fresh-docs bar. It carried a quote attributed to the hooks page that + is not on that page — re-fetched 2026-08-08 and searched: the page's only placeholder-quoting + guidance is the generic "In shell form, wrap each placeholder in double quotes", and it says + nothing about PowerShell resolving an undefined variable. The underlying claim also depends on + whether the harness substitutes the *bare* `$NAME` spelling before PowerShell ever parses it, + which the page does not document either. An audit checklist that emits findings a consumer cannot + trace to a documented rule is the exact defect 0.21.9 removed and this release corrects; a row + resting on an unverifiable premise is worse than no row. The generic quoting row already covers + the safe advice. + +### Added + +- **`audit`: a Category D row asserting hook `timeout` is expressed in SECONDS.** The hooks + reference states: "Seconds before canceling. Defaults: 600 for `command`, `http`, and `mcp_tool`; + 30 for `prompt`; 60 for `agent`." A consumer run found three hooks configured in milliseconds. + + The row flags a **recognizably millisecond-scale** value — a round thousands multiple such as + `30000` or `120000`, which read as seconds are 8 and 33 hours — and deliberately does NOT flag + merely-large ones. The page documents defaults, not a maximum, so a long-running hook may + legitimately exceed 600, and a rule keyed on `> 600` would manufacture findings against correct + configuration. Where the value is large but not millisecond-shaped, the checklist asks for + corroboration from the hook's expected runtime before anything is reported. The likely source of + the confusion is the Bash/PowerShell tools' own `tool_input.timeout`, whose example value on this + page is `120000`; the page does not state that field's unit in prose, so the checklist does not + claim it does. + ## [0.21.9] ### Removed diff --git a/plugins/claude-config/skills/audit/SKILL.md b/plugins/claude-config/skills/audit/SKILL.md index 997109b62..ba300b089 100644 --- a/plugins/claude-config/skills/audit/SKILL.md +++ b/plugins/claude-config/skills/audit/SKILL.md @@ -121,7 +121,7 @@ Eight categories — names + the question each answers below; **full per-check c - **A — Schema & Structure**: `$schema` present, no unknown keys, `mcpServers` not in `settings.local.json` - **B — Permissions**: baseline deny/ask patterns present (list in [reference/required-permissions.md](reference/required-permissions.md), plus any additional patterns the consuming repo's own rules declare as required), deny-rules-in-`settings.json`-only (bug #8961) - **C — MCP Servers**: commands resolve, `${VAR}` syntax, `disabledMcpjsonServers` match, documented disable reasons -- **D — Hooks**: paths resolve + readable, sane timeouts, valid matchers, quoted `$CLAUDE_PROJECT_DIR`, no duplicates, valid events +- **D — Hooks**: paths resolve + readable, `timeout` in seconds and sane, valid matchers, quoted path placeholders in shell form, exec-form `command` resolvable on Windows, no duplicates, valid events - **E — Plugins**: static checks (marketplace membership) + live upstream drift detection (`scripts/check-plugin-drift.sh` — ORPHAN/NEW/RENAME modes, auto-fix policy table in the context file) - **F — Environment Variables**: documented/justified vars, secrets in `settings.local.json` only, forward-slash paths - **G — Skill-listing budget**: `/doctor` overflow check and trim levers (description trimming, `skillOverrides`, budget settings) diff --git a/plugins/claude-config/skills/audit/context/validation-categories.md b/plugins/claude-config/skills/audit/context/validation-categories.md index a80de0d68..a9b17c7c7 100644 --- a/plugins/claude-config/skills/audit/context/validation-categories.md +++ b/plugins/claude-config/skills/audit/context/validation-categories.md @@ -39,11 +39,16 @@ Load the audit checklist alongside these: [audit-checklist.md](../reference/audi - All hook script paths resolve to existing files on disk - Scripts are readable (not permission-denied) +- `timeout` is a seconds value — flag a recognizably millisecond-scale figure (a round thousands + multiple like `30000` or `120000`), not merely a large one: the docs give defaults, not a maximum - Timeouts are reasonable: 5-15s for simple formatters, 30s for slow-startup tools (pwsh) - Matchers take their intended evaluation path — only letters, digits, `_`, `-`, spaces, `,`, `|` makes it an exact-string list; any other character makes it an unanchored JavaScript regex, which needs `^…$` to match a whole string (`Edit.*` also matches `NotebookEdit`) -- `$CLAUDE_PROJECT_DIR` references are properly quoted in commands +- A shell-form hook quotes each path placeholder; exec form is the docs' preference but shell form + is correct when the hook needs pipes, `&&`, redirects, or a `.cmd`/`.bat` shim — do not flag it +- On a Windows-targeting repo, exec-form `command` resolves to a real executable — `bash` there + finds the WSL relay and the hook silently never launches - No duplicate hooks (same script registered twice for same event) - Hook events are valid (cross-reference against official docs) diff --git a/plugins/claude-config/skills/audit/reference/audit-checklist.md b/plugins/claude-config/skills/audit/reference/audit-checklist.md index dd9796564..14d8f170d 100644 --- a/plugins/claude-config/skills/audit/reference/audit-checklist.md +++ b/plugins/claude-config/skills/audit/reference/audit-checklist.md @@ -85,9 +85,11 @@ additional ask-gates) get those checked at the same severities. | --- | --- | --- | | All hook scripts exist on disk | error | Resolve `$CLAUDE_PROJECT_DIR` to the project root, check file exists | | Hook scripts are readable | error | `[[ -r ]]` | +| `timeout` is a seconds value, not milliseconds | warning | The [hooks reference](https://code.claude.com/docs/en/hooks) states for `timeout`: "Seconds before canceling. Defaults: 600 for `command`, `http`, and `mcp_tool`; 30 for `prompt`; 60 for `agent`." Flag a **recognizably millisecond-scale** value — a round thousands multiple such as `30000` or `120000`, which read as seconds are 8 h and 33 h. Do NOT flag merely-large values: the page documents defaults, not a maximum, so a deliberately long-running hook may legitimately exceed 600. When the value is large but not millisecond-shaped, corroborate against the hook's expected runtime before reporting anything | | Timeouts are reasonable (5-30s for formatters, up to 60s for heavy tools) | warning | Compare against known good values | | Matcher takes its intended evaluation path | warning | Classify the matcher by its characters, confirm exact-match vs regex matches intent, anchor regex-path matchers with `^…$` | -| Command hooks use quoted shell form | warning | Pattern: `"command": "\"$CLAUDE_PROJECT_DIR\"/.claude/hooks/..."`, **no `args`**; the `"command":"bash"`+`args` (exec-form) variant backslash-mangles `${CLAUDE_PROJECT_DIR}` on native Windows | +| Shell-form path placeholders are quoted | warning | Same page: "Prefer exec form for any hook that references a path placeholder. In shell form, wrap each placeholder in double quotes." Flag a shell-form hook whose project/plugin placeholder is unquoted, in **either** spelling — braced (`${CLAUDE_PROJECT_DIR}`, `${CLAUDE_PLUGIN_ROOT}`, `${CLAUDE_PLUGIN_DATA}`) or bare-dollar (`$CLAUDE_PROJECT_DIR`), since both reach the shell and an unquoted path breaks on a space either way. Report exec form as the page's preference, but do **not** flag shell form itself — the page endorses omitting `args` when the hook needs pipes, `&&`, redirects, or a `.cmd`/`.bat` shim, and quoted shell form is a documented, correct spelling | +| Exec-form `command` resolves on every platform the repo targets | error | Same page: "On Windows, exec form requires `command` to resolve to a real executable such as a `.exe`." Windows-only constraint — `bash` and `sh` are real executables on macOS/Linux, so flag only for a repo that runs on Windows. There `bash` resolves to the WSL relay `System32\bash.exe` and the launch fails; a failed launch is a non-blocking error, so a gate hook silently enforces nothing. Fixes per the page: a real binary plus the script path in `args` (`"command": "node"`), or shell form with `"shell": "bash"`, which Claude Code routes through Git Bash instead of a PATH lookup | | No duplicate hooks (same script registered twice for same event) | info | Compare commands within each event | | Hook events are valid per official docs | error | Cross-reference against the [hooks reference](https://code.claude.com/docs/en/hooks) — fetch it live rather than trusting a recalled event list |