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: set minimal permissions on GitHub Workflow #919

Closed
diogoteles08 opened this issue Mar 9, 2023 · 5 comments · Fixed by #1163
Closed

CI: set minimal permissions on GitHub Workflow #919

diogoteles08 opened this issue Mar 9, 2023 · 5 comments · Fixed by #1163
Assignees

Comments

@diogoteles08
Copy link

Hi!

I see that your GitHub workflows currently don't specify the permissions of their jobs -- in this way their privileges are being determined by GitHub's defaults. If you define minimal permissions you would be secured against erroneous or malicious actions from external jobs you call from your workflow. It's specially important for the case they get compromised, for example.

The idea would be to set a top-level read-only permission on all workflows, so that it would be inherited by all jobs that don't define job-level permissions. With this setup, any required write permission would be declared only where it's needed, as a job-level permissions.

Setting minimum permissions for workflows is recommended by GitHub itself and also by other security tools, such as Scorecards and StepSecurity.

Let me know what you think about this. I'd be happy to raise a PR with the changes if you agree.

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 😊

@gevtushenko
Copy link
Collaborator

@trxcllnt could you please take a triage this issue?

@jrhemstad
Copy link
Collaborator

Hey @diogoteles08. Thanks for the heads up! We're actually in the process of migrating to an entirely new repo with an entirely new actions setup very soon. We'll definitely be sure to address the issue you raised as part of that migration.

@diogoteles08
Copy link
Author

Hey @jrhemstad! Sorry for the delayed reply, but it makes perfect sense to wait for the migration to target this issue.

Does the project have any sort of timeline for this migration?

@jarmak-nv jarmak-nv transferred this issue from NVIDIA/cub Nov 8, 2023
@github-project-automation github-project-automation bot moved this to Todo in CCCL Nov 8, 2023
@jrhemstad
Copy link
Collaborator

For my own reference, here is how RAPIDS is setting this in their workflows: https://github.com/rapidsai/shared-workflows/blob/12579029b463eb77ffa8119acd05b02952a1b41e/.github/workflows/conda-cpp-build.yaml#L29-L42

@jrhemstad jrhemstad self-assigned this Nov 8, 2023
@diogoteles08
Copy link
Author

diogoteles08 commented Nov 9, 2023

@jrhemstad As an FWI, if you define any permission, all the non-mentioned permissions are automatically set to none(doc ref here), so no need to write all other permissions as none.

That said, what I usually suggest is to set only

permissions:
    contents: read

at the workflow level, and set any other needed permission (as the id-token: write) at job level, like in this example.

@cccl-authenticator-app cccl-authenticator-app bot moved this from Todo to In Progress in CCCL Nov 29, 2023
@cccl-authenticator-app cccl-authenticator-app bot moved this from In Progress to In Review in CCCL Nov 29, 2023
@github-project-automation github-project-automation bot moved this from In Review to Done in CCCL Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants