ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source
https://scorecard.dev
Apache License 2.0
4.47k stars 489 forks source link

Feature: make `fuzzed` tool detection more granular #3231

Open laurentsimon opened 1 year ago

laurentsimon commented 1 year ago

We currently check for multiple tools for javascript property-based testing https://github.com/ossf/scorecard/blob/main/checks/raw/fuzzing.go#L82-L115

The proves have the same granularity it. We need to have a probe for each tool.

SO we need to update both the raw part of the check and the probe definitions

github-actions[bot] commented 1 year ago

Stale issue message

spencerschrock commented 8 months ago

The proves have the same granularity it. We need to have a probe for each tool.

I lean against the one probe per tool approach. It's ~250 lines per tool, for essentially what is a 1 line tool to regex mapping. They all use the same structure under the hood.

If we want people to be able to write policies against specific tools, I bet we could expose the tool in the finding another way.

laurentsimon commented 8 months ago

How do we handle remediation in this case? Could we instead have a generic tool helper function where we pass the tool name as input, like we do for fuzzer probes? That mostly removes the need to code anything besides adding a few headers (which maybe we can clean up as part of https://github.com/ossf/scorecard/issues/3720)? Wdut?

pnacht commented 8 months ago

Yeah, I'd rather have a single isFuzzed probe. And then do what's already done for the Best-Practices-Badge: indicate which fuzzer is used by adding it to finding.Values:

https://github.com/ossf/scorecard/blob/8ac9ca15a30b916c0990e03525378f6c8ef9ef4b/checks/evaluation/cii_best_practices.go#L60-L63

This would also mean new fuzzers only need to change things in one place: probes/isFuzzed, while the current layout requires a new probe and modifying the evaluation logic:

https://github.com/ossf/scorecard/blob/8ac9ca15a30b916c0990e03525378f6c8ef9ef4b/checks/evaluation/fuzzing.go#L21-L53

The same idea could also apply to SAST and dependency-update tools, which also have one probe per tool.

This change does mean, however, that policies would need to have access to finding.Values if they only want to accept certain tools (or give projects partial score for a silver best-practices badge).

spencerschrock commented 8 months ago

How do we handle remediation in this case? Could we instead have a generic tool helper function where we pass the tool name as input, like we do for fuzzer probes? That mostly removes the need to code anything besides adding a few headers (which maybe we can clean up as part of #3720)? Wdut?

Have a generic remediation text like: "adopt a language specific fuzzer for your project. For a list of fuzzers we detect visit (some/path/to/README.md)"?

It's not that different from what we do know, which is link to some other fuzzer-specific readme.

The same idea could also apply to SAST and dependency-update tools, which also have one probe per tool.

Agreed.

spencerschrock commented 6 months ago

Reworded title as we can make the tool detection more granular without everything being a separate probe (which would be in conflict with #3824)