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 known wrapper checksums to avoid network requests #167

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions .github/workflows/update-checksums-file.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Was thinking about writing this myself and wasn't looking forward to it. Solid bit of work here!

Javascript is definitely not my strongest area, ngl. 😆

Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Updates the `wrapper-checksums.json` file
*
* This is intended to be executed by the GitHub workflow, but can also be run
* manually.
*/

// @ts-check

const httpm = require('typed-rest-client/HttpClient')

const path = require('path')
const fs = require('fs')

/**
* @returns {Promise<void>}
*/
async function main() {
const httpc = new httpm.HttpClient(
'gradle/wrapper-validation-action/update-checksums-workflow',
undefined,
{allowRetries: true, maxRetries: 3}
)

/**
* @param {string} url
* @returns {Promise<string>}
*/
async function httpGetText(url) {
const response = await httpc.get(url)
return await response.readBody()
}

/**
* @typedef {Object} ApiVersionEntry
* @property {string} version - version name
* @property {string=} wrapperChecksumUrl - wrapper checksum URL; not present for old versions
* @property {boolean} snapshot - whether this is a snapshot version
*/

/**
* @returns {Promise<ApiVersionEntry[]>}
*/
async function httpGetVersions() {
return JSON.parse(
await httpGetText('https://services.gradle.org/versions/all')
)
}

const versions = (await httpGetVersions())
// Only include versions with checksum
.filter(e => e.wrapperChecksumUrl !== undefined)
// Ignore snapshots; they are changing frequently so no point in including them in checksums file
.filter(e => !e.snapshot)
console.info(`Got ${versions.length} relevant Gradle versions`)

// Note: For simplicity don't sort the entries but keep the order from the API; this also has the
// advantage that the latest versions come first, so compared to appending versions at the end
// this will not cause redundant Git diff due to trailing `,` being forbidden by JSON

/**
* @typedef {Object} FileVersionEntry
* @property {string} version
* @property {string} checksum
*/
/** @type {FileVersionEntry[]} */
const fileVersions = []
for (const entry of versions) {
/** @type {string} */
// @ts-ignore
const checksumUrl = entry.wrapperChecksumUrl
const checksum = await httpGetText(checksumUrl)
fileVersions.push({version: entry.version, checksum})
}

const jsonPath = path.resolve(
__dirname,
'..',
'..',
'src',
'wrapper-checksums.json'
)
console.info(`Writing checksums file to ${jsonPath}`)
// Write pretty-printed JSON (and add trailing line break)
fs.writeFileSync(jsonPath, JSON.stringify(fileVersions, null, 2) + '\n')
}

main().catch(e => {
console.error(e)
// Manually set error exit code, otherwise error is logged but script exits successfully
process.exitCode = 1
})
49 changes: 49 additions & 0 deletions .github/workflows/update-checksums-file.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: 'Update Wrapper checksums file'

on:
schedule:
# Run weekly (at arbitrary time)
- cron: '24 5 * * 6'
# Support running workflow manually
workflow_dispatch:

jobs:
update-checksums:
name: Update checksums
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: 20.x
cache: npm

- name: Install dependencies
run: |
npm install [email protected] --no-save

- name: Update checksums file
run: node ./.github/workflows/update-checksums-file.js

# If there are no changes, this action will not create a pull request
- name: Create or update pull request
uses: peter-evans/create-pull-request@v6
with:
branch: wrapper-checksums-update
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the branch name should use a prefix such as bot/ to prevent accidentally manually editing this branch, which might then be overwritten by this workflow.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds completely reasonable to prefix it with bot/

commit-message: Update known wrapper checksums
title: Update known wrapper checksums
# Note: Unfortunately this action cannot trigger the regular workflows for the PR automatically, see
# https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#triggering-further-workflow-runs
# Therefore suggest below to close and then reopen the PR
body: |
Automatically generated pull request to update the known wrapper checksums.

In case of conflicts, manually run the workflow from the [Actions tab](https://github.com/gradle/wrapper-validation-action/actions/workflows/update-checksums-file.yml), the changes will then be force-pushed onto this pull request branch.
Do not manually update the pull request branch; those changes might get overwritten.

> [!IMPORTANT]
> GitHub workflows have not been executed for this pull request yet. Before merging, close and then directly reopen this pull request to trigger the workflows.
4 changes: 2 additions & 2 deletions __tests__/checksums.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ jest.setTimeout(30000)

test('fetches wrapper jars checksums', async () => {
const validChecksums = await checksums.fetchValidChecksums(false)
expect(validChecksums.length).toBeGreaterThan(10)
expect(validChecksums.size).toBeGreaterThan(10)
})

describe('retry', () => {
Expand All @@ -25,7 +25,7 @@ describe('retry', () => {
})

const validChecksums = await checksums.fetchValidChecksums(false)
expect(validChecksums.length).toBeGreaterThan(10)
expect(validChecksums.size).toBeGreaterThan(10)
nock.isDone()
})
})
Expand Down
24 changes: 24 additions & 0 deletions __tests__/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,30 @@ test('succeeds if all found wrapper jars are valid', async () => {
])

expect(result.isValid()).toBe(true)
// Only hardcoded and explicitly allowed checksums should have been used
expect(result.fetchedChecksums).toBe(false)

expect(result.toDisplayString()).toBe(
'✓ Found known Gradle Wrapper JAR files:\n' +
' e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 __tests__/data/invalid/gradle-wrapper.jar\n' +
' e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 __tests__/data/invalid/gradlе-wrapper.jar\n' + // homoglyph
' 3888c76faa032ea8394b8a54e04ce2227ab1f4be64f65d450f8509fe112d38ce __tests__/data/valid/gradle-wrapper.jar'
)
})

test('succeeds if all found wrapper jars are valid (and checksums are fetched from Gradle API)', async () => {
const knownValidChecksums = new Map<string, Set<string>>()
const result = await validate.findInvalidWrapperJars(
baseDir,
1,
false,
['e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'],
knownValidChecksums
)

expect(result.isValid()).toBe(true)
// Should have fetched checksums because no known checksums were provided
expect(result.fetchedChecksums).toBe(true)

expect(result.toDisplayString()).toBe(
'✓ Found known Gradle Wrapper JAR files:\n' +
Expand Down
30 changes: 28 additions & 2 deletions src/checksums.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,40 @@
import * as httpm from 'typed-rest-client/HttpClient'

import fileWrapperChecksums from './wrapper-checksums.json'

const httpc = new httpm.HttpClient(
'gradle/wrapper-validation-action',
undefined,
{allowRetries: true, maxRetries: 3}
)

function getKnownValidChecksums(): Map<string, Set<string>> {
const versionsMap = new Map<string, Set<string>>()
for (const entry of fileWrapperChecksums) {
const checksum = entry.checksum

let versionNames = versionsMap.get(checksum)
if (versionNames === undefined) {
versionNames = new Set()
versionsMap.set(checksum, versionNames)
}

versionNames.add(entry.version)
}

return versionsMap
}

/**
* Known checksums from previously published Wrapper versions.
*
* Maps from the checksum to the names of the Gradle versions whose wrapper has this checksum.
*/
export const KNOWN_VALID_CHECKSUMS = getKnownValidChecksums()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a unit test that verifies that this is never empty, and always contains some reasonable versions as a spot-check that this logic never breaks?


export async function fetchValidChecksums(
allowSnapshots: boolean
): Promise<string[]> {
): Promise<Set<string>> {
const all = await httpGetJsonArray('https://services.gradle.org/versions/all')
const withChecksum = all.filter(
entry =>
Expand All @@ -27,7 +53,7 @@ export async function fetchValidChecksums(
const checksums = await Promise.all(
checksumUrls.map(async (url: string) => httpGetText(url))
)
return [...new Set(checksums)]
return new Set(checksums)
}

async function httpGetJsonArray(url: string): Promise<unknown[]> {
Expand Down
31 changes: 25 additions & 6 deletions src/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ export async function findInvalidWrapperJars(
gitRepoRoot: string,
minWrapperCount: number,
allowSnapshots: boolean,
allowChecksums: string[]
allowedChecksums: string[],
knownValidChecksums: Map<
string,
Set<string>
> = checksums.KNOWN_VALID_CHECKSUMS
): Promise<ValidationResult> {
const wrapperJars = await find.findWrapperJars(gitRepoRoot)
const result = new ValidationResult([], [])
Expand All @@ -16,14 +20,28 @@ export async function findInvalidWrapperJars(
)
}
if (wrapperJars.length > 0) {
const validChecksums = await checksums.fetchValidChecksums(allowSnapshots)
validChecksums.push(...allowChecksums)
const notYetValidatedWrappers = []
for (const wrapperJar of wrapperJars) {
const sha = await hash.sha256File(wrapperJar)
if (!validChecksums.includes(sha)) {
result.invalid.push(new WrapperJar(wrapperJar, sha))
} else {
if (allowedChecksums.includes(sha) || knownValidChecksums.has(sha)) {
result.valid.push(new WrapperJar(wrapperJar, sha))
} else {
notYetValidatedWrappers.push(new WrapperJar(wrapperJar, sha))
}
}

// Otherwise fall back to fetching checksums from Gradle API and compare against them
if (notYetValidatedWrappers.length > 0) {
result.fetchedChecksums = true
const fetchedValidChecksums =
await checksums.fetchValidChecksums(allowSnapshots)

for (const wrapperJar of notYetValidatedWrappers) {
if (!fetchedValidChecksums.has(wrapperJar.checksum)) {
result.invalid.push(wrapperJar)
} else {
result.valid.push(wrapperJar)
}
}
}
}
Expand All @@ -33,6 +51,7 @@ export async function findInvalidWrapperJars(
export class ValidationResult {
valid: WrapperJar[]
invalid: WrapperJar[]
fetchedChecksums = false
errors: string[] = []

constructor(valid: WrapperJar[], invalid: WrapperJar[]) {
Expand Down
Loading