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

✨ Add GitHub git compatibility mode #4474

Merged
merged 12 commits into from
Feb 12, 2025

Conversation

spencerschrock
Copy link
Member

@spencerschrock spencerschrock commented Dec 31, 2024

What kind of change does this PR introduce?

feature

What is the current behavior?

All GitHub repo files are fetched using the export archive tarball

What is the new behavior (if this is a feature change)?**

  • There is a new git mode for maximum compatibility (which avoids the issue of export-ignore).
    • This is enabled from the CLI with a new --file-mode flag. e.g. --file-mode git
    • This is enabled programmatically with the WithFileModeGit() option when running scorecard.Run or creating a GitHub RepoClient, which can be done from Scorecard Action as well. How we expose that to users will be part of a different PR in that repository.
  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Related to #2489

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

Added a flag to fetch repository files via Git. This produces the most accurate results for repositories with .gitattributes files at the cost of analysis speed.

This is primarily aimed at helping in cases where a repository's
.gitattributes file causes files to not be analyzed.

Signed-off-by: Spencer Schrock <[email protected]>
This will let us use the new entrypoint in a backwards compatible way,
similar to the scorecard.Run change made in the v5 release.

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Copy link

codecov bot commented Dec 31, 2024

Codecov Report

Attention: Patch coverage is 38.69347% with 122 lines in your changes missing coverage. Please review.

Project coverage is 68.19%. Comparing base (353ed60) to head (bbeb280).
Report is 113 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4474      +/-   ##
==========================================
+ Coverage   66.80%   68.19%   +1.39%     
==========================================
  Files         230      247      +17     
  Lines       16602    18622    +2020     
==========================================
+ Hits        11091    12700    +1609     
- Misses       4808     5080     +272     
- Partials      703      842     +139     

@spencerschrock
Copy link
Member Author

spencerschrock commented Dec 31, 2024

Thoughts on the CLI flag? It's not GitHub specific, and can be added for the GitLab client in a future PR

Flags:
      # ...
      --git-mode           fetch repository files using git for maximum compatibility
go run main.go --repo kokkos/kokkos --checks Dangerous-Workflow --git-mode

And then the Scorecard Action would have a corresponding input:

      - name: "Run analysis"
        uses: ossf/scorecard-action@62b2cac7ed8198b15735ed49ab1e5cf35480ba46 # v2.4.0
        with:
          results_file: results.sarif
          results_format: sarif
          publish_results: true
          git_compatibility_mode: true

or it could be a selection of options if we think we'll add a local file option (for a repo you already have cloned for example):

      - name: "Run analysis"
        uses: ossf/scorecard-action@62b2cac7ed8198b15735ed49ab1e5cf35480ba46 # v2.4.0
        with:
          results_file: results.sarif
          results_format: sarif
          publish_results: true
          file_mode:  git # one of [archive, git, local]

EDIT: With a similar file mode flag, the CLI would be:

Flags:
      # ...
      --file-mode string   mode to fetch repository files: archive, git (default "archive")

export-ignore is not a github specific feature, and other forges, like
gitlab, suffer from the same bug.

Signed-off-by: Spencer Schrock <[email protected]>
This will allow sharing with GitLab in a followup PR

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
also moves a func around for smaller PR diff.

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock
Copy link
Member Author

/scdiff generate Binary-Artifacts,Dangerous-Workflow,Dependency-Update-Tool,Fuzzing,License,Packaging,Pinned-Dependencies,SAST,Security-Policy,Token-Permissions,Vulnerabilities

Copy link

github-actions bot commented Jan 2, 2025

@spencerschrock spencerschrock marked this pull request as ready for review January 2, 2025 18:21
@spencerschrock spencerschrock requested a review from a team as a code owner January 2, 2025 18:21
@spencerschrock spencerschrock requested review from justaugustus and raghavkaul and removed request for a team January 2, 2025 18:21
@masterleinad
Copy link

I like a file_mode option the best for the Scorecard Action and would make git-mode also take a similar argument.

Signed-off-by: Spencer Schrock <[email protected]>
Copy link

This pull request has been marked stale because it has been open for 10 days with no activity

@github-actions github-actions bot added Stale and removed Stale labels Jan 20, 2025
@JBludau
Copy link

JBludau commented Jan 27, 2025

We would really like to see this merged. We think it is good practice to not export the workflows of your repository in the tarball. Furthermore, this would allow users to check a repo depending on the way of downloading/cloneing it.

@spencerschrock
Copy link
Member Author

@justaugustus any time to look at this while Raghav is OOO?

options/options.go Show resolved Hide resolved
options/options_test.go Outdated Show resolved Hide resolved
options/options_test.go Outdated Show resolved Hide resolved
internal/gitfile/gitfile.go Show resolved Hide resolved
the value isn't used to connect to anything though.

Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock merged commit b0143fc into ossf:main Feb 12, 2025
37 of 38 checks passed
@spencerschrock spencerschrock deleted the git-checkout branch February 12, 2025 18:34
@JBludau
Copy link

JBludau commented Feb 12, 2025

Thanks @spencerschrock we greatly appreciate it. Is there a plan for when to expose this in the github action?

@spencerschrock
Copy link
Member Author

The plan is to release v5.1.0 here in the next day or so, and then v2.5.0 of scorecard action next week.

spencerschrock added a commit to spencerschrock/scorecard-action that referenced this pull request Feb 12, 2025
spencerschrock added a commit to spencerschrock/scorecard-action that referenced this pull request Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants