ci(fuzz): build the fuzzer crates on pull requests - #32
Merged
Conversation
`bridge_requires_prebuilt_fuzzer` called `Result::unwrap_err()`, which
requires the `Ok` type to implement `Debug`. `SyzBridge` does not, so the
test failed to compile:
error[E0277]: `syz_bridge::SyzBridge` doesn't implement `Debug`
--> src/syz_bridge.rs:170:42
The module is gated `#[cfg(all(test, feature = "qemu-executor"))]`, and
nothing in CI builds the crate's test targets with that feature — the
nightly job runs `cargo fuzz run`, which does not compile lib tests — so
the breakage was never surfaced.
Destructure the result instead of unwrapping it. That needs no `Debug`
bound, avoids widening a public type's derives to satisfy one test, and
lets the assertion carry a message where it previously had none.
Verified: `cargo check --locked --manifest-path fuzz/Cargo.toml
--all-targets --all-features` goes from exit 101 to exit 0.
`fuzz/Cargo.lock` was out of date in two independent ways, and
`cargo check --locked` against it failed outright:
error: cannot update the lock file ... because --locked was passed
1. It still pinned `uart_16550 0.3.2` and its `x86 0.52.0` dependency,
plus duplicated `bitflags` (1.3.2 + 2.13.0) and `raw-cpuid 10.7.0`
entries that only `uart_16550` required. Dropping the unused
dependency regenerated the root `Cargo.lock` but not this one, so
resolution here now removes `uart_16550`/`x86`, collapses `bitflags`
to a single entry, and moves `raw-cpuid` to 11.6.0 under `x86_64` —
matching what the root lockfile already records.
2. The entire `qemu-executor` optional dependency set — `anyhow`, `hex`,
`nix`, `rand`, `serde`, `serde_json`, `sha2`, `tempfile` and their
transitives — had never been resolved into the file at all.
Nothing caught either drift because no job builds this crate with
`--locked`: the nightly matrix runs `cargo fuzz run`, which silently
re-resolves. The committed lockfile was effectively decorative.
Regenerated on the Linux devbox so the committed lock is authoritative.
`userspace/fuzzer` and `fuzz` were the only crates in the repository that no pull-request check compiled. `ci.yml` never touches either, and `make clippy` / `make fmt-check` cover the bootloader, kernel, and `userspace` workspaces only — both fuzzing crates are separate workspaces with their own lockfiles. The consequence was concrete: two Dependabot PRs bumping `hmac` to 0.13 and `sha2` to 0.11 each reported six green checks without ever building the crate they changed. Both were individually uncompilable — they share a `digest` version and must move together — and either would have turned `main` red on merge. Two independent halves: - Detection. A paths-scoped `pull_request` trigger, so a PR touching either crate is actually built. `fuzz-targets-check` is added for the `fuzz` crate, which the existing matrix only compiles on push and on the nightly schedule — far too heavy to gate a PR on. It passes `--all-features` because `sha2`, `nix`, `rand` and the rest of the QEMU executor dependencies are optional behind `qemu-executor`, so a default-feature check would skip exactly the crates a dependency bump changes; and `--locked`, which is what makes the committed lockfile authoritative rather than decorative. - Prevention. A `rustcrypto` group for both fuzzing directories, so coupled crates arrive as one PR. It deliberately carries no `update-types` filter: Cargo treats a leading-zero bump such as 0.12 -> 0.13 as incompatible, so Dependabot classifies these as semver-major and a minor+patch filter would exclude precisely the bumps that need grouping. The wildcard groups gained matching `exclude-patterns` so membership does not depend on how group precedence is resolved. `aggregate-report` is excluded from `pull_request`: its `!cancelled()` guard would otherwise summarise four skipped jobs and an empty artifact set. Concurrency collapses superseded PR runs only — every other event keeps a run-unique group, so scheduled campaigns and `main` pushes can never cancel or queue behind one another. Verified on the Linux devbox: - actionlint 1.7.7 clean, before and after. - Exactly two jobs run on a pull request (`fuzz-tools-test`, `fuzz-targets-check`). Every heavier job skips, including `candidate-reporting`, whose `issues: write` scope is unreachable because its `needs` result is `skipped`, not `success`. - Job selection is byte-identical to the previous workflow for `push`, both cron schedules, and all three dispatch modes. - `fuzz-tools-test`: 26 tests pass, clippy `-D warnings` clean. - `fuzz-targets-check`: exit 0 cold in 25s. No `-D warnings` is applied; the fuzz targets carry 90 pre-existing dead-code warnings, so this is a compile gate, not a lint gate. Still uncovered: a `kernel/**` change can break the host fuzz harness without any PR check noticing, since `ci.yml` builds the kernel for bare metal and never for the host target these crates link against.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
userspace/fuzzerandfuzzwere the only crates in the repository that no pull-request check compiled.ci.ymlnever touches either.make clippyandmake fmt-checkcover the bootloader, kernel, anduserspaceworkspaces — and both fuzzing crates are separate workspaces with their own lockfiles, so neither is included.fuzz.ymlis the only workflow that builds them, and it triggered onschedule/workflow_dispatch/push: main— neverpull_request.The consequence was concrete, not theoretical. #26 (
hmac0.12.1 → 0.13.0) and #27 (sha20.10.9 → 0.11.0) each reported six green checks without ever building the crate they changed. Both were individually uncompilable —crash-triagebuildsHmac<Sha256>, so the two crates share adigestversion and must move together — and either would have turnedmainred on merge.Two independent halves
Detection —
pull_requesttriggerPaths-scoped to
userspace/fuzzer/**,fuzz/**, and the workflow file.fuzz-targets-checkis new, for thefuzzcrate. The existingcargo-fuzz-targetsmatrix only compiles it on push and on the nightly schedule, and is far too heavy to gate a PR on. Two flags carry weight:--all-featuresis required, not cosmetic.sha2,nix,randand the rest of the QEMU executor dependencies are optional behindqemu-executor, so a default-feature check would silently skip exactly the crates a dependency bump changes.--lockedis what makes the committed lockfile authoritative.cargo-fuzz-targetsrunscargo fuzz runwithout it and therefore re-resolves silently — which is howfuzz/Cargo.lockdrifted out of date unnoticed (see below).Prevention — Dependabot grouping
A
rustcryptogroup for both fuzzing directories, so coupled crates arrive as one PR.It deliberately carries no
update-typesfilter. Cargo treats a leading-zero bump such as0.12 → 0.13as incompatible, so Dependabot classifies these as semver-major — and aminor+patchfilter would exclude precisely the bumps that need grouping. That is why #26 and #27 arrived ungrouped despitefuzzer-tools-minor-and-patchalready existing.The wildcard groups gained matching
exclude-patterns, so membership does not depend on how group precedence is resolved.Two latent defects this surfaced
Both were found by writing the check and watching it fail, and both are fixed in their own commits.
1.
fuzz/Cargo.lockwas stale in two ways (773cf7c) —cargo check --lockedfailed outright against it:uart_16550 0.3.2and itsx86dependency, plus duplicatedbitflags(1.3.2 + 2.13.0) andraw-cpuid 10.7.0entries that onlyuart_16550required. chore(deps): drop the unused uart_16550 dependency #29 regenerated the rootCargo.lockbut not this one.qemu-executoroptional dependency set —anyhow,hex,nix,rand,serde,serde_json,sha2,tempfile— had never been resolved into the file at all.2. A
#[cfg(test)]module did not compile (63d9276) —bridge_requires_prebuilt_fuzzercalledResult::unwrap_err(), which requires theOktype to implDebug;SyzBridgedoes not:Gated
#[cfg(all(test, feature = "qemu-executor"))], and nothing builds the crate's test targets with that feature, so it was never surfaced. Now destructured instead of unwrapped — noDebugbound, no widening a public type's derives for one test, and the assertion gained a message.Blast radius
aggregate-reportis excluded frompull_request: its!cancelled()guard would otherwise summarise four skipped jobs and an empty artifact set.Concurrency collapses superseded PR runs only — every other event keeps a run-unique group (
github.run_id), so scheduled campaigns andmainpushes retain today's behaviour and can never cancel or queue behind one another.Verification (Linux devbox)
pull_requestfuzz-tools-test,fuzz-targets-checkcandidate-reporting(issues: write) on PRneedsresult isskipped, notsuccesspush/ both crons / all 3 dispatch modesfuzz-tools-test-D warningscleanfuzz-targets-checkcargo check --all-targetsbefore thesyz_bridgefixJob selection was verified by evaluating each job's
if:under every event and diffing against the pre-change workflow, not by inspection.No
-D warningsis applied tofuzz-targets-check: the fuzz targets carry 90 pre-existing dead-code warnings, so this is a compile gate, not a lint gate. Tightening those is separate work.Still uncovered
A
kernel/**change can break the host fuzz harness without any PR check noticing.ci.ymlbuilds the kernel for bare metal; it never builds it for the host target thatfuzz/linksmm/net/vfs/kernel_coreagainst viamm/host_harness. Closing that means running this check onkernel/**too, which changes the cost profile of every kernel PR — a deliberate judgment call left out of this change.Note
This PR modifies
.github/workflows/fuzz.yml, so it is matched by its own new trigger and will be the first PR to exercise it.