test(fuzz): revive the fuzz target and run it nightly in CI#281
Conversation
The fuzz target silently became a no-op when `resolve` turned async: the returned future was dropped instead of awaited, so the resolution algorithm never ran. Drive it with `block_on`, vary a few resolver options (condition names, extensions, alias, symlinks) from the input for wider coverage, and run it nightly via a scheduled workflow.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request makes two related changes to the fuzzing setup. The fuzz target ( 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/fuzz.yml:
- Around line 9-12: The max_total_time workflow input is vulnerable to command
injection when passed directly to a shell command. Change the input type from
the default string to number type in the input definition for max_total_time to
enforce numeric constraints at the GitHub Actions level. Additionally, before
the shell command at line 50 that uses the max_total_time input, add validation
logic to ensure the input is a valid positive integer and reject or use a safe
default value if the validation fails. This prevents arbitrary shell command
injection through the workflow input expansion.
In `@fuzz/fuzz_targets/resolver.rs`:
- Line 41: The `unwrap()` call on `std::env::current_dir()` at line 41 in the
fuzz harness can cause panics during environment setup failures, leading to
false-positive fuzz crashes. Replace the `unwrap()` with proper error handling
that uses a match statement or the `?` operator to return early from the harness
function if current_dir fails, allowing the fuzzer to skip that input gracefully
instead of crashing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 606452ed-0f9b-490d-b242-aa6686ff895f
📒 Files selected for processing (3)
.github/workflows/fuzz.ymlfuzz/Cargo.tomlfuzz/fuzz_targets/resolver.rs
There was a problem hiding this comment.
Pull request overview
Revives the cargo-fuzz resolver harness so it actually executes the async resolution logic, and adds a scheduled GitHub Actions workflow to run fuzzing nightly to catch regressions earlier.
Changes:
- Drive the async
Resolver::resolvefuture to completion in the fuzz target and expand coverage by varying severalResolveOptionsbased on input bytes. - Add
futuresas a dependency of the fuzz crate to enablefutures::executor::block_on. - Introduce a nightly CI workflow to run
cargo fuzzon a schedule (and via manual dispatch), with corpus caching and crash artifact upload.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
fuzz/fuzz_targets/resolver.rs |
Fixes the harness to actually run the async resolver and broadens option coverage per input. |
fuzz/Cargo.toml |
Adds the futures dependency needed to synchronously drive the async resolve call. |
.github/workflows/fuzz.yml |
Adds nightly/manual fuzzing in CI with caching and artifact upload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0bb614965
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GitHub's built-in failure notification for scheduled workflows only pings the user who last edited the cron, subject to their personal settings, so a nightly fuzz crash can go unnoticed. File a GitHub issue on scheduled-run failure instead (deduped against an already-open one). Manual workflow_dispatch failures are excluded.
Tag the auto-filed crash issue with a fuzz-crash label for filtering, and use that label as the dedup key. The step creates the label idempotently (--force) so it works without pre-creating it in the repo.
Lets this PR exercise the Fuzz job in real Actions before merge, since workflow_dispatch/schedule only work once the file is on the default branch. To be removed before merge so the workflow stays nightly-only.
cargo-binstall ships a musl binary, so `cargo binstall cargo-fuzz` installed the musl build, which defaults its build target to x86_64-unknown-linux-musl — whose std isn't on the gnu runner, failing with "can't find crate for core". Build cargo-fuzz from source instead.
The PR-triggered run validated the workflow end-to-end (cargo-fuzz install, instrumented build, 300s fuzz, failure-only steps skipping on success). Revert to nightly schedule + manual dispatch only.
The harness resolves against std::env::current_dir(). With working-directory: fuzz that was the near-empty fuzz crate, so the campaign barely exercised real resolution. Running from the repo root (cargo fuzz auto-discovers fuzz/) raised corpus-replay edge coverage from 1156 to 1368. Corpus/artifact paths stay under fuzz/.
Measured the cwd effect rigorously (same binary, same corpus): repo root gives identical coverage to fuzz/ (~1369 edges) but ~14% lower throughput, because random specifiers rarely form valid paths/package names, so the fs-dependent code is unreached regardless of the tree. Keep working-directory: fuzz. Restores the validated-green workflow.
…d-nightly-ci # Conflicts: # fuzz/Cargo.toml
Why
The fuzz target silently stopped testing anything. When
resolvebecameasync, the target'slet _ = resolver.resolve(...)began dropping thereturned future instead of awaiting it — Rust futures are lazy, so the
resolution algorithm never ran. Each iteration only built a
Resolverandread
cwd. It was also never wired into CI, so the regression went unnoticed.What
futures::executor::block_onso the resolver actuallyruns (no tokio runtime needed — the FS layer uses
std::fs).import/require, extension sets, optional alias, symlinks) for wider coverage
instead of default options only.
.github/workflows/fuzz.yml: a nightly (03:00 UTC) short run plus manualdispatch, on the nightly toolchain via
cargo-binstall cargo-fuzz, with apersisted corpus cache and crash-artifact upload on failure.