bug(repo): report five error-handling defects in vendored skills to mattpocock/skills

[orioltf]

Parent: #297 · Found by the comprehensive review of #298 (H2, M2–M5, L5)

Problem

Five reproduced defects in the vendored mattpocock/skills tree. All are upstream-owned:
.agents/skills/** is overwritten by every npx skills add, so a local patch is the one fix
guaranteed to disappear. The interim caveat for H2 already landed in AGENTS.md § Upgrading.

H2 · git-guardrails-claude-code/scripts/block-dangerous-git.sh fails open. It reads the
PreToolUse payload with jq and never checks the extraction worked. No set -euo pipefail.
Reproduced locally:

Input	Exit	Meaning
malformed JSON	0	allow
{} (empty COMMAND)	0	allow
{"tool_input":{"command":"git -C . push origin main"}}	0	allow
{"tool_input":{"command":"echo \"remember to git push later\""}}	2	block (false positive)

The git -C bypass is the sharpest of these and was not in the original filing: the patterns are
unanchored substrings, so any git invocation with a global flag before the subcommand walks
through. No missing jq required. Hook stderr on a pass is not surfaced, so absence of protection
looks identical to presence, and the skill's own verification step (SKILL.md step 5) tests only
the matching-command path.

M2 · wizard/template.sh:78-88 — open_url runs its opener chain in a brace group redirected
to >/dev/null 2>&1; the else branch calls warn, which writes to stdout, so the fallback
warning is discarded too. The brace group exits 0 and the trailing || warn never fires. Nothing
opens, nothing is said. Root cause is warn on stdout.

M3 · wizard/template.sh:105-147 — ask/ask_secret end with read -r input || true, so EOF
is indistinguishable from an empty Enter. write_env then writes KEY=, prints a green ✓ wrote,
and exits 0. ask_secret does not echo, so nothing on screen hints the value was empty.

M4 · wizard/template.sh:151-176 — set_secret/set_var funnel every non-zero exit into
"gh not ready; set it later", hiding no-origin, insufficient scope, wrong repo, rate limit and
SSO-gated org. The human re-authenticates, succeeds, re-runs, gets the same line. Surfacing gh's
stderr is safe: the value goes in on stdin, not argv.

M5 · diagnosing-bugs/scripts/hitl-loop.template.sh:17-27,40-42 — bare read -r -p under
set -euo pipefail. On EOF the script dies before the --- Captured --- block that is its whole
output contract. Documented usage is "the agent runs the script", so without a tty every run exits 1
with nothing parseable.

L5 · wizard/template.sh:98-102 — _existing's guard is load-bearing on pipefail; one-line
note in the same report.

Note the inconsistency worth telling upstream: wizard/template.sh writes read -r input || true
everywhere, hitl-loop.template.sh writes bare read. Same release, opposite handling, no
diagnostic in either.

Acceptance criteria

• One issue (or PR) opened at mattpocock/skills covering H2, M2–M5 and L5, each with its
  reproduction
• H2 proposes set -euo pipefail, a command -v jq precondition, anchored patterns, and a
  fail-closed default
• The upstream reference is recorded here, and this issue closes when it is filed — not when
  upstream merges
• No file under .agents/skills/** is modified in this repo (git diff proves it)

Out of scope

Patching the vendored tree locally. AGENTS.md § Agent skills states why.

---

🤖 Filed with Claude Code.

[orioltf]

Two more bypasses for the H2 report, from Copilot's review of #298

Both reproduced against .agents/skills/git-guardrails-claude-code/scripts/block-dangerous-git.sh:

Command	Exit	Meaning
git checkout -- .	0	allow — discards every unstaged change in the tree
git restore -- .	0	allow — same
git checkout .	2	block
git restore .	2	block

The patterns git checkout \. and git restore \. match only the no-flag form. -- . is the more
idiomatic spelling of exactly the same destructive operation, and it is the one that passes. Together
with the git -C . push bypass already in this issue, the pattern list has a shape problem rather than
three separate gaps: it matches substrings of a command string instead of parsing the git invocation.

Worth putting to upstream as a single point — anchoring the patterns will not fix git -C, and adding
-- . variants will not fix the next flag someone puts between git and the subcommand. The robust
form is to parse out the subcommand and its arguments, then match on those.

Copilot's second observation matches this issue's existing framing: the hook fails open when jq is
missing or the payload will not parse. No change needed there.

---

🤖 Filed with Claude Code.