diff --git a/README.md b/README.md index 8099fad4c..bd0320af1 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,7 @@ user opts in with `/plugin enable`; an existing install is never flipped by cata ### Security -- [`guardrails`](plugins/guardrails) — Nine safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, commit subjects and gh pr create titles that violate the repo's tracked team convention (when one is declared in .claude/source-control.md), (advisory) hallucinated CLI flags, (advisory) un-throttled Workflow fan-out that risks burst 529s, and (advisory) direct git commit/gh pr create calls bypassing this marketplace's own commit/pull-request skills — each independently toggleable. +- [`guardrails`](plugins/guardrails) — Eleven safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, commit subjects and gh pr create titles that violate the repo's tracked team convention (when one is declared in .claude/source-control.md), (advisory) hallucinated CLI flags, (advisory) /plugin:skill references that do not resolve, (advisory) un-throttled Workflow fan-out that risks burst 529s, and (advisory) direct git commit/gh pr create calls bypassing this marketplace's own commit/pull-request skills — each independently toggleable. ### Workflow diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index 23f30afad..f658731cf 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -1,8 +1,8 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "guardrails", - "version": "0.14.3", - "description": "Nine safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, commit subjects and gh pr create titles that violate the repo's tracked team convention (when one is declared in .claude/source-control.md), (advisory) hallucinated CLI flags, (advisory) un-throttled Workflow fan-out that risks burst 529s, and (advisory) direct git commit/gh pr create calls bypassing this marketplace's own commit/pull-request skills — each independently toggleable.", + "version": "0.15.0", + "description": "Eleven safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, commit subjects and gh pr create titles that violate the repo's tracked team convention (when one is declared in .claude/source-control.md), (advisory) hallucinated CLI flags, (advisory) /plugin:skill references that do not resolve, (advisory) un-throttled Workflow fan-out that risks burst 529s, and (advisory) direct git commit/gh pr create calls bypassing this marketplace's own commit/pull-request skills — each independently toggleable.", "author": { "name": "Melodic Software", "email": "info@melodicsoftware.com" @@ -68,6 +68,12 @@ "description": "Advise on hallucinated CLI flags written to files (never blocks)", "default": true }, + "skill_reference_verify_enabled": { + "type": "boolean", + "title": "skill-reference-verify guard", + "description": "Advise when markdown cites a /plugin:skill reference this repo owns but cannot resolve (never blocks)", + "default": true + }, "workflow_resilience_check_enabled": { "type": "boolean", "title": "workflow-resilience-check guard", diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index 6d97f2f42..86cc5fbfc 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,47 @@ All notable changes to the `guardrails` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.15.0] + +### Added + +- `skill-reference-verify` (advisory, PostToolUse Write|Edit): flags a + `/plugin:skill` reference in markdown that does not resolve. Gated twice — it + does nothing outside a marketplace repo, and within one it only adjudicates a + plugin that repo's own manifests own. Resolution goes through manifest `name` + and skill frontmatter `name`; a renamed skill's DIRECTORY name is deliberately + not an alias, since treating it as one would suppress exactly the stale + pre-rename references this guard exists to catch. The reference is the leading + command token of a code span, so argument-bearing invocations + (`/plugin:skill --apply`) are scanned. `CHANGELOG.md` is excluded as an + append-only historical record: a rename entry must keep naming the old command. + Declared **detect-then-judge**, not deterministic — globbing a plugins tree is + exact only where the reference is locally owned, so the finding is a prompt for + a human verdict and never an auto-fix. +- A README enforceability-tier section stating each guard's oracle class, so the + detect-then-judge guard cannot be read as deterministic. + +### Fixed + +- README guard counts were stale before this change: the prose said "nine safety + guards" and the table omitted `block-convention-violation` while ten were + wired. Counts are now measured against the manifest's toggle set, and the + missing row is present. + +### Not shipped + +- An `asserted-path-verify` guard was built alongside this one and withdrawn on + measurement. Swept across all 975 tracked markdown files it fired on 23.7% of + them — roughly one in four writes — producing 389 findings with **zero** true + positives. 72% were consumer-project config paths (`.claude/**` and similar) + that a doc describes for a CONSUMING repo and that correctly do not exist in a + marketplace; its first-segment gate passed only because this repo happens to + carry same-named top-level directories. Fixing the three dominant causes still + left ~4% firing at zero true positives, so a repo-root filesystem test is the + wrong oracle for a repo whose docs are largely about other repos' trees. The + measurement is attached to its follow-up issue for rescoping rather than + discarded. + ## [0.14.3] ### Documentation diff --git a/plugins/guardrails/README.md b/plugins/guardrails/README.md index 8102ee5d9..27047c0c9 100644 --- a/plugins/guardrails/README.md +++ b/plugins/guardrails/README.md @@ -1,6 +1,6 @@ # guardrails -A Claude Code plugin bundling nine **safety guards** that catch risky agent +A Claude Code plugin bundling eleven **safety guards** that catch risky agent actions the moment they happen — before a write lands or a bash command runs. Each guard is independently toggleable, so you run exactly the subset you want. @@ -16,12 +16,23 @@ Each guard is independently toggleable, so you run exactly the subset you want. | **cli-flag-verify** | PostToolUse · Write \| Edit | **Advisory** (exit 0) | Hallucinated CLI flags — a `--flag` written as a command that does not exist in the binary's actual `--help` output. Surfaces via `additionalContext`, never blocks. | | **workflow-resilience-check** | PreToolUse · Workflow | **Advisory** (exit 0) | Un-throttled Workflow fan-out — a script calling `parallel()` / `pipeline()` with no wave-cap throttle (`inWaves` / `inWavesPipeline`) and no retry wrapper (`agentRetry`), which risks a burst 529 under wide Opus fan-out. Surfaces a resilience checklist via `additionalContext`, never blocks. | | **block-noncanonical-commit** | PreToolUse · Bash | **Blocks** (exit 2) | `git commit` that does not pipe its message via `-F -` / `--file -` — `-m` flattens newlines unpredictably across shells. Exempt: `--amend`, `-C`/`-c`/`--reuse-message`/`--reedit-message`, `--fixup`/`--squash`, `-F `, and any commit taken while a merge/rebase/cherry-pick/revert is in progress. Resolves `bash -lc` wrappers and git aliases (inline `-c` and persisted config alike). | +| **block-convention-violation** | PreToolUse · Bash | **Blocks** (exit 2) | A commit subject or `gh pr create --title` that violates the team-tracked convention pattern declared in `.claude/source-control.md`. No tracked pattern means no enforcement. Same exemptions as `block-noncanonical-commit`. | | **flag-commit-pr-skill-bypass** | PreToolUse · Bash | **Advisory** (exit 0) | Any `gh pr create`, bypassing this marketplace's own `/pull-request create` skill. Only fires when the consuming project's own `.claude/settings.json` enables the `source-control` plugin — silent otherwise. Surfaces via `additionalContext`, never blocks. | +| **skill-reference-verify** | PostToolUse · Write \| Edit | **Advisory** (exit 0) | A `` `/plugin:skill` `` reference in markdown that does not resolve. Only fires inside a marketplace repo, and only for a plugin that repo's own manifests own — a reference to another marketplace is left alone. Resolves through manifest and frontmatter `name`, so a renamed directory still matches. Surfaces via `additionalContext`, never blocks. | -The six blocking guards feed their stderr message back to Claude as -actionable fix guidance. The three advisory guards surface their findings the same +The seven blocking guards feed their stderr message back to Claude as +actionable fix guidance. The four advisory guards surface their findings the same way but always allow the operation. +### Enforceability tiers + +Ten guards are **deterministic** — their oracle is a mechanical test with no +judgment step. `skill-reference-verify` is **detect-then-judge**: globbing a +plugins tree is exact only inside a marketplace repo that owns the referenced +plugin, so its finding is a prompt for a human verdict, never a determination and +never an auto-fix. `cli-flag-verify` is deterministic in its oracle but advisory in +its action, because a written claim can be deliberately forward-looking. + ### Scope notes - **Hook-manager coverage.** `block-no-verify` recognizes the disable env-var diff --git a/plugins/guardrails/hooks/hooks.json b/plugins/guardrails/hooks/hooks.json index 3404d1c60..7a15235d0 100644 --- a/plugins/guardrails/hooks/hooks.json +++ b/plugins/guardrails/hooks/hooks.json @@ -85,6 +85,12 @@ "command": "\"${CLAUDE_PLUGIN_ROOT}\"/hooks/cli-flag-verify.sh", "timeout": 30, "statusMessage": "Verifying CLI flags..." + }, + { + "type": "command", + "command": "\"${CLAUDE_PLUGIN_ROOT}\"/hooks/skill-reference-verify.sh", + "timeout": 30, + "statusMessage": "Verifying skill references..." } ] } diff --git a/plugins/guardrails/hooks/skill-reference-verify.sh b/plugins/guardrails/hooks/skill-reference-verify.sh new file mode 100755 index 000000000..b9ab9c784 --- /dev/null +++ b/plugins/guardrails/hooks/skill-reference-verify.sh @@ -0,0 +1,285 @@ +#!/usr/bin/env bash +# PostToolUse hook: verify `/:` references written to markdown +# resolve to a skill that exists in the working tree. +# Triggered on Write|Edit of *.md files. +# +# Catches subagent / training-recall hallucinations — a slash-command reference +# written AS a capability that does not exist, or one left behind by a rename. +# Sibling of cli-flag-verify and asserted-path-verify: same defect class, third +# oracle. +# +# ENFORCEABILITY TIER: Detect-then-judge — ADVISORY PLUS A HUMAN VERDICT, never +# an auto-fix. Globbing a plugins tree is exact only inside a marketplace repo +# that owns the referenced plugin. In a consuming repo the reference may name a +# plugin from another marketplace, or one the operator has simply not installed, +# and this hook cannot distinguish that from a hallucination. Per the org +# convention on enforceability tiers, that means the finding is a prompt for a +# human decision, not a determination. The plugins-root gate below is what keeps +# the noise inside the one context where the oracle is meaningful. +# +# PLUGINS-ROOT GATE: the hook does nothing unless the file being written lives in +# a repo with a `plugins/` directory containing at least one `.claude-plugin/` +# manifest — i.e. a marketplace repo. Outside one there is no local authority to +# resolve a reference against, so the guard stays silent rather than guessing. +# +# PLUGIN-SCOPE GATE: within a marketplace repo, a reference is only adjudicated +# when its PLUGIN half resolves locally. `/some-other-marketplace:thing` is left +# alone; `/guardrails:nonexistent` is reported, because this repo owns +# `guardrails` and can therefore say the skill is not there. +# +# NON-BLOCKING (advisory): exits 0 with hookSpecificOutput additionalContext. +# +# Disable with the skill_reference_verify_enabled userConfig option set to false. + +set -uo pipefail + +# High-res start stamp for telemetry (Bash 5.0+; empty on older bash → skip). +start=${EPOCHREALTIME:-} + +# shellcheck source=hook-utils.sh +source "$(dirname "${BASH_SOURCE[0]}")/hook-utils.sh" + +hook::check_enabled "SKILL_REFERENCE_VERIFY" + +hook::ctx_reset + +INPUT=$(hook::buffer_stdin) || exit 0 + +hook::require_jq "PostToolUse" "guardrails-skill-reference-verify" "$INPUT" + +FILE=$(printf '%s' "$INPUT" | hook::read_file_path) || exit 0 +case "$FILE" in +# A CHANGELOG is an append-only historical record: an entry saying a skill was +# renamed MUST keep naming the old command, so every rename permanently adds an +# unresolvable reference to one. Measured on this repo: 56 of 63 findings (89%) +# were CHANGELOG rename entries, all correct as written, and excluding them moves +# the guard from 3.4% of files firing at 6% precision to 0.5% firing at 57%. +# +# Deliberately narrow. `docs/topics/*/PLAN.md` completion records are arguably the +# same shape, but two of the four real findings on this corpus live there — a +# broader "historical by contract" rule would cost half the signal. +*/CHANGELOG.md | CHANGELOG.md) exit 0 ;; +*.md) ;; +*) exit 0 ;; +esac + +# Diff-scope: verify only the content THIS tool call wrote, never re-read the +# whole file from disk. +TOOL=$(printf '%s' "$INPUT" | jq -r '.tool_name // empty' 2>/dev/null | tr -d '\r') +case "$TOOL" in +Edit) SCAN_CONTENT=$(printf '%s' "$INPUT" | jq -r '.tool_input.new_string // empty' 2>/dev/null | tr -d '\r') ;; +Write) SCAN_CONTENT=$(printf '%s' "$INPUT" | jq -r '.tool_input.content // empty' 2>/dev/null | tr -d '\r') ;; +*) exit 0 ;; +esac +[[ -n "$SCAN_CONTENT" ]] || exit 0 + +REPO_ROOT="$(hook::repo_root "$(dirname "$FILE")")" +PLUGINS_DIR="$REPO_ROOT/plugins" + +# PLUGINS-ROOT GATE. Outside a marketplace repo there is no local authority. +[[ -d "$PLUGINS_DIR" ]] || exit 0 +shopt -s nullglob +manifests=("$PLUGINS_DIR"/*/.claude-plugin/plugin.json) +shopt -u nullglob +((${#manifests[@]} > 0)) || exit 0 + +# A plugin's command namespace is its manifest `name`, which need not equal its +# directory name. Build the name → directory map from the manifests themselves so +# a renamed directory or a name override resolves correctly. +declare -A PLUGIN_DIR=() +for m in "${manifests[@]}"; do + pdir="${m%/.claude-plugin/plugin.json}" + pname=$(jq -r '.name // empty' "$m" 2>/dev/null | tr -d '\r') + [[ -n "$pname" ]] || pname="${pdir##*/}" + PLUGIN_DIR["$pname"]="$pdir" +done + +# Extract a plugin skill's frontmatter `name`, or nothing when it declares none. +# Tolerates quoting and a trailing YAML comment (`name: renamed # public +# command`); comments are stripped first so the value pattern stays anchored to +# end-of-line. +skill_frontmatter_name() { + sed -n '1,40p' "$1" 2>/dev/null | + sed -E 's/[[:space:]]+#.*$//' | + sed -nE 's/^name:[[:space:]]*"?'"'"'?([A-Za-z0-9_-]+)"?'"'"'?[[:space:]]*$/\1/p' | + head -1 +} + +# A plugin skill's command segment comes from its frontmatter `name` when set, +# and from the directory name ONLY when it does not. +# +# The directory name is not an alias. A `skills/legacy-dir/SKILL.md` declaring +# `name: renamed` answers to `/plugin:renamed` and NOT to `/plugin:legacy-dir` — +# treating the directory as a second valid spelling would suppress the advisory +# for exactly the stale pre-rename references this guard exists to catch. +# +# A directory alone is also not a skill: it must carry a SKILL.md, or a leftover +# empty `skills//` would suppress the advisory for a command that never +# existed. +skill_resolves() { + local pdir="$1" skill="$2" sd fname + local direct="$pdir/skills/$skill/SKILL.md" + if [[ -f "$direct" ]]; then + fname=$(skill_frontmatter_name "$direct") + # No declared name → the directory name IS the command segment. + [[ -z "$fname" || "$fname" == "$skill" ]] && return 0 + fi + for sd in "$pdir"/skills/*/SKILL.md; do + [[ -f "$sd" ]] || continue + fname=$(skill_frontmatter_name "$sd") + [[ -n "$fname" && "$fname" == "$skill" ]] && return 0 + done + return 1 +} + +# Candidate references: `/:` inside inline-code spans only. Prose +# is not scanned — an unbackticked `/a:b` is as likely to be a URL fragment, a +# Windows drive path, or a time range as a command. +# +# The reference need not be the whole span: an invocation commonly carries +# arguments (`/plugin:skill --apply`, `/plugin:skill `). Take the leading +# command token from each span rather than requiring the span to match exactly, +# or argument-bearing invocations — the common form in this repo — go unchecked. +emit_refs() { + # shellcheck disable=SC2016 # backticks are literal ERE data, not expansions + printf '%s' "$SCAN_CONTENT" | grep -oE '`[^`]+`' 2>/dev/null | + sed -E 's/^`+//; s/`+$//' | + sed -nE 's|^(/[a-z][a-z0-9-]*:[a-z][a-z0-9-]*)([[:space:]].*)?$|\1|p' +} + +# Partial-replacement context reconstruction (Edit only), mirroring +# cli-flag-verify's reconstruct_partial_edit. +# +# An Edit may replace an arbitrary substring: swapping `setup` for `ghost` inside +# an existing `/alpha:setup` leaves `/alpha:ghost` on disk, but the hunk is the +# bare word `ghost` and carries no command for emit_refs to find, so a +# newly-broken reference would be silently missed. +# +# Recover bounded context: the edit is already applied by PostToolUse time, so pull +# from disk only the lines carrying one of the hunk's word tokens, scan those, and +# keep only references whose plugin or skill segment appears in the hunk. That +# token filter is what preserves the diff-scope contract — a pre-existing unrelated +# reference sharing one of those lines never fires. The anchor is the token, not +# the line, since a bare-word hunk carries no positional information. +reconstruct_partial_edit() { + [[ "$TOOL" == "Edit" && -f "$FILE" ]] || return 0 + # Anchor tokens come from the hunk with any COMPLETE reference removed first. + # A complete reference is already handled by the direct scan, and leaving it in + # would contribute its own plugin name as an anchor — `alpha` then matches every + # reference to that plugin, including untouched ones on neighbouring lines, which + # breaks diff-scope. What remains is the genuinely bare edited text. + local residue + residue=$(printf '%s' "$SCAN_CONTENT" | sed -E 's#/[a-z][a-z0-9-]*:[a-z][a-z0-9-]*##g') + # Minimum anchor length. A substring match on a very short token matches almost + # any segment — stripping a reference out of `Run `/alpha:x` now.` leaves the + # fragment `un`, which substring-matches `untouched-ghost` on an untouched line + # and breaks diff-scope. 4 characters is the shortest command segment worth + # anchoring on; below that the token carries no locating power. + local -a toks=() + mapfile -t toks < <(printf '%s' "$residue" | grep -oE '[a-z][a-z0-9-]{3,}' 2>/dev/null | sort -u) + ((${#toks[@]})) || return 0 + local tok lines ctx="" + for tok in "${toks[@]}"; do + lines=$(grep -F -- "$tok" "$FILE" 2>/dev/null) + [[ -n "$lines" ]] && ctx+="$lines"$'\n' + done + [[ -n "$ctx" ]] || return 0 + local saved="$SCAN_CONTENT" + SCAN_CONTENT=$(printf '%s' "$ctx" | grep -vE '^[[:space:]]*$' | head -40) + local ref seg plug skl + while IFS= read -r ref; do + [[ -n "$ref" ]] || continue + plug="${ref#/}" + plug="${plug%%:*}" + skl="${ref##*:}" + # SUBSTRING match, not equality: an Edit can replace part of a segment + # (`up` -> `host` turns `/alpha:setup` into `/alpha:sethost`), so the hunk + # token is a substring of the segment rather than the whole of it. + for seg in "${toks[@]}"; do + if [[ "$plug" == *"$seg"* || "$skl" == *"$seg"* ]]; then + printf '%s\n' "$ref" + break + fi + done + done < <(emit_refs) + SCAN_CONTENT="$saved" +} + +declare -A CHECKED=() +UNRESOLVED=() +REFS=() +mapfile -t REFS < <(emit_refs) +# Reconstruction runs on EVERY Edit, not only when the hunk yielded nothing. One +# hunk can both carry a complete reference and change a substring inside another +# (`setup and placeholder` -> `ghost` plus a literal `/alpha:setup`), so gating on +# an empty REFS would miss the partial half. Duplicates are harmless — CHECKED +# dedupes below. +if [[ "$TOOL" == "Edit" ]]; then + mapfile -t -O "${#REFS[@]}" REFS < <(reconstruct_partial_edit) +fi + +for ref in "${REFS[@]}"; do + [[ -n "$ref" ]] || continue + [[ -n "${CHECKED[$ref]:-}" ]] && continue + CHECKED["$ref"]=1 + plugin="${ref#/}" + plugin="${plugin%%:*}" + skill="${ref##*:}" + # PLUGIN-SCOPE GATE: only adjudicate a plugin this repo owns. + pdir="${PLUGIN_DIR[$plugin]:-}" + [[ -n "$pdir" ]] || continue + skill_resolves "$pdir" "$skill" && continue + UNRESOLVED+=("$ref") +done + +emit_tel() { + [[ -n "$start" ]] || return 0 + hook::telemetry_enabled || return 0 + local file_rel="$FILE" findings_json="[]" + if command -v cygpath >/dev/null 2>&1; then + local _file_lm _root_lm + _file_lm=$(cygpath -lm "$FILE" 2>/dev/null) + _root_lm=$(cygpath -lm "$REPO_ROOT" 2>/dev/null) + [[ -n "$_file_lm" && -n "$_root_lm" ]] && file_rel="${_file_lm#"$_root_lm"/}" + else + file_rel="${FILE#"$REPO_ROOT"/}" + fi + # Redaction: a path that could not be made repo-relative degrades to its + # basename — never an absolute path, which would embed the developer's + # username. + case "$file_rel" in + /* | [A-Za-z]:*) + file_rel="${file_rel##*/}" + file_rel="${file_rel##*\\}" + ;; + *) ;; + esac + if ((${#UNRESOLVED[@]} > 0)); then + local r raw_list="" + for r in "${UNRESOLVED[@]}"; do raw_list+="$r"$'\n'; done + findings_json=$(printf '%s' "$raw_list" | jq -R . | jq -s . 2>/dev/null) || findings_json="[]" + fi + local data + data=$(jq -n --arg file "$file_rel" --argjson findings "$findings_json" \ + '{tool:"",file:$file,findings:$findings}' 2>/dev/null) || + data='{"tool":"","file":"","findings":[]}' + hook::emit_telemetry "skill-reference-verify" "PostToolUse" "ok" "$start" "$data" "$REPO_ROOT" +} + +if ((${#UNRESOLVED[@]} > 0)); then + hook::ctx_append "skill-reference-verify: ${#UNRESOLVED[@]} skill reference(s) do not resolve in $FILE" + hook::ctx_append "This repo owns each plugin named below, so it can say the skill is not there:" + for r in "${UNRESOLVED[@]}"; do + plugin="${r#/}" + plugin="${plugin%%:*}" + hook::ctx_append " UNRESOLVED_SKILL: $r (no such skill under plugins/${PLUGIN_DIR[$plugin]##*/}/skills/)" + done + hook::ctx_append "" + hook::ctx_append "Detect-then-judge: this is a prompt for your verdict, not a determination." + hook::ctx_append "Confirm against the tree. A reference retained deliberately — documenting" + hook::ctx_append "a rename, or a capability another marketplace ships — is correct as written." + hook::ctx_flush PostToolUse +fi + +emit_tel +exit 0 diff --git a/plugins/guardrails/hooks/skill-reference-verify.test.sh b/plugins/guardrails/hooks/skill-reference-verify.test.sh new file mode 100755 index 000000000..7e0a195c4 --- /dev/null +++ b/plugins/guardrails/hooks/skill-reference-verify.test.sh @@ -0,0 +1,344 @@ +#!/usr/bin/env bash +# Contract test for skill-reference-verify.sh (guardrails plugin). +# +# Black-box subprocess invocation. Builds a synthetic marketplace working tree so +# the plugins-root and plugin-scope gates have both a hit and a miss to +# distinguish, then feeds a PostToolUse Write|Edit payload on stdin. + +# shellcheck disable=SC2016 +# Every fixture below is markdown fed to the hook as literal data, and backticks +# are the code-span delimiters the hook scans for — single quotes are required so +# nothing expands. Disabled file-wide rather than on ~20 individual lines. + +set -uo pipefail + +HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$HOOK_DIR/skill-reference-verify.sh" +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +# shellcheck source=guardrails-test-helpers.sh +source "$HOOK_DIR/guardrails-test-helpers.sh" + +unset CLAUDE_PROJECT_DIR +RC=0 + +# hook::buffer_stdin bounds its fd0 read at CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT +# seconds, default 2, and a timeout makes an advisory hook exit 0 silently. A +# single invocation costs 10-20 s of wall time on a loaded Windows/Git Bash box, so +# the default bound produces empty output and a false FAIL that looks like a logic +# defect. Raised here because these cases test detection, not the read bound. +export CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT=30 + +# --- Synthetic marketplace --------------------------------------------------- +# Two plugins. `alpha`'s manifest name matches its directory. `beta`'s manifest +# name deliberately DIVERGES from its directory (`beta-dir`), which is what +# proves resolution goes through the manifest rather than the directory listing. +REPO="$TEST_TMPDIR/market" +mkdir -p "$REPO" +git -C "$REPO" init -q + +mk_plugin() { + local dir="$1" name="$2" + mkdir -p "$REPO/plugins/$dir/.claude-plugin" + MSYS_NO_PATHCONV=1 jq -n --arg n "$name" '{name:$n,version:"0.1.0"}' \ + >"$REPO/plugins/$dir/.claude-plugin/plugin.json" +} +# mk_skill [frontmatter-name] +mk_skill() { + local pdir="$1" sdir="$2" fname="${3:-}" + mkdir -p "$REPO/plugins/$pdir/skills/$sdir" + local f="$REPO/plugins/$pdir/skills/$sdir/SKILL.md" + if [[ -n "$fname" ]]; then + { + printf -- '---\n' + printf 'name: %s\n' "$fname" + printf 'description: x\n' + printf -- '---\n' + } >"$f" + else + : >"$f" + fi +} + +mk_plugin alpha alpha +mk_skill alpha setup +mk_skill alpha audit +# Command name comes from frontmatter, not the directory. +mk_skill alpha legacy-dir renamed-command +# Quoted frontmatter name must resolve identically. +mk_skill alpha quoted-dir '"quoted-name"' + +mk_plugin beta-dir beta +mk_skill beta-dir check + +TARGET="$REPO/notes.md" +: >"$TARGET" + +run() { + local out + out=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(write_json "$TARGET" "$1")" 2>&1) + RC=$? + printf '%s' "$out" +} +run_edit() { + local out + out=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(edit_json "$TARGET" "$1")" 2>&1) + RC=$? + printf '%s' "$out" +} + +# ============================ MUST FIRE ===================================== + +OUT=$(run 'Run `/alpha:nonexistent` next.') +assert_exit "unresolved skill → exit 0 (advisory)" 0 "$RC" +assert_contains "unresolved skill → named" "$OUT" "UNRESOLVED_SKILL: /alpha:nonexistent" +assert_contains "unresolved skill → names the searched plugin dir" "$OUT" "plugins/alpha/skills/" + +OUT=$(run 'Both `/alpha:ghost` and `/beta:missing` are gone.') +assert_contains "two unresolved → count" "$OUT" "2 skill reference(s) do not resolve" +assert_contains "two unresolved → manifest-named plugin resolves to its real dir" "$OUT" \ + "plugins/beta-dir/skills/" + +OUT=$(run_edit 'Now cites `/alpha:vanished`.') +assert_contains "Edit hunk scanned via new_string" "$OUT" "UNRESOLVED_SKILL: /alpha:vanished" + +# The advisory must state its tier — the issue this guard ships under requires +# the third guard never read as deterministic. +OUT=$(run 'Run `/alpha:nonexistent`.') +assert_contains "advisory states detect-then-judge tier" "$OUT" "Detect-then-judge" + +# Repro-first regressions for review findings on #1284. + +# A bare directory under skills/ is NOT a skill. Without a SKILL.md there is no +# command, so the advisory must still fire — the old directory-only test +# suppressed it. +mkdir -p "$REPO/plugins/alpha/skills/ghost" +OUT=$(run 'Run `/alpha:ghost`.') +assert_contains "skills/ dir without SKILL.md → still unresolved" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost" + +# A frontmatter name carrying a trailing YAML comment is a valid rename and must +# resolve; the old end-of-line-anchored parser extracted nothing. +mk_skill alpha commented-dir 'commented-name # public command' +OUT=$(run 'Run `/alpha:commented-name`.') +assert_silent "frontmatter name with trailing YAML comment → resolves" "$OUT" + +# The DIRECTORY name of a renamed skill is not an alias. `skills/legacy-dir/` +# declaring `name: renamed-command` answers to /alpha:renamed-command only — the +# stale pre-rename spelling must still fire, since suppressing it defeats the +# guard's whole purpose. +OUT=$(run 'Stale ref `/alpha:legacy-dir`.') +assert_contains "renamed skill's directory name is NOT an alias → fires" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:legacy-dir" +OUT=$(run 'Current ref `/alpha:renamed-command`.') +assert_silent "the declared frontmatter name still resolves" "$OUT" + +# A skill declaring no frontmatter name answers to its directory name. +OUT=$(run 'Run `/alpha:setup`.') +assert_silent "no frontmatter name → directory name is the command" "$OUT" + +# An invocation carrying arguments is the common form in this repo. The reference +# is the leading token of the span, not the whole span. +OUT=$(run 'Run `/alpha:ghost-arg --apply` now.') +assert_contains "argument-bearing invocation → leading token extracted" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost-arg" +OUT=$(run 'Run `/alpha:audit --dry-run `.') +assert_silent "argument-bearing invocation of a real skill → silent" "$OUT" + +# ============================ MUST STAY QUIET =============================== + +OUT=$(run 'Run `/alpha:setup` and `/alpha:audit` first.') +assert_exit "resolving skills → exit 0" 0 "$RC" +assert_silent "resolving skills → silent" "$OUT" + +# Manifest name diverges from directory name; the reference uses the manifest name. +OUT=$(run 'Run `/beta:check`.') +assert_silent "plugin resolved via manifest name, not directory name → silent" "$OUT" + +# Frontmatter name diverges from the skill directory name. +OUT=$(run 'Run `/alpha:renamed-command`.') +assert_silent "skill resolved via frontmatter name → silent" "$OUT" + +OUT=$(run 'Run `/alpha:quoted-name`.') +assert_silent "skill resolved via quoted frontmatter name → silent" "$OUT" + +# PLUGIN-SCOPE GATE: this repo does not own the plugin, so it has no authority. +OUT=$(run 'Install `/some-other-market:thing` from elsewhere.') +assert_silent "unowned plugin → not adjudicated" "$OUT" + +# A reference to a directory that exists but carries no manifest is not a plugin. +mkdir -p "$REPO/plugins/not-a-plugin/skills/x" +OUT=$(run 'Run `/not-a-plugin:x`.') +assert_silent "directory without a manifest → not a plugin, silent" "$OUT" + +OUT=$(run 'Unbackticked /alpha:nonexistent in prose is not a command.') +assert_silent "unbackticked reference → not scanned" "$OUT" + +OUT=$(run 'A URL fragment `https://x.test/a:b` and a time `12:30` are not references.') +assert_silent "URL fragment and time → silent" "$OUT" + +OUT=$(run 'Uppercase `/Alpha:Setup` does not match the command grammar.') +assert_silent "uppercase reference → silent (not command-shaped)" "$OUT" + +# PLUGINS-ROOT GATE: outside a marketplace repo there is no local authority. +PLAIN="$TEST_TMPDIR/plain" +mkdir -p "$PLAIN" +git -C "$PLAIN" init -q +PLAIN_TARGET="$PLAIN/notes.md" +: >"$PLAIN_TARGET" +OUT=$(CLAUDE_PROJECT_DIR="$PLAIN" bash "$HOOK" <<<"$(write_json "$PLAIN_TARGET" 'Run `/alpha:nonexistent`.')" 2>&1) +RC=$? +assert_exit "no plugins/ tree → exit 0" 0 "$RC" +assert_silent "no plugins/ tree → silent (no local authority)" "$OUT" + +# A plugins/ directory with no manifests is not a marketplace either. +EMPTY="$TEST_TMPDIR/empty-market" +mkdir -p "$EMPTY/plugins/whatever" +git -C "$EMPTY" init -q +EMPTY_TARGET="$EMPTY/notes.md" +: >"$EMPTY_TARGET" +OUT=$(CLAUDE_PROJECT_DIR="$EMPTY" bash "$HOOK" <<<"$(write_json "$EMPTY_TARGET" 'Run `/alpha:nonexistent`.')" 2>&1) +RC=$? +assert_exit "plugins/ with no manifests → exit 0" 0 "$RC" +assert_silent "plugins/ with no manifests → silent" "$OUT" + +# Non-markdown files are out of scope. +SCRIPT="$REPO/tool.sh" +: >"$SCRIPT" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(write_json "$SCRIPT" 'echo `/alpha:nonexistent`')" 2>&1) +RC=$? +assert_exit "non-markdown file → exit 0" 0 "$RC" +assert_silent "non-markdown file → not scanned" "$OUT" + +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(notebook_json "$TARGET" '`/alpha:nonexistent`')" 2>&1) +RC=$? +assert_exit "NotebookEdit payload → exit 0" 0 "$RC" +assert_silent "NotebookEdit payload → silent" "$OUT" + +OUTSIDE="$TEST_TMPDIR/outside.md" +: >"$OUTSIDE" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(write_json "$OUTSIDE" 'Run `/alpha:nonexistent`.')" 2>&1) +RC=$? +assert_exit "file outside project dir → exit 0" 0 "$RC" +assert_silent "file outside project dir → silent" "$OUT" + +# A CHANGELOG is an append-only historical record — a rename entry must keep +# naming the old command, so the guard must not adjudicate one. Measured cause of +# 89% of this guard's findings on the marketplace corpus. +CL="$REPO/plugins/alpha/CHANGELOG.md" +: >"$CL" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(write_json "$CL" 'Renamed the `/alpha:nonexistent` skill to `/alpha:audit`.')" 2>&1) +RC=$? +assert_exit "CHANGELOG.md → exit 0" 0 "$RC" +assert_silent "CHANGELOG.md is historical by contract → silent" "$OUT" + +# The exclusion is basename-scoped, not a substring: a file merely mentioning the +# word must still be adjudicated. +NOTCL="$REPO/docs-CHANGELOG-notes.md" +mkdir -p "$(dirname "$NOTCL")" +: >"$NOTCL" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(write_json "$NOTCL" 'Run `/alpha:nonexistent`.')" 2>&1) +assert_contains "CHANGELOG-in-name but not a CHANGELOG → still adjudicated" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:nonexistent" + +# PARTIAL-EDIT RECONSTRUCTION. An Edit may replace an arbitrary substring, so a +# hunk can be a bare word with no command in it. The edit is already applied by +# PostToolUse time, so the containing reference is recovered from disk, anchored to +# the hunk's tokens. +PARTIAL="$REPO/partial.md" +printf 'Run `/alpha:ghost-partial` to begin.\n' >"$PARTIAL" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(edit_json "$PARTIAL" 'ghost-partial')" 2>&1) +RC=$? +assert_exit "bare-word Edit hunk → exit 0" 0 "$RC" +assert_contains "bare-word Edit hunk → containing reference recovered" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost-partial" + +# Diff-scope is preserved: a PRE-EXISTING unrelated broken reference on a +# neighbouring line must NOT fire just because reconstruction read from disk. +PARTIAL2="$REPO/partial2.md" +printf 'Stale `/alpha:untouched-ghost` here.\nRun `/alpha:ghost-two` to begin.\n' >"$PARTIAL2" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(edit_json "$PARTIAL2" 'ghost-two')" 2>&1) +assert_contains "reconstruction reports the edited reference" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost-two" +assert_absent "reconstruction does NOT report an untouched neighbour" "$OUT" \ + "untouched-ghost" + +# A hunk that already carries a full command is scanned directly. +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(edit_json "$PARTIAL2" 'Run `/alpha:ghost-three` now.')" 2>&1) +assert_contains "full-command hunk → scanned directly" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost-three" +assert_absent "full-command hunk → no disk reconstruction leakage" "$OUT" \ + "untouched-ghost" + +# An Edit can replace PART of a segment: `up` -> `host` turns `/alpha:setup` into +# `/alpha:sethost`. The hunk token is then a substring of the segment, not the +# whole of it, so the filter must match on substring. +SUBST="$REPO/substr.md" +printf 'Run `/alpha:sethost` to begin.\n' >"$SUBST" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(edit_json "$SUBST" 'host')" 2>&1) +assert_contains "substring Edit inside a segment → recovered" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:sethost" + +# A hunk carrying BOTH a complete reference and a substring change to another must +# report both — gating reconstruction on an empty scan would miss the partial half. +MIXED="$REPO/mixed.md" +printf 'First `/alpha:ghost-mixed` here.\nSecond `/alpha:audit` is fine.\n' >"$MIXED" +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(edit_json "$MIXED" 'ghost-mixed and `/alpha:ghost-direct`')" 2>&1) +assert_contains "mixed hunk → direct reference reported" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost-direct" +assert_contains "mixed hunk → reconstructed reference also reported" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost-mixed" + +# Reconstruction must not double-report a reference reachable both ways. +OUT=$(CLAUDE_PROJECT_DIR="$REPO" bash "$HOOK" <<<"$(edit_json "$MIXED" 'Run `/alpha:ghost-mixed` again.')" 2>&1) +assert_contains "reference reachable both ways → reported" "$OUT" \ + "UNRESOLVED_SKILL: /alpha:ghost-mixed" +assert_contains "reference reachable both ways → counted once" "$OUT" \ + "1 skill reference(s) do not resolve" + +# ============================ KILL SWITCH =================================== + +OUT=$(CLAUDE_PROJECT_DIR="$REPO" CLAUDE_PLUGIN_OPTION_SKILL_REFERENCE_VERIFY_ENABLED=false \ + bash "$HOOK" <<<"$(write_json "$TARGET" 'Run `/alpha:nonexistent`.')" 2>&1) +RC=$? +assert_exit "kill switch → exit 0" 0 "$RC" +assert_silent "kill switch → silent" "$OUT" + +# ============================ EMPTY STDIN =================================== + +OUT=$(bash "$HOOK" &1) +RC=$? +assert_exit "empty stdin → exit 0" 0 "$RC" +assert_silent "empty stdin → no output" "$OUT" + +# ============================ MISSING PREREQUISITE ========================== + +# Runtime jq-removal is not portably simulable — an isolated bin dir without jq +# cannot host bash + coreutils across Git Bash and Linux, the same constraint +# secret-pattern-detection.test.sh and require-jq-notice-isolation.test.sh both +# document. Assert the fail-open guard is present via the shared helper; +# require_jq's own behavior is covered by lib/hook-utils.test.sh, and this hook's +# notice key is proven unique plugin-wide by require-jq-notice-isolation.test.sh. +HOOK_SRC=$(cat "$HOOK") +assert_contains "jq guard: uses hook::require_jq" "$HOOK_SRC" 'hook::require_jq' +assert_contains "jq guard: hook-specific notice key" "$HOOK_SRC" 'guardrails-skill-reference-verify' +assert_contains "repo root is file-anchored" "$HOOK_SRC" 'hook::repo_root "$(dirname "$FILE")"' + +# ============================ TELEMETRY ===================================== + +TEL="$(mktemp -p "$TEST_TMPDIR")" +SINK="$(make_sink "cat >\"$TEL\"")" +CLAUDE_PROJECT_DIR="$REPO" HOOK_TELEMETRY_SINK="$SINK" \ + bash "$HOOK" <<<"$(write_json "$TARGET" 'Run `/alpha:nonexistent`.')" >/dev/null 2>&1 || true +if wait_for_sink "$TEL"; then + assert_contains "telemetry: hook id" "$(jq -r '.hook' "$TEL")" "skill-reference-verify" + assert_contains "telemetry: status ok" "$(jq -r '.status' "$TEL")" "ok" + assert_contains "telemetry: findings name the reference" "$(jq -r '.data.findings[]' "$TEL")" "/alpha:nonexistent" + assert_absent "telemetry: file path is repo-relative, never absolute" \ + "$(jq -r '.data.file' "$TEL")" "$TEST_TMPDIR" +else + bad "telemetry: no envelope written" +fi + +report