Skip to content

Commit

Permalink
✨ Give low importance to github-owned actions (#802)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
nanikjava committed Sep 5, 2021
1 parent 2ae8910 commit d007f73
Show file tree
Hide file tree
Showing 7 changed files with 377 additions and 16 deletions.
83 changes: 72 additions & 11 deletions checks/pinned_dependencies.go
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -58,6 +57,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 {
Expand Down Expand Up @@ -161,6 +167,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 {
Expand Down Expand Up @@ -496,31 +540,33 @@ func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []by

// 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)
}

// Check file content.
func validateGitHubActionWorkflow(pathfn string, content []byte,
dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := dataAsResultPointer(data)
pdata := dataAsWorkflowResultPointer(data)

if !CheckFileContainsCommands(content, "#") {
addPinnedResult(pdata, true)
addWorkflowPinnedResult(pdata, true, true)
addWorkflowPinnedResult(pdata, true, true)
return true, nil
}

Expand All @@ -533,7 +579,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
Expand All @@ -544,20 +589,36 @@ 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
}

// 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
Expand Down
101 changes: 96 additions & 5 deletions checks/pinned_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestGithubWorkflowPinning(t *testing.T) {
Error: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -50,7 +50,7 @@ func TestGithubWorkflowPinning(t *testing.T) {
Error: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -61,7 +61,7 @@ func TestGithubWorkflowPinning(t *testing.T) {
Error: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -70,12 +70,103 @@ 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
if tt.filename == "" {
content = make([]byte, 0)
} else {
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
Expand Down Expand Up @@ -624,7 +715,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)
Expand Down
38 changes: 38 additions & 0 deletions checks/testdata/workflow-mix-github-and-non-github-not-pinned.yaml
Original file line number Diff line number Diff line change
@@ -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
38 changes: 38 additions & 0 deletions checks/testdata/workflow-mix-github-and-non-github-pinned.yaml
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit d007f73

Please sign in to comment.