ossf / scorecard

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

add ability to pass ignore list #1406

Open Dentrax opened 2 years ago

Dentrax commented 2 years ago

What would you like to be added:

We are skipping the binary-check for the testdata folders as implemented in isTestdataFile() function.

Not sure but passing a new ignoreDirs []string field to struct CheckRequest will solve the issue. (It should apply *foo* logic for every element that we traversed?) Which we call in BinaryArtifacts function afterward.

Why is this needed:

To prevent Binary-Artifacts false positives.

From the discussion #593 started by @developer-guy; @westonsteimel said we probably need some enhancements to consider other names for test directories, as @laurentsimon already proposed an idea to ability to skip given folders in the config.

Additional context:

Out of context: In the pull https://github.com/ossf/scorecard/pull/1263/files I'm not sure whether it's that secure to check a single line of if command to trigger skip logic by force asserting *test* matching. Can someone please enlighten me about the following logic; wouldn't it be dangerous from the security perspective? If I put some vulnerable executables in random *test* folders?

if strings.Contains(strings.ToLower(path), "test")
laurentsimon commented 2 years ago

Thanks for the issue.

Why is this needed:

To prevent Binary-Artifacts false positives.

Can you provide a repo example where you're observing too many false positives?

From the discussion #593 started by @developer-guy; @westonsteimel said we probably need some enhancements to consider other names for test directories, as @laurentsimon already proposed an idea to ability to skip given folders in the config.

Additional context:

Out of context: In the pull https://github.com/ossf/scorecard/pull/1263/files I'm not sure whether it's that secure to check a single line of if command to trigger skip logic by force asserting *test* matching. Can someone please enlighten me about the following logic; wouldn't it be dangerous from the security perspective? If I put some vulnerable executables in random *test* folders?

if strings.Contains(strings.ToLower(path), "test")

You're correct there is a risk of false negatives, e.g., if the file is loaded and executed by someone (unit tests). If you're a repo owner, we'd expect you to know what you're doing with the executables (scorecard only helps you flag potential risky practices). One legitimate use case may be fuzzing inputs.

If you're a repo consumer, then it's up to you to decide whether you consider the results acceptable or not. That said, it may be useful for repo owners to declare explicitly (with a description) why it may be acceptable for some directories to not pass certain scorecard checks. A human writing a policy on top of scorecard results may use them as hints to decide what is or is not acceptable (cc @scovetta @david-a-wheeler relevant discussion around security insight work.). The GitHub action we'll release in Jan'22 may be a good place for users to declare which directories are acceptable for their repo and why. We have not worked on this yet, though. We think it may be easier when we close this issue https://github.com/ossf/scorecard/issues/1245 (WIP)

Note this related to https://github.com/ossf/scorecard/issues/1270 A list of acceptable directories may be useful for other checks such as Pinned-Dependencies: currently we're not able to distinguish between external dependencies and internal dependencies (e.g., a docker image that belongs to the same repo and is used without pinning intentionally). The requirement to pin internal dependencies may be acceptable in certain cases.

github-actions[bot] commented 11 months ago

This issue is stale because it has been open for 60 days with no activity.

github-actions[bot] commented 7 months ago

This issue has been marked stale because it has been open for 60 days with no activity.

rdehuyss commented 2 months ago

We would also like to have this to ignore the gradle-wrapper.jar which is required to checkin.