v0.76.0: campaign-#3 hardening (settings .bak recovery, restore stack_name validation, quiesce marker quarantine)
S1: corrupt settings.json recovers from .bak / safe-defaults+preserve, no crash-loop. F2: validStackName gates restore + export handlers (reject /,\,..,NUL traversal). S3: corrupt quiesce marker logged + quarantined, not silently dropped. Tests T-S1/F2/S3 + red-proofs. Agent/hub untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -319,7 +319,12 @@ func (l *Loop) readMarker() (Marker, bool) {
|
||||
return Marker{}, false
|
||||
}
|
||||
var m Marker
|
||||
if json.Unmarshal(data, &m) != nil {
|
||||
if err := json.Unmarshal(data, &m); err != nil {
|
||||
// S3: a corrupt marker is NOT silently dropped — log it LOUD and quarantine the bad file (a real
|
||||
// corrupted-mid-quiesce marker would otherwise skip stack-recovery with no trace). Still return
|
||||
// false: "no usable marker" ⇒ no recovery is the correct contract.
|
||||
l.logger.Printf("[WARN] [quiesce] marker at %s is corrupt (%v) — quarantining; stacks not auto-recovered from it", l.markerPath, err)
|
||||
_ = os.Rename(l.markerPath, fmt.Sprintf("%s.corrupt-%d", l.markerPath, l.now().Unix()))
|
||||
return Marker{}, false
|
||||
}
|
||||
return m, true
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
package quiesce
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// T-S3a: a corrupt marker is quarantined + logged (not silently dropped). Still returns false.
|
||||
// Companion: pre-fix readMarker (silent return false) → no *.corrupt-* + no log → this fails.
|
||||
func TestReadMarker_QuarantinesCorrupt(t *testing.T) {
|
||||
mp := filepath.Join(t.TempDir(), "quiesce-state.json")
|
||||
if err := os.WriteFile(mp, []byte(`{ "active": true, BROKEN not json`), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var buf bytes.Buffer
|
||||
l := New(Options{
|
||||
MarkerPath: mp, Poll: time.Hour, StatusPoll: time.Millisecond, MaxQuiesce: time.Second,
|
||||
Logger: log.New(&buf, "", 0),
|
||||
})
|
||||
if _, ok := l.readMarker(); ok {
|
||||
t.Fatal("corrupt marker must return ok=false")
|
||||
}
|
||||
if m, _ := filepath.Glob(mp + ".corrupt-*"); len(m) == 0 {
|
||||
t.Error("corrupt marker must be quarantined as *.corrupt-*")
|
||||
}
|
||||
if !strings.Contains(buf.String(), "corrupt") {
|
||||
t.Errorf("expected a WARN about the corrupt marker, got: %q", buf.String())
|
||||
}
|
||||
}
|
||||
|
||||
// T-S3b: valid marker → (marker, true), no quarantine (regression).
|
||||
func TestReadMarker_HappyPath(t *testing.T) {
|
||||
mp := filepath.Join(t.TempDir(), "quiesce-state.json")
|
||||
if err := os.WriteFile(mp, []byte(`{"active":true,"stopped_stacks":["romm","komga"]}`), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
l := New(Options{
|
||||
MarkerPath: mp, Poll: time.Hour, StatusPoll: time.Millisecond, MaxQuiesce: time.Second,
|
||||
Logger: log.New(&bytes.Buffer{}, "", 0),
|
||||
})
|
||||
m, ok := l.readMarker()
|
||||
if !ok {
|
||||
t.Fatal("valid marker must return ok=true")
|
||||
}
|
||||
if len(m.StoppedStacks) != 2 {
|
||||
t.Errorf("expected 2 stopped stacks, got %d", len(m.StoppedStacks))
|
||||
}
|
||||
if g, _ := filepath.Glob(mp + ".corrupt-*"); len(g) != 0 {
|
||||
t.Error("happy path must NOT quarantine")
|
||||
}
|
||||
}
|
||||
@@ -21,6 +21,10 @@ type Settings struct {
|
||||
log *log.Logger `json:"-"`
|
||||
debug bool `json:"-"`
|
||||
|
||||
// LoadWarning is set (non-empty, Hungarian) when Load recovered from a corrupt settings.json —
|
||||
// surfaced to the dashboard as a persistent banner. Not persisted.
|
||||
LoadWarning string `json:"-"`
|
||||
|
||||
// Auth
|
||||
PasswordHash string `json:"password_hash,omitempty"` // bcrypt hash, overrides controller.yaml
|
||||
|
||||
@@ -198,7 +202,24 @@ func Load(path string, logger *log.Logger) (*Settings, error) {
|
||||
}
|
||||
|
||||
if err := json.Unmarshal(data, s); err != nil {
|
||||
return nil, fmt.Errorf("parsing settings file: %w", err)
|
||||
// CORRUPT primary — never crash-loop. Recover from the last-known-good .bak; failing that,
|
||||
// preserve the corrupt file for forensics and start on safe defaults (recoverable: an empty
|
||||
// PasswordHash falls back to controller.yaml; the storage registry re-discovers on startup).
|
||||
logger.Printf("[ERROR] [settings] primary settings corrupt (%v) — attempting recovery from .bak", err)
|
||||
if bak, berr := os.ReadFile(path + ".bak"); berr == nil {
|
||||
s2 := &Settings{path: path, log: logger}
|
||||
if json.Unmarshal(bak, s2) == nil {
|
||||
logger.Printf("[WARN] [settings] recovered settings from .bak; re-promoting to primary")
|
||||
_ = os.WriteFile(path, bak, 0644) // best-effort promote
|
||||
s2.LoadWarning = "settings.json volt sérült — visszaállítva biztonsági másolatból"
|
||||
s2.migrateResticToRsync()
|
||||
return s2, nil
|
||||
}
|
||||
}
|
||||
corrupt := fmt.Sprintf("%s.corrupt-%d", path, time.Now().Unix())
|
||||
_ = os.Rename(path, corrupt)
|
||||
logger.Printf("[ERROR] [settings] settings unrecoverable — preserved as %s; starting with safe defaults", corrupt)
|
||||
return &Settings{path: path, log: logger, LoadWarning: "settings.json sérült és helyreállíthatatlan — alapértelmezett beállítások"}, nil
|
||||
}
|
||||
|
||||
logger.Printf("[INFO] [settings] Loaded settings from %s", path)
|
||||
@@ -266,6 +287,12 @@ func (s *Settings) save() error {
|
||||
return fmt.Errorf("renaming settings file: %w", err)
|
||||
}
|
||||
|
||||
// last-known-good: written AFTER the primary rename succeeds, so .bak only ever holds settings that
|
||||
// parsed + saved cleanly. Best-effort — a failed .bak must NOT fail the save.
|
||||
if err := os.WriteFile(s.path+".bak", data, 0644); err != nil && s.log != nil {
|
||||
s.log.Printf("[WARN] [settings] could not write .bak: %v", err)
|
||||
}
|
||||
|
||||
if s.debug {
|
||||
s.log.Printf("[DEBUG] [settings] saved to %s (%d bytes)", s.path, len(data))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
package settings
|
||||
|
||||
import (
|
||||
"io"
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func discardLog() *log.Logger { return log.New(io.Discard, "", 0) }
|
||||
|
||||
// T-S1a: a corrupt primary recovers from the last-known-good .bak (no error, no crash-loop).
|
||||
// Companion: pre-fix Load (return error on parse) → err != nil → this fails.
|
||||
func TestLoad_RecoversFromBak(t *testing.T) {
|
||||
p := filepath.Join(t.TempDir(), "settings.json")
|
||||
if err := os.WriteFile(p+".bak", []byte(`{"password_hash":"bak-hash"}`), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(p, []byte(`{"password_hash": BROKEN not json`), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
s, err := Load(p, discardLog())
|
||||
if err != nil {
|
||||
t.Fatalf("Load must not error (no crash-loop): %v", err)
|
||||
}
|
||||
if s.GetPasswordHash() != "bak-hash" {
|
||||
t.Fatalf("want recovered hash bak-hash, got %q", s.GetPasswordHash())
|
||||
}
|
||||
if s.LoadWarning == "" {
|
||||
t.Error("expected LoadWarning to be set after recovery")
|
||||
}
|
||||
if b, _ := os.ReadFile(p); !strings.Contains(string(b), "bak-hash") {
|
||||
t.Error("primary should be re-promoted from .bak")
|
||||
}
|
||||
}
|
||||
|
||||
// T-S1b: unrecoverable (corrupt primary, no .bak) → safe defaults, NO error, corrupt file preserved.
|
||||
func TestLoad_UnrecoverableNoBak(t *testing.T) {
|
||||
p := filepath.Join(t.TempDir(), "settings.json")
|
||||
if err := os.WriteFile(p, []byte(`{ BROKEN not json`), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
s, err := Load(p, discardLog())
|
||||
if err != nil {
|
||||
t.Fatalf("Load must not error (no crash-loop): %v", err)
|
||||
}
|
||||
if s == nil {
|
||||
t.Fatal("expected non-nil safe-default settings")
|
||||
}
|
||||
if s.LoadWarning == "" {
|
||||
t.Error("expected LoadWarning to be set")
|
||||
}
|
||||
if m, _ := filepath.Glob(p + ".corrupt-*"); len(m) == 0 {
|
||||
t.Error("corrupt file must be preserved as *.corrupt-*")
|
||||
}
|
||||
if _, e := os.Stat(p); e == nil {
|
||||
t.Error("original corrupt primary should have been renamed away")
|
||||
}
|
||||
}
|
||||
|
||||
// T-S1c: save() writes a .bak equal to the primary (last-known-good).
|
||||
func TestSave_WritesBak(t *testing.T) {
|
||||
p := filepath.Join(t.TempDir(), "settings.json")
|
||||
s, err := Load(p, discardLog()) // no file → defaults
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := s.SetPasswordHash("h1"); err != nil { // SetPasswordHash persists via save()
|
||||
t.Fatal(err)
|
||||
}
|
||||
prim, _ := os.ReadFile(p)
|
||||
bak, e := os.ReadFile(p + ".bak")
|
||||
if e != nil {
|
||||
t.Fatalf(".bak missing after save: %v", e)
|
||||
}
|
||||
if string(prim) != string(bak) {
|
||||
t.Error(".bak must equal the primary after save")
|
||||
}
|
||||
}
|
||||
|
||||
// T-S1d: happy path — valid primary loads normally, no warning (regression).
|
||||
func TestLoad_HappyPath(t *testing.T) {
|
||||
p := filepath.Join(t.TempDir(), "settings.json")
|
||||
if err := os.WriteFile(p, []byte(`{"password_hash":"good"}`), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
s, err := Load(p, discardLog())
|
||||
if err != nil || s.GetPasswordHash() != "good" || s.LoadWarning != "" {
|
||||
t.Fatalf("happy load failed: err=%v hash=%q warn=%q", err, s.GetPasswordHash(), s.LoadWarning)
|
||||
}
|
||||
}
|
||||
@@ -166,6 +166,11 @@ func (s *Server) apiExportStart(w http.ResponseWriter, r *http.Request) {
|
||||
jsonError(w, "Missing stack_name or dest_drive", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// F2 (defense-in-depth): reject a traversal/escape stack_name before any export work.
|
||||
if !validStackName(req.StackName) {
|
||||
jsonError(w, "Invalid stack_name", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
if !s.isValidDrivePath(req.DestDrive) {
|
||||
s.logger.Printf("[DEBUG] [web] apiExportStart: invalid drive path %q", req.DestDrive)
|
||||
|
||||
@@ -151,6 +151,7 @@ func (s *Server) dashboardHandler(w http.ResponseWriter, r *http.Request) {
|
||||
sysInfo := system.GetInfo(s.primaryHDDPath(), s.cpuCollector)
|
||||
|
||||
data := s.baseData("dashboard", "Vezérlőpult")
|
||||
data["SettingsWarning"] = s.settings.LoadWarning // non-empty if settings.json was recovered from corruption
|
||||
data["Stacks"] = deployedStacks
|
||||
data["MissingStorage"] = s.missingStorageMap(deployedStacks)
|
||||
data["RunningCount"] = running
|
||||
@@ -795,6 +796,13 @@ func (s *Server) backupRestoreHandler(w http.ResponseWriter, r *http.Request) {
|
||||
http.Redirect(w, r, "/backups?flash_error=Hi%C3%A1nyz%C3%B3+param%C3%A9terek", http.StatusFound)
|
||||
return
|
||||
}
|
||||
// F2 (defense-in-depth): a stack name is a single segment, never a path. Reject traversal before any
|
||||
// restore work — never let it reach RestoreFromRecoveryUnit.
|
||||
if !validStackName(stackName) {
|
||||
s.logger.Printf("[WARN] [web] restore rejected: invalid stack_name %q from %s", stackName, r.RemoteAddr)
|
||||
http.Redirect(w, r, "/backups?flash_error=%C3%89rv%C3%A9nytelen+alkalmaz%C3%A1sn%C3%A9v", http.StatusFound)
|
||||
return
|
||||
}
|
||||
|
||||
if s.backupMgr == nil {
|
||||
http.Redirect(w, r, "/backups?flash_error=Ment%C3%A9s+nincs+be%C3%A1ll%C3%ADtva", http.StatusFound)
|
||||
|
||||
@@ -87,6 +87,14 @@
|
||||
{{end}}
|
||||
{{end}}
|
||||
</div>
|
||||
{{if .SettingsWarning}}
|
||||
<div class="inline-warnings">
|
||||
<div class="inline-warning inline-warning-error">
|
||||
<span class="inline-warning-dot">●</span>
|
||||
<span class="inline-warning-text">{{.SettingsWarning}}</span>
|
||||
</div>
|
||||
</div>
|
||||
{{end}}
|
||||
{{if .DiskWarnings}}
|
||||
<div class="inline-warnings">
|
||||
{{range .DiskWarnings}}
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
package web
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// validStackName reports whether name is a safe stack identifier: a single path segment, never a path.
|
||||
// Rejects traversal/escape (`..`, `/`, `\`, NUL) so a stack_name can never be turned into a filesystem
|
||||
// path that escapes the stacks/userdata tree. (Storage `where=` is validated separately by gateWhere.)
|
||||
func validStackName(name string) bool {
|
||||
if name == "" || name == "." || name == ".." {
|
||||
return false
|
||||
}
|
||||
if strings.ContainsAny(name, "/\\\x00") {
|
||||
return false
|
||||
}
|
||||
return name == filepath.Clean(name)
|
||||
}
|
||||
@@ -0,0 +1,61 @@
|
||||
package web
|
||||
|
||||
import (
|
||||
"io"
|
||||
"log"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-controller/internal/appexport"
|
||||
"gitea.dooplex.hu/admin/felhom-controller/internal/config"
|
||||
)
|
||||
|
||||
// T-F2a: validStackName rejects traversal/escape, accepts real single-segment names. (pure)
|
||||
func TestValidStackName(t *testing.T) {
|
||||
for _, bad := range []string{"../../etc", "a/b", "..", ".", "", "a\x00b", "a\\b", "../x", "/etc"} {
|
||||
if validStackName(bad) {
|
||||
t.Errorf("validStackName(%q) = true, want false", bad)
|
||||
}
|
||||
}
|
||||
for _, good := range []string{"paperless-ngx", "immich", "romm", "uptime-kuma"} {
|
||||
if !validStackName(good) {
|
||||
t.Errorf("validStackName(%q) = false, want true", good)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// T-F2b: the restore handler rejects a traversal stack_name with the flash-error redirect and never
|
||||
// reaches RestoreFromRecoveryUnit (backupMgr is nil — if the gate failed to fire, it would nil-panic
|
||||
// or fall through to the "Mentés nincs beállítva" path, not the "Érvénytelen" one).
|
||||
// Companion: drop the gate → traversal falls through to the nil-backupMgr path → different redirect → fails.
|
||||
func TestBackupRestoreHandler_RejectsTraversal(t *testing.T) {
|
||||
s := &Server{cfg: &config.Config{}, logger: log.New(io.Discard, "", 0)} // backupMgr == nil
|
||||
form := "stack_name=../../../etc&snapshot_id=x"
|
||||
req := httptest.NewRequest("POST", "/backup/restore", strings.NewReader(form))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
s.backupRestoreHandler(rec, req) // must not panic
|
||||
|
||||
loc := rec.Header().Get("Location")
|
||||
if !strings.Contains(loc, "rv%C3%A9nytelen") { // "Érvénytelen" (invalid name) — the gate's redirect
|
||||
t.Fatalf("expected traversal rejection redirect, got Location=%q", loc)
|
||||
}
|
||||
}
|
||||
|
||||
// T-F2c: the export handler rejects a traversal stack_name with 400 (before any export work).
|
||||
func TestExportStart_RejectsTraversal(t *testing.T) {
|
||||
// non-nil exporter so we pass the "not available" guard and reach the stack_name gate (the gate
|
||||
// returns before the exporter is ever used).
|
||||
s := &Server{cfg: &config.Config{}, logger: log.New(io.Discard, "", 0), appExporter: &appexport.Exporter{}}
|
||||
body := `{"stack_name":"../../etc","dest_drive":"/mnt/x"}`
|
||||
req := httptest.NewRequest("POST", "/api/export/start", strings.NewReader(body))
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
s.apiExportStart(rec, req) // must not panic
|
||||
|
||||
if rec.Code != 400 {
|
||||
t.Fatalf("expected 400 for traversal stack_name, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user