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

Binary Artifact detection gradle-wrapper.jar Incorrect logic #2357

Closed
anantshri opened this issue Oct 16, 2022 · 7 comments · Fixed by #4097
Closed

Binary Artifact detection gradle-wrapper.jar Incorrect logic #2357

anantshri opened this issue Oct 16, 2022 · 7 comments · Fixed by #4097
Labels
check/Binary-Artifacts kind/bug Something isn't working

Comments

@anantshri
Copy link

This is hilarious bug, took me a few tries to get to the exact issue.

  1. gradle-wrapper.jar is detected as a binary-artifact
  2. suggested recommendation is to do a wrapper validation Binary Artifacts should allow well-known artifacts, if best practices are followed #1815
  3. even with wrapper validation script there was error

The issue gets closed only when following conditions are met.

  1. gradle/wrapper-validation-action@v<version_number> is specified
  2. The configuration is in an independent file.

The issue is not closed in following conditions.
2. Config is in part of other files
3. gradle/wrapper-validation-action@<hash> is specified : Hash pinning which is the recommended action by scorecard and gets raised if my action doesnt have hashes is raised and if we fix that binary artifact issue reappears

I am no golang expert but if i am looking at the right file then there is a very specific @v condition check in this code.

https://github.com/ossf/scorecard/blob/main/checks/raw/binary_artifact.go#L36

@anantshri anantshri added the kind/bug Something isn't working label Oct 16, 2022
@naveensrinivasan
Copy link
Member

Thanks, @anantshri. Would you like to send a PR to solve this issue?

@anantshri
Copy link
Author

Like i said not good at programming. wont want to commit on doing something which i dont understand.
Thats why i waited to raise the bug to ensure I have checked as many conditions as possible.

@big-andy-coates
Copy link

big-andy-coates commented Dec 18, 2022

Also affecting all my Gradle based repos. :'(

I don't think its as simple as removing the v from the regex as there is also a Gradle version check, which would require resolving the version hash to a semantic version number. The fix is therefore beyond my Go knowledge.

https://github.com/ossf/scorecard/blob/main/checks/raw/binary_artifact.go#L37

@AdamKorcz
Copy link
Contributor

Is the solution here to verify that it is a digest after the @?

@spencerschrock
Copy link
Member

Is the solution here to verify that it is a digest after the @?

The issue is the regex accepts a version or commit SHA:

`^gradle\/wrapper-validation-action@v?(.+)$`

But the later on we always try to parse it as a version. This check was added here, but I don't see the significance of it.

I think it was this comment:

is it enough to have the gradle validation action or must it be the "latest version" of the action (is this too expensive to calculate?)

@AdamKorcz
Copy link
Contributor

What is the expected behaviour? Ie: Should the check pass with either version or digest?

@anantshri
Copy link
Author

Ideally it should only pass with digest coz overall guidance is not to have version tags. but if we want to be atomic in nature then it should pass for both version or digest. As there is another check which fails if it sees version tags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
check/Binary-Artifacts kind/bug Something isn't working
Projects
Status: Done
5 participants