diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml
new file mode 100644
index 0000000..b5cf90e
--- /dev/null
+++ b/.gitea/workflows/ci.yml
@@ -0,0 +1,51 @@
+name: CI
+
+on:
+ push:
+ branches: ['*']
+ pull_request:
+
+jobs:
+ lint:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+
+ - name: Install Go
+ run: |
+ curl -sSL https://go.dev/dl/go1.24.1.linux-amd64.tar.gz -o /tmp/go.tar.gz
+ tar -C /usr/local -xzf /tmp/go.tar.gz
+ echo "/usr/local/go/bin" >> $GITHUB_PATH
+
+ - name: go vet
+ run: go vet ./internal/... ./cmd/syncwarden/... ./cmd/setup/...
+
+ - name: Install golangci-lint
+ run: |
+ curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.1.6
+ echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
+
+ - name: golangci-lint
+ run: golangci-lint run ./internal/... ./cmd/syncwarden/... ./cmd/setup/...
+
+ - name: Install govulncheck
+ run: go install golang.org/x/vuln/cmd/govulncheck@latest
+
+ - name: govulncheck
+ run: govulncheck ./internal/... ./cmd/syncwarden/... ./cmd/setup/...
+
+ test:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+
+ - name: Install Go
+ run: |
+ curl -sSL https://go.dev/dl/go1.24.1.linux-amd64.tar.gz -o /tmp/go.tar.gz
+ tar -C /usr/local -xzf /tmp/go.tar.gz
+ echo "/usr/local/go/bin" >> $GITHUB_PATH
+
+ - name: Test
+ run: go test -race -count=1 ./internal/...
diff --git a/.golangci.yml b/.golangci.yml
new file mode 100644
index 0000000..7357fb2
--- /dev/null
+++ b/.golangci.yml
@@ -0,0 +1,13 @@
+linters:
+ enable:
+ - errcheck
+ - gosec
+ - govet
+ - ineffassign
+ - staticcheck
+ - unused
+
+issues:
+ exclude-dirs:
+ - cmd/panel
+ - cmd/icongen
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d4f6874..504e580 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,14 @@
# Changelog
+## v0.3.0
+
+- Fix memory leak: reuse HTTP client for long-poll events instead of creating a new one every ~30s
+- Add `syncthing_insecure_tls` config field (default: true) — TLS skip-verify is now opt-out
+- Log previously swallowed errors: config parse, config save (tray, menu, monitor)
+- Add unit tests for all monitor trackers, config round-trip, state aggregation, and detect
+- Add CI pipeline: go vet, golangci-lint, govulncheck, `go test -race`
+- Add `.golangci.yml` with errcheck, gosec, govet, ineffassign, staticcheck, unused
+
## v0.2.0
- Detect missing Syncthing installation (checks PATH and config file)
diff --git a/README.md b/README.md
index 3ea7a0f..024da18 100644
--- a/README.md
+++ b/README.md
@@ -12,8 +12,10 @@
-
-
+
+
+
+
diff --git a/internal/config/config.go b/internal/config/config.go index 1ef7c4a..a51242d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,6 +2,7 @@ package config import ( "encoding/json" + "log" "os" "sync" ) @@ -9,9 +10,10 @@ import ( // Config holds SyncWarden configuration. type Config struct { // Connection - SyncthingAddress string `json:"syncthing_address"` - SyncthingAPIKey string `json:"syncthing_api_key"` - SyncthingUseTLS bool `json:"syncthing_use_tls"` + SyncthingAddress string `json:"syncthing_address"` + SyncthingAPIKey string `json:"syncthing_api_key"` + SyncthingUseTLS bool `json:"syncthing_use_tls"` + SyncthingInsecureTLS bool `json:"syncthing_insecure_tls"` // Feature toggles EnableNotifications bool `json:"enable_notifications"` @@ -36,6 +38,7 @@ var defaults = Config{ SyncthingAddress: "localhost:8384", SyncthingAPIKey: "", SyncthingUseTLS: false, + SyncthingInsecureTLS: true, EnableNotifications: true, EnableRecentFiles: true, EnableConflictAlerts: true, @@ -65,7 +68,9 @@ func Load() Config { if err != nil { return cfg } - _ = json.Unmarshal(data, &cfg) + if err := json.Unmarshal(data, &cfg); err != nil { + log.Printf("config: parse error: %v", err) + } cached = &cfg return cfg } @@ -75,7 +80,7 @@ func Save(cfg Config) error { mu.Lock() defer mu.Unlock() - dir := ConfigDir() + dir := configDir() if err := os.MkdirAll(dir, 0o755); err != nil { return err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..1f658b4 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,90 @@ +package config + +import ( + "os" + "testing" +) + +func withTempDir(t *testing.T) { + t.Helper() + dir := t.TempDir() + configDirOverride = dir + t.Cleanup(func() { configDirOverride = "" }) +} + +func TestLoad_Defaults(t *testing.T) { + withTempDir(t) + + cfg := Load() + if cfg.SyncthingAddress != "localhost:8384" { + t.Errorf("expected default address, got %q", cfg.SyncthingAddress) + } + if !cfg.SyncthingInsecureTLS { + t.Error("SyncthingInsecureTLS should default to true") + } + if !cfg.EnableNotifications { + t.Error("EnableNotifications should default to true") + } +} + +func TestSaveLoadRoundTrip(t *testing.T) { + withTempDir(t) + + cfg := Load() + cfg.SyncthingAPIKey = "test-key-12345" + cfg.SyncthingAddress = "192.168.1.100:8384" + cfg.EnableRecentFiles = false + + if err := Save(cfg); err != nil { + t.Fatalf("Save failed: %v", err) + } + + // Clear cache so Load reads from disk + mu.Lock() + cached = nil + mu.Unlock() + + loaded := Load() + if loaded.SyncthingAPIKey != "test-key-12345" { + t.Errorf("API key not round-tripped: got %q", loaded.SyncthingAPIKey) + } + if loaded.SyncthingAddress != "192.168.1.100:8384" { + t.Errorf("address not round-tripped: got %q", loaded.SyncthingAddress) + } + if loaded.EnableRecentFiles { + t.Error("EnableRecentFiles should be false after round-trip") + } +} + +func TestBaseURL_TLSToggle(t *testing.T) { + cfg := Config{ + SyncthingAddress: "localhost:8384", + SyncthingUseTLS: false, + } + if cfg.BaseURL() != "http://localhost:8384" { + t.Errorf("expected http URL, got %q", cfg.BaseURL()) + } + + cfg.SyncthingUseTLS = true + if cfg.BaseURL() != "https://localhost:8384" { + t.Errorf("expected https URL, got %q", cfg.BaseURL()) + } +} + +func TestLoad_InvalidJSON(t *testing.T) { + withTempDir(t) + + // Write invalid JSON to the config path + if err := os.MkdirAll(configDir(), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(ConfigPath(), []byte("{invalid"), 0o644); err != nil { + t.Fatal(err) + } + + // Should return defaults without panicking + cfg := Load() + if cfg.SyncthingAddress != "localhost:8384" { + t.Errorf("expected defaults on invalid JSON, got %q", cfg.SyncthingAddress) + } +} diff --git a/internal/config/paths.go b/internal/config/paths.go index 054f898..e3cc30f 100644 --- a/internal/config/paths.go +++ b/internal/config/paths.go @@ -2,7 +2,18 @@ package config import "path/filepath" +// configDirOverride allows tests to redirect config I/O to a temp directory. +var configDirOverride string + +// configDir returns the override dir if set, otherwise the platform default. +func configDir() string { + if configDirOverride != "" { + return configDirOverride + } + return ConfigDir() +} + // ConfigPath returns the path to config.json. func ConfigPath() string { - return filepath.Join(ConfigDir(), "config.json") + return filepath.Join(configDir(), "config.json") } diff --git a/internal/monitor/conflicts_test.go b/internal/monitor/conflicts_test.go new file mode 100644 index 0000000..da83f35 --- /dev/null +++ b/internal/monitor/conflicts_test.go @@ -0,0 +1,28 @@ +package monitor + +import "testing" + +func TestConflictTracker_IncrementAndCount(t *testing.T) { + ct := NewConflictTracker() + if ct.Count() != 0 { + t.Fatalf("initial count should be 0, got %d", ct.Count()) + } + + ct.Increment() + ct.Increment() + ct.Increment() + + if ct.Count() != 3 { + t.Errorf("expected 3, got %d", ct.Count()) + } +} + +func TestConflictTracker_SetCount(t *testing.T) { + ct := NewConflictTracker() + ct.Increment() + ct.SetCount(42) + + if ct.Count() != 42 { + t.Errorf("expected 42 after SetCount, got %d", ct.Count()) + } +} diff --git a/internal/monitor/folders_test.go b/internal/monitor/folders_test.go new file mode 100644 index 0000000..9b16278 --- /dev/null +++ b/internal/monitor/folders_test.go @@ -0,0 +1,67 @@ +package monitor + +import ( + "testing" + + st "git.davoryn.de/calic/syncwarden/internal/syncthing" +) + +func TestFolderTracker_UpdateFromConfig(t *testing.T) { + ft := NewFolderTracker() + ft.UpdateFromConfig([]st.FolderConfig{ + {ID: "docs", Label: "Documents", Path: "/home/user/docs"}, + {ID: "photos", Label: "Photos", Path: "/home/user/photos"}, + }) + + folders := ft.Folders() + if len(folders) != 2 { + t.Fatalf("expected 2 folders, got %d", len(folders)) + } + if folders[0].Label != "Documents" { + t.Errorf("expected label 'Documents', got %q", folders[0].Label) + } + if folders[0].State != "unknown" { + t.Errorf("initial state should be 'unknown', got %q", folders[0].State) + } +} + +func TestFolderTracker_EmptyLabelFallback(t *testing.T) { + ft := NewFolderTracker() + ft.UpdateFromConfig([]st.FolderConfig{ + {ID: "my-folder", Label: "", Path: "/data"}, + }) + + folders := ft.Folders() + if folders[0].Label != "my-folder" { + t.Errorf("empty label should fall back to ID, got %q", folders[0].Label) + } +} + +func TestFolderTracker_UpdateStatus(t *testing.T) { + ft := NewFolderTracker() + ft.UpdateFromConfig([]st.FolderConfig{ + {ID: "docs", Label: "Docs", Path: "/docs"}, + }) + + ft.UpdateStatus("docs", "syncing") + + folders := ft.Folders() + if folders[0].State != "syncing" { + t.Errorf("expected 'syncing', got %q", folders[0].State) + } +} + +func TestFolderTracker_UpdateStatusNonexistent(t *testing.T) { + ft := NewFolderTracker() + ft.UpdateFromConfig([]st.FolderConfig{ + {ID: "docs", Label: "Docs", Path: "/docs"}, + }) + + // Should not panic + ft.UpdateStatus("nonexistent", "idle") + + folders := ft.Folders() + if folders[0].State != "unknown" { + t.Errorf("existing folder should be unchanged, got %q", folders[0].State) + } +} diff --git a/internal/monitor/monitor.go b/internal/monitor/monitor.go index 7799a51..69b9b41 100644 --- a/internal/monitor/monitor.go +++ b/internal/monitor/monitor.go @@ -82,7 +82,9 @@ func (m *Monitor) Stop() { m.mu.Lock() m.cfg.LastEventID = m.events.LastEventID() m.mu.Unlock() - _ = config.Save(m.cfg) + if err := config.Save(m.cfg); err != nil { + log.Printf("config save error: %v", err) + } } func (m *Monitor) pollLoop() { diff --git a/internal/monitor/recent_test.go b/internal/monitor/recent_test.go new file mode 100644 index 0000000..3807219 --- /dev/null +++ b/internal/monitor/recent_test.go @@ -0,0 +1,44 @@ +package monitor + +import "testing" + +func TestRecentTracker_AddOrder(t *testing.T) { + rt := NewRecentTracker() + rt.Add("a.txt", "docs") + rt.Add("b.txt", "docs") + + files := rt.Files() + if len(files) != 2 { + t.Fatalf("expected 2 files, got %d", len(files)) + } + if files[0].Name != "b.txt" { + t.Errorf("most recent should be first, got %s", files[0].Name) + } + if files[1].Name != "a.txt" { + t.Errorf("oldest should be last, got %s", files[1].Name) + } +} + +func TestRecentTracker_RingBufferOverflow(t *testing.T) { + rt := NewRecentTracker() + for i := 0; i < 15; i++ { + rt.Add("file", "f") + } + files := rt.Files() + if len(files) != maxRecentFiles { + t.Errorf("expected %d files after overflow, got %d", maxRecentFiles, len(files)) + } +} + +func TestRecentTracker_FilesCopy(t *testing.T) { + rt := NewRecentTracker() + rt.Add("a.txt", "docs") + + files := rt.Files() + files[0].Name = "mutated" + + original := rt.Files() + if original[0].Name != "a.txt" { + t.Error("Files() should return a copy, but internal state was mutated") + } +} diff --git a/internal/monitor/speed_test.go b/internal/monitor/speed_test.go new file mode 100644 index 0000000..774ef33 --- /dev/null +++ b/internal/monitor/speed_test.go @@ -0,0 +1,60 @@ +package monitor + +import ( + "testing" + "time" +) + +func TestSpeedTracker_FirstUpdateBaseline(t *testing.T) { + s := NewSpeedTracker() + s.Update(1000, 500) + + down, up := s.Rates() + if down != 0 || up != 0 { + t.Errorf("first update should be baseline (0,0), got (%f,%f)", down, up) + } +} + +func TestSpeedTracker_RateCalculation(t *testing.T) { + s := NewSpeedTracker() + + // Seed baseline + s.mu.Lock() + s.lastIn = 0 + s.lastOut = 0 + s.lastTime = time.Now().Add(-1 * time.Second) + s.mu.Unlock() + + s.Update(1000, 500) + + down, up := s.Rates() + // Allow some tolerance for timing + if down < 900 || down > 1100 { + t.Errorf("expected ~1000 B/s down, got %f", down) + } + if up < 400 || up > 600 { + t.Errorf("expected ~500 B/s up, got %f", up) + } +} + +func TestSpeedTracker_NegativeDeltaClamped(t *testing.T) { + s := NewSpeedTracker() + + // Seed with high values + s.mu.Lock() + s.lastIn = 5000 + s.lastOut = 3000 + s.lastTime = time.Now().Add(-1 * time.Second) + s.mu.Unlock() + + // Update with lower values (counter reset) + s.Update(100, 50) + + down, up := s.Rates() + if down < 0 { + t.Errorf("negative download rate not clamped: %f", down) + } + if up < 0 { + t.Errorf("negative upload rate not clamped: %f", up) + } +} diff --git a/internal/monitor/state_test.go b/internal/monitor/state_test.go new file mode 100644 index 0000000..1cef7e5 --- /dev/null +++ b/internal/monitor/state_test.go @@ -0,0 +1,68 @@ +package monitor + +import ( + "testing" + + "git.davoryn.de/calic/syncwarden/internal/icons" +) + +func TestStateFromFolders_Empty(t *testing.T) { + got := stateFromFolders(nil, false) + if got != icons.StateIdle { + t.Errorf("empty folders should be idle, got %d", got) + } +} + +func TestStateFromFolders_Idle(t *testing.T) { + folders := []FolderInfo{ + {ID: "a", State: "idle"}, + {ID: "b", State: "idle"}, + } + got := stateFromFolders(folders, false) + if got != icons.StateIdle { + t.Errorf("all idle should be StateIdle, got %d", got) + } +} + +func TestStateFromFolders_Syncing(t *testing.T) { + folders := []FolderInfo{ + {ID: "a", State: "idle"}, + {ID: "b", State: "syncing"}, + } + got := stateFromFolders(folders, false) + if got != icons.StateSyncing { + t.Errorf("syncing folder should produce StateSyncing, got %d", got) + } +} + +func TestStateFromFolders_ScanningIsSyncing(t *testing.T) { + folders := []FolderInfo{ + {ID: "a", State: "scanning"}, + } + got := stateFromFolders(folders, false) + if got != icons.StateSyncing { + t.Errorf("scanning should map to StateSyncing, got %d", got) + } +} + +func TestStateFromFolders_ErrorOverSyncing(t *testing.T) { + folders := []FolderInfo{ + {ID: "a", State: "syncing"}, + {ID: "b", State: "error"}, + } + got := stateFromFolders(folders, false) + if got != icons.StateError { + t.Errorf("error should take priority over syncing, got %d", got) + } +} + +func TestStateFromFolders_PausedOverAll(t *testing.T) { + folders := []FolderInfo{ + {ID: "a", State: "error"}, + {ID: "b", State: "syncing"}, + } + got := stateFromFolders(folders, true) + if got != icons.StatePaused { + t.Errorf("paused should override all states, got %d", got) + } +} diff --git a/internal/syncthing/client.go b/internal/syncthing/client.go index 088ca87..d58024c 100644 --- a/internal/syncthing/client.go +++ b/internal/syncthing/client.go @@ -11,21 +11,28 @@ import ( // Client talks to the Syncthing REST API. type Client struct { - baseURL string - apiKey string - httpClient *http.Client + baseURL string + apiKey string + httpClient *http.Client + longPollClient *http.Client } // NewClient creates a Syncthing API client. -func NewClient(baseURL, apiKey string) *Client { +// When insecureTLS is true, TLS certificate verification is skipped. +// This is the common case for local Syncthing instances that use self-signed certs. +func NewClient(baseURL, apiKey string, insecureTLS bool) *Client { + //nolint:gosec // Syncthing typically uses self-signed certs; controlled by config + tlsCfg := &tls.Config{InsecureSkipVerify: insecureTLS} return &Client{ baseURL: baseURL, apiKey: apiKey, httpClient: &http.Client{ - Timeout: 10 * time.Second, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, + Timeout: 10 * time.Second, + Transport: &http.Transport{TLSClientConfig: tlsCfg}, + }, + longPollClient: &http.Client{ + Timeout: 40 * time.Second, + Transport: &http.Transport{TLSClientConfig: tlsCfg}, }, } } @@ -143,13 +150,6 @@ func (c *Client) FolderStatus(folderID string) (*FolderStatus, error) { // Events long-polls for new events since the given ID. func (c *Client) Events(since int, timeout int) ([]Event, error) { path := fmt.Sprintf("/rest/events?since=%d&timeout=%d", since, timeout) - // Use a longer HTTP timeout for long-polling - client := &http.Client{ - Timeout: time.Duration(timeout+10) * time.Second, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, - } req, err := http.NewRequest("GET", c.baseURL+path, nil) if err != nil { return nil, err @@ -157,7 +157,7 @@ func (c *Client) Events(since int, timeout int) ([]Event, error) { if c.apiKey != "" { req.Header.Set("X-API-Key", c.apiKey) } - resp, err := client.Do(req) + resp, err := c.longPollClient.Do(req) if err != nil { return nil, err } diff --git a/internal/syncthing/detect_test.go b/internal/syncthing/detect_test.go new file mode 100644 index 0000000..5db8d8d --- /dev/null +++ b/internal/syncthing/detect_test.go @@ -0,0 +1,9 @@ +package syncthing + +import "testing" + +func TestIsInstalled_NoPanic(t *testing.T) { + // IsInstalled should not panic regardless of environment. + // We don't assert the result since it depends on the host. + _ = IsInstalled() +} diff --git a/internal/tray/menu.go b/internal/tray/menu.go index 941766c..624f925 100644 --- a/internal/tray/menu.go +++ b/internal/tray/menu.go @@ -208,7 +208,9 @@ func (a *App) toggleSetting(field *bool, item *systray.MenuItem) { } else { item.Uncheck() } - _ = config.Save(cfg) + if err := config.Save(cfg); err != nil { + log.Printf("config save error: %v", err) + } } func (a *App) rediscoverAPIKey() { @@ -227,6 +229,8 @@ func (a *App) rediscoverAPIKey() { a.mu.Unlock() a.client.SetAPIKey(key) - _ = config.Save(a.cfg) + if err := config.Save(a.cfg); err != nil { + log.Printf("config save error: %v", err) + } log.Printf("re-discovered API key") } diff --git a/internal/tray/tray.go b/internal/tray/tray.go index 898bafd..45d8c3a 100644 --- a/internal/tray/tray.go +++ b/internal/tray/tray.go @@ -14,7 +14,7 @@ import ( st "git.davoryn.de/calic/syncwarden/internal/syncthing" ) -const version = "0.2.0" +const version = "0.3.0" // App manages the tray icon and Syncthing monitoring. type App struct { @@ -54,12 +54,14 @@ func (a *App) onReady() { if a.cfg.SyncthingAPIKey == "" { if key, err := st.DiscoverAPIKey(); err == nil && key != "" { a.cfg.SyncthingAPIKey = key - _ = config.Save(a.cfg) + if err := config.Save(a.cfg); err != nil { + log.Printf("config save error: %v", err) + } log.Printf("auto-discovered Syncthing API key") } } - a.client = st.NewClient(a.cfg.BaseURL(), a.cfg.SyncthingAPIKey) + a.client = st.NewClient(a.cfg.BaseURL(), a.cfg.SyncthingAPIKey, a.cfg.SyncthingInsecureTLS) // Check if Syncthing is installed if !st.IsInstalled() {