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

CI: defining minimal workflow permissions #1294

Closed
diogoteles08 opened this issue Jun 15, 2023 · 2 comments · Fixed by #1295
Closed

CI: defining minimal workflow permissions #1294

diogoteles08 opened this issue Jun 15, 2023 · 2 comments · Fixed by #1295

Comments

@diogoteles08
Copy link
Contributor

Hi!

I'm here to suggest that you set minimal permissions to your workflow go.yml, because currently it doesn't specify the permissions for its jobs and their privileges are being determined by GitHub's defaults.

As the change would be really simple and this very similar PR was already merged, I'll take the liberty and already open a PR with the suggested changes 😊

Context

I'm Diogo and I work on Google's Open Source Security Team(GOSST) in cooperation with the Open Source Security Foundation (OpenSSF). My core job is to suggest and implement security changes on widely used open source projects 😊

@diogoteles08
Copy link
Contributor Author

diogoteles08 commented Sep 8, 2023

Hey @bwplotka, I noticed that one of the changes made by the PR #1180 was reverted, and now one of your workflows lack the minimal permissions. I'm getting back to this issue to expose that and try to help understand what happened.

The change was made on this commit, which seems to be automated and supposedly syncing the file with this file on prometheus/prometheus. Do you think it makes sense to send a PR to prometheus/prometheus to add the minimal permissions to that original file?

Another think that I could suggest is that you change (if you haven't already) the repository configuration to set the default permissions to read-only. In this case, if any workflow gets changed (or created) and forget to explicitly declare the permissions, it would run with read-only permissions and any action that requires write-permissions would fail.

@diogoteles08
Copy link
Contributor Author

I took the liberty and sent a PR to prometheus/prometheus and changed that original file. Now when your workflow imports the file according to the one in prometheus/prometheus, it should keep the minimal permissions.

That said, I'll send a small PR also resetting the minimal permission here, so we won't need to wait for the import from prometheus/prometheus to get it secured.

diogoteles08 added a commit to diogoteles08/client_golang that referenced this issue Dec 29, 2023
Implemented as discussed at prometheus#1294 and already implemented at prometheus/prometheus

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
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 a pull request may close this issue.

1 participant