Handle Docker URLs for GitHub actions workflows (#2594)

Signed-off-by: Raghav Kaul <raghavkaul@google.com> Signed-off-by: Raghav Kaul <raghavkaul@google.com> Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
ossf · Jan 24, 2023 · e6a900d · e6a900d
1 parent 3f372e9
commit e6a900d
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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