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

Add read-only token permissions to GitHub Action workflows #12718

Merged
merged 3 commits into from
Mar 27, 2023

Conversation

pnacht
Copy link
Contributor

@pnacht pnacht commented Mar 23, 2023

Description

As described in the issue below, this PR adds read-only token permissions to all GitHub Action workflows, including their respective templates.

close_stale_pull_requests.yml needs pull_request: write permissions, so those were granted at the job level, ensuring that any future jobs added to the workflow don't have that permission unnecessarily.

Related Issue(s)

Fixes #12717

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on the CI
  • Documentation was added or is not required

pnacht added 2 commits March 23, 2023 21:34
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
Almost all workflows recreated with `make generate_ci_workflows`.

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
@pnacht pnacht changed the title Token permissions Add read-only token permissions to GitHub Action workflows Mar 23, 2023
@frouioui frouioui added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Build/CI labels Mar 24, 2023
Copy link
Member

@frouioui frouioui left a comment

Choose a reason for hiding this comment

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

Hello Pedro and thank you for your first contribution. This looks good to me. However, I'll apply your change to even more workflows in a subsequent commit.

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Copy link
Member

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

LGTM

@pnacht
Copy link
Contributor Author

pnacht commented Mar 24, 2023

@frouioui thanks for the additional changes! I can't believe I missed so many workflows!

One question, though: why did you grant create_release.yml write-all permissions? Is it for the csexton/release-asset-action? Given that it authenticates using a secret token (not the workflow's normal token), I don't think that's necessary.

@frouioui
Copy link
Member

frouioui commented Mar 27, 2023

One question, though: why did you grant create_release.yml write-all permissions? Is it for the csexton/release-asset-action? Given that it authenticates using a secret token (not the workflow's normal token), I don't think that's necessary.

Hello @pnacht! It seems like we are using the auto-generated GitHub Token that Actions gives us. I think we are required to have the GITHUB_TOKEN set to write if we want csexton/release-asset-action to write to our releases. I might be wrong, but their README is not mentioning anything special.

@frouioui
Copy link
Member

frouioui commented Mar 27, 2023

Merging this now, we can always modify create_release.yml in a subsequent Pull Request if we are not agreeing.

@frouioui frouioui merged commit e1b79be into vitessio:main Mar 27, 2023
@pnacht
Copy link
Contributor Author

pnacht commented Mar 27, 2023

Hello @pnacht! It seems like we are using the auto-generated GitHub Token that Actions gives us.

Ah yes! I hadn't seen it called as secretes.GITHUB_TOKEN before, just as github.token (they're equivalent, I just hadn't seen the former), got confused.

I think we are required to have the GITHUB_TOKEN set to write if we want csexton/release-asset-action to write to our releases. I might be wrong, but their README is not mentioning anything special.

That Action doesn't specify anything, but similar actions only require

jobs:
  build:
    permissions:
      contents: write

It's also worth mentioning that csexton/release-asset-action is no longer actively maintained (csexton/release-asset-action#20). Here are some alternatives I've found:

Also, create_release.yml has the following:

    # TEMPORARY WHILE GITHUB FIXES THIS https://github.com/actions/virtual-environments/issues/3185
    - name: Add the current IP address, long hostname and short hostname record to /etc/hosts file
      run: |
        echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
    # DON'T FORGET TO REMOVE CODE ABOVE WHEN ISSUE IS ADRESSED!

The linked issue has been fixed.

@frouioui
Copy link
Member

Hey @pnacht, thanks for reporting this! Do you mind opening an issue for this matter and assign it to me? I will take a look, we don't want to rely on unmaintained actions :)

@frouioui
Copy link
Member

Also, create_release.yml has the following:

    # TEMPORARY WHILE GITHUB FIXES THIS https://github.com/actions/virtual-environments/issues/3185
    - name: Add the current IP address, long hostname and short hostname record to /etc/hosts file
      run: |
        echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
    # DON'T FORGET TO REMOVE CODE ABOVE WHEN ISSUE IS ADRESSED!

The linked issue has been fixed.

I'll open a PR now to remove this unnecessary bit of code.

@pnacht pnacht deleted the token_permissions branch April 28, 2023 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Build/CI Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set read-only token permissions for all workflows
3 participants