mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
agentic-actions-cli-detection
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 kz-tob
|
43d7af8064 |
gh-cli: intercept GitHub fetches from MCP fetch tools (#274)
* gh-cli: intercept GitHub fetches from MCP fetch tools The fetch hook only matched WebFetch, so a GitHub URL fetched through an MCP fetch tool bypassed it entirely. Match `mcp__.*[Ff]etch` as well, which covers Exa's `web_fetch_exa` and equivalents from other MCP servers. A matcher alone is not enough: WebFetch passes a single `url`, while MCP fetch tools pass a `urls` array and batch several pages into one call. Read both shapes. A tool call is atomic, so one GitHub URL anywhere in a batch denies the whole call, and each offending URL is labeled with its own suggestion. Single-URL calls keep the message they had. Split the URL classification into suggest_api, suggest_raw, and suggest_github_com behind a suggest_for_url dispatcher so it can run per URL in a loop, and collapse eight verbatim copies of the clone hint into one helper. Behavior is unchanged; blob and tree merge into one branch because they emitted identical text. The plugin README's interception table now also lists the pull, issues, releases, and gist patterns the hook already handled but never documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * gh-cli: address review on the MCP fetch interceptor Test the matcher. Nothing exercised the regex that decides whether the hook runs at all, so a matcher firing on nothing would still pass every other suite here. matcher.bats reads it out of hooks.json and checks it against real tool names, and refuses to run against an empty matcher, which would make every match assertion pass vacuously. Widen the matcher to fetch, scrape, crawl, and extract. Firecrawl's scrape and Tavily's extract retrieve a URL like any fetch tool but carry no "fetch" in the name, so they bypassed the hook while the README promised "any MCP fetch tool". The README now names what matches. Check a string-valued `urls`. A server declaring `urls: string | string[]` sent a bare string, `arrays` dropped it, and the fetch went out unauthenticated. Listing the field twice keeps it under both shapes. `prompt` is still not scanned, so a prompt mentioning a GitHub URL does not false-deny. Give the closing note its own line when several URLs are denied; it previously trailed only the last entry. Guard assert_suggestion_starts_with against an empty reason and an empty prefix, either of which let it pass while inspecting nothing. Document the api.github.com contents, releases, and actions rows. The generic `gh api` row was actively wrong for /contents/, pointing readers at the anti-pattern the shim exists to block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
William Tan
|
00c013dba7 |
gh-cli: Replace skill with hooks-only enforcement (#114)
* Add hooks to block gh api contents fallback and enforce session-scoped 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> * Replace regex-based PreToolUse hooks with a gh PATH shim 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> * Fix gh shim clone bypass, exec error handling, and docs accuracy - 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> * Fix review findings: dead exec handler, inaccurate docs, add tests 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> * Address PR review: shim executable check, tests, comment/doc accuracy - 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> * Remove using-gh-cli skill, keep hooks-only enforcement 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> * Fix CI: marketplace description mismatch, shellcheck, bats test - 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> * Remove skill reference from gh-cli README Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix curl hook circular suggestion, shim false positives, and session 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> * Add error handling for shim exec failure and session ID persistence Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix review issues: exec dead code, leading-slash bypass, exit codes, 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> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
William Tan
|
d356b6b5bb |
improve gh hook to prefer cloning repo instead of using api to fetch files (#88)
* improve gh hook to prefer cloning repo instead of using api to fetch files This should make finding stuff way more efficient Repos are saved to a per claude session directory and cleaned up at the end of the session * fix formatting * fix: resolve code review findings for PR #88 P1 (CI-blocking): - Fix shell formatting: convert tabs to 2-space indent per project shfmt config (shfmt -i 2 -ci). The prior "fix formatting" commit used default shfmt settings (tabs) instead of project settings. P2 (important): - Add ${TMPDIR:-/tmp} fallback in suggestion strings across both intercept hooks. Without the fallback, $TMPDIR expands to empty on Linux systems where it's unset, producing paths at filesystem root. - Add session_id format validation in persist-session-id.sh and cleanup-clones.sh to prevent shell injection via malformed input. Session IDs are now required to match ^[a-zA-Z0-9_-]+$. - Add stderr warning in persist-session-id.sh when CLAUDE_ENV_FILE is unset, since the entire clone workflow silently breaks without it (CLAUDE_SESSION_ID never gets defined). - Remove 2>/dev/null from rm -rf in cleanup-clones.sh and add directory existence check. The stderr suppression hid real failures like permission errors. Add justification comment for rm -rf per project conventions. - Add missing test for tree/ URL pattern in fetch hook. - Add bats tests for persist-session-id.sh and cleanup-clones.sh covering: invalid JSON, empty session_id, shell metacharacter rejection, CLAUDE_ENV_FILE behavior, actual directory cleanup. - Add test for gist.github.com in curl hook. - Add test verifying clone suggestions include CLAUDE_SESSION_ID and --depth 1. P3 (nice to have): - Add test for github.com/settings allow path (site pages). - Strip query strings and fragment identifiers from URLs before parsing in fetch hook to handle edge cases like github.com/owner/repo?tab=issues. Dismissed findings: - P3 #8 (version jump 1.0.0→1.3.0): Author's choice, not a bug. - P4 #13 (missing gh silently disables plugin): Intentional design — a broken hook should not block user operations. - P4 #14 (loose test assertions): Addressed partially with new CLAUDE_SESSION_ID/--depth 1 assertion; full template validation would add test fragility without proportional value. All 75 bats tests pass. shellcheck and shfmt clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Dan Guido <dan@trailofbits.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
William Tan
|
dca4d846ad |
Add hook for gh-cli (#78)
* Add hook for gh-cli This should prevent claude code from wasting cycles trying to fetch from Github which will not work 90% of the time. * Add guidance for claude for updating CODEOWNERS * update codeowners * fix: resolve code review findings for PR #78 - Add actions and gist endpoint handling to curl hook (parity with fetch hook) - Skip non-repo github.com paths (settings, notifications, login) in fetch hook - Move gh-cli entry to correct alphabetical position in marketplace.json - Fix stale "prefer-gh-cli" name in test_helper.bash comment - Remove unused Grep from SKILL.md allowed-tools Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: improve agent self-correction suggestions for PR #78 Add specific pattern matching for github.com PR, issue, release-download, and tree URLs so the deny message suggests the exact correct gh command (e.g., `gh pr view 78 --repo owner/repo`) instead of the generic `gh repo view owner/repo` catch-all. Also add releases/download pattern to the curl hook. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dan Guido <dan@trailofbits.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |