Added fuzzer to be run with OSS-Fuzz

[DavidKorczynski]

Adds a fuzzer as a step to integrating into OSS-Fuzz.

Cross-referencing

• Integrating continuous fuzzing by way of OSS-Fuzz #771
• jsonschema: initial integration. google/oss-fuzz#4996

@Julian I now added more structure to it by way of Hypothesis and am happy to put more effort in. I think it would be nice to get it up and running on OSS-Fuzz though, so we can start to see when results happen.

@Zac-HD Thanks for your assistance in the other issue. Will keep digging into Hypothesis!

[codecov]

Codecov Report

Merging #772 (d9b5ca8) into main (b4a3340) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #772   +/-   ##
=======================================
  Coverage   96.37%   96.37%           
=======================================
  Files          17       17           
  Lines        2703     2703           
  Branches      309      309           
=======================================
  Hits         2605     2605           
  Misses         79       79           
  Partials       19       19           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4a3340...d9b5ca8. Read the comment docs.

[Julian]

@Julian I now added more structure to it by way of Hypothesis and am happy to put more effort in. I think it would be nice to get it up and running on OSS-Fuzz though, so we can start to see when results happen.

Cool! So the idea here which I think I'm OK with is to get something here that we can run and iterate on without needing to touch the upstream OSS-Fuzz repo, right? I think to @Zac-HD's point what's here so far likely wouldn't "do" much utility wise but with some tweaks hopefully would. So we could just push forward and work on those tweaks.

I do think much as it might seem annoying, that we should address the coverage failure -- meaning, it would be good to have a "test" that executes the fuzzer for some extremely tiny amount, just so that we know any API changes don't suddenly silently break integration with OSS-Fuzz.

Does that make sense to you / seem like it'd be non-totally-terrible to do? The hardest part may be that module here which may need stubbing that's OSS-Fuzz specific? Let me know thoughts otherwise I can think myself about what that might look like, but yeah do think it'd be nice if somewhere locally here in CI we knew this script continued to function.

[DavidKorczynski]

@Julian I do think much as it might seem annoying, that we should address the coverage failure -- meaning, it would be good to have a "test" that executes the fuzzer for some extremely tiny amount, just so that we know any API changes don't suddenly silently break integration with OSS-Fuzz.

Does that make sense to you / seem like it'd be non-totally-terrible to do? The hardest part may be that module here which may need stubbing that's OSS-Fuzz specific? Let me know thoughts otherwise I can think myself about what that might look like, but yeah do think it'd be nice if somewhere locally here in CI we knew this script continued to function.

I understand your concern although having integrated loads of projects into OSS-Fuzz it's a first to actually write tests that tests the fuzzers. How about doing the following approach - I added a main.yml file that integrates jsonschema into CI-fuzz, i.e. the fuzzer will be run for 600 seconds for every pull request. This might be a good approach to this? Let me know if not and I will then add a test.

[DavidKorczynski]

@Julian Cool! So the idea here which I think I'm OK with is to get something here that we can run and iterate on without needing to touch the upstream OSS-Fuzz repo, right?

Yep, no iterations needed from you on OSS-Fuzz. Also, we are collaborating with the OSS-Fuzz folks so you can ping me whenever if you think there is something going that needs fixing in OSS-Fuzz and I will handle it for you. But by default you will not need to touch OSS-Fuzz.

[Julian]

I added a main.yml file that integrates jsonschema into CI-fuzz, i.e. the fuzzer will be run for 600 seconds for every pull request. This might be a good approach to this? Let me know if not and I will then add a test.

I think even if this were 2 seconds rather than 600 seconds that my concern would be addressed. It's just "make sure the thing runs" any time someone makes a change to it.

[DavidKorczynski]

The reason I set 600 seconds is because OSS-Fuzz sets it as a minimum (https://google.github.io/oss-fuzz/getting-started/continuous-integration/#integrating-into-your-repository)

[Julian]

The reason I set 600 seconds is because OSS-Fuzz sets it as a minimum (https://google.github.io/oss-fuzz/getting-started/continuous-integration/#integrating-into-your-repository)

It's not really a minimum, that's the docs saying any less than that and it's not very useful for fuzzing not that it will fail the build or anything -- let's set it to 30 seconds I think.

Looks like it's failing now though for some config reason?

[DavidKorczynski]

Looks like it's failing now though for some config reason?

I recon this is because the project is not ready integrated in OSS-Fuzz (we are waiting for this to merge)

[Julian]

Ah fair enough. Can you maybe make that job not fail the build then (that might be reasonable even going forward perhaps)?

After that sounding like we're close to merge.

[DavidKorczynski]

Ah fair enough. Can you maybe make that job not fail the build then (that might be reasonable even going forward perhaps)?

After that sounding like we're close to merge.

Could you clarify on this one - am not sure I understand (this is the first time I integrate a CI job).

Do you mean make it not fail by merging things in OSS-Fuzz, or do you mean not fail by changing something in the main.yml file? I put in a continue-on-error: true for the fuzzing job, is this perhaps what you were looking for?

[DavidKorczynski]

I think I got a solution you were looking for now @Julian - now the CIFuzz job will exit with success in case the build step fails.

[Julian]

Hooray. Will have a close look in the morning when I've got a clean mind but think that's perfect!

[Zac-HD]

One nice way to ensure everything is covered is to run the fuzz harness as a test function! Specifically:

• Rename the file as e.g. test_foo_fuzz.py, so that pytest collects it;
• Adjust the glob pattern in the OSS-Fuzz config to detect *fuzz.py instead of fuzz_*.py
• Move import atheris into the if __name__ == "__main__": block, so that it's a fuzz-time but not a test-time dependency

And then it'll be covered as expected (modulo perhaps a coverage pragma on the __name__ branch...)

[Julian]

@DavidKorczynski apologies for the delay here, have been dealing with some other stuff first, but hopefully will get to merging this by the end of the weekend.

[DavidKorczynski]

No problem @Julian - please take your time and handle this when it makes sense to you. There is no stress from my side.

[Julian]

@DavidKorczynski apologies here that this slipped, but I'm likely about to merge as soon as CI passes (which I put some minor changes in for).

I did re-rename this back to fuzz_* rather than being prefixed with test_ even though I agree that's a good thing in the long term, but for now hypothesis isn't a testing dependency so it was failing the normal CI run when that module was imported, and rather than bother to futz with installing hypothesis to be able to import that file, I just moved it back for now.

You'll obviously see when/if I merge in a bit after I see the build go green.

[DavidKorczynski]

Wohoooo - we got it done :)! google/oss-fuzz#4996

Thanks @Zac-HD for the input and thanks @Julian for getting it merged in.

[Julian]

Thank you!