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.

Sign up for GitHub

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

Jump to bottom

fix(ci): Manage secrets/permissions #3396

Merged
merged 2 commits into from
Dec 10, 2023
+36 −1
Merged

fix(ci): Manage secrets/permissions #3396

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
093aebf
fix(ci): Manage secrets/permissions
jsoref Feb 27, 2023
b0a787c
Merge branch 'master' into issue-3395
cmsj Dec 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion .github/workflows/ci_nightly.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,29 @@ on:
- master

jobs:
config:
runs-on: "ubuntu-latest"
permissions:
contents: none
Comment on lines +11 to +12
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.

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

steps:
- name: "Check for secrets"
id: check
shell: bash
run: |
if [ -n "${{ (secrets.REPO_GPG_PASSPHRASE != '' && secrets.NIGHTLY_KEYCHAIN_PASSPHRASE != '') || '' }}" ]; then
echo "has-secrets=1" >> "$GITHUB_OUTPUT"
Comment on lines +20 to +21
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

fi

build:
needs: config
if: needs.config.outputs.has-secrets
Comment on lines +25 to +26
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

name: Build and Sign

runs-on: macos-12
permissions:
contents: read
Comment on lines +30 to +31
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.

env:
IS_CI: 1
IS_NIGHTLY: 1
Expand Down Expand Up @@ -114,5 +133,9 @@ jobs:
path: archive/${{ steps.version.outputs.NIGHTLY_VERSION }}/Release-build.log

- name: Send notification
if: env.CI_WEBHOOK_URL != '' && env.CI_WEBHOOK_SECRET != ''
env:
CI_WEBHOOK_URL: ${{ secrets.CI_WEBHOOK_URL }}
CI_WEBHOOK_SECRET: ${{ secrets.CI_WEBHOOK_SECRET }}
run: |
/usr/bin/curl -H "X-CI-WebHook: true" -H "Content-Type: application/json" -d '{"secret": "${{ secrets.CI_WEBHOOK_SECRET }}", "repository": "hammerspoon", "workflow": "Dev Build", "message": "New development build: ${{ steps.version.outputs.NIGHTLY_VERSION }}"}' ${{ secrets.CI_WEBHOOK_URL }}
/usr/bin/curl -H "X-CI-WebHook: true" -H "Content-Type: application/json" -d '{"secret": "$CI_WEBHOOK_SECRET", "repository": "hammerspoon", "workflow": "Dev Build", "message": "New development build: ${{ steps.version.outputs.NIGHTLY_VERSION }}"}' $CI_WEBHOOK_URL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in #3394, the lack of this secret pair shouldn't result in the workflow dying / skipping, so we don't put it in the other job...

Copy link
Member

Choose a reason for hiding this comment

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

Is there a particular reason for pulling the secrets into env here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If doesn't have access to secrets. Since the if needs access to the existence of the secret and the run needs the secret or seemed vaguely worth just sticking the secret into the env instead of just an existence. But, no, it could just be an existence variable and the run could retain its direct reference to the secret.

2 changes: 2 additions & 0 deletions .github/workflows/ci_testbuild.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

env:
IS_CI: 1

Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/ci_testbuild_results.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ jobs:
publish:
name: "Process CI Results"
runs-on: ubuntu-latest
permissions:
actions: read
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

if: always()

steps:
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/new_tag.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

https://github.com/ncipollo/release-action#notes

steps:
- name: Checkout
uses: actions/checkout@v2
Expand Down Expand Up @@ -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

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).

steps:
- uses: actions/checkout@v2
with:
Expand Down