v0.12.4 — 15 bug fixes (CRITICAL/HIGH/MEDIUM)

CRITICAL:
- C1: SetAppBackupBulk data loss + nil map panic (settings.go)
- C2: UpdateStackConfig nil Env map panic (deploy.go)
- C3: ValidateDump missing scanner.Err() check (dbdump.go)

HIGH:
- H1: nextDailyRun DST bug — use time.Date(day+1) not Add(24h)
- H2: Cache Europe/Budapest timezone with sync.Once in scheduler
- H3: settings.save() leaks .tmp file on WriteFile failure
- H4: SetNotificationPrefs nil pointer panic
- H5: appDirSize + getDirSizeBytes ignore Sscanf return value
- H6: getDirSizeBytes has no timeout — add 30s context
- H7: dbdump.go tmpFile not using defer Close
- H8: UpdateCrossDriveStatus misleading comment

MEDIUM:
- M1: Replace custom containsBytes with strings.Contains
- M2: scheduler.Every() validates interval > 0
- M3: executeJob panic recovery now sets LastRun
- M4: logPostStartStatus copies env slice before goroutine
- M5: Cache timezone in web package via getTimezone() sync.Once

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -1,5 +1,32 @@
 ## Changelog
 
+### What was just completed (2026-02-18 session 40)
+- **v0.12.4 — Correctness & Robustness Bug Fixes (TASK.md — 15 bugs fixed):**
+
+  **CRITICAL fixes (data loss, panics):**
+  - **C1: `SetAppBackupBulk` data loss + nil map panic** — Fixed: now updates map IN PLACE instead of replacing it, so stacks absent from the input are preserved. Added nil guard for `s.AppBackup`. (`internal/settings/settings.go`)
+  - **C2: `UpdateStackConfig` nil Env map panic** — Added nil check `if appCfg.Env == nil { appCfg.Env = make(...) }` before the field assignment loop. (`internal/stacks/deploy.go`)
+  - **C3: `ValidateDump` missing scanner.Err() check** — Added `if err := scanner.Err()` check after the scan loop so I/O errors don't silently mark a partial dump as valid. (`internal/backup/dbdump.go`)
+
+  **HIGH fixes (logic errors, resource leaks):**
+  - **H1: `nextDailyRun` DST bug** — Replaced `next.Add(24 * time.Hour)` with `time.Date(day+1, ...)` for correct scheduling across Europe/Budapest DST transitions. (`internal/scheduler/scheduler.go`)
+  - **H2: `nextDailyRun` repeated `LoadLocation`** — Cached timezone in package-level `sync.Once` variable; `getBudapestLocation()` now loaded only once. (`internal/scheduler/scheduler.go`)
+  - **H3: `settings.save()` .tmp file leak** — Added `os.Remove(tmpPath)` cleanup on `WriteFile` failure path. (`internal/settings/settings.go`)
+  - **H4: `SetNotificationPrefs` nil pointer panic** — Added nil guard at start of function, returns error instead of panicking. (`internal/settings/settings.go`)
+  - **H5: `appDirSize` ignores `Sscanf` return value** — Now checks `n != 1` and returns `(0, "?")` on parse failure. Same fix applied to `getDirSizeBytes` in `stacks/delete.go`. (`internal/backup/appdata.go`, `internal/stacks/delete.go`)
+  - **H6: `getDirSizeBytes` no timeout** — Added `exec.CommandContext` with 30s timeout. Added `"context"` import. (`internal/stacks/delete.go`)
+  - **H7: `dbdump.go` tmpFile not using `defer Close`** — Replaced explicit `tmpFile.Close()` call with `defer tmpFile.Close()` so the file handle is released even on panic. (`internal/backup/dbdump.go`)
+  - **H8: `UpdateCrossDriveStatus` misleading comment** — Updated comment to accurately describe the "does nothing if nil" behavior instead of claiming it "creates one if nil". (`internal/settings/settings.go`)
+
+  **MEDIUM fixes (code quality, edge cases):**
+  - **M1: Custom `contains`/`containsBytes` replaced** — Removed bespoke `containsBytes` and simplified `contains` to delegate to `strings.Contains`. Added `"strings"` import. (`internal/notify/notifier.go`)
+  - **M2: `scheduler.Every()` doesn't validate interval** — Added early return with error log if `interval <= 0` to prevent panic in `time.NewTicker`. (`internal/scheduler/scheduler.go`)
+  - **M3: `executeJob` panic recovery missing `LastRun`** — Panic recovery defer now also sets `job.LastRun = time.Now()` so the job status shows a timestamp after a panic. (`internal/scheduler/scheduler.go`)
+  - **M4: `logPostStartStatus` goroutine captures env by reference** — Copies the env slice before launching the goroutine (`envCopy`). (`internal/stacks/manager.go`)
+  - **M5: Multiple `time.LoadLocation` calls in web package** — Added package-level `getTimezone()` with `sync.Once` in `funcmap.go`. Replaced all `time.LoadLocation("Europe/Budapest")` calls in the web package with `getTimezone()`. (`internal/web/funcmap.go`, `internal/web/handlers.go`)
+
+  **Files modified (8):** `internal/settings/settings.go`, `internal/stacks/deploy.go`, `internal/backup/dbdump.go`, `internal/scheduler/scheduler.go`, `internal/backup/appdata.go`, `internal/stacks/delete.go`, `internal/stacks/manager.go`, `internal/notify/notifier.go`, `internal/web/funcmap.go`, `internal/web/handlers.go`
+
 ### What was just completed (2026-02-17 session 39)
 - **v0.12.3 — Security & Correctness Bug Fixes (TASK.md — 33 bugs fixed):**
 

controller/internal/backup/appdata.go

@@ -147,7 +147,9 @@ func appDirSize(path string) (int64, string) {
 		return 0, "?"
 	}
 	var size int64
-	fmt.Sscanf(fields[0], "%d", &size)
+	if n, _ := fmt.Sscanf(fields[0], "%d", &size); n != 1 {
+		return 0, "?"
+	}
 	return size, humanizeBytes(size)
 }
 

controller/internal/backup/dbdump.go

@@ -181,13 +181,13 @@ func DumpOne(ctx context.Context, db DiscoveredDB, dumpDir string, logger *log.L
 		result.Duration = time.Since(start)
 		return result
 	}
+	defer tmpFile.Close()
 
 	cmd.Stdout = tmpFile
 	var stderr strings.Builder
 	cmd.Stderr = &stderr
 
 	err = cmd.Run()
-	tmpFile.Close()
 
 	if err != nil {
 		os.Remove(tmpPath)
@@ -293,6 +293,13 @@ func ValidateDump(filePath string, dbType DBType) DumpValidation {
 			tableCount++
 		}
 	}
+
+	if err := scanner.Err(); err != nil {
+		v.Error = fmt.Sprintf("hiba az olvasás közben: %v", err)
+		log.Printf("[WARN] ValidateDump FAIL: %s — scanner error: %v", filePath, err)
+		return v
+	}
+
 	v.TableCount = tableCount
 
 	if !headerFound {

controller/internal/notify/notifier.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"log"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 
@@ -314,14 +315,5 @@ func classifyWarning(message string) string {
 }
 
 func contains(s, substr string) bool {
-	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsBytes(s, substr))
+	return strings.Contains(s, substr)
-}
-
-func containsBytes(s, substr string) bool {
-	for i := 0; i <= len(s)-len(substr); i++ {
-		if s[i:i+len(substr)] == substr {
-			return true
-		}
-	}
-	return false
 }

controller/internal/scheduler/scheduler.go

@@ -8,6 +8,23 @@ import (
 	"time"
 )
 
+var (
+	budapestLoc     *time.Location
+	budapestLocOnce sync.Once
+)
+
+func getBudapestLocation() *time.Location {
+	budapestLocOnce.Do(func() {
+		loc, err := time.LoadLocation("Europe/Budapest")
+		if err != nil {
+			log.Printf("[ERROR] Cannot load Europe/Budapest timezone: %v — using UTC", err)
+			loc = time.UTC
+		}
+		budapestLoc = loc
+	})
+	return budapestLoc
+}
+
 // JobFunc is the function signature for scheduler jobs.
 type JobFunc func(ctx context.Context) error
 
@@ -41,6 +58,11 @@ func New(logger *log.Logger) *Scheduler {
 
 // Every registers a periodic job that runs every interval.
 func (s *Scheduler) Every(name string, interval time.Duration, fn JobFunc) {
+	if interval <= 0 {
+		s.logger.Printf("[ERROR] Periodic job %s has invalid interval %s — job not registered", name, interval)
+		return
+	}
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -187,6 +209,7 @@ func (s *Scheduler) executeJob(job *Job, quiet bool) {
 		if r := recover(); r != nil {
 			s.mu.Lock()
 			job.LastErr = fmt.Errorf("panic: %v", r)
+			job.LastRun = time.Now()
 			s.mu.Unlock()
 			s.logger.Printf("[ERROR] Job %s panicked: %v", job.Name, r)
 		}
@@ -238,18 +261,16 @@ func nextDailyRun(timeStr string) time.Time {
 		return time.Now().Add(24 * time.Hour)
 	}
 
-	loc, err := time.LoadLocation("Europe/Budapest")
+	loc := getBudapestLocation()
-	if err != nil {
-		// Fallback to UTC if timezone not available
-		loc = time.UTC
-	}
 
 	now := time.Now().In(loc)
 	next := time.Date(now.Year(), now.Month(), now.Day(), hour, min, 0, 0, loc)
 
-	// If the time has already passed today, schedule for tomorrow
+	// If the time has already passed today, schedule for tomorrow.
+	// Use time.Date with day+1 instead of Add(24h) to correctly handle DST transitions
+	// (spring forward/fall back in Europe/Budapest would shift by 1 hour with Add(24h)).
 	if !next.After(now) {
-		next = next.Add(24 * time.Hour)
+		next = time.Date(now.Year(), now.Month(), now.Day()+1, hour, min, 0, 0, loc)
 	}
 
 	return next

controller/internal/settings/settings.go

@@ -136,6 +136,7 @@ func (s *Settings) save() error {
 	}
 
 	if err := os.WriteFile(tmpPath, data, 0644); err != nil {
+		os.Remove(tmpPath) // clean up partial file
 		return fmt.Errorf("writing tmp settings: %w", err)
 	}
 
@@ -215,6 +216,9 @@ func (s *Settings) GetNotificationPrefs() *NotificationPrefs {
 // SetNotificationPrefs updates notification preferences and saves to disk.
 // H17: Deep-copies prefs so caller mutations after the call don't affect stored state.
 func (s *Settings) SetNotificationPrefs(prefs *NotificationPrefs) error {
+	if prefs == nil {
+		return fmt.Errorf("notification preferences cannot be nil")
+	}
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	copy := *prefs
@@ -267,17 +271,18 @@ func (s *Settings) GetAppBackupMap() map[string]bool {
 }
 
 // SetAppBackupBulk updates backup prefs for all stacks at once and saves to disk.
-// Preserves existing CrossDrive configs.
+// Preserves existing CrossDrive configs and stacks not present in the input.
 func (s *Settings) SetAppBackupBulk(prefs map[string]bool) error {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	newMap := make(map[string]AppBackupPrefs, len(prefs))
+	if s.AppBackup == nil {
+		s.AppBackup = make(map[string]AppBackupPrefs)
+	}
 	for name, enabled := range prefs {
 		existing := s.AppBackup[name] // preserves CrossDrive
 		existing.Enabled = enabled
-		newMap[name] = existing
+		s.AppBackup[name] = existing
 	}
-	s.AppBackup = newMap
 	return s.save()
 }
 
@@ -321,7 +326,8 @@ func (s *Settings) SetCrossDriveConfig(stackName string, cfg *CrossDriveBackup)
 }
 
 // UpdateCrossDriveStatus updates runtime status fields for a cross-drive backup in-place.
-// fn receives a pointer to the CrossDriveBackup (creates one if nil) and may mutate it.
+// fn receives a pointer to the CrossDriveBackup and may mutate it.
+// If no cross-drive config exists for the stack, does nothing and returns nil.
 func (s *Settings) UpdateCrossDriveStatus(stackName string, fn func(*CrossDriveBackup)) error {
 	s.mu.Lock()
 	defer s.mu.Unlock()

controller/internal/stacks/delete.go

@@ -2,6 +2,7 @@ package stacks
 
 import (
 	"bufio"
+	"context"
 	"fmt"
 	"os"
 	"os/exec"
@@ -278,7 +279,9 @@ func getDirSizeHuman(path string) string {
 
 // getDirSizeBytes returns the total size in bytes for a directory.
 func getDirSizeBytes(path string) int64 {
-	cmd := exec.Command("du", "-sb", path)
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, "du", "-sb", path)
 	output, err := cmd.Output()
 	if err != nil {
 		return 0
@@ -286,7 +289,9 @@ func getDirSizeBytes(path string) int64 {
 	fields := strings.Fields(string(output))
 	if len(fields) > 0 {
 		var size int64
-		fmt.Sscanf(fields[0], "%d", &size)
+		if n, _ := fmt.Sscanf(fields[0], "%d", &size); n != 1 {
+			return 0
+		}
 		return size
 	}
 	return 0

controller/internal/stacks/deploy.go

@@ -233,6 +233,10 @@ func (m *Manager) UpdateStackConfig(name string, values map[string]string) error
 		return fmt.Errorf("stack %q is not deployed yet", name)
 	}
 
+	if appCfg.Env == nil {
+		appCfg.Env = make(map[string]string)
+	}
+
 	lockedSet := make(map[string]bool)
 	for _, f := range appCfg.LockedFields {
 		lockedSet[f] = true

controller/internal/stacks/manager.go

@@ -628,10 +628,12 @@ func truncateStr(s string, maxLen int) string {
 // logPostStartStatus queries container states after a start/deploy operation
 // and logs them. This runs asynchronously to avoid blocking the HTTP response.
 func (m *Manager) logPostStartStatus(name, stackDir string, env []string) {
+	envCopy := make([]string, len(env))
+	copy(envCopy, env)
 	go func() {
 		time.Sleep(3 * time.Second)
 
-		output, err := m.composeExecCustomEnv(stackDir, env, "ps", "-a", "--format", "table {{.Name}}\t{{.Image}}\t{{.State}}\t{{.Status}}")
+		output, err := m.composeExecCustomEnv(stackDir, envCopy, "ps", "-a", "--format", "table {{.Name}}\t{{.Image}}\t{{.State}}\t{{.Status}}")
 		if err != nil {
 			m.logger.Printf("[WARN] Post-start status check failed for %s: %v", name, err)
 			return

controller/internal/web/funcmap.go

@@ -4,18 +4,34 @@ import (
 	"fmt"
 	"html/template"
 	"strings"
+	"sync"
 	"time"
 
 	"gitea.dooplex.hu/admin/felhom-controller/internal/backup"
 	"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
 )
 
-// templateFuncMap returns the FuncMap used by all HTML templates.
+var (
-func (s *Server) templateFuncMap() template.FuncMap {
+	webTimezone     *time.Location
+	webTimezoneOnce sync.Once
+)
+
+// getTimezone returns the Europe/Budapest timezone, cached after first load.
+// Falls back to UTC if tzdata is unavailable.
+func getTimezone() *time.Location {
+	webTimezoneOnce.Do(func() {
 		loc, err := time.LoadLocation("Europe/Budapest")
 		if err != nil {
 			loc = time.UTC
 		}
+		webTimezone = loc
+	})
+	return webTimezone
+}
+
+// templateFuncMap returns the FuncMap used by all HTML templates.
+func (s *Server) templateFuncMap() template.FuncMap {
+	loc := getTimezone()
 
 	return template.FuncMap{
 		"stateColor": func(state stacks.ContainerState) string {

controller/internal/web/handlers.go

@@ -447,12 +447,7 @@ func (s *Server) backupsHandler(w http.ResponseWriter, r *http.Request) {
 			}
 			if cfg.LastRun != "" {
 				if t, err := time.Parse(time.RFC3339, cfg.LastRun); err == nil {
-					// M7: Handle LoadLocation error — fall back to UTC if tzdata missing.
+					item.LastRunShort = t.In(getTimezone()).Format("01-02 15:04")
-					loc, err := time.LoadLocation("Europe/Budapest")
-					if err != nil {
-						loc = time.UTC
-					}
-					item.LastRunShort = t.In(loc).Format("01-02 15:04")
 				}
 			}
 			fullStatus.CrossDriveSummary = append(fullStatus.CrossDriveSummary, item)
@@ -543,11 +538,7 @@ func (s *Server) buildAppBackupRows(
 	crossConfigs map[string]*settings.CrossDriveBackup,
 	destLabels map[string]string,
 ) []AppBackupRow {
-	// M7: Handle LoadLocation error — fall back to UTC if tzdata missing.
+	loc := getTimezone()
-	loc, err := time.LoadLocation("Europe/Budapest")
-	if err != nil {
-		loc = time.UTC
-	}
 
 	// Build a quick lookup: which stacks have a DB dump?
 	dbStacks := make(map[string]bool)