refactor: fix gosec lint issues (#1586)

.golangci.yml

@@ -11,6 +11,7 @@ linters:
     - gocritic
     - godoclint
     - godot
+    - gosec
     - govet
     - ineffassign
     - misspell
@@ -50,6 +51,36 @@ linters:
       - linters:
           - testpackage
         text: 'package should be `formatter_test` instead of `formatter`'
+      - linters:
+          - gosec
+        path: rule/epoch_naming.go
+      - linters:
+          - gosec
+        path: rule/context_keys_type.go
+      - linters:
+          - gosec
+        path: rule/errorf.go
+      - linters:
+          - gosec
+        path: rule/modifies_value_receiver.go
+      - linters:
+          - gosec
+        path: rule/range_val_address.go
+      - linters:
+          - gosec
+        path: rule/string_of_int.go
+      - linters:
+          - gosec
+        path: rule/time_naming.go
+      - linters:
+          - gosec
+        path: rule/unexported_return.go
+      - linters:
+          - gosec
+        path: rule/unhandled_error.go
+      - linters:
+          - gosec
+        path: rule/var_declarations.go
 
   settings:
     gocritic:

cli/main_test.go

@@ -25,13 +25,21 @@ func TestXDGConfigDirIsPreferredFirst(t *testing.T) {
 
 	xdgDirPath := filepath.FromSlash("/tmp-iofs/xdg/config")
 	homeDirPath := filepath.FromSlash("/tmp-iofs/home/tester")
-	AppFs.MkdirAll(xdgDirPath, 0o755)
+	if err := AppFs.MkdirAll(xdgDirPath, 0o755); err != nil {
-	AppFs.MkdirAll(homeDirPath, 0o755)
+		t.Fatal(err)
+	}
+	if err := AppFs.MkdirAll(homeDirPath, 0o755); err != nil {
+		t.Fatal(err)
+	}
 
-	afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0o644)
+	if err := afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
 	t.Setenv("XDG_CONFIG_HOME", xdgDirPath)
 
-	afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0o644)
+	if err := afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
 	setHome(t, homeDirPath)
 
 	got := buildDefaultConfigPath()
@@ -45,9 +53,13 @@ func TestXDGConfigDirIsPreferredFirst(t *testing.T) {
 func TestHomeConfigDir(t *testing.T) {
 	t.Cleanup(func() { AppFs = afero.NewMemMapFs() })
 	homeDirPath := filepath.FromSlash("/tmp-iofs/home/tester")
-	AppFs.MkdirAll(homeDirPath, 0o755)
+	if err := AppFs.MkdirAll(homeDirPath, 0o755); err != nil {
+		t.Fatal(err)
+	}
 
-	afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0o644)
+	if err := afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
 	setHome(t, homeDirPath)
 
 	got := buildDefaultConfigPath()
@@ -71,9 +83,13 @@ func setHome(t *testing.T, dir string) {
 func TestXDGConfigDir(t *testing.T) {
 	t.Cleanup(func() { AppFs = afero.NewMemMapFs() })
 	xdgDirPath := filepath.FromSlash("/tmp-iofs/xdg/config")
-	AppFs.MkdirAll(xdgDirPath, 0o755)
+	if err := AppFs.MkdirAll(xdgDirPath, 0o755); err != nil {
+		t.Fatal(err)
+	}
 
-	afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0o644)
+	if err := afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
 	t.Setenv("XDG_CONFIG_HOME", xdgDirPath)
 
 	got := buildDefaultConfigPath()

config/config.go

@@ -187,12 +187,8 @@ func actualRuleName(name string) string {
 	}
 }
 
-func parseConfig(path string, config *lint.Config) error {
+func parseConfig(data []byte, config *lint.Config) error {
-	file, err := os.ReadFile(path)
+	err := toml.Unmarshal(data, config)
-	if err != nil {
-		return errors.New("cannot read the config file")
-	}
-	err = toml.Unmarshal(file, config)
 	if err != nil {
 		return fmt.Errorf("cannot parse the config file: %w", err)
 	}
@@ -252,7 +248,11 @@ func GetConfig(configPath string) (*lint.Config, error) {
 	switch {
 	case configPath != "":
 		config.Confidence = defaultConfidence
-		err := parseConfig(configPath, config)
+		data, err := os.ReadFile(configPath) //nolint:gosec // ignore G304: potential file inclusion via variable
+		if err != nil {
+			return nil, errors.New("cannot read the config file")
+		}
+		err = parseConfig(data, config)
 		if err != nil {
 			return nil, err
 		}

formatter/formatter_test.go

@@ -3,6 +3,7 @@ package formatter_test
 import (
 	"go/token"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -128,9 +129,8 @@ test.go
 		},
 	} {
 		t.Run(td.formatter.Name(), func(t *testing.T) {
-			dir := t.TempDir()
 			realStdout := os.Stdout
-			fakeStdout, err := os.Create(dir + "/fakeStdout")
+			fakeStdout, err := os.Create(filepath.Join(t.TempDir(), "fakeStdout"))
 			if err != nil {
 				t.Fatal(err)
 			}

formatter/friendly.go

@@ -130,12 +130,12 @@ func table(rows [][]string) string {
 	var buf bytes.Buffer
 	tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0)
 	for _, row := range rows {
-		tw.Write([]byte{'\t'})
+		_, _ = tw.Write([]byte{'\t'})
 		for _, col := range row {
-			tw.Write(append([]byte(col), '\t'))
+			_, _ = tw.Write(append([]byte(col), '\t'))
 		}
-		tw.Write([]byte{'\n'})
+		_, _ = tw.Write([]byte{'\n'})
 	}
-	tw.Flush()
+	_ = tw.Flush()
 	return buf.String()
 }

formatter/sarif.go

@@ -32,7 +32,10 @@ func (*Sarif) Format(failures <-chan lint.Failure, cfg lint.Config) (string, err
 	}
 
 	buf := new(bytes.Buffer)
-	sarifLog.PrettyWrite(buf)
+	err := sarifLog.PrettyWrite(buf)
+	if err != nil {
+		return "", err
+	}
 
 	return buf.String(), nil
 }

internal/astutils/ast_utils.go

@@ -3,7 +3,7 @@ package astutils
 
 import (
 	"bytes"
-	"crypto/md5"
+	"crypto/md5" //nolint:gosec // G501: Blocklisted import crypto/md5: weak cryptographic primitive
 	"encoding/hex"
 	"fmt"
 	"go/ast"
@@ -201,14 +201,14 @@ var gofmtConfig = &printer.Config{Tabwidth: 8}
 func GoFmt(x any) string {
 	buf := bytes.Buffer{}
 	fs := token.NewFileSet()
-	gofmtConfig.Fprint(&buf, fs, x)
+	_ = gofmtConfig.Fprint(&buf, fs, x)
 	return buf.String()
 }
 
 // NodeHash yields the MD5 hash of the given AST node.
 func NodeHash(node ast.Node) string {
 	hasher := func(in string) string {
-		binHash := md5.Sum([]byte(in))
+		binHash := md5.Sum([]byte(in)) //nolint:gosec // G401: Weak cryptographic primitive
 		return hex.EncodeToString(binHash[:])
 	}
 	str := GoFmt(node)

lint/linter.go

@@ -162,7 +162,7 @@ func detectGoMod(dir string) (rootDir string, ver *goversion.Version, err error)
 		return "", nil, fmt.Errorf("%q doesn't seem to be part of a Go module", dir)
 	}
 
-	mod, err := os.ReadFile(modFileName)
+	mod, err := os.ReadFile(modFileName) //nolint:gosec // ignore G304: potential file inclusion via variable
 	if err != nil {
 		return "", nil, fmt.Errorf("failed to read %q, got %w", modFileName, err)
 	}

lint/linter_test.go

@@ -9,13 +9,13 @@ import (
 func TestRetrieveModFile(t *testing.T) {
 	t.Run("go.mod file exists", func(t *testing.T) {
 		nestedDir := filepath.Join(t.TempDir(), "nested", "dir", "structure")
-		err := os.MkdirAll(nestedDir, 0o755)
+		err := os.MkdirAll(nestedDir, 0o750)
 		if err != nil {
 			t.Fatal(err)
 		}
 
 		modFilePath := filepath.Join(nestedDir, "go.mod")
-		err = os.WriteFile(modFilePath, []byte("module example.com/test"), 0o644)
+		err = os.WriteFile(modFilePath, []byte("module example.com/test"), 0o600)
 		if err != nil {
 			t.Fatal(err)
 		}

logging/logger.go

@@ -9,7 +9,10 @@ import (
 
 const logFile = "revive.log"
 
-var logger *slog.Logger
+var (
+	logger     *slog.Logger
+	loggerFile *os.File
+)
 
 // GetLogger retrieves an instance of an application logger which outputs
 // to a file if the debug flag is enabled.
@@ -24,14 +27,23 @@ func GetLogger() (*slog.Logger, error) {
 		return slog.New(slog.DiscardHandler), nil
 	}
 
-	fileWriter, err := os.Create(logFile)
+	var err error
+	loggerFile, err = os.Create(logFile)
 	if err != nil {
 		return nil, err
 	}
 
-	logger = slog.New(slog.NewTextHandler(io.MultiWriter(os.Stderr, fileWriter), nil))
+	logger = slog.New(slog.NewTextHandler(io.MultiWriter(os.Stderr, loggerFile), nil))
 
 	logger.Info("Logger initialized", "logFile", logFile)
 
 	return logger, nil
 }
+
+// Close closes the logger file if it was opened.
+func Close() error {
+	if loggerFile == nil {
+		return nil
+	}
+	return loggerFile.Close()
+}

logging/logger_test.go

@@ -23,7 +23,14 @@ func TestGetLogger(t *testing.T) {
 
 	t.Run("debug", func(t *testing.T) {
 		t.Setenv("DEBUG", "1")
-		t.Cleanup(func() { os.Remove("revive.log") })
+		t.Cleanup(func() {
+			if err := logging.Close(); err != nil {
+				t.Error(err)
+			}
+			if err := os.Remove("revive.log"); err != nil {
+				t.Error(err)
+			}
+		})
 
 		logger, err := logging.GetLogger()
 		if err != nil {
@@ -35,24 +42,14 @@ func TestGetLogger(t *testing.T) {
 		if _, err := os.Stat("revive.log"); os.IsNotExist(err) {
 			t.Error("expected revive.log file to be created")
 		}
-	})
-
-	t.Run("reuse logger", func(t *testing.T) {
-		t.Setenv("DEBUG", "1")
-		t.Cleanup(func() { os.Remove("revive.log") })
-
-		logger1, err := logging.GetLogger()
-		if err != nil {
-			t.Fatalf("expected no error, got %v", err)
-		}
 
 		logger2, err := logging.GetLogger()
 		if err != nil {
 			t.Fatalf("expected no error, got %v", err)
 		}
 
-		if logger1 != logger2 {
+		if logger != logger2 {
-			t.Errorf("expected the same logger instance to be returned: logger1=%+v, logger2=%+v", logger1, logger2)
+			t.Errorf("expected the same logger instance to be returned: logger1=%+v, logger2=%+v", logger, logger2)
 		}
 	})
 }

revivelib/core.go

@@ -97,7 +97,7 @@ func (r *Revive) Lint(patterns ...*LintPattern) (<-chan lint.Failure, error) {
 	}
 
 	revive := lint.New(func(file string) ([]byte, error) {
-		contents, err := os.ReadFile(file)
+		contents, err := os.ReadFile(file) //nolint:gosec // ignore G304: potential file inclusion via variable
 		if err != nil {
 			return nil, fmt.Errorf("reading file %v: %w", file, err)
 		}

test/file_filter_test.go

@@ -29,7 +29,9 @@ func TestFileExcludeFilterAtRuleLevel(t *testing.T) {
 	t.Run("is called if exclude not match", func(t *testing.T) {
 		rule := &TestFileFilterRule{}
 		cfg := &lint.RuleConfig{Exclude: []string{"no_matched.go"}}
-		cfg.Initialize()
+		if err := cfg.Initialize(); err != nil {
+			t.Fatal(err)
+		}
 		testRule(t, "file_to_exclude", rule, cfg)
 		if !rule.WasApplied {
 			t.Fatal("should call rule if no excludes")
@@ -39,7 +41,9 @@ func TestFileExcludeFilterAtRuleLevel(t *testing.T) {
 	t.Run("not called if exclude not match", func(t *testing.T) {
 		rule := &TestFileFilterRule{}
 		cfg := &lint.RuleConfig{Exclude: []string{"../testdata/file_to_exclude.go"}}
-		cfg.Initialize()
+		if err := cfg.Initialize(); err != nil {
+			t.Fatal(err)
+		}
 		testRule(t, "file_to_exclude", rule, cfg)
 		if rule.WasApplied {
 			t.Fatal("should not call rule if excluded")

test/golint_test.go

@@ -58,16 +58,14 @@ func TestAll(t *testing.T) {
 		}
 		t.Run(fi.Name(), func(t *testing.T) {
 			filePath := filepath.Join(baseDir, fi.Name())
-			src, err := os.ReadFile(filePath)
+			src, err := os.ReadFile(filePath) //nolint:gosec // ignore G304: potential file inclusion via variable
 			if err != nil {
 				t.Fatalf("Failed reading %s: %v", fi.Name(), err)
 			}
 
 			ins := parseInstructions(t, filePath, src)
 
-			if err := assertFailures(t, filePath, rules, map[string]lint.RuleConfig{}, ins); err != nil {
+			assertFailures(t, filePath, rules, map[string]lint.RuleConfig{}, ins)
-				t.Errorf("Linting %s: %v", fi.Name(), err)
-			}
 		})
 	}
 }

test/utils_test.go

@@ -39,7 +39,7 @@ func testRule(t *testing.T, filename string, rule lint.Rule, config ...*lint.Rul
 	baseDir := filepath.Join("..", "testdata", filepath.Dir(filename))
 	filename = filepath.Base(filename) + ".go"
 	fullFilePath := filepath.Join(baseDir, filename)
-	src, err := os.ReadFile(fullFilePath)
+	src, err := os.ReadFile(fullFilePath) //nolint:gosec // ignore G304: potential file inclusion via variable
 	if err != nil {
 		t.Fatalf("Bad filename path in test for %s: %v", rule.Name(), err)
 	}
@@ -60,7 +60,7 @@ func testRule(t *testing.T, filename string, rule lint.Rule, config ...*lint.Rul
 	assertFailures(t, fullFilePath, []lint.Rule{rule}, c, ins)
 }
 
-func assertSuccess(t *testing.T, filePath string, rules []lint.Rule, config map[string]lint.RuleConfig) error {
+func assertSuccess(t *testing.T, filePath string, rules []lint.Rule, config map[string]lint.RuleConfig) {
 	t.Helper()
 
 	l := lint.New(os.ReadFile, 0)
@@ -69,7 +69,8 @@ func assertSuccess(t *testing.T, filePath string, rules []lint.Rule, config map[
 		Rules: config,
 	})
 	if err != nil {
-		return err
+		t.Errorf("Linting %s: %v", filePath, err)
+		return
 	}
 
 	failures := ""
@@ -79,10 +80,9 @@ func assertSuccess(t *testing.T, filePath string, rules []lint.Rule, config map[
 	if failures != "" {
 		t.Errorf("Expected the rule to pass but got the following failures: %s", failures)
 	}
-	return nil
 }
 
-func assertFailures(t *testing.T, filePath string, rules []lint.Rule, config map[string]lint.RuleConfig, ins []instruction) error {
+func assertFailures(t *testing.T, filePath string, rules []lint.Rule, config map[string]lint.RuleConfig, ins []instruction) {
 	t.Helper()
 
 	l := lint.New(os.ReadFile, 0)
@@ -91,7 +91,8 @@ func assertFailures(t *testing.T, filePath string, rules []lint.Rule, config map
 		Rules: config,
 	})
 	if err != nil {
-		return err
+		t.Errorf("Linting %s: %v", filePath, err)
+		return
 	}
 
 	failures := []lint.Failure{}
@@ -194,8 +195,6 @@ func assertFailures(t *testing.T, filePath string, rules []lint.Rule, config map
 	if errorMessage != "" {
 		t.Error(errorMessage)
 	}
-
-	return nil
 }
 
 type instruction struct {
@@ -425,7 +424,7 @@ func mkdirTempDotGit(t *testing.T, root string) {
 	}
 
 	gitDir := filepath.Join(dir, ".git")
-	if err := os.MkdirAll(gitDir, 0o755); err != nil {
+	if err := os.MkdirAll(gitDir, 0o750); err != nil {
 		t.Fatalf("Failed to create .git directory: %v", err)
 	}