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

Replace check gradle checksums with fetched script #488

Merged
merged 5 commits into from
Nov 13, 2019

Conversation

ZacSweers
Copy link
Contributor

I extracted this script out to a library with an easy install, proposing adding this here since it's more up to date. Idea is to make this more portable and usable in other repos

@nedtwigg
Copy link
Member

Thanks! I think that centralizing the trust model for gradle wrappers is a great idea, and I'd like for Spotless to help and be one of your pilot projects. As committed, we're trusting that the master branch of your personal repo doesn't get compromised. Can we instead delegate our trust to a specific hash in your repo?

I'm also curious what @JLLeitschuh thinks. My memory is murky, but I believe he tried to get GitHub to do something like this in their vulnerability scanner, and he has since joined gradle directly, so he might know of officially supported plans.

@ZacSweers
Copy link
Contributor Author

Can we instead delegate our trust to a specific hash in your repo?

Do you mean a specific sha? Or a different kind of hash?

@nedtwigg
Copy link
Member

Yep, specific sha.

@ZacSweers
Copy link
Contributor Author

ZacSweers commented Nov 12, 2019

Yep can do, would just be something like this https://raw.githubusercontent.com/ZacSweers/check-gradle-checksums/c8dc2ae0756a8041e240cdc6fa6c38c256dfeab0/check-gradle-checksums.sh. Will update with this

.travis.yml Outdated Show resolved Hide resolved
@ZacSweers ZacSweers changed the title (Proposal) Replace check gradle checksums with fetched script Replace check gradle checksums with fetched script Nov 12, 2019
.travis.yml Outdated Show resolved Hide resolved
@nedtwigg
Copy link
Member

Although I'm curious what @JLLeitschuh has to say, I also don't see any downside to merging this now, so in it goes. Feel free to list us as a user / example config / whatever you'd like.

Because this script is running on a CI server, it would be a great place for a hacker to exfiltrate all our publishing secrets, which is the nightmare scenario. Imo, especially because it is run by an individual rather than a first-party like Gradle, it would be unwise to rely on anything besides the hash, so if it were me I would make that the default usecase in your readme, but that's your call.

Thanks!

@nedtwigg nedtwigg merged commit d4b9b88 into diffplug:master Nov 13, 2019
@JLLeitschuh
Copy link
Member

I'm wondering whether or not we (Gradle) should be hosting this instead.

I agree that tying yourself to a specific SHA is better.

@ZacSweers
Copy link
Contributor Author

ZacSweers commented Nov 13, 2019 via email

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 this pull request may close these issues.

3 participants