Migrate to golangci-lint v2 and fix linting issues (#3)

* Add FTP container implementation for testing FTP operations

- Add FTPTestContainer for testing FTP file transfers
- Implement methods for uploading, downloading, and listing files
- Add comprehensive path handling for directories
- Add secure file operations with path validation
- Update README with documentation and examples

* Update linting configuration and fix issues

- Add golangci-lint v2 configuration file
- Update CI workflow to use golangci-lint v2
- Fix error handling in stdout/stderr captures
- Improve file security with path validation
- Add safe handling for file operations
- Fix unhandled errors in environment variable operations
- Update test assertions for better reliability

* Update containers/ftp.go

fix typo

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: umputun

.github/workflows/ci.yml

@@ -30,9 +30,9 @@ jobs:
           TZ: "America/Chicago"
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v7
         with:
-          version: latest
+          version: v2.0.2
 
       - name: install goveralls
         run: go install github.com/mattn/goveralls@latest

.gitignore

@@ -13,9 +13,9 @@
 
 # Output of the go coverage tool, specifically when used with LiteIDE
 *.out
-
-# Dependency directories (remove the comment below to include it)
-# vendor/
+*.cov
 
 # Go workspace file
 go.work
+
+**/CLAUDE.local.md

.golangci.yml

@@ -0,0 +1,61 @@
+version: "2"
+run:
+  concurrency: 4
+linters:
+  default: none
+  enable:
+    - copyloopvar
+    - gochecknoinits
+    - gocritic
+    - gosec
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - prealloc
+    - revive
+    - staticcheck
+    - unconvert
+    - unparam
+    - unused
+    - testifylint
+    - nestif
+  settings:
+    goconst:
+      min-len: 2
+      min-occurrences: 2
+    gocritic:
+      disabled-checks:
+        - wrapperFunc
+      enabled-tags:
+        - performance
+        - style
+        - experimental
+    gocyclo:
+      min-complexity: 15
+    lll:
+      line-length: 140
+    misspell:
+      locale: US
+  exclusions:
+    generated: lax
+    rules:
+      - linters:
+          - gosec
+        text: 'G114: Use of net/http serve function that has no support for setting timeouts'
+      - linters:
+          - revive
+          - unparam
+        path: _test\.go$
+        text: unused-parameter
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

capture.go

@@ -1,3 +1,4 @@
+// Package testutils provides utilities for testing Go applications
 package testutils
 
 import (

capture_test.go

@@ -134,8 +134,14 @@ func TestCaptureStdoutAndStderr(t *testing.T) {
 			wantOut: string([]byte{0, 1, 2, 3}),
 			wantErr: string([]byte{4, 5, 6, 7}),
 			f: func() {
-				os.Stdout.Write([]byte{0, 1, 2, 3})
-				os.Stderr.Write([]byte{4, 5, 6, 7})
+				_, err := os.Stdout.Write([]byte{0, 1, 2, 3})
+				if err != nil {
+					t.Logf("failed to write to stdout: %v", err)
+				}
+				_, err = os.Stderr.Write([]byte{4, 5, 6, 7})
+				if err != nil {
+					t.Logf("failed to write to stderr: %v", err)
+				}
 			},
 		},
 		{

containers/mongo_test.go

@@ -60,13 +60,19 @@ func TestMongoTestContainer(t *testing.T) {
 		// save current MONGO_TEST value
 		origEnv := os.Getenv("MONGO_TEST")
 		testValue := "mongodb://original-value:27017"
-		os.Setenv("MONGO_TEST", testValue)
+		if err := os.Setenv("MONGO_TEST", testValue); err != nil {
+			t.Fatalf("Failed to set MONGO_TEST environment variable: %v", err)
+		}
 		defer func() {
 			// restore original value
 			if origEnv == "" {
-				os.Unsetenv("MONGO_TEST")
+				if err := os.Unsetenv("MONGO_TEST"); err != nil {
+					t.Logf("Warning: failed to unset MONGO_TEST environment variable: %v", err)
+				}
 			} else {
-				os.Setenv("MONGO_TEST", origEnv)
+				if err := os.Setenv("MONGO_TEST", origEnv); err != nil {
+					t.Logf("Warning: failed to restore MONGO_TEST environment variable: %v", err)
+				}
 			}
 		}()
 
@@ -82,11 +88,15 @@ func TestMongoTestContainer(t *testing.T) {
 	t.Run("close with no original environment variable", func(t *testing.T) {
 		// save current MONGO_TEST value
 		origEnv := os.Getenv("MONGO_TEST")
-		os.Unsetenv("MONGO_TEST")
+		if err := os.Unsetenv("MONGO_TEST"); err != nil {
+			t.Fatalf("Failed to unset MONGO_TEST environment variable: %v", err)
+		}
 		defer func() {
 			// restore original value
 			if origEnv != "" {
-				os.Setenv("MONGO_TEST", origEnv)
+				if err := os.Setenv("MONGO_TEST", origEnv); err != nil {
+					t.Logf("Warning: failed to restore MONGO_TEST environment variable: %v", err)
+				}
 			}
 		}()
 

containers/ssh_test.go

@@ -3,7 +3,6 @@ package containers
 import (
 	"context"
 	"os/exec"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -36,11 +35,13 @@ func TestSSHTestContainer(t *testing.T) {
 		defer func() { require.NoError(t, ssh.Close(ctx)) }()
 
 		// use ssh-keyscan to verify the host is accessible
-		cmd := exec.Command("ssh-keyscan", "-p", ssh.Port.Port(), ssh.Host)
+		// values come from the test container itself, so this is not vulnerable to command injection
+		// ssh.Port and ssh.Host are generated by the test container and are not user input
+		cmd := exec.Command("ssh-keyscan", "-p", ssh.Port.Port(), ssh.Host) // #nosec G204 -- these params are from our test container
 		out, err := cmd.CombinedOutput()
 		require.NoError(t, err)
 		t.Logf("ssh-keyscan output: %s", out)
-		assert.True(t, strings.Contains(string(out), "ssh-"), "should return ssh key")
+		assert.Contains(t, string(out), "ssh-", "should return ssh key")
 	})
 
 	t.Run("multiple containers", func(t *testing.T) {

file_utils_test.go

@@ -19,7 +19,10 @@ func TestWriteTestFile(t *testing.T) {
 		require.False(t, os.IsNotExist(err), "WriteTestFile did not create file at %s", filePath)
 
 		// check content
-		data, err := os.ReadFile(filePath)
+		// although ReadFile takes a path from a test-generated file, this is safe
+		// because the file is created in a temporary directory controlled by the test
+		// this is safe because the file path was created by WriteTestFile in a controlled manner
+		data, err := os.ReadFile(filePath) // #nosec G304 -- safe file access in test
 		require.NoError(t, err, "Failed to read test file")
 		require.Equal(t, content, string(data), "File content doesn't match expected")
 
@@ -43,7 +46,10 @@ func TestWriteTestFile(t *testing.T) {
 		content := "line 1\nline 2\nline 3"
 		filePath := WriteTestFile(t, content)
 
-		data, err := os.ReadFile(filePath)
+		// although ReadFile takes a path from a test-generated file, this is safe
+		// because the file is created in a temporary directory controlled by the test
+		// this is safe because the file path was created by WriteTestFile in a controlled manner
+		data, err := os.ReadFile(filePath) // #nosec G304 -- safe file access in test
 		require.NoError(t, err)
 		require.Equal(t, content, string(data))
 	})

http_utils_test.go

@@ -17,8 +17,10 @@ func TestMockHTTPServer(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 		// in real request handling, the error is typically ignored as it's a disconnect which HTTP server handles
 		// ResponseWriter interface doesn't have a way to check for errors in tests
-		// nolint:errcheck // http.ResponseWriter errors are handled by the HTTP server
-		w.Write([]byte(`{"status":"ok"}`))
+		_, err := w.Write([]byte(`{"status":"ok"}`))
+		if err != nil {
+			t.Logf("failed to write response: %v", err)
+		}
 	})
 
 	// get the server URL and cleanup function
@@ -68,8 +70,10 @@ func TestHTTPRequestCaptor(t *testing.T) {
 	// create a test handler that will receive forwarded requests
 	testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
-		// nolint:errcheck // http.ResponseWriter errors are handled by the HTTP server
-		w.Write([]byte("response"))
+		_, err := w.Write([]byte("response"))
+		if err != nil {
+			t.Logf("failed to write response: %v", err)
+		}
 	})
 
 	// create the request captor
@@ -119,7 +123,7 @@ func TestHTTPRequestCaptor(t *testing.T) {
 
 	// test GetRequests
 	allRequests := captor.GetRequests()
-	require.Equal(t, 3, len(allRequests), "Wrong number of requests from GetRequests")
+	require.Len(t, allRequests, 3, "Wrong number of requests from GetRequests")
 
 	// test Reset
 	captor.Reset()