Skip to content

refactor: fix gosec lint issues#1586

Merged
alexandear merged 2 commits intomgechev:masterfrom
alexandear:refactor/gosec-linter
Nov 22, 2025
Merged

refactor: fix gosec lint issues#1586
alexandear merged 2 commits intomgechev:masterfrom
alexandear:refactor/gosec-linter

Conversation

@alexandear
Copy link
Copy Markdown
Collaborator

@alexandear alexandear commented Nov 20, 2025 •
edited
Loading

This PR enables the gosec linter and fixes lint issues.

The issue about adding an error check to file.Pkg.TypeCheck() is disabled for now. Because it's not easy to fix and should be done carefully. See #1277 and #1310.

Full list of gosec issues 

❯ golangci-lint run
cli/main_test.go:28:2: G104: Errors unhandled (gosec)
        AppFs.MkdirAll(xdgDirPath, 0o755)
        ^
cli/main_test.go:29:2: G104: Errors unhandled (gosec)
        AppFs.MkdirAll(homeDirPath, 0o755)
        ^
cli/main_test.go:31:2: G104: Errors unhandled (gosec)
        afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0o644)
        ^
cli/main_test.go:34:2: G104: Errors unhandled (gosec)
        afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0o644)
        ^
cli/main_test.go:48:2: G104: Errors unhandled (gosec)
        AppFs.MkdirAll(homeDirPath, 0o755)
        ^
cli/main_test.go:50:2: G104: Errors unhandled (gosec)
        afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0o644)
        ^
cli/main_test.go:74:2: G104: Errors unhandled (gosec)
        AppFs.MkdirAll(xdgDirPath, 0o755)
        ^
cli/main_test.go:76:2: G104: Errors unhandled (gosec)
        afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0o644)
        ^
config/config.go:191:15: G304: Potential file inclusion via variable (gosec)
        file, err := os.ReadFile(path)
                     ^
formatter/formatter_test.go:133:23: G304: Potential file inclusion via variable (gosec)
                        fakeStdout, err := os.Create(dir + "/fakeStdout")
                                           ^
formatter/friendly.go:133:3: G104: Errors unhandled (gosec)
                tw.Write([]byte{'\t'})
                ^
formatter/friendly.go:135:4: G104: Errors unhandled (gosec)
                        tw.Write(append([]byte(col), '\t'))
                        ^
formatter/friendly.go:137:3: G104: Errors unhandled (gosec)
                tw.Write([]byte{'\n'})
                ^
formatter/friendly.go:139:2: G104: Errors unhandled (gosec)
        tw.Flush()
        ^
formatter/sarif.go:35:2: G104: Errors unhandled (gosec)
        sarifLog.PrettyWrite(buf)
        ^
internal/astutils/ast_utils.go:6:2: G501: Blocklisted import crypto/md5: weak cryptographic primitive (gosec)
        "crypto/md5"
        ^
internal/astutils/ast_utils.go:204:2: G104: Errors unhandled (gosec)
        gofmtConfig.Fprint(&buf, fs, x)
        ^
internal/astutils/ast_utils.go:211:14: G401: Use of weak cryptographic primitive (gosec)
                binHash := md5.Sum([]byte(in))
                           ^
lint/linter.go:165:14: G304: Potential file inclusion via variable (gosec)
        mod, err := os.ReadFile(modFileName)
                    ^
lint/linter_test.go:12:10: G301: Expect directory permissions to be 0750 or less (gosec)
                err := os.MkdirAll(nestedDir, 0o755)
                       ^
lint/linter_test.go:18:9: G306: Expect WriteFile permissions to be 0600 or less (gosec)
                err = os.WriteFile(modFilePath, []byte("module example.com/test"), 0o644)
                      ^
logging/logger_test.go:26:22: G104: Errors unhandled (gosec)
                t.Cleanup(func() { os.Remove("revive.log") })
                                   ^
logging/logger_test.go:42:22: G104: Errors unhandled (gosec)
                t.Cleanup(func() { os.Remove("revive.log") })
                                   ^
revivelib/core.go:100:20: G304: Potential file inclusion via variable (gosec)
                contents, err := os.ReadFile(file)
                                 ^
test/file_filter_test.go:32:3: G104: Errors unhandled (gosec)
                cfg.Initialize()
                ^
test/file_filter_test.go:42:3: G104: Errors unhandled (gosec)
                cfg.Initialize()
                ^
test/golint_test.go:61:16: G304: Potential file inclusion via variable (gosec)
                        src, err := os.ReadFile(filePath)
                                    ^
test/utils_test.go:42:14: G304: Potential file inclusion via variable (gosec)
        src, err := os.ReadFile(fullFilePath)
                    ^
test/utils_test.go:57:3: G104: Errors unhandled (gosec)
                assertSuccess(t, fullFilePath, []lint.Rule{rule}, c)
                ^
test/utils_test.go:60:2: G104: Errors unhandled (gosec)
        assertFailures(t, fullFilePath, []lint.Rule{rule}, c, ins)
        ^
test/utils_test.go:428:12: G301: Expect directory permissions to be 0750 or less (gosec)
        if err := os.MkdirAll(gitDir, 0o755); err != nil {
                  ^
31 issues:
* gosec: 31

@alexandear alexandear requested a review from Copilot November 20, 2025 16:29
Copilot AI reviewed Nov 20, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables the gosec security linter and addresses various security-related code issues it identified, focusing on proper error handling and secure file permissions.

Key changes:

  • Added error checking for previously ignored function returns across test files and formatters
  • Updated file and directory permissions to more restrictive values (0o750 for directories, 0o600 for files)
  • Added appropriate //nolint:gosec suppressions for intentional use of MD5 (for non-cryptographic hashing)
  • Configured .golangci.yml to exclude gosec checks from rule files where TypeCheck() error handling is deferred

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/utils_test.go Changed assertSuccess/assertFailures to use t.Errorf instead of returning errors; updated .git directory permissions to 0o750
test/golint_test.go Removed error handling wrapper since assertFailures no longer returns an error
test/file_filter_test.go Added error checking for cfg.Initialize() calls
rule/time_equal.go Removed unused TypeCheck() error check (gosec excluded via config)
rule/epoch_naming.go Added proper TypeCheck() error handling with internal failure reporting
logging/logger_test.go Added error checking in cleanup functions for os.Remove()
lint/linter_test.go Updated directory permissions to 0o750 and file permissions to 0o600
internal/astutils/ast_utils.go Added gosec suppressions for MD5 usage and ignored Fprint error (writes to buffer)
formatter/sarif.go Added error handling for PrettyWrite()
formatter/friendly.go Explicitly ignored errors from tabwriter operations (writes to buffer)
cli/main_test.go Added error checking for MkdirAll and WriteFile operations
.golangci.yml Enabled gosec linter with exclusions for rule files and G304 (file inclusion)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rule/time_equal.go Outdated
Comment thread logging/logger_test.go
Comment thread logging/logger_test.go Outdated
@alexandear alexandear force-pushed the refactor/gosec-linter branch 4 times, most recently from f3e205a to bbd6f64 Compare November 20, 2025 16:57
ccoVeille
ccoVeille reviewed Nov 20, 2025
View reviewed changes
Comment thread .golangci.yml
Comment thread .golangci.yml Outdated
Comment thread formatter/friendly.go
Comment thread test/utils_test.go
Comment thread internal/astutils/ast_utils.go
@alexandear alexandear force-pushed the refactor/gosec-linter branch from bbd6f64 to 0158306 Compare November 21, 2025 14:19
@alexandear
Copy link
Copy Markdown
Collaborator Author

alexandear commented Nov 22, 2025

@ccoVeille thank you for review.

@alexandear alexandear merged commit 10e8619 into mgechev:master Nov 22, 2025
10 checks passed
@alexandear alexandear deleted the refactor/gosec-linter branch November 22, 2025 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@ccoVeille ccoVeille ccoVeille approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alexandear @ccoVeille