aboutsummaryrefslogtreecommitdiff
path: root/internal/admin
diff options
context:
space:
mode:
Diffstat (limited to 'internal/admin')
-rw-r--r--internal/admin/api_entity_path_test.go49
-rw-r--r--internal/admin/api_modules.go4
-rw-r--r--internal/admin/api_rooms.go70
-rw-r--r--internal/admin/api_rooms_bulk_delete.go3
-rw-r--r--internal/admin/api_techs.go4
-rw-r--r--internal/admin/api_triggers.go6
-rw-r--r--internal/admin/server.go79
-rw-r--r--internal/admin/undo.go41
8 files changed, 232 insertions, 24 deletions
diff --git a/internal/admin/api_entity_path_test.go b/internal/admin/api_entity_path_test.go
index 27ecbe8..75fed7a 100644
--- a/internal/admin/api_entity_path_test.go
+++ b/internal/admin/api_entity_path_test.go
@@ -62,6 +62,55 @@ func TestEntityAPICannotEscapeData(t *testing.T) {
}
}
+// TestTechModuleCreateRejectsTraversalID: POST /api/techs and /api/modules
+// once joined an unvalidated id into a filesystem path, allowing an
+// arbitrary write outside the data directory (and a destructive undo).
+func TestTechModuleCreateRejectsTraversalID(t *testing.T) {
+ root := t.TempDir()
+ dir := filepath.Join(root, "data")
+ for _, sub := range []string{"techs", "modules", "players/accounts"} {
+ if err := os.MkdirAll(filepath.Join(dir, sub), 0o700); err != nil {
+ t.Fatal(err)
+ }
+ }
+ accounts := player.NewAccountStore(dir)
+ if err := accounts.SaveAccount(&player.Account{Name: "auditor", Admin: true}); err != nil {
+ t.Fatal(err)
+ }
+ s, err := NewServer(config.Default(), false, accounts, nil, nil, nil, nil, dir,
+ nil, nil, nil, nil, filepath.Join(dir, "config.yaml"),
+ func(fn func()) bool { fn(); return true })
+ if err != nil {
+ t.Fatal(err)
+ }
+ cookieRec := httptest.NewRecorder()
+ s.setAuthCookie(cookieRec, "auditor")
+ cookie := cookieRec.Result().Cookies()[0]
+
+ post := func(path, body string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(body))
+ req.AddCookie(cookie)
+ rec := httptest.NewRecorder()
+ s.ServeHTTP(rec, req)
+ return rec
+ }
+
+ for _, tc := range []struct{ path, body string }{
+ {"/api/techs", `{"id":"../../outside_tech","name":"x"}`},
+ {"/api/modules", `{"id":"../../outside_mod","name":"x"}`},
+ } {
+ if rec := post(tc.path, tc.body); !strings.Contains(rec.Body.String(), "invalid id") {
+ t.Fatalf("%s traversal id not rejected: status %d body %s", tc.path, rec.Code, rec.Body.String())
+ }
+ }
+ if _, err := os.Stat(filepath.Join(root, "outside_tech.yaml")); !os.IsNotExist(err) {
+ t.Fatal("tech create escaped the data directory")
+ }
+ if _, err := os.Stat(filepath.Join(root, "outside_mod.yaml")); !os.IsNotExist(err) {
+ t.Fatal("module create escaped the data directory")
+ }
+}
+
// TestEntityRenameRejectsDuplicateIDInOtherDirectory: entity IDs are global
// across the type's subdirectory tree (the runtime path indexes resolve by
// filename anywhere under data/<subdir>), so renaming into an ID that
diff --git a/internal/admin/api_modules.go b/internal/admin/api_modules.go
index 1d8c25b..8356b2d 100644
--- a/internal/admin/api_modules.go
+++ b/internal/admin/api_modules.go
@@ -30,6 +30,10 @@ func (s *AdminServer) handleModules(w http.ResponseWriter, r *http.Request) {
failJSON(w, "missing id")
return
}
+ if !validEntityID(id) {
+ failJSON(w, "invalid id")
+ return
+ }
delete(m, "id")
path := filepath.Join(s.dataDir, "modules", id+".yaml")
if _, err := os.Stat(path); err == nil {
diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go
index d84449b..7f317cc 100644
--- a/internal/admin/api_rooms.go
+++ b/internal/admin/api_rooms.go
@@ -343,6 +343,11 @@ func (s *AdminServer) handleRoomByID(w http.ResponseWriter, r *http.Request) {
s.world.RebuildRoomIndex(s.dataDir)
s.world.ClearRoomState(id)
+ // Drop the room's live mob instances too, or they keep wandering and
+ // are returned by MobsInRoom for a room that no longer exists.
+ if s.mobStore != nil {
+ s.mobStore.RemoveMobsInRoom(id)
+ }
// Drop any cached in-room games (tables/machines) and refund their
// outstanding wagers — same eviction semantics as bulk delete.
if s.evictRoom != nil {
@@ -490,6 +495,11 @@ func (s *AdminServer) handleRenameRoomDir(w http.ResponseWriter, r *http.Request
failJSON(w, "rename failed: "+err.Error())
return
}
+ // Undo history stores absolute paths: re-prefix them so a later undo
+ // does not recreate the old directory and write a duplicate room file.
+ if s.undoStack != nil {
+ s.undoStack.RewritePaths(oldPath, newPath)
+ }
s.world.RebuildRoomIndex(s.dataDir)
writeJSON(w, map[string]any{"ok": true, "new_dir": newRel})
}
@@ -674,39 +684,63 @@ func (s *AdminServer) handleRoomLink(w http.ResponseWriter, r *http.Request) {
writeJSONError(w, "room path not found", http.StatusInternalServerError)
return
}
-
- oldA, _ := snapshotFile(pathA)
- newA, err := writeYAMLFile(pathA, roomA)
+ newA, err := marshalYAML(roomA)
if err != nil {
- writeJSONError(w, "write A: "+err.Error(), http.StatusInternalServerError)
+ writeJSONError(w, "serialize A: "+err.Error(), http.StatusInternalServerError)
return
}
- s.undoStack.Push(ChangeDesc{
- Description: fmt.Sprintf("link %d %s %d", body.From, dir, body.To),
- FilePath: pathA,
- OldContent: oldA,
- NewContent: newA,
- })
+ oldA, _ := snapshotFile(pathA)
+ pathB := ""
+ var oldB, newB []byte
if !body.OneWay {
- pathB, okB2 := s.world.GetRoomPath(body.To)
+ var okB2 bool
+ pathB, okB2 = s.world.GetRoomPath(body.To)
if !okB2 {
writeJSONError(w, "room B path not found", http.StatusInternalServerError)
return
}
- oldB, _ := snapshotFile(pathB)
- newB, err := writeYAMLFile(pathB, roomB)
+ newB, err = marshalYAML(roomB)
if err != nil {
+ writeJSONError(w, "serialize B: "+err.Error(), http.StatusInternalServerError)
+ return
+ }
+ oldB, _ = snapshotFile(pathB)
+ }
+
+ // Both sides are resolved and serialized before any write, and both
+ // are applied in one journaled transaction so a failure cannot leave
+ // an asymmetric link, and one Undo reverses the whole action.
+ txn := persistence.NewTransaction(filepath.Join(s.dataDir, ".journal"))
+ if err := txn.Replace(pathA, newA, 0644); err != nil {
+ writeJSONError(w, "write A: "+err.Error(), http.StatusInternalServerError)
+ return
+ }
+ if !body.OneWay {
+ if err := txn.Replace(pathB, newB, 0644); err != nil {
writeJSONError(w, "write B: "+err.Error(), http.StatusInternalServerError)
return
}
- s.undoStack.Push(ChangeDesc{
- Description: fmt.Sprintf("link %d %s %d (reverse)", body.To, oppDir, body.From),
- FilePath: pathB,
- OldContent: oldB,
- NewContent: newB,
+ }
+ if err := txn.Commit(); err != nil {
+ writeJSONError(w, "commit link: "+err.Error(), http.StatusInternalServerError)
+ return
+ }
+
+ desc := ChangeDesc{
+ Description: fmt.Sprintf("link %d %s %d", body.From, dir, body.To),
+ FilePath: pathA,
+ OldContent: oldA,
+ NewContent: newA,
+ }
+ if !body.OneWay {
+ desc.ExtraFiles = append(desc.ExtraFiles, ExtraFile{
+ FilePath: pathB,
+ OldContent: oldB,
+ NewContent: newB,
})
}
+ s.undoStack.Push(desc)
writeJSON(w, map[string]any{"ok": true})
case http.MethodDelete:
diff --git a/internal/admin/api_rooms_bulk_delete.go b/internal/admin/api_rooms_bulk_delete.go
index 6017eed..bba1879 100644
--- a/internal/admin/api_rooms_bulk_delete.go
+++ b/internal/admin/api_rooms_bulk_delete.go
@@ -100,6 +100,9 @@ func (s *AdminServer) handleBulkDelete(w http.ResponseWriter, r *http.Request) {
s.world.RebuildRoomIndex(s.dataDir)
for _, snap := range snapshots {
s.world.ClearRoomState(snap.id)
+ if s.mobStore != nil {
+ s.mobStore.RemoveMobsInRoom(snap.id)
+ }
if s.evictRoom != nil {
s.evictRoom(snap.id)
}
diff --git a/internal/admin/api_techs.go b/internal/admin/api_techs.go
index 91eb929..9e742e1 100644
--- a/internal/admin/api_techs.go
+++ b/internal/admin/api_techs.go
@@ -38,6 +38,10 @@ func (s *AdminServer) handleTechs(w http.ResponseWriter, r *http.Request) {
failJSON(w, "missing id")
return
}
+ if !validEntityID(id) {
+ failJSON(w, "invalid id")
+ return
+ }
delete(m, "id")
if _, _, err := s.findTechFile(id); err == nil {
failJSON(w, "tech already exists")
diff --git a/internal/admin/api_triggers.go b/internal/admin/api_triggers.go
index 08e2855..299ca10 100644
--- a/internal/admin/api_triggers.go
+++ b/internal/admin/api_triggers.go
@@ -152,6 +152,9 @@ func (s *AdminServer) handleTriggerByID(w http.ResponseWriter, r *http.Request)
FilePath: newPath,
OldContent: oldContent,
NewContent: triggerData,
+ // A PUT to a missing trigger creates it; undo must delete the
+ // file rather than overwrite it with the nil before-image.
+ IsCreate: findErr != nil,
}
if findErr == nil && newSubdir != oldDir {
// A kind change moves the file between subdirectories. Record the
@@ -184,6 +187,9 @@ func (s *AdminServer) handleTriggerByID(w http.ResponseWriter, r *http.Request)
Description: "Delete trigger " + id,
FilePath: path,
OldContent: oldContent,
+ // Redo of a delete must delete again, not write the nil
+ // new-content image over the file.
+ IsDelete: true,
})
s.rebuildTriggerIndex()
writeJSON(w, map[string]any{"ok": true})
diff --git a/internal/admin/server.go b/internal/admin/server.go
index 98400c2..05586ef 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -473,6 +473,7 @@ type loginLimiter struct {
type loginAttempts struct {
failures int
+ inflight int
blockedTill time.Time
lastSeen time.Time
}
@@ -481,6 +482,66 @@ func newLoginLimiter() *loginLimiter {
return &loginLimiter{entries: make(map[string]loginAttempts), now: time.Now}
}
+// begin atomically checks the cooldown and reserves an in-flight slot for
+// ip. Reserving at admission time closes the TOCTOU where a concurrent burst
+// all passes allow() before any of them records a failure: the number of
+// simultaneous verifications for one address is bounded by the same
+// threshold. Every successful begin must be paired with finish.
+func (l *loginLimiter) begin(ip string) bool {
+ l.mu.Lock()
+ defer l.mu.Unlock()
+ now := l.now()
+ e, ok := l.entries[ip]
+ if !ok {
+ if !l.makeRoomLocked(now) {
+ // The tracked map is full of cooling-down addresses; do not
+ // reserve (preserves the existing bounded-map behavior).
+ return true
+ }
+ e = loginAttempts{}
+ }
+ if now.Before(e.blockedTill) {
+ return false
+ }
+ if e.failures+e.inflight >= adminLoginMaxFailures {
+ return false
+ }
+ e.inflight++
+ e.lastSeen = now
+ l.entries[ip] = e
+ return true
+}
+
+// finish releases the in-flight slot reserved by begin and records the
+// outcome: success clears the address, failure advances the cooldown.
+func (l *loginLimiter) finish(ip string, ok bool) {
+ l.mu.Lock()
+ defer l.mu.Unlock()
+ now := l.now()
+ e, tracked := l.entries[ip]
+ if !tracked {
+ return
+ }
+ if e.inflight > 0 {
+ e.inflight--
+ }
+ e.lastSeen = now
+ if ok {
+ delete(l.entries, ip)
+ return
+ }
+ if now.Before(e.blockedTill) {
+ l.entries[ip] = e
+ return
+ }
+ e.failures++
+ if e.failures >= adminLoginMaxFailures {
+ e.blockedTill = now.Add(adminLoginCooldown)
+ e.failures = 0
+ }
+ l.entries[ip] = e
+}
+
// allow reports whether ip may attempt a login now.
func (l *loginLimiter) allow(ip string) bool {
l.mu.Lock()
@@ -540,7 +601,7 @@ func (l *loginLimiter) makeRoomLocked(now time.Time) bool {
return true
}
for ip, e := range l.entries {
- if e.failures == 0 && !now.Before(e.blockedTill) {
+ if e.failures == 0 && e.inflight == 0 && !now.Before(e.blockedTill) {
delete(l.entries, ip)
}
}
@@ -616,10 +677,12 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) {
// to make password guessing expensive, so it must not cost a parse.
limiter := s.limitLogins()
ip := clientIP(r.RemoteAddr)
- if !limiter.allow(ip) {
+ if !limiter.begin(ip) {
http.Error(w, "too many failed login attempts; try again later", http.StatusTooManyRequests)
return
}
+ loginOK := false
+ defer func() { limiter.finish(ip, loginOK) }()
// /login is registered before authMiddleware, so bound and buffer its body
// here before r.FormValue can parse it: without the cap a multipart POST
@@ -646,20 +709,17 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) {
// primitive ("account=../../rooms/1"). Every other account-name entry
// point validates first.
if err := player.ValidName(account); err != nil {
- limiter.fail(ip)
s.renderPage(w, r, "login.html", map[string]any{"Error": "Invalid account name or password."})
return
}
acc, err := s.accountStore.LoadAccount(account)
if err != nil || !player.CheckPassword(password, acc.PasswordHash) {
- limiter.fail(ip)
s.renderPage(w, r, "login.html", map[string]any{"Error": "Invalid account name or password."})
return
}
if !acc.Admin {
- limiter.fail(ip)
s.renderPage(w, r, "login.html", map[string]any{"Error": "Account is not authorized as admin."})
return
}
@@ -693,7 +753,7 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) {
return
}
s.setAuthCookie(w, account)
- limiter.succeed(ip)
+ loginOK = true
http.Redirect(w, r, "/", http.StatusFound)
}
@@ -898,6 +958,13 @@ func (s *AdminServer) handleEntityDirs(entityType string) http.HandlerFunc {
failJSON(w, err.Error())
return
}
+ // Undo history stores absolute paths; re-prefix them so a later
+ // undo does not recreate the old entity directory.
+ if s.undoStack != nil {
+ oldPrefix := filepath.Join(s.dataDir, entityType, body.Dir)
+ newPrefix := filepath.Join(s.dataDir, entityType, body.NewName)
+ s.undoStack.RewritePaths(oldPrefix, newPrefix)
+ }
writeJSON(w, map[string]any{"ok": true, "name": body.NewName})
default:
failJSON(w, "unknown action")
diff --git a/internal/admin/undo.go b/internal/admin/undo.go
index 24ce9c1..c2ea61d 100644
--- a/internal/admin/undo.go
+++ b/internal/admin/undo.go
@@ -115,6 +115,47 @@ func (us *UndoStack) SetPlayerFileOnlineCheck(fn func(relPath string) bool) {
us.playerFileOnlineCheck = fn
}
+// RewritePaths re-prefixes every stored history path under oldPrefix so a
+// directory rename does not leave stale absolute paths behind. Without this
+// a later undo recreates the old directory and writes a duplicate file while
+// the live file stays in the new directory. newPrefix may be empty to leave
+// paths untouched.
+func (us *UndoStack) RewritePaths(oldPrefix, newPrefix string) {
+ if oldPrefix == "" || newPrefix == "" || oldPrefix == newPrefix {
+ return
+ }
+ us.mu.Lock()
+ defer us.mu.Unlock()
+ sep := string(os.PathSeparator)
+ rewrite := func(p string) string {
+ if p == "" {
+ return p
+ }
+ if p == oldPrefix {
+ return newPrefix
+ }
+ if strings.HasPrefix(p, oldPrefix+sep) {
+ return filepath.Join(newPrefix, strings.TrimPrefix(p, oldPrefix+sep))
+ }
+ return p
+ }
+ rewriteDesc := func(c *ChangeDesc) {
+ c.FilePath = rewrite(c.FilePath)
+ c.NewFilePath = rewrite(c.NewFilePath)
+ for i := range c.ExtraFiles {
+ c.ExtraFiles[i].FilePath = rewrite(c.ExtraFiles[i].FilePath)
+ c.ExtraFiles[i].NewFilePath = rewrite(c.ExtraFiles[i].NewFilePath)
+ }
+ }
+ for i := range us.history {
+ rewriteDesc(&us.history[i])
+ }
+ for i := range us.redo {
+ rewriteDesc(&us.redo[i])
+ }
+ us.save()
+}
+
func (us *UndoStack) Push(desc ChangeDesc) {
us.mu.Lock()
defer us.mu.Unlock()
generated by cgit v1.2.3 (git 2.25.1) at 2026-10-24 12:37:23 +0000