15 Commits

Author SHA1 Message Date
kz-tob 4b1b74b181 Give differential-review a trigger, and name every component in its README (#278)
* Give differential-review a trigger, and name every component in its README

differential-review's description listed what it does and never named a
situation, so it competed on capability wording alone. It now closes with
the triggers its own README already documents — reviewing a PR, commit,
or diff; checking whether a change re-introduces a fixed bug; asking what
else a change could break; finding modified code with no test.

The same plugin's README never mentioned adversarial-modeler, which is
what Phase 5 dispatches for HIGH RISK changes. Checking whether that was
isolated turned up more of it, and the sweep found three kinds of gap:

zeroize-audit's agent table was missing three of its eleven agents —
0-preflight, which gates the entire run, plus 5b-poc-validator and
5c-poc-verifier. All three appear in the phase diagram directly above the
table, which is why they read as present.

constant-time-analysis documents the ct-analyzer CLI end to end and never
says the plugin also ships a skill and a command. entry-point-analyzer
lists phrases that trigger its skill but never names the skill or its
command.

Three more READMEs describe their skill without naming it. That matters
most where the skill name is not the plugin name and a user cannot guess
it: chrome-mcp-troubleshooting and interpreting-culture-index.

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

* Fix review findings and make the README sweep a gate

The two PoC rows I added to zeroize-audit said Phase 4. The diagram three
lines above them, SKILL.md, and workflows/phase-5-poc-validation.md all
say Phase 5, steps 5a and 5b. "Wave 5a" is a label that exists nowhere.
A debugger consulting the table — the artifact this branch designates as
what runs when — would have opened phase-4-poc-generation.md and found no
validation in it. Also corrected the sentence introducing that table,
which still said 10 agents across 8 phases against 11 across 9, and the
Phase 0 diagram line, which still credited the orchestrator for a gate
the new row credits to 0-preflight.

differential-review's README claimed the agent is "dispatched", and named
it bare in a column whose other rows are namespaced. Nothing dispatches
it: the only instruction is prose in SKILL.md, and a bare subagent_type
fails at runtime. Namespaced both, and corrected the five stale line
counts in the same file — reporting.md is 369 lines, not the ~120 the
token-efficiency section budgets for.

Drop the dead `name: trailofbits:<cmd>` key from five command files.
The three newest command files carry no name: at all, #275 namespaced 22
bare invocations, and this branch documents the `/<plugin>:<cmd>` form —
so the key contradicts the docs it sits next to.

Then make the sweep repeatable. Doing this by hand three times found
eight gaps and missed two more, both of the same shape: a workflow ships
under meta.name, not its filename, so a README citing the filename never
writes the name a reader types. The validator now checks that a README
names every skill, agent, command, and workflow its plugin ships, reading
meta.name for workflows. It refuses a run that inspected zero components,
and six self-test assertions hold it to known-bad fixtures.

It found git-cleanup on its first run: ships as /git-cleanup:git-cleanup-analysis,
README cites workflows/analyze-branches.js four times and that name never.
static-analysis had the same gap for codeql-build.

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

* Fix both P2s: the gate was a substring test, and the dispatch was still bare

The README gate ran `name not in text`. That reads as thorough and could
not fail for a large share of what it counted: `draw` was satisfied by
"(draw cards instead)", `semgrep-rule` by the plugin's own name in the
install line, `burp-search` by a `scripts/burp-search.sh` path that is a
different thing, and `audit` by the prose "shared-state struct audit".

Match by kind instead. Commands and workflows are reachable only as
`/<plugin>:<name>`, so require that literal — it is the only string a
user can type. Agents are dispatched by identifier and never typed as
prose, so require an identifier-shaped mention. Skills are genuinely
referred to by bare name, so require only a delimited occurrence, which
is what stops "draws" counting as `draw`.

That surfaced seven real gaps, the four above plus insecure-defaults'
audit-pipeline workflow, mutation-testing's skill, and trailmark's
code-slice-worker. All seven fixed.

adversarial-modeler was still bare at SKILL.md:96. Line 77 was the
decision-tree mention; line 96 is the "Delegate to this agent"
instruction a model actually acts on, so the runtime failure the last
commit claimed to fix survived it. Namespaced, and it now says why.

Also from the review: a per-kind floor, since a single total stays
healthy while skill_files() — 63% of coverage — silently stops matching;
workflow_names anchored to the meta block, because a bare search takes
any earlier `name:` in a comment, and .mjs was invisible; and AGENTS.md
documents the new hard failure. Self-test 88 -> 96, each new rule with a
negative control.

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 08:54:12 -04:00
Dan Guido 9e06dc67a3 Make every documented command runnable under our own python shims (#258)
* Make every documented command runnable under our own python shims

The modern-python plugin ships PATH shims that refuse `python <script>`,
`pip install`, `python -m pip` and `uv pip install`. Twelve other plugins
in this marketplace issued exactly those forms, so installing our own
plugin broke our own skills — and CI was green throughout.

The worst case was not theoretical. c-review and rust-review both call
their Phase 4 planner as `python3 "${PLUGIN_ROOT}/scripts/build_run_plan.py"`,
so with the shim installed every run died before spawning a worker.
Verified both directions: the new form exits 0 with the shim on PATH, the
old form exits 1.

Phase 1's reading pass named 16 skills. A mechanical sweep found 96
candidate lines across 44 files, and scanning shell scripts as well as
markdown found 10 more the docs sweep had missed. That gap is the reason
the check below exists.

The fix is not one substitution. Four classes needed different treatment:

- Our own scripts become `uv run --no-project <script>`. Not bare `uv run`,
  because these execute inside the *target* repo, which may be a Python
  project that cannot resolve; verified against a broken pyproject.toml and
  against validate_artifacts.py's sibling import of generate_sarif.
- Package installs become `uv add` for a dependency, `uv tool install` for a
  CLI, `uv sync` for a project's own editable install.
- Third-party CLIs we merely document — OSS-Fuzz's infra/helper.py, yarGen —
  become `uv run --no-project python <script>`, which keeps upstream's exact
  semantics rather than handing their script an environment we manage.
- atheris's instrumented build keeps its source build, as
  `uv add --no-binary-package cbor2`. Dropping that flag would silently
  produce an uninstrumented fuzzer, which is worse than a visible failure.
  Its prose was updated to name the flag it now uses.

Two factual corrections fell out. `pip install caracal` was wrong twice
over: caracal is a Rust tool (Cargo.toml at its root), so it is now
upstream's own `cargo install --git`, not a uv equivalent that would fetch
an unrelated PyPI package. And `pip install uv` cannot bootstrap uv under
a shim that intercepts pip, so culture-index now points at the official
installer.

Thirteen lines stay as they are, each deliberately: Dockerfile `RUN` lines
and oss-fuzz's build.sh run in containers where our shims are absent;
codeql's pip calls install the *analysed* project's dependencies, and that
project is arbitrary; trailmark's dispatch skills must keep saying "Do NOT
run `pip install`"; and modern-python documents what it intercepts.

`make shell-suites` passes again as a result — exit 0 with the 1.6.0 shim,
where AGENTS.md previously recorded it as broken by variant-analysis.

The guardrail: check_python_invocations scans 698 markdown and shell files
and fails on the four refused forms, with structural exemptions for
dockerfile fences and an `allow-legacy-python: <reason>` marker that scopes
to its code block. Eleven self-test fixtures cover it, four asserting it
fires and seven asserting it stays quiet on the compliant forms. It was
mutation-tested in both languages, and it caught its own worst bug during
development: unanchored patterns first flagged `uv run --no-project python
fuzz.py`, the very form the advice recommends. Self-test goes 45 -> 56.

* Review pass: fix the atheris flow, drop a stray exemption, trim comments

Three corrections from reviewing the branch diff:

- atheris's install now opens with `uv init --bare`, without which the
  documented `uv add atheris` errors in a bare harness directory. The old
  pip form assumed an activated venv, so setup was always implicit; now
  it is one explicit line.
- ossfuzz carried an allow-legacy-python marker on a C++ build block that
  contains no python at all — yesterday's insertion matched the first of
  three "Build in build.sh" headings instead of the python one. The
  exemption now sits only on the block that needs it.
- The anti-vacuity message said "read no markdown" for a scan that also
  covers shell scripts.

The rest is weight: the new check's comment blocks, the hardcoded-path
constants' commentary, the AGENTS.md bullets and the three exemption
markers all said the same things at two to three times the length. Each
keeps its one-line why; the narratives are gone. No behavioural change —
self-test still passes 56 assertions and the full scan is unchanged.

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

* Address the review: fix where packages land, widen the check to match the shim

The review's core insight was right twice over. Several substitutions had
changed WHERE a package lands, breaking the documented next step, and the
checker enforced a narrower invariant than the shim it exists to mirror.

Where packages land:

- trailmark is imported as a library from five skills, and a `uv tool
  install` environment is not importable — the retry loop at
  trailmark/SKILL.md:47-51 would have spun forever on the exact error it
  names. The CLI install stays `uv tool install`; the import snippets now
  run under `uv run --with trailmark python -`.
- `uv add` writes to the manifest of whatever project you are standing
  in, which for sarif-parsing is the audited repo. Its scripting rows,
  ijson comment and jsonschema example now use `uv run --with <pkg>`,
  which leaves no trace. atheris keeps `uv add` deliberately: the fuzzing
  harness is the user's own project, made explicit by `uv init --bare`.
- `uv sync` leaves ct-analyzer in .venv/bin, so the README's very next
  line failed with command not found. Now `uv tool install .`, verified
  end to end: the console script lands on PATH and --help runs.
- yarGen needs pefile/lxml/yara-python, which `--no-project` had detached;
  now `uv run --with-requirements requirements.txt`.
- The cbor2 source-build preference now persists via
  `no-binary-package = ["cbor2"]` under [tool.uv] (field verified against
  uv's accepted-settings list), so a later `uv sync` cannot silently swap
  in an uninstrumented wheel.

The checker, widened to the shim's actual behaviour:

- `python3 --version` and `python3 -u foo.py` are refused by the shim but
  passed the old patterns; one live instance (constant-time-analysis
  README) proved it. Both forms are now caught.
- Every `uv pip` subcommand is refused, not just install; `-t` joins the
  allowed tool-managed flags.
- .py files are scanned too: usage strings and error messages told users
  to run refused commands from ten scripts, including the --help of the
  very planner this PR fixed. All rewritten.
- The evals/tests exemption now tests path parts relative to plugins/, so
  a checkout under a directory named tests no longer exempts every file.
- An allow-marker's scope ends at a blank line as well as a fence, so one
  marker cannot blanket a whole file; quality-assessment.md gains the
  second marker that scoping made necessary.

Also from the review: zeroize's preflight gets `which python3` back (a
helper script still needs the binary; the shim never required removing
it), the Makefile's shell-suites note no longer describes an interception
that is gone, and the cairo CI example warns that it rebuilds caracal
from source each run.

Self-test 56 -> 63; every new pattern and exemption is fixture-covered
and was mutation-probed against the real tree. Full scan: 0 findings over
773 files.

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

* Address the second review: prerequisite probe, checker parity, package placement

The review's P2 was a regression this PR introduced for a population the
first fix ignored: c-review and rust-review now require uv, and a box
with python3 but no uv would die at Phase 4 exactly the way shimmed boxes
died before. Phase 1 (Prerequisites) in both skills now probes
`command -v uv` and aborts with install guidance. zeroize-audit's
preflight already checked uv. The four converted shell suites gain the
same guard with a clear message instead of a bare 127 mid-run.

Checker parity with the shims, second pass:

- pipx and the non-install pip subcommands are refused by catch-all shim
  arms and passed the checker; both get named-subcommand patterns.
- A script named by variable or path (`python3 "$MERGE"`) has no `.py`
  token; a new pattern covers it and immediately caught one live
  instance — a codeql test stub that fakes uv itself, now carrying an
  allow-marker with its reason.
- finditer everywhere: a compliant `uv run` earlier on a line no longer
  masks a refused command later on it, which was exactly the table-cell
  case the unanchored design exists for.
- Prohibition phrases now test the text BEFORE the match, so
  "Use `pip install semgrep` instead of the tarball" is flagged while
  "Do NOT run `pip install`" stays exempt.
- The uv-pip allowance matches whole flags after the command, so
  `--target-dir` no longer counts as `--target` and a trailing `-t /tmp`
  does; `uv pip` precedes `pip` in the pattern order so its lines get
  the right advice; a pip match directly after `uv ` defers to the
  uv-pip verdict instead of double-reporting.

Package placement, continued from the same insight as round one:

- yarGen regains --no-project alongside --with-requirements, plus a cd
  into the checkout so requirements.txt resolves where it lives.
- sarif-parsing's jsonschema example no longer names a script that does
  not exist, and the table's run-forms show a concrete script.py.
- culture-index's two messages now agree and name the actual remedy
  (`uv run --project` on the scripts directory) instead of re-adding a
  dependency its pyproject already declares.
- merge_sarif's usage line gains --no-project; the generator plugin's
  install section stops prescribing a venv its own runner never uses.
- generate_poc declared requires-python >=3.9 while using `str | None`
  in a signature, a TypeError on 3.9 that uv's interpreter selection
  made reachable; now >=3.10.
- The GitLab CI example exports ~/.local/bin onto PATH, without which
  `uv tool install` warns and the next line dies command-not-found.

Self-test 63 -> 71; the masking, prohibition-direction, flag-position
and pipx cases are all fixtures, and each new pattern was probed live
against the tree (plant, error, remove, clean — 0 findings over 773
files).

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

* Address the third review: importable trailmark, honest probes, sturdier scan

The P2 was the residue of round two's own fix, applied to the siblings
but not the flagship: trailmark/SKILL.md told the model to cure an import
error with `uv tool install`, which cannot cure it — a tool env is not
importable — while forbidding every fallback. The install block now says
what each remedy is for: `uv tool install` for the CLI, `uv run --with
trailmark python -` for the snippets, and the other five library-first
docs carry the same one-line annotation next to their install command.

Empirically settled rather than taken from the review: `uv run python3
<script>` works fine under the shims — uv prepends its environment's bin
directory, so python3 resolves to a real interpreter, not the shim. The
review's claim to the contrary would have meant rewriting the Makefile
and a bats suite; a two-minute transcript said no. Also declined: a
zeroize uv-prerequisite (its preflight already lists uv and uvx; the
C/C++ `which` line now names uv too).

Real and fixed:

- ct-analyzer's availability probe ran `python3 --version` by subprocess
  — the one refused form — so under the shims it reported "Python is not
  available" on machines where it plainly is. It now probes
  sys.executable, the interpreter the analyzer itself runs under.
  Verified under the shim: probe returns True.
- The flag step-over in both script patterns handles long and
  value-taking flags (`python3 -W ignore harness.py`, `--verbose
  tool.py`), matching the shim's two-slot consumption.
- A bare `allow-legacy-python:` with no reason no longer exempts
  anything; the reason the docs demand is now enforced.
- `uv run {baseDir}/...` gets --no-project at the ten semgrep and
  culture-index call sites that round two missed, and the culture-index
  remediation strings now name that same runnable command instead of a
  --project mechanism nothing uses.
- pip gains cache/config; the pattern comment now says the subcommand
  list is deliberately a subset.
- Both filesystem scans skip .venv/node_modules-style directories, after
  a stray local .venv (left by this session's own uv probe, and invisible
  to CI) turned the path scan red.

Smaller review items: the uv-probe prose says "Phase 4 onward" rather
than a wrong phase range, run_fixtures' comment stops claiming PEP 723
headers its stdlib-only helpers do not have, the yarGen one-liners say
to run from the checkout, `uv tool install` sites note or export the
tool bin dir the way a fresh container needs, and sarif-parsing's table
column says Install / run and stops naming a file that does not exist.

Self-test 71 -> 74. Full scan: 0 findings over 773 files.

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-19 13:39:07 -04:00
Jamie Reid 5577119331 fix(semgrep-rule-creator): correct 404ing semgrep-docs links (#179)
* fix(semgrep-rule-creator): correct 404ing semgrep-docs links

The semgrep-docs repo migrated these writing-rules pages from .md to
.mdx, so the WebFetch links in SKILL.md were returning 404. Update the
five affected links to their .mdx paths (pattern-syntax was already
.mdx). All seven links now return HTTP 200.

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

* Bump version to 1.2.2 in plugin.json

* Update semgrep-rule-creator version to 1.2.2

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: ahpaleus <38883201+ahpaleus@users.noreply.github.com>
2026-06-10 15:34:58 -04:00
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
Jonathan Hefner debfb29c8e Fix allowed-tools to use spec-compliant space-delimited strings (#139)
* Fix `allowed-tools` to use spec-compliant space-delimited strings

Per the agentskills.io specification, `allowed-tools` must be a single
string of space-delimited patterns, not a YAML list. Converted all 23
SKILL.md files from the `- Item` list format to the correct
`"Item1 Item2"` string format. Also updated the frontmatter examples in
CLAUDE.md and the workflow-skill-design skill template to match.

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

* Fix remaining allowed-tools format in firebase-apk-scanner and workflow-skill-design docs

- Convert firebase-apk-scanner from comma-separated to space-delimited
- Update anti-patterns.md and tool-assignment-guide.md examples from YAML lists to space-delimited strings
- Remove unnecessary quotes from SKILL.md template placeholder

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

* Cover commands, new SKILL.md files, and fix template placeholder

Extends the previous spec-compliance fixes:

* Convert command frontmatter (commands/*.md) — per Claude Code
  docs, command files use the same frontmatter as skills, so the
  same space-delimited rule applies.
* Convert three SKILL.md files added since the original PR:
  mutation-testing, trailmark-structural, trailmark-summary.
* Fix the placeholder in the workflow-skill-design template.
  The previous "[minimum tools needed, space-delimited]" was YAML
  flow-sequence syntax, which parses as a list — the opposite of
  what the placeholder claims. Replaced with a concrete-looking
  space-delimited example plus a comment.

Zeroize-audit agent files still use `allowed-tools:` in YAML list
form. They are intentionally excluded: per the project's own docs
(workflow-skill-design references), agents declare tools with
`tools:` (not `allowed-tools:`). Fixing those requires changing
the field name as well as the format and is out of scope for this
PR.

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

* zeroize-audit agents: switch allowed-tools to tools

Subagents declare their tool allowlist via `tools:` (comma-separated),
not `allowed-tools:` — see Claude Code's subagent docs and this
repo's own designing-workflow-skills/SKILL.md:47:

> Skills use `allowed-tools:` in frontmatter. Agents use `tools:`
> in frontmatter.

Before this change, the zeroize-audit agents declared their tool list
under `allowed-tools:`, which Claude Code does not read for subagents.
The field was effectively a no-op; the spawned agents had no tool
restriction enforced.

Renames the field on all 11 agents to `tools:` and reformats the YAML
list as comma-separated to match the documented format and existing
agents elsewhere in the repo (e.g. function-analyzer.md,
spec-compliance-checker.md). Tool sets are unchanged.

Behavior change: tools now actually constrain what each spawned agent
can call. The lists are the ones the original author intended.

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

* skill-improver: convert command allowed-tools to space-delimited

The two command files in plugins/skill-improver/commands/ still used
the JSON flow-array format (`allowed-tools: ["..."]`), which the rest
of this PR converted everywhere else. Convert them to the spec-compliant
space-delimited string form for consistency.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Dan Guido <dan@trailofbits.com>
2026-04-28 19:50:30 -04:00
mschwager 8655937ac0 Add some improvements to semgrep-rule-creator (#116)
* Add some improvements to semgrep-rule-creator

* Bump second version location

* Use raw GH markdown for docs links

* Fix YAML indentation for by-side-effect in Focus example

The by-side-effect key was at column 3 (same as the list dash),
causing a YAML parse error. Moved to column 5 to be part of the
list item mapping alongside patterns.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix invalid by-side-effect value in taint options example

The pseudo-syntax (true|only) is not valid YAML and is rejected by
semgrep --validate. Use a real value with a comment documenting the
alternative.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Wrap multi-line comment example in inline code backticks

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add missing pattern-sources to taint mode anti-pattern example

Taint mode requires both pattern-sources and pattern-sinks.
Without sources, semgrep --validate rejects the rule.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix review issues: restore {baseDir} paths, improve quick-reference clarity

- Restore {baseDir} in SKILL.md and workflow.md path references per repo convention
- Normalize YAML indentation in focus-metavariable taint example
- Preserve behavioral description in by-side-effect comment
- Rename ambiguous "Operators" section to "Matching Operators"

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

---------

Co-authored-by: Maciej Domanski <maciej.domanski@trailofbits.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Dan Guido <dan@trailofbits.com>
2026-03-16 13:45:31 -04:00
Dan Guido 224f51731e ci: enforce pre-commit formatting hooks in CI (#99)
* ci: enforce pre-commit formatting hooks in CI

check-yaml, check-json, end-of-file-fixer, and trailing-whitespace
only ran locally via pre-commit. Contributors who skip pre-commit
can introduce formatting drift (see #97). Add a CI job using
pre-commit/action to enforce these four hooks on every PR.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* ci: consolidate lint jobs into single pre-commit pass

Run all pre-commit hooks (ruff, shellcheck, shfmt, check-yaml,
check-json, end-of-file-fixer, trailing-whitespace) in one job
instead of separate CI jobs per tool. Fixes the extra_args error
where pre-commit run only accepts one hook ID positionally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: fix trailing whitespace and EOF in semgrep-rule-creator

Pre-commit hooks caught two files missed by the earlier formatting
PR: trailing blank line in quick-reference.md and trailing
whitespace plus missing final newline in workflow.md.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-18 10:20:31 -07:00
ahpaleus 7934f0b5b9 Improve semgrep-rule-creator based on testing (#56)
* Improve semgrep-rule-creator skill based on testing feedback

- Add "one YAML = one rule" principle and forbid generic language matching
- Remove todoruleid/todook test annotations (only ruleid/ok allowed)
- Add Step 7: Final Run with message validation requirements
- Add constant propagation to required documentation links
- Expand workflow steps with clearer rationale and debugging commands
- Move detailed step descriptions to workflow.md for brevity

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* other changes

* Bump semgrep-rule-creator version to 1.1.0

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-30 11:53:58 -05:00
Dan Guido 45e2ed25bc Fix Windows compatibility: remove colons from command filenames (#52)
Rename 10 command files that contained `:` in their filenames, which is
invalid on Windows filesystems (reserved for drive letters).

The `trailofbits:` namespace is preserved in the frontmatter `name` field,
so slash commands like `/trailofbits:audit-context` continue to work.

Fixes #51

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-29 11:07:05 -05:00
Dan Guido a6ed466329 Add trailofbits: prefixed slash commands to 9 plugins (#49)
Add slash commands that wrap existing skills for easier invocation:
- /trailofbits:scan-apk - Firebase APK security scanner
- /trailofbits:burp-search - Burp Suite project file search
- /trailofbits:entry-points - Smart contract entry point analyzer
- /trailofbits:variants - Vulnerability variant analysis
- /trailofbits:semgrep-rule - Semgrep rule creator
- /trailofbits:diff-review - Differential security review
- /trailofbits:ct-check - Constant-time analysis
- /trailofbits:spec-compliance - Spec-to-code compliance checker
- /trailofbits:audit-context - Audit context builder

Also rename existing fix-review command to trailofbits:fix-review
for consistency with the naming convention.

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-29 00:11:55 -05:00
ahpaleus 09b6ebe643 Improve semgrep-rule-creator skill (#46)
* Improve semgrep-rule-creator skill description and terminology

Apply Claude Code Skills best practices for description writing and
consistent terminology throughout the skill.

Description improvements:
- Use third-person voice ("Creates..." not "should be used")
- Remove redundant verb list ("create", "write", "make", "build")
- Make triggers concise and clear

Terminology standardization:
- Standardize to "writing" for rule creation verbs
- Standardize to "tests" instead of "test cases"
- Standardize to "taint mode" (no hyphen) for feature,
  "taint-mode rules" (hyphenated) as adjective
- Standardize to "safe cases" for testing context
- Use "Semgrep rules" for first mention, then "rules" thereafter

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Streamline quick-reference.md and workflow.md

quick-reference.md:
- Remove advanced Rule Options section (too detailed)
- Add Troubleshooting section (moved from workflow.md)
- Update commands to use <rule-id> placeholders
- Simplify Common Pitfalls section

workflow.md:
- Remove detailed Taint Rules section (covered in quick-reference)
- Remove Complete Taint Rule example (65+ lines)
- Move Troubleshooting to quick-reference.md
- Swap order: pattern matching before taint mode
- Strengthen language ("must include", "is crucial")
- Update commands to use <rule-id> placeholders
- Add clarifying notes to section headers

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Clean up SKILL.md structure and reduce documentation links

- Remove redundant "Documentation reading is required" bullet
  (already covered by "Read documentation first")
- Convert Quick Reference to bullet list with both reference links
- Remove "Next Steps" section (links moved to Quick Reference)
- Reduce documentation links from 5 to 4:
  - Remove Testing Rules link (covered by ToB handbook)
  - Reorder ToB Testing Handbook to position 3

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Add workflow checklist and standardize terminology

- Add copy-paste progress checklist to Workflow section (best practice)
- Standardize "taint mode" terminology (remove hyphens)
- Align Step 2 naming: "Write Tests First" in both files

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Standardize terminology across all skill files

Fixes identified during deep terminology audit:
- Add "the" to checklist step 1: "Analyze the problem"
- Change "your rule" → "the rule" in workflow.md
- Standardize on "taint mode rules" (not "taint rules")
- Standardize on "CRITICAL" (not "Critical") for emphasis
- Standardize on "MUST NOT" (not "must NOT") for consistency
- Use consistent <rule-id>.<ext> placeholder (not <test-file>)
- Fix Common Pitfalls numbering (was 1,2,5 → now 1,2,3)
- Align directory structure comments between files

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Fix remaining terminology inconsistencies

- Standardize <language> placeholder (was <lang> in one place)
- Standardize <rule-id>.<ext> placeholder (was bare "file" in one place)
- Add missing "the" before "target language"
- Standardize "Verification checkpoint": format
- Use digit "2" instead of "two" for file count

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Apply additional terminology and consistency fixes

- Changed "plugin" to "skill" in references
- Standardized "taint mode" and "pattern matching" (no hyphens) in README
- Changed "rule ID" to "rule-id" for consistency with placeholders
- Expanded AST acronym in heading, use acronym in body
- Removed Pro-only taint options (control, at-exit)
- Simplified test annotation examples (my-rule-id → rule-id)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Simplify example rule ID in quick-reference

- Changed "rule-id-here" to "rule-id" for consistency

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Remove Pro-only pattern-propagators from quick-reference

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* glue together troubleshooting

* remove emoji

* remove emoji in optimization checklist

* multiline comments disallowed

* Add back Quick Start example with minimal taint rule

Restores the Quick Start section that provides a minimal working
example of a taint-mode rule, helping users get started quickly.

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>
2026-01-28 18:22:10 -05:00
ahpaleus 721e016efe Clean up semgrep-rule-creator skill (#31) 2026-01-23 11:08:24 -05:00
ahpaleus 5d766fc025 fix semgrep-rule-creator docs reference in Key Requirements (#29)
* fix semgrep-rule-creator docs reference in Key Requirements

Change "Fetch official Semgrep docs" to reference the Documentation
section below, avoiding contradiction with existing specific links.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix task completion criteria contradiction in workflow.md

- Step 7: Change "task is complete ONLY when" to "verification checkpoint"
- Step 8: Add "Task complete ONLY when: All tests pass after optimization"

This aligns workflow.md with SKILL.md which requires optimization for task
completion, resolving the contradiction where Step 7 said task was complete
but Step 8 (optimization) followed.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix documentation requirements to be unconditional

- SKILL.md: Change "Before writing complex rules" to "Before writing any rule"
- workflow.md: Reference Documentation section instead of hardcoding one URL

Documentation is now consistently required for all rules, not just complex ones.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* simplify appsec.guide link description

Change "Advanced patterns" to "Patterns" in the Trail of Bits
Testing Handbook link description.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* remove quote variants optimization guidance

Quote normalization is language-dependent and the blanket advice
could be misleading for languages where quotes have different semantics.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix documentation requirement scope in Strictness Level section

Remove "complex" qualifier so documentation is required for all rules,
not just complex ones. This aligns with the Documentation section which
says "Before creating any rule".

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* use recommended severity values instead of legacy ERROR

Update all examples to use HIGH instead of ERROR. Semgrep recommends
LOW/MEDIUM/HIGH/CRITICAL over the legacy ERROR/WARNING/INFO values.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* consolidate workflow.md steps to match SKILL.md 6-step structure

Renumber workflow.md from 8 steps to 6 steps:
- Steps 1-3: unchanged (Analyze, Test Cases, AST)
- Step 4: consolidated from old steps 4-6 (Pattern Operators, Taint Rules, Validate and Test) under "Write the Rule"
- Step 5: renamed from step 7 "Iterate Until Pass" to "Iterate Until Tests Pass"
- Step 6: renumbered from step 8 "Optimize the Rule"

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* standardize verification checkpoint wording without checkmark

Use "All tests passed" consistently instead of `✓ All tests passed`
to match workflow.md wording.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* add language caveat to quote equivalence comment

Quote normalization is language-dependent (e.g., Python treats single
and double quotes as equivalent, but C/C++ does not). Add "in Python"
qualifier to make this explicit in the example.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* remove reference to non-existent semgrep skill

The `semgrep` skill referenced in "When NOT to Use" doesn't exist.
Remove the parenthetical reference rather than pointing to the wrong
skill name.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* clarify test command requires rule directory context

workflow.md shows `cd <rule-directory>` before running tests, but
SKILL.md didn't mention this requirement. Add clarification to both
the inline example and Quick Reference table.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* improve test annotation documentation with language examples

- Use generic <comment> placeholder instead of hardcoded //
- Add examples of language-appropriate comment syntax
- Note that annotation line must contain only the annotation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* remove Common Patterns by Language section

This section duplicates information readily available in Semgrep's
official documentation and pattern registry. Removing to reduce
maintenance burden and keep the quick reference focused.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-22 10:26:22 -05:00
ahpaleus 219ee11896 Clarify documentation requirement for Semgrep rules (#6)
Updated the wording in the documentation to clarify the requirement for reading Semgrep documentation before rule creation.
2026-01-15 10:50:20 -05:00
Dan Guido 695119c312 Initial release of Trail of Bits Skills Marketplace
16 plugins for security analysis, smart contract auditing, and verification:

Smart Contract Security:
- building-secure-contracts
- entry-point-analyzer

Code Auditing:
- audit-context-building
- burpsuite-project-parser
- differential-review
- semgrep-rule-creator
- sharp-edges
- testing-handbook-skills
- variant-analysis

Verification:
- constant-time-analysis
- property-based-testing
- spec-to-code-compliance

Audit Lifecycle:
- fix-review

Reverse Engineering:
- dwarf-expert

Development:
- ask-questions-if-underspecified

Team Management:
- culture-index

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-14 15:24:03 -05:00