Skip to content
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

add ability to pass ignore list #1406

Open
Dentrax opened this issue Dec 21, 2021 · 4 comments
Open

add ability to pass ignore list #1406

Dentrax opened this issue Dec 21, 2021 · 4 comments
Labels
kind/enhancement New feature or request Stale

Comments

@Dentrax
Copy link
Contributor

Dentrax commented Dec 21, 2021

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")
@Dentrax Dentrax added the kind/enhancement New feature or request label Dec 21, 2021
@laurentsimon
Copy link
Contributor

laurentsimon commented Dec 21, 2021

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 #1245 (WIP)

Note this related to #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.

Copy link

github-actions bot commented Nov 3, 2023

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

Copy link

github-actions bot commented Mar 8, 2024

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

@github-actions github-actions bot added the Stale label Mar 8, 2024
@rdehuyss
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request Stale
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

4 participants