From 1040cfe22592e675afc45921835f62d1cc6509de Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 8 Oct 2026 14:32:20 +0200 Subject: [PATCH] R-35 (D4): dashboard sign-ins survive the controller's own restart; disk holds only a fingerprint Sessions are keyed by sha256(cookie) and persisted to dashboard-sessions.json (0600, tmp+fsync+rename) in the data dir: fingerprint, expiry, CSRF token. Loaded in NewServer; expired rows dropped at load and save. Logout and invalidateAllSessions (password change, claim reset) write the file at once. Corrupt/unreadable file = start with no sessions (never fatal). Red-proof: with load/save as no-ops the restart test fails ('the old cookie no longer signs in'); with the raw token as the key the file test fails ('the sessions file holds the cookie value'). Also: TestR650_NoBareDockerExec skips a non-.go file that vanished mid-walk (a parallel stacks test's update-journal.json.tmp raced it in a full run). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- .../internal/dockerexec/dockerexec_test.go | 6 + controller/internal/web/auth.go | 15 +- controller/internal/web/server.go | 1 + controller/internal/web/session_store.go | 147 +++++++++++++ controller/internal/web/session_store_test.go | 197 ++++++++++++++++++ 5 files changed, 362 insertions(+), 4 deletions(-) create mode 100644 controller/internal/web/session_store.go create mode 100644 controller/internal/web/session_store_test.go diff --git a/controller/internal/dockerexec/dockerexec_test.go b/controller/internal/dockerexec/dockerexec_test.go index 95f30f4..dd35e53 100644 --- a/controller/internal/dockerexec/dockerexec_test.go +++ b/controller/internal/dockerexec/dockerexec_test.go @@ -75,6 +75,12 @@ func TestR650_NoBareDockerExec(t *testing.T) { n := 0 err := filepath.Walk(root, func(p string, info os.FileInfo, err error) error { if err != nil { + // Another package's test, running in parallel, may create and rename a scratch file (for example + // internal/stacks/update-journal.json.tmp) between the directory read and the lstat. A file that vanished + // is not a Go source file; skip it. Seen 2026-10-08 in a full `go test ./...`. + if os.IsNotExist(err) && !strings.HasSuffix(p, ".go") { + return nil + } return err } if info.IsDir() || !strings.HasSuffix(p, ".go") || strings.HasSuffix(p, "_test.go") { diff --git a/controller/internal/web/auth.go b/controller/internal/web/auth.go index 58e4ca6..980c8e6 100644 --- a/controller/internal/web/auth.go +++ b/controller/internal/web/auth.go @@ -239,7 +239,8 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { } if cookie, err := r.Cookie(sessionCookieName); err == nil { s.sessionsMu.Lock() - delete(s.sessions, cookie.Value) + delete(s.sessions, sessionFingerprint(cookie.Value)) + _ = s.saveSessionsLocked() // R-35: a signed-out cookie stays signed out across a restart s.sessionsMu.Unlock() } s.logger.Printf("[INFO] [web] User logged out from %s", r.RemoteAddr) @@ -257,10 +258,12 @@ func (s *Server) createSession() string { csrfToken := hex.EncodeToString(csrfB) s.sessionsMu.Lock() - s.sessions[token] = &session{ + // R-35: keyed by the fingerprint, never the cookie value — this map is what session_store.go writes to disk. + s.sessions[sessionFingerprint(token)] = &session{ expiresAt: time.Now().Add(sessionMaxAge), csrfToken: csrfToken, } + _ = s.saveSessionsLocked() sessionCount := len(s.sessions) s.sessionsMu.Unlock() @@ -276,7 +279,7 @@ func (s *Server) createSession() string { func (s *Server) csrfTokenForSession(sessionToken string) string { s.sessionsMu.RLock() defer s.sessionsMu.RUnlock() - sess, ok := s.sessions[sessionToken] + sess, ok := s.sessions[sessionFingerprint(sessionToken)] if !ok || time.Now().After(sess.expiresAt) { return "" } @@ -286,7 +289,7 @@ func (s *Server) csrfTokenForSession(sessionToken string) string { func (s *Server) isValidSession(token string) bool { s.sessionsMu.RLock() defer s.sessionsMu.RUnlock() - sess, ok := s.sessions[token] + sess, ok := s.sessions[sessionFingerprint(token)] return ok && time.Now().Before(sess.expiresAt) } @@ -296,6 +299,7 @@ func (s *Server) invalidateAllSessions() { s.sessionsMu.Lock() count := len(s.sessions) s.sessions = make(map[string]*session) + _ = s.saveSessionsLocked() // R-35: the password change ends every session on disk too s.sessionsMu.Unlock() s.logger.Printf("[INFO] [web] All sessions invalidated (cleared %d)", count) } @@ -318,6 +322,9 @@ func (s *Server) cleanupSessions() { } } remaining := len(s.sessions) + if expired > 0 { + _ = s.saveSessionsLocked() + } s.sessionsMu.Unlock() if expired > 0 { s.logger.Printf("[INFO] [web] Cleaned up %d expired sessions, %d remaining", expired, remaining) diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go index c270b18..adfa631 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -282,6 +282,7 @@ func NewServer(cfg *config.Config, stackMgr *stacks.Manager, cpuCollector *syste } s.loadTemplates() + s.loadSessions() // R-35: dashboard sign-ins survive the controller's own restart (session_store.go) go s.cleanupSessions() // .fab download staging (v0.124.0): sweep aged bundles left by a crash/abandoned download. if cfg.Paths.DataDir != "" { diff --git a/controller/internal/web/session_store.go b/controller/internal/web/session_store.go new file mode 100644 index 0000000..7bf6ec4 --- /dev/null +++ b/controller/internal/web/session_store.go @@ -0,0 +1,147 @@ +package web + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "os" + "path/filepath" + "sort" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/logx" +) + +// R-35 (D4, operator ruling 2026-10-08, `09` §3 decision 188): the dashboard's sign-ins survive the controller's own +// restarts — a settings push, an update, a crash. Before this, s.sessions lived only in memory and every restart +// signed the household out mid-flow. +// +// The disk holds a FINGERPRINT, never the cookie: the map is keyed by sha256(cookie), and only that key, the expiry +// and the CSRF token are written. The data dir rides in the whole-guest archive (plaintext by design, `07` §5), so a +// copy of the archive must not hold anything a browser could present. sha256 of 32 random bytes cannot be turned back +// into the cookie, and presenting the fingerprint itself is hashed again and misses (TestR35_FileHoldsNoToken). +// +// Expiry is unchanged (sessionMaxAge from creation; an expired row is dropped at load and at every save). Logout and +// invalidateAllSessions (password change, claim reset) write the file at once, so an ended session stays ended +// across a restart (TestR35_LogoutEndsSessionAcrossRestart, TestR35_InvalidateAllEndsSessionAcrossRestart). +// +// A missing file is normal; an unreadable or corrupt one is logged and the server starts with no sessions — never +// fatal, and the failure direction is "sign in again", never "signed in" (TestR35_CorruptFileStartsEmpty). + +const sessionsFileName = "dashboard-sessions.json" + +type sessionsFile struct { + Version int `json:"version"` + Sessions []sessionDisk `json:"sessions"` +} + +type sessionDisk struct { + Fingerprint string `json:"fingerprint"` + ExpiresAt time.Time `json:"expires_at"` + CSRFToken string `json:"csrf_token"` +} + +// sessionFingerprint is the map key and the on-disk id of a session: hex(sha256(cookie value)). +func sessionFingerprint(token string) string { + sum := sha256.Sum256([]byte(token)) + return hex.EncodeToString(sum[:]) +} + +func (s *Server) sessionsPath() string { + if s.cfg == nil || s.cfg.Paths.DataDir == "" { + return "" + } + return filepath.Join(s.cfg.Paths.DataDir, sessionsFileName) +} + +// loadSessions reads the persisted sessions into s.sessions. Called once from NewServer. +func (s *Server) loadSessions() { + path := s.sessionsPath() + if path == "" { + return + } + b, err := os.ReadFile(path) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + logx.Warnf(s.logger, "[web] sessions: cannot read %s, starting with none: %v", path, err) + } else { + logx.Debugf(s.logger, "[web] sessions: no %s yet, starting with none", sessionsFileName) + } + return + } + var f sessionsFile + if err := json.Unmarshal(b, &f); err != nil { + logx.Warnf(s.logger, "[web] sessions: %s is not valid JSON, starting with none: %v", path, err) + return + } + now := time.Now() + loaded, expired, bad := 0, 0, 0 + s.sessionsMu.Lock() + for _, d := range f.Sessions { + if len(d.Fingerprint) != sha256.Size*2 || d.CSRFToken == "" { + bad++ + continue + } + if !now.Before(d.ExpiresAt) { + expired++ + continue + } + s.sessions[d.Fingerprint] = &session{expiresAt: d.ExpiresAt, csrfToken: d.CSRFToken} + loaded++ + } + s.sessionsMu.Unlock() + logx.Infof(s.logger, "[web] sessions: restored %d dashboard session(s) from disk (dropped %d expired, %d malformed)", loaded, expired, bad) +} + +// saveSessionsLocked writes the live sessions (expired ones dropped) atomically, 0600, fsynced. The caller holds +// sessionsMu for writing. A failure is logged and returned; the in-memory state stays authoritative for this process. +func (s *Server) saveSessionsLocked() error { + path := s.sessionsPath() + if path == "" { + return nil + } + now := time.Now() + f := sessionsFile{Version: 1, Sessions: []sessionDisk{}} + for fp, sess := range s.sessions { + if !now.Before(sess.expiresAt) { + continue + } + f.Sessions = append(f.Sessions, sessionDisk{Fingerprint: fp, ExpiresAt: sess.expiresAt, CSRFToken: sess.csrfToken}) + } + sort.Slice(f.Sessions, func(i, j int) bool { return f.Sessions[i].Fingerprint < f.Sessions[j].Fingerprint }) + b, err := json.MarshalIndent(f, "", " ") + if err != nil { + return err + } + if err := writeSessionsAtomic(path, b); err != nil { + logx.Warnf(s.logger, "[web] sessions: cannot save %s (sessions stay valid until this process ends): %v", path, err) + return err + } + logx.Debugf(s.logger, "[web] sessions: saved %d session(s) to %s", len(f.Sessions), sessionsFileName) + return nil +} + +// writeSessionsAtomic is tmp + fsync + rename at 0600 — the family.json shape (internal/family saveLocked). +func writeSessionsAtomic(path string, b []byte) error { + tmp := path + ".tmp" + fh, err := os.OpenFile(tmp, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600) + if err != nil { + return err + } + if _, err := fh.Write(b); err != nil { + fh.Close() + os.Remove(tmp) + return err + } + if err := fh.Sync(); err != nil { + fh.Close() + os.Remove(tmp) + return err + } + if err := fh.Close(); err != nil { + os.Remove(tmp) + return err + } + return os.Rename(tmp, path) +} diff --git a/controller/internal/web/session_store_test.go b/controller/internal/web/session_store_test.go new file mode 100644 index 0000000..07115cd --- /dev/null +++ b/controller/internal/web/session_store_test.go @@ -0,0 +1,197 @@ +package web + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "io" + "log" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-35 (D4, operator ruling 2026-10-08): a dashboard sign-in survives the controller's own restart, and the disk holds +// only a fingerprint of each session. Every test builds its servers with NewServer — the call cmd/controller/main.go +// makes — so the load is pinned on the production constructor, not on a helper a test calls by hand. +// +// RED-PROOF (seen 2026-10-08): with loadSessions and saveSessionsLocked reduced to no-ops (today's in-memory-only map), +// TestR35_RestartKeepsSession and TestR35_FileHoldsNoToken fail ("restart: the old cookie no longer signs in"; +// "no sessions file written"), and the invalidate/logout/expired cases pass vacuously — which is why the restart +// case is the positive control for each of them: they assert the SAME cookie worked before they assert it stopped. + +func newR35Server(t *testing.T, dir string) *Server { + t.Helper() + lg := log.New(io.Discard, "", 0) + sett, err := settings.Load(filepath.Join(dir, "settings.json"), lg) + if err != nil { + t.Fatalf("settings: %v", err) + } + cfg := &config.Config{} + cfg.Paths.DataDir = dir + s := NewServer(cfg, nil, nil, nil, nil, sett, nil, nil, nil, lg, "test") + t.Cleanup(s.Close) + return s +} + +func TestR35_RestartKeepsSession(t *testing.T) { + dir := t.TempDir() + s1 := newR35Server(t, dir) + tok := s1.createSession() + csrf := s1.csrfTokenForSession(tok) + if !s1.isValidSession(tok) || csrf == "" { + t.Fatal("setup: a fresh session must be valid on the server that made it") + } + s1.Close() + + s2 := newR35Server(t, dir) // the restart + if !s2.isValidSession(tok) { + t.Fatal("restart: the old cookie no longer signs in") + } + if got := s2.csrfTokenForSession(tok); got != csrf { + t.Fatalf("restart: CSRF token changed (%d chars, want the same %d) — every open page's forms would 403", len(got), len(csrf)) + } + if s2.isValidSession(tok + "0") { + t.Fatal("a cookie that was never issued must not sign in") + } +} + +func TestR35_FileHoldsNoToken(t *testing.T) { + dir := t.TempDir() + s := newR35Server(t, dir) + tok := s.createSession() + b, err := os.ReadFile(filepath.Join(dir, sessionsFileName)) + if err != nil { + t.Fatalf("no sessions file written: %v", err) + } + if bytes.Contains(b, []byte(tok)) { + t.Fatal("the sessions file holds the cookie value — a stolen archive would sign in") + } + sum := sha256.Sum256([]byte(tok)) + if !bytes.Contains(b, []byte(hex.EncodeToString(sum[:]))) { + t.Fatal("the sessions file does not hold the session's fingerprint (positive control)") + } + fi, err := os.Stat(filepath.Join(dir, sessionsFileName)) + if err != nil { + t.Fatal(err) + } + if fi.Mode().Perm() != 0o600 { + t.Fatalf("sessions file mode %o, want 600", fi.Mode().Perm()) + } + + // The file ALONE cannot sign in: presenting the fingerprint itself as the cookie is rejected on a restarted server. + s.Close() + s2 := newR35Server(t, dir) + if s2.isValidSession(hex.EncodeToString(sum[:])) { + t.Fatal("the fingerprint read from disk signs in as a cookie") + } + if !s2.isValidSession(tok) { + t.Fatal("positive control: the real cookie must still sign in after the restart") + } +} + +func TestR35_InvalidateAllEndsSessionAcrossRestart(t *testing.T) { + dir := t.TempDir() + s1 := newR35Server(t, dir) + tok := s1.createSession() + s1.Close() + s2 := newR35Server(t, dir) + if !s2.isValidSession(tok) { + t.Fatal("setup: the session must survive one restart first") + } + s2.invalidateAllSessions() // the password change and the claim reset call this + s2.Close() + s3 := newR35Server(t, dir) + if s3.isValidSession(tok) { + t.Fatal("after a password change the old cookie signs in again after a restart") + } +} + +func TestR35_LogoutEndsSessionAcrossRestart(t *testing.T) { + dir := t.TempDir() + s1 := newR35Server(t, dir) + tok := s1.createSession() + keep := s1.createSession() + s1.Close() + s2 := newR35Server(t, dir) + if !s2.isValidSession(tok) { + t.Fatal("setup: the session must survive one restart first") + } + req := httptest.NewRequest(http.MethodPost, "/logout", nil) + req.AddCookie(&http.Cookie{Name: sessionCookieName, Value: tok}) + s2.handleLogout(httptest.NewRecorder(), req) + if s2.isValidSession(tok) { + t.Fatal("logout: the session is still valid in the same process") + } + s2.Close() + s3 := newR35Server(t, dir) + if s3.isValidSession(tok) { + t.Fatal("logout: the signed-out cookie signs in again after a restart") + } + if !s3.isValidSession(keep) { + t.Fatal("logout of one browser ended another browser's session") + } +} + +func TestR35_ExpiredRowNotLoaded(t *testing.T) { + dir := t.TempDir() + s1 := newR35Server(t, dir) + live := s1.createSession() + old := s1.createSession() + // Age one session past its expiry, as 7 days would — through the map the server owns, then persisted. + s1.sessionsMu.Lock() + s1.sessions[sessionFingerprint(old)].expiresAt = time.Now().Add(-time.Minute) + _ = s1.saveSessionsLocked() + s1.sessionsMu.Unlock() + s1.Close() + + var f sessionsFile + b, _ := os.ReadFile(filepath.Join(dir, sessionsFileName)) + if err := json.Unmarshal(b, &f); err != nil { + t.Fatalf("sessions file: %v", err) + } + if len(f.Sessions) != 1 { + t.Fatalf("an expired row was written to disk: %d rows, want 1", len(f.Sessions)) + } + + s2 := newR35Server(t, dir) + if s2.isValidSession(old) { + t.Fatal("an expired session signs in after a restart") + } + if !s2.isValidSession(live) { + t.Fatal("positive control: the live session must survive") + } + s2.sessionsMu.RLock() + n := len(s2.sessions) + s2.sessionsMu.RUnlock() + if n != 1 { + t.Fatalf("loaded %d sessions, want 1", n) + } +} + +// A corrupt or foreign file never stops the controller and never signs anyone in. +func TestR35_CorruptFileStartsEmpty(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, sessionsFileName), []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + s := newR35Server(t, dir) + s.sessionsMu.RLock() + n := len(s.sessions) + s.sessionsMu.RUnlock() + if n != 0 { + t.Fatalf("a corrupt file loaded %d sessions", n) + } + tok := s.createSession() // and the next save repairs the file + s.Close() + if !newR35Server(t, dir).isValidSession(tok) { + t.Fatal("after a corrupt file, a new session must persist again") + } +}