🐛 Check OSS Fuzz build file for Fuzzing check

[spencerschrock]

What kind of change does this PR introduce?

bug fix

• PR title follows the guidelines defined in our pull request documentation

What is the current behavior?

The fuzzing check uses a GitHub repo client to search google/oss-fuzz for a project to determine if it's fuzzed.
Searching the google/oss-fuzz hasn't been working for a while.

What is the new behavior (if this is a feature change)?**

There's a new ossfuzz client which only implements the Search method.
This method fetches the OSS Fuzz status file once and parses the projects.

Using a client involves stubbing all of the unneeded methods, but allows the change to be made in a backwards compatible manner for everyone that calls pkg.RunScorecard.

• Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #2670

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

"githubrepo.CreateOssFuzzRepoClient" has been deprecated. If you directly call this function, please switch to 
"ossfuzz.CreateOSSFuzzClient" or "ossfuzz.CreateOSSFuzzClientEager". If you use `checker.GetClients`, no change is needed.

[naveensrinivasan]

This cool! Thanks

[spencerschrock]

Hmm, the unit tests work locally for me when run with make unit-test.
When I emulate the GitHub action with act -r -j unit-test -W ./.github/workflows/main.yml it's failing like here. Some of our other tests do an os.ReadFile without problem. Will need to debug on Monday.

[codecov]

Codecov Report

Merging #2719 (5b7acc1) into main (c06ac74) will increase coverage by 0.19%.
The diff coverage is 56.45%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2719      +/-   ##
==========================================
+ Coverage   41.20%   41.39%   +0.19%     
==========================================
  Files         123      124       +1     
  Lines       10041    10164     +123     
==========================================
+ Hits         4137     4207      +70     
- Misses       5609     5659      +50     
- Partials      295      298       +3     

[azeemshaikh38]

Do we really need a OssFuzzClient at all? How bad would it be if we always made a HTTP call and parsed the JSON?

[spencerschrock]

Do we really need a OssFuzzClient at all? How bad would it be if we always made a HTTP call and parsed the JSON?

It's very simple to do it just with a HTTP call, the initial commit did just that (ignore the byte search instead of JSON parsing which is very much needed)

scorecard/checks/raw/fuzzing.go

Lines 173 to 185 in 5335f4b

	resp, err := http.Get(ossFuzzProjectURL)
	if err != nil {
	return false, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("http.Get: %v", err))
	}
	defer resp.Body.Close()
	if resp.StatusCode >= 400 {
	return false, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("fetch OSS-Fuzz project list: %s", resp.Status))
	}
	body, err := io.ReadAll(resp.Body)
	if err != nil {
	return false, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("io.ReadAll: %v", err))
	}
	return bytes.Contains(body, []byte(c.RepoClient.URI())), nil

But there are numerous problems with this.

• Each pkg.RunScorecard() call would redownload a nearly 1MB file for each repo in the cron job. Which would be O(TB) each week.
• It's not easily testable with a hardcoded URL. We could include the URL in the CheckRequest struct or as a global var, but then the existing OssFuzzRepo is unused. Removing it would break other code that depends on it, and there is external code that depends on it.

In my opinion the OSSFuzzClient is a minimal implementation. It's still just a simple HTTP call and JSON parse, with some caching. The approach results in minimal changes within the code base and outside of it. What are your concerns with the approach?

[azeemshaikh38]

LGTM. For context, I'm hoping to reduce the number of dependencies we inject into RunScorecards fn. These were added for improving cron performance, but its causing spaghetti dependencies in the code. If keeping the structure is simpler in terms of code change, let's stick with that for now.