-
Notifications
You must be signed in to change notification settings - Fork 70
per fuzzer coverage information in project overview #2102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I will fix the CI issues tomorrow. Is there a command to run these CI tests locally? I ran the formatting shell script but it returned success. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! Could you use single quotes instead of double quotes where possible?
Once the CI is green then we can land this. We don't have helper scripts for the ci yet. In general, if you have typing and run yapf -i
on the modules, then you should be good. We'd be happy to land some scripts that does this locally though, it's about automating https://github.com/ossf/fuzz-introspector/blob/main/.github/workflows/main.yml and https://github.com/ossf/fuzz-introspector/blob/main/.github/workflows/mypy.yml
Just to confirm, are you able to see the output of the failed CI runs?
Good to hear. Of course, will update.
Ok, sounds good. I'll see if I get around to those scripts.
|
I fixed the formatting. However, two coverage reports that are needed for the webapp-api-test are not available, so this check does not succeed, removing those projects from the must_include file "fixes" this, how should we proceed? https://storage.googleapis.com/oss-fuzz-coverage/abseil-cpp/reports/20250216/linux/summary.json https://storage.googleapis.com/oss-fuzz-coverage/leveldb/reports/20250216/linux/summary.json |
50b2874
to
efa09bb
Compare
efa09bb
to
086e7a6
Compare
Signed-off-by: phi-go <[email protected]>
Signed-off-by: phi-go <[email protected]>
086e7a6
to
43bed73
Compare
Signed-off-by: phi-go <[email protected]> Signed-off-by: phi-go <[email protected]>
43bed73
to
bee9b7a
Compare
We shouldn't fail if there is no coverage in this manner. Something like the following solves it: $ git diff ./
diff --git a/tools/web-fuzzing-introspection/app/static/assets/db/web_db_creator_from_summary.py b/tools/web-fuzzing-introspection/app/static/assets/db/web_db_creator_from_summary.py
index 42f3470..4b98b52 100644
--- a/tools/web-fuzzing-introspection/app/static/assets/db/web_db_creator_from_summary.py
+++ b/tools/web-fuzzing-introspection/app/static/assets/db/web_db_creator_from_summary.py
@@ -319,7 +319,8 @@ def prepare_code_coverage_dict(
project_language: str) -> Optional[Dict[str, Any]]:
"""Gets coverage URL and line coverage total of a project"""
line_total_summary = extract_code_coverage_data(code_coverage_summary)
-
+ if not line_total_summary:
+ return None
coverage_url = oss_fuzz.get_coverage_report_url(project_name,
date_str.replace('-', ''),
project_language)
@@ -457,7 +458,9 @@ def extract_local_project_data(project_name, oss_fuzz_path,
code_coverage_data_dict = prepare_code_coverage_dict(
code_coverage_summary, project_name, '', project_language)
-
+ if not code_coverage_data_dict:
+ return
+
if cov_fuzz_stats is not None:
all_fuzzers = cov_fuzz_stats.split("\n")
if all_fuzzers[-1] == '':
@@ -719,7 +722,9 @@ def extract_project_data(project_name, date_str, should_include_details,
code_coverage_data_dict = prepare_code_coverage_dict(
code_coverage_summary, project_name, date_str, project_language)
-
+ if not code_coverage_data_dict:
+ return
+
per_fuzzer_cov = {}
if cov_fuzz_stats is not None:
all_fuzzers = cov_fuzz_stats.split("\n")
|
Signed-off-by: phi-go <[email protected]>
Oof, I see, my bad. Thank you for taking a look! Only the first return should be necessary, neither the local nor the normal run needed this check before and no logic was changed in that regard. I updated as such. |
</td> | ||
<td> | ||
{% if fuzzer_data['got_lost'] %} | ||
Fuzzer no longer available! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at
I have to say that it's really cool. That being said that fuzz target was turned off on OSS-Fuzz on purpose so it would probably be great if it was possible to somehow indicate that it's known or something like that. It should also probably help when fuzz targets get renamed (though that use case is different and should probably be automated). I'm not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree, would be great to add have that feature. This would require some way to specify expected or disabled harnesses which currently does not exist as far as I know. This would probably need to be supported by oss-fuzz, sadly it seems PRs there are much slower to be reviewed.
For now this only considers the past 30 days, so this fuzzer will be removed after a month.
Though, if you have another idea how to approach this, please let me know.
Adds per fuzzer summary.json (based on
https://storage.googleapis.com/oss-fuzz-coverage/{0}/reports-by-target/{1}/{2}/linux/summary.json
) to the project timestamp data and generates per fuzzer coverage plots for the project overview, example below.This PR is mainly to understand if an approach like this would be acceptable, though to implement some metrics in #2054 it is probably needed to have a separate stage that takes a look at project data over time and not only in report generation.
Still this PR addresses an issue but it might take too much screen space (especially for large numbers of fuzzers), it might also require too much storage space, data ingress and runtime for data generation, and could have noticeable effects on webpage load time as well.
There is a overview as well now: #2102 (comment).