mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
cfe5d7b161
* 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>
323 lines
10 KiB
Python
323 lines
10 KiB
Python
"""Regression tests for Phase 7 artifact validation."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from validate_artifacts import (
|
|
flatten_claimed_count_args,
|
|
normalize_worker_id,
|
|
parse_args,
|
|
parse_claimed_counts,
|
|
validate_plan,
|
|
)
|
|
|
|
|
|
def _write_plan(tmp_path: Path) -> Path:
|
|
plan = {
|
|
"version": 1,
|
|
"run": {"output_dir": str(tmp_path)},
|
|
"workers": [
|
|
{
|
|
"worker_n": 1,
|
|
"cluster_id": "memory-safety",
|
|
"pass_prefixes": ["BOF", "UAF"],
|
|
"pass_bug_classes": ["buffer-overflow", "use-after-free"],
|
|
}
|
|
],
|
|
}
|
|
plan_path = tmp_path / "plan.json"
|
|
plan_path.write_text(json.dumps(plan), encoding="utf-8")
|
|
(tmp_path / "findings").mkdir()
|
|
(tmp_path / "findings-index.d").mkdir()
|
|
(tmp_path / "coverage").mkdir()
|
|
return plan_path
|
|
|
|
|
|
def _write_coverage(tmp_path: Path, rows: list[tuple[str, str, str]]) -> None:
|
|
body = [
|
|
"# Coverage gate - worker-1",
|
|
"",
|
|
"| Pass prefix | Bug class | Outcome |",
|
|
"|-------------|-----------|---------|",
|
|
]
|
|
body.extend(f"| {prefix} | {bug_class} | {outcome} |" for prefix, bug_class, outcome in rows)
|
|
(tmp_path / "coverage" / "worker-1.md").write_text("\n".join(body) + "\n", encoding="utf-8")
|
|
|
|
|
|
def _touch_shard(tmp_path: Path, lines: list[str] | None = None) -> None:
|
|
content = "" if lines is None else "\n".join(lines) + "\n"
|
|
(tmp_path / "findings-index.d" / "worker-1.txt").write_text(content, encoding="utf-8")
|
|
|
|
|
|
def test_cli_accepts_grouped_claimed_counts(tmp_path: Path) -> None:
|
|
args = parse_args(
|
|
[
|
|
str(tmp_path / "plan.json"),
|
|
"--claimed-count",
|
|
"worker-1=0",
|
|
"worker-2=3",
|
|
]
|
|
)
|
|
|
|
counts = parse_claimed_counts(flatten_claimed_count_args(args.claimed_count))
|
|
|
|
assert counts == {"worker-1": 0, "worker-2": 3}
|
|
|
|
|
|
def test_cli_accepts_repeated_claimed_count_flags(tmp_path: Path) -> None:
|
|
args = parse_args(
|
|
[
|
|
str(tmp_path / "plan.json"),
|
|
"--claimed-count",
|
|
"worker-1=0",
|
|
"--claimed-count",
|
|
"worker-2=3",
|
|
]
|
|
)
|
|
|
|
counts = parse_claimed_counts(flatten_claimed_count_args(args.claimed_count))
|
|
|
|
assert counts == {"worker-1": 0, "worker-2": 3}
|
|
|
|
|
|
def test_zero_finding_worker_with_cleared_coverage_passes(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
_touch_shard(tmp_path)
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "cleared (no unsafe indexing)"),
|
|
("UAF", "use-after-free", "cleared (no raw pointer frees)"),
|
|
],
|
|
)
|
|
|
|
assert validate_plan(plan_path, workers=["worker-1"], claimed_counts={"worker-1": 0}) == []
|
|
|
|
|
|
def test_filed_finding_with_shard_and_coverage_passes(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
finding = tmp_path / "findings" / "BOF-001.md"
|
|
finding.write_text("---\nid: BOF-001\n---\n", encoding="utf-8")
|
|
_touch_shard(tmp_path, [str(finding)])
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "filed: BOF-001"),
|
|
("UAF", "use-after-free", "cleared (no raw pointer frees)"),
|
|
],
|
|
)
|
|
|
|
assert validate_plan(plan_path, workers=["1"], claimed_counts={"worker-1": 1}) == []
|
|
|
|
|
|
def test_missing_shard_fails(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "cleared"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("missing shard" in error for error in errors)
|
|
|
|
|
|
def test_missing_coverage_file_fails(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
_touch_shard(tmp_path)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("missing coverage file" in error for error in errors)
|
|
|
|
|
|
def test_coverage_missing_assigned_pass_fails(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
_touch_shard(tmp_path)
|
|
_write_coverage(tmp_path, [("BOF", "buffer-overflow", "cleared")])
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("missing coverage row for UAF / use-after-free" in error for error in errors)
|
|
|
|
|
|
def test_skipped_coverage_outcome_fails(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
_touch_shard(tmp_path)
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "skipped: no obvious bugs"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("invalid coverage outcome for BOF" in error for error in errors)
|
|
|
|
|
|
def test_filed_id_absent_from_shard_or_disk_fails(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
_touch_shard(tmp_path)
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "filed: BOF-001"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("filed ID BOF-001 is absent from shard" in error for error in errors)
|
|
|
|
|
|
def test_claimed_count_mismatch_fails(tmp_path: Path) -> None:
|
|
plan_path = _write_plan(tmp_path)
|
|
finding = tmp_path / "findings" / "BOF-001.md"
|
|
finding.write_text("---\nid: BOF-001\n---\n", encoding="utf-8")
|
|
_touch_shard(tmp_path, [str(finding)])
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "filed: BOF-001"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"], claimed_counts={"worker-1": 0})
|
|
|
|
assert any("claimed 0 finding files but shard has 1 entries" in error for error in errors)
|
|
|
|
|
|
def test_shard_id_undeclared_in_coverage_fails(tmp_path: Path) -> None:
|
|
"""A finding on disk/shard that no coverage row declares (filed under a
|
|
`cleared` row instead) is a misfiling the Phase-7 gate must catch."""
|
|
plan_path = _write_plan(tmp_path)
|
|
finding = tmp_path / "findings" / "BOF-001.md"
|
|
finding.write_text("---\nid: BOF-001\n---\n", encoding="utf-8")
|
|
_touch_shard(tmp_path, [str(finding)])
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "cleared (seed returned empty)"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("shard ID BOF-001 is not declared in coverage" in error for error in errors)
|
|
|
|
|
|
def test_filed_id_prefix_mismatch_fails(tmp_path: Path) -> None:
|
|
"""A finding id filed under the wrong pass row (prefix mismatch) is rejected."""
|
|
plan_path = _write_plan(tmp_path)
|
|
finding = tmp_path / "findings" / "UAF-001.md"
|
|
finding.write_text("---\nid: UAF-001\n---\n", encoding="utf-8")
|
|
_touch_shard(tmp_path, [str(finding)])
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "filed: UAF-001"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("filed ID UAF-001 does not match pass prefix BOF" in error for error in errors)
|
|
|
|
|
|
def test_worker_absent_from_plan_fails(tmp_path: Path) -> None:
|
|
"""Validating a worker id that plan.json never declared is surfaced, not
|
|
silently passed."""
|
|
plan_path = _write_plan(tmp_path)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-9"])
|
|
|
|
assert any("worker-9: not present in" in error for error in errors)
|
|
|
|
|
|
def test_normalize_worker_id_rejects_non_numeric() -> None:
|
|
with pytest.raises(ValueError, match="invalid worker id"):
|
|
normalize_worker_id("worker-abc")
|
|
|
|
|
|
def test_frontmatter_id_mismatch_fails(tmp_path: Path) -> None:
|
|
"""A finding file whose frontmatter id disagrees with its filename (e.g.
|
|
BOF-001.md carrying id: UAF-999) must be rejected — Phase 7 otherwise keys on
|
|
the stem while the judges/SARIF trust the frontmatter."""
|
|
plan_path = _write_plan(tmp_path)
|
|
finding = tmp_path / "findings" / "BOF-001.md"
|
|
finding.write_text("---\nid: UAF-999\nbug_class: use-after-free\n---\n", encoding="utf-8")
|
|
_touch_shard(tmp_path, [str(finding)])
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "filed: BOF-001"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("frontmatter id 'UAF-999' does not match" in error for error in errors)
|
|
|
|
|
|
def test_shard_path_outside_findings_fails(tmp_path: Path) -> None:
|
|
"""A shard that lists a finding file outside output_dir/findings is rejected —
|
|
Phase 7's canonical index only scans findings/, so an outside file would pass
|
|
validation but never reach the report."""
|
|
plan_path = _write_plan(tmp_path)
|
|
outside = tmp_path / "BOF-001.md" # NOT under findings/
|
|
outside.write_text("---\nid: BOF-001\n---\n", encoding="utf-8")
|
|
_touch_shard(tmp_path, [str(outside)])
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "filed: BOF-001"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("outside findings/" in error for error in errors)
|
|
|
|
|
|
def test_malformed_frontmatter_fails(tmp_path: Path) -> None:
|
|
"""A finding whose frontmatter generate_sarif's parser rejects (a scalar key
|
|
followed by a ` - ` list item) must be a hard validation error — otherwise the
|
|
validator passes a file generate_sarif silently drops from results, so a real
|
|
finding never reaches the report."""
|
|
plan_path = _write_plan(tmp_path)
|
|
finding = tmp_path / "findings" / "BOF-001.md"
|
|
finding.write_text("---\nid: BOF-001\nseverity: HIGH\n - bogus\n---\n", encoding="utf-8")
|
|
_touch_shard(tmp_path, [str(finding)])
|
|
_write_coverage(
|
|
tmp_path,
|
|
[
|
|
("BOF", "buffer-overflow", "filed: BOF-001"),
|
|
("UAF", "use-after-free", "cleared"),
|
|
],
|
|
)
|
|
|
|
errors = validate_plan(plan_path, workers=["worker-1"])
|
|
|
|
assert any("unparseable frontmatter" in error for error in errors)
|
|
|
|
|
|
if __name__ == "__main__":
|
|
import sys
|
|
|
|
raise SystemExit(pytest.main([__file__, *sys.argv[1:]]))
|