chore: Add fuzz tests #399
Open
Conversation
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #399 +/- ##
=====================================
Coverage 93.0% 93.0%
=====================================
Files 231 231
Lines 80461 80461
=====================================
Hits 74904 74904
Misses 5557 5557
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
son-oz
force-pushed
the
ps-35-add-cargo-fuzz
branch
from
c475ea1 to
b4acf16
Compare
WalkthroughAdds fuzzing infrastructure using cargo-fuzz, including a fuzz target for DER public key extraction, a dedicated fuzz workspace, and ignore rules. Introduces a new Cargo feature flag Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
🧹 Nitpick comments (1)
fuzz/README.md (1)
1-25: Good documentation, but add seed corpus guidance.The README clearly explains setup and execution. Consider adding:
- Seed corpus guidance: Include a section on creating a seed corpus with valid DER-encoded public keys to improve fuzzing effectiveness (mentioned in Limitations but not explained).
- Feature flag note: If the fuzzing feature needs manual activation (pending the fix to fuzz/Cargo.toml), document it here.
Example addition:
## Seed Corpus To improve fuzzing effectiveness, provide seed inputs with valid DER-encoded ECDSA public keys: ```bash mkdir -p fuzz/corpus/extract_public_key_from_der # Add valid DER samples to fuzz/corpus/extract_public_key_from_der/Seed samples help the fuzzer understand the expected input structure and explore edge cases more efficiently.
</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 971ab9b49c1397247a36d6daf54738249e854132 and b4acf1663fda8ad8630ac3969143184c1c09d2cb. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `fuzz/Cargo.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (6)</summary> * `Cargo.toml` (1 hunks) * `fuzz/.gitignore` (1 hunks) * `fuzz/Cargo.toml` (1 hunks) * `fuzz/README.md` (1 hunks) * `fuzz/fuzz_targets/extract_public_key_from_der.rs` (1 hunks) * `src/utils/mod.rs` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>fuzz/fuzz_targets/extract_public_key_from_der.rs (1)</summary><blockquote> <details> <summary>src/utils/der.rs (1)</summary> * `extract_public_key_from_der` (15-27) </details> </blockquote></details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)</summary> * GitHub Check: boostsecurity - boostsecurityio/semgrep-pro * GitHub Check: test * GitHub Check: clippy * GitHub Check: msrv * GitHub Check: Redirect rules - openzeppelin-relayer * GitHub Check: Header rules - openzeppelin-relayer * GitHub Check: Pages changed - openzeppelin-relayer * GitHub Check: Analyze (rust) * GitHub Check: semgrep/ci </details> <details> <summary>🔇 Additional comments (6)</summary><blockquote> <details> <summary>fuzz/.gitignore (1)</summary><blockquote> `1-4`: **LGTM! Standard fuzzing artifact exclusions.** The gitignore patterns correctly exclude all standard cargo-fuzz outputs: build artifacts, corpus data, crash artifacts, and coverage reports. </blockquote></details> <details> <summary>Cargo.toml (2)</summary><blockquote> `139-140`: **LGTM! Proper lint configuration for custom cfg.** The `check-cfg` lint correctly declares the `fuzzing` cfg to avoid warnings about unexpected configuration predicates. --- `136-137`: **Ensure the `fuzzing` feature is enabled for fuzz targets.** The root `Cargo.toml` declares `fuzzing`, but the fuzz workspace doesn’t activate it. Add `features = ["fuzzing"]` in `fuzz/Cargo.toml` (or pass `--features fuzzing` when invoking) so that `#[cfg(fuzzing)]` blocks compile. </blockquote></details> <details> <summary>fuzz/Cargo.toml (1)</summary><blockquote> `1-21`: **Standard cargo-fuzz configuration is correct.** The fuzz package metadata, libfuzzer-sys dependency, and binary target configuration follow cargo-fuzz conventions. </blockquote></details> <details> <summary>fuzz/README.md (1)</summary><blockquote> `17-20`: **Accurate limitations noted.** The acknowledgment of needing more fuzz targets and structure-aware fuzzing demonstrates awareness of the current state and future improvements. </blockquote></details> <details> <summary>src/utils/mod.rs (1)</summary><blockquote> `23-27`: **Keep public re-export outside cfg; current visibility pattern is correct.** The `pub use der::*` must remain unconditional to expose `extract_public_key_from_der` and `DerError` in non-fuzzing builds—moving it inside a `#[cfg(fuzzing)]` block would break the public API. The conditional on `pub mod der` only affects the module path, not the re-exported items, which matches the intended design. > Likely an incorrect or invalid review comment. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+13
to
+14
| [dependencies.openzeppelin-relayer] | ||
| path = ".." |
There was a problem hiding this comment.
Activate the fuzzing feature for the dependency.
The fuzz workspace depends on the parent crate but doesn't activate the fuzzing feature. This means cfg(fuzzing) conditions in src/utils/mod.rs won't be satisfied, and the fuzz target won't be able to access utils::der::extract_public_key_from_der.
Apply this diff to activate the fuzzing feature:
[dependencies.openzeppelin-relayer]
path = ".."
+features = ["fuzzing"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [dependencies.openzeppelin-relayer] | |
| path = ".." | |
| [dependencies.openzeppelin-relayer] | |
| path = ".." | |
| features = ["fuzzing"] |
🤖 Prompt for AI Agents
In fuzz/Cargo.toml around lines 13 to 14, the workspace dependency on the parent
crate does not enable the fuzzing feature so cfg(fuzzing) items in
src/utils/mod.rs are disabled; update the dependency entry to enable the fuzzing
feature by adding a features array (e.g. change the dependency to use path =
".." and features = ["fuzzing"]) so the fuzz target can access
utils::der::extract_public_key_from_der.
Comment on lines
+1
to
+8
| #![no_main] | ||
|
|
||
| use libfuzzer_sys::fuzz_target; | ||
| use openzeppelin_relayer::utils::der::extract_public_key_from_der; | ||
|
|
||
| fuzz_target!(|data: &[u8]| { | ||
| let _ = extract_public_key_from_der(data); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
LGTM! Clean fuzzing harness.
The fuzz target is correctly structured with no_main and a minimal harness that feeds arbitrary bytes to extract_public_key_from_der. Ignoring the result is appropriate—libFuzzer will catch panics and crashes automatically.
However, verify that:
- The
fuzzingfeature is activated soutils::deris accessible (see comment on root Cargo.toml) - A seed corpus with valid DER samples exists to bootstrap fuzzing effectively
🏁 Script executed:
#!/bin/bash
# Description: Check for seed corpus and verify der module accessibility
# Check if a seed corpus directory exists
echo "=== Checking for seed corpus ==="
fd -t d 'corpus|seeds' fuzz/
# List any existing corpus files
if [ -d "fuzz/corpus" ] || [ -d "fuzz/seeds" ]; then
echo -e "\n=== Corpus files found ==="
fd . fuzz/corpus fuzz/seeds 2>/dev/null | head -20
else
echo "No seed corpus found. Consider adding valid DER samples to fuzz/corpus/extract_public_key_from_der/"
fi
# Verify that der module is conditionally compiled
echo -e "\n=== Checking der module conditional compilation ==="
rg -n 'cfg\(fuzzing\)' src/utils/mod.rs
# Check if fuzz Cargo.toml activates the fuzzing feature
echo -e "\n=== Checking if fuzzing feature is activated ==="
rg -n 'features.*=.*\[.*fuzzing' fuzz/Cargo.tomlLength of output: 254
🏁 Script executed:
#!/bin/bash
# List fuzz directory
echo "=== fuzz directory contents ==="
ls -1 fuzz
# Show fuzz/Cargo.toml contents
echo -e "\n=== fuzz/Cargo.toml ==="
sed -n '1,200p' fuzz/Cargo.toml
# Show openzeppelin_relayer dependency lines
echo -e "\n=== openzeppelin_relayer dependency ==="
rg -n 'openzeppelin_relayer' -C3 fuzz/Cargo.toml || echo "No openzeppelin_relayer entry found"Length of output: 537
Enable fuzzing feature for DER module and add seed corpus
- In fuzz/Cargo.toml, enable the
fuzzingfeature on the openzeppelin-relayer dependency:[dependencies.openzeppelin-relayer] path = ".." features = ["fuzzing"]
- Create a seed corpus directory at
fuzz/corpus/extract_public_key_from_der/with representative DER samples.
🤖 Prompt for AI Agents
In fuzz/fuzz_targets/extract_public_key_from_der.rs lines 1-8: the fuzz target
uses openzeppelin_relayer's DER utilities but the fuzzing feature isn't enabled
and there's no seed corpus; update fuzz/Cargo.toml to set the
openzeppelin-relayer dependency to the workspace path and enable features =
["fuzzing"], then create a seed corpus directory at
fuzz/corpus/extract_public_key_from_der/ and add several representative DER
sample files (e.g., small valid/invalid DER blobs) so the fuzzer has initial
inputs to exercise extract_public_key_from_der.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
(PS-35)
Testing Process
Checklist
Summary by CodeRabbit
Note: No changes to runtime behavior or public interfaces; impact is limited to testing and development workflows.