You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by as...@apache.org on 2021/11/24 12:52:57 UTC

[camel-k] 09/19: fix(gosec): deferring unsafe method "Close" on type "*os.File" (G307)

This is an automated email from the ASF dual-hosted git repository.

astefanutti pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel-k.git

commit 612620c7be0a7c515e6da359c46517aa7a4a2148
Author: Luca Burgazzoli <lb...@gmail.com>
AuthorDate: Mon Nov 22 21:12:20 2021 +0100

    fix(gosec): deferring unsafe method "Close" on type "*os.File" (G307)
---
 cmd/util/doc-gen/generators/traitdocgen.go      |  46 ++++-----
 cmd/util/doc-gen/generators/traitmetadatagen.go |  29 +++---
 pkg/builder/image.go                            |   3 +-
 pkg/builder/spectrum.go                         |   1 +
 pkg/cmd/dump.go                                 |  26 ++---
 pkg/cmd/init.go                                 |   9 +-
 pkg/util/digest/digest.go                       |  27 +++--
 pkg/util/maven/maven_command.go                 |   4 +-
 pkg/util/util.go                                | 128 +++++++++++++++++-------
 9 files changed, 157 insertions(+), 116 deletions(-)

diff --git a/cmd/util/doc-gen/generators/traitdocgen.go b/cmd/util/doc-gen/generators/traitdocgen.go
index 93c20c5..ad07dd3 100644
--- a/cmd/util/doc-gen/generators/traitdocgen.go
+++ b/cmd/util/doc-gen/generators/traitdocgen.go
@@ -90,17 +90,15 @@ func (g *traitDocGen) GenerateType(context *generator.Context, t *types.Type, ou
 
 	g.generatedTraitFiles = append(g.generatedTraitFiles, traitFile)
 
-	file, content, err := readFile(filename)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
+	return util.WithFileContent(filename, func(file *os.File, data []byte) error {
+		content := strings.Split(string(data), "\n")
 
-	writeTitle(traitID, &content)
-	writeDescription(t, traitID, &content)
-	writeFields(t, traitID, &content)
+		writeTitle(traitID, &content)
+		writeDescription(t, traitID, &content)
+		writeFields(t, traitID, &content)
 
-	return writeFile(file, content)
+		return writeFile(file, content)
+	})
 }
 
 func (g *traitDocGen) Finalize(c *generator.Context, w io.Writer) error {
@@ -112,25 +110,23 @@ func (g *traitDocGen) FinalizeNav(*generator.Context) error {
 	navPath := g.arguments.CustomArgs.(*CustomArgs).NavPath
 	filename := path.Join(docDir, navPath)
 
-	file, content, err := readFile(filename)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
+	return util.WithFileContent(filename, func(file *os.File, data []byte) error {
+		content := strings.Split(string(data), "\n")
 
-	pre, post := split(content, adocNavMarkerStart, adocNavMarkerEnd)
+		pre, post := split(content, adocNavMarkerStart, adocNavMarkerEnd)
 
-	content = append([]string(nil), pre...)
-	content = append(content, adocNavMarkerStart)
-	sort.Strings(g.generatedTraitFiles)
-	for _, t := range g.generatedTraitFiles {
-		name := traitNameFromFile(t)
-		content = append(content, "** xref:traits:"+t+"["+name+"]")
-	}
-	content = append(content, adocNavMarkerEnd)
-	content = append(content, post...)
+		content = append([]string(nil), pre...)
+		content = append(content, adocNavMarkerStart)
+		sort.Strings(g.generatedTraitFiles)
+		for _, t := range g.generatedTraitFiles {
+			name := traitNameFromFile(t)
+			content = append(content, "** xref:traits:"+t+"["+name+"]")
+		}
+		content = append(content, adocNavMarkerEnd)
+		content = append(content, post...)
 
-	return writeFile(file, content)
+		return writeFile(file, content)
+	})
 }
 
 func traitNameFromFile(file string) string {
diff --git a/cmd/util/doc-gen/generators/traitmetadatagen.go b/cmd/util/doc-gen/generators/traitmetadatagen.go
index 5358883..681b46a 100644
--- a/cmd/util/doc-gen/generators/traitmetadatagen.go
+++ b/cmd/util/doc-gen/generators/traitmetadatagen.go
@@ -88,27 +88,24 @@ func (g *traitMetaDataGen) GenerateType(context *generator.Context, t *types.Typ
 	return nil
 }
 
-func (g *traitMetaDataGen) Finalize(c *generator.Context, w io.Writer) error {
+func (g *traitMetaDataGen) Finalize(c *generator.Context, w io.Writer) (err error) {
 	deployDir := g.arguments.CustomArgs.(*CustomArgs).ResourceDir
 	traitFile := "traits.yaml"
 	filename := path.Join(deployDir, traitFile)
 
-	var file *os.File
-	var err error
-	if file, err = util.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0o777); err != nil {
-		return err
-	}
-	if err = file.Truncate(0); err != nil {
-		return err
-	}
-	defer file.Close()
+	return util.WithFile(filename, os.O_RDWR|os.O_CREATE, 0o777, func(file *os.File) error {
+		if err := file.Truncate(0); err != nil {
+			return err
+		}
 
-	data, err := yaml.Marshal(g.Root)
-	if err != nil {
-		fmt.Fprintf(file, "error: %v", err)
-	}
-	fmt.Fprintf(file, "%s", string(data))
-	return nil
+		data, err := yaml.Marshal(g.Root)
+		if err != nil {
+			fmt.Fprintf(file, "error: %v", err)
+		}
+		fmt.Fprintf(file, "%s", string(data))
+
+		return nil
+	})
 }
 
 func (g *traitMetaDataGen) getTraitID(t *types.Type) string {
diff --git a/pkg/builder/image.go b/pkg/builder/image.go
index 1eec4a7..3a200be 100644
--- a/pkg/builder/image.go
+++ b/pkg/builder/image.go
@@ -175,7 +175,8 @@ func imageContext(ctx *builderContext, selector artifactsSelector) error {
 
 	for _, entry := range ctx.Resources {
 		filePath, fileName := path.Split(entry.Target)
-		if err := util.WriteFileWithContent(path.Join(contextDir, filePath), fileName, entry.Content); err != nil {
+		fullPath := path.Join(contextDir, filePath, fileName)
+		if err := util.WriteFileWithContent(fullPath, entry.Content); err != nil {
 			return err
 		}
 	}
diff --git a/pkg/builder/spectrum.go b/pkg/builder/spectrum.go
index 95d3fa9..06f0ead 100644
--- a/pkg/builder/spectrum.go
+++ b/pkg/builder/spectrum.go
@@ -104,6 +104,7 @@ func (t *spectrumTask) Do(ctx context.Context) v1.BuildStatus {
 	}
 
 	newStdR, newStdW, pipeErr := os.Pipe()
+	// #nosec G307
 	defer newStdW.Close()
 
 	if pipeErr != nil {
diff --git a/pkg/cmd/dump.go b/pkg/cmd/dump.go
index fcdc6da..71cfbca 100644
--- a/pkg/cmd/dump.go
+++ b/pkg/cmd/dump.go
@@ -55,29 +55,21 @@ type dumpCmdOptions struct {
 	LogLines int `mapstructure:"logLines"`
 }
 
-func (o *dumpCmdOptions) dump(cmd *cobra.Command, args []string) error {
+func (o *dumpCmdOptions) dump(cmd *cobra.Command, args []string) (err error) {
 	c, err := o.GetCmdClient()
 	if err != nil {
-		return err
+		return
 	}
+
 	if len(args) == 1 {
-		fileName := args[0]
-		writer, err := util.OpenFile(fileName, os.O_RDWR|os.O_CREATE, 0o777)
-		if err != nil {
-			return err
-		}
-		err = dumpNamespace(o.Context, c, o.Namespace, writer, o.LogLines)
-		if err != nil {
-			return err
-		}
-		defer writer.Close()
+		err = util.WithFile(args[0], os.O_RDWR|os.O_CREATE, 0o644, func(file *os.File) error {
+			return dumpNamespace(o.Context, c, o.Namespace, file, o.LogLines)
+		})
 	} else {
-		err := dumpNamespace(o.Context, c, o.Namespace, cmd.OutOrStdout(), o.LogLines)
-		if err != nil {
-			return err
-		}
+		err = dumpNamespace(o.Context, c, o.Namespace, cmd.OutOrStdout(), o.LogLines)
 	}
-	return nil
+
+	return
 }
 
 func dumpNamespace(ctx context.Context, c client.Client, ns string, out io.Writer, logLines int) error {
diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go
index 0b30b0b..0a669e1 100644
--- a/pkg/cmd/init.go
+++ b/pkg/cmd/init.go
@@ -99,13 +99,10 @@ func (o *initCmdOptions) writeFromTemplate(language v1.Language, fileName string
 	if err != nil {
 		return err
 	}
-	out, err := util.OpenFile(fileName, os.O_RDWR|os.O_CREATE, 0o777)
-	if err != nil {
-		return err
-	}
-	defer out.Close()
 
-	return tmpl.Execute(out, params)
+	return util.WithFile(fileName, os.O_RDWR|os.O_CREATE, 0o644, func(file *os.File) error {
+		return tmpl.Execute(file, params)
+	})
 }
 
 func (o *initCmdOptions) extractLanguage(fileName string) *v1.Language {
diff --git a/pkg/util/digest/digest.go b/pkg/util/digest/digest.go
index f364b5c..1e62e02 100644
--- a/pkg/util/digest/digest.go
+++ b/pkg/util/digest/digest.go
@@ -18,7 +18,8 @@ limitations under the License.
 package digest
 
 import (
-	// nolint: gosec
+	// this is needed to generate an SHA1 sum for Jars
+	// #nosec G505
 	"crypto/sha1"
 	"crypto/sha256"
 	"encoding/base64"
@@ -262,17 +263,21 @@ func sortedTraitAnnotationsKeys(it *v1.Integration) []string {
 func ComputeSHA1(elem ...string) (string, error) {
 	file := path.Join(elem...)
 
-	f, err := util.Open(file)
-	if err != nil {
-		return "", err
-	}
-	defer f.Close()
-
 	// #nosec G401
 	h := sha1.New()
-	if _, err := io.Copy(h, f); err != nil {
-		return "", err
-	}
 
-	return base64.StdEncoding.EncodeToString(h.Sum(nil)), nil
+	err := util.WithFileReader(file, func(file io.Reader) error {
+		if _, err := io.Copy(h, file); err != nil {
+			return err
+		}
+
+		return nil
+	})
+
+	var sum string
+
+	if err != nil {
+		sum = base64.StdEncoding.EncodeToString(h.Sum(nil))
+	}
+	return sum, err
 }
diff --git a/pkg/util/maven/maven_command.go b/pkg/util/maven/maven_command.go
index b4ec58e..e42d9cd 100644
--- a/pkg/util/maven/maven_command.go
+++ b/pkg/util/maven/maven_command.go
@@ -198,7 +198,7 @@ func generateProjectStructure(context Context, project Project) error {
 	}
 
 	if context.SettingsContent != nil {
-		if err := util.WriteFileWithContent(context.Path, "settings.xml", context.SettingsContent); err != nil {
+		if err := util.WriteFileWithContent(path.Join(context.Path, "settings.xml"), context.SettingsContent); err != nil {
 			return err
 		}
 	}
@@ -221,7 +221,7 @@ func generateProjectStructure(context Context, project Project) error {
 		if len(bytes) > 0 {
 			Log.Infof("write entry: %s (%d bytes)", k, len(bytes))
 
-			err = util.WriteFileWithContent(context.Path, k, bytes)
+			err = util.WriteFileWithContent(path.Join(context.Path, k), bytes)
 			if err != nil {
 				return err
 			}
diff --git a/pkg/util/util.go b/pkg/util/util.go
index 903c9fb..2ceb69c 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -22,6 +22,7 @@ import (
 	"encoding/json"
 	"encoding/xml"
 	"fmt"
+	"go.uber.org/multierr"
 	"io"
 	"io/ioutil"
 	"os"
@@ -208,67 +209,52 @@ func EncodeXML(content interface{}) ([]byte, error) {
 	return w.Bytes(), nil
 }
 
-func CopyFile(src, dst string) (int64, error) {
+func CopyFile(src, dst string) (nBytes int64, err error) {
 	stat, err := os.Stat(src)
 	if err != nil {
-		return 0, err
+		return
 	}
 
 	if !stat.Mode().IsRegular() {
-		return 0, fmt.Errorf("%s is not a regular file", src)
+		err = fmt.Errorf("%s is not a regular file", src)
+		return
 	}
 
 	source, err := Open(src)
 	if err != nil {
-		return 0, err
+		return
 	}
-	defer source.Close()
+
+	defer func() {
+		err = Close(err, source)
+	}()
 
 	err = os.MkdirAll(path.Dir(dst), 0o700)
 	if err != nil {
-		return 0, err
+		return
 	}
 
 	destination, err := OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, stat.Mode())
 	if err != nil {
-		return 0, err
+		return
 	}
 
-	defer destination.Close()
-	nBytes, err := io.Copy(destination, source)
-
-	return nBytes, err
-}
+	defer func() {
+		err = Close(err, destination)
+	}()
 
-func WriteFileWithContent(buildDir string, relativePath string, content []byte) error {
-	filePath := path.Join(buildDir, relativePath)
-	fileDir := path.Dir(filePath)
-	// Create dir if not present
-	err := os.MkdirAll(fileDir, 0o700)
-	if err != nil {
-		return errors.Wrap(err, "could not create dir for file "+relativePath)
-	}
-	// Create file
-	file, err := os.Create(filePath)
-	if err != nil {
-		return errors.Wrap(err, "could not create file "+relativePath)
-	}
-	defer file.Close()
+	nBytes, err = io.Copy(destination, source)
 
-	_, err = file.Write(content)
-	if err != nil {
-		return errors.Wrap(err, "could not write to file "+relativePath)
-	}
-	return nil
+	return
 }
 
-func WriteFileWithBytesMarshallerContent(buildDir string, relativePath string, content BytesMarshaller) error {
+func WriteFileWithBytesMarshallerContent(basePath string, filePath string, content BytesMarshaller) error {
 	data, err := content.MarshalBytes()
 	if err != nil {
 		return err
 	}
 
-	return WriteFileWithContent(buildDir, relativePath, data)
+	return WriteFileWithContent(path.Join(basePath, filePath), data)
 }
 
 func FindAllDistinctStringSubmatch(data string, regexps ...*regexp.Regexp) []string {
@@ -320,18 +306,22 @@ func DirectoryExists(directory string) (bool, error) {
 	return info.IsDir(), nil
 }
 
-func DirectoryEmpty(directory string) (bool, error) {
+func DirectoryEmpty(directory string) (ok bool, err error) {
 	f, err := Open(directory)
 	if err != nil {
-		return false, err
+		return
 	}
-	defer f.Close()
+
+	defer func() {
+		err = Close(err, f)
+	}()
 
 	_, err = f.Readdirnames(1)
 	if errors.Is(err, io.EOF) {
-		return true, nil
+		ok = true
 	}
-	return false, err
+
+	return
 }
 
 func CreateDirectory(directory string) error {
@@ -778,3 +768,65 @@ func OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) {
 func ReadFile(filename string) ([]byte, error) {
 	return os.ReadFile(filepath.Clean(filename))
 }
+
+func Close(err error, closer io.Closer) error {
+	return multierr.Append(err, closer.Close())
+}
+
+// WithFile a safe wrapper to process a file.
+func WithFile(name string, flag int, perm os.FileMode, consumer func(file *os.File) error) error {
+	// #nosec G304
+	file, err := os.OpenFile(filepath.Clean(name), flag, perm)
+	if err == nil {
+		err = consumer(file)
+	}
+
+	return Close(err, file)
+}
+
+// WithFileReader a safe wrapper to process a file.
+func WithFileReader(name string, consumer func(reader io.Reader) error) error {
+	// #nosec G304
+	file, err := os.Open(filepath.Clean(name))
+	if err == nil {
+		err = consumer(file)
+	}
+
+	return Close(err, file)
+}
+
+// WithFileContent a safe wrapper to process a file content.
+func WithFileContent(name string, consumer func(file *os.File, data []byte) error) error {
+	return WithFile(name, os.O_RDWR|os.O_CREATE, 0o644, func(file *os.File) error {
+		content, err := ReadFile(name)
+		if err != nil {
+			return err
+		}
+
+		return consumer(file, content)
+	})
+}
+
+// WriteFileWithContent a safe wrapper to write content to a file.
+func WriteFileWithContent(filePath string, content []byte) error {
+	fileDir := path.Dir(filePath)
+
+	// Create dir if not present
+	err := os.MkdirAll(fileDir, 0o700)
+	if err != nil {
+		return errors.Wrap(err, "could not create dir for file "+filePath)
+	}
+
+	// Create file
+	file, err := os.Create(filePath)
+	if err != nil {
+		return errors.Wrap(err, "could not create file "+filePath)
+	}
+
+	_, err = file.Write(content)
+	if err != nil {
+		err = errors.Wrap(err, "could not write to file "+filePath)
+	}
+
+	return Close(err, file)
+}