From 92c6dd41b2c00a34d743b7a34cbe9d9a462963a4 Mon Sep 17 00:00:00 2001 From: Templum Date: Sat, 17 Sep 2022 22:48:21 +0000 Subject: [PATCH 01/15] :heavy_plus_sign: Added github.com/stretchr/testify --- go.mod | 4 ++++ go.sum | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/go.mod b/go.mod index 8c62cc2..bd0a3d3 100644 --- a/go.mod +++ b/go.mod @@ -5,20 +5,24 @@ go 1.19 require golang.org/x/vuln v0.0.0-20220914160157-cac67f5c7c81 require ( + github.com/davecgh/go-spew v1.1.1 // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/mattn/go-colorable v0.1.12 // indirect github.com/mattn/go-isatty v0.0.14 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 // indirect golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.28.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) require ( github.com/google/go-github/v47 v47.0.0 github.com/owenrumney/go-sarif/v2 v2.1.2 github.com/rs/zerolog v1.28.0 + github.com/stretchr/testify v1.8.0 golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect golang.org/x/oauth2 v0.0.0-20220909003341-f21342109be1 diff --git a/go.sum b/go.sum index 538dac3..ed571c0 100644 --- a/go.sum +++ b/go.sum @@ -36,8 +36,12 @@ github.com/rs/xid v1.4.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.28.0 h1:MirSo27VyNi7RJYP3078AA1+Cyzd2GB66qy3aUHvsWY= github.com/rs/zerolog v1.28.0/go.mod h1:NILgTygv/Uej1ra5XxGf82ZFSLk58MFGAUS2o6usyD0= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI= github.com/zclconf/go-cty v1.10.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk= @@ -79,5 +83,7 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.2.2 h1:MNh1AVMyVX23VUHE2O27jm6lNj3vjO5DexS4A1xvnzk= mvdan.cc/unparam v0.0.0-20211214103731-d0ef000c54e5 h1:Jh3LAeMt1eGpxomyu3jVkmVZWW2MxZ1qIIV2TZ/nRio= From 6c4c937954d282211b59bf247b0de7785ed94f0e Mon Sep 17 00:00:00 2001 From: Templum Date: Sat, 17 Sep 2022 22:54:15 +0000 Subject: [PATCH 02/15] :white_check_mark: Added Test for FindVulnerableCallSite --- pkg/action/preprocessor_test.go | 105 ++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 pkg/action/preprocessor_test.go diff --git a/pkg/action/preprocessor_test.go b/pkg/action/preprocessor_test.go new file mode 100644 index 0000000..0bc1f0d --- /dev/null +++ b/pkg/action/preprocessor_test.go @@ -0,0 +1,105 @@ +package action + +import ( + "go/token" + "testing" + + "github.com/stretchr/testify/assert" + "golang.org/x/vuln/vulncheck" +) + +func TestFindVulnerableCallSite(t *testing.T) { + userCallSite := vulncheck.StackEntry{ + Function: &vulncheck.FuncNode{ + ID: 2, + Name: "Testcase", + RecvType: "", + PkgPath: "github.com/Templum/playground/pkg/json", + Pos: &token.Position{ + Filename: "/workspaces/unit/pkg/json/testcase.go", + Offset: 130, + Line: 10, + Column: 6, + }, + CallSites: []*vulncheck.CallSite{}, // Not needed for this function + }, + Call: &vulncheck.CallSite{ + Parent: 2, + Name: "Get", + RecvType: "", + Resolved: true, + Pos: &token.Position{ + Filename: "/workspaces/unit/pkg/json/testcase.go", + Offset: 162, + Line: 11, + Column: 20, + }, + }, + } + + stack := []vulncheck.StackEntry{ + userCallSite, + { + Function: &vulncheck.FuncNode{ + ID: 12, + Name: "Get", + RecvType: "", + PkgPath: "github.com/tidwall/gjson", + Pos: &token.Position{ + Filename: "/go/pkg/mod/github.com/tidwall/gjson@v1.6.4/gjson.go", + Offset: 37859, + Line: 1873, + Column: 6, + }, + CallSites: []*vulncheck.CallSite{}, // Not needed for this function + }, + Call: &vulncheck.CallSite{ + Parent: 12, + Name: "parseObject", + RecvType: "", + Resolved: true, + Pos: &token.Position{ + Filename: "/go/pkg/mod/github.com/tidwall/gjson@v1.6.4/gjson.go", + Offset: 39894, + Line: 1963, + Column: 16, + }, + }, + }, + { + Function: &vulncheck.FuncNode{ + ID: 16, + Name: "parseObject", + RecvType: "", + PkgPath: "github.com/tidwall/gjson", + Pos: &token.Position{ + Filename: "/go/pkg/mod/github.com/tidwall/gjson@v1.6.4/gjson.go", + Offset: 21927, + Line: 1114, + Column: 2, + }, + CallSites: []*vulncheck.CallSite{}, // Not needed for this function + }, + Call: nil, + }, // Vulnerability + } + + t.Run("should return empty entry if nothing is found", func(t *testing.T) { + callSite := FindVulnerableCallSite("/workspaces/other", stack) + + assert.Nil(t, callSite.Call, "should have no call") + assert.Nil(t, callSite.Function, "should have no function") + }) + + t.Run("should return first calling site located in user code", func(t *testing.T) { + callSite := FindVulnerableCallSite("/workspaces/unit", stack) + + assert.NotNil(t, callSite.Call, "should have a call") + assert.NotNil(t, callSite.Function, "should have a function") + assert.Equal(t, userCallSite, callSite, "should find the correct call site") + }) +} + +func TestVulncheckProcessor_RemoveDuplicates(t *testing.T) { + +} From fa0f1d6ad9fc688ddcac37f8fe4067974853a92c Mon Sep 17 00:00:00 2001 From: Templum Date: Sat, 17 Sep 2022 22:54:47 +0000 Subject: [PATCH 03/15] :construction_worker: Added Unit Testing to Pipeline Also renamed file to be more reflective of that change --- .github/workflows/{build.yml => ci.yml} | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) rename .github/workflows/{build.yml => ci.yml} (51%) diff --git a/.github/workflows/build.yml b/.github/workflows/ci.yml similarity index 51% rename from .github/workflows/build.yml rename to .github/workflows/ci.yml index 22fa6dd..62e4345 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/ci.yml @@ -19,3 +19,21 @@ jobs: go-version: 1.19 - name: Build run: go build -v ./... + unit-testing: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + 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 + # If this run was triggered by a pull request event, then checkout + # the head of the pull request instead of the merge commit. + - run: git checkout HEAD^2 + if: ${{ github.event_name == 'pull_request' }} + - name: Set up Go + uses: actions/setup-go@v3 + with: + go-version: 1.19 + - name: Build + run: go test -race ./... \ No newline at end of file From 63ab256946c5a1f39fc9b9c29025c53767c81217 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 14:09:40 +0000 Subject: [PATCH 04/15] :fire: Removed all action description --- hack/old.action.yml | 23 ----------------------- 1 file changed, 23 deletions(-) delete mode 100644 hack/old.action.yml diff --git a/hack/old.action.yml b/hack/old.action.yml deleted file mode 100644 index 5c051c4..0000000 --- a/hack/old.action.yml +++ /dev/null @@ -1,23 +0,0 @@ -name: "Golang Vulncheck" -description: "Performs vulnerability scan using govulncheck and afterwards uploads it as Sarif Report to Github" -author: "Templum" -inputs: - package: - description: "The package you want to scan, by default will be ./..." - required: false - default: "./..." - github-token: - description: "Github App token to upload sarif report. Needs write permissions for security_events. By default it will use 'github.token' value" - default: ${{ github.token }} - required: true - -runs: - using: "docker" - image: "Dockerfile" - env: - GITHUB_TOKEN: "${{ inputs.github-token }}" - PACKAGE: "${{ inputs.package }}" - -branding: - icon: "alert-octagon" - color: "red" From 074fbf990815d58a0fc5eec92834e7b7542ef786 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 14:10:13 +0000 Subject: [PATCH 05/15] :white_check_mark: Implemented RemoveDuplicates Test --- pkg/action/preprocessor_test.go | 51 +++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/pkg/action/preprocessor_test.go b/pkg/action/preprocessor_test.go index 0bc1f0d..096e179 100644 --- a/pkg/action/preprocessor_test.go +++ b/pkg/action/preprocessor_test.go @@ -2,8 +2,12 @@ package action import ( "go/token" + "path" "testing" + "github.com/Templum/govulncheck-action/pkg/types" + helper "github.com/Templum/govulncheck-action/pkg/vulncheck" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "golang.org/x/vuln/vulncheck" ) @@ -100,6 +104,53 @@ func TestFindVulnerableCallSite(t *testing.T) { }) } +func CalculateTotalFindings(input types.VulnerableStacks) int { + output := 0 + + for _, findings := range input { + output += len(findings) + } + + return output +} + func TestVulncheckProcessor_RemoveDuplicates(t *testing.T) { + scanner := helper.NewLocalScanner(zerolog.Nop(), path.Join("..", "..", "hack", "found.json")) + result, _ := scanner.Scan() + input := helper.Resolve(result) + + hasDuplicateCallsites := make(types.VulnerableStacks) + hasDuplicateVuln := make(types.VulnerableStacks) + + for key, value := range input { + if key.OSV.ID == "GO-2021-0113" { + hasDuplicateVuln[key] = value + } + + if key.OSV.ID == "GO-2021-0061" && key.Symbol == "decoder.unmarshal" { + hasDuplicateCallsites[key] = value + } + } + + t.Run("should remove duplicates which are called from the same site", func(t *testing.T) { + target := NewVulncheckProcessor() + target.workDir = "/workspaces/govulncheck-action" + + reduced := target.RemoveDuplicates(hasDuplicateCallsites) + + assert.NotNil(t, reduced, "should not be nil") + assert.Equal(t, len(reduced), len(hasDuplicateCallsites), "should have same amount of entries") + assert.Less(t, CalculateTotalFindings(reduced), CalculateTotalFindings(hasDuplicateCallsites), "reduced should be less after removal of duplicates") + }) + + t.Run("should remove duplicates which are for the same vulnerability", func(t *testing.T) { + target := NewVulncheckProcessor() + target.workDir = "/workspaces/govulncheck-action" + reduced := target.RemoveDuplicates(hasDuplicateVuln) + + assert.NotNil(t, reduced, "should not be nil") + assert.Less(t, len(reduced), len(hasDuplicateVuln), "should only have one entry now") + assert.Less(t, CalculateTotalFindings(reduced), CalculateTotalFindings(hasDuplicateVuln), "reduced should be less after removal of duplicates") + }) } From d7bc846d6f18b9696cf762a040714b25920efb61 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 14:10:34 +0000 Subject: [PATCH 06/15] :recycle: Simplified Error handling of UploadReport --- pkg/github/sarif_report.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/github/sarif_report.go b/pkg/github/sarif_report.go index 1bbc190..0a51192 100644 --- a/pkg/github/sarif_report.go +++ b/pkg/github/sarif_report.go @@ -7,7 +7,6 @@ import ( "context" "encoding/base64" "encoding/json" - "errors" "os" "strings" @@ -74,11 +73,7 @@ func (g *GithubSarifUploader) UploadReport(report types.Reporter) error { return nil } - if err != nil { - return err - } - - return errors.New("unexpected response from github") + return err } func (g *GithubSarifUploader) prepareReport(report types.Reporter) (string, error) { From b581d8f9261cfb11dfb9e974a5af2dcbfdddb2c0 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 14:10:56 +0000 Subject: [PATCH 07/15] :white_check_mark: Created Test for UploadReport --- pkg/github/sarif_report_test.go | 161 ++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 pkg/github/sarif_report_test.go diff --git a/pkg/github/sarif_report_test.go b/pkg/github/sarif_report_test.go new file mode 100644 index 0000000..ca20bec --- /dev/null +++ b/pkg/github/sarif_report_test.go @@ -0,0 +1,161 @@ +package github + +import ( + "bytes" + "compress/gzip" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "testing" + + "github.com/Templum/govulncheck-action/pkg/types" + "github.com/google/go-github/v47/github" + "github.com/owenrumney/go-sarif/v2/sarif" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" +) + +type MockReport struct{ fail bool } + +func NewMockReport(shouldFail bool) types.Reporter { + return &MockReport{fail: shouldFail} +} + +func (m *MockReport) Convert(result types.VulnerableStacks) error { + return nil +} + +func (m *MockReport) Write(dest io.Writer) error { + if m.fail { + return errors.New("version [1.1.1] is not supported") + } + + emptyReport, _ := sarif.New(sarif.Version210) + run := sarif.NewRunWithInformationURI("govulncheck", "") + run.Tool.Driver.WithVersion("0.0.1") + run.Tool.Driver.WithFullName("govulncheck") + run.ColumnKind = "utf16CodeUnits" + emptyReport.AddRun(run) + + return emptyReport.Write(dest) +} + +func ExtractSarifString(body io.ReadCloser) ([]byte, error) { + request, _ := io.ReadAll(body) + var report github.SarifAnalysis + + err := json.Unmarshal(request, &report) + if err != nil { + return []byte{}, err + } + + return []byte(*report.Sarif), nil +} + +func DecodeSarifString(base64String []byte) ([]byte, error) { + return base64.StdEncoding.DecodeString(string(base64String)) +} + +func DecompressSarifString(compressedString []byte) ([]byte, error) { + buffered := bytes.NewBuffer(compressedString) + gzipReader, _ := gzip.NewReader(buffered) + decompressed, err := io.ReadAll(gzipReader) + + if err != nil { + return []byte{}, err + } + return decompressed, err +} + +func TestMain(m *testing.M) { + os.Setenv("GITHUB_REPOSITORY", "Templum/playground") + os.Setenv("GITHUB_REF", "refs/heads/unit") + os.Setenv("GITHUB_SHA", "ffac537e6cbbf934b08745a378932722df287a53") + os.Setenv("GITHUB_TOKEN", "Token") + + exitVal := m.Run() + + os.Unsetenv("GITHUB_REPOSITORY") + os.Unsetenv("GITHUB_REF") + os.Unsetenv("GITHUB_SHA") + os.Unsetenv("GITHUB_TOKEN") + + os.Exit(exitVal) +} + +func TestGithubSarifUploader_UploadReport(t *testing.T) { + uploadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + base64String, err := ExtractSarifString(r.Body) + if err != nil { + t.Errorf("Extracting Sarif String failed with %v", err) + } + + compressedString, err := DecodeSarifString(base64String) + if err != nil { + t.Errorf("Decoding Sarif String failed with %v", err) + } + + sarifReport, err := DecompressSarifString(compressedString) + if err != nil { + t.Errorf("Decompressing Sarif String failed with %v", err) + } + + if !strings.HasPrefix(string(sarifReport), "{\"version\":\"2.1.0\",\"$schema\":\"https://json.schemastore.org/sarif-2.1.0-rtm.5.json\"") { + t.Error("Sarif Report did not start as expected") + } + + response := github.SarifID{ + ID: github.String("0f971e9e-36d1-11ed-9b72-683377ed374"), + URL: github.String("https://api.github.com/repos/Templum/unit/code-scanning/analyses/43004097"), + } + out, _ := json.Marshal(response) + w.WriteHeader(202) + _, _ = w.Write(out) + })) + uploadUrl, _ := url.Parse(fmt.Sprintf("%s/", uploadServer.URL)) + defer uploadServer.Close() + + unreachableServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(502) + _, _ = w.Write([]byte{}) + })) + + unreachableUrl, _ := url.Parse(fmt.Sprintf("%s/", unreachableServer.URL)) + defer unreachableServer.Close() + + t.Run("should upload report as gzip compressed base64", func(t *testing.T) { + target := NewSarifUploader(zerolog.Nop()) + ref := target.(*GithubSarifUploader) + ref.client = github.NewClient(uploadServer.Client()) + ref.client.BaseURL = uploadUrl + + err := target.UploadReport(NewMockReport(false)) + assert.Nil(t, err, "should not fail") + }) + + t.Run("should return error if status code is not 202", func(t *testing.T) { + target := NewSarifUploader(zerolog.Nop()) + ref := target.(*GithubSarifUploader) + ref.client = github.NewClient(unreachableServer.Client()) + ref.client.BaseURL = unreachableUrl + + err := target.UploadReport(NewMockReport(false)) + assert.NotNil(t, err, "should fail") + assert.Contains(t, err.Error(), "502") + }) + + t.Run("should return received error if report writing fails", func(t *testing.T) { + target := NewSarifUploader(zerolog.Nop()) + + err := target.UploadReport(NewMockReport(true)) + assert.NotNil(t, err, "should fail") + assert.Contains(t, err.Error(), "version [1.1.1] is not supported") + }) +} From 52fe496061cc312df6ce6a5a18e5edc73e8460d0 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 14:24:38 +0000 Subject: [PATCH 08/15] :heavy_plus_sign: Added mock package --- go.mod | 1 + go.sum | 1 + 2 files changed, 2 insertions(+) diff --git a/go.mod b/go.mod index bd0a3d3..b48b547 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/mattn/go-colorable v0.1.12 // indirect github.com/mattn/go-isatty v0.0.14 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/objx v0.4.0 // indirect golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 // indirect golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/go.sum b/go.sum index ed571c0..d8a2220 100644 --- a/go.sum +++ b/go.sum @@ -36,6 +36,7 @@ github.com/rs/xid v1.4.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.28.0 h1:MirSo27VyNi7RJYP3078AA1+Cyzd2GB66qy3aUHvsWY= github.com/rs/zerolog v1.28.0/go.mod h1:NILgTygv/Uej1ra5XxGf82ZFSLk58MFGAUS2o6usyD0= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0 h1:M2gUjqZET1qApGOWNSnZ49BAIMX4F/1plDv3+l31EJ4= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= From 0d88ba140509442be98ff9535732e2fe146c0647 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 14:24:56 +0000 Subject: [PATCH 09/15] :recycle: Using mock pkg now in test --- pkg/github/sarif_report_test.go | 49 +++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/pkg/github/sarif_report_test.go b/pkg/github/sarif_report_test.go index ca20bec..64042c8 100644 --- a/pkg/github/sarif_report_test.go +++ b/pkg/github/sarif_report_test.go @@ -20,31 +20,32 @@ import ( "github.com/owenrumney/go-sarif/v2/sarif" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) -type MockReport struct{ fail bool } - -func NewMockReport(shouldFail bool) types.Reporter { - return &MockReport{fail: shouldFail} +type MockReport struct { + mock.Mock } func (m *MockReport) Convert(result types.VulnerableStacks) error { - return nil + args := m.Called(result) + return args.Error(0) } func (m *MockReport) Write(dest io.Writer) error { - if m.fail { - return errors.New("version [1.1.1] is not supported") + args := m.Called(dest) + + if args.Error(0) == nil { + emptyReport, _ := sarif.New(sarif.Version210) + run := sarif.NewRunWithInformationURI("govulncheck", "") + run.Tool.Driver.WithVersion("0.0.1") + run.Tool.Driver.WithFullName("govulncheck") + run.ColumnKind = "utf16CodeUnits" + emptyReport.AddRun(run) + _ = emptyReport.Write(dest) } - emptyReport, _ := sarif.New(sarif.Version210) - run := sarif.NewRunWithInformationURI("govulncheck", "") - run.Tool.Driver.WithVersion("0.0.1") - run.Tool.Driver.WithFullName("govulncheck") - run.ColumnKind = "utf16CodeUnits" - emptyReport.AddRun(run) - - return emptyReport.Write(dest) + return args.Error(0) } func ExtractSarifString(body io.ReadCloser) ([]byte, error) { @@ -136,8 +137,13 @@ func TestGithubSarifUploader_UploadReport(t *testing.T) { ref.client = github.NewClient(uploadServer.Client()) ref.client.BaseURL = uploadUrl - err := target.UploadReport(NewMockReport(false)) + mockReporter := new(MockReport) + mockReporter.On("Write", mock.Anything).Return(nil) + + err := target.UploadReport(mockReporter) + mockReporter.AssertExpectations(t) assert.Nil(t, err, "should not fail") + }) t.Run("should return error if status code is not 202", func(t *testing.T) { @@ -146,15 +152,22 @@ func TestGithubSarifUploader_UploadReport(t *testing.T) { ref.client = github.NewClient(unreachableServer.Client()) ref.client.BaseURL = unreachableUrl - err := target.UploadReport(NewMockReport(false)) + mockReporter := new(MockReport) + mockReporter.On("Write", mock.Anything).Return(nil) + + err := target.UploadReport(mockReporter) + mockReporter.AssertExpectations(t) assert.NotNil(t, err, "should fail") assert.Contains(t, err.Error(), "502") }) t.Run("should return received error if report writing fails", func(t *testing.T) { target := NewSarifUploader(zerolog.Nop()) + mockReporter := new(MockReport) + mockReporter.On("Write", mock.Anything).Return(errors.New("version [1.1.1] is not supported")) - err := target.UploadReport(NewMockReport(true)) + err := target.UploadReport(mockReporter) + mockReporter.AssertExpectations(t) assert.NotNil(t, err, "should fail") assert.Contains(t, err.Error(), "version [1.1.1] is not supported") }) From 1a67a40f2c26e6e5a25ed71c782c1d41cf2b391d Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 14:26:05 +0000 Subject: [PATCH 10/15] :wrench: Updated Names of steps --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62e4345..cde9bae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: uses: actions/setup-go@v3 with: go-version: 1.19 - - name: Build + - name: Compile Action run: go build -v ./... unit-testing: runs-on: ubuntu-latest @@ -35,5 +35,5 @@ jobs: uses: actions/setup-go@v3 with: go-version: 1.19 - - name: Build + - name: Run Unittest run: go test -race ./... \ No newline at end of file From f6bf5dc452f6f91311e3e4883a3ceb9cf6071497 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 18:57:42 +0000 Subject: [PATCH 11/15] :recycle: Workdir is provided now --- pkg/action/preprocessor.go | 5 +---- pkg/action/preprocessor_test.go | 8 ++------ pkg/sarif/reporter.go | 23 +++++++---------------- pkg/vulncheck/runner.go | 13 ++++++------- 4 files changed, 16 insertions(+), 33 deletions(-) diff --git a/pkg/action/preprocessor.go b/pkg/action/preprocessor.go index cdbc955..d877883 100644 --- a/pkg/action/preprocessor.go +++ b/pkg/action/preprocessor.go @@ -1,7 +1,6 @@ package action import ( - "os" "strings" "github.com/Templum/govulncheck-action/pkg/types" @@ -12,9 +11,7 @@ type VulncheckProcessor struct { workDir string } -func NewVulncheckProcessor() *VulncheckProcessor { - workDir, _ := os.Getwd() - +func NewVulncheckProcessor(workDir string) *VulncheckProcessor { return &VulncheckProcessor{ workDir: workDir, } diff --git a/pkg/action/preprocessor_test.go b/pkg/action/preprocessor_test.go index 096e179..500f8f5 100644 --- a/pkg/action/preprocessor_test.go +++ b/pkg/action/preprocessor_test.go @@ -133,9 +133,7 @@ func TestVulncheckProcessor_RemoveDuplicates(t *testing.T) { } t.Run("should remove duplicates which are called from the same site", func(t *testing.T) { - target := NewVulncheckProcessor() - target.workDir = "/workspaces/govulncheck-action" - + target := NewVulncheckProcessor("/workspaces/govulncheck-action") reduced := target.RemoveDuplicates(hasDuplicateCallsites) assert.NotNil(t, reduced, "should not be nil") @@ -144,9 +142,7 @@ func TestVulncheckProcessor_RemoveDuplicates(t *testing.T) { }) t.Run("should remove duplicates which are for the same vulnerability", func(t *testing.T) { - target := NewVulncheckProcessor() - target.workDir = "/workspaces/govulncheck-action" - + target := NewVulncheckProcessor("/workspaces/govulncheck-action") reduced := target.RemoveDuplicates(hasDuplicateVuln) assert.NotNil(t, reduced, "should not be nil") diff --git a/pkg/sarif/reporter.go b/pkg/sarif/reporter.go index 3ceca2b..885bab4 100644 --- a/pkg/sarif/reporter.go +++ b/pkg/sarif/reporter.go @@ -32,16 +32,12 @@ type SarifReporter struct { workDir string } -func NewSarifReporter(logger zerolog.Logger) types.Reporter { - localDir, _ := os.Getwd() - - return &SarifReporter{report: nil, run: nil, log: logger, workDir: localDir} +func NewSarifReporter(logger zerolog.Logger, workDir string) types.Reporter { + return &SarifReporter{report: nil, run: nil, log: logger, workDir: workDir} } func (sr *SarifReporter) Convert(result types.VulnerableStacks) error { - if err := sr.createEmptyReport("initial"); err != nil { - return fmt.Errorf("failed to create an empty sarif report due to %v", err) - } + sr.createEmptyReport("initial") sr.log.Debug().Msgf("Scan showed code being impacted by %d vulnerabilities", len(result)) for vuln, callStacks := range result { @@ -63,26 +59,21 @@ func (sr *SarifReporter) Convert(result types.VulnerableStacks) error { } func (sr *SarifReporter) Write(dest io.Writer) error { - sr.run.ColumnKind = "utf16CodeUnits" sr.report.AddRun(sr.run) return sr.report.PrettyWrite(dest) } -func (sr *SarifReporter) createEmptyReport(vulncheckVersion string) error { - report, err := sarif.New(sarif.Version210) - if err != nil { - return err - } +func (sr *SarifReporter) createEmptyReport(vulncheckVersion string) { + report, _ := sarif.New(sarif.Version210) run := sarif.NewRunWithInformationURI(shortName, uri) run.Tool.Driver.WithVersion("0.0.1") // TODO: Get version from tag run.Tool.Driver.WithFullName(fullName) + run.ColumnKind = "utf16CodeUnits" sr.report = report sr.run = run - - return nil } func (sr *SarifReporter) addRule(vuln *vulncheck.Vuln) { @@ -200,7 +191,7 @@ func (sr *SarifReporter) generateResultMessage(vuln *vulncheck.Vuln, entry vulnc markBuilder.WriteString("Stacktrace: \n") - for _, line := range types.Stack(stack) { + for _, line := range types.FormatCallStack(stack) { txtBuilder.WriteString(fmt.Sprintf("%s \n", line)) markBuilder.WriteString(fmt.Sprintf("* %s \n", line)) } diff --git a/pkg/vulncheck/runner.go b/pkg/vulncheck/runner.go index e790bd1..693e7b4 100644 --- a/pkg/vulncheck/runner.go +++ b/pkg/vulncheck/runner.go @@ -21,21 +21,20 @@ type Scanner interface { } type CmdScanner struct { - log zerolog.Logger + log zerolog.Logger + workDir string } -func NewScanner(logger zerolog.Logger) Scanner { - return &CmdScanner{log: logger} +func NewScanner(logger zerolog.Logger, workDir string) Scanner { + return &CmdScanner{log: logger, workDir: workDir} } func (r *CmdScanner) Scan() (*vulncheck.Result, error) { pkg := os.Getenv(envPackage) - workDir, _ := os.Getwd() - - r.log.Info().Msgf("Running govulncheck for package %s in dir %s", pkg, workDir) + r.log.Info().Msgf("Running govulncheck for package %s in dir %s", pkg, r.workDir) cmd := exec.Command(command, flag, pkg) - cmd.Dir = workDir + cmd.Dir = r.workDir out, cmdErr := cmd.Output() if err, ok := cmdErr.(*exec.ExitError); ok { From 774bcd3a7abfecc132550f032e6d21e20ab4e347 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 18:58:17 +0000 Subject: [PATCH 12/15] :coffin: Removed unused function --- pkg/types/call_chain.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/pkg/types/call_chain.go b/pkg/types/call_chain.go index 54481f9..4ee046f 100644 --- a/pkg/types/call_chain.go +++ b/pkg/types/call_chain.go @@ -3,7 +3,6 @@ package types import ( "fmt" - "github.com/rs/zerolog" "golang.org/x/vuln/vulncheck" ) @@ -29,13 +28,7 @@ func (c *CallChain) CreateCallStack() vulncheck.CallStack { return append(vulncheck.CallStack{vulncheck.StackEntry{Function: c.Fn, Call: c.Called}}, c.Child.CreateCallStack()...) } -func PrintStack(log zerolog.Logger, stack vulncheck.CallStack) { - for _, line := range Stack(stack) { - log.Info().Msg(line) - } -} - -func Stack(stack vulncheck.CallStack) []string { +func FormatCallStack(stack vulncheck.CallStack) []string { var output []string for i, current := range stack { From d43b1af1f54bcacf46d8ddad67cbf356477b1877 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 18:58:32 +0000 Subject: [PATCH 13/15] :sparkles: Workdir is provided by main now --- main.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index 4e59f34..42a0513 100644 --- a/main.go +++ b/main.go @@ -13,16 +13,17 @@ import ( func main() { zerolog.SetGlobalLevel(zerolog.InfoLevel) - logger := zerolog.New(zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: zerolog.TimeFormatUnix}). With(). Timestamp(). Logger() // Main Logger - reporter := sarif.NewSarifReporter(logger) + workDir, _ := os.Getwd() + github := github.NewSarifUploader(logger) - scanner := vulncheck.NewScanner(logger) - processor := action.NewVulncheckProcessor() + reporter := sarif.NewSarifReporter(logger, workDir) + scanner := vulncheck.NewScanner(logger, workDir) + processor := action.NewVulncheckProcessor(workDir) if os.Getenv("DEBUG") == "true" { zerolog.SetGlobalLevel(zerolog.DebugLevel) From 148d53a1f37363a52ab8fbbe1ed2805c644e99f0 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 18:58:56 +0000 Subject: [PATCH 14/15] :white_check_mark: Added Test for Sarif Reporter --- pkg/sarif/reporter_test.go | 84 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 pkg/sarif/reporter_test.go diff --git a/pkg/sarif/reporter_test.go b/pkg/sarif/reporter_test.go new file mode 100644 index 0000000..fb4a42d --- /dev/null +++ b/pkg/sarif/reporter_test.go @@ -0,0 +1,84 @@ +package sarif + +import ( + "bytes" + "encoding/json" + "io" + "path" + "testing" + + "github.com/Templum/govulncheck-action/pkg/action" + "github.com/Templum/govulncheck-action/pkg/types" + helper "github.com/Templum/govulncheck-action/pkg/vulncheck" + "github.com/owenrumney/go-sarif/v2/sarif" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" +) + +func TestSarifReporter_Convert(t *testing.T) { + scanner := helper.NewLocalScanner(zerolog.Nop(), path.Join("..", "..", "hack", "found.json")) + preprocessor := action.NewVulncheckProcessor("/workspaces/govulncheck-action") + result, _ := scanner.Scan() + input := helper.Resolve(result) + input = preprocessor.RemoveDuplicates(input) + + t.Run("Should convert a preprocessed report into sarif format", func(t *testing.T) { + target := NewSarifReporter(zerolog.Nop(), "/workspaces/govulncheck-action") + ref := target.(*SarifReporter) + + _ = target.Convert(input) + + assert.NotNil(t, ref.report, "should have create an empty report") + assert.NotNil(t, ref.run, "should have filled a run with details") + + assert.GreaterOrEqual(t, len(ref.run.Results), 9, "example report should have 9 calls to vulnerabilities") + assert.GreaterOrEqual(t, len(ref.run.Tool.Driver.Rules), 6, "example report should have 6 vulnerabilities") + assert.Equal(t, len(ref.report.Runs), 0, "should have not yet added the run to the report") + }) + + t.Run("Should create a empty report if nothing was found", func(t *testing.T) { + target := NewSarifReporter(zerolog.Nop(), "/workspaces/govulncheck-action") + ref := target.(*SarifReporter) + + _ = target.Convert(make(types.VulnerableStacks)) + + assert.NotNil(t, ref.report, "should have create an empty report") + assert.NotNil(t, ref.run, "should have filled a run with details") + + assert.GreaterOrEqual(t, len(ref.run.Results), 0, "should not find call sites in an empty report") + assert.GreaterOrEqual(t, len(ref.run.Tool.Driver.Rules), 0, "should not find vulnerabilities in an empty report") + assert.Equal(t, len(ref.report.Runs), 0, "should have not yet added the run to the report") + }) +} + +func TestSarifReporter_Write(t *testing.T) { + t.Run("should add the run to the report before writing it", func(t *testing.T) { + target := NewSarifReporter(zerolog.Nop(), "/workspaces/govulncheck-action") + ref := target.(*SarifReporter) + ref.createEmptyReport("") + + assert.NotNil(t, ref.report, "should have create an empty report") + assert.NotNil(t, ref.run, "should have filled a run with details") + assert.Equal(t, len(ref.report.Runs), 0, "should have not yet added the run to the report") + + _ = target.Write(io.Discard) + assert.Equal(t, len(ref.report.Runs), 1, "should have added the run during the write") + }) + + t.Run("should write the report and run to the provided writer", func(t *testing.T) { + target := NewSarifReporter(zerolog.Nop(), "/workspaces/govulncheck-action") + ref := target.(*SarifReporter) + ref.createEmptyReport("") + + var writer bytes.Buffer + + _ = target.Write(&writer) + + assert.Greater(t, writer.Len(), 0, "should have written something to writer") + var report *sarif.Report + + err := json.Unmarshal(writer.Bytes(), &report) + + assert.Nil(t, err, "should be able to parse write output back into a sarif report") + }) +} From 6684e9e9a12ee40fbe09fe16465acf8eb78c57f6 Mon Sep 17 00:00:00 2001 From: Templum Date: Sun, 18 Sep 2022 19:09:02 +0000 Subject: [PATCH 15/15] :wrench: Added Code Coverage to CI Pipeline --- .github/workflows/ci.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cde9bae..620f3ef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,5 +35,15 @@ jobs: uses: actions/setup-go@v3 with: go-version: 1.19 - - name: Run Unittest - run: go test -race ./... \ No newline at end of file + - name: Run Unit Test with Racecondition Detector + run: go test -race ./... + - name: Run Unit Tests with Coverage + run: go test -coverprofile=coverage.txt -covermode=atomic -v ./... + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v3 + with: + # token is not needed for public repositories + files: coverage.txt + flags: unit-tests + name: codecov-action + fail_ci_if_error: false \ No newline at end of file