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

fix(ci): Manage secrets/permissions #3396

Merged
merged 2 commits into from
Dec 10, 2023
Merged

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Feb 27, 2023

fixes #3395

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['pr-fix', 'pr-change', 'pr-feature', 'pr-maintenance']

Comment on lines +20 to +21
if [ -n "${{ (secrets.REPO_GPG_PASSPHRASE != '' && secrets.NIGHTLY_KEYCHAIN_PASSPHRASE != '') || '' }}" ]; then
echo "has-secrets=1" >> "$GITHUB_OUTPUT"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main logic for checking for secrets

permissions:
contents: none
outputs:
has-secrets: ${{ steps.check.outputs.has-secrets }}
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 output then arrives here

Comment on lines +25 to +26
needs: config
if: needs.config.outputs.has-secrets
Copy link
Contributor Author

Choose a reason for hiding this comment

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

and is checked here

Comment on lines +11 to +12
permissions:
contents: none
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this job doesn't really need any permissions, so we say so.

Comment on lines +30 to +31
permissions:
contents: read
Copy link
Contributor Author

Choose a reason for hiding this comment

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

separately, we declare the permissions.

Afaict, this job only needs to be able to checkout.

@@ -10,6 +10,8 @@ jobs:
build:
name: Build and Test
runs-on: macos-12
permissions:
contents: read
Copy link
Contributor Author

Choose a reason for hiding this comment

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

afaict, the only permissions this workflow needs are to checkout, so let's be explicit (this reduces scope from the default which is much broader)

permissions:
checks: write
pull-requests: 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.

I missed that this is using github.actions.listWorkflowRunArtifacts https://docs.github.com/en/rest/actions/artifacts?apiVersion=2022-11-28#list-artifacts-for-a-repository

@@ -10,6 +10,8 @@ jobs:
generate-release-notes:
name: Generate Release Notes
runs-on: ubuntu-latest
permissions:
contents: 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.

@@ -52,6 +54,9 @@ jobs:
create-next-milestone:
name: Create next milestone
runs-on: ubuntu-latest
permissions:
contents: read
Copy link
Contributor Author

Choose a reason for hiding this comment

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

actions/checkout needs read

@@ -52,6 +54,9 @@ jobs:
create-next-milestone:
name: Create next milestone
runs-on: ubuntu-latest
permissions:
contents: read
issues: 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.

afaict creating a milestone requires issues write (I looked this up but closed the tab -- I'm not particularly happy that the action doesn't state that explicitly).

@cmsj cmsj merged commit f30eef2 into Hammerspoon:master Dec 10, 2023
1 of 2 checks passed
@jsoref jsoref deleted the issue-3395 branch December 10, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Development Builds" fails miserably on forks
2 participants