Skip to content

gh-cli: Replace skill with hooks-only enforcement#114

Merged
dguido merged 11 commits intomainfrom
better-gh
Mar 4, 2026
Merged

gh-cli: Replace skill with hooks-only enforcement#114
dguido merged 11 commits intomainfrom
better-gh

Conversation

@Ninja3047
Copy link
Contributor

@Ninja3047 Ninja3047 commented Mar 4, 2026 •
edited
Loading

Summary

  • Adds PreToolUse hooks to intercept WebFetch and curl/wget for GitHub URLs, redirecting to gh CLI
  • Adds a PATH shim for gh that blocks gh api repos/.../contents/ (forces cloning instead)
  • Adds SessionStart hooks for shim setup and session ID persistence, and SessionEnd hook for clone cleanup
  • Removes the using-gh-cli skill — eval testing showed ~0-11% recall because Claude's system prompt already covers gh usage; the hooks enforce the critical behaviors regardless of skill triggering
  • Includes bats test coverage for hooks and shims

Test plan

  • Verify WebFetch on a github.com URL is blocked with a helpful gh suggestion
  • Verify curl/wget on GitHub API URLs is blocked with contextual gh alternatives
  • Verify gh api repos/.../contents/... is blocked by the shim with clone suggestion
  • Verify gh repo clone works normally through the shim
  • Run bats plugins/gh-cli/hooks/*.bats for hook and shim tests (130/130 pass)

🤖 Generated with Claude Code

Ninja3047 and others added 6 commits March 3, 2026 19:02
@Ninja3047 @claude
Add hooks to block gh api contents fallback and enforce session-scope…
9d61582 
…d clone paths

Add two new PreToolUse hooks:
- intercept-gh-api-contents: blocks `gh api repos/.../contents/... | base64 -d`
  and suggests cloning instead
- intercept-gh-clone-path: denies `gh repo clone` to non-session-scoped temp
  paths, enforcing the `$TMPDIR/gh-clones-$CLAUDE_SESSION_ID/` convention

Update existing fetch/curl hooks to warn against the `gh api` contents
anti-pattern in their denial messages. Update skill docs and references to
discourage base64-decoding file contents via the API.

Bump version 1.3.0 → 1.6.0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Replace regex-based PreToolUse hooks with a gh PATH shim
8cb2b6c 
Swap intercept-gh-api-contents.sh and intercept-gh-clone-path.sh
(regex-matched PreToolUse hooks) for a single shims/gh wrapper
prepended to PATH via a SessionStart hook. The shim receives
properly tokenized $@ args, eliminating the class of regex bypass
bugs identified in PR review (subshell, @base64d, compound command
gaps). All /contents/ API access is now blocked unconditionally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Fix gh shim clone bypass, exec error handling, and docs accuracy
1051f70 
- Replace hardcoded $4 clone target with arg iteration to prevent
  flag-based bypass (e.g., -u upstream owner/repo /tmp/bad-path)
- Handle exec failure on real gh binary with error message and exit 126
- Add diagnostic message when gh not found on PATH in setup-shims.sh
- Add error handling for CLAUDE_ENV_FILE write failure
- Replace blocked gh api contents example in SKILL.md Quick Start
- Fix incorrect --branch <sha> docs (git clone --branch requires a
  branch/tag name, not a commit SHA)
- Rename misleading "exits silently" test names to "exits gracefully"
- Add test for clone bypass with flags before target path

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Fix review findings: dead exec handler, inaccurate docs, add tests
954bb54 
Remove unreachable || block after exec in the gh shim (dead code
since exec replaces the process). Fix shallow clone + SHA checkout
guidance to note --depth 1 must be omitted. Qualify README passthrough
list to account for shim interceptions. Improve anti-pattern docs
and api.md Headers section. Add 4 tests: bare gh passthrough, tmp
substring path, session-scoped /var/folders/, missing shims dir.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Address PR review: shim executable check, tests, comment/doc accuracy
b7e871d 
- Validate shims/gh is executable in setup-shims.sh before writing PATH
- Add tests: API contents with query params, long-form flags before clone
  target, non-executable shim file
- Fix README "silently pass through" wording
- Clarify shim comment on flag-value scanning behavior
- Rename test for clarity on flag-preceding-target behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Remove using-gh-cli skill, keep hooks-only enforcement
4616cca 
The skill rarely triggered (~0-11% recall across eval runs) because
Claude's system prompt already includes gh CLI instructions. The hooks
(WebFetch/curl interception, gh shim, clone cleanup) enforce the
critical behaviors regardless of skill triggering, making the skill
redundant.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 Ninja3047 requested a review from dguido as a code owner March 4, 2026 00:03
Ninja3047 and others added 5 commits March 3, 2026 19:06
@Ninja3047 @claude
Fix CI: marketplace description mismatch, shellcheck, bats test
b82c7ad 
- Sync marketplace.json description with plugin.json
- Add shellcheck disable directives for bats subshell warnings
- Use absolute bash path in setup-shims test to handle PATH filtering

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Remove skill reference from gh-cli README
00a356a 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Fix curl hook circular suggestion, shim false positives, and session …
2bb7929 
…ID diagnostic

- Curl hook: add api.github.com/repos/.../contents/ branch before
  generic catch-all so it suggests clone instead of gh api (which the
  shim would then block)
- Shim: anchor /contents/ regex to ^repos/ and skip flags so jq filter
  values don't cause false positives
- Shim: emit specific diagnostic when CLAUDE_SESSION_ID is unset
  instead of a self-contradicting suggestion
- Curl hook: remove redundant gh/git early exits so compound commands
  like "curl github.com/... && gh version" are correctly denied
- Tests: add coverage for all three fixes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Ninja3047 @claude
Add error handling for shim exec failure and session ID persistence
996e90b 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dguido @claude
Fix review issues: exec dead code, leading-slash bypass, exit codes, …
e0ba908 
…fetch contents gap

- Fix exec failure handler in gh shim: disable set -e around exec so
  the error message is actually reachable, use resolved path consistently
- Handle leading-slash API endpoints (gh api /repos/.../contents/...)
  by changing regex from ^repos/ to ^/?repos/
- Exit 1 (not 0) when CLAUDE_ENV_FILE is missing in setup-shims.sh,
  since this is a runtime contract violation, not graceful degradation
- Add /contents/ branch to fetch hook for api.github.com URLs so
  WebFetch to api.github.com/repos/.../contents/ gets the clone
  suggestion instead of the generic gh api suggestion
- Add tests for all fixes (leading slash, /private/tmp session path,
  CLAUDE_ENV_FILE exit code, fetch contents endpoint)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dguido dguido merged commit 00c013d into main Mar 4, 2026
6 checks passed
@dguido dguido deleted the better-gh branch March 4, 2026 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dguido dguido Awaiting requested review from dguido dguido is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Ninja3047 @dguido