Skip to content

Conversation

marcofranssen
Copy link
Contributor

@marcofranssen marcofranssen commented Dec 20, 2022

This allows to narrow down workflow permissions in GitHub settings

See https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs
and https://docs.github.com/en/actions/security-guides/automatic-token-authentication\#permissions-for-the-github_token

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?

Affected functionality

Description of change

This allows to set workflows by default to readonly and only request the permissions as they are needed for a certain job, reducing potential attack surface.

A repo admin can change the following setting once this is merged.

https://github.com/spiffe/spire/settings/actions

image

Which issue this PR fixes


env:
NIGHTLY: true

jobs:
build-and-publish-images:
runs-on: ubuntu-20.04

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.

Moved to job level, so we don't run the chance new job added in this workflow will also have write access by default.

@marcofranssen
Copy link
Contributor Author

Looks like flaky test. Can someone retrigger this test?

@@ -1,12 +1,13 @@
name: 'Dependency Review'
on: [pull_request]

permissions:
Copy link
Member

Choose a reason for hiding this comment

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

If we leave this action level permission here, does it apply to any jobs that omit the permissions configuration? If so, it seems safer to leave it in so that any future jobs that neglect to configure permissions will only get read (assuming the org level default is higher).

Copy link
Contributor Author

@marcofranssen marcofranssen Dec 20, 2022

Choose a reason for hiding this comment

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

contents: read is the default anyhow. You don't even have to define that.

I just like to define it explicitly so it is clear what permissions a job has.

See the link in the pr description.

@evan2645 evan2645 added this to the 1.5.4 milestone Dec 20, 2022
@MarcosDY MarcosDY merged commit 9d0b194 into spiffe:main Dec 22, 2022
@marcofranssen marcofranssen deleted the workflow-permissions branch December 23, 2022 10:08
divaspathak pushed a commit to divaspathak/spire that referenced this pull request Dec 24, 2022
rturner3 pushed a commit to rturner3/spire that referenced this pull request Jan 12, 2023
rturner3 pushed a commit to rturner3/spire that referenced this pull request Jan 13, 2023
rturner3 pushed a commit to rturner3/spire that referenced this pull request Jan 13, 2023
azdagron added a commit to azdagron/spire that referenced this pull request Jan 13, 2023
- Fixes a recent regression in permissions on the publish-artifact job
  (introduced by spiffe#3706).
- Makes image publishing rely on the same jobs as artifact publishing so
  that we don't publish images but fail to publish the release if there
  is a failure in the windows jobs.

Signed-off-by: Andrew Harding <[email protected]>
@azdagron azdagron mentioned this pull request Jan 13, 2023
MarcosDY pushed a commit that referenced this pull request Jan 13, 2023
- Fixes a recent regression in permissions on the publish-artifact job
  (introduced by #3706).
- Makes image publishing rely on the same jobs as artifact publishing so
  that we don't publish images but fail to publish the release if there
  is a failure in the windows jobs.

Signed-off-by: Andrew Harding <[email protected]>
stevend-uber pushed a commit to stevend-uber/spire that referenced this pull request Oct 16, 2023
- Fixes a recent regression in permissions on the publish-artifact job
  (introduced by spiffe#3706).
- Makes image publishing rely on the same jobs as artifact publishing so
  that we don't publish images but fail to publish the release if there
  is a failure in the windows jobs.

Signed-off-by: Andrew Harding <[email protected]>
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.

4 participants