fix(claude-ops): use type -P for jq prereq check to avoid wrapper shadowing

[kyle-sexton]

Summary

telemetry-upsert.sh and machine-behavior.sh each define a jq() wrapper function (to strip CRLF from native-Windows jq output) before checking for the jq binary with command -v jq. Since command -v resolves shell functions ahead of PATH executables, the prerequisite check passed even with no jq binary installed — defeating the guard and letting both scripts fail open instead of the documented exit 4.

Fix: use type -P jq (and, in the telemetry-upsert.sh loop, type -P for both gh/jq), which searches PATH executables only and ignores functions/aliases/builtins — so the guard now catches a missing binary regardless of definition order.

Closes #812

Sites fixed

• plugins/claude-ops/skills/lanes/scripts/telemetry-upsert.sh (prereq loop, was L73-78)
• plugins/claude-ops/skills/lanes/scripts/machine-behavior.sh (prereq check, was L63-66)

A third file with a jq() wrapper, plugins/claude-ops/skills/plugins/scripts/fleet-state.sh, was checked and does not have this bug — its command -v jq check already runs before the wrapper is defined, so it wasn't touched.

Empirical verification

Before (bug reproduced against the actual scripts, jq absent from PATH):

$ PATH=<no-jq> bash ./machine-behavior.sh --repo .
== MACHINE-BEHAVIOR ==
...
  unavailable (installed_plugins.json is not valid JSON: ...)
EXIT CODE: 0        # should be 4

After (same PATH):

$ PATH=<no-jq> bash ./machine-behavior.sh --repo .
ERROR: jq not found (required)
EXIT CODE: 4

$ PATH=<no-jq> bash ./telemetry-upsert.sh --repo o/r --issue 1 --marker "lane:x" --body-file /dev/null
ERROR: jq not found (required)
EXIT CODE: 4

Regression tests were also confirmed to actually catch the bug: reverting the one-line telemetry-upsert.sh fix and re-running telemetry-upsert.test.sh fails the new case (exit 3 with a dirname: command not found error, since the falsely-passed guard let the script proceed further before breaking) — proving the test isn't vacuously passing.

Test plan

• Added a regression case to machine-behavior.test.sh and telemetry-upsert.test.sh: run the script with a PATH containing the gh/git stub(s) but no jq binary anywhere, assert exit 4 and the "jq not found" message.
• Confirmed both new cases fail against the pre-fix code and pass against the fix.
• Full existing suites pass: machine-behavior.test.sh (26/26), telemetry-upsert.test.sh (38/38).
• shellcheck clean on all four modified files.

Related

• plugins/claude-ops/skills/plugins/scripts/fleet-state.sh — carries the same jq() CRLF-stripping wrapper, but orders its command -v jq guard before the wrapper is defined, so it is not vulnerable to this shadowing and is deliberately left unchanged by this PR.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[claude]

Claude finished @kyle-sexton's task in 2m 3s —— View job

---

PR Review — fix(claude-ops): use type -P for jq prereq check

• Gather context and read changed files
• Review machine-behavior.sh fix
• Review telemetry-upsert.sh fix
• Review test additions
• Check portability and edge cases
• Post final review

---

Verdict: LGTM ✓ — Correct diagnosis, minimal fix, well-tested, no issues found.

---

What changed and why it matters

Both scripts define a jq() shell function (for CRLF stripping on native-Windows jq) before the prerequisite guard runs. Because command -v resolves shell functions before PATH executables, the guard always found "jq" — the function — even with no binary installed, and proceeded to fail unpredictably rather than exiting with the documented code 4.

type -P (bash built-in, PATH executables only, ignores functions/aliases/builtins) is the right tool here. Both scripts are #!/usr/bin/env bash with set -uo pipefail, so bash-only behaviour is appropriate and shellcheck won't flag it.

---

Fix review

machine-behavior.sh (L63)

# before
command -v jq >/dev/null 2>&1 || { err "jq not found (required)"; exit 4; }
# after
type -P jq >/dev/null 2>&1  || { err "jq not found (required)"; exit 4; }

Correct. The jq() wrapper is defined at L57 (before the guard), making command -v jq unreliable. type -P closes that gap. The wrapper itself correctly calls command jq internally, so no infinite-recursion concern.

telemetry-upsert.sh (L74)

# before
for bin in gh jq; do command -v "$bin" >/dev/null 2>&1 || { … }; done
# after
for bin in gh jq; do type -P  "$bin" >/dev/null 2>&1 || { … }; done

Correct. The loop checks both gh and jq — gh has no wrapper function in this script so command -v gh would have worked fine either way, but unifying both checks under type -P is cleaner and more consistent.

---

Scope check — nothing was missed

The PR's assessment of out-of-scope files is accurate:

File	Has jq() wrapper?	Guard before or after wrapper?	Action
machine-behavior.sh	Yes (L57)	Guard was after	Fixed ✓
telemetry-upsert.sh	Yes (L69)	Guard was after	Fixed ✓
fleet-state.sh	Yes (L81)	Guard is before (L71)	Correctly left alone ✓
lane-launcher.sh	No	N/A — no wrapper defined	command -v jq is safe here ✓

machine-behavior.sh also has a command -v gh at L129 inside gh_identity() — no gh() wrapper exists in this script, so command -v is correct there and was rightly left alone.

---

Test review

Both new regression cases (machine-behavior.test.sh L201–205, telemetry-upsert.test.sh L310–315) use the same sound pattern:

BASH_BIN="$(command -v bash)"   # capture bash path before restricting PATH
out="$(PATH="$STUB_BIN" "$BASH_BIN" "$SCRIPT" … 2>&1)"

Resolving $BASH_BIN first (while the test's own PATH is still intact) then restricting the child's PATH to stub-only is the correct approach for platform-portable PATH isolation — it sidesteps the shebang lookup problem and guarantees no jq binary leaks through. The assertions check both exit code 4 and the "jq not found" message string, which together pin down the exact failure path.

The PR description's empirical verification (reverting the fix and re-running the test to confirm failure) is a good anti-vacuousness check and worth noting as a model for future regression tests.

---

No issues found. The fix is correct, minimal, and the tests are meaningful.

[claude]

Claude finished @kyle-sexton's task in 1m 14s —— View job

---

Security Review

• Gather context and understand the PR
• Read PR diff and changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found. This PR is a net security improvement.

What was reviewed

The four changed files:

• machine-behavior.sh: one-line guard change
• telemetry-upsert.sh: one-line guard change (inside the for bin in gh jq loop)
• machine-behavior.test.sh and telemetry-upsert.test.sh: regression test additions

Analysis

The fix closes a fail-open path. Before this PR, both scripts defined a jq() shell wrapper before running the prerequisite guard. Because command -v resolves shell functions ahead of PATH executables, the guard silently passed even with no real jq binary in PATH — the scripts would then proceed and fail later in unpredictable ways rather than exiting cleanly with code 4. type -P searches only PATH executables, ignoring functions/aliases/builtins, so the guard now correctly catches an absent binary regardless of definition order.

No new attack surface introduced. type -P is a bash builtin — it doesn't fork a subprocess, can't be shadowed by a PATH entry, and has identical semantics to command -v when the binary is present.

Unchanged command -v gh usage at machine-behavior.sh L129 is correct. Neither script defines a gh() wrapper, so there is no shadowing risk there. This was correctly left untouched.

Test additions are safe. BASH_BIN="$(command -v bash)" resolves the real bash binary while the test still has its normal PATH (before PATH is restricted to $STUB_BIN). The restricted-PATH subprocess is a well-understood testing pattern. $BODY, $SAFE_DIR, and $REPO are all test-controlled variables with no external input.

No injection surfaces, no credential exposure, no permission changes introduced by this PR.

[kyle-sexton]

Tower merge-sit: merge-drive lane dark ~5.5h (rate-limit hold since ~10:36Z); tower sitting merges per stall protocol. Gate-verified live: CLEAN, 0 unresolved threads. This was generated by AI (control tower, session 6).