diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 2250a506..77f45c45 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -115,7 +115,7 @@ }, { "name": "gh-cli", - "version": "1.4.1", + "version": "1.4.2", "description": "Intercepts GitHub URL fetches and curl/wget commands, redirecting to the authenticated gh CLI.", "author": { "name": "William Tan", diff --git a/plugins/gh-cli/.claude-plugin/plugin.json b/plugins/gh-cli/.claude-plugin/plugin.json index 74c51078..c6041bc6 100644 --- a/plugins/gh-cli/.claude-plugin/plugin.json +++ b/plugins/gh-cli/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "gh-cli", - "version": "1.4.1", + "version": "1.4.2", "description": "Intercepts GitHub URL fetches and curl/wget commands, redirecting to the authenticated gh CLI.", "author": { "name": "William Tan", diff --git a/plugins/gh-cli/hooks/shims/gh b/plugins/gh-cli/hooks/shims/gh index 70aaeffd..28e4fc2b 100755 --- a/plugins/gh-cli/hooks/shims/gh +++ b/plugins/gh-cli/hooks/shims/gh @@ -65,5 +65,38 @@ for dir in "${path_entries[@]}"; do fi done +# Fallback for PATH-stripping environments. Some parents fork with PATH +# stripped to internal/system dirs only — most notably Homebrew's superenv +# during `brew install` with HOMEBREW_VERIFY_ATTESTATIONS=true (issue #145): +# the shim wins ensure_executable!'s ORIGINAL_PATHS resolution, then runs +# under a stripped child PATH that no longer contains the Homebrew prefix, +# so the PATH walk above can't find the real gh. +# +# Check well-known install paths directly. The Cellar opt symlinks +# (.../opt/gh/bin/gh) are stable across upgrades; the prefix bin entries +# cover manual installs and non-Homebrew system packages. Tests can +# override the list via GH_SHIM_FALLBACKS (colon-separated absolute paths). +if [[ -n "${GH_SHIM_FALLBACKS:-}" ]]; then + IFS=: read -ra fallbacks <<<"$GH_SHIM_FALLBACKS" +else + fallbacks=( + /opt/homebrew/opt/gh/bin/gh + /opt/homebrew/bin/gh + /usr/local/opt/gh/bin/gh + /usr/local/bin/gh + /home/linuxbrew/.linuxbrew/opt/gh/bin/gh + /home/linuxbrew/.linuxbrew/bin/gh + /usr/bin/gh + ) +fi +for fallback in "${fallbacks[@]}"; do + if [[ -x "$fallback" ]]; then + set +e + exec "$fallback" "$@" + echo "ERROR: gh-cli shim: failed to exec $fallback (exit $?)" >&2 + exit 126 + fi +done + echo "ERROR: real gh binary not found on PATH" >&2 exit 127 diff --git a/plugins/gh-cli/hooks/shims/gh-shim.bats b/plugins/gh-cli/hooks/shims/gh-shim.bats index 06911764..44eee72a 100644 --- a/plugins/gh-cli/hooks/shims/gh-shim.bats +++ b/plugins/gh-cli/hooks/shims/gh-shim.bats @@ -23,6 +23,7 @@ setup() { teardown() { rm -rf "$FAKE_GH_DIR" + [[ -n "${STRIP_PATH_ESSENTIALS_DIR:-}" ]] && rm -rf "$STRIP_PATH_ESSENTIALS_DIR" export PATH="$ORIG_PATH" } @@ -342,5 +343,103 @@ assert_blocked() { path_without_gh="${path_without_gh:+${path_without_gh}:}$dir" done export PATH="${SHIM_DIR}:${path_without_gh}" + # Neutralize the Homebrew-superenv fallback list so this test exercises the + # PATH-walk branch in isolation (CI runners often have /usr/bin/gh installed, + # which the default fallback list would pick up). + export GH_SHIM_FALLBACKS="/__gh_shim_test_no_fallback__/gh" run -127 bash -c '"$0" "$@" 2>&1' "$SHIM" pr list } + +# ============================================================================= +# Homebrew superenv fallback (issue #145) +# ============================================================================= +# +# The shim wins ensure_executable!'s ORIGINAL_PATHS resolution inside +# Homebrew, then forks under a stripped PATH that no longer contains the +# Homebrew prefix. Without a fallback the PATH walk fails and `brew install` +# breaks under HOMEBREW_VERIFY_ATTESTATIONS=true. These tests pin that the +# fallback fires only when the PATH walk finds nothing. + +# Helper: rebuild PATH with the shim dir and only directories that don't +# contain a gh binary, so the PATH walk is forced into the fallback path. +# On CI runners gh is apt-installed into /usr/bin alongside bash/rm/mktemp, +# so naively skipping every dir that contains gh strips core utilities. We +# symlink the essentials we need (run_shim invokes `bash`, teardown uses +# `rm`, several tests call `mktemp`) into a clean dir and keep that on PATH. +strip_path_of_gh() { + local path_without_gh="" + local IFS=: + for dir in $ORIG_PATH; do + [[ "$dir" == "$FAKE_GH_DIR" ]] && continue + [[ -x "$dir/gh" ]] && continue + path_without_gh="${path_without_gh:+${path_without_gh}:}$dir" + done + STRIP_PATH_ESSENTIALS_DIR="$(mktemp -d)" + local cmd cmd_path + for cmd in bash sh rm mktemp printf chmod cat env; do + if cmd_path="$(command -v "$cmd" 2>/dev/null)"; then + ln -sf "$cmd_path" "${STRIP_PATH_ESSENTIALS_DIR}/${cmd}" + fi + done + export PATH="${SHIM_DIR}:${STRIP_PATH_ESSENTIALS_DIR}${path_without_gh:+:${path_without_gh}}" +} + +@test "shim: falls back to GH_SHIM_FALLBACKS when PATH walk finds nothing" { + strip_path_of_gh + export GH_SHIM_FALLBACKS="$FAKE_GH_DIR/gh" + run_shim pr list + assert_passthrough +} + +@test "shim: prefers PATH-found gh over fallback" { + # FAKE_GH_DIR is on PATH (setup) and emits REAL_GH_CALLED. If the shim + # incorrectly preferred the fallback, output would carry MARKER_FALLBACK + # instead. + local marker_dir + marker_dir="$(mktemp -d)" + printf '#!/usr/bin/env bash\necho "MARKER_FALLBACK"\n' >"$marker_dir/gh" + chmod +x "$marker_dir/gh" + export GH_SHIM_FALLBACKS="$marker_dir/gh" + run_shim pr list + assert_passthrough + if [[ "$output" == *"MARKER_FALLBACK"* ]]; then + echo "Unexpected fallback invocation:" + echo "$output" + return 1 + fi + rm -rf "$marker_dir" +} + +@test "shim: tries fallbacks in order, skipping non-executable entries" { + strip_path_of_gh + local nonexec_dir + nonexec_dir="$(mktemp -d)" + : >"$nonexec_dir/gh" # exists but not executable + export GH_SHIM_FALLBACKS="$nonexec_dir/gh:$FAKE_GH_DIR/gh" + run_shim issue view 1 + assert_passthrough + rm -rf "$nonexec_dir" +} + +@test "shim: exits 127 when neither PATH nor fallback have gh" { + strip_path_of_gh + export GH_SHIM_FALLBACKS="/__gh_shim_test_no_fallback__/gh" + run -127 bash -c '"$0" "$@" 2>&1' "$SHIM" pr list +} + +@test "shim: anti-pattern checks fire before fallback (api contents)" { + # Even when gh is reachable only via fallback, the deny rules run first, + # exit 1, and never invoke the fallback binary. + strip_path_of_gh + export GH_SHIM_FALLBACKS="$FAKE_GH_DIR/gh" + run_shim api repos/owner/repo/contents/file.txt + assert_blocked +} + +@test "shim: anti-pattern checks fire before fallback (clone temp path)" { + strip_path_of_gh + export GH_SHIM_FALLBACKS="$FAKE_GH_DIR/gh" + unset CLAUDE_SESSION_ID 2>/dev/null || true + run_shim repo clone owner/repo /tmp/repos/repo + assert_blocked +}