fix(hooks): fail closed on missing/malformed jq in four PreToolUse gates

.gitapex/ssot.json

@@ -84,7 +84,7 @@
       "id": "bash-cli-write-and-install-guard",
       "kind": "script",
       "script": "hooks/check-bash-safety.sh",
-      "rule": "Denies a Bash call matching a package/plugin-install verb; denies gh issue/gh pr write subcommands and gh api writes (POST/PUT/PATCH/DELETE, a field flag, or 'mutation' in gh api graphql); on git push, runs skills/outward-artifact-preflight/scripts/gitapex_scan_provenance.py against the outgoing commit range and warns (never blocks) if it flags something.",
+      "rule": "Denies a Bash call matching a package/plugin-install verb; denies gh issue/gh pr write subcommands and gh api writes (POST/PUT/PATCH/DELETE, a field flag, or 'mutation' in gh api graphql); on git push, runs skills/outward-artifact-preflight/scripts/gitapex_scan_provenance.py against the outgoing commit range and warns (never blocks) if it flags something. Fails closed (denies) rather than allowing the call through when jq is missing from PATH, the payload or tool_input is not a JSON object, tool_name is present but not a string, or tool_input.command is present but not a string.",
       "planes": ["pretooluse"],
       "local_exclusion": "PreToolUse-only: grades a Claude Code tool-call JSON payload arriving on stdin, which has no working-tree equivalent to reconstruct ahead of a push.",
       "trigger": "PreToolUse matcher Bash (hooks/hooks.json)",
@@ -114,7 +114,7 @@
       "id": "template-overwrite-guard",
       "kind": "script",
       "script": "hooks/check-template-overwrite.sh",
-      "rule": "Denies a Write call whose file_path case-insensitively matches an existing GitHub/GitLab issue/PR/MR template location.",
+      "rule": "Denies a Write call whose file_path case-insensitively matches an existing GitHub/GitLab issue/PR/MR template location. Fails closed (denies) rather than allowing the call through when jq is missing from PATH, the payload or tool_input is not a JSON object, tool_name is present but not a string, or tool_input.file_path is present but not a string.",
       "planes": ["pretooluse"],
       "local_exclusion": "PreToolUse-only: grades a Claude Code Write tool-call payload, not repository state.",
       "trigger": "PreToolUse matcher Write (hooks/hooks.json)",
@@ -163,7 +163,7 @@
       "id": "merge-pull-request-block",
       "kind": "script",
       "script": "hooks/check-merge-pull-request-block.sh",
-      "rule": "Denies mcp__github__merge_pull_request unconditionally, no override -- per planning-a-branch-from-an-issue/SKILL.md (\"Do not merge or enable auto-merge; that is a separate, explicit human or CI decision\"), drafting-a-pr-to-merge/SKILL.md's own step 9 (a PR's terminal action is converting to DRAFT, never merging it), and the ranking-the-open-queue Routine specs' \"100% human review of any pull request merge\" policy; closes the gap where check-bash-safety.sh's shell-only 'gh pr merge' block had no equivalent for the platform-integrated MCP tool call.",
+      "rule": "Denies mcp__github__merge_pull_request unconditionally, no override -- per planning-a-branch-from-an-issue/SKILL.md (\"Do not merge or enable auto-merge; that is a separate, explicit human or CI decision\"), drafting-a-pr-to-merge/SKILL.md's own step 9 (a PR's terminal action is converting to DRAFT, never merging it), and the ranking-the-open-queue Routine specs' \"100% human review of any pull request merge\" policy; closes the gap where check-bash-safety.sh's shell-only 'gh pr merge' block had no equivalent for the platform-integrated MCP tool call. Fails closed (denies) rather than allowing the call through when jq is missing from PATH, the payload is not a JSON object, or tool_name is present but not a string, since none of those states let the hook confirm the call is not a disguised merge_pull_request.",
       "planes": ["pretooluse"],
       "local_exclusion": "Grades a Claude Code tool-call matcher only (mcp__github__merge_pull_request), an unconditional deny with no payload inspection; nothing in the working tree to reconstruct this decision against.",
       "trigger": "PreToolUse matcher mcp__github__merge_pull_request (hooks/hooks.json)",
@@ -188,7 +188,7 @@
         ".github/workflows/skill-audit-gate.yml",
         "hooks/hooks.json"
       ],
-      "rule": "Requires a PR touching skills/*/SKILL.md to disclose battle-testing-a-skill / evaluating-skill-quality verdicts or waivers under a '## Skill audit evidence' heading; the CI twin additionally requires disclosure for description changes, security-relevant skills, changed design docs, changed checker scripts, changed deterministic gates, and (issue #998) changed checker-or-gate scripts (the union of the checker-script and gate-script scopes). Gate membership is the union of the .github/scripts/gate_*.py / scan_*.py naming convention, every gates[].script path registered in this file, and this file itself; deletions and renames count, and the disclosure must be 'deterministic-gate-quality: RAN' or an explicit waiver with a reason, never 'NOT-RUN'. 'defeat-test-disclosure' shares checker-script-adversarial-review's RAN/NOT-RUN/WAIVED shape instead, disclosing that at least one test was constructed to defeat (not merely exercise the happy path of) the new or changed detection logic. The applicability computation lives in one place (gitapex_compute_skill_audit_flags.py), which the CI diff step and the local pre-push wrapper (gitapex_gate_skill_audit_disclosure.py --check-diff BASE HEAD --body-file PATH, invoked by the PreToolUse hook when .github/ is present) both call, so the full verdict including every conditional extension is reachable before a push.",
+      "rule": "Requires a PR touching skills/*/SKILL.md to disclose battle-testing-a-skill / evaluating-skill-quality verdicts or waivers under a '## Skill audit evidence' heading; the CI twin additionally requires disclosure for description changes, security-relevant skills, changed design docs, changed checker scripts, changed deterministic gates, and (issue #998) changed checker-or-gate scripts (the union of the checker-script and gate-script scopes). Gate membership is the union of the .github/scripts/gate_*.py / scan_*.py naming convention, every gates[].script path registered in this file, and this file itself; deletions and renames count, and the disclosure must be 'deterministic-gate-quality: RAN' or an explicit waiver with a reason, never 'NOT-RUN'. 'defeat-test-disclosure' shares checker-script-adversarial-review's RAN/NOT-RUN/WAIVED shape instead, disclosing that at least one test was constructed to defeat (not merely exercise the happy path of) the new or changed detection logic. The applicability computation lives in one place (gitapex_compute_skill_audit_flags.py), which the CI diff step and the local pre-push wrapper (gitapex_gate_skill_audit_disclosure.py --check-diff BASE HEAD --body-file PATH, invoked by the PreToolUse hook when .github/ is present) both call, so the full verdict including every conditional extension is reachable before a push. The PreToolUse hook itself fails closed (denies) when jq is missing from PATH, the payload or tool_input is not a JSON object, or tool_name is present but not a string -- an unparseable payload is treated as indeterminate, not as evidence the disclosure requirement does not apply.",
       "planes": ["pretooluse", "ci"],
       "local_exclusion": "Grades a PR body against the changed-skill list; the body does not exist until the PR is opened.",
       "trigger": "PreToolUse matcher mcp__github__create_pull_request and mcp__github__update_pull_request (hooks/hooks.json); .github/workflows/skill-audit-gate.yml on pull_request",
@@ -634,7 +634,7 @@
         ".github/scripts/gitapex_scan_ssot_schema.py",
         ".github/scripts/_gitapex_argv_safety.py"
       ],
-      "rule": "Validates .gitapex/ssot.json against .gitapex/ssot.schema.json (JSON Schema draft 2020-12); fails if any gates[].script path doesn't exist as a real file, or any policy_refs[] value doesn't resolve to a real policy_sources[].id. The argv-safety predicates this gate applies to local_invocation/local_stdin live in _gitapex_argv_safety.py, registered here (issue #904) so gitapex_detect_changed_gate_scripts.py selects an edit to them as a gate change requiring disclosure.",
+      "rule": "Validates .gitapex/ssot.json against .gitapex/ssot.schema.json (JSON Schema draft 2020-12); fails if any gates[].script path doesn't exist as a real file, or any policy_refs[] value doesn't resolve to a real policy_sources[].id. The argv-safety predicates this gate applies to local_invocation/local_stdin live in _gitapex_argv_safety.py, registered here (issue #904) so gitapex_detect_changed_gate_scripts.py selects an edit to them as a gate change requiring disclosure. Known gap (issue #1227): checks only that a registered entry's script path is real, not the reverse -- a real gate-shaped file (matching the naming convention gitapex_detect_changed_gate_scripts.py already applies for a different purpose) with no gates[] entry passes this check undetected.",
       "planes": ["ci", "local"],
       "local_invocation": ["uv", "run", "--frozen", "python3", ".github/scripts/gitapex_scan_ssot_schema.py"],
       "trigger": "tests/test_gitapex_scan_ssot_schema.py inside the pytest step of .github/workflows/test.yml",

hooks/check-bash-safety.sh

@@ -19,8 +19,75 @@
 
 set -euo pipefail
 
+# Issue #1208: this deny path must not itself depend on jq -- if jq is
+# missing from PATH entirely (a broken environment, not a malformed
+# payload), every jq call below would crash under `set -e` with exit 127
+# ("command not found"), an exit code Claude Code's PreToolUse contract
+# treats as non-blocking (the tool call proceeds unchecked). Checked first,
+# via a fixed, statically-escaped JSON literal (no interpolation, so no
+# JSON-escaping risk), same pattern as
+# hooks/check-pr-issue-acm-disclosure.sh's own jq-missing guard.
+if ! command -v jq >/dev/null 2>&1; then
+  printf '%s\n' "{\"hookSpecificOutput\": {\"permissionDecision\": \"deny\"}, \"systemMessage\": \"Blocked by hooks/check-bash-safety.sh: jq is not available on PATH -- cannot verify the Bash command. Failing closed.\"}" >&2
+  exit 2
+fi
+
+deny() {
+  local reason="$1"
+  # Piped via stdin (jq -Rs: raw input, slurped to one string), not
+  # `--arg` -- same ARG_MAX-avoidance reason as
+  # hooks/check-pr-issue-acm-disclosure.sh's own deny().
+  printf '%s' "$reason" | jq -Rs \
+    '{"hookSpecificOutput": {"permissionDecision": "deny"}, "systemMessage": .}' >&2
+  exit 2
+}
+
+# Non-blocking counterpart to deny(): surfaces a systemMessage but allows the
+# tool call to proceed (exit 0). Used where the underlying check is
+# documented as advisory (surfaces candidates, does not decide) rather than
+# a deterministic write/read classifier -- see the git-push handling below.
+# Found by code review (PR #1213): its only call site interpolates
+# $scan_output, the provenance scan's own report over the *entire* outgoing
+# push's commit messages and patches -- large enough on a big branch to
+# blow the OS's ARG_MAX the same way `deny()`'s own pre-hardening form did
+# (live-confirmed: `jq -n --arg msg "$BIG"` on a 3MB string exits 126,
+# "Argument list too long"). Under `set -euo pipefail` that crash aborts
+# the whole script before `exit 0`, past this function's own advisory
+# intent -- the push still proceeds either way (any non-2 exit is
+# non-blocking), but the warning itself is silently lost instead of
+# reaching the operator. Same `jq -Rs` piped-stdin fix as deny() above.
+warn() {
+  local reason="$1"
+  printf '%s' "$reason" | jq -Rs '{"systemMessage": .}'
+  exit 0
+}
+
 input=$(cat)
 
+# Issue #1208: a malformed payload (invalid JSON, or valid JSON that isn't
+# an object) would otherwise make every field-extraction jq call below exit
+# non-zero, crashing past deny() under `set -e` with an exit code Claude
+# Code's PreToolUse contract treats as non-blocking -- the same fail-open
+# class hooks/check-pr-issue-acm-disclosure.sh's own adversarial review
+# found and fixed. Validate the shape up front instead.
+if ! printf '%s' "$input" | jq -e 'if type == "object" then . else empty end' >/dev/null 2>&1; then
+  deny "Blocked by hooks/check-bash-safety.sh: the tool-call payload on stdin is not a JSON object. Failing closed."
+fi
+
+# Found by code review (PR #1213): jq -r never errors on a non-string
+# `.tool_name` (e.g. `["Bash"]`) -- it pretty-prints the JSON form across
+# multiple lines instead, which then never equals the plain "Bash" string
+# the check below compares against. That silently falls through as "not
+# our tool" (exit 0) rather than failing closed on a malformed field this
+# gate structurally depends on -- live-confirmed: an array-wrapped
+# tool_name let a `gh pr merge` command straight through this hook.
+# `.tool_name == null` covers both absent and explicit null (an absent
+# key indexes as null in jq); only a present non-string, non-null value
+# denies.
+if ! printf '%s' "$input" | jq -e '(.tool_name == null) or (.tool_name | type == "string")' >/dev/null 2>&1; then
+  deny "Blocked by hooks/check-bash-safety.sh: tool_name in the payload is not a string. Failing closed."
+fi
+
 tool_name=$(printf '%s' "$input" | jq -r '.tool_name // empty')
 
 # Defense in depth: the hooks.json matcher already restricts this hook to
@@ -29,6 +96,36 @@ if [ "$tool_name" != "Bash" ]; then
   exit 0
 fi
 
+# Issue #1208: tool_input could be a non-object (array/string/number/bool)
+# in an otherwise well-formed payload, which would crash the
+# `.tool_input.command` access below with jq's own "Cannot index X with
+# string" runtime error -- same fail-open class as the top-level check
+# above. `(.tool_input // {})` alone is not enough: jq's `//` treats JSON
+# `false` the same as `null` (both are falsy), so a `tool_input: false`
+# payload slipped past that form and crashed the extraction below anyway
+# -- found by code review (PR #1213), live-confirmed with
+# `jq -e '(.tool_input // {}) | type == "object"' <<< '{"tool_input":false}'`,
+# which wrongly reports true. Checking `.tool_input == null` directly
+# (true for both absent and explicit null, never for `false`) closes
+# that gap.
+if ! printf '%s' "$input" | jq -e '(.tool_input == null) or (.tool_input | type == "object")' >/dev/null 2>&1; then
+  deny "Blocked by hooks/check-bash-safety.sh: tool_input in the payload is not a JSON object. Failing closed."
+fi
+
+# Issue #1208 (round 4): a well-formed, object-shaped tool_input can still
+# carry `.tool_input.command` as a JSON array or object (e.g.
+# `["gh","pr","merge","1"]`) instead of a string. `jq -r` never errors on
+# this -- it pretty-prints the value across multiple lines, which splits
+# the dangerous substring across JSON punctuation (quotes, commas,
+# brackets) and breaks every `[[:space:]]`-anchored danger-pattern regex
+# below, silently letting a genuinely dangerous command through with
+# exit 0 instead of exit 2 -- found by code review (PR #1213),
+# live-confirmed against `gh pr merge`, `pip install`, and `gh api -X
+# POST` payloads wrapped as arrays. Must deny before extraction.
+if ! printf '%s' "$input" | jq -e '(.tool_input.command == null) or (.tool_input.command | type == "string")' >/dev/null 2>&1; then
+  deny "Blocked by hooks/check-bash-safety.sh: tool_input.command in the payload is not a string. Failing closed."
+fi
+
 command=$(printf '%s' "$input" | jq -r '.tool_input.command // empty')
 
 if [ -z "$command" ]; then
@@ -37,24 +134,6 @@ fi
 
 lc_command=$(printf '%s' "$command" | tr '[:upper:]' '[:lower:]')
 
-deny() {
-  local reason="$1"
-  jq -n --arg msg "$reason" \
-    '{"hookSpecificOutput": {"permissionDecision": "deny"}, "systemMessage": $msg}' >&2
-  exit 2
-}
-
-# Non-blocking counterpart to deny(): surfaces a systemMessage but allows the
-# tool call to proceed (exit 0). Used where the underlying check is
-# documented as advisory (surfaces candidates, does not decide) rather than
-# a deterministic write/read classifier -- see the git-push handling below.
-warn() {
-  local reason="$1"
-  jq -n --arg msg "$reason" \
-    '{"systemMessage": $msg}'
-  exit 0
-}
-
 # --- Shared boundary: pre-command anchor that also swallows an absolute or
 # relative path prefix -----------------------------------------------------
 # The boundary is "start of string, or any character that cannot be part of a