From d4380c0aea5a676ed6515c274858dd203391f192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torjus=20H=C3=A5kestad?= Date: Sat, 14 Feb 2026 21:43:49 +0100 Subject: [PATCH] chore: add golangci-lint config and fix all lint issues Enable 15 additional linters (gosec, errorlint, gocritic, modernize, misspell, bodyclose, sqlclosecheck, nilerr, unconvert, durationcheck, sloglint, wastedassign, usestdlibvars) with sensible exclusion rules. Fix all findings: errors.Is for error comparisons, run() pattern in main to avoid exitAfterDefer, ReadHeaderTimeout for Slowloris protection, bounds check in escape sequence reader, WaitGroup.Go, slices.Contains, range-over-int loops, and http.MethodGet constants. Co-Authored-By: Claude Opus 4.6 --- .golangci.yml | 79 +++++++++++++++++++++++++++++++ cmd/oubliette/main.go | 36 +++++++------- internal/auth/auth_test.go | 14 +++--- internal/detection/scorer_test.go | 22 ++++----- internal/notify/webhook.go | 8 +--- internal/shell/bash/bash.go | 10 ++-- internal/shell/bash/bash_test.go | 3 +- internal/storage/sqlite_test.go | 6 +-- internal/storage/store_test.go | 6 +-- internal/web/web_test.go | 12 ++--- 10 files changed, 134 insertions(+), 62 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..8f6d42e --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,79 @@ +version: "2" + +linters: + enable: + # Bug detectors. + - bodyclose + - durationcheck + - errorlint + - gocritic + - nilerr + - sqlclosecheck + + # Security. + - gosec + + # Style and modernization. + - misspell + - modernize + - unconvert + - usestdlibvars + + # Logging. + - sloglint + + # Dead code. + - wastedassign + + settings: + errcheck: + exclude-functions: + # Terminal I/O writes (honeypot shell output). + - fmt.Fprint + - fmt.Fprintf + # Low-level byte I/O in shell readLine (escape sequences, echo). + - (io.ReadWriter).Read + - (io.ReadWriter).Write + - (io.ReadWriteCloser).Read + - (io.ReadWriteCloser).Write + - (io.Reader).Read + - (io.Writer).Write + + gosec: + excludes: + # File reads from config paths — expected in a CLI tool. + - G304 + # Weak RNG for shell selection — crypto/rand not needed. + - G404 + + exclusions: + rules: + # Ignore unchecked Close() — standard resource cleanup. + - linters: [errcheck] + text: "Error return value of .+\\.Close.+ is not checked" + + # Ignore unchecked Rollback() — called in error paths before returning. + - linters: [errcheck] + text: "Error return value of .+\\.Rollback.+ is not checked" + + # Ignore unchecked Reply/Reject — SSH protocol; nothing useful on failure. + - linters: [errcheck] + text: "Error return value of .+\\.(Reply|Reject).+ is not checked" + + # Test files: allow unchecked errors. + - linters: [errcheck] + path: "_test\\.go" + + # Test files: InsecureIgnoreHostKey, file permissions, unhandled errors are expected. + - linters: [gosec] + path: "_test\\.go" + + # Unhandled errors for cleanup/protocol ops — mirrors errcheck exclusions. + - linters: [gosec] + text: "G104" + source: "\\.(Close|Rollback|Reject|Reply|Read|Write)\\(" + + # SQL with safe column interpolation from a fixed switch — not user input. + - linters: [gosec] + text: "G201" + path: "internal/storage/" diff --git a/cmd/oubliette/main.go b/cmd/oubliette/main.go index 26fccd0..030484e 100644 --- a/cmd/oubliette/main.go +++ b/cmd/oubliette/main.go @@ -4,12 +4,14 @@ import ( "context" "errors" "flag" + "fmt" "log/slog" "net/http" "os" "os/signal" "sync" "syscall" + "time" "git.t-juice.club/torjus/oubliette/internal/config" "git.t-juice.club/torjus/oubliette/internal/server" @@ -20,13 +22,19 @@ import ( const Version = "0.2.0" func main() { + if err := run(); err != nil { + slog.Error("fatal error", "err", err) + os.Exit(1) + } +} + +func run() error { configPath := flag.String("config", "oubliette.toml", "path to config file") flag.Parse() cfg, err := config.Load(*configPath) if err != nil { - slog.Error("failed to load config", "err", err) - os.Exit(1) + return fmt.Errorf("load config: %w", err) } level := new(slog.LevelVar) @@ -53,8 +61,7 @@ func main() { store, err := storage.NewSQLiteStore(cfg.Storage.DBPath) if err != nil { - logger.Error("failed to open database", "err", err) - os.Exit(1) + return fmt.Errorf("open database: %w", err) } defer store.Close() @@ -65,8 +72,7 @@ func main() { srv, err := server.New(*cfg, store, logger) if err != nil { - logger.Error("failed to create server", "err", err) - os.Exit(1) + return fmt.Errorf("create server: %w", err) } var wg sync.WaitGroup @@ -75,23 +81,21 @@ func main() { if cfg.Web.Enabled { webHandler, err := web.NewServer(store, logger.With("component", "web")) if err != nil { - logger.Error("failed to create web server", "err", err) - os.Exit(1) + return fmt.Errorf("create web server: %w", err) } httpServer := &http.Server{ - Addr: cfg.Web.ListenAddr, - Handler: webHandler, + Addr: cfg.Web.ListenAddr, + Handler: webHandler, + ReadHeaderTimeout: 10 * time.Second, } - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { logger.Info("web server listening", "addr", cfg.Web.ListenAddr) if err := httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { logger.Error("web server error", "err", err) } - }() + }) // Graceful shutdown on context cancellation. go func() { @@ -103,10 +107,10 @@ func main() { } if err := srv.ListenAndServe(ctx); err != nil { - logger.Error("server error", "err", err) - os.Exit(1) + return fmt.Errorf("server: %w", err) } wg.Wait() logger.Info("server stopped") + return nil } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index c083db3..c74f036 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -36,7 +36,7 @@ func TestStaticCredentialsWrongPassword(t *testing.T) { func TestRejectionBeforeThreshold(t *testing.T) { a := newTestAuth(3, time.Hour) - for i := 0; i < 2; i++ { + for i := range 2 { d := a.Authenticate("1.2.3.4", "user", "pass") if d.Accepted { t.Fatalf("attempt %d should be rejected", i+1) @@ -49,7 +49,7 @@ func TestRejectionBeforeThreshold(t *testing.T) { func TestThresholdAcceptance(t *testing.T) { a := newTestAuth(3, time.Hour) - for i := 0; i < 2; i++ { + for i := range 2 { d := a.Authenticate("1.2.3.4", "user", "pass") if d.Accepted { t.Fatalf("attempt %d should be rejected", i+1) @@ -65,7 +65,7 @@ func TestPerIPIsolation(t *testing.T) { a := newTestAuth(3, time.Hour) // IP1 gets 2 failures. - for i := 0; i < 2; i++ { + for range 2 { a.Authenticate("1.1.1.1", "user", "pass") } @@ -157,12 +157,10 @@ func TestConcurrentAccess(t *testing.T) { a := newTestAuth(5, time.Hour) var wg sync.WaitGroup - for i := 0; i < 100; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range 100 { + wg.Go(func() { a.Authenticate("1.2.3.4", "user", "pass") - }() + }) } wg.Wait() } diff --git a/internal/detection/scorer_test.go b/internal/detection/scorer_test.go index 8835a3c..702bf75 100644 --- a/internal/detection/scorer_test.go +++ b/internal/detection/scorer_test.go @@ -87,7 +87,7 @@ func TestScorer_OutputIgnored(t *testing.T) { now := time.Now() // Only output events — should not affect score. - for i := 0; i < 100; i++ { + for range 100 { s.RecordEvent(now, DirOutput, []byte("some output\n")) now = now.Add(10 * time.Millisecond) } @@ -103,25 +103,21 @@ func TestScorer_ThreadSafety(t *testing.T) { now := time.Now() var wg sync.WaitGroup - for i := 0; i < 10; i++ { - wg.Add(1) - go func(offset int) { - defer wg.Done() - for j := 0; j < 100; j++ { - ts := now.Add(time.Duration(offset*100+j) * time.Millisecond) + for i := range 10 { + wg.Go(func() { + for j := range 100 { + ts := now.Add(time.Duration(i*100+j) * time.Millisecond) s.RecordEvent(ts, DirInput, []byte("a")) } - }(i) + }) } // Concurrently read score. - wg.Add(1) - go func() { - defer wg.Done() - for i := 0; i < 50; i++ { + wg.Go(func() { + for range 50 { _ = s.Score() } - }() + }) wg.Wait() diff --git a/internal/notify/webhook.go b/internal/notify/webhook.go index c96cc25..098289f 100644 --- a/internal/notify/webhook.go +++ b/internal/notify/webhook.go @@ -6,6 +6,7 @@ import ( "encoding/json" "log/slog" "net/http" + "slices" "sync" "time" @@ -98,12 +99,7 @@ func (n *Notifier) shouldSend(wh config.WebhookNotifyConfig, eventType string) b if len(wh.Events) == 0 { return true // empty = all events } - for _, ev := range wh.Events { - if ev == eventType { - return true - } - } - return false + return slices.Contains(wh.Events, eventType) } func (n *Notifier) send(ctx context.Context, wh config.WebhookNotifyConfig, payload webhookPayload) { diff --git a/internal/shell/bash/bash.go b/internal/shell/bash/bash.go index fa03178..0688af9 100644 --- a/internal/shell/bash/bash.go +++ b/internal/shell/bash/bash.go @@ -2,6 +2,7 @@ package bash import ( "context" + "errors" "fmt" "io" "strings" @@ -55,7 +56,7 @@ func (b *BashShell) Handle(ctx context.Context, sess *shell.SessionContext, rw i } line, err := readLine(ctx, rw) - if err == io.EOF { + if errors.Is(err, io.EOF) { fmt.Fprint(rw, "logout\r\n") return nil } @@ -81,7 +82,9 @@ func (b *BashShell) Handle(ctx context.Context, sess *shell.SessionContext, rw i // Log command and output to store. if sess.Store != nil { - sess.Store.AppendSessionLog(ctx, sess.SessionID, trimmed, output) + if err := sess.Store.AppendSessionLog(ctx, sess.SessionID, trimmed, output); err != nil { + return fmt.Errorf("append session log: %w", err) + } } if result.exit { @@ -145,8 +148,7 @@ func readLine(ctx context.Context, rw io.ReadWriter) (string, error) { // Read and discard the rest of the escape sequence. // Most are 3 bytes: ESC [ X (arrow keys, etc.) next := make([]byte, 1) - rw.Read(next) - if next[0] == '[' { + if n, _ := rw.Read(next); n > 0 && next[0] == '[' { rw.Read(next) // read the final byte } diff --git a/internal/shell/bash/bash_test.go b/internal/shell/bash/bash_test.go index bd3c8d9..c845731 100644 --- a/internal/shell/bash/bash_test.go +++ b/internal/shell/bash/bash_test.go @@ -3,6 +3,7 @@ package bash import ( "bytes" "context" + "errors" "io" "strings" "testing" @@ -108,7 +109,7 @@ func TestReadLineCtrlD(t *testing.T) { ctx := context.Background() _, err := readLine(ctx, rw) - if err != io.EOF { + if !errors.Is(err, io.EOF) { t.Fatalf("expected io.EOF, got %v", err) } } diff --git a/internal/storage/sqlite_test.go b/internal/storage/sqlite_test.go index 4e93b2a..5739cd8 100644 --- a/internal/storage/sqlite_test.go +++ b/internal/storage/sqlite_test.go @@ -205,11 +205,7 @@ func TestDeleteRecordsBefore(t *testing.T) { } func TestNewSQLiteStoreCreatesFile(t *testing.T) { - dbPath := filepath.Join(t.TempDir(), "subdir", "test.db") - // Parent directory doesn't exist yet; SQLite should create it. - // Actually, SQLite doesn't create parent dirs, but the file itself. - // Use a path in the temp dir directly. - dbPath = filepath.Join(t.TempDir(), "test.db") + dbPath := filepath.Join(t.TempDir(), "test.db") store, err := NewSQLiteStore(dbPath) if err != nil { t.Fatalf("creating store: %v", err) diff --git a/internal/storage/store_test.go b/internal/storage/store_test.go index 5d0bd53..50d6c27 100644 --- a/internal/storage/store_test.go +++ b/internal/storage/store_test.go @@ -37,17 +37,17 @@ func seedData(t *testing.T, store Store) { ctx := context.Background() // Login attempts: root/toor from two IPs, admin/admin from one IP. - for i := 0; i < 5; i++ { + for range 5 { if err := store.RecordLoginAttempt(ctx, "root", "toor", "10.0.0.1"); err != nil { t.Fatalf("seeding attempt: %v", err) } } - for i := 0; i < 3; i++ { + for range 3 { if err := store.RecordLoginAttempt(ctx, "root", "toor", "10.0.0.2"); err != nil { t.Fatalf("seeding attempt: %v", err) } } - for i := 0; i < 2; i++ { + for range 2 { if err := store.RecordLoginAttempt(ctx, "admin", "admin", "10.0.0.1"); err != nil { t.Fatalf("seeding attempt: %v", err) } diff --git a/internal/web/web_test.go b/internal/web/web_test.go index ad722ac..7415806 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -27,7 +27,7 @@ func newSeededTestServer(t *testing.T) *Server { store := storage.NewMemoryStore() ctx := context.Background() - for i := 0; i < 5; i++ { + for range 5 { if err := store.RecordLoginAttempt(ctx, "root", "toor", "10.0.0.1"); err != nil { t.Fatalf("seeding attempt: %v", err) } @@ -54,7 +54,7 @@ func newSeededTestServer(t *testing.T) *Server { func TestDashboardHandler(t *testing.T) { t.Run("empty store", func(t *testing.T) { srv := newTestServer(t) - req := httptest.NewRequest("GET", "/", nil) + req := httptest.NewRequest(http.MethodGet, "/", nil) w := httptest.NewRecorder() srv.ServeHTTP(w, req) @@ -73,7 +73,7 @@ func TestDashboardHandler(t *testing.T) { t.Run("with data", func(t *testing.T) { srv := newSeededTestServer(t) - req := httptest.NewRequest("GET", "/", nil) + req := httptest.NewRequest(http.MethodGet, "/", nil) w := httptest.NewRecorder() srv.ServeHTTP(w, req) @@ -93,7 +93,7 @@ func TestDashboardHandler(t *testing.T) { func TestFragmentStats(t *testing.T) { srv := newSeededTestServer(t) - req := httptest.NewRequest("GET", "/fragments/stats", nil) + req := httptest.NewRequest(http.MethodGet, "/fragments/stats", nil) w := httptest.NewRecorder() srv.ServeHTTP(w, req) @@ -113,7 +113,7 @@ func TestFragmentStats(t *testing.T) { func TestFragmentActiveSessions(t *testing.T) { srv := newSeededTestServer(t) - req := httptest.NewRequest("GET", "/fragments/active-sessions", nil) + req := httptest.NewRequest(http.MethodGet, "/fragments/active-sessions", nil) w := httptest.NewRecorder() srv.ServeHTTP(w, req) @@ -144,7 +144,7 @@ func TestStaticAssets(t *testing.T) { for _, tt := range tests { t.Run(tt.path, func(t *testing.T) { - req := httptest.NewRequest("GET", tt.path, nil) + req := httptest.NewRequest(http.MethodGet, tt.path, nil) w := httptest.NewRecorder() srv.ServeHTTP(w, req)