wrapper-validation-action icon indicating copy to clipboard operation
wrapper-validation-action copied to clipboard

Validate gradlew scripts

Open di72nn opened this issue 4 years ago • 7 comments

Also validate gradlew and gradlew.bat scripts. These are not blobs so it is harder to overlook something, but I think these should be checked too.

di72nn avatar Feb 28 '20 12:02 di72nn

That doesn't always really work. Some organizations modify their gradlew scripts when updating them in order to, for example, increase the memory the wrapper is given. When doing that, the checksum can't be validated.

https://gradle/gradle:buildSrc/subprojects/versioning/src/main/kotlin/org/gradle/gradlebuild/versioning/WrapperPlugin.kt@298ac44#L34-L42

JLLeitschuh avatar Mar 03 '20 03:03 JLLeitschuh

That's a fair point, but I still think that there should be an option (or another step in the workflow) to check the scripts for those who don't need to modify them.

di72nn avatar Mar 03 '20 09:03 di72nn

@eskatos how difficult would it be to also generate SHA checksums for the gradlew and gradle.bat files? One problem that you'd also run into here is that, unless you had your .gitattributes (for example) file setup correctly, depending upon the OS that created the repository, you may have different line endings (windows vs unix line endings) in the file, thus causing the SHA checksums to be different.

JLLeitschuh avatar Mar 03 '20 17:03 JLLeitschuh

@JLLeitschuh it shouldn't be difficult. But it'll require work to automate generating the hashes, publishing them where appropriate etc.. You could have a look at how the wrapper jar hashes are handled, it should be similar both in where and how it's done. Then we could update this action to consume them for more validation.

As said above, I wonder what the signal/noise ratio would be given the line endings potential problem and the fact it is intended that people modify it if they need to e.g. change the Gradle client VM options.

eskatos avatar Mar 04 '20 09:03 eskatos

I would vote for an opt-out of the additional check (in the sense of security first).

robstoll avatar Apr 25 '20 23:04 robstoll

As said above, I wonder what the signal/noise ratio would be given the line endings potential problem and the fact it is intended that people modify it if they need to e.g. change the Gradle client VM options.

  1. The verifier could normalize line endings (gradlew to LF and gradlew.bat to CRLF) before doing the comparison (it might make sense to normalize to LF always).

  2. Frankly speaking, I have not seen a case when users customized gradlew scripts. I assume an opt-out would really help here: the extra skip-gradlew-scripts-validation: true would make it explicit that the files are augmented on purpose.

vlsi avatar Mar 17 '21 10:03 vlsi

I also agree. While some users may modify their scripts, I have not personally seen it before. It would be incredibly nice to have in order to verify the integrity of the scripts. An opt out is a fine solution in my opinion. And I think normalization of line endings would be the way to go as already mentioned.

tlf30 avatar Feb 06 '23 20:02 tlf30