Skip to content

chore(fuzz): add cargo-fuzz suite and untrusted-input coverage gate - #279

Open
crowecawcaw wants to merge 4 commits into
OpenJobDescription:mainfrom
crowecawcaw:add-fuzzing
Open

chore(fuzz): add cargo-fuzz suite and untrusted-input coverage gate#279
crowecawcaw wants to merge 4 commits into
OpenJobDescription:mainfrom
crowecawcaw:add-fuzzing

Conversation

@crowecawcaw

@crowecawcaw crowecawcaw commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a coverage-guided libFuzzer suite (via cargo-fuzz) over the crates that parse, decode, or evaluate untrusted input, a CI workflow that runs it on PRs and merges to main, and a hard "all untrusted inputs are fuzzed" gate.

Why: the reports/*-quality-evaluation-report.md cycle keeps re-discovering the same class of defect by hand — arithmetic overflow and char-boundary panics reachable from attacker-controlled expression text, templates, and manifests. Encoding that class as executable checks stops it from being re-litigated every review cycle.

Fuzz targets (12)

Each asserts the same invariant: any input returns Ok or Err — never panics, aborts, or hangs.

Target Entry point
expr_parse ParsedExpression::new / with_profile
expr_evaluate parse + evaluate under both POSIX and Windows path formats
range_expr RangeExpr::from_str + len/get/iter
int_range_new IntRange::new with arbitrary i64 triples
range_expr_slice RangeExpr::slice with arbitrary i64 indices
expr_type_parse ExprType::parse
format_string FormatString::new + resolve_string_with
copy_symbol_value copy_symbol_value dotted-name walk
model_decode document_string_to_object + decode_{job,environment,}_template
model_create_job preprocess_job_parameters + create_job (instantiation — one layer past decode)
snapshot_decode decode_manifest + Manifest::validate
snapshot_ops diff/compose/partition/subtree/filter over decoded manifests

The fuzz crate lives outside the root workspace (its own empty [workspace] table), so the stable build/test/clippy/MSRV jobs are untouched. It builds only under a pinned nightly with AddressSanitizer, overflow-checks, and debug-assertions on.

Coverage gate (the hard check)

scripts/check_fuzz_coverage.py + fuzz/fuzz_coverage.toml, wired into the Compliance CI job — pure source+manifest analysis on stable Python, no Rust build, no nightly. Every public function taking a &str / &[u8] / serde_json::Value / &Path parameter must be classified in the registry as either fuzzed-by-a-target or explicitly not-untrusted-with-a-reason. The check enforces:

  • no orphan targets (a fuzz_targets/*.rs missing from the registry),
  • no phantom targets (a registry entry with no target file),
  • no dangling classifications (a classified fn that was renamed/removed/made private), and
  • the ratchet — a newly-added untrusted-input entry point fails CI until a human triages it.

108 input-shaped public functions are classified today (69 fuzzed, 39 not-untrusted). All four failure modes were verified to fail CI.

CI / running

  • One job, short smoke run (FUZZ_SECONDS=10 per target) on PRs and merges to main. scripts/run_fuzz.sh derives the target list from cargo fuzz list, so new targets are picked up automatically — nothing to maintain in YAML.
  • Locally: scripts/run_fuzz.sh (all targets), FUZZ_SECONDS=300 scripts/run_fuzz.sh (deeper), or name specific targets. See fuzz/README.md.

Seeds

Curated, per-file, one per structurally-distinct input shape plus deliberate edge cases (multibyte strings, i64 boundaries, symbol-table-prefixed expressions) — 189 files total, in the typical range for committed fuzz seeds. .gitignore excludes libFuzzer's SHA-named discovered inputs so they can't leak into the tree.

Bug found & fixed

path_starts_with under the Windows path format sliced a path by the base string's byte length, panicking when that offset fell inside a multibyte char (expr report finding X8) — surfaced once expr_evaluate exercised the Windows path format. Fixed with a byte-slice comparison (equivalent for ASCII case-folding, boundary-safe), plus a regression test.

Rebase note

Earlier revisions of this branch also fixed RangeExpr/IntRange integer overflows the fuzzer found. Those are now superseded by upstream #276 (bounded range values), so this branch is rebased onto it and drops the redundant range changes — keeping the fuzz targets (int_range_new, range_expr_slice) that guard the area.

All openjd-expr tests pass; cargo clippy -D warnings is clean workspace-wide.

🤖 Draft — opened for review, not yet marked ready.

Comment thread crates/openjd-expr/src/range_expr.rs Outdated
@crowecawcaw
crowecawcaw force-pushed the add-fuzzing branch 3 times, most recently from bc3fef2 to 7113bad Compare July 24, 2026 17:08
Comment thread crates/openjd-expr/src/range_expr.rs Outdated
Comment thread fuzz/seeds/expr_evaluate/expr_0
Comment thread scripts/check_fuzz_coverage.py
Comment thread .github/workflows/fuzz.yml
@crowecawcaw crowecawcaw changed the title feat(fuzz): add cargo-fuzz suite + CI, fix RangeExpr overflow panics chore: add cargo-fuzz suite + CI, fix RangeExpr overflow panics Jul 24, 2026
@crowecawcaw crowecawcaw changed the title chore: add cargo-fuzz suite + CI, fix RangeExpr overflow panics chore(fuzz): add cargo-fuzz suite and untrusted-input coverage gate Jul 24, 2026
Comment thread scripts/run_fuzz.sh Outdated
Comment thread fuzz/fuzz_targets/model_create_job.rs Outdated
@crowecawcaw
crowecawcaw marked this pull request as ready for review July 24, 2026 18:30
@crowecawcaw
crowecawcaw requested a review from a team as a code owner July 24, 2026 18:30
Comment thread scripts/check_fuzz_coverage.py Outdated
@crowecawcaw
crowecawcaw enabled auto-merge (squash) July 24, 2026 20:19
Adds a coverage-guided libFuzzer suite (via cargo-fuzz) over the crates that
parse, decode, or evaluate untrusted input, a CI workflow that runs it on PRs
and merges to main, and a hard "all untrusted inputs are fuzzed" gate.

Motivation: the quality-evaluation reports kept re-discovering the same class
of defect by hand — arithmetic overflow and char-boundary panics reachable from
attacker-controlled expression text, templates, and manifests. Encoding that
class as executable checks stops it from being re-litigated every review cycle.

Twelve fuzz targets, each asserting "any input returns Ok or Err — never panic,
abort, or hang":
  expr_parse, expr_evaluate (both POSIX and Windows path formats),
  range_expr, int_range_new, range_expr_slice, expr_type_parse,
  format_string, copy_symbol_value, model_decode, model_create_job,
  snapshot_decode, snapshot_ops.

The fuzz crate is outside the root workspace (its own empty [workspace] table)
so the stable build/test/clippy/MSRV jobs are untouched; it builds only under a
pinned nightly with AddressSanitizer, overflow-checks, and debug-assertions on.
Curated seed corpora (mined from the crates' own tests, sample templates, and
i64/multibyte edge cases) live in fuzz/seeds/ and are committed. scripts/run_fuzz.sh
derives the target list from `cargo fuzz list`, so new targets are picked up by
CI automatically with nothing to maintain.

Fuzz-coverage gate (scripts/check_fuzz_coverage.py + fuzz/fuzz_coverage.toml,
runs on stable in the Compliance CI job — no nightly, no fuzz build):
every public function taking a &str / &[u8] / serde_json::Value / &Path
parameter must be classified in the registry as either fuzzed-by-a-target or
explicitly not-untrusted-with-a-reason. The check enforces no orphan/phantom
targets, no dangling classifications, and — the ratchet — that a newly-added
untrusted-input entry point fails CI until triaged. 108 input-shaped public
functions are classified today (69 fuzzed, 39 not-untrusted).

Also fixes one bug the suite found, with a regression test: path_starts_with
under the Windows path format sliced a path by the base string's byte length,
panicking when that offset fell inside a multibyte char (expr report finding
X8), surfaced once expr_evaluate exercised the Windows path format. Fixed with a
byte-slice comparison (equivalent for ASCII case-folding, boundary-safe).

Note: earlier revisions of this branch also fixed RangeExpr/IntRange integer
overflows the fuzzer found; those are now superseded by upstream OpenJobDescription#276 (bounded
range values), so this branch rebases onto that and drops the redundant range
changes, keeping the fuzz targets that guard the area.

All openjd-expr tests pass; cargo clippy -D warnings is clean workspace-wide.

Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
Comment thread .github/workflows/fuzz.yml
Comment thread scripts/check_fuzz_coverage.py Outdated
… AGENTS.md and specs

Per AGENTS.md report-driven development, strike through the report items this
PR's cargo-fuzz suite resolves:

- expr report: recommendation 20 (fuzz harness), the §4 structural gap, the
  §5 test_fuzz.py parity gap, and the X8 path_starts_with char-boundary panic
  (item 16 / recommendation 5), fixed in this PR with a regression test.
- model report: gap 5 / recommendation 12 (fuzz testing for parser
  robustness), covered by the model_decode and model_create_job targets.

Also document the new surface where developers will look for it:

- AGENTS.md: quick-reference entries for scripts/run_fuzz.sh and
  scripts/check_fuzz_coverage.py, the Fuzz workflow and the Compliance job's
  fuzz-coverage gate in the CI table, and a note that new untrusted-input
  public fns must be classified in fuzz/fuzz_coverage.toml.
- specs/architecture.md: fuzz/ in the project layout and a Key Design
  Decisions entry describing the suite and the coverage gate.

The sessions report's directive-parsing fuzz note stays open — the suite
does not cover openjd-sessions.

Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
The private-module regex in check_fuzz_coverage.py used a `(pub\s+)?`
prefix, which cannot match a restricted-visibility declaration like
`pub(crate) mod foo;` — there is no whitespace after `pub`, so the
optional group matches empty and `mod` is then tried against `pub`,
failing the line entirely. Such modules were therefore never added to
`private_tops`, and their crate-internal `pub fn`s were scanned as part
of the untrusted public surface, contradicting the function's own
documented contract.

Match an optional `(...)` visibility restriction explicitly and treat
any restriction as private, since `pub(crate)`, `pub(super)`, and
`pub(in path)` are all narrower than `pub`.

This drops openjd-expr's `pub(crate) mod edit_distance` from the scanned
surface (111 -> 109 input-shaped public functions), so its two now-dangling
[[fuzzed]] entries are removed from fuzz_coverage.toml and replaced with a
note recording that `expr_evaluate` still exercises them transitively.

The previous behavior was fail-safe — over-inclusion never let a real
entry point escape the ratchet — so this is a contract/accuracy fix, not
a soundness fix.

Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>

# Conflicts:
#	reports/expr-quality-evaluation-report.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants