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

🐛 Handle pinning by Docker URLs for GitHub actions workflows #2594

Merged
merged 5 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
21 changes: 16 additions & 5 deletions checks/raw/pinned_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,6 @@ var validateGitHubActionWorkflow fileparser.DoWhileTrueOnFileContent = func(
return false, fileparser.FormatActionlintError(errs)
}

hashRegex := regexp.MustCompile(`^.*@[a-f\d]{40,}`)
for jobName, job := range workflow.Jobs {
jobName := jobName
job := job
Expand Down Expand Up @@ -470,10 +469,7 @@ var validateGitHubActionWorkflow fileparser.DoWhileTrueOnFileContent = func(
continue
}

// Ensure a hash at least as large as SHA1 is used (40 hex characters).
// Example: action-name@hash
match := hashRegex.MatchString(execAction.Uses.Value)
if !match {
if !isActionDependencyPinned(execAction.Uses.Value) {
dep := checker.Dependency{
Location: &checker.File{
Path: pathfn,
Expand All @@ -498,3 +494,18 @@ var validateGitHubActionWorkflow fileparser.DoWhileTrueOnFileContent = func(

return true, nil
}

func isActionDependencyPinned(actionUses string) bool {
localActionRegex := regexp.MustCompile(`^\..+[^/]`)
if localActionRegex.MatchString(actionUses) {
return true
}

publicActionRegex := regexp.MustCompile(`.*@[a-fA-F\d]{40,}`)
if publicActionRegex.MatchString(actionUses) {
return true
}

dockerhubActionRegex := regexp.MustCompile(`docker://.*@sha256:[a-fA-F\d]{64}`)
return dockerhubActionRegex.MatchString(actionUses)
}
83 changes: 82 additions & 1 deletion checks/raw/pinned_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestGithubWorkflowPinning(t *testing.T) {
{
name: "Non-pinned workflow",
filename: "./testdata/.github/workflows/workflow-not-pinned.yaml",
warns: 1,
warns: 2,
},
{
name: "Non-yaml file",
Expand Down Expand Up @@ -99,6 +99,87 @@ func TestGithubWorkflowPinning(t *testing.T) {
}
}

func TestGithubWorkflowPinningPattern(t *testing.T) {
tests := []struct {
desc string
uses string
ispinned bool
}{
{
desc: "checking out mutable tag",
uses: "actions/checkout@v3",
ispinned: false,
},
{
desc: "hecking out mutable tag",
uses: "actions/[email protected]",
ispinned: false,
},
{
desc: "checking out mutable tag",
uses: "actions/checkout@main",
ispinned: false,
},
{
desc: "checking out mutable tag",
uses: "actions/[email protected]",
ispinned: false,
},
{
desc: "checking out mutable tag",
uses: "actions/aws/ec2@main",
ispinned: false,
},
{
desc: "checking out specific commmit from github with truncated SHA-1",
uses: "actions/checkout@a81bbbf",
ispinned: false,
},
{
desc: "checking out specific commmit from github with SHA-1",
uses: "actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675",
ispinned: true,
},
{
desc: "local workflow",
uses: "./.github/uses.yml",
ispinned: true,
},
{
desc: "non-github docker image pinned by digest",
//nolint:lll
uses: "docker://gcr.io/distroless/static-debian11@sha256:9e6f8952f12974d088f648ed6252ea1887cdd8641719c8acd36bf6d2537e71c0",
ispinned: true,
},
{
desc: "non-github docker image pinned to mutable tag",
//nolint:lll
uses: "docker://gcr.io/distroless/static-debian11:sha256-3876708467ad6f38f263774aa107d331e8de6558a2874aa223b96fc0d9dfc820.sig",
ispinned: false,
},
{
desc: "non-github docker image pinned to mutable version",
uses: "docker://rhysd/actionlint:latest",
ispinned: false,
},
{
desc: "non-github docker image pinned by digest",
uses: "docker://rhysd/actionlint:latest@sha256:5f957b2a08d223e48133e1a914ed046bea12e578fe2f6ae4de47fdbe691a2468",
ispinned: true,
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.desc, func(t *testing.T) {
t.Parallel()
p := isActionDependencyPinned(tt.uses)
if p != tt.ispinned {
t.Fatalf("dependency %s ispinned?: %v expected?: %v", tt.uses, p, tt.ispinned)
}
})
}
}

func TestNonGithubWorkflowPinning(t *testing.T) {
t.Parallel()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ jobs:
# a pull request then we can checkout the head.
fetch-depth: 2

raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
- name: Dockerhub actionlint # (just to test docker://) urls for the uses parameter, not needed for the workflow
uses: docker://rhysd/actionlint@latest
with:
args: -color
- name: Get build targets
run: |
. .github/workflows/get_build_targets.sh
Expand Down
4 changes: 4 additions & 0 deletions checks/raw/testdata/.github/workflows/workflow-pinned.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ jobs:
# a pull request then we can checkout the head.
fetch-depth: 2

- name: Dockerhub actionlint # (just to test docker://) urls for the uses parameter, not needed for the workflow
uses: docker://rhysd/actionlint@sha256:5f957b2a08d223e48133e1a914ed046bea12e578fe2f6ae4de47fdbe691a2468
with:
args: -color
- name: Get build targets
run: |
. .github/workflows/get_build_targets.sh
Expand Down