mirror of
https://github.com/trailofbits/skills
synced 2026-06-21 14:12:00 +00:00
feat/opencode-support
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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> |
||
|
Paweł Płatek
|
f4b2a7218f |
better ci validations, fix marketplace error (#85)
* better ci validations, fix marketplace error * fix lints * fix lint script * fix lint script2 * fix ruff errors * fix abs path regex * fix shellcheck lint * fix: resolve code review findings for PR #85 - Remove dead CHANGED_FILES code path (unreachable in CI) - Add version and description consistency checks between plugin.json and marketplace.json - Eliminate duplicate plugin.json reads via parse_plugin_json() - Fix bats test discovery for filenames with spaces (print0/xargs -0) - Sync marketplace.json with plugin.json for 5 pre-existing mismatches (second-opinion version, ask-questions/burpsuite/fix-review/insecure-defaults descriptions) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: gh-cli bats tests fail when gh is in /usr/bin The _no_gh test helpers used PATH=/usr/bin:/bin to exclude gh, but on Ubuntu CI runners gh is installed at /usr/bin/gh. Fix by creating a temp directory with symlinks to only jq and bash. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: improve plugin descriptions for better skill triggering - ask-questions-if-underspecified: restore trigger context ("asking questions") while keeping invocation constraint - burpsuite-project-parser: drop filler "directly from the command line", use outcome-oriented "for security analysis" - insecure-defaults: restore specific scenarios (hardcoded credentials, fallback secrets, weak auth defaults) for better matching 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> |