7 Commits

Author SHA1 Message Date
Lixin2026 d5fe2e6a78 feat(codex): add UI metadata for skills (#175)
* feat(codex): add skill UI metadata

* Use official Trail of Bits logo

* fix: resolve code review findings for PR #175

Codex silently drops the icons as authored: its loader
(codex-rs/core-skills resolve_asset_path) requires icon paths
containing '..' to resolve under <plugin_root>/assets/, and the
repo-root .codex/assets location fails that containment check.
Verified empirically via codex app-server plugin/read: every
iconSmall/iconLarge came back null; only brand_color applied.

P1 fixed:
- Vendor trail-of-bits-mark.svg into plugins/<name>/assets/ for
  all 38 plugins with skills and point every openai.yaml at
  ../../assets/trail-of-bits-mark.svg (the supported plugin-level
  shared asset pattern). Icons now resolve for marketplace
  installs too, since nothing escapes the plugin root.
- Drop the .codex/ additions: .codex/skills/gh-cli/agents/
  openai.yaml resolved nowhere (.codex/skills is not a Codex
  discovery root) and PR #173 removes the whole .codex/ tree

P2 fixed:
- Patch-bump all 38 touched plugins in plugin.json and
  marketplace.json so installed clients pick up the metadata

Verified:
- Static check replicating Codex's resolution algorithm: all 73
  yaml files resolve under their plugin assets/ and exist
- Live codex app-server probe: 71/72 loadable skills report
  resolved iconSmall/iconLarge and brand_color #D83A34
  (claude-in-chrome-troubleshooting fails to load on main due to
  a pre-existing 64-char qualified-name limit, fixed by #173's
  rename; zeroize-audit's manifest mcpServers object is likewise
  a pre-existing Codex incompatibility fixed by #173)
- validate_codex_skills.py, validate_plugin_metadata.py, prek all
  pass

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(codex): use skill-local icon assets

---------

Co-authored-by: Dan Guido <dan@trailofbits.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 12:28:41 -04:00
William Tan 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>
2026-06-05 12:18:09 -04:00
David Maynor 38793f6757 fix(gh-cli): exit 0 when CLAUDE_ENV_FILE is unset (#135)
* fix(gh-cli): exit 0 when CLAUDE_ENV_FILE is unset in setup-shims.sh

When CLAUDE_ENV_FILE is not set by the runtime, setup-shims.sh exits 1,
causing a SessionStart hook error in Claude Code. This is a graceful
degradation case (shims simply won't be installed), not a fatal error.

The gh-not-found guard on line 10 already exits 0 for the same reason.
This change makes the CLAUDE_ENV_FILE guard consistent.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(gh-cli): update bats test for exit 0 and bump version to 1.4.1

The bats test for the CLAUDE_ENV_FILE-unset guard still expected exit 1.
Update it to match the new graceful-degradation behavior, and bump the
plugin version so clients pick up the fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: David Maynor <dmaynor@Davids-MacBook-Air-2.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Dan Guido <dan@trailofbits.com>
2026-04-28 19:28:59 -04:00
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>
2026-03-03 17:28:44 -08:00
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>
2026-02-12 21:16:05 -05:00
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>
2026-02-12 10:28:33 -05:00
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>
2026-02-11 19:46:50 -05:00