Commit Graph

2 Commits

Author SHA1 Message Date
Andrea Cappa (zi0Black) cfe5d7b161 Rust review plugin (#178)
* rust-review: add Rust security review plugin

Add the rust-review plugin: a comprehensive Rust security review skill
with clustered finders covering memory safety, concurrency/data races,
panic-induced DoS, FFI/cross-language boundaries, error handling,
resource handling, async runtime, and static hygiene.

Includes worker, dedup-judge, fp-judge, and planner agents, SARIF
generation with rule descriptions and regression tests, deterministic
cluster chunking, and Codex skills mapping. Versioned at 1.0.0 and
registered in the marketplace, CODEOWNERS, and root README.

* c-review: backport rust-review protocol fixes and planner chunking

Port the language-agnostic fixes made while building rust-review (which
was ported from c-review) back into c-review:

- worker/fp-judge: force findings, coverage gate, and REPORT.md to disk
  via Write instead of returning content in the reply (orchestrator
  context-bloat hardening); add a pre-complete file-existence check.
- worker: move the cache-primer block below the normal self-check and
  pre-work budget so a non-primer worker does not start under a global
  "no tool calls" rule.
- planner: add --max-passes-per-worker (default 4) with deterministic
  split_oversized_clusters chunking; skill passes the flag and documents
  the chunked-subset worker rule.
- scripts: add test_split.py and test_generate_sarif.py regression
  tests. The SARIF test caught a missing RULE_DESCRIPTIONS entry for
  uninitialized-data, now added.

Bump c-review to 1.2.0.

* c-review/rust-review: validate artifacts, index-aware SARIF, protocol cleanups

- Add validate_artifacts.py (+ tests) to both plugins to check worker
  shard, coverage, and finding files before accepting completions.
- generate_sarif.py now reads the canonical findings-index.txt when
  present, falling back to findings/*.md only if the index is absent.
- Merge the worker step-6 verification paragraphs and drop orchestrator
  -internal Phase 7 / plan.json jargon in favor of worker-facing stakes.
- Tighten uninitialized-read-finder guidance: primitive integers still
  require initialization.

* rust-review/c-review: per-cluster max_passes_per_worker override

Lets output-heavy clusters declare a smaller manifest-level
max_passes_per_worker so each expensive pass group gets its own worker,
validated by a single shared cluster_max_passes_per_worker helper and
honored by split_oversized_clusters via an explicit override (0 is
rejected rather than silently falling back to the global cap). rust-review
opts in concurrency-locking and recursion-dos; c-review ports the
capability for parity. validate_artifacts now accepts grouped or repeated
--claimed-count values.

* rust-review: broaden bug-class coverage with capability-gated clusters

Add layout-safety, input-os-safety, and info-disclosure clusters behind new
has_packed_repr / has_fs_io capability gates so packed-repr, path, and
pointer-exposure passes only run where they apply, and gate unsafe-only passes
behind has_unsafe to cut noise on safe crates.

Extend existing clusters with new bug classes: RefCell double-borrow panics,
unflushed BufWriter, string-comparison bypasses, serialize_struct mismatches,
nondeterminism, in-collection key mutation, and destructor-skip cleanup leaks.

Fix detector regexes that missed or over-matched real Rust (packed-field
borrows, RefCell try_borrow_mut, HashMap substrings, path push, packed inner
attrs, fs/path probes) and add a regression test pinning them to snippets.

* fix dedup

* safety-net check for REPORT.md

* on-disk data -> shards reconciliation

* on-disk data -> shards reconciliation - v2

* ls -> glob

* memory-safety gate

* path validation

* fix numbers/counting

* rm PACKEDREF from FFI cluster prompt, it is in layout-safety

* fix unsafe-boundary count

* minor fixes for prompts

* do not filter unknown-severity findings, just mark them as such

* fix minor behavior changes in worker

* Correctness:
- generate_sarif: clamp startLine >=1 (`:0` produced schema-invalid SARIF)
- generate_sarif: don't drop a judged survivor with blank severity
- dedup-judge: Tier-2 carry-forward so a primary can't be demoted/orphaned
- dedup-judge: crash-recovery unions shards with findings/*.md (empty-shard trap)

Robustness:
- generate_sarif: skip frontmatter-less files; add originalUriBaseIds

Contracts:
- SKILL: gate dedup-judge before fp-judge (prevent concurrent-spawn race)
- worker: verbatim coverage cells; sub_prompt_paths omitted-not-empty;
  skip_subclasses reserved; Codebase comma format

* improve prompts regexes, add missing deconflictions

* prompt factual fixes

* fix dozen of small prompt inconsistencies and add missing sections

* more prompt fixes, fix retry guard in SKILL, small fixes in agents

* dozen more small fixes

* final regex fixes

* fixes from rust to c-review

* agents cannot use write tool for reports (strange cc limitation) - bypass via bash

* spawnings agents is capped to 20 - explicit handling for that

* fix glob -> read (glob is blocked for agents that has also bash)

* fix regex patterns to work with grep

* soften output requirements - they were violated anyway

* consolidated clusters are no longer chunked — one worker owns the whole cluster, builds its shared Phase-A inventory once, and runs every phase

* fix judge finding counting and low-severity guidance

* fix metadata

* small fix for skipped findings

* Carry forward guard for `also_known_as` bucket

* Gracefully handle parse_frontmatter error

* Extend has_ffi coverage

* Broader gate for has_concurrency

* Update FFI-safe layout regex to support C, C+packed, and C+u32 in unsafe-boundary and dyn-trait-ffi-finder prompts

* Small refine of regex patterns

* Improve regex patterns for recursive type detection to include Mutex and RwLock

* rm global .codex/rust-review

* backport fixes to c-review

* merge changes

* Backport SARIF merge-survivor + malformed-frontmatter guards to c-review, mark missing locations, fix prompt-regex test extractor, and harden planner/validator scripts across both review plugins

* fix pytest

* fix global gitignore, adds / and ruff_cache

* small fixes from pr-review

* small fixes from pr-review - 2

* fix copilot finding

---------

Co-authored-by: GrosQuildu <e2.8a.95@gmail.com>
2026-06-30 11:14:46 -04:00
Paweł Płatek 870955f1af C review (#156)
* init c review

* lsp

* agents -> prompts

* wip

* add windows, update judges

* improve

* upgrade

* size update

* rm toon format, improve workflow, cluster agents/prompts by issue type, improve prompt cache

* improve general workflow, fix bugs

* sarif via script, cluster manifest

* fix bugs

* fix workflow2

* workflow updates

* more fixes

* more fixes

* improvements

* update readme

* update codeowners

* update codeowners2

* fix small inconsistencies

* Address review feedback on c-review plugin

Critical:
- Move SKILL.md into named skill subdirectory (plugins/c-review/skills/c-review/)
  so plugin discovery and the Codex validator find it; add .codex/skills/c-review
  symlink.
- Convert allowed-tools in SKILL.md from YAML list to space-delimited string
  (spec compliance per #139).
- Fix parse_scalar in generate_sarif.py to respect quoted strings when splitting
  inline lists; ["a,b", c] no longer corrupts to ['"a', 'b"', 'c'].
- Fix location_parts trailing-colon handling so 'src/foo.c:' resolves to
  ('src/foo.c', 1) instead of keeping the colon in the filename.

Important:
- Convert agent tools: from YAML list to comma-separated string in worker,
  dedup-judge, fp-judge.
- Refactor build_run_plan.py main() (131 → 77 lines) by extracting
  _validate_run_inputs / _render_workers / _print_summary helpers.
- Fix ty possibly-missing-attribute warning by typing workers list explicitly.
- Add PEP 723 inline metadata + plugins/c-review/scripts/pyproject.toml.
- Rewrite SKILL.md description as scenario-based; add When to Use /
  When NOT to Use section headers.
- Add Usage section to README.
- Resolve Tier 2 contradiction in dedup-judge: unparseable/multi findings
  now skip Tier 2 and go straight to Tier 3.
- Standardize placeholder convention in fp-judge ({var} not <var>).
- Fix "Widthness Overflows" → "Width Truncation" in integer-overflow-finder.
- Standardize "Bug Patterns to Find" heading in signal-handler and
  thread-safety finders.
- Replace ls -1 glob in worker shard-write with find for shell portability.
- Bump version 1.1.0 → 1.1.1 in plugin.json + marketplace.json.

Verification: codex validator passes (73 plugin skills); ruff + ty clean;
main() 77 lines (limit 100); SARIF generator runtime tests pass; end-to-end
build_run_plan.py produces all 11 clusters with cache primer.

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

* Address claude[bot] review feedback on c-review

- Phase 1 is_posix/is_windows probes in SKILL.md now include C++ extensions
  (.cpp, .cxx, .cc, .hpp, .hh) in their --include lists. A pure C++ POSIX
  daemon was silently dropping ~17 POSIX-gated passes plus all is_windows
  clusters because pthread.h / windows.h includes only in .cpp/.hpp files
  failed both --include='*.c' --include='*.h' filters.
- generate_sarif.py informationUri points at trailofbits/skills (the actual
  repo) instead of trailofbits/tob-skills (404).
- CODEOWNERS: add @dguido co-owner to /plugins/c-review/ and move it to the
  top of the c* alphabetical group (- < l < o < u under ASCII collation).
- README.md: move c-review row after burpsuite-project-parser (b < c).
- Bump version 1.1.1 → 1.1.2.

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

---------

Co-authored-by: Dan Guido <dan@trailofbits.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-03 15:59:55 -04:00