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

chore(ci): address overly broad permissions scopes #16996

Merged
merged 3 commits into from
Oct 31, 2024

Conversation

miketheman
Copy link
Member

  • inform checkout to not persist credentials
  • move permissions from workflow to job scope

Thanks, Dr. Zizmor!
Refs: https://github.com/woodruffw/zizmor

@miketheman miketheman requested a review from a team as a code owner October 29, 2024 16:32
@miketheman miketheman added testing Test infrastructure and individual tests security Security-related issues and pull requests labels Oct 29, 2024
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM! 🌈

.github/workflows/ci.yml Outdated Show resolved Hide resolved
- inform checkout to not persist credentials
- move permissions from workflow to job scope

Thanks, Dr. Zizmor!
Refs: https://github.com/woodruffw/zizmor

Signed-off-by: Mike Fiedler <[email protected]>
@miketheman miketheman force-pushed the miketheman/gha-audit branch from 5f476d4 to b2f9529 Compare October 30, 2024 21:22
Copy link
Member

@ewdurbin ewdurbin left a comment

Choose a reason for hiding this comment

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

looks like excellent best practices work. assuming this was based on the output of a tool, can the tool be included in our linting to make sure we don’t regress and that new workflows and permissions are similarly well scoped?

@woodruffw
Copy link
Member

looks like excellent best practices work. assuming this was based on the output of a tool, can the tool be included in our linting to make sure we don’t regress and that new workflows and permissions are similarly well scoped?

Yep! The easiest thing to do might be to copy this workflow, which uses the current SARIF support to integrate zizmor's results directly into GitHub's security tool/reporting infrastructure: https://github.com/woodruffw/zizmor/blob/main/.github/workflows/zizmor.yml

(The other option would be to just run zizmor in CI, and manually triage the results it produces.)

@woodruffw
Copy link
Member

For ref, this is what the SARIF integration looks like (it's currently barebones, but will get better over time):

Screenshot 2024-10-30 at 5 34 50 PM

Signed-off-by: Mike Fiedler <[email protected]>
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@miketheman
Copy link
Member Author

can the tool be included in our linting to make sure we don’t regress and that new workflows and permissions are similarly well scoped?

Done in 311777b

@miketheman miketheman merged commit 7738439 into pypi:main Oct 31, 2024
20 checks passed
@miketheman miketheman deleted the miketheman/gha-audit branch October 31, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Security-related issues and pull requests testing Test infrastructure and individual tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants