From 94ccfd18c7082e2eca0b151a43b213ee12fd35c9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 20 Jul 2026 14:21:17 -0400 Subject: [PATCH 1/6] feat(guardrails): block non-canonical git commit; drop --trailer conjunct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The advisory covering `git commit` was overridden 11 times in a single session. An advisory that is always overridden trains the reader to filter it out, so the commit half becomes a blocking guard. It gates the MECHANIC, not the ritual. `git commit -m ""` flattens newlines unpredictably across shells; `-F -` is what prevents that. Gating on "did you invoke /commit" is not available anyway — anthropics/claude-code#22655 (expose skill_name to hooks) is closed as not planned — and shape is the better target regardless: it enforces an outcome a reviewer can verify in git log. Exempt, because no message-on-stdin form exists for them: --amend, -C/-c/--reuse-message/--reedit-message, --fixup/--squash, -F , and any commit taken mid-merge/rebase/cherry-pick/revert. Gating those would strand a conflict resolution. The old condition also required --trailer, which was a latent false positive: /commit omits the trailer under trailer_policy `none`, so a repo forbidding co-author trailers saw its own conformant commits flagged every time. That had to be settled before blocking on the same condition — requiring --trailer to pass would have permanently blocked the skill in that configuration. gh pr create stays advisory. /pull-request create issues that exact command itself and no shape distinguishes them, so blocking it would deadlock the skill. Closes #731 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_011ogVV7z2Heg9ojJ88nqNxh --- plugins/guardrails/.claude-plugin/plugin.json | 14 +- plugins/guardrails/CHANGELOG.md | 34 +++ .../hooks/block-noncanonical-commit.sh | 198 ++++++++++++++++++ .../hooks/block-noncanonical-commit.test.sh | 134 ++++++++++++ .../hooks/flag-commit-pr-skill-bypass.sh | 34 +-- .../hooks/flag-commit-pr-skill-bypass.test.sh | 36 ++-- plugins/guardrails/hooks/hooks.json | 10 + 7 files changed, 416 insertions(+), 44 deletions(-) create mode 100755 plugins/guardrails/hooks/block-noncanonical-commit.sh create mode 100755 plugins/guardrails/hooks/block-noncanonical-commit.test.sh diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index bc1e605e1..a7bc14fc0 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "guardrails", - "version": "0.8.0", + "version": "0.9.0", "description": "Eight 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, (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.", "author": { "name": "Melodic Software", @@ -50,6 +50,12 @@ "description": "Block Bash file-write workarounds that circumvent Write/Edit hook gates", "default": true }, + "block_noncanonical_commit_enabled": { + "type": "boolean", + "title": "block-noncanonical-commit guard", + "description": "Block `git commit` that does not pipe its message via `-F -`; --amend, -C/-c, --fixup/--squash, -F , and an in-progress merge/rebase are exempt", + "default": true + }, "cli_flag_verify_enabled": { "type": "boolean", "title": "cli-flag-verify guard", @@ -85,6 +91,12 @@ "title": "block-dangerous-git allow-list", "description": "Comma-separated forms block-dangerous-git permits: push-force, reset-hard, clean-force, checkout-dot, restore-dot, checkout-force; empty blocks all", "default": "" + }, + "block_noncanonical_commit_allow": { + "type": "string", + "title": "block-noncanonical-commit allow-list", + "description": "Comma-separated form tokens to allow (currently: message-flag, which permits a bare `-m`)", + "default": "" } } } diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index c745861c5..f2b4c6012 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,40 @@ 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.9.0] + +### Added + +- **`block-noncanonical-commit` — `git commit` must pipe its message via `-F -`.** The advisory that + previously covered this was overridden 11 times in a single session; an advisory that is always + overridden trains the reader to filter it out. The guard enforces the *mechanic*, not the ritual: + `git commit -m ""` flattens newlines unpredictably across shells, and the stdin form is + what prevents it. Exempt, because no message-on-stdin form exists for them and gating them would + strand real work: `--amend`/`--no-edit`, `-C`/`-c`/`--reuse-message`/`--reedit-message`, + `--fixup`/`--squash`, `-F `, and any commit taken while a merge, rebase, cherry-pick, or + revert is in progress. Kill switch `block_noncanonical_commit_enabled`; allow-list + `block_noncanonical_commit_allow` (`message-flag` permits a bare `-m`). Detection reuses the + argv-grammar-faithful parser, so `bash -lc` wrappers and git aliases resolve and a commit body + merely *mentioning* `git commit -m` never fires. + +### Fixed + +- **`flag-commit-pr-skill-bypass` no longer demands `--trailer`.** The old condition required both + `-F -` **and** `--trailer`, but `/commit` omits the trailer when the resolved `trailer_policy` is + `none` — so in a repo whose convention forbids a co-author trailer, the skill's own conformant + output was flagged on every commit. The trailer is policy; only the stdin form is mechanic. This + also had to be settled before the new guard could block on the same condition: requiring + `--trailer` to pass would have permanently blocked `/commit` in that configuration. + +### Changed + +- **`flag-commit-pr-skill-bypass` is now `gh pr create`-only.** The `git commit` branch moved to + `block-noncanonical-commit`, so the two never double-fire on one command. `gh pr create` stays + advisory and cannot become otherwise: `/pull-request create` issues that exact command itself, and + [anthropics/claude-code#22655](https://github.com/anthropics/claude-code/issues/22655) (expose + `skill_name` to hooks) is closed as not planned — a hook cannot tell a skill-driven call from an + ad hoc one, so blocking it would deadlock the skill. + ## [0.8.0] ### Changed diff --git a/plugins/guardrails/hooks/block-noncanonical-commit.sh b/plugins/guardrails/hooks/block-noncanonical-commit.sh new file mode 100755 index 000000000..beb87651c --- /dev/null +++ b/plugins/guardrails/hooks/block-noncanonical-commit.sh @@ -0,0 +1,198 @@ +#!/usr/bin/env bash +# PreToolUse hook: block `git commit` that does not feed its message via stdin. +# Triggered on Bash tool calls. +# +# WHAT IT ENFORCES — the mechanic, not the ritual: +# `git commit -F -` (or `--file -`), the form the /commit skill emits. The +# failure mode it prevents is real and silent: `git commit -m ""` +# flattens newlines unpredictably across shells, so a body that looked right +# in the tool call lands mangled in history. +# +# WHY NOT `--trailer`: the trailer is POLICY, not mechanic. /commit itself +# omits it when the resolved trailer_policy is `none`, and a repo whose +# convention forbids a co-author trailer is a documented, supported case. +# Gating on it would permanently block the skill's own canonical output in that +# configuration. Only the stdin form belongs in a gate. +# +# WHY NOT "did you type /commit": a hook cannot tell a skill-driven Bash call +# from an ad hoc one — anthropics/claude-code#22655 (add skill_name to hook +# payloads) is closed as not planned. Gating on command SHAPE is what is +# actually available, and is the better target anyway: it enforces the outcome +# a reviewer can verify in `git log`, not the ceremony that produced it. +# +# NOT BLOCKED (no message-on-stdin form exists for these, and gating them would +# break conflict resolution and history rewriting): +# --amend / --no-edit reusing an existing message +# -C / --reuse-message " +# -c / --reedit-message " +# --fixup / --squash message derived from another commit +# -F / --file mechanic satisfied, just not via stdin +# -m during an in-progress sequencer (merge / rebase / cherry-pick / revert): +# conflict resolution and rebase continuation +# must never be gated +# +# Per-repo/per-user allow-list: the `block_noncanonical_commit_allow` userConfig +# option is a comma-separated list of form tokens (currently just +# "message-flag", which allows a bare `-m`). Set it with +# `/plugin configure guardrails` or headless via `claude plugin install +# --config`; read from the CLAUDE_PLUGIN_OPTION_BLOCK_NONCANONICAL_COMMIT_ALLOW +# process mirror. Kill switch: `block_noncanonical_commit_enabled` set to false. +# +# Detection is ARGV-GRAMMAR-FAITHFUL via the shared parser in hook-utils.sh, so +# a commit body merely MENTIONING `git commit -m` never fires, and `bash -lc` +# wrappers plus git aliases are resolved. Static matching over the literal +# command string only: shell variable / command substitution is not evaluated. +# +# BLOCKING: exits 2 when a commit would take its message off the command line. + +set -uo pipefail + +# shellcheck source=hook-utils.sh +source "$(dirname "${BASH_SOURCE[0]}")/hook-utils.sh" + +hook::check_enabled "BLOCK_NONCANONICAL_COMMIT" + +# High-res start stamp for the telemetry envelope. EPOCHREALTIME is Bash 5.0+; +# on older bash it is unset, so default to empty and skip telemetry (the block +# still fires). Referencing it bare under `set -u` would abort before exit. +start=${EPOCHREALTIME:-} + +# jq is required to parse the tool payload. Fail OPEN when it is absent, but +# make the degraded state visible rather than silently disabling the guard. +if ! command -v jq >/dev/null 2>&1; then + echo "guardrails/block-noncanonical-commit: jq not found on PATH — guard disabled (install jq to enable)." >&2 + exit 0 +fi + +# hook::buffer_stdin encapsulates the Win32-pipe-safe bounded fd0 read. rc 1 +# (empty stdin) skips; rc 2 (read timed out before a complete payload) FAILS +# CLOSED — the guard cannot evaluate the tool call, and a silent skip would pass +# exactly the traffic this guard exists to stop. +INPUT=$(hook::buffer_stdin) || { + rc=$? + ((rc == 2)) && exit 2 + exit 0 +} +COMMAND=$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null | tr -d '\r') +[[ -n "$COMMAND" ]] || exit 0 +HOOK_CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // empty' 2>/dev/null | tr -d '\r') + +SUBJECT=$(hook::extract_bash_subject "Bash" "$COMMAND") + +emit_tel() { + [[ -n "$start" ]] || return 0 + hook::telemetry_enabled || return 0 + local data + data=$(jq -n --arg subject "$SUBJECT" --arg form "$2" \ + '{tool:"Bash",subject:$subject,form:$form}' 2>/dev/null) || data='{"tool":"Bash","subject":"","form":""}' + hook::emit_telemetry "block-noncanonical-commit" "PreToolUse" "$1" "$start" "$data" "${CLAUDE_PROJECT_DIR:-}" +} + +# Is a form token in the block_noncanonical_commit_allow userConfig comma list? +# shellcheck disable=SC2329 # reached via the hook::bash_parse_segments callback chain +allowed() { + local tok="$1" list=",${CLAUDE_PLUGIN_OPTION_BLOCK_NONCANONICAL_COMMIT_ALLOW:-}," + [[ "$list" == *,"$tok",* ]] +} + +# Is a merge / rebase / cherry-pick / revert in progress? Those commits carry a +# prepared message git supplies, and `git commit` there is the documented way to +# conclude the operation — gating it would strand a conflict resolution +# mid-flight. Unknown git state answers "no": this is the permissive branch, so +# an uncertain answer must not silently open the gate. +# shellcheck disable=SC2329 # reached via the hook::bash_parse_segments callback chain +sequencer_in_progress() { + local dir f + # --absolute-git-dir, not --git-dir: the latter answers relative to the repo, + # which would resolve against the HOOK's cwd here and silently miss every + # sequencer file. + dir=$(git -C "${HOOK_CWD:-${CLAUDE_PROJECT_DIR:-.}}" rev-parse --absolute-git-dir 2>/dev/null) || return 1 + [[ -n "$dir" ]] || return 1 + for f in MERGE_HEAD CHERRY_PICK_HEAD REVERT_HEAD rebase-merge rebase-apply; do + [[ -e "$dir/$f" ]] && return 0 + done + return 1 +} + +# shellcheck disable=SC2329 # invoked indirectly as the hook::bash_parse_segments callback +check_segment() { + local -a w=() + local gi sub sub_idx nseg k word next stdin_form=0 exempt=0 saw_commit=0 + + # A shell -c wrapper (`bash -lc 'git commit -m x'`) executes its operand as a + # full shell command — re-parse it with the same tokenizer. + if hook::shell_c_operand "$@"; then + hook::bash_parse_segments "$HOOK_SHELL_C_OPERAND" check_segment + return 0 + fi + + hook::git_resolve_index "$@" || return 0 + gi=$HOOK_GIT_RESOLVED_GI + w=("${HOOK_GIT_RESOLVED_WORDS[@]}") + nseg=${#w[@]} + + hook::git_resolve_subcommand "$gi" "${w[@]}" || return 0 + sub=$HOOK_GIT_SUB + sub_idx=$HOOK_GIT_SUB_IDX + [[ "$sub" == "commit" ]] || return 0 + saw_commit=1 + + # Scan only the words AFTER the subcommand: a top-level `git -c foo=bar` is + # config, while `-c` after `commit` is --reedit-message. + for ((k = sub_idx + 1; k < nseg; k++)); do + word="${w[k]}" + next="" + ((k + 1 < nseg)) && next="${w[k + 1]}" + case "$word" in + --) + break + ;; + -F | --file) + if [[ "$next" == "-" ]]; then stdin_form=1; else exempt=1; fi + ((k++)) + ;; + -F- | --file=-) + stdin_form=1 + ;; + --file=*) + exempt=1 + ;; + -F*) + exempt=1 + ;; + --amend | --no-edit | --fixup | --squash | -C | -c | --reuse-message | --reedit-message) + exempt=1 + ;; + --fixup=* | --squash=* | --reuse-message=* | --reedit-message=*) + exempt=1 + ;; + -C* | -c*) + exempt=1 + ;; + *) + # Any other word (paths, -a, -S, --cleanup, the -m payload) is not a + # message-source marker and needs no handling here. + ;; + esac + done + + ((saw_commit)) || return 0 + ((stdin_form || exempt)) && return 0 + allowed "message-flag" && return 0 + sequencer_in_progress && return 0 + + echo "BLOCKED: \`git commit\` without \`-F -\` — the message must be piped via stdin." >&2 + echo "Use the /commit skill (source-control plugin), or its canonical form directly:" >&2 + echo " git commit -F - --cleanup=verbatim <<'EOF'" >&2 + echo " " >&2 + echo " EOF" >&2 + echo "A \`-m\` message flattens newlines unpredictably across shells. --amend, -C/-c," >&2 + echo "--fixup/--squash, -F , and an in-progress merge/rebase are exempt." >&2 + emit_tel "blocked" "message-flag" + exit 2 +} + +hook::bash_parse_segments "$COMMAND" check_segment + +emit_tel "ok" "" +exit 0 diff --git a/plugins/guardrails/hooks/block-noncanonical-commit.test.sh b/plugins/guardrails/hooks/block-noncanonical-commit.test.sh new file mode 100755 index 000000000..1409dafd5 --- /dev/null +++ b/plugins/guardrails/hooks/block-noncanonical-commit.test.sh @@ -0,0 +1,134 @@ +#!/usr/bin/env bash +# Contract test for block-noncanonical-commit.sh (guardrails plugin). +# +# Black-box: invokes the hook as a subprocess, pipes PreToolUse Bash JSON on +# stdin, asserts on exit code (2 = blocked, 0 = allowed). Self-contained — no +# host-repo assertion library. + +set -uo pipefail + +HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$HOOK_DIR/block-noncanonical-commit.sh" +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +# shellcheck source=guardrails-test-helpers.sh +source "$HOOK_DIR/guardrails-test-helpers.sh" + +# run