✨ Structured results for Fuzzing check #2658
Conversation
laurentsimon
commented
Feb 13, 2023
laurentsimon
commented
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
laurentsimon
requested review from
azeemshaikh38,
justaugustus,
naveensrinivasan and
spencerschrock
as code owners
laurentsimon
had a problem deploying
to
integration-test
GitHub Actions
Failure
Signed-off-by: laurentsimon <[email protected]>
laurentsimon
had a problem deploying
to
integration-test
GitHub Actions
Failure
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2658 +/- ##
==========================================
+ Coverage 41.27% 49.91% +8.64%
==========================================
Files 123 159 +36
Lines 10043 12661 +2618
==========================================
+ Hits 4145 6320 +2175
- Misses 5602 5966 +364
- Partials 296 375 +79 |
|
Stale pull request message |
| short: Check that the project is fuzzed using OSS-Fuzz | ||
| desc: This rule checks that whether the project is fuzzed with OSS-Fuzz |
There was a problem hiding this comment.
copy-paste error with OSS-Fuzz instead of cluster fuzz lite
| - If no fuzzing functions are found in the source code, a finding with OutcomeNegative (0) is returned. | ||
| risk: Medium | ||
| remediation: | ||
| effort: High |
There was a problem hiding this comment.
Would Go native fuzzing be considered lower effort? Assuming it's a go project I'd say probably?
| } | ||
| return checker.CreateMinScoreResult(name, "project is not fuzzed") | ||
| } | ||
| fuzzers := []string{} |
There was a problem hiding this comment.
Since we know the length of r.Fuzzers, we can minimize allocations with:
fuzzers := make([]string, 0, len(r.Fuzzers)
| NumberOfWarn: 4, | ||
| NumberOfInfo: 0, |
There was a problem hiding this comment.
This should probably be 3 warn and 1 info.
The test is broken right now though #2670
| @@ -371,3 +371,25 @@ type TokenPermission struct { | |||
| Msg *string | |||
| Type PermissionLevel | |||
| } | |||
|
|
|||
| // Location generates location from a file. | |||
| func (f *File) Location() *finding.Location { | |||
There was a problem hiding this comment.
i assume these are identical between this PR and #2634 ?
| @@ -199,3 +200,30 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult { | |||
| Reason: e.Error(), // Note: message already accessible by caller thru `Error`. | |||
| } | |||
| } | |||
|
|
|||
| func LogFinding(rules embed.FS, ruleName, text string, loc *finding.Location, | |||
There was a problem hiding this comment.
identical between here and #2634 ?
| fuzzer := r.Fuzzers[i] | ||
| // NOTE: files are not populated for all fuzzers yet. | ||
| // To simplify the code, we currently do not report the locations. | ||
| switch fuzzer.Name { |
There was a problem hiding this comment.
there's a lot of similarity here that could probably be extracted out if we had a map from fuzzer.Name -> rule name.
The switch statement could be replaced with a map. Something like
m := map[string]string{
"GoNativeFuzzer": fuzzingWithGoNative,
// and so on
// ...
}
fuzzRule, ok := m[fuzzer.Name]
if !ok {
// unsupported rule
}
if err := checker.LogFinding(rules, fuzzRule,
"project fuzzed with " + fuzzer.Name,
nil, finding.OutcomePositive, dl); err != nil {
return checker.CreateRuntimeErrorResult(name, err)
}
reportedRuleResults[fuzzRule] = true
WDUT?
|
|
||
| func logDefaultFindings(dl checker.DetailLogger, r map[string]bool) error { | ||
| // We always report at least one finding for each rule. | ||
| if !r[fuzzingWithGoNative] { |
There was a problem hiding this comment.
same comment about the map. so the map could be an unexported package level var?
|
Stale pull request message |
laurentsimon
had a problem deploying
to
integration-test
GitHub Actions
Failure
|
Stale pull request message |
|
@laurentsimon A Friendly ping! |
|
Stale pull request message |
