From 69235cac5b3da5ff0dc20310e7474cabd8e63ba3 Mon Sep 17 00:00:00 2001
From: historia <[not public]>
Date: Tue, 15 Sep 2026 20:14:02 -0400
Subject: feat: route all html editor API mutations through single owner task

---
 internal/admin/server.go | 62 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 48 insertions(+), 14 deletions(-)

(limited to 'internal/admin/server.go')

diff --git a/internal/admin/server.go b/internal/admin/server.go
index 81f0cce..3dbe8ad 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -50,6 +50,10 @@ type AdminServer struct {
 	tmpl          *template.Template
 	cookieSecret  []byte
 	reloadCourses func()
+	// runOnOwner synchronously executes an API task, or returns false when
+	// the game is shutting down. All runtime callbacks below run INSIDE
+	// that task and must not dispatch to the owner again.
+	runOnOwner func(func()) bool
 	// rewriteRoomIDs migrates room-ID-embedded state across all characters
 	// (online + offline) when a room ID changes. Wired from the Game so the
 	// admin web GUI can trigger the same migration as the swapid command
@@ -69,7 +73,10 @@ type AdminServer struct {
 	rebuildCraftIndex func()
 }
 
-func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), evictRoom func(roomID int), rebuildCraftIndex func(), configPath string) (*AdminServer, error) {
+func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), evictRoom func(roomID int), rebuildCraftIndex func(), configPath string, runOnOwner func(func()) bool) (*AdminServer, error) {
+	if runOnOwner == nil {
+		return nil, fmt.Errorf("admin server requires an owner dispatcher")
+	}
 	secret := make([]byte, 32)
 	if _, err := rand.Read(secret); err != nil {
 		return nil, fmt.Errorf("cookie secret: %w", err)
@@ -117,6 +124,7 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
 		evictRoom:         evictRoom,
 		rebuildCraftIndex: rebuildCraftIndex,
 		configPath:        configPath,
+		runOnOwner:        runOnOwner,
 	}
 	// Undo/redo of room-ID changes applies the (inverse) runtime migration
 	// through the same wired callback the rename uses.
@@ -206,7 +214,7 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
 	apiMux.HandleFunc("/api/colors", s.handleColors)
 	apiMux.HandleFunc("/api/colors/reset", s.handleColorsReset)
 
-	mux.Handle("/api/", s.authMiddleware(apiMux))
+	mux.Handle("/api/", s.authMiddleware(s.ownerMiddleware(apiMux)))
 
 	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 		if !s.checkAuth(r) {
@@ -264,8 +272,12 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
 	})
 
 	s.httpServer = &http.Server{
-		Addr:    fmt.Sprintf(":%d", port),
-		Handler: mux,
+		Addr:              fmt.Sprintf(":%d", port),
+		Handler:           mux,
+		ReadHeaderTimeout: 5 * time.Second,
+		ReadTimeout:       30 * time.Second,
+		WriteTimeout:      60 * time.Second,
+		IdleTimeout:       60 * time.Second,
 	}
 	if useTLS {
 		s.httpServer.TLSConfig = &tls.Config{
@@ -277,6 +289,11 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
 	return s, nil
 }
 
+// ServeHTTP exposes the same authenticated routes used by ListenAndServe.
+func (s *AdminServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	s.httpServer.Handler.ServeHTTP(w, r)
+}
+
 func (s *AdminServer) ListenAndServe() error {
 	ln, err := net.Listen("tcp", s.httpServer.Addr)
 	if err != nil {
@@ -364,24 +381,41 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Transparent upgrade: legacy single-pass SHA-256 hashes rehash to
-	// Argon2id after a successful verification.
+	if !acc.Admin {
+		s.renderPage(w, r, "login.html", map[string]any{"Error": "Account is not authorized as admin."})
+		return
+	}
+
+	// Expensive password hashing stays off the owner. Reload the account
+	// inside the task so an upgrade cannot overwrite a concurrent character
+	// rename/create or a password/admin change made since authentication.
+	var upgradedHash string
 	if player.NeedsRehash(acc.PasswordHash) {
-		if hash, hashErr := player.HashPassword(password); hashErr == nil {
-			acc.PasswordHash = hash
-			if err := s.accountStore.SaveAccount(acc); err != nil {
+		upgradedHash, _ = player.HashPassword(password)
+	}
+	authorized := false
+	if !s.runOnOwner(func() {
+		current, err := s.accountStore.LoadAccount(account)
+		if err != nil || !current.Admin || current.PasswordHash != acc.PasswordHash {
+			return
+		}
+		authorized = true
+		if upgradedHash != "" {
+			current.PasswordHash = upgradedHash
+			if err := s.accountStore.SaveAccount(current); err != nil {
 				log.Printf("admin login: password rehash save for %s: %v", account, err)
 			}
 		}
+		s.undoStack.Clear()
+	}) {
+		writeJSONError(w, "game is shutting down", http.StatusServiceUnavailable)
+		return
 	}
-
-	if !acc.Admin {
-		s.renderPage(w, r, "login.html", map[string]any{"Error": "Account is not authorized as admin."})
+	if !authorized {
+		http.Error(w, "account changed; please sign in again", http.StatusUnauthorized)
 		return
 	}
-
 	s.setAuthCookie(w, account)
-	s.undoStack.Clear()
 	http.Redirect(w, r, "/", http.StatusFound)
 }
 
-- 
cgit v1.2.3