-
Notifications
You must be signed in to change notification settings - Fork 521
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
chore: pin and update all workflow dependencies; add permission scopes #2138
Conversation
f42ef81
to
f17ea04
Compare
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
f17ea04
to
0407528
Compare
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
jobs: | ||
analyze: | ||
name: Analyze | ||
runs-on: ubuntu-latest | ||
|
||
permissions: | ||
# required for all workflow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand this comment. Do you mean all workflows
? Is there an "all" workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry - this was lifted from their docs - "All" I think workflows and job might be conflated here.
https://github.com/github/codeql-action/blob/main/.github/workflows/codeql.yml
You can see where they dogfood their own action how the security events write
permission is set for build and version checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the comment since you're right it is confusing using the "workflow" noun here
@@ -1,4 +1,4 @@ | |||
FROM golang:latest as builder | |||
FROM golang:latest@sha256:cffaba795c36f07e372c7191b35ceaae114d74c31c3763d442982e3a4df3b39e as builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do when golang:latest
moves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question - I think this fails - let me pin this to a strict version
@@ -1,4 +1,4 @@ | |||
FROM alpine:latest | |||
FROM alpine:latest@sha256:7144f7bab3d4c2648d7e59409f15ec52a18006a128c733fcff20d3a4a54ba44a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question: will this break if alpine:latest
moves?
Really I have this question about all tag + sha, but :latest
seems guaranteed to move.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same response for golang - I think we pin to a more specific version - I'm not sure if dockerhub keeps a running log of latest
or if it updates it in place
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@@ -1,4 +1,4 @@ | |||
FROM alpine:latest | |||
FROM alpine:latest@sha256:7144f7bab3d4c2648d7e59409f15ec52a18006a128c733fcff20d3a4a54ba44a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another sha + latest
. (Sorry for not marking all of them in the first round of comments.)
I wonder if it makes sense to just us alpine@sha...
? (Maybe I should have done that in #2137 as well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok to specify the version here - it doesn't make a different on the Docker side of things and communicates more information to future reads what the exact version is that this sha pins to so they don't have to hunt for that.
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@@ -1,2 +1,2 @@ | |||
# An image containing the example hello-auditable binary from https://github.com/Shnatsel/rust-audit/tree/master/hello-auditable | |||
FROM docker.io/tofay/hello-rust-auditable:latest | |||
FROM docker.io/tofay/hello-rust-auditable:latest@sha256:1d35d1e007180b3f7500aae5e27560697909132ca9a6d480c4c825534c1c47a9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another latest
+ sha.
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
anchore#2138) --------- Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Summary
This PR takes a few steps on the github actions side of our code to protect against potential security threats.
READ
(writes can still be scoped to individual steps)Methodology
For the Audit I used https://github.com/ossf/scorecard#basic-usage with the
--show-details
flag. The sections I looked at for this PR were:Dangerous-Workflow
Pinned-Dependencies
Token-Permissions
Unofficial Actions we use (not managed by github) to consider keeping, moving, or changing from
https://github.com/fountainhead/action-wait-for-check
https://github.com/tibdex/github-app-token/releases/tag/v2.0.0