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

use release and tag APIs to enhance imposter commit verification #682

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

spencerschrock
Copy link
Member

This PR does a few things, and is grouped by commit to make review easier. Ignoring the file moves, this PR is closer to +227 -48 in size, which is hopefully more manageable.

bump GitHub library to v65

This is just a simple dependency bump since dependabot doesn't handle Go major versions.

move githubVerifier to its own file

Just a 1:1 movement to keep the files organized

define a commit type

Just switched some arguments to a struct, which isn't too important but helps the next commit which requires storing owner/repo/sha values in a map.

use release and tag APIs to enhance imposter commit verification

The old verification process assumed a commit was always in the default
branch, or came from a select number of hardcoded release branches. This
was error prone whenever new releases branches were used by some of the
actions we check. (see ossf/scorecard-action#1367 (comment))

The commit verification workflow now uses dynamic data to determine
which branches to check. The steps are now:

  1. Check if the commit is one of the latest 100 tags, which should be
    the most common case.
  2. Check the default branch.
  3. Check branches associated with the most recent releases. This removes
    the hardcoded checks and should require fewer updates in the future.

Note

github/codeql-action required some special logic as their releases are all tagged from main, even though the tags come from different release branches. Handling this required parsing GitHub action semantic versions, which I used golang.org/x/mod/semver for.

Package semver implements comparison of semantic version strings. In this package, semantic version strings must begin with a leading "v", as in "v1.0.0".
...
This package follows Semantic Versioning 2.0.0 (see semver.org) with two exceptions. First, it requires the "v" prefix. Second, it recognizes vMAJOR and vMAJOR.MINOR (with no prerelease or build suffixes) as shorthands for vMAJOR.0.0 and vMAJOR.MINOR.0.

Both of these assumptions currently match the versioning used by the codeql-action, so the fact that this is a go.mod parsing package shouldn't matter (but feel free to comment if you disagree).

Signed-off-by: Spencer Schrock <[email protected]>
this helps us use a map later.

Signed-off-by: Spencer Schrock <[email protected]>
The old verification process assumed a commit was always in the default
branch, or came from a select number of hardcoded release branches. This
was error prone whenever new releases branches were used by some of the
actions we check.

The commit verification workflow now uses dynamic data to determine
which branches to check. The steps are now:
1. Check if the commit is one of the latest 100 tags, which should be
   the most common case.
2. Check the default branch (unchanged from before).
3. Check branches associated with the most recent releases. This removes
   the hardcoded checks and should require fewer updates in the future.

Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock requested review from a team, naveensrinivasan and raghavkaul and removed request for a team September 24, 2024 21:27
Copy link

netlify bot commented Sep 24, 2024

Deploy Preview for ossf-scorecard canceled.

Name Link
🔨 Latest commit c67d706
🔍 Latest deploy log https://app.netlify.com/sites/ossf-scorecard/deploys/66f32ee217796300084c0a9a

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.

1 participant