fix: economy recovery double-refund/erased-liability/cap-drops, entity editor path traversal

13 files changed, 149 insertions, 1 deletions

diff --git a/internal/admin/api_drops.go b/internal/admin/api_drops.go
index 0d14841..3724a6c 100644
--- a/internal/admin/api_drops.go
+++ b/internal/admin/api_drops.go
@@ -37,6 +37,10 @@ func (s *AdminServer) handleDropByID(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing drop id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid drop id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
@@ -142,6 +146,10 @@ func (s *AdminServer) createDrop(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing drop id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid drop id"}`, http.StatusBadRequest)
+		return
+	}
 
 	path := filepath.Join(s.dataDir, "drops", id+".yaml")
 	if _, err := os.Stat(path); err == nil {
diff --git a/internal/admin/api_entity_path_test.go b/internal/admin/api_entity_path_test.go
new file mode 100644
index 0000000..7abf65f
--- /dev/null
+++ b/internal/admin/api_entity_path_test.go
@@ -0,0 +1,62 @@
+package admin
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"thehouseoficarus/internal/config"
+	"thehouseoficarus/internal/player"
+)
+
+// TestEntityAPICannotEscapeData: entity create/rename IDs and directory
+// targets join into file paths, so a traversal ID ("../../x") must be
+// rejected instead of writing outside the data directory.
+func TestEntityAPICannotEscapeData(t *testing.T) {
+	root := t.TempDir()
+	dir := filepath.Join(root, "data")
+	for _, sub := range []string{"objects", "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
+	}
+
+	// Traversal entity ID via the structured create route.
+	if rec := post("/api/objects", `{"id":"../../outside","name":"Escaped object"}`); rec.Code != http.StatusBadRequest {
+		t.Fatalf("traversal object id: status %d body %s", rec.Code, rec.Body.String())
+	}
+	// Traversal new ID via the entity rename route: the rename handler
+	// must reject the request, not rename anything.
+	rec := post("/api/objects/rename", `{"id":"test","new_id":"../../outside"}`)
+	if !strings.Contains(rec.Body.String(), "error") {
+		t.Fatalf("traversal rename id not rejected: status %d body %s", rec.Code, rec.Body.String())
+	}
+	// Nothing may exist outside the data directory.
+	if _, err := os.Stat(filepath.Join(root, "outside.yaml")); !os.IsNotExist(err) {
+		t.Fatal("entity API escaped the data directory")
+	}
+}
diff --git a/internal/admin/api_hazards.go b/internal/admin/api_hazards.go
index 405c9ab..fac68c4 100644
--- a/internal/admin/api_hazards.go
+++ b/internal/admin/api_hazards.go
@@ -35,6 +35,10 @@ func (s *AdminServer) handleHazardByID(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing hazard id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid hazard id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
@@ -140,6 +144,10 @@ func (s *AdminServer) createHazard(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing hazard id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid hazard id"}`, http.StatusBadRequest)
+		return
+	}
 
 	path := filepath.Join(s.dataDir, "hazards", id+".yaml")
 	if _, err := os.Stat(path); err == nil {
diff --git a/internal/admin/api_helpers.go b/internal/admin/api_helpers.go
index 3224598..11ad958 100644
--- a/internal/admin/api_helpers.go
+++ b/internal/admin/api_helpers.go
@@ -4,10 +4,25 @@ import (
 	"bytes"
 	"fmt"
 	"os"
+	"strings"
 
 	"gopkg.in/yaml.v3"
 )
 
+// validEntityID reports whether an entity ID can be safely joined into a
+// file path under its data directory. IDs come from URL path segments and
+// JSON bodies, so an unchecked join could otherwise escape the data
+// directory ("../") or touch dot files.
+func validEntityID(id string) bool {
+	if id == "" || id == "." || id == ".." {
+		return false
+	}
+	if strings.ContainsAny(id, `/\`) || strings.ContainsRune(id, 0) {
+		return false
+	}
+	return true
+}
+
 func yamlToMap(data []byte) (map[string]any, error) {
 	var v any
 	if err := yaml.Unmarshal(data, &v); err != nil {
diff --git a/internal/admin/api_items.go b/internal/admin/api_items.go
index 728997a..749e8ff 100644
--- a/internal/admin/api_items.go
+++ b/internal/admin/api_items.go
@@ -33,6 +33,10 @@ func (s *AdminServer) handleItemByID(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing item id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid item id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
@@ -152,6 +156,10 @@ func (s *AdminServer) createItem(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing item id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid item id"}`, http.StatusBadRequest)
+		return
+	}
 
 	if _, _, err := s.findItemFile(id); err == nil {
 		http.Error(w, `{"error":"item already exists"}`, http.StatusConflict)
diff --git a/internal/admin/api_mobs.go b/internal/admin/api_mobs.go
index 0646753..8531e40 100644
--- a/internal/admin/api_mobs.go
+++ b/internal/admin/api_mobs.go
@@ -35,6 +35,10 @@ func (s *AdminServer) handleMobByID(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing mob id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid mob id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
@@ -143,6 +147,10 @@ func (s *AdminServer) createMob(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing mob id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid mob id"}`, http.StatusBadRequest)
+		return
+	}
 	if _, _, err := s.findMobFile(id); err == nil {
 		http.Error(w, `{"error":"mob already exists"}`, http.StatusConflict)
 		return
diff --git a/internal/admin/api_modules.go b/internal/admin/api_modules.go
index e468c79..074eeea 100644
--- a/internal/admin/api_modules.go
+++ b/internal/admin/api_modules.go
@@ -59,6 +59,10 @@ func (s *AdminServer) handleModuleByID(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
diff --git a/internal/admin/api_objects.go b/internal/admin/api_objects.go
index 340c87c..8f9eac5 100644
--- a/internal/admin/api_objects.go
+++ b/internal/admin/api_objects.go
@@ -33,6 +33,10 @@ func (s *AdminServer) handleObjectByID(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing object id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid object id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
@@ -142,6 +146,10 @@ func (s *AdminServer) createObject(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing object id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid object id"}`, http.StatusBadRequest)
+		return
+	}
 
 	if _, _, err := s.findObjectFile(id); err == nil {
 		http.Error(w, `{"error":"object already exists"}`, http.StatusConflict)
diff --git a/internal/admin/api_room_courses.go b/internal/admin/api_room_courses.go
index 097b9e8..47e4d8c 100644
--- a/internal/admin/api_room_courses.go
+++ b/internal/admin/api_room_courses.go
@@ -153,6 +153,10 @@ func (s *AdminServer) handleCourses(w http.ResponseWriter, r *http.Request) {
 			writeJSON(w, map[string]any{"error": "missing id"})
 			return
 		}
+		if !validEntityID(id) {
+			writeJSON(w, map[string]any{"error": "invalid id"})
+			return
+		}
 		delete(body, "id")
 		path := s.courseFilePath(id)
 		if _, err := os.Stat(path); err == nil {
@@ -186,6 +190,10 @@ func (s *AdminServer) handleCourseByID(w http.ResponseWriter, r *http.Request) {
 		writeJSON(w, map[string]any{"error": "missing id"})
 		return
 	}
+	if !validEntityID(id) {
+		writeJSON(w, map[string]any{"error": "invalid id"})
+		return
+	}
 	path := s.courseFilePath(id)
 
 	switch r.Method {
diff --git a/internal/admin/api_techs.go b/internal/admin/api_techs.go
index 293b1db..ba3288a 100644
--- a/internal/admin/api_techs.go
+++ b/internal/admin/api_techs.go
@@ -67,6 +67,10 @@ func (s *AdminServer) handleTechByID(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, `{"error":"missing id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
diff --git a/internal/admin/api_triggers.go b/internal/admin/api_triggers.go
index 749f5a1..87eb82e 100644
--- a/internal/admin/api_triggers.go
+++ b/internal/admin/api_triggers.go
@@ -40,6 +40,10 @@ func (s *AdminServer) handleTriggers(w http.ResponseWriter, r *http.Request) {
 			writeJSON(w, map[string]any{"error": "missing id"})
 			return
 		}
+		if !validEntityID(id) {
+			writeJSON(w, map[string]any{"error": "invalid id"})
+			return
+		}
 		delete(m, "id")
 
 		kind := triggerKind(m)
@@ -74,6 +78,10 @@ func (s *AdminServer) handleTriggerByID(w http.ResponseWriter, r *http.Request)
 		http.Error(w, `{"error":"missing id"}`, http.StatusBadRequest)
 		return
 	}
+	if !validEntityID(id) {
+		http.Error(w, `{"error":"invalid id"}`, http.StatusBadRequest)
+		return
+	}
 
 	switch r.Method {
 	case http.MethodGet:
diff --git a/internal/admin/server.go b/internal/admin/server.go
index 3dbe8ad..fb0c53b 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -622,7 +622,8 @@ func (s *AdminServer) handleEntityRename(entityType string) http.HandlerFunc {
 			ID    string `json:"id"`
 			NewID string `json:"new_id"`
 		}
-		if err := readJSON(r, &body); err != nil || body.ID == "" || body.NewID == "" {
+		if err := readJSON(r, &body); err != nil || body.ID == "" || body.NewID == "" ||
+			!validEntityID(body.NewID) {
 			writeJSON(w, map[string]any{"error": "invalid"})
 			return
 		}
diff --git a/internal/admin/yaml_util.go b/internal/admin/yaml_util.go
index 7157f15..9ef8058 100644
--- a/internal/admin/yaml_util.go
+++ b/internal/admin/yaml_util.go
@@ -182,6 +182,9 @@ func findEntityPath(dataDir, subdir, id string) (string, error) {
 }
 
 func moveEntityFile(dataDir, subdir, id, targetDir string) (string, string, []byte, error) {
+	if targetDir != "" && !validEntityID(targetDir) {
+		return "", "", nil, fmt.Errorf("invalid directory: %q", targetDir)
+	}
 	oldPath, err := findEntityPath(dataDir, subdir, id)
 	if err != nil {
 		return "", "", nil, fmt.Errorf("entity not found: %s/%s", subdir, id)
@@ -228,6 +231,9 @@ func createEntityDir(dataDir, subdir, name string) error {
 }
 
 func renameEntityFile(dataDir, subdir, oldID, newID string) (string, string, []byte, error) {
+	if !validEntityID(newID) {
+		return "", "", nil, fmt.Errorf("invalid new id: %q", newID)
+	}
 	oldPath, err := findEntityPath(dataDir, subdir, oldID)
 	if err != nil {
 		return "", "", nil, fmt.Errorf("entity not found: %s/%s", subdir, oldID)