Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

🔐 Reduce permissions in documentation.yml workflow #208

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

connormaglynn
Copy link
Contributor

No description provided.

@@ -5,18 +5,20 @@ on:
branches:
- main

permissions: { }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The checkgov issue didn't like the default permissions, setting these removes them all

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it still use the default permissions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removes all the permissions (REF)

You can use the following syntax to disable permissions for all of the available scopes:
permissions: {}

Copy link
Contributor

Choose a reason for hiding this comment

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

This action writes to pull requests so you will need a minimum of:

permissions:
   contents: read
   pull_request: write

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jakemulley turns out it's

permissions:
   contents: write

Since you need contents: write to write to a file and pull_request:write is for data on the PR such as labels, assignees, reviewers, comments etc. (REF)

@connormaglynn connormaglynn marked this pull request as ready for review October 17, 2023 14:45
@connormaglynn connormaglynn requested a review from a team as a code owner October 17, 2023 14:45
@connormaglynn connormaglynn changed the title 🔥 Remove permissions from documentation.yml workflow 🔐 Reduce permissions in documentation.yml workflow Oct 17, 2023
Copy link
Contributor

@dms1981 dms1981 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 for the discussion on the permissions used!

@connormaglynn connormaglynn merged commit 8115197 into main Oct 18, 2023
3 checks passed
@connormaglynn connormaglynn deleted the fix-checkgov-issue branch October 18, 2023 09:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants