aboutsummaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
Diffstat (limited to 'internal')
-rw-r--r--internal/admin/api_rooms.go3
-rw-r--r--internal/admin/live_owner_test.go227
-rw-r--r--internal/admin/owner.go82
-rw-r--r--internal/admin/owner_routes_test.go68
-rw-r--r--internal/admin/owner_test.go152
-rw-r--r--internal/admin/server.go62
-rw-r--r--internal/admin/undo.go4
-rw-r--r--internal/game/admin_owner_test.go27
-rw-r--r--internal/game/exit_migrate.go14
-rw-r--r--internal/game/owner.go31
10 files changed, 650 insertions, 20 deletions
diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go
index c93e7c6..3c96d53 100644
--- a/internal/admin/api_rooms.go
+++ b/internal/admin/api_rooms.go
@@ -850,7 +850,8 @@ func (s *AdminServer) handleRoomMove(w http.ResponseWriter, r *http.Request) {
// 2. The file changes apply as a single journal transaction, rolling back
// on any write failure.
// 3. The wired runtime migration (players, Hub buckets, casino room keys,
-// world room-keyed state, mob instances) runs through one owner task.
+// world room-keyed state, mob instances) runs in the SAME owner task
+// as preflight and the file commit (the API middleware owns dispatch).
// 4. The undo record captures every before/after image and the idMap, so
// undo/redo restore the whole plan, not just the renamed file.
func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) {
diff --git a/internal/admin/live_owner_test.go b/internal/admin/live_owner_test.go
new file mode 100644
index 0000000..7e94a1e
--- /dev/null
+++ b/internal/admin/live_owner_test.go
@@ -0,0 +1,227 @@
+package admin
+
+import (
+ "fmt"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "os"
+ "path/filepath"
+ "strings"
+ "sync"
+ "sync/atomic"
+ "testing"
+ "time"
+
+ "thehouseoficarus/internal/casino"
+ "thehouseoficarus/internal/config"
+ "thehouseoficarus/internal/game"
+ gamenet "thehouseoficarus/internal/net"
+ "thehouseoficarus/internal/player"
+)
+
+// Use the real authenticated HTTP router, not direct handler calls: production
+// must commit files, migrate runtime state AND rebuild caches in one owner task.
+func TestAdminEditorAtomicWithGameplay(t *testing.T) {
+ h, sess := newEditorHarness(t)
+ h.g.RunOnOwner(func() {
+ sess.Player.RoomID = 2
+ h.hub.EnterRoom(sess, 2)
+ h.g.AccountStore.SaveCharacter(sess.Player)
+ h.g.Casino.JoinMachine(2, casino.MachineConfig{ID: "slots", Game: casino.GameSlots, MinBet: 1}, "alice")
+ })
+
+ // Pause after the handler has committed files but before runtime migration.
+ // A tick/input/observer queued here must not see the torn intermediate state.
+ midMutation := make(chan struct{}, 1)
+ releaseMigration := make(chan struct{})
+ var releaseOnce sync.Once
+ release := func() { releaseOnce.Do(func() { close(releaseMigration) }) }
+ defer release()
+ var pause atomic.Bool
+ migrate := func(ids map[int]int) {
+ if pause.Load() {
+ midMutation <- struct{}{}
+ <-releaseMigration
+ }
+ h.g.MigrateRoomIDsOnOwner(ids)
+ }
+ s, err := NewServer(config.Default(), false, h.g.AccountStore, h.g.World, h.g.ItemStore, h.g.ObjectStore, h.g.MobStore, h.dir,
+ h.g.CourseStore.ReloadAll, migrate, h.g.EvictCasinoRoom, h.g.RebuildCraftIndex, filepath.Join(h.dir, "config.yaml"), h.g.RunAdminOnOwner)
+ if err != nil {
+ t.Fatal(err)
+ }
+ form := url.Values{"account": {"alice"}, "password": {"pw"}}
+ login := httptest.NewRequest("POST", "/login", strings.NewReader(form.Encode()))
+ login.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ loginRec := httptest.NewRecorder()
+ s.ServeHTTP(loginRec, login)
+ cookies := loginRec.Result().Cookies()
+ if loginRec.Code != http.StatusFound || len(cookies) == 0 {
+ t.Fatalf("login: %d %s", loginRec.Code, loginRec.Body.String())
+ }
+ request := func(path, body string) error {
+ r := httptest.NewRequest("POST", path, strings.NewReader(body))
+ r.AddCookie(cookies[0])
+ rec := httptest.NewRecorder()
+ s.ServeHTTP(rec, r)
+ if rec.Code != http.StatusOK || strings.Contains(rec.Body.String(), `"error"`) {
+ return fmt.Errorf("%s: %d %s", path, rec.Code, rec.Body.String())
+ }
+ return nil
+ }
+
+ var observations atomic.Int64
+ // Called ONLY on the owner. Both tick and session observers assert the
+ // disk/index/player/Hub/casino snapshot agrees, not merely that it is race-free.
+ check := func() {
+ observations.Add(1)
+ id := sess.Player.RoomID
+ path, ok := h.g.World.GetRoomPath(id)
+ if !ok {
+ t.Errorf("player in unindexed room %d", id)
+ return
+ }
+ if _, err := os.Stat(path); err != nil {
+ t.Errorf("player room %d missing on disk: %v", id, err)
+ }
+ if room, err := h.g.World.LoadRoom(id); err != nil || room.ID != id {
+ t.Errorf("room file/index disagrees with player %d: %v", id, err)
+ }
+ if players := h.hub.PlayersInRoom(id); len(players) != 1 || players[0] != sess {
+ t.Errorf("Hub disagrees with player room %d", id)
+ }
+ if machine, ok := h.g.Casino.MachineFor("alice"); !ok || machine.RoomID != id {
+ t.Errorf("casino disagrees with player room %d", id)
+ }
+ if saved, err := h.g.AccountStore.LoadCharacter("alice"); err != nil || saved.RoomID != id {
+ t.Errorf("saved character disagrees with player room %d: %v", id, err)
+ }
+ }
+ tickID := h.g.SubscribeTicks(1, func() bool {
+ h.g.RunTickSystems()
+ check()
+ return true
+ })
+ defer h.g.Ticks.Unsubscribe(tickID)
+
+ pause.Store(true)
+ requestDone := make(chan error, 1)
+ go func() { requestDone <- request("/api/rooms/rename", `{"id":2,"new_id":4}`) }()
+ select {
+ case <-midMutation:
+ case <-time.After(5 * time.Second):
+ t.Fatal("rename did not reach migration (nested owner dispatch?)")
+ }
+ observed := make(chan struct{})
+ go func() {
+ h.g.RunOnOwner(func() {
+ h.g.HandleSession(sess, "look")
+ check()
+ close(observed)
+ })
+ }()
+ select {
+ case <-observed:
+ t.Error("gameplay ran between file commit and runtime migration")
+ case <-time.After(20 * time.Millisecond):
+ }
+ pause.Store(false)
+ release()
+ select {
+ case err := <-requestDone:
+ if err != nil {
+ t.Fatal(err)
+ }
+ case <-time.After(5 * time.Second):
+ t.Fatal("rename deadlocked")
+ }
+ <-observed
+
+ // Repeat real rename/undo/redo while input and ticks compete for ownership.
+ inputDone := make(chan struct{})
+ go func() {
+ defer close(inputDone)
+ for i := 0; i < 100; i++ {
+ h.g.HandleSessionOnOwner(sess, "look")
+ h.g.RunOnOwner(check)
+ }
+ }()
+ for i := 0; i < 5; i++ {
+ for _, path := range []string{"/api/undo/undo", "/api/undo/redo"} {
+ if err := request(path, ""); err != nil {
+ t.Error(err)
+ }
+ }
+ }
+ <-inputDone
+ h.g.RunOnOwner(check)
+ if observations.Load() < 100 {
+ t.Fatalf("too few gameplay observations: %d", observations.Load())
+ }
+}
+
+type editorHarness struct {
+ g *game.Game
+ hub *gamenet.Hub
+ dir string
+}
+
+type discardConn struct{}
+
+func (discardConn) ReadMessage() (string, error) { return "", io.EOF }
+func (discardConn) Write(p []byte) (int, error) { return len(p), nil }
+func (discardConn) Close() error { return nil }
+func (discardConn) SetWriteDeadline(time.Time) error { return nil }
+func (discardConn) SetEcho(bool) error { return nil }
+
+func newEditorHarness(t *testing.T) (*editorHarness, *gamenet.Session) {
+ t.Helper()
+ dir := t.TempDir()
+ for _, subdir := range []string{"rooms", "players/accounts", "players/characters"} {
+ if err := os.MkdirAll(filepath.Join(dir, subdir), 0755); err != nil {
+ t.Fatal(err)
+ }
+ }
+ for id, contents := range map[int]string{
+ 1: "id: 1\nname: Central\nexits:\n north:\n room: 2\n",
+ 2: "id: 2\nname: North\nexits:\n south:\n room: 1\n",
+ } {
+ if err := os.WriteFile(filepath.Join(dir, "rooms", fmt.Sprintf("%d.yaml", id)), []byte(contents), 0644); err != nil {
+ t.Fatal(err)
+ }
+ }
+ colors := &config.ColorsConfig{}
+ g := game.New(dir, colors, colors, config.ValidationConfig{}, 1)
+ hub := gamenet.NewHub()
+ g.SetHub(hub)
+ stopped := make(chan struct{})
+ g.SetTransportStop(func() { close(stopped) })
+ t.Cleanup(func() {
+ g.RequestShutdown("test complete")
+ select {
+ case <-stopped:
+ case <-time.After(5 * time.Second):
+ t.Error("game shutdown timed out")
+ }
+ })
+ hash, err := player.HashPassword("pw")
+ if err != nil {
+ t.Fatal(err)
+ }
+ acc := &player.Account{Name: "alice", PasswordHash: hash, Admin: true, Characters: []string{"alice"}}
+ p := player.New("alice")
+ p.Account, p.RoomID = "alice", 1
+ if err := g.AccountStore.SaveAccount(acc); err != nil {
+ t.Fatal(err)
+ }
+ if err := g.AccountStore.SaveCharacter(p); err != nil {
+ t.Fatal(err)
+ }
+ sess := &gamenet.Session{Conn: discardConn{}, Account: acc, Player: p, State: gamenet.StateGame}
+ hub.Add(sess)
+ hub.EnterRoom(sess, 1)
+ g.Ticks.Start(2)
+ return &editorHarness{g: g, hub: hub, dir: dir}, sess
+}
diff --git a/internal/admin/owner.go b/internal/admin/owner.go
new file mode 100644
index 0000000..02288f7
--- /dev/null
+++ b/internal/admin/owner.go
@@ -0,0 +1,82 @@
+package admin
+
+import (
+ "bytes"
+ "errors"
+ "io"
+ "net/http"
+)
+
+const maxAdminRequestBytes = 8 << 20
+
+// ownerMiddleware is the single execution boundary for ALL editor API routes,
+// including reads (which must not observe partial edits) and undo/redo. Planning,
+// file writes, runtime callbacks, reloads and history updates stay in one task.
+// Register new API routes behind this boundary; handlers must never redispatch.
+// Network I/O happens outside the task so slow browsers cannot block gameplay.
+func (s *AdminServer) ownerMiddleware(next http.Handler) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, maxAdminRequestBytes))
+ r.Body.Close()
+ if err != nil {
+ status := http.StatusBadRequest
+ var tooLarge *http.MaxBytesError
+ if errors.As(err, &tooLarge) {
+ status = http.StatusRequestEntityTooLarge
+ }
+ writeJSONError(w, "cannot read request body: "+err.Error(), status)
+ return
+ }
+ if r.Context().Err() != nil {
+ return
+ }
+ r = r.Clone(r.Context())
+ r.Body = io.NopCloser(bytes.NewReader(body))
+ response := &bufferedResponse{header: make(http.Header)}
+ var panicValue any
+ if !s.runOnOwner(func() {
+ // Preserve net/http's panic recovery on the HTTP goroutine;
+ // a handler panic must not kill the game's owner loop.
+ defer func() { panicValue = recover() }()
+ if r.Context().Err() == nil {
+ next.ServeHTTP(response, r)
+ }
+ }) {
+ writeJSONError(w, "game is shutting down", http.StatusServiceUnavailable)
+ return
+ }
+ if panicValue != nil {
+ panic(panicValue)
+ }
+ for key, values := range response.header {
+ w.Header()[key] = values
+ }
+ if response.status != 0 {
+ w.WriteHeader(response.status)
+ }
+ _, _ = w.Write(response.body.Bytes())
+ })
+}
+
+// bufferedResponse deliberately provides no streaming/hijacking interfaces:
+// editor API handlers produce finite responses, never network streams.
+type bufferedResponse struct {
+ header http.Header
+ status int
+ body bytes.Buffer
+}
+
+func (w *bufferedResponse) Header() http.Header { return w.header }
+
+func (w *bufferedResponse) WriteHeader(status int) {
+ if w.status == 0 {
+ w.status = status
+ }
+}
+
+func (w *bufferedResponse) Write(p []byte) (int, error) {
+ if w.status == 0 {
+ w.WriteHeader(http.StatusOK)
+ }
+ return w.body.Write(p)
+}
diff --git a/internal/admin/owner_routes_test.go b/internal/admin/owner_routes_test.go
new file mode 100644
index 0000000..a061b0e
--- /dev/null
+++ b/internal/admin/owner_routes_test.go
@@ -0,0 +1,68 @@
+package admin
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "thehouseoficarus/internal/config"
+)
+
+// The production router must not leave file-only mutations or undo/redo on
+// the HTTP goroutine. A rejecting dispatcher prevents ALL API handlers from
+// touching files, including future routes and read endpoints.
+func TestAPIRoutesRequireOwnerAdmission(t *testing.T) {
+ h, _ := newEditorHarness(t)
+ tasks := 0
+ s, err := NewServer(config.Default(), false, h.g.AccountStore, h.g.World, h.g.ItemStore, h.g.ObjectStore, h.g.MobStore, h.dir,
+ h.g.CourseStore.ReloadAll, h.g.MigrateRoomIDsOnOwner, h.g.EvictCasinoRoom, h.g.RebuildCraftIndex, filepath.Join(h.dir, "config.yaml"),
+ func(func()) bool { tasks++; return false })
+ if err != nil {
+ t.Fatal(err)
+ }
+ cookieRec := httptest.NewRecorder()
+ s.setAuthCookie(cookieRec, "alice")
+ cookie := cookieRec.Result().Cookies()[0]
+
+ routes := []string{
+ "GET /api/map", "GET /api/dashboard", "GET /api/undo/state",
+ "POST /api/rooms", "PUT /api/rooms/2", "DELETE /api/rooms/2",
+ "POST /api/rooms/new-empty", "POST /api/rooms/link", "DELETE /api/rooms/link",
+ "POST /api/rooms/insert", "POST /api/rooms/remove", "POST /api/rooms/move",
+ "POST /api/rooms/rename", "POST /api/rooms/swap", "POST /api/rooms/bulk-edit",
+ "POST /api/rooms/bulk-delete", "POST /api/rooms/resolve-duplicate",
+ "POST /api/room-dirs/create", "POST /api/room-dirs/rename",
+ "POST /api/courses", "PUT /api/courses/test", "DELETE /api/courses/test",
+ "POST /api/triggers", "PUT /api/triggers/test", "DELETE /api/triggers/test",
+ "PUT /api/files?path=test.yaml", "POST /api/files?path=test.yaml",
+ "DELETE /api/files?path=test.yaml", "POST /api/files/rename",
+ "POST /api/colors", "POST /api/colors/reset",
+ "POST /api/undo/undo", "POST /api/undo/redo",
+ }
+ for _, entity := range []string{"objects", "items", "mobs", "drops", "hazards", "techs", "modules"} {
+ for _, route := range []string{"POST /api/%s", "PUT /api/%s/test", "DELETE /api/%s/test", "POST /api/%s/rename", "POST /api/%s/move", "POST /api/%s/dirs"} {
+ routes = append(routes, strings.ReplaceAll(route, "%s", entity))
+ }
+ }
+ for _, route := range routes {
+ t.Run(route, func(t *testing.T) {
+ method, path, _ := strings.Cut(route, " ")
+ r := httptest.NewRequest(method, path, strings.NewReader(`{}`))
+ r.AddCookie(cookie)
+ rec := httptest.NewRecorder()
+ before := tasks
+ s.ServeHTTP(rec, r)
+ if tasks != before+1 || rec.Code != http.StatusServiceUnavailable {
+ t.Fatalf("dispatcher calls=%d status=%d", tasks-before, rec.Code)
+ }
+ })
+ }
+ before := tasks
+ rec := httptest.NewRecorder()
+ s.ServeHTTP(rec, httptest.NewRequest("POST", "/api/rooms", nil))
+ if tasks != before || rec.Code != http.StatusUnauthorized {
+ t.Fatal("unauthenticated request reached owner")
+ }
+}
diff --git a/internal/admin/owner_test.go b/internal/admin/owner_test.go
new file mode 100644
index 0000000..35538bc
--- /dev/null
+++ b/internal/admin/owner_test.go
@@ -0,0 +1,152 @@
+package admin
+
+import (
+ "context"
+ "errors"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+)
+
+type checkedBody struct {
+ t *testing.T
+ onOwner *bool
+ io.Reader
+}
+
+func (b checkedBody) Read(p []byte) (int, error) {
+ if *b.onOwner {
+ b.t.Error("network request read on owner")
+ }
+ return b.Reader.Read(p)
+}
+
+func (b checkedBody) Close() error { return nil }
+
+type checkedWriter struct {
+ t *testing.T
+ onOwner *bool
+ *httptest.ResponseRecorder
+}
+
+func (w checkedWriter) Write(p []byte) (int, error) {
+ if *w.onOwner {
+ w.t.Error("network response write on owner")
+ }
+ return w.ResponseRecorder.Write(p)
+}
+
+func (w checkedWriter) WriteHeader(status int) {
+ if *w.onOwner {
+ w.t.Error("network response headers written on owner")
+ }
+ w.ResponseRecorder.WriteHeader(status)
+}
+
+func TestOwnerMiddlewareBuffersNetworkIO(t *testing.T) {
+ for _, method := range []string{"GET", "POST", "PUT", "DELETE", "PATCH"} {
+ t.Run(method, func(t *testing.T) {
+ onOwner, tasks := false, 0
+ s := &AdminServer{runOnOwner: func(fn func()) bool {
+ tasks++
+ onOwner = true
+ fn()
+ onOwner = false
+ return true
+ }}
+ handler := s.ownerMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if !onOwner {
+ t.Error("handler outside owner task")
+ }
+ body, err := io.ReadAll(r.Body)
+ if err != nil || string(body) != "request" {
+ t.Errorf("body = %q, %v", body, err)
+ }
+ w.Header().Set("X-Test", "buffered")
+ w.WriteHeader(http.StatusCreated)
+ w.WriteHeader(http.StatusBadRequest) // first status wins
+ _, _ = w.Write([]byte("response"))
+ }))
+ r := httptest.NewRequest(method, "/api/test", nil)
+ r.Body = checkedBody{t, &onOwner, strings.NewReader("request")}
+ rec := httptest.NewRecorder()
+ handler.ServeHTTP(checkedWriter{t, &onOwner, rec}, r)
+ if tasks != 1 || rec.Code != http.StatusCreated || rec.Body.String() != "response" || rec.Header().Get("X-Test") != "buffered" {
+ t.Fatalf("tasks=%d response=%+v", tasks, rec)
+ }
+ })
+ }
+}
+
+type failingBody struct{}
+
+func (failingBody) Read([]byte) (int, error) { return 0, errors.New("broken upload") }
+func (failingBody) Close() error { return nil }
+
+func TestOwnerMiddlewareRejectsBeforeDispatch(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ body io.ReadCloser
+ status int
+ }{
+ {"oversize", io.NopCloser(strings.NewReader(strings.Repeat("x", maxAdminRequestBytes+1))), http.StatusRequestEntityTooLarge},
+ {"read error", failingBody{}, http.StatusBadRequest},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ s := &AdminServer{runOnOwner: func(func()) bool {
+ t.Error("invalid request dispatched")
+ return false
+ }}
+ r := httptest.NewRequest("POST", "/api/test", nil)
+ r.Body = tc.body
+ rec := httptest.NewRecorder()
+ s.ownerMiddleware(nil).ServeHTTP(rec, r)
+ if rec.Code != tc.status {
+ t.Fatalf("status=%d, want %d", rec.Code, tc.status)
+ }
+ })
+ }
+}
+
+func TestOwnerMiddlewareShutdownAndCancellation(t *testing.T) {
+ t.Run("shutdown", func(t *testing.T) {
+ s := &AdminServer{runOnOwner: func(func()) bool { return false }}
+ rec := httptest.NewRecorder()
+ s.ownerMiddleware(nil).ServeHTTP(rec, httptest.NewRequest("POST", "/api/test", nil))
+ if rec.Code != http.StatusServiceUnavailable {
+ t.Fatalf("status=%d", rec.Code)
+ }
+ })
+ t.Run("cancelled while queued", func(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ s := &AdminServer{runOnOwner: func(fn func()) bool {
+ cancel()
+ fn()
+ return true
+ }}
+ s.ownerMiddleware(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+ t.Error("cancelled request executed")
+ })).ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/api/test", nil).WithContext(ctx))
+ })
+}
+
+func TestOwnerMiddlewarePanicReturnsToHTTPGoroutine(t *testing.T) {
+ ownerReturned := false
+ s := &AdminServer{runOnOwner: func(fn func()) bool {
+ fn()
+ ownerReturned = true
+ return true
+ }}
+ defer func() {
+ if value := recover(); value != "handler panic" || !ownerReturned {
+ t.Errorf("panic=%v, owner returned=%v", value, ownerReturned)
+ }
+ }()
+ s.ownerMiddleware(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+ panic("handler panic")
+ })).ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/api/test", nil))
+ t.Fatal("panic was swallowed")
+}
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)
}
diff --git a/internal/admin/undo.go b/internal/admin/undo.go
index 0f9744c..98d8d9a 100644
--- a/internal/admin/undo.go
+++ b/internal/admin/undo.go
@@ -61,6 +61,10 @@ type UndoInfo struct {
RedoSize int `json:"redo_size"`
}
+// UndoStack protects history with a mutex, not live game state. Production
+// callers must execute Push/Undo/Redo/Clear within the editor's owner task.
+// Undo/Redo and the subsequent rebuildAfterUndo are one task, not separate
+// dispatches; migrateRoomIDs is an owner-only callback.
type UndoStack struct {
mu sync.Mutex
dataDir string
diff --git a/internal/game/admin_owner_test.go b/internal/game/admin_owner_test.go
new file mode 100644
index 0000000..e592660
--- /dev/null
+++ b/internal/game/admin_owner_test.go
@@ -0,0 +1,27 @@
+package game
+
+import "testing"
+
+func TestRunAdminOnOwnerRejectsShutdown(t *testing.T) {
+ g := &Game{owner: newOwner()}
+ defer g.stopOwner()
+ if !g.RunAdminOnOwner(func() {}) {
+ t.Fatal("running owner rejected task")
+ }
+ g.setShuttingDown()
+ if g.RunAdminOnOwner(func() { t.Error("task ran during shutdown") }) {
+ t.Error("shutdown accepted task")
+ }
+ g.stopOwner()
+ if g.RunAdminOnOwner(func() { t.Error("task ran after owner stopped") }) {
+ t.Error("stopped owner accepted task")
+ }
+}
+
+func TestRunAdminOnOwnerNeverFallsBackInline(t *testing.T) {
+ g := &Game{owner: newOwner()}
+ g.stopOwner()
+ if g.RunAdminOnOwner(func() { t.Error("task ran inline after stop") }) {
+ t.Error("stopped owner accepted task")
+ }
+}
diff --git a/internal/game/exit_migrate.go b/internal/game/exit_migrate.go
index db70c8d..c678591 100644
--- a/internal/game/exit_migrate.go
+++ b/internal/game/exit_migrate.go
@@ -152,7 +152,7 @@ func (g *Game) RewriteRoomIDs(idMap map[int]int) {
}
// MigrateRoomIDs is the complete room-ID migration entry point for other
-// goroutines (the admin API and swapid command): player state, Hub room
+// goroutines: player state, Hub room
// buckets, casino room keys, world room-keyed runtime state, and live mob
// instances all move together under one owner task. This is what makes a
// live rename leave NO gameplay or broadcast attached to the old location.
@@ -161,13 +161,17 @@ func (g *Game) MigrateRoomIDs(idMap map[int]int) {
return
}
g.RunOnOwner(func() {
- g.migrateRoomIDsLocked(idMap)
+ g.MigrateRoomIDsOnOwner(idMap)
})
}
-// migrateRoomIDsLocked performs the full runtime migration; caller must
-// already be on the owner goroutine.
-func (g *Game) migrateRoomIDsLocked(idMap map[int]int) {
+// MigrateRoomIDsOnOwner performs the full runtime migration within an existing
+// owner task (e.g. the admin editor's file commit + migration + reload task).
+// It must not dispatch again: a nested RunOnOwner would deadlock.
+func (g *Game) MigrateRoomIDsOnOwner(idMap map[int]int) {
+ if len(idMap) == 0 {
+ return
+ }
if g.Hub != nil {
g.Hub.RekeyRooms(idMap)
}
diff --git a/internal/game/owner.go b/internal/game/owner.go
index 2fb4777..4a3646e 100644
--- a/internal/game/owner.go
+++ b/internal/game/owner.go
@@ -122,6 +122,37 @@ func (g *Game) run(fn func()) {
<-task
}
+// RunAdminOnOwner synchronously executes one complete editor request. Unlike
+// RunOnOwner it rejects work during shutdown and NEVER falls back to executing
+// on the HTTP goroutine when the owner has stopped. Call only from outside the
+// owner; runtime callbacks within fn must call owner-only methods directly.
+func (g *Game) RunAdminOnOwner(fn func()) bool {
+ if g.isShuttingDown() {
+ return false
+ }
+ accepted := false
+ task := func() {
+ if !g.isShuttingDown() {
+ fn()
+ accepted = true
+ }
+ }
+ if g.owner == nil {
+ // Synthetic Games only; production always has an owner.
+ task()
+ return accepted
+ }
+ done := make(chan struct{})
+ if !g.owner.enqueue(func() {
+ task()
+ close(done)
+ }) {
+ return false
+ }
+ <-done
+ return accepted
+}
+
// tryRun executes fn on the owner if it is currently idle-draining; it is
// not currently used for correctness paths but exists for future non-blocking
// administrative reads. It returns false when the queue is full.
generated by cgit v1.2.3 (git 2.25.1) at 2026-10-24 13:38:42 +0000