From e6a900dd88216382d7e46be316ea4a27c4d71231 Mon Sep 17 00:00:00 2001 From: raghavkaul <8695110+raghavkaul@users.noreply.github.com> Date: Tue, 24 Jan 2023 14:09:51 -0500 Subject: [PATCH] Handle Docker URLs for GitHub actions workflows (#2594) Signed-off-by: Raghav Kaul Signed-off-by: Raghav Kaul Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com> --- checks/raw/pinned_dependencies.go | 21 +++-- checks/raw/pinned_dependencies_test.go | 83 ++++++++++++++++++- .../workflows/workflow-not-pinned.yaml | 4 + .../.github/workflows/workflow-pinned.yaml | 4 + 4 files changed, 106 insertions(+), 6 deletions(-) diff --git a/checks/raw/pinned_dependencies.go b/checks/raw/pinned_dependencies.go index 0b608b77972..622cc2c25cd 100644 --- a/checks/raw/pinned_dependencies.go +++ b/checks/raw/pinned_dependencies.go @@ -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 @@ -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, @@ -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) +} diff --git a/checks/raw/pinned_dependencies_test.go b/checks/raw/pinned_dependencies_test.go index da521b94edc..8c7c011d766 100644 --- a/checks/raw/pinned_dependencies_test.go +++ b/checks/raw/pinned_dependencies_test.go @@ -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", @@ -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/checkout@v3.2.0", + ispinned: false, + }, + { + desc: "checking out mutable tag", + uses: "actions/checkout@main", + ispinned: false, + }, + { + desc: "checking out mutable tag", + uses: "actions/aws@v2.0.1", + 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() diff --git a/checks/raw/testdata/.github/workflows/workflow-not-pinned.yaml b/checks/raw/testdata/.github/workflows/workflow-not-pinned.yaml index cf73f568ae3..917f8712549 100644 --- a/checks/raw/testdata/.github/workflows/workflow-not-pinned.yaml +++ b/checks/raw/testdata/.github/workflows/workflow-not-pinned.yaml @@ -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@latest + with: + args: -color - name: Get build targets run: | . .github/workflows/get_build_targets.sh diff --git a/checks/raw/testdata/.github/workflows/workflow-pinned.yaml b/checks/raw/testdata/.github/workflows/workflow-pinned.yaml index ae7e61a76ea..51e1a4a20b3 100644 --- a/checks/raw/testdata/.github/workflows/workflow-pinned.yaml +++ b/checks/raw/testdata/.github/workflows/workflow-pinned.yaml @@ -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