fix: slop casino and economy fixes

6 files changed, 78 insertions, 20 deletions

diff --git a/internal/admin/api_entity.go b/internal/admin/api_entity.go
index 204c59e..04b9dca 100644
--- a/internal/admin/api_entity.go
+++ b/internal/admin/api_entity.go
@@ -230,6 +230,10 @@ func (s *AdminServer) entityCreate(w http.ResponseWriter, r *http.Request, c ent
 		c.fail(w, http.StatusBadRequest, c.invalidID())
 		return
 	}
+	// Editor-only keys never belong in the file (the update path drops them
+	// too); without this a GET->POST round trip poisons the YAML.
+	delete(m, "_path")
+	delete(m, "_raw")
 	if _, _, err := c.find(s, id); err == nil {
 		c.fail(w, http.StatusConflict, c.noun+" already exists")
 		return
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go
index a19cb3b..b1b3fff 100644
--- a/internal/admin/api_files.go
+++ b/internal/admin/api_files.go
@@ -234,8 +234,13 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		// A redacted account hash is restored from the file on disk before
-		// writing, so the raw editor can never disclose or overwrite it.
-		body = restoreAccountSecret(relPath, fullPath, body)
+		// writing, so the raw editor can never disclose or overwrite it. A
+		// non-mapping body is refused rather than blanking the hash.
+		body, err = restoreAccountSecret(relPath, fullPath, body)
+		if err != nil {
+			failJSON(w, err.Error())
+			return
+		}
 
 		oldContent, existed, err := snapshotBefore(fullPath)
 		if err != nil {
@@ -375,31 +380,34 @@ func redactAccountContent(relPath string, data []byte) []byte {
 
 // restoreAccountSecret copies the on-disk password hash back into a submitted
 // account file, so a redacted value (or any client-supplied value) can never
-// change or blank an existing hash through the raw file editor.
-func restoreAccountSecret(relPath, fullPath string, body []byte) []byte {
+// change or blank an existing hash through the raw file editor. It returns an
+// error when the existing hash cannot be preserved (for example a non-mapping
+// body), so the caller refuses the write instead of blanking the hash.
+func restoreAccountSecret(relPath, fullPath string, body []byte) ([]byte, error) {
 	if !isAccountFilePath(relPath) {
-		return body
+		return body, nil
 	}
 	existing, err := os.ReadFile(fullPath)
 	if err != nil {
-		return body
+		// A missing account file is a create; nothing to preserve.
+		return body, nil
 	}
 	em, err := yamlToMap(existing)
 	if err != nil {
-		return body
+		return nil, fmt.Errorf("existing account file is not a YAML mapping: %w", err)
 	}
 	hash, ok := em["password_hash"]
 	if !ok {
-		return body
+		return body, nil
 	}
 	bm, err := yamlToMap(body)
 	if err != nil {
-		return body
+		return nil, fmt.Errorf("submitted account file is not a YAML mapping; refusing to blank the password hash")
 	}
 	bm["password_hash"] = hash
 	out, err := marshalMapYAML(bm)
 	if err != nil {
-		return body
+		return nil, fmt.Errorf("marshal account file: %w", err)
 	}
-	return out
+	return out, nil
 }
diff --git a/internal/admin/api_helpers.go b/internal/admin/api_helpers.go
index 37cc4ae..d5e23c3 100644
--- a/internal/admin/api_helpers.go
+++ b/internal/admin/api_helpers.go
@@ -17,12 +17,17 @@ import (
 // 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 == ".." {
+	if id == "" || id == "." || id == ".." || strings.HasPrefix(id, ".") {
 		return false
 	}
 	if strings.ContainsAny(id, `/\`) || strings.ContainsRune(id, 0) {
 		return false
 	}
+	for _, r := range id {
+		if r < 32 || r == 127 {
+			return false
+		}
+	}
 	return true
 }
 
diff --git a/internal/admin/api_room_courses.go b/internal/admin/api_room_courses.go
index db74adf..099ebef 100644
--- a/internal/admin/api_room_courses.go
+++ b/internal/admin/api_room_courses.go
@@ -420,8 +420,9 @@ func (s *AdminServer) handleRoomAttachCourse(w http.ResponseWriter, r *http.Requ
 
 // Detach a room from its course; delete the course file if it becomes empty
 
-// detachRoomFromCourse removes roomID from the course it belongs to, if any.
-// Returns the course ID that was modified, or "" if none match.
+// detachRoomFromCourse removes roomID from every course it belongs to, if any.
+// Returns the first affected course ID (or "" if none match) and whether any
+// course was deleted.
 func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted bool) {
 	ids, err := s.listCourseIDs()
 	if err != nil {
@@ -448,15 +449,23 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted
 		remaining = append(remaining, obstacles[idx+1:]...)
 
 		if len(remaining) == 0 {
-			os.Remove(path)
+			// A failed unlink must not be reported as a detach: the course
+			// would keep listing a deleted room.
+			if err := os.Remove(path); err != nil {
+				continue
+			}
 			s.undoStack.Push(ChangeDesc{
 				Description: fmt.Sprintf("delete course %s (last obstacle removed)", id),
 				FilePath:    path,
 				OldContent:  oldContent,
 				IsDelete:    true,
 			})
+			if courseID == "" {
+				courseID = id
+			}
+			deleted = true
 			s.reloadCoursesIfWired()
-			return id, true
+			continue
 		}
 
 		anyList := make([]any, len(remaining))
@@ -466,7 +475,7 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted
 		m["obstacles"] = anyList
 		newContent, err := writeMapAsYAML(path, m)
 		if err != nil {
-			return "", false
+			continue
 		}
 		s.undoStack.Push(ChangeDesc{
 			Description: fmt.Sprintf("detach room %d from course %s", roomID, id),
@@ -474,10 +483,12 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted
 			OldContent:  oldContent,
 			NewContent:  newContent,
 		})
+		if courseID == "" {
+			courseID = id
+		}
 		s.reloadCoursesIfWired()
-		return id, false
 	}
-	return "", false
+	return courseID, deleted
 }
 
 func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Request, roomID int) {
diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go
index 381fb5c..60073f7 100644
--- a/internal/admin/correct_audit_fixes_test.go
+++ b/internal/admin/correct_audit_fixes_test.go
@@ -1,6 +1,11 @@
 package admin
 
-import "testing"
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
 
 // Regression: the raw file API must canonicalize repeated "/" and "data/"
 // prefixes, or "//players/..." reaches the real file while the classifiers
@@ -20,3 +25,27 @@ func TestCanonicalDataRelPathStripsRepeatedPrefixes(t *testing.T) {
 		}
 	}
 }
+
+// Regression: a valid-but-non-mapping body used to pass through unchanged and
+// blank password_hash. It must now be refused, and a mapping body must keep
+// the on-disk hash.
+func TestRestoreAccountSecretRefusesNonMappingBody(t *testing.T) {
+	dir := t.TempDir()
+	full := filepath.Join(dir, "players", "accounts", "alice.yaml")
+	if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(full, []byte("name: alice\npassword_hash: secret\n"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := restoreAccountSecret("players/accounts/alice.yaml", full, []byte("# comment only\n")); err == nil {
+		t.Fatal("non-mapping body accepted; the password hash would be blanked")
+	}
+	out, err := restoreAccountSecret("players/accounts/alice.yaml", full, []byte("name: alice\npassword_hash: \"[redacted]\"\n"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(out), "secret") {
+		t.Fatalf("password hash not preserved: %s", out)
+	}
+}
diff --git a/internal/admin/live_owner_test.go b/internal/admin/live_owner_test.go
index ae53606..de3ade8 100644
--- a/internal/admin/live_owner_test.go
+++ b/internal/admin/live_owner_test.go
@@ -178,6 +178,7 @@ 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) SetReadDeadline(time.Time) error  { return nil }
 func (discardConn) SetEcho(bool) error               { return nil }
 
 func newEditorHarness(t *testing.T) (*editorHarness, *gamenet.Session) {