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.

Sign up for GitHub

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

Jump to bottom

CI: Resolve OSSF GitHub token permissions security alerts #2891

Merged
merged 1 commit into from
Jul 2, 2023
Merged

CI: Resolve OSSF GitHub token permissions security alerts #2891

merged 1 commit into from
Jul 2, 2023

Conversation

qwerty541
Copy link
Collaborator

No description provided.

@qwerty541 qwerty541 requested a review from rickstaa June 28, 2023 17:53
@vercel
Copy link

vercel bot commented Jun 28, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
github-readme-stats ✅ Ready (Inspect) Visit Preview Jun 28, 2023 5:53pm

@github-actions github-actions bot added the ci CI related features. label Jun 28, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 28, 2023
View reviewed changes
.github/workflows/generate-theme-doc.yml
permissions:
actions: read
checks: read
contents: write

Check failure

Code scanning / Scorecard

Token-Permissions

score is 0: topLevel 'contents' permission set to 'write' Remediation tip: Visit [https://app.stepsecurity.io/secureworkflow](https://app.stepsecurity.io/secureworkflow//generate-theme-doc.yml/?enable=permissions). Tick the 'Restrict permissions for GITHUB_TOKEN' Untick other options NOTE: If you want to resolve multiple issues at once, you can visit [https://app.stepsecurity.io/securerepo](https://app.stepsecurity.io/securerepo) instead. Click Remediation section below for further remediation help
Copy link
Collaborator

@rickstaa rickstaa Jun 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@anuraghazra, would it be possible to restrict the `GITHUB_TOKEN permissions so we can merge this pull request?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rickstaa This alerts can be dismissed with "Used only in tests" reason. We cannot resolve them because this permissions required for workflows running. I think that if you approve it we can merge it now.

Copy link
Owner

Choose a reason for hiding this comment

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

Not sure whats this about, but approved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rickstaa This alerts can be dismissed with "Used only in tests" reason. We cannot resolve them because this permissions required for workflows running. I think that if you approve it we can merge it now.

Thanks for the explanation, makes sense!

.github/workflows/prs-cache-clean.yml
@@ -4,6 +4,21 @@
types:
- closed

permissions:
actions: write

Check failure

Code scanning / Scorecard

Token-Permissions

score is 0: topLevel 'actions' permission set to 'write' Remediation tip: Visit [https://app.stepsecurity.io/secureworkflow](https://app.stepsecurity.io/secureworkflow//prs-cache-clean.yml/?enable=permissions). Tick the 'Restrict permissions for GITHUB_TOKEN' Untick other options NOTE: If you want to resolve multiple issues at once, you can visit [https://app.stepsecurity.io/securerepo](https://app.stepsecurity.io/securerepo) instead. Click Remediation section below for further remediation help
@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (ee978f3) 97.61% compared to head (8dc86cb) 97.61%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #2891   +/-   ##
=======================================
  Coverage   97.61%   97.61%           
=======================================
  Files          24       24           
  Lines        5156     5156           
  Branches      460      460           
=======================================
  Hits         5033     5033           
  Misses        122      122           
  Partials        1        1

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@qwerty541 qwerty541 changed the title CI: Resolve OSSF GitHub token permissions security alert CI: Resolve OSSF GitHub token permissions security alerts Jun 28, 2023
@rickstaa rickstaa merged commit 888c4ce into master Jul 2, 2023
@rickstaa rickstaa deleted the resolve_ossf_github_token_permissions_alert branch July 2, 2023 07:06
@rickstaa
Copy link
Collaborator

rickstaa commented Jul 2, 2023 •
edited
Loading

@qwerty541 looks like your changes broke the prs-cache-clean.yml action (see https://github.com/anuraghazra/github-readme-stats/actions/runs/5435282842/workflow) 🤔. Very strange since read should be valid for the id-token permission (see https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs). Maybe the indentation is incorrect?

@rickstaa
Copy link
Collaborator

rickstaa commented Jul 2, 2023 •
edited
Loading

It looks like the vscode-github-action plugin also flags this as a syntax error:

image

As the error goes away when I change to write or None maybe this permission can only take these two values and the documentation is incorrect 🤔?

References

@rickstaa
Copy link
Collaborator

rickstaa commented Jul 2, 2023

It looks like all actions with id-token are affected. I opened an issue with github to fix this (see github/docs#26481), but in the meantime, I think we can use none instead of read. I created https://github.com/anuraghazra/github-readme-stats/pull/2903/files to fix this.

@rickstaa
Copy link
Collaborator

rickstaa commented Jul 2, 2023

@qwerty541 as far as I could understand it, we can also solve this by removing the id-token key since all undefined permissions are automatically set to none. I created #2904 for an implicit solution.

J00MZ pushed a commit to J00MZ/github-readme-stats that referenced this pull request Jul 23, 2023
@qwerty541 @J00MZ
CI: Resolve OSSF GitHub token permissions security alert (anuraghazra…
a1b8bb7 
…#2891)
@commit-lint commit-lint bot mentioned this pull request Sep 17, 2023
Open
devantler pushed a commit to devantler/github-readme-stats that referenced this pull request Sep 24, 2023
@qwerty541 @devantler
CI: Resolve OSSF GitHub token permissions security alert (anuraghazra…
da96bb3 
…#2891)
setdebarr pushed a commit to setdebarr/github-readme-stats that referenced this pull request Jan 12, 2024
@qwerty541 @setdebarr
CI: Resolve OSSF GitHub token permissions security alert (anuraghazra…
47da3d8 
…#2891)
@qwerty541 qwerty541 mentioned this pull request Mar 2, 2024
Merged
jacobbexten pushed a commit to jacobbexten/github-readme-stats that referenced this pull request Nov 6, 2024
@qwerty541 @jacobbexten
CI: Resolve OSSF GitHub token permissions security alert (anuraghazra…
08598bf 
…#2891)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anuraghazra anuraghazra anuraghazra approved these changes

@rickstaa rickstaa Awaiting requested review from rickstaa

Assignees
No one assigned
Labels
ci CI related features.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@qwerty541 @rickstaa @anuraghazra