mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
main
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c199e0cc7d |
Narrow the modern-python shims to the commands uv run replaces (#255)
* Narrow the modern-python shims to the commands uv run replaces Closes #207. The shims sit on PATH, so they intercept every subprocess any tool spawns, not just what Claude types. Two of the intercepted invocations were not package management at all, and blocking them broke real tooling. `uv pip` now passes through when it carries --project, --directory or --target. Those say a tool is building an environment it owns, where `uv add` is not the available advice: prek installs every hook with `uv pip install --project / --directory <cache>`, so the refusal made `git commit` fail in any repo whose hooks need a Python environment. A bare `uv pip install requests` is still refused. `python -c`, `python -m <module>` and `python -` now reach the real interpreter. None of them resolves a script against a project's dependencies, which is what `uv run` exists to do, and `uv run python3 -` is not a drop-in replacement inside a pipeline. `python -m pip` stays intercepted, as do bare `python` and `python script.py`. Passing anything through is new for the python shim, which previously ended every branch in exit 1, so it gains the same skip-my-own-dir PATH walk the uv shim already had. That walk now uses parameter expansion rather than basename, because the one case where it must report failure is a PATH holding nothing but the shim, where shelling out to coreutils fails first with a confusing error. Verified by A/B on the two symptoms #207 reports, running each suite against the old shim and the new one: - zeroize-audit's rust-regression smoke test: FAILED at line 72 before, "Rust regression smoke checks passed." after. - prek hook installation from a cold cache: refused before, "check json Passed" after. bats goes from 19 cases to 38. Five python cases inverted rather than being deleted: the ones asserting that -c and -m are refused now assert they run. AGENTS.md's note on `make shell-suites` is corrected rather than removed — the #207 interceptions are gone, but the target still fails because variant-analysis invokes `python3 <script>.py`, which the shim intercepts by design. That one belongs to variant-analysis. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Decide on the mode selector, not on argument position Two gaps in the narrowing, both from review. `uv pip install --help` documents `-t, --target <TARGET>`, so the short form has to be exempt alongside the long one. Without it the same tool-managed install was allowed or refused depending on spelling. The python shim read only $1 to find the mode selector, so `python -u -c 'code'` was refused while `python -c 'code'` ran, even though they are the same invocation. It now steps over interpreter flags to find the selector, giving `-W`, `-X` and `--check-hash-based-pycs` the two slots they take. `-u -m pip` is still refused, and so is `-u script.py`: a script path is what `uv run` replaces regardless of what precedes it. bats 38 -> 43. Both #207 regressions re-verified after the restructure: zeroize-audit's smoke test passes and prek installs hooks from a cold cache. Not fixed here, deliberately: `uv --no-progress pip install requests` still slips past the refusal, because the subcommand check reads $1 as well. Parsing that correctly means knowing which uv global flags take a value, and getting it wrong would refuse a command that works today. The failure mode is a missed nudge rather than a breakage — the real uv runs and behaves correctly — so it does not belong in a change whose purpose is to refuse less. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3b316e6ac7 |
fix(modern-python): suggest exact uv run python so shim advice works outside projects (#196)
* fix(modern-python): suggest exact `uv run python` so the advice works outside projects The python/python3 shim suggested `uv run $cmd ...`, echoing back whichever name was invoked. For `python3` that advice is self-defeating on machines with no uv-managed interpreters: uv resolves the `python3` command through an ordinary PATH lookup, which hits the shim again and fails with the same suggestion. uv special-cases the exact command name `python` (uv >= 0.4.0) and executes its resolved interpreter directly, so always suggesting `uv run python ...` works everywhere. Reproduced on stock Debian + uv 0.11.27 (apt python3, zero managed pythons, no project): `uv run python3 script.py` fails via the shim while `uv run python script.py` succeeds, across script/-c/-m/REPL forms. Reported in #195. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(modern-python): satisfy shellcheck SC2016 in new bats assertions Escaped backticks in double quotes instead of literal backticks in single quotes, which shellcheck flags as a possible unintended non-expansion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(modern-python): requote shim suggestions and carry all arguments through Review findings on #196: the -m branch interpolated only the module name, so `python -m http.server 8000` suggested a command missing the port, and `${*}` flattened arguments without quoting, so `python -c 'print(1+1)'` suggested a command that is a bash syntax error if run verbatim (plus a trailing space inside the backticks for bare invocations). Both branches now build the suggestion from %q-requoted arguments, with regression tests for each case. Also consolidates the exact-`python` rationale into a single canonical copy in the shim's header comment; README, setup-shims.sh, and the bats file now point there instead of paraphrasing it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
f09e5c729a |
Remove legacy codex compatiblity scripts/shims. (#173)
* Remove legacy codex compatiblity scripts/shims. Codex supports claude plugins so this shouldn't be necessary. Add a script to test the plugin loadablility in both claude and codex * fix: resolve code review findings for PR #173 Review findings addressed (4 reviewers: pr-review-toolkit agents, Codex gpt-5.3-codex, direct diff review): P2 fixed: - Bump versions for the 5 substantively changed plugins in both plugin.json and marketplace.json (gh-cli 1.5.0 new skill, claude-in-chrome-troubleshooting 1.1.0 skill rename, modern-python 1.5.1 / skill-improver 1.0.3 hooks change, zeroize-audit 0.1.1 MCP config relocation) so clients pick up the changes - README Codex install: replace unpasteable /plugins slash-command block with verified CLI syntax (codex plugin marketplace add) - check_claude_loadability: parse_json_output now fails fast with command context on empty CLI output instead of returning None - check_codex_loadability: surface skipped RPC error messages in timeout failures instead of a bare TimeoutError P3 fixed: - Both checkers: error out when marketplace.json lists no plugins instead of passing vacuously Dismissed: - @latest CLI installs in validate.yml: deliberate; the check validates against the clients users actually run - select.select portability: CI-only script on ubuntu-latest - Divergent mcpServers validation between checkers: intentional; the Codex checker enforces the repo's .mcp.json convention Verified: ruff, prek, validate_plugin_metadata.py, and both loadability checks pass end-to-end (39 plugins, 74 skills, 2 MCP servers load in Claude Code and Codex) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dan Guido <dan@trailofbits.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
dc1bc277f0 |
Also hook pipx (#90)
* Also hook pipx * fix: resolve code review findings for PR #90 - Add upgrade-all case to pipx shim (pipx upgrade-all → uv tool upgrade --all) - Add ensurepath case to pipx shim (pipx ensurepath → uv tool update-shell) - Update setup-shims.sh comment to mention pipx - Add README rows for upgrade-all and ensurepath mappings - Add tests for upgrade-all, ensurepath, and unknown subcommand - Rename generic ensurepath test to unknown subcommand test P2 fixed: missing upgrade-all subcommand mapping P3 fixed: stale setup-shims.sh comment, ensurepath deserves specific mapping P3 dismissed: -ne 0 vs -eq 1 in tests (matches existing convention) P4 noted: pipefail no-op, Gemini false positive on bats syntax All 37 bats tests pass. Shellcheck 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> |
||
|
|
994fe9687b |
use PATH shim override technique instead of hooking every pre tool usage (#89)
* use PATH shim override technique instead of hooking every pre tool usage * remove unecessary check * fix: resolve code review findings for PR #89 P1 fixes: - setup-shims.sh: Add CLAUDE_ENV_FILE guard to prevent opaque 'unbound variable' crash when env var is unset (matches gh-cli plugin pattern) P2 fixes: - setup-shims.sh: Guard shims_dir resolution failure to prevent empty PATH prefix on partial installs - shims/uv: Fix shfmt formatting (here-string spacing) - shims/uv: Resolve PATH entries before comparison to prevent exec loop with symlinked/unnormalized paths - shims/uv: Use ${PATH:-} to handle unset PATH defensively - setup-shims.bats: Add test for unset CLAUDE_ENV_FILE - uv-shim.bats: Add test for 'real uv not found' error path P3 dismissed: - CI linting gap for extensionless shim scripts: accepted as-is since shims must masquerade as real binaries (no .sh extension); manual review covers these files - Unset PATH in uv shim: addressed by ${PATH:-} fix above All 27 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> |
||
|
|
e9dec942d1 |
Add BATS test suite for intercept-legacy-python hook (#43)
* Add BATS test suite for intercept-legacy-python hook - Add 64 tests covering all code paths: early exits, allow cases (uv run, diagnostics, search tools), deny cases (python/pip execution, uv pip, piped commands, compound commands) - Add test_helper.bash with run_hook, assert_allow, assert_deny, and assert_suggestion_contains helpers - Add bats job to CI workflow - Fix hook to properly detect python execution in piped commands like `python script.py | grep foo` and `find . | xargs python` Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix CI failures in hook test suite - Suppress jq stderr in run_hook_no_uv to avoid "Broken pipe" error when hook script exits early due to missing uv - Fix shfmt formatting: remove trailing backslash, single space before inline comments Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> |
||
|
|
4aeaa737f6 |
Add hook to modern-python to intercept python3 and pip calls (#40)
* Add hook to modern-python to intercept python3 and pip calls Uses tree-sitter-bash parse the AST for more robust hooking * Fix ruff line-length lint in intercept-legacy-python hook Run ruff format to properly break long lines into multi-line tuples. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Rewrite legacy Python hook in shell for performance Replace tree-sitter-based Python implementation with a shell script using jq and regex patterns. This eliminates the overhead of spawning Python and loading tree-sitter on every Bash command. - Use jq for JSON parsing (fast, commonly available) - Regex-based pattern matching for python/pip commands - Allow uv run commands to pass through - Context-aware suggestions based on subcommand - Passes shellcheck validation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Allow diagnostic commands and simplify error message - Skip `which`, `type`, `whereis`, `command -v` for python/pip - Simplify denial message (Claude already knows uv commands) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Allow search tools through legacy Python hook Add allowlist for grep, rg, ag, ack, find to prevent false positives when searching for "python" or "pip" as text patterns. Also normalize character class ordering for consistency with other patterns. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix shfmt formatting (2-space indent) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add hook development guidance to CLAUDE.md Document design lessons from PR #40's Python-to-shell rewrite: - Performance: prefer shell+jq over Python with dependencies - Simplicity: accept false positives when performance gain is worth it - False positive awareness: diagnostic commands, search tools, filenames - Explicit tradeoff documentation in PR descriptions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Refine CLAUDE.md formatting for consistent density Remove section breaks (horizontal rules) between major sections and condense the Hooks section from verbose multi-paragraph format into a unified list while preserving all guidance. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |