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 non github action

* Modify existing test as score calculation has changed
  • Loading branch information
nanikjava committed Aug 31, 2021
1 parent d6ba2cd commit 52142d3
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 11 deletions.
72 changes: 63 additions & 9 deletions checks/pinned_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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 +168,37 @@ 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
}

if r.thirdParties != notPinned {
score += 8
}

if r.gitHubOwned == pinned {
dl.Info(infoMsg)
}

return score, nil
}

func dataAsResultPointer(data FileCbData) *pinnedResult {
pdata, ok := data.(*pinnedResult)
if !ok {
Expand Down Expand Up @@ -496,31 +534,32 @@ 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,
func createReturnForIsGitHubActionsWorkflowPinned(r worklowPinningResult, dl checker.DetailLogger, err error) (int, error) {
return createReturnValuesForGitHubActionsWorkflowPinned(r,
"GitHub 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 +572,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 +582,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 bool, 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
62 changes: 60 additions & 2 deletions checks/pinned_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ 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: 0,
NumberOfDebug: 0,
Expand Down Expand Up @@ -104,6 +104,64 @@ func TestGithubWorkflowPinning(t *testing.T) {
}
}

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: 0,
NumberOfDebug: 0,
},
},
{
name: "Pinned github workflow",
filename: "./testdata/workflow-non-github-not-pinned.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
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 TestDockerfilePinning(t *testing.T) {
t.Parallel()
tests := []struct {
Expand Down Expand Up @@ -624,7 +682,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
48 changes: 48 additions & 0 deletions checks/testdata/workflow-non-github-not-pinned.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# 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:
CodeQL-Build:

strategy:
fail-fast: false

# CodeQL runs on ubuntu-latest and windows-latest
runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: action/checkout@daadedc81d5f9d3c06d2c92f49202a3cc2b919ba
with:
# We must fetch at least the immediate parents so that if this is
# a pull request then we can checkout the head.
fetch-depth: 2

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@daadedc81d5f9d3c06d2c92f49202a3cc2b919ba # some comment
# Override language selection by uncommenting this and choosing your languages
with:
languages: cpp

# acme something
- name: Initialize Acme Something
uses: acme/something/init@v1
with:
languages: c
42 changes: 42 additions & 0 deletions checks/testdata/workflow-non-github-pinned.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# 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:
CodeQL-Build:

strategy:
fail-fast: false

# CodeQL runs on ubuntu-latest and windows-latest
runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: acme/checkout@daadedc81d5f9d3c06d2c92f49202a3cc2b919ba
with:
# We must fetch at least the immediate parents so that if this is
# a pull request then we can checkout the head.
fetch-depth: 2

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: acme/codeql-action/init@daadedc81d5f9d3c06d2c92f49202a3cc2b919ba # some comment
# Override language selection by uncommenting this and choosing your languages
with:
languages: cpp

0 comments on commit 52142d3

Please sign in to comment.