enable gosec (#147)

* enable gosec

Signed-off-by: Kim Tsao <[email protected]>

* enable gosec - fix errors

Signed-off-by: Kim Tsao <[email protected]>

* enable gosec - switch to using makefile rule to avoid setting GOROOT

Signed-off-by: Kim Tsao <[email protected]>

* update PR template

Signed-off-by: Kim Tsao <[email protected]>

* Switch to Sarif

Signed-off-by: Kim Tsao <[email protected]>

* Update deprecated action

Signed-off-by: Kim Tsao <[email protected]>

* Handle gosec warnings

Signed-off-by: Kim Tsao <[email protected]>

* Handle gosec errors

Signed-off-by: Kim Tsao <[email protected]>

Signed-off-by: Kim Tsao <[email protected]>

Author: kim-tsao

.github/PULL_REQUEST_TEMPLATE.md

@@ -31,5 +31,8 @@ Testing and documentation do not need to be complete in order for this PR to be
 
   <!-- _Do we have anything that can break our clients?  If so, open a notifying issue_ -->
 
+- [ ] Gosec scans
+  <!-- _Review scan results from the PR.  Fix all MEDIUM and higher findings and/or annotate a finding per gosec instructions: https://github.com/securego/gosec#annotating-code to address why a finding is not a security issue_-->
+
 
 ### How to test changes / Special notes to the reviewer:

.github/workflows/go.yml

@@ -58,5 +58,21 @@ jobs:
     - name: Run Go Tests
       run: make test
 
+    - name: Run Gosec Security Scanner
+      run: |
+        go install github.com/securego/gosec/v2/cmd/gosec@latest
+        make gosec
+        if [[ $? != 0 ]]
+        then
+          echo "gosec scanner failed to run "
+          exit 1
+        fi   
+
+    - name: Upload SARIF file
+      uses: github/codeql-action/upload-sarif@v2
+      with:
+        # Path to SARIF file relative to the root of the repository
+        sarif_file: gosec.sarif
+
     - name: Upload coverage to Codecov
       uses: codecov/[email protected]

Makefile

@@ -31,17 +31,24 @@ ifneq ($(shell command -v addlicense 2> /dev/null),)
 	@echo 'addlicense -v -f license_header.txt **/*.go'
 	@addlicense -v -f license_header.txt $$(find . -name '*.go')
 else
-	$(error addlicense must be installed for this rule: go get -u github.com/google/addlicense)
+	$(error "addlicense must be installed for this command: go install github.com/google/addlicense@latest")
 endif
 
 ### check_fmt: Checks for missing licenses on files in repo
 check_license:
   ifeq ($(shell command -v addlicense 2> /dev/null),)
-	  $(error "error addlicense must be installed for this rule: go get -u github.com/google/addlicense")
+	  $(error "error addlicense must be installed for this command: go install github.com/google/addlicense@latest")
   endif
 
 	  if ! addlicense -check -f license_header.txt $$(find . -not -path '*/\.*' -name '*.go'); then \
 	    echo "Licenses are not formatted; run 'make fmt_license'"; exit 1 ;\
 	  fi \
 
 
+
+### gosec - runs the gosec scanner for non-test files in this repo
+.PHONY: gosec
+gosec:
+	# Run this command to install gosec, if not installed:
+	# go install github.com/securego/gosec/v2/cmd/gosec@latest
+	gosec -no-fail -fmt=sarif -out=gosec.sarif -exclude-dir pkg/testingutil -exclude-dir tests ./...

pkg/devfile/parser/configurables.go

@@ -105,7 +105,7 @@ func (d DevfileObj) SetMemory(memory string) error {
 	for _, component := range components {
 		if component.Container != nil {
 			component.Container.MemoryLimit = memory
-			d.Data.UpdateComponent(component)
+			_ = d.Data.UpdateComponent(component)
 		}
 	}
 	return d.WriteYamlDevfile()

pkg/devfile/parser/data/v2/containers.go

@@ -36,7 +36,7 @@ func (d *DevfileV2) AddEnvVars(containerEnvMap map[string][]v1alpha2.EnvVar) err
 	for _, component := range components {
 		if component.Container != nil {
 			component.Container.Env = merge(component.Container.Env, containerEnvMap[component.Name])
-			d.UpdateComponent(component)
+			_ = d.UpdateComponent(component)
 		}
 	}
 	return nil
@@ -55,7 +55,7 @@ func (d *DevfileV2) RemoveEnvVars(containerEnvMap map[string][]string) error {
 			if err != nil {
 				return err
 			}
-			d.UpdateComponent(component)
+			_ = d.UpdateComponent(component)
 		}
 	}
 	return nil
@@ -76,7 +76,7 @@ func (d *DevfileV2) SetPorts(containerPortsMap map[string][]string) error {
 		}
 		if component.Container != nil {
 			component.Container.Endpoints = addEndpoints(component.Container.Endpoints, endpoints)
-			d.UpdateComponent(component)
+			_ = d.UpdateComponent(component)
 		}
 	}
 
@@ -97,7 +97,7 @@ func (d *DevfileV2) RemovePorts(containerPortsMap map[string][]string) error {
 			if err != nil {
 				return err
 			}
-			d.UpdateComponent(component)
+			_ = d.UpdateComponent(component)
 		}
 	}
 

pkg/devfile/parser/parse.go

@@ -742,6 +742,7 @@ func parseKubeResourceFromURI(devObj DevfileObj) error {
 	}
 	for _, kubeComp := range kubeComponents {
 		if kubeComp.Kubernetes != nil && kubeComp.Kubernetes.Uri != "" {
+			/* #nosec G601 -- not an issue, kubeComp is de-referenced in sequence*/
 			err := convertK8sLikeCompUriToInlined(&kubeComp, devObj.Ctx)
 			if err != nil {
 				return errors.Wrapf(err, "failed to convert Kubernetes Uri to inlined for component '%s'", kubeComp.Name)
@@ -754,6 +755,7 @@ func parseKubeResourceFromURI(devObj DevfileObj) error {
 	}
 	for _, openshiftComp := range openshiftComponents {
 		if openshiftComp.Openshift != nil && openshiftComp.Openshift.Uri != "" {
+			/* #nosec G601 -- not an issue, openshiftComp is de-referenced in sequence*/
 			err := convertK8sLikeCompUriToInlined(&openshiftComp, devObj.Ctx)
 			if err != nil {
 				return errors.Wrapf(err, "failed to convert Openshift Uri to inlined for component '%s'", openshiftComp.Name)

pkg/util/util.go

@@ -494,7 +494,7 @@ func GetIgnoreRulesFromDirectory(directory string) ([]string, error) {
 		}
 	}
 
-	file, err := os.Open(pathIgnore)
+	file, err := os.Open(filepath.Clean(pathIgnore))
 	if err != nil {
 		return nil, err
 	}
@@ -707,7 +707,7 @@ func HTTPGetFreePort() (int, error) {
 // shamelessly taken from: https://stackoverflow.com/questions/30697324/how-to-check-if-directory-on-path-is-empty
 // this helps detect any edge cases where an empty directory is copied over
 func IsEmpty(name string) (bool, error) {
-	f, err := os.Open(name)
+	f, err := os.Open(filepath.Clean(name))
 	if err != nil {
 		return false, err
 	}
@@ -997,7 +997,7 @@ func Unzip(src, dest, pathToUnzip string) ([]string, error) {
 			return filenames, err
 		}
 
-		outFile, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, ModeReadWriteFile)
+		outFile, err := os.OpenFile(filepath.Clean(fpath), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, ModeReadWriteFile)
 		if err != nil {
 			return filenames, err
 		}
@@ -1015,8 +1015,8 @@ func Unzip(src, dest, pathToUnzip string) ([]string, error) {
 		_, err = io.Copy(outFile, limited)
 
 		// Close the file without defer to close before next iteration of loop
-		outFile.Close()
-		rc.Close()
+		_ = outFile.Close()
+		_ = rc.Close()
 
 		if err != nil {
 			return filenames, err
@@ -1214,14 +1214,14 @@ func CopyFile(srcPath string, dstPath string, info os.FileInfo) error {
 	}
 
 	// Open source file
-	srcFile, err := os.Open(srcPath)
+	srcFile, err := os.Open(filepath.Clean(srcPath))
 	if err != nil {
 		return err
 	}
 	defer srcFile.Close() // #nosec G307
 
 	// Create destination file
-	dstFile, err := os.Create(dstPath)
+	dstFile, err := os.Create(filepath.Clean(dstPath))
 	if err != nil {
 		return err
 	}

replaceSchemaFile.go

@@ -38,7 +38,7 @@ func ReplaceSchemaFile() {
 	fmt.Printf("Writing to file: %s\n", filePath)
 	fileContent := fmt.Sprintf("package %s\n\n// %s\nconst %s = `%s\n`\n", packageVersion, schemaURL, jsonSchemaVersion, newSchema)
 
-	if err := ioutil.WriteFile(filePath, []byte(fileContent), 0755); err != nil {
+	if err := ioutil.WriteFile(filePath, []byte(fileContent), 0644); err != nil {
 		printErr(err)
 		os.Exit(1)
 	}