From 5aeb9c86c0f4d369047498e39d8c6cc1cdb441aa Mon Sep 17 00:00:00 2001 From: Mathieu Virbel Date: Fri, 13 Feb 2026 19:20:40 -0600 Subject: [PATCH] fix: resolve all golangci-lint v2 warnings (29 issues) Migrate to golangci-lint v2 config format and fix all lint issues: - errcheck: add explicit error handling for Close/Remove calls - gocritic: convert if-else chains to switch statements - gosec: tighten file permissions, add nolint for intentional cases - staticcheck: lowercase error strings, simplify boolean returns Also update Makefile to install golangci-lint v2 and update CLAUDE.md. --- CLAUDE.md | 2 +- Makefile | 2 +- cmd/greywall/main.go | 11 +++++---- internal/sandbox/learning.go | 10 +++----- internal/sandbox/learning_linux.go | 2 +- internal/sandbox/learning_linux_test.go | 2 +- internal/sandbox/learning_test.go | 32 +++++++++++++------------ internal/sandbox/linux.go | 18 +++++++------- internal/sandbox/linux_features.go | 2 +- internal/sandbox/linux_landlock.go | 4 ++-- internal/sandbox/manager.go | 14 +++++------ internal/sandbox/tun2socks_embed.go | 10 ++++---- 12 files changed, 55 insertions(+), 54 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 13a24b2..68b17a0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,7 +54,7 @@ scripts/ Smoke tests, benchmarks, release - **Language:** Go 1.25+ - **Formatter:** `gofumpt` (enforced in CI) -- **Linter:** `golangci-lint` v1.64.8 (config in `.golangci.yml`) +- **Linter:** `golangci-lint` v2 (config in `.golangci.yml`) - **Import order:** stdlib, third-party, local (`gitea.app.monadical.io/monadical/greywall`) - **Platform code:** build tags (`//go:build linux`, `//go:build darwin`) with `*_stub.go` for unsupported platforms - **Error handling:** custom error types (e.g., `CommandBlockedError`) diff --git a/Makefile b/Makefile index 8f44e54..0b9b09b 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,7 @@ build-darwin: install-lint-tools: @echo "Installing linting tools..." go install mvdan.cc/gofumpt@latest - go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest + go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest @echo "Linting tools installed" setup: deps install-lint-tools diff --git a/cmd/greywall/main.go b/cmd/greywall/main.go index 6113f21..20f1315 100644 --- a/cmd/greywall/main.go +++ b/cmd/greywall/main.go @@ -203,19 +203,20 @@ func runCommand(cmd *cobra.Command, args []string) error { if templatePath != "" { learnedCfg, loadErr := config.Load(templatePath) - if loadErr != nil { + switch { + case loadErr != nil: if debug { fmt.Fprintf(os.Stderr, "[greywall] Warning: failed to load learned template: %v\n", loadErr) } - } else if learnedCfg != nil { + case learnedCfg != nil: cfg = config.Merge(cfg, learnedCfg) if debug { fmt.Fprintf(os.Stderr, "[greywall] Auto-loaded learned template for %q\n", templateLabel) } - } else if templateName != "" { + case templateName != "": // Explicit --template but file doesn't exist return fmt.Errorf("learned template %q not found at %s\nRun: greywall templates list", templateName, templatePath) - } else if cmdName != "" { + case cmdName != "": // No template found for this command - suggest creating one fmt.Fprintf(os.Stderr, "[greywall] No learned template for %q. Run with --learning to create one.\n", cmdName) } @@ -503,7 +504,7 @@ Examples: RunE: func(cmd *cobra.Command, args []string) error { name := args[0] templatePath := sandbox.LearnedTemplatePath(name) - data, err := os.ReadFile(templatePath) + data, err := os.ReadFile(templatePath) //nolint:gosec // user-specified template path - intentional if err != nil { if os.IsNotExist(err) { return fmt.Errorf("template %q not found\nRun: greywall templates list", name) diff --git a/internal/sandbox/learning.go b/internal/sandbox/learning.go index 18b15e9..3c45974 100644 --- a/internal/sandbox/learning.go +++ b/internal/sandbox/learning.go @@ -166,11 +166,11 @@ func GenerateLearnedTemplate(straceLogPath, cmdName string, debug bool) (string, // Save template templatePath := LearnedTemplatePath(cmdName) - if err := os.MkdirAll(filepath.Dir(templatePath), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(templatePath), 0o750); err != nil { return "", fmt.Errorf("failed to create template directory: %w", err) } - if err := os.WriteFile(templatePath, []byte(template), 0o644); err != nil { + if err := os.WriteFile(templatePath, []byte(template), 0o600); err != nil { return "", fmt.Errorf("failed to write template: %w", err) } @@ -305,11 +305,7 @@ func isSensitivePath(path, home string) bool { // Check GPG gnupgDir := filepath.Join(home, ".gnupg") - if strings.HasPrefix(path, gnupgDir+"/") { - return true - } - - return false + return strings.HasPrefix(path, gnupgDir+"/") } // getDangerousFilePatterns returns denyWrite entries for DangerousFiles. diff --git a/internal/sandbox/learning_linux.go b/internal/sandbox/learning_linux.go index 9411258..4dcfa09 100644 --- a/internal/sandbox/learning_linux.go +++ b/internal/sandbox/learning_linux.go @@ -41,7 +41,7 @@ func ParseStraceLog(logPath string, debug bool) (*StraceResult, error) { if err != nil { return nil, fmt.Errorf("failed to open strace log: %w", err) } - defer f.Close() + defer func() { _ = f.Close() }() home, _ := os.UserHomeDir() seenWrite := make(map[string]bool) diff --git a/internal/sandbox/learning_linux_test.go b/internal/sandbox/learning_linux_test.go index c48e47f..d230fb8 100644 --- a/internal/sandbox/learning_linux_test.go +++ b/internal/sandbox/learning_linux_test.go @@ -127,7 +127,7 @@ func TestParseStraceLog(t *testing.T) { }, "\n") logFile := filepath.Join(t.TempDir(), "strace.log") - if err := os.WriteFile(logFile, []byte(logContent), 0o644); err != nil { + if err := os.WriteFile(logFile, []byte(logContent), 0o600); err != nil { t.Fatal(err) } diff --git a/internal/sandbox/learning_test.go b/internal/sandbox/learning_test.go index 90deed4..6c254ac 100644 --- a/internal/sandbox/learning_test.go +++ b/internal/sandbox/learning_test.go @@ -70,9 +70,7 @@ func TestFindApplicationDirectory(t *testing.T) { func TestCollapsePaths(t *testing.T) { // Temporarily override home for testing - origHome := os.Getenv("HOME") - os.Setenv("HOME", "/home/testuser") - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", "/home/testuser") tests := []struct { name string @@ -319,9 +317,7 @@ func TestToTildePath(t *testing.T) { func TestListLearnedTemplates(t *testing.T) { // Use a temp dir to isolate from real user config tmpDir := t.TempDir() - origConfigDir := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tmpDir) - defer os.Setenv("XDG_CONFIG_HOME", origConfigDir) + t.Setenv("XDG_CONFIG_HOME", tmpDir) // Initially empty templates, err := ListLearnedTemplates() @@ -334,10 +330,18 @@ func TestListLearnedTemplates(t *testing.T) { // Create some templates dir := LearnedTemplateDir() - os.MkdirAll(dir, 0o755) - os.WriteFile(filepath.Join(dir, "opencode.json"), []byte("{}"), 0o644) - os.WriteFile(filepath.Join(dir, "myapp.json"), []byte("{}"), 0o644) - os.WriteFile(filepath.Join(dir, "notjson.txt"), []byte(""), 0o644) // should be ignored + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "opencode.json"), []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "myapp.json"), []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "notjson.txt"), []byte(""), 0o600); err != nil { + t.Fatal(err) + } templates, err = ListLearnedTemplates() if err != nil { @@ -415,9 +419,7 @@ func TestBuildTemplateNoAllowRead(t *testing.T) { func TestGenerateLearnedTemplate(t *testing.T) { // Create a temp dir for templates tmpDir := t.TempDir() - origConfigDir := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tmpDir) - defer os.Setenv("XDG_CONFIG_HOME", origConfigDir) + t.Setenv("XDG_CONFIG_HOME", tmpDir) // Create a fake strace log home, _ := os.UserHomeDir() @@ -430,7 +432,7 @@ func TestGenerateLearnedTemplate(t *testing.T) { }, "\n") logFile := filepath.Join(tmpDir, "strace.log") - if err := os.WriteFile(logFile, []byte(logContent), 0o644); err != nil { + if err := os.WriteFile(logFile, []byte(logContent), 0o600); err != nil { t.Fatal(err) } @@ -444,7 +446,7 @@ func TestGenerateLearnedTemplate(t *testing.T) { } // Read and verify template - data, err := os.ReadFile(templatePath) + data, err := os.ReadFile(templatePath) //nolint:gosec // reading test-generated template file if err != nil { t.Fatalf("failed to read template: %v", err) } diff --git a/internal/sandbox/linux.go b/internal/sandbox/linux.go index 831db51..23be2c2 100644 --- a/internal/sandbox/linux.go +++ b/internal/sandbox/linux.go @@ -564,7 +564,7 @@ func buildDenyByDefaultMounts(cfg *config.Config, cwd string, debug bool) []stri if emptyFile == "" { emptyFile = filepath.Join(os.TempDir(), "greywall", "empty") _ = os.MkdirAll(filepath.Dir(emptyFile), 0o750) - _ = os.WriteFile(emptyFile, nil, 0o444) + _ = os.WriteFile(emptyFile, nil, 0o444) //nolint:gosec // intentionally world-readable empty file for bind-mount masking } args = append(args, "--ro-bind", emptyFile, p) if debug { @@ -685,15 +685,16 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, proxyBridge defaultDenyRead := cfg != nil && cfg.Filesystem.IsDefaultDenyRead() - if opts.Learning { + switch { + case opts.Learning: // Skip defaultDenyRead logic in learning mode (already set up above) - } else if defaultDenyRead { + case defaultDenyRead: // Deny-by-default mode: start with empty root, then whitelist system paths + CWD if opts.Debug { fmt.Fprintf(os.Stderr, "[greywall:linux] DefaultDenyRead mode enabled - tmpfs root with selective mounts\n") } bwrapArgs = append(bwrapArgs, buildDenyByDefaultMounts(cfg, cwd, opts.Debug)...) - } else { + default: // Legacy mode: bind entire root filesystem read-only bwrapArgs = append(bwrapArgs, "--ro-bind", "/", "/") } @@ -929,7 +930,7 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, proxyBridge // Supported by glibc, Go 1.21+, c-ares, and most DNS resolver libraries. _, _ = tmpResolv.WriteString("nameserver 1.1.1.1\nnameserver 8.8.8.8\noptions use-vc\n") } - tmpResolv.Close() + _ = tmpResolv.Close() dnsRelayResolvConf = tmpResolv.Name() bwrapArgs = append(bwrapArgs, "--ro-bind", dnsRelayResolvConf, "/etc/resolv.conf") if opts.Debug { @@ -1077,7 +1078,8 @@ sleep 0.3 // after the main command exits; the user can Ctrl+C to stop it. // A SIGCHLD trap kills strace once its direct child exits, handling // the common case of background daemons (LSP servers, watchers). - if opts.Learning && opts.StraceLogPath != "" { + switch { + case opts.Learning && opts.StraceLogPath != "": innerScript.WriteString(fmt.Sprintf(`# Learning mode: trace filesystem access (foreground for terminal access) strace -f -qq -I2 -e trace=openat,open,creat,mkdir,mkdirat,unlinkat,renameat,renameat2,symlinkat,linkat -o %s -- %s GREYWALL_STRACE_EXIT=$? @@ -1089,7 +1091,7 @@ exit $GREYWALL_STRACE_EXIT `, ShellQuoteSingle(opts.StraceLogPath), command, )) - } else if useLandlockWrapper { + case useLandlockWrapper: // Use Landlock wrapper if available // Pass config via environment variable (serialized as JSON) // This ensures allowWrite/denyWrite rules are properly applied @@ -1110,7 +1112,7 @@ exit $GREYWALL_STRACE_EXIT // Use exec to replace bash with the wrapper (which will exec the command) innerScript.WriteString(fmt.Sprintf("exec %s\n", ShellQuote(wrapperArgs))) - } else { + default: innerScript.WriteString(command) innerScript.WriteString("\n") } diff --git a/internal/sandbox/linux_features.go b/internal/sandbox/linux_features.go index 8cef9dd..55cf662 100644 --- a/internal/sandbox/linux_features.go +++ b/internal/sandbox/linux_features.go @@ -370,7 +370,7 @@ func suggestInstallCmd(features *LinuxFeatures) string { } func readSysctl(name string) string { - data, err := os.ReadFile("/proc/sys/" + name) + data, err := os.ReadFile("/proc/sys/" + name) //nolint:gosec // reading sysctl values - trusted kernel path if err != nil { return "" } diff --git a/internal/sandbox/linux_landlock.go b/internal/sandbox/linux_landlock.go index 12ddd50..6021e46 100644 --- a/internal/sandbox/linux_landlock.go +++ b/internal/sandbox/linux_landlock.go @@ -201,7 +201,7 @@ type LandlockRuleset struct { func NewLandlockRuleset(debug bool) (*LandlockRuleset, error) { features := DetectLinuxFeatures() if !features.CanUseLandlock() { - return nil, fmt.Errorf("Landlock not available (kernel %d.%d, need 5.13+)", + return nil, fmt.Errorf("landlock not available (kernel %d.%d, need 5.13+)", features.KernelMajor, features.KernelMinor) } @@ -438,7 +438,7 @@ func (l *LandlockRuleset) addPathRule(path string, access uint64) error { // Apply applies the Landlock ruleset to the current process. func (l *LandlockRuleset) Apply() error { if !l.initialized { - return fmt.Errorf("Landlock ruleset not initialized") + return fmt.Errorf("landlock ruleset not initialized") } // Set NO_NEW_PRIVS first (required for Landlock) diff --git a/internal/sandbox/manager.go b/internal/sandbox/manager.go index 3ef9d76..8ae7ce4 100644 --- a/internal/sandbox/manager.go +++ b/internal/sandbox/manager.go @@ -78,7 +78,7 @@ func (m *Manager) Initialize() error { bridge, err := NewProxyBridge(m.config.Network.ProxyURL, m.debug) if err != nil { if m.tun2socksPath != "" { - os.Remove(m.tun2socksPath) + _ = os.Remove(m.tun2socksPath) } return fmt.Errorf("failed to initialize proxy bridge: %w", err) } @@ -90,7 +90,7 @@ func (m *Manager) Initialize() error { if err != nil { m.proxyBridge.Cleanup() if m.tun2socksPath != "" { - os.Remove(m.tun2socksPath) + _ = os.Remove(m.tun2socksPath) } return fmt.Errorf("failed to initialize DNS bridge: %w", err) } @@ -108,7 +108,7 @@ func (m *Manager) Initialize() error { m.proxyBridge.Cleanup() } if m.tun2socksPath != "" { - os.Remove(m.tun2socksPath) + _ = os.Remove(m.tun2socksPath) } return fmt.Errorf("failed to initialize reverse bridge: %w", err) } @@ -166,7 +166,7 @@ func (m *Manager) wrapCommandLearning(command string) (string, error) { if err != nil { return "", fmt.Errorf("failed to create strace log file: %w", err) } - tmpFile.Close() + _ = tmpFile.Close() m.straceLogPath = tmpFile.Name() m.logDebug("Strace log file: %s", m.straceLogPath) @@ -193,7 +193,7 @@ func (m *Manager) GenerateLearnedTemplate(cmdName string) (string, error) { } // Clean up strace log since we've processed it - os.Remove(m.straceLogPath) + _ = os.Remove(m.straceLogPath) m.straceLogPath = "" return templatePath, nil @@ -211,10 +211,10 @@ func (m *Manager) Cleanup() { m.proxyBridge.Cleanup() } if m.tun2socksPath != "" { - os.Remove(m.tun2socksPath) + _ = os.Remove(m.tun2socksPath) } if m.straceLogPath != "" { - os.Remove(m.straceLogPath) + _ = os.Remove(m.straceLogPath) m.straceLogPath = "" } m.logDebug("Sandbox manager cleaned up") diff --git a/internal/sandbox/tun2socks_embed.go b/internal/sandbox/tun2socks_embed.go index 37af78c..580eed8 100644 --- a/internal/sandbox/tun2socks_embed.go +++ b/internal/sandbox/tun2socks_embed.go @@ -38,14 +38,14 @@ func extractTun2Socks() (string, error) { } if _, err := tmpFile.Write(data); err != nil { - tmpFile.Close() - os.Remove(tmpFile.Name()) + _ = tmpFile.Close() + _ = os.Remove(tmpFile.Name()) return "", fmt.Errorf("tun2socks: failed to write binary: %w", err) } - tmpFile.Close() + _ = tmpFile.Close() - if err := os.Chmod(tmpFile.Name(), 0o755); err != nil { - os.Remove(tmpFile.Name()) + if err := os.Chmod(tmpFile.Name(), 0o755); err != nil { //nolint:gosec // executable binary needs execute permission + _ = os.Remove(tmpFile.Name()) return "", fmt.Errorf("tun2socks: failed to make executable: %w", err) }