on.<push|pull_request>.paths filter to trigger validation only on sensitive pushes

[vlsi]

https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#onpushpull_requestpaths

Even though wrapper-validation-action is fast, it does produce lots of irrelevant logs (e.g. in GitHub Actions UI) for each and every pull request.

Have you considered to use on.<push|pull_request>.paths to reduce the scope of the validator triggering?

[JLLeitschuh]

Have thought about it, but there are many cases where the gradle-wrapper.jar doesn't live in [PROJECT_HOME]/gradle/gradle-wrapper.jar. For example, there are two gradle-wrapper.jar files in this repository.

https://github.com/jlleitschuh/ktlint-gradle

You are welcome to add paths filter triggers if that works better for your use case, but in general, the best security here will be a job that always runs.

[vlsi]

The following should cover all the cases:

paths:
- **/gradle-wrapper.jar

What I mean is even though GitHub Actions is free, it is not very wise to use that resource for each and every case. Even though I understand security is important, I think it would be sane to confine pull request executions for **/gradle-wrapper.jar only.

It might be the same for branches as well. It is just enough to trigger check if **/gradle-wrapper.jar is modified.

Note: AFAIK wrapper-validation-action is meant to be activated for every repository that uses Gradle, which might take noticeable resources worldwide.

[JLLeitschuh]

Pulling from the readme:

Additionally, the action will find and SHA-256 hash all homoglyph variants of files named gradle-wrapper.jar, for example a file named gradlе-wrapper.jar (which uses a Cyrillic е instead of e). The goal is to prevent homoglyph attacks which may be very difficult to spot in a GitHub diff.

If you use a wildcard like that **/gradle-wrapper.jar you won't catch homoglyph attacks.

[vlsi]

What's the point in homoglyphs?

[JLLeitschuh]

It's an attack that would be very difficult to detect via visual inspection.

Here's a demo/example of a homoglyph attack update PR. https://github.com/JLLeitschuh/playframework/pull/1/files

[vlsi]

Here's a demo/example of a homoglyph attack update PR. https://github.com/JLLeitschuh/playframework/pull/1/files

Is **/gradle-wrapper.jar impacted there? If it is impacted, then Actions will be triggered. If the file is not impacted, then Actions won't be triggered.

I assume that Gradle uses file with the exact file name. That is why if **/gradle-wrapper.jar is not impacted, then the contents of the file is irrelevant as the file is not going to be used by Gradle.

[vlsi]

However, gradlew and gradlew.bat might indeed be complicated :-/

[JLLeitschuh]

I assume that Gradle uses file with the exact file name.

Notice how the attack changes the gradlew and gradle.bat scripts to use the updated file name instead. Gradle doesn't use the gradle-wrapper.jar with the exact names because the wrapper scripts are the things launching the wrapper.

[vlsi]

Just in case: what I mean by gradlew.bat might indeed be complicated is that gradlew itself might be spelled with weird characters. It would be hard to notice in case of add Gradle scripts commit.

Do you think there should be another check that verifies just homoglyphs in file names?

What I mean is it might be generally unexpected to have weird file names (depending on the definition on weird).

[JLLeitschuh]

What I mean is it might be generally unexpected to have weird file names (depending on the definition on weird).

I somewhat describe this here, but maybe I should do a better job. https://github.com/gradle/wrapper-validation-action/blob/master/CONTRIBUTING.md

When we scoped this work, we wanted to specifically focus on the gradle-wrapper.jar since it's a binary blob of data that GitHub doesn't render diffs of. We consider files you can read 'out-of-scope' for this project. We make the one exception for the homoglyph attack that is demonstrated in my POC since it's easy hard to miss.

The general assumption that we make about gradlew and gradle.bat files are that since they are human-readable, someone trying to sneak something malicious into those files is far more likely (but not guaranteed) to be spotted.

[vlsi]

The general assumption that we make about gradlew and gradle.bat files are that since they are human-readable

Well. gradlew (with cyrrilic а) It might call grade-wraper.jar Have you noticed single p?

I guess gradlew scripts can still be checksum validated.

[JLLeitschuh]

I guess gradlew scripts can still be checksum validated.

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://github.com/gradle/gradle/blob/298ac44389e242630d4787cf88e9a6044069b7e2/buildSrc/subprojects/versioning/src/main/kotlin/org/gradle/gradlebuild/versioning/WrapperPlugin.kt#L34-L42

We also don't publish the checksums for the gradlew and gradle.bat files currently.

[mockitoguy]

@JLLeitschuh, it would great if the Gradle team provided crisp recommendation on path filtering, and explicitly documented it in the action's documentation.

BTW. This action is great, thank you for sharing!

[JLLeitschuh]

We don't have a recommendation here. For the best security, don't enable path filtering. But you understand your project best.