✨ Add line number to unpinned dependency: GitHub workflow "uses" field (

#821)

* Display line number for github workflow "uses" field

* Adding test for line numbers

* Updating comment

* Updating this log message to use SARIF format

Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>

checks/pinned_dependencies.go

@@ -20,7 +20,7 @@ import (
 	"strings"
 
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/ossf/scorecard/v2/checker"
 	sce "github.com/ossf/scorecard/v2/errors"
@@ -37,11 +37,11 @@ type gitHubActionWorkflowConfig struct {
 	Jobs map[string]struct {
 		Name  string `yaml:"name"`
 		Steps []struct {
-			Name  string `yaml:"name"`
-			ID    string `yaml:"id"`
-			Uses  string `yaml:"uses"`
-			Shell string `yaml:"shell"`
-			Run   string `yaml:"run"`
+			Name  string         `yaml:"name"`
+			ID    string         `yaml:"id"`
+			Uses  stringWithLine `yaml:"uses"`
+			Shell string         `yaml:"shell"`
+			Run   string         `yaml:"run"`
 		}
 		Defaults struct {
 			Run struct {
@@ -52,6 +52,23 @@ type gitHubActionWorkflowConfig struct {
 	Name string `yaml:"name"`
 }
 
+// stringWithLine is for when you want to keep track of the line number that the string came from.
+type stringWithLine struct {
+	Value string
+	Line  int
+}
+
+func (ws *stringWithLine) UnmarshalYAML(value *yaml.Node) error {
+	err := value.Decode(&ws.Value)
+	if err != nil {
+		//nolint:wrapcheck
+		return sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("error decoding stringWithLine Value: %v", err))
+	}
+	ws.Line = value.Line
+
+	return nil
+}
+
 //nolint:gochecknoinits
 func init() {
 	registerCheck(CheckPinnedDependencies, PinnedDependencies)
@@ -522,13 +539,16 @@ func validateGitHubActionWorkflow(pathfn string, content []byte,
 			jobName = job.Name
 		}
 		for _, step := range job.Steps {
-			if len(step.Uses) > 0 {
+			if len(step.Uses.Value) > 0 {
 				// Ensure a hash at least as large as SHA1 is used (40 hex characters).
 				// Example: action-name@hash
-				match := hashRegex.Match([]byte(step.Uses))
+				match := hashRegex.Match([]byte(step.Uses.Value))
 				if !match {
 					ret = false
-					dl.Warn("unpinned dependency detected in %v: '%v' (job '%v')", pathfn, step.Uses, jobName)
+					dl.Warn3(&checker.LogMessage{
+						Path: pathfn, Type: checker.FileTypeSource, Offset: step.Uses.Line, Snippet: step.Uses.Value,
+						Text: fmt.Sprintf("unpinned dependency detected (job '%v')", jobName),
+					})
 				}
 			}
 		}

checks/pinned_dependencies_test.go

@@ -17,6 +17,7 @@ package checks
 import (
 	"fmt"
 	"io/ioutil"
+	"strings"
 	"testing"
 
 	"github.com/ossf/scorecard/v2/checker"
@@ -542,3 +543,72 @@ func TestGitHubWorflowRunDownload(t *testing.T) {
 		})
 	}
 }
+
+func TestGitHubWorkflowUsesLineNumber(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		filename string
+		expected []struct {
+			dependency string
+			lineNumber int
+		}
+	}{
+		{
+			name:     "unpinned dependency in uses",
+			filename: "testdata/github-workflow-permissions-run-codeql-write.yaml",
+			expected: []struct {
+				dependency string
+				lineNumber int
+			}{
+				{
+					dependency: "github/codeql-action/analyze@v1",
+					lineNumber: 25,
+				},
+			},
+		},
+		{
+			name:     "multiple unpinned dependency in uses",
+			filename: "testdata/github-workflow-multiple-unpinned-uses.yaml",
+			expected: []struct {
+				dependency string
+				lineNumber int
+			}{
+				{
+					dependency: "github/codeql-action/analyze@v1",
+					lineNumber: 22,
+				},
+				{
+					dependency: "docker/build-push-action@1.2.3",
+					lineNumber: 26,
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt // Re-initializing variable so it is not changed while executing the closure below
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			content, err := ioutil.ReadFile(tt.filename)
+			if err != nil {
+				t.Errorf("cannot read file: %w", err)
+			}
+			dl := scut.TestDetailLogger{}
+			var pinned pinnedResult
+			_, err = validateGitHubActionWorkflow(tt.filename, content, &dl, &pinned)
+			if err != nil {
+				t.Errorf("error during validateGitHubActionWorkflow: %w", err)
+			}
+			for _, expectedLog := range tt.expected {
+				isExpectedLog := func(logMessage checker.LogMessage, logType checker.DetailType) bool {
+					return logMessage.Offset == expectedLog.lineNumber && logMessage.Path == tt.filename &&
+						logMessage.Snippet == expectedLog.dependency && logType == checker.DetailWarn &&
+						strings.Contains(logMessage.Text, "unpinned dependency detected")
+				}
+				if !scut.ValidateLogMessage(isExpectedLog, &dl) {
+					t.Errorf("test failed: log message not present: %+v", tt.expected)
+				}
+			}
+		})
+	}
+}

checks/testdata/github-workflow-multiple-unpinned-uses.yaml

@@ -0,0 +1,26 @@
+# Copyright 2021 Security Scorecard Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+name: absent workflow
+on: [push]
+
+jobs:
+  Explore-GitHub-Actions:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v1
+        run: echo "unpinned dependency"
+      - name: some name
+        run: echo "unpinned dependency"
+        uses: docker/build-push-action@1.2.3

go.mod

@@ -34,5 +34,6 @@ require (
 	google.golang.org/genproto v0.0.0-20210821163610-241b8fcbd6c8
 	google.golang.org/protobuf v1.27.1
 	gopkg.in/yaml.v2 v2.4.0
+	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
 	mvdan.cc/sh/v3 v3.3.1
 )

utests/utlib.go

@@ -140,3 +140,14 @@ func ValidateTestReturn(t *testing.T, name string, te *TestReturn,
 	tr *checker.CheckResult, dl *TestDetailLogger) bool {
 	return ValidateTestValues(t, name, te, tr.Score, tr.Error2, dl)
 }
+
+// ValidateLogMessage tests that at least one log message returns true for isExpectedMessage.
+func ValidateLogMessage(isExpectedMessage func(checker.LogMessage, checker.DetailType) bool,
+	dl *TestDetailLogger) bool {
+	for _, message := range dl.messages {
+		if isExpectedMessage(message.Msg, message.Type) {
+			return true
+		}
+	}
+	return false
+}