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

Hardcode list of known checksums to avoid network requests in most cases #161

Closed
Marcono1234 opened this issue Nov 7, 2023 · 4 comments
Closed
Milestone

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Nov 7, 2023

This goes in the same direction as #96; was also suggested in #57 (comment)

What do you think about hardcoding the list of all known Gradle wrapper checksums into this action? Then during validation, the action first checks if the checksum is in that hardcoded list, or is specified in allow-checksums. And only if both checks fail it sends network requests to fetch the checksums.

This would hopefully have the following advantages:

  • Action is more efficient and reduces network traffic for Gradle site
  • Fewer spurious action failures due to network issues

It would not be an issue if the hardcoded list of checksums becomes outdated (possibly by months or even years); it probably still provides an advantage for the majority of the users, assuming they use older wrapper versions.

Alternative

Tell users to use the allow-checksums input. However, that risks reducing security because it might then become a usual action to update the allow-checksums input when updating the wrapper, and maintainers might then not properly verify that the allow-checksums is actually one of the officially listed ones. This allows a malicious user to simply specify the checksum of their malicious wrapper, and claim it is the official checksum, hoping the maintainer does not verify it.

@JLLeitschuh
Copy link
Contributor

This is a solid idea, and absolutely worth doing. I poked at this a bit this afternoon, but I don't have the time right now to fully implement it.

@JLLeitschuh
Copy link
Contributor

@big-guy @bigdaz thoughts? Would you accept a pull request that added this?

@bigdaz
Copy link
Member

bigdaz commented Nov 9, 2023

I think it's a good idea. I always thought it was a bit brain-dead that the action fetched all of the checksums every time.
An alternative implementation could be to use GitHub Actions caching to save the values that have been fetched dynamically.

@Marcono1234
Copy link
Contributor Author

@JLLeitschuh, I have created an initial implementation for this in #167. I hope that is ok; but if you were already working on a solution for this, feel free to close my PR, and optionally use parts from it if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants