✨ Give low importance to github-owned actions (#802)

* Different calculation between github and non-github actions

* Add test case for different kind of github and non-github action

* Modify existing test as score calculation has changed

checks/pinned_dependencies.go

@@ -1,4 +1,3 @@
-// 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.
@@ -102,6 +101,13 @@ type stringWithLine struct {
  Line int
 }
 
+// Structure to host information about pinned github
+// or third party dependencies.
+type worklowPinningResult struct {
+ thirdParties pinnedResult
+ gitHubOwned pinnedResult
+}
+
 func (ws *stringWithLine) UnmarshalYAML(value *yaml.Node) error {
  err := value.Decode(&ws.Value)
  if err != nil {
@@ -207,6 +213,44 @@ func addPinnedResult(r *pinnedResult, to bool) {
  }
 }
 
+func dataAsWorkflowResultPointer(data FileCbData) *worklowPinningResult {
+ pdata, ok := data.(*worklowPinningResult)
+ if !ok {
+ // panic if it is not correct type
+ panic("type need to be of worklowPinningResult")
+ }
+ return pdata
+}
+
+func createReturnValuesForGitHubActionsWorkflowPinned(r worklowPinningResult, infoMsg string,
+ dl checker.DetailLogger, err error) (int, error) {
+ if err != nil {
+ return checker.InconclusiveResultScore, err
+ }
+
+ score := checker.MinResultScore
+
+ if r.gitHubOwned != notPinned {
+ score += 2
+ // TODO: set Snippet and line numbers.
+ dl.Info3(&checker.LogMessage{
+ Type: checker.FileTypeSource,
+ Text: fmt.Sprintf("%s %s", "GitHub-owned", infoMsg),
+ })
+ }
+
+ if r.thirdParties != notPinned {
+ score += 8
+ // TODO: set Snippet and line numbers.
+ dl.Info3(&checker.LogMessage{
+ Type: checker.FileTypeSource,
+ Text: fmt.Sprintf("%s %s", "Third-party", infoMsg),
+ })
+ }
+
+ return score, nil
+}
+
 func dataAsResultPointer(data FileCbData) *pinnedResult {
  pdata, ok := data.(*pinnedResult)
  if !ok {
@@ -610,20 +654,21 @@ func isStepWindows(step *gitHubActionWorkflowStep) (bool, error) {
 
 // Check pinning of github actions in workflows.
 func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) (int, error) {
- var r pinnedResult
+ var r worklowPinningResult
  err := CheckFilesContent(".github/workflows/*", true, c, validateGitHubActionWorkflow, &r)
  return createReturnForIsGitHubActionsWorkflowPinned(r, c.Dlogger, err)
 }
 
 // Create the result.
-func createReturnForIsGitHubActionsWorkflowPinned(r pinnedResult, dl checker.DetailLogger, err error) (int, error) {
- return createReturnValues(r,
- "GitHub actions are pinned",
+func createReturnForIsGitHubActionsWorkflowPinned(r worklowPinningResult, dl checker.DetailLogger,
+ err error) (int, error) {
+ return createReturnValuesForGitHubActionsWorkflowPinned(r,
+ "actions are pinned",
  dl, err)
 }
 
 func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
- var r pinnedResult
+ var r worklowPinningResult
  _, err := validateGitHubActionWorkflow(pathfn, content, dl, &r)
  return createReturnForIsGitHubActionsWorkflowPinned(r, dl, err)
 }
@@ -632,14 +677,11 @@ func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker
 // should continue executing after this file.
 func validateGitHubActionWorkflow(pathfn string, content []byte,
  dl checker.DetailLogger, data FileCbData) (bool, error) {
- if !isWorkflowFile(pathfn) {
- return true, nil
- }
-
- pdata := dataAsResultPointer(data)
+ pdata := dataAsWorkflowResultPointer(data)
 
  if !CheckFileContainsCommands(content, "#") {
- addPinnedResult(pdata, true)
+ addWorkflowPinnedResult(pdata, true, true)
+ addWorkflowPinnedResult(pdata, true, true)
  return true, nil
  }
 
@@ -652,7 +694,6 @@ func validateGitHubActionWorkflow(pathfn string, content []byte,
  }
 
  hashRegex := regexp.MustCompile(`^.*@[a-f\d]{40,}`)
- ret := true
  for jobName, job := range workflow.Jobs {
  if len(job.Name) > 0 {
  jobName = job.Name
@@ -663,17 +704,18 @@ func validateGitHubActionWorkflow(pathfn string, content []byte,
  // Example: action-name@hash
  match := hashRegex.Match([]byte(step.Uses.Value))
  if !match {
- ret = false
  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),
  })
  }
+
+ githubOwned := isGitHubOwnedAction(step.Uses.Value)
+ addWorkflowPinnedResult(pdata, match, githubOwned)
  }
  }
  }
 
- addPinnedResult(pdata, ret)
  return true, nil
 }
 
@@ -689,6 +731,21 @@ func isWorkflowFile(pathfn string) bool {
  }
 }
 
+// isGitHubOwnedAction check github specific action.
+func isGitHubOwnedAction(v string) bool {
+ a := strings.HasPrefix(v, "actions/")
+ c := strings.HasPrefix(v, "github/")
+ return a || c
+}
+
+func addWorkflowPinnedResult(w *worklowPinningResult, to, isGitHub bool) {
+ if isGitHub {
+ addPinnedResult(&w.gitHubOwned, to)
+ } else {
+ addPinnedResult(&w.thirdParties, to)
+ }
+}
+
 // Check presence of lock files thru validatePackageManagerFile().
 func isPackageManagerLockFilePresent(c *checker.CheckRequest) (int, error) {
  var r pinnedResult

checks/pinned_dependencies_test.go

@@ -42,7 +42,7 @@ func TestGithubWorkflowPinning(t *testing.T) {
  Error: nil,
  Score: checker.MaxResultScore,
  NumberOfWarn: 0,
- NumberOfInfo: 1,
+ NumberOfInfo: 2,
  NumberOfDebug: 0,
  },
  },
@@ -53,7 +53,7 @@ func TestGithubWorkflowPinning(t *testing.T) {
  Error: nil,
  Score: checker.MaxResultScore,
  NumberOfWarn: 0,
- NumberOfInfo: 1,
+ NumberOfInfo: 2,
  NumberOfDebug: 0,
  },
  },
@@ -64,7 +64,7 @@ func TestGithubWorkflowPinning(t *testing.T) {
  Error: nil,
  Score: checker.MaxResultScore,
  NumberOfWarn: 0,
- NumberOfInfo: 1,
+ NumberOfInfo: 2,
  NumberOfDebug: 0,
  },
  },
@@ -73,12 +73,101 @@ func TestGithubWorkflowPinning(t *testing.T) {
  filename: "./testdata/workflow-not-pinned.yaml",
  expected: scut.TestReturn{
  Error: nil,
- Score: checker.MinResultScore,
+ Score: checker.MaxResultScore - 2,
  NumberOfWarn: 1,
+ NumberOfInfo: 1,
+ NumberOfDebug: 0,
+ },
+ },
+ }
+ 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()
+ var content []byte
+ var err error
+
+ content, err = ioutil.ReadFile(tt.filename)
+ if err != nil {
+ panic(fmt.Errorf("cannot read file: %w", err))
+ }
+
+ dl := scut.TestDetailLogger{}
+ s, e := testIsGitHubActionsWorkflowPinned(tt.filename, content, &dl)
+ actual := checker.CheckResult{
+ Score: s,
+ Error2: e,
+ }
+ if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &actual, &dl) {
+ t.Fail()
+ }
+ })
+ }
+}
+
+func TestNonGithubWorkflowPinning(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ filename string
+ expected scut.TestReturn
+ }{
+ {
+ name: "Pinned non-github workflow",
+ filename: "./testdata/workflow-non-github-pinned.yaml",
+ expected: scut.TestReturn{
+ Error: nil,
+ Score: checker.MaxResultScore,
+ NumberOfWarn: 0,
+ NumberOfInfo: 2,
+ NumberOfDebug: 0,
+ },
+ },
+ {
+ name: "Pinned github workflow",
+ filename: "./testdata/workflow-mix-github-and-non-github-not-pinned.yaml",
+ expected: scut.TestReturn{
+ Error: nil,
+ Score: checker.MinResultScore,
+ NumberOfWarn: 2,
  NumberOfInfo: 0,
  NumberOfDebug: 0,
  },
  },
+ {
+ name: "Pinned github workflow",
+ filename: "./testdata/workflow-mix-github-and-non-github-pinned.yaml",
+ expected: scut.TestReturn{
+ Error: nil,
+ Score: checker.MaxResultScore,
+ NumberOfWarn: 0,
+ NumberOfInfo: 2,
+ NumberOfDebug: 0,
+ },
+ },
+ {
+ name: "Mix of pinned and non-pinned GitHub actions",
+ filename: "./testdata/workflow-mix-pinned-and-non-pinned-github.yaml",
+ expected: scut.TestReturn{
+ Error: nil,
+ Score: checker.MaxResultScore - 2,
+ NumberOfWarn: 1,
+ NumberOfInfo: 1,
+ NumberOfDebug: 0,
+ },
+ },
+ {
+ name: "Mix of pinned and non-pinned non-GitHub actions",
+ filename: "./testdata/workflow-mix-pinned-and-non-pinned-non-github.yaml",
+ expected: scut.TestReturn{
+ Error: nil,
+ Score: checker.MaxResultScore - 8,
+ NumberOfWarn: 1,
+ NumberOfInfo: 1,
+ NumberOfDebug: 0,
+ },
+ },
  }
  for _, tt := range tests {
  tt := tt // Re-initializing variable so it is not changed while executing the closure below
@@ -627,7 +716,7 @@ func TestGitHubWorkflowUsesLineNumber(t *testing.T) {
  t.Errorf("cannot read file: %w", err)
  }
  dl := scut.TestDetailLogger{}
- var pinned pinnedResult
+ var pinned worklowPinningResult
  _, err = validateGitHubActionWorkflow(tt.filename, content, &dl, &pinned)
  if err != nil {
  t.Errorf("error during validateGitHubActionWorkflow: %w", err)

checks/testdata/workflow-mix-github-and-non-github-not-pinned.yaml

@@ -0,0 +1,38 @@
+# 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.
+on:
+ push:
+ paths:
+ - 'source/common/**'
+ pull_request:
+
+jobs:
+ Some-Build:
+
+ strategy:
+ fail-fast: false
+
+ # CodeQL runs on ubuntu-latest and windows-latest
+ runs-on: ubuntu-latest
+
+ steps:
+ - name: Checkout repository
+ uses: actions/checkout@v2
+ with:
+ fetch-depth: 2
+
+ - name: Acme CodeQL
+ uses: acme/codeql-action/init@v2 # some comment
+ with:
+ languages: cpp

checks/testdata/workflow-mix-github-and-non-github-pinned.yaml

@@ -0,0 +1,38 @@
+# 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.
+on:
+ push:
+ paths:
+ - 'source/common/**'
+ pull_request:
+
+jobs:
+ Some-Build:
+
+ strategy:
+ fail-fast: false
+
+ # CodeQL runs on ubuntu-latest and windows-latest
+ runs-on: ubuntu-latest
+
+ steps:
+ - name: Checkout repository
+ uses: actions/checkout@daadedc81d5f9d3c06d2c92f49202a3cc2b919ba
+ with:
+ fetch-depth: 2
+
+ - name: Acme CodeQL
+ uses: acme/codeql-action/init@daadedc81d5f9d3c06d2c92f49202a3cc2b919ba
+ with:
+ languages: cpp