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

✨ Token-Permissions, Allow contents: write permission only for jobs that are releasing #1663

Merged
merged 3 commits into from
Feb 23, 2022

Conversation

ristomcgehee
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Feature

  • What is the current behavior? (You can also link to an open issue here)
    If a GitHub workflow job has contents: write permission and it is a publishing job, it will not have its score reduced.
    Feature: differentiate between package and contents permission in Token-Permission  #1254

  • What is the new behavior (if this is a feature change)?
    If a GitHub workflow job has contents: write permission and it is a publishing job, it will have its score reduced. If a GitHub workflow job has contents: write permission and it is a releasing job, it will not have its score reduced.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    No

  • Other information:
    I looked through all of isPackagingWorkflow and determined the only ones that looked like they needed contents: write were goreleaser and python-semantic-release. I left goreleaser in isPackagingWorkflow so as not to alter the behavior of the Packaging check, even though that action does not need the packages: write permission. python-semantic-release requires both packages: write and contents: write.

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2022

Codecov Report

Merging #1663 (7f320c2) into main (e41f859) will increase coverage by 5.82%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##             main    #1663      +/-   ##
==========================================
+ Coverage   55.86%   61.69%   +5.82%     
==========================================
  Files          72       58      -14     
  Lines        6508     5801     -707     
==========================================
- Hits         3636     3579      -57     
+ Misses       2618     1979     -639     
+ Partials      254      243      -11     

@github-actions
Copy link

Integration tests success for
[5967ad5]
(https://github.com/ossf/scorecard/actions/runs/1869440122)

Allowing `contents: write` permission only for jobs that are releasing
jobs, not just packaging jobs.
@ristomcgehee ristomcgehee temporarily deployed to integration-test February 19, 2022 17:20 Inactive
@github-actions
Copy link

Integration tests success for
[577a8fd]
(https://github.com/ossf/scorecard/actions/runs/1869459377)

Copy link
Member

@naveensrinivasan naveensrinivasan left a comment

Choose a reason for hiding this comment

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

Thanks

checks/fileparser/github_workflow.go Show resolved Hide resolved
}

// matches returns true if the job matches the job matcher.
func (m *JobMatcher) matches(job *actionlint.Job) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment , should this be a job *actionlint.Job pointer?

Copy link
Contributor

@azeemshaikh38 azeemshaikh38 left a comment

Choose a reason for hiding this comment

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

Thanks Chris!

Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

Thanks Chris!

@justaugustus justaugustus enabled auto-merge (squash) February 23, 2022 00:13
@justaugustus justaugustus temporarily deployed to integration-test February 23, 2022 00:14 Inactive
@github-actions
Copy link

Integration tests success for
[edd59ff]
(https://github.com/ossf/scorecard/actions/runs/1884665674)

@github-actions
Copy link

Integration tests success for
[7f320c2]
(https://github.com/ossf/scorecard/actions/runs/1884668490)

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.

5 participants