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

github/workflows: set read-only default permissions to approve workflow #18368

Merged

Conversation

ivanvc
Copy link
Member

@ivanvc ivanvc commented Jul 25, 2024

As discovered by the OpenSSF Scorecard, the gh-workflow-approve.yaml action didn't specify default permissions. This pull request fixes that issue.

Part of #18362.

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@k8s-ci-robot k8s-ci-robot added github_actions Pull requests that update GitHub Actions code size/XS labels Jul 25, 2024
@ivanvc ivanvc changed the title github/workdlows: set read-only default permissions to approve workflow github/workflows: set read-only default permissions to approve workflow Jul 25, 2024
@ivanvc ivanvc force-pushed the set-default-permissions-to-approve-workflow branch from 94bffec to 5a02298 Compare July 25, 2024 22:39
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@serathius serathius left a comment

Choose a reason for hiding this comment

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

This is intentional. We cannot change status of CI without having write permissions.

Context: #15659

The problem: https://github.com/orgs/community/discussions/49688

@@ -1,5 +1,6 @@
---
name: Approve GitHub Workflows
permissions: read-all
Copy link
Member

Choose a reason for hiding this comment

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

Probably something like below?

permissions:
  actions:write

Copy link
Member

@serathius serathius Jul 26, 2024

Choose a reason for hiding this comment

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

That should work! Thanks, I forgot that the issue is about the default permission.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was initially going to set it with explicit permission at the top of the file. However, I based this implementation on other workflows. For example, the scorecard workflow declares the read-all permission at the top of the file:

# Declare default permissions as read only.
permissions: read-all

Then, in the job, sets the write permissions:

permissions:
# Needed to upload the results to code-scanning dashboard.
security-events: write
# Used to receive a badge.
id-token: write

I tested it on my fork, and seems to work fine: https://github.com/ivanvc/etcd/actions/runs/10115486985/job/27976358983?pr=210

Let me know if you still want me to change this.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I just realised that you already added at the job level,

permissions:
  actions:write

https://github.com/ivanvc/etcd/blob/5a02298ad5a947214ba02655b0a93ac01d4c178a/.github/workflows/gh-workflow-approve.yaml#L20-L21

Looks good to me.

@ahrtr
Copy link
Member

ahrtr commented Jul 26, 2024

This PR should be good to merge.

@ivanvc ivanvc requested a review from serathius July 26, 2024 22:19
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks @ivanvc

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, fuweid, ivanvc, jmhbnz, serathius

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [ahrtr,jmhbnz,serathius]

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

@serathius serathius merged commit 1e1ed85 into etcd-io:main Jul 29, 2024
43 checks passed
@ivanvc ivanvc deleted the set-default-permissions-to-approve-workflow branch July 29, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved github_actions Pull requests that update GitHub Actions code size/XS
Development

Successfully merging this pull request may close these issues.

6 participants