Skip to content

ci: read-only token-permissions for all workflows #7278

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

Closed
wants to merge 2 commits into from

Conversation

pnacht
Copy link

@pnacht pnacht commented Sep 12, 2023

Fixes kubeflow/notebooks#80.

This PR ensures all workflows run with read-only permissions.

From what I could tell, the triage_issues.yml workflow uses the secret PAT instead of the default workflow token, and as such doesn't require additional permissions.

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yanniszark for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@thesuperzapper
Copy link
Member

@pnacht thanks for this!

We should confirm that the repo is not already set to use "read" permission by default, which would make this redundant.

/cc @kimwnasptd

@google-oss-prow google-oss-prow bot requested a review from kimwnasptd October 5, 2023 03:08
@pnacht
Copy link
Author

pnacht commented Oct 12, 2023

Hey @thesuperzapper, sorry for the late reply.

Looking at your Action logs, we can see the workflows are running with write-all permissions: https://github.com/kubeflow/kubeflow/actions/runs/6500263891/job/17655300160#step:1:19

While setting default read-only tokens is certainly what I'd recommend, I'd suggest also adding these explicit permissions to the workflows for added transparency to your users.

@pnacht
Copy link
Author

pnacht commented Dec 21, 2023

Hey @thesuperzapper & @kimwnasptd, let me know if this is something you'd be interested in.

I'll happily handle the conflicts if so.

@thesuperzapper
Copy link
Member

@pnacht we could possibly do the other way around (change the repo to "read only" by default, and "inclusive" specify permissions that are needed beyond read in each job).

@james-jwu @zijianjoy can make the change, but we need to make the PR give appropriate permissions to each job first l.

@pnacht if you want to check which jobs need more than "read only", we can update the PR to give "write" access, and then update the repo settings.

@pnacht
Copy link
Author

pnacht commented Dec 26, 2023

Hey @thesuperzapper, I've just pushed a new commit that adds the minimal permissions needed for all workflows.

The only ones that need any write permission are the example_notebook_servers_publish.yml and ..._TEMPLATE.yml workflows, which need packages: write to push to the GitHub Container Repository.

Everything else should work fine with read-only tokens.

I've still left all workflows declaring permissions: contents: read at the top-level. This will be "redundant" if you modify the default token to read-only, but is still useful to let users know the workflows are adequately permissioned. These top-level permissions also serve as a backstop in case anyone accidentally changes the setting back to write-all (which many Actions unfortunately suggest doing instead of stating which permissions they actually need).

That being said, let me know if you'd rather I drop the top-level read-only permissions and leave that to the repo settings.

@thesuperzapper thesuperzapper changed the title Add read-only token-permissions to all workflows ci: read-only token-permissions for all workflows May 11, 2024
@thesuperzapper thesuperzapper modified the milestones: v1.9.0, v1.9.1 May 11, 2024
Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

The Kubeflow Dashboard code has been migrated to the new repo, please re-open PR at: https://github.com/kubeflow/dashboard
/close

@google-oss-prow google-oss-prow bot closed this May 2, 2025
Copy link

@andreyvelich: Closed this PR.

In response to this:

The Kubeflow Dashboard code has been migrated to the new repo, please re-open PR at: https://github.com/kubeflow/dashboard
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set workflows to run with read-only permissions
3 participants