ENG-4691: rustyroute cargo-fuzz target + OSS-Fuzz submission - #14
ENG-4691: rustyroute cargo-fuzz target + OSS-Fuzz submission#14jimbofreedman wants to merge 15 commits into
Conversation
Self-contained fuzz/ package (its own [workspace] root, excluded from the root build now and if a root workspace is ever added). Two libfuzzer targets: - load_archive: feeds arbitrary bytes to Graph::from_bytes. Promotes the transient buffer to 'static then reclaims it after the (dropped) Graph so RSS stays flat across libFuzzer's in-process iterations — a plain Box::leak would trip a false-positive OOM with the 411KB seed driving -max_len up. - route_inputs: fixed 50km graph, fuzzes from/to/blocked composition via an arbitrary-derived input. Committed seed: one valid 100km archive (411,248 B, RRG1+v1). Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Runs cargo fuzz build (both targets, catching a route_inputs compile break) then cargo fuzz run load_archive -- -max_total_time=60 per PR. Mirrors ci.yaml conventions; nightly + cargo-fuzz is the sole deviation (sanitizers require nightly). Deep continuous fuzzing runs on OSS-Fuzz. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Stages projects/rustyroute/{project.yaml,Dockerfile,build.sh} for a
google/oss-fuzz PR plus oss-fuzz/README.md runbook. The submission itself is
external (public repo + 1-3 week approval + co-maintainer email TODO) and is
tracked as a follow-up on ENG-4691.
Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
String-assertion tests for fuzz.yaml shape and the fuzz crate structure (mirrors tests/ci_workflow.rs), a seed header check, and the load-bearing acceptance guard: root cargo metadata must not list rustyroute-fuzz as a member (spawned cargo, per tests/downstream_consumer_smoke.rs). Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
OSS-Fuzz ingests seed corpora only from $OUT/<target>_seed_corpus.zip; loose .rkyv files copied into $OUT were ignored, so the committed 100km seed never reached the fuzzer. Package it into load_archive_seed_corpus.zip instead. (Peer review P3.) Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Codifies acceptance criterion 4 (OSS-Fuzz project files present + valid): project.yaml required keys, Dockerfile base image, build.sh builds every declared target + packages the seed as <target>_seed_corpus.zip + is committed executable (git mode 100755). Keeps the staging files in sync with fuzz/Cargo.toml so the eventual google/oss-fuzz submission passes check_build. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
On GitHub runners cargo-fuzz defaulted to x86_64-unknown-linux-musl, and
ASan is incompatible with musl's statically linked libc
("sanitizer is incompatible with statically linked libc"). Pin
--target x86_64-unknown-linux-gnu on cargo fuzz build/run and declare the
target on the toolchain. Update the workflow test to match.
Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
1fc5b96 to
fd907a4
Compare
There was a problem hiding this comment.
Pull request overview
Adds a first-class fuzzing setup to continuously harden rustyroute’s graph loading and routing APIs against malformed inputs, with both a nightly GitHub Actions quick-pass and an OSS-Fuzz submission staging area. This fits into the repo’s existing CI/testing approach by introducing workflow-structure tests that prevent the fuzzing/OSS-Fuzz scaffolding from drifting silently.
Changes:
- Introduces a standalone
fuzz/cargo-fuzz package with two libFuzzer targets (load_archive,route_inputs) plus a committed seed corpus. - Adds a nightly GitHub Actions workflow (
fuzz.yaml) to build both fuzz targets and run a boundedload_archivefuzz pass per PR. - Stages OSS-Fuzz project files under
oss-fuzz/and adds integration tests to lock the intended repository/workflow structure.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/fuzz_workflow.rs |
Adds integration tests asserting the fuzz workflow shape, fuzz-crate structure, seed header validity, OSS-Fuzz file presence, and that root cargo metadata excludes the fuzz package. |
oss-fuzz/README.md |
Documents how to submit the staged OSS-Fuzz project directory to google/oss-fuzz and how to validate locally. |
oss-fuzz/projects/rustyroute/project.yaml |
Adds OSS-Fuzz project configuration (language, contacts, engines, sanitizers). |
oss-fuzz/projects/rustyroute/Dockerfile |
Adds an OSS-Fuzz base-builder Rust Dockerfile that clones this repo and stages build.sh. |
oss-fuzz/projects/rustyroute/build.sh |
Builds fuzz targets in OSS-Fuzz and copies fuzzer binaries + seed corpus zip into $OUT. |
fuzz/fuzz_targets/route_inputs.rs |
Adds a structured-input fuzz target exercising Graph::route on a fixed baked-in graph. |
fuzz/fuzz_targets/load_archive.rs |
Adds a bytes-input fuzz target exercising Graph::from_bytes, with buffer lifetime management to avoid RSS growth across iterations. |
fuzz/Cargo.toml |
Defines the standalone cargo-fuzz package/workspace and declares both fuzz binaries plus dependencies. |
fuzz/.gitignore |
Ignores fuzz artifacts/corpora while allowing committed seed_* files. |
CHANGELOG.md |
Documents the new fuzzing + OSS-Fuzz scaffolding addition. |
.github/workflows/fuzz.yaml |
Adds the nightly cargo-fuzz workflow (build both targets, run load_archive with a time bound). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace the derived Arbitrary on RouteInput with a hand-written impl that reads the blocked-id count from a single u8 (<=255). Blocked-set size does not affect Graph::route correctness (unknown ids are no-op filters), but an unbounded Vec<u32> grows with fuzzer input size and can cause slow-unit/OOM false positives under OSS-Fuzz. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
oss-fuzz/README.md:28
- This step refers to “replace the TODO placeholder” in
auto_ccs, butproject.yamldoesn’t include a placeholder entry—only a TODO comment. Reword to “add” the co-maintainer address underauto_ccs.
1. Confirm the co-maintainer email and replace the `TODO` placeholder in
`project.yaml` `auto_ccs`.
Copilot review: the &'static [u8] promoted from the raw pointer stayed in scope across Box::from_raw, so a live shared reference existed at the moment the allocation was freed. Even though it was never read again, that ordering is not something to rely on under optimization. Confine the borrow (and the Graph holding it) to an inner scope so both are provably dead before the free. No behavior change: same copy/promote/reclaim lifecycle, same flat RSS. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot review: RouteInput switched to a hand-written Arbitrary impl in 2a9599a, so nothing in the fuzz crate uses #[derive(Arbitrary)] any more (only derive(Debug), which is std). Dropping the feature removes the derive-macro dependency from fuzz and OSS-Fuzz builds. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot review: the runbook told the reader to 'replace the TODO placeholder' in project.yaml auto_ccs, but auto_ccs has one real entry followed by a TODO(ENG-4691) comment -- there is no placeholder entry to replace. Reword the prerequisite and step 1 to say the second address must be added as a new entry and the comment deleted. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot review, two robustness fixes to the OSS-Fuzz staging files: build.sh ran 'cargo fuzz build -O' with no --target but copied from fuzz/target/x86_64-unknown-linux-gnu/release. cargo-fuzz defaults to the host triple, which is not reliably gnu -- it resolved to musl on GitHub's runners, which is what fd907a4 had to pin for the CI workflow. Same failure mode here would break the cp and fail OSS-Fuzz check_build, and ASan (project.yaml sanitizers: address) needs a dynamically linked libc anyway. Pin the triple in one variable and derive the output dir from it so the two can't disagree. Dockerfile cloned into a relative 'rustyroute' dir and set a relative WORKDIR, while build.sh does 'cd $SRC/rustyroute' -- correct only while the base image's default working dir is $SRC. Spell $SRC explicitly in both places. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
fuzz/fuzz_targets/load_archive.rs:19
Graph::from_bytestakes&'static [u8]becauseGraphstores that slice internally (seesrc/loader.rs:179-187). Creating a&'static [u8]from a freshly allocated buffer and then freeing it at the end of the iteration violates the'staticcontract and can be UB even if the reference is scoped (the compiler may assume'staticdata remains valid for the program lifetime). This can make the fuzz target produce misleading results.
To keep RSS flat without unsound lifetime extension, reuse a single process-lifetime buffer with fixed capacity (never reallocated) and copy/truncate each input into it before calling from_bytes.
fuzz_target!(|data: &[u8]| {
// `Graph::from_bytes` takes `&'static [u8]`. Promote a copy of the
// transient fuzz buffer to `'static`, then reclaim it after use so RSS
// stays flat across libFuzzer's many in-process iterations. A plain
// `Box::leak` would accumulate one copy per iteration and — with the
// large committed seed driving `-max_len` up — climb toward
// `-rss_limit_mb` and trip a false-positive OOM crash.
let ptr = Box::into_raw(data.to_vec().into_boxed_slice());
OSS-Fuzz requires two maintainer addresses in project.yaml auto_ccs. Adds the confirmed co-maintainer and drops the TODO(ENG-4691) comment that held its place. Marks the two now-satisfied prerequisites in the runbook. The google/oss-fuzz submission PR itself is intentionally NOT opened -- it stays a human follow-up, so steps 3-6 of the runbook are unchanged. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
fuzz/fuzz_targets/load_archive.rs:34
- Same issue here: referencing
src/loader.rs:<line>in a SAFETY comment will go stale. Point at the enum variant/type name (and optionally the module) instead of a line number.
// SAFETY: a `Graph` holds only `GraphBacking::Static(&'static [u8])`
// (src/loader.rs:167) — a borrow, not an owner — and both it and `leaked`
// went out of scope above, so no reference into `ptr` survives.
// Reconstructing the `Box` to free it is sound.
Copilot review: the fuzz-target doc comments cited src/loader.rs:533, :167 and :426, which drift as loader.rs changes. Two of the three had already drifted -- :167 pointed past the GraphBacking enum (164) and :426 past Graph::route (420). Name the symbols instead: validate_header, GraphBacking, Graph::route. Comments only; no code change. Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ENG-4691 — cargo-fuzz target + OSS-Fuzz submission
Continuously fuzz the graph reader so malformed inputs can't panic, OOM, or segfault.
What changed
fuzz/— self-contained cargo-fuzz package (its own[workspace]root, so it's excluded from the root build now and if a root workspace is ever added). Two targets:load_archive— feeds arbitrary bytes toGraph::from_bytes. Promotes a copy of the transient buffer to'static, then reclaims it so RSS stays flat across libFuzzer's in-process iterations — a plainBox::leakwould trip a false-positive OOM with the 411 KB seed driving-max_lenup. The promoted borrow and theGraphholding it are confined to an inner scope, so neither is live when the allocation is freed.route_inputs— fixed 50 km graph, fuzzesfrom/to/blocked composition.Arbitraryis hand-written (not derived) so the blocked-id count comes from a singleu8, capping the set at 255 — an unboundedVec<u32>would grow with fuzzer input size and surface as slow-unit/OOM false positives unrelated toroutecorrectness.RRG1+v1)..github/workflows/fuzz.yaml— nightly cargo-fuzz quick-pass: builds both targets, runsload_archive -- -max_total_time=60per PR. Mirrorsci.yamlconventions; nightly is the sole deviation (sanitizers require it), and thegnutarget is pinned because ASan is incompatible with a statically linked libc.oss-fuzz/projects/rustyroute/—Dockerfile,build.sh,project.yamlfor agoogle/oss-fuzzsubmission, plus aREADME.mdrunbook. Seed shipped via<target>_seed_corpus.zip.build.shpins the samegnutriple as the CI workflow and derives its copy path from it; theDockerfileuses absolute$SRCpaths to match.tests/fuzz_workflow.rs— 12 durable tests locking the workflow shape, fuzz-crate structure, seed header, OSS-Fuzz file validity, and the acceptance guard that rootcargo metadataexcludes the fuzz package.Verification
CI on the current head is the authoritative gate — fmt, clippy
-D warnings, tests across 6 OS/toolchain combos, 3 feature rows, docs (-D warnings), coverage, pre-commit e2e, and thefuzzjob that builds both targets and runs the 60sload_archivepass.The figures below were measured locally on the pre-review tree (
2a9599a). The three review-fix commits after it were verified by CI only — this dev environment has no Rust toolchain installed, so local re-runs were not possible:cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings: cleancargo test: 26 test binaries, 0 failurescargo fuzz run load_archive -- -max_total_time=30: 58,160 runs, exit 0, no crash, RSS flatcargo fuzz run route_inputs: clean; single-seed replay cleancargo metadata --format-version 1:rustyroute-fuzzabsent from members and packagesbuild.shdry-run: emits both ELF fuzzer binaries +load_archive_seed_corpus.zipFollow-up (external, not in this PR)
Both submission prerequisites are now satisfied in-repo:
project.yamlauto_ccslistsjimbo@spot-ship.comandjimbo@freedman.io.spotship/rustyrouteis public.What remains is deliberately not done here: the submission PR to
google/oss-fuzz(fork, copyprojects/rustyroute/, validate withinfra/helper.py check_build, open the PR) is left as a human follow-up by request. Approval then takes 1–3 weeks, after which the tracking link goes on ENG-4691. Runbook:oss-fuzz/README.md.🤖 Generated with Claude Code