feat: better card ascii art

13 files changed, 155 insertions, 84 deletions

diff --git a/internal/admin/api_colors.go b/internal/admin/api_colors.go
index 7989e35..b6285c4 100644
--- a/internal/admin/api_colors.go
+++ b/internal/admin/api_colors.go
@@ -12,6 +12,7 @@ import (
 	"thehouseoficarus/internal/color"
 	"thehouseoficarus/internal/config"
 	"thehouseoficarus/internal/game"
+	"thehouseoficarus/internal/persistence"
 )
 
 // colorCategory describes one editable color category: its config key, a
@@ -595,13 +596,13 @@ func (s *AdminServer) writeConfigFile() error {
 		if merr != nil {
 			return merr
 		}
-		return os.WriteFile(s.configPath, out, 0644)
+		return persistence.WriteFile(s.configPath, out, 0644)
 	}
 	out, err := patchColorMaps(data, s.cfg.DefaultColors, s.cfg.ConstantColors)
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(s.configPath, out, 0644)
+	return persistence.WriteFile(s.configPath, out, 0644)
 }
 
 // backupConfigFile copies the current config.yaml next to itself before an
@@ -615,7 +616,7 @@ func (s *AdminServer) backupConfigFile() error {
 		}
 		return err
 	}
-	return os.WriteFile(s.configPath+".bak", data, 0600)
+	return persistence.WriteFile(s.configPath+".bak", data, 0600)
 }
 
 // patchColorMaps updates the default_colors and constant_colors mappings in a
diff --git a/internal/admin/api_entity_path_test.go b/internal/admin/api_entity_path_test.go
index 75fed7a..3a0f87b 100644
--- a/internal/admin/api_entity_path_test.go
+++ b/internal/admin/api_entity_path_test.go
@@ -56,10 +56,31 @@ func TestEntityAPICannotEscapeData(t *testing.T) {
 	if !strings.Contains(rec.Body.String(), "error") {
 		t.Fatalf("traversal rename id not rejected: status %d body %s", rec.Code, rec.Body.String())
 	}
+	// Traversal SOURCE id via the rename and move routes: the source is
+	// joined into a path too, so it must be rejected before it can move a
+	// file outside data/ (e.g. the server's own config.yaml).
+	sentinel := filepath.Join(root, "sentinel.yaml")
+	if err := os.WriteFile(sentinel, []byte("name: Sentinel\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	for _, tc := range []struct{ path, body string }{
+		{"/api/objects/rename", `{"id":"../../sentinel","new_id":"stolen"}`},
+		{"/api/objects/move", `{"id":"../../sentinel","dir":"sub"}`},
+	} {
+		if rec := post(tc.path, tc.body); !strings.Contains(rec.Body.String(), "error") {
+			t.Fatalf("traversal source id not rejected at %s: status %d body %s", tc.path, rec.Code, rec.Body.String())
+		}
+	}
+	if _, err := os.Stat(sentinel); err != nil {
+		t.Fatalf("traversal source id moved a file outside data/: %v", err)
+	}
 	// 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")
 	}
+	if _, err := os.Stat(filepath.Join(root, "stolen.yaml")); !os.IsNotExist(err) {
+		t.Fatal("traversal rename escaped the data directory")
+	}
 }
 
 // TestTechModuleCreateRejectsTraversalID: POST /api/techs and /api/modules
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go
index 2aa0e5f..b9f54c5 100644
--- a/internal/admin/api_files.go
+++ b/internal/admin/api_files.go
@@ -227,9 +227,10 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
 		// writing, so the raw editor can never disclose or overwrite it.
 		body = restoreAccountSecret(relPath, fullPath, body)
 
-		var oldContent []byte
-		if existing, err := snapshotFile(fullPath); err == nil {
-			oldContent = existing
+		oldContent, existed, err := snapshotBefore(fullPath)
+		if err != nil {
+			failJSON(w, "read existing file: "+err.Error())
+			return
 		}
 
 		dir := filepath.Dir(fullPath)
@@ -243,7 +244,7 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		isCreate := oldContent == nil
+		isCreate := !existed
 		desc := "Updated file " + relPath
 		if isCreate {
 			desc = "Created file " + relPath
diff --git a/internal/admin/api_modules.go b/internal/admin/api_modules.go
index 8356b2d..ea547c5 100644
--- a/internal/admin/api_modules.go
+++ b/internal/admin/api_modules.go
@@ -96,9 +96,10 @@ func (s *AdminServer) handleModuleByID(w http.ResponseWriter, r *http.Request) {
 		delete(body, "_raw")
 
 		path := filepath.Join(s.dataDir, "modules", id+".yaml")
-		var oldContent []byte
-		if existing, err := snapshotFile(path); err == nil {
-			oldContent = existing
+		oldContent, existed, err := snapshotBefore(path)
+		if err != nil {
+			failJSON(w, "read existing file: "+err.Error())
+			return
 		}
 
 		newContent, err := writeMapAsYAML(path, body)
@@ -107,7 +108,7 @@ func (s *AdminServer) handleModuleByID(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		isCreate := oldContent == nil
+		isCreate := !existed
 		desc := "Updated module " + id
 		if isCreate {
 			desc = "Created module " + id
diff --git a/internal/admin/api_room_courses.go b/internal/admin/api_room_courses.go
index 5a33e6a..db74adf 100644
--- a/internal/admin/api_room_courses.go
+++ b/internal/admin/api_room_courses.go
@@ -204,16 +204,17 @@ func (s *AdminServer) handleCourseByID(w http.ResponseWriter, r *http.Request) {
 		delete(body, "_raw")
 		delete(body, "_path")
 
-		var oldContent []byte
-		if existing, err := snapshotFile(path); err == nil {
-			oldContent = existing
+		oldContent, existed, err := snapshotBefore(path)
+		if err != nil {
+			failJSON(w, "read existing file: "+err.Error())
+			return
 		}
 		newContent, err := writeMapAsYAML(path, body)
 		if err != nil {
 			failJSON(w, "write error: "+err.Error())
 			return
 		}
-		isCreate := oldContent == nil
+		isCreate := !existed
 		desc := "Updated course " + id
 		if isCreate {
 			desc = "Created course " + id
diff --git a/internal/admin/api_room_insert_remove.go b/internal/admin/api_room_insert_remove.go
index 45537d6..5f03b72 100644
--- a/internal/admin/api_room_insert_remove.go
+++ b/internal/admin/api_room_insert_remove.go
@@ -11,6 +11,7 @@ import (
 
 	"gopkg.in/yaml.v3"
 	"thehouseoficarus/internal/behavior"
+	"thehouseoficarus/internal/persistence"
 	"thehouseoficarus/internal/world"
 )
 
@@ -368,7 +369,7 @@ func rollbackWrite(path string, oldContent []byte) {
 	if oldContent == nil {
 		return
 	}
-	if err := os.WriteFile(path, oldContent, 0644); err != nil {
+	if err := persistence.WriteFile(path, oldContent, 0644); err != nil {
 		fmt.Printf("admin: rollback write %s failed: %v\n", path, err)
 	}
 }
diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go
index 32a6f1c..863af47 100644
--- a/internal/admin/api_rooms.go
+++ b/internal/admin/api_rooms.go
@@ -365,7 +365,7 @@ func (s *AdminServer) handleRoomByID(w http.ResponseWriter, r *http.Request) {
 			if loadErr != nil {
 				continue
 			}
-			changed := false
+			changed := otherRoom.RemoveRoomRefs(id)
 			for _, dir := range world.ExitOrder {
 				exit, exists := otherRoom.Exits[dir]
 				if !exists || exit.Room != id {
diff --git a/internal/admin/api_rooms_bulk_delete.go b/internal/admin/api_rooms_bulk_delete.go
index bba1879..d15549e 100644
--- a/internal/admin/api_rooms_bulk_delete.go
+++ b/internal/admin/api_rooms_bulk_delete.go
@@ -120,6 +120,11 @@ func (s *AdminServer) handleBulkDelete(w http.ResponseWriter, r *http.Request) {
 			continue
 		}
 		changed := false
+		for deletedID := range deletedSet {
+			if otherRoom.RemoveRoomRefs(deletedID) {
+				changed = true
+			}
+		}
 		for _, dir := range world.ExitOrder {
 			exit, exists := otherRoom.Exits[dir]
 			if !exists || !deletedSet[exit.Room] {
diff --git a/internal/admin/api_techs.go b/internal/admin/api_techs.go
index 9e742e1..d2c5f65 100644
--- a/internal/admin/api_techs.go
+++ b/internal/admin/api_techs.go
@@ -111,9 +111,16 @@ func (s *AdminServer) handleTechByID(w http.ResponseWriter, r *http.Request) {
 
 		var oldContent []byte
 		if !isCreate {
-			if existing, err := snapshotFile(path); err == nil {
-				oldContent = existing
+			existing, existed, err := snapshotBefore(path)
+			if err != nil {
+				failJSON(w, "read existing file: "+err.Error())
+				return
 			}
+			if !existed {
+				failJSON(w, "not found: "+id)
+				return
+			}
+			oldContent = existing
 		}
 
 		newContent, err := writeMapAsYAML(path, body)
diff --git a/internal/admin/api_triggers.go b/internal/admin/api_triggers.go
index 299ca10..0dd086c 100644
--- a/internal/admin/api_triggers.go
+++ b/internal/admin/api_triggers.go
@@ -11,6 +11,7 @@ import (
 	"gopkg.in/yaml.v3"
 
 	"thehouseoficarus/internal/behavior"
+	"thehouseoficarus/internal/persistence"
 )
 
 func (s *AdminServer) handleTriggers(w http.ResponseWriter, r *http.Request) {
@@ -138,7 +139,7 @@ func (s *AdminServer) handleTriggerByID(w http.ResponseWriter, r *http.Request)
 			return
 		}
 
-		if err := os.WriteFile(newPath, triggerData, 0644); err != nil {
+		if err := persistence.WriteFile(newPath, triggerData, 0644); err != nil {
 			failJSON(w, "write error: "+err.Error())
 			return
 		}
diff --git a/internal/admin/server.go b/internal/admin/server.go
index 05586ef..f35be84 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -982,8 +982,7 @@ 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 == "" ||
-			!validEntityID(body.NewID) {
+		if err := readJSON(r, &body); err != nil || !validEntityID(body.ID) || !validEntityID(body.NewID) {
 			failJSON(w, "invalid")
 			return
 		}
@@ -1015,7 +1014,7 @@ func (s *AdminServer) handleEntityMove(entityType string) http.HandlerFunc {
 			ID  string `json:"id"`
 			Dir string `json:"dir"`
 		}
-		if err := readJSON(r, &body); err != nil || body.ID == "" {
+		if err := readJSON(r, &body); err != nil || !validEntityID(body.ID) {
 			failJSON(w, "invalid")
 			return
 		}
diff --git a/internal/admin/undo.go b/internal/admin/undo.go
index c2ea61d..3001c8b 100644
--- a/internal/admin/undo.go
+++ b/internal/admin/undo.go
@@ -227,6 +227,17 @@ func (us *UndoStack) Redo() *ChangeDesc {
 	return &last
 }
 
+// requireSnapshot rejects a nil before/after image on a change that must
+// restore real content. A failed before-image read leaves nil, and writing
+// it would truncate the file to zero bytes (which parses as a zero-value
+// def), so a missing image must fail the undo/redo instead.
+func requireSnapshot(content []byte, path, action string) error {
+	if content == nil {
+		return fmt.Errorf("%s %s: no snapshot recorded", action, path)
+	}
+	return nil
+}
+
 // applyUndo reverts a change: the primary file, then every extra file.
 // Extra-file failures propagate: a multi-file undo that restored only part
 // of a change must be reported as failed, never as success.
@@ -270,6 +281,9 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error {
 			return fmt.Errorf("remove created %s: %w", c.FilePath, err)
 		}
 	} else {
+		if err := requireSnapshot(c.OldContent, c.FilePath, "revert"); err != nil {
+			return err
+		}
 		if err := us.writeSnapshot(c.FilePath, restored); err != nil {
 			return fmt.Errorf("revert %s: %w", c.FilePath, err)
 		}
@@ -332,6 +346,9 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error {
 		if c.NewFilePath != "" {
 			target = c.NewFilePath
 		}
+		if err := requireSnapshot(c.NewContent, target, "reapply"); err != nil {
+			return err
+		}
 		if err := us.writeSnapshot(target, restored); err != nil {
 			return fmt.Errorf("reapply %s: %w", target, err)
 		}
@@ -362,6 +379,9 @@ func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error {
 			return fmt.Errorf("undo restore %s: %w", ef.FilePath, err)
 		}
 	case ef.NewFilePath != "":
+		if err := requireSnapshot(ef.OldContent, ef.FilePath, "undo restore"); err != nil {
+			return err
+		}
 		if err := os.Remove(ef.NewFilePath); err != nil && !os.IsNotExist(err) {
 			return fmt.Errorf("undo remove %s: %w", ef.NewFilePath, err)
 		}
@@ -369,6 +389,9 @@ func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error {
 			return fmt.Errorf("undo restore %s: %w", ef.FilePath, err)
 		}
 	default:
+		if err := requireSnapshot(ef.OldContent, ef.FilePath, "undo revert"); err != nil {
+			return err
+		}
 		if err := us.writeRestored(ef.FilePath, ef.OldContent); err != nil {
 			return fmt.Errorf("undo revert %s: %w", ef.FilePath, err)
 		}
@@ -387,6 +410,9 @@ func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error {
 			return fmt.Errorf("redo create %s: %w", ef.FilePath, err)
 		}
 	case ef.NewFilePath != "":
+		if err := requireSnapshot(ef.NewContent, ef.NewFilePath, "redo create"); err != nil {
+			return err
+		}
 		if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) {
 			return fmt.Errorf("redo remove %s: %w", ef.FilePath, err)
 		}
@@ -394,6 +420,9 @@ func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error {
 			return fmt.Errorf("redo create %s: %w", ef.NewFilePath, err)
 		}
 	default:
+		if err := requireSnapshot(ef.NewContent, ef.FilePath, "redo reapply"); err != nil {
+			return err
+		}
 		if err := us.writeRestored(ef.FilePath, ef.NewContent); err != nil {
 			return fmt.Errorf("redo reapply %s: %w", ef.FilePath, err)
 		}
@@ -450,7 +479,9 @@ func (us *UndoStack) writeSnapshot(path string, content []byte) error {
 	if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
 		return err
 	}
-	if err := os.WriteFile(path, content, us.fileMode(path)); err != nil {
+	// Atomic temp+fsync+rename: a crash mid-undo must never leave a
+	// truncated world file (empty YAML parses as a zero-value def).
+	if err := persistence.WriteFile(path, content, us.fileMode(path)); err != nil {
 		return err
 	}
 	if us.isPlayerDataPath(path) {
@@ -513,39 +544,46 @@ func (us *UndoStack) Info() UndoInfo {
 	return info
 }
 
+// undoExtraEntry and undoEntry are the persisted undo/redo wire types. They
+// are declared ONCE and shared by save and load: when each function declared
+// its own copy, a field added to one and not the other silently dropped
+// persisted history on the next restart.
+type undoExtraEntry struct {
+	FilePath    string `json:"file_path"`
+	NewFilePath string `json:"new_file_path,omitempty"`
+	OldContent  string `json:"old_content"`
+	NewContent  string `json:"new_content"`
+	IsDelete    bool   `json:"is_delete,omitempty"`
+	IsCreate    bool   `json:"is_create,omitempty"`
+	IsDirDelete bool   `json:"is_dir_delete,omitempty"`
+}
+
+type undoEntry struct {
+	Time        string           `json:"time"`
+	Description string           `json:"description"`
+	FilePath    string           `json:"file_path"`
+	NewFilePath string           `json:"new_file_path,omitempty"`
+	OldContent  string           `json:"old_content"`
+	NewContent  string           `json:"new_content"`
+	IsDelete    bool             `json:"is_delete"`
+	IsCreate    bool             `json:"is_create"`
+	IsDirDelete bool             `json:"is_dir_delete,omitempty"`
+	ExtraFiles  []undoExtraEntry `json:"extra_files,omitempty"`
+	IDMap       map[int]int      `json:"id_map,omitempty"`
+}
+
+type undoFile struct {
+	History []undoEntry `json:"history"`
+	Redo    []undoEntry `json:"redo"`
+}
+
 func (us *UndoStack) save() {
-	type extraEntry struct {
-		FilePath    string `json:"file_path"`
-		NewFilePath string `json:"new_file_path,omitempty"`
-		OldContent  string `json:"old_content"`
-		NewContent  string `json:"new_content"`
-		IsDelete    bool   `json:"is_delete,omitempty"`
-		IsCreate    bool   `json:"is_create,omitempty"`
-		IsDirDelete bool   `json:"is_dir_delete,omitempty"`
-	}
-	type entry struct {
-		Time        string       `json:"time"`
-		Description string       `json:"description"`
-		FilePath    string       `json:"file_path"`
-		NewFilePath string       `json:"new_file_path,omitempty"`
-		OldContent  string       `json:"old_content"`
-		NewContent  string       `json:"new_content"`
-		IsDelete    bool         `json:"is_delete"`
-		IsCreate    bool         `json:"is_create"`
-		IsDirDelete bool         `json:"is_dir_delete,omitempty"`
-		ExtraFiles  []extraEntry `json:"extra_files,omitempty"`
-		IDMap       map[int]int  `json:"id_map,omitempty"`
-	}
-	type saveData struct {
-		History []entry `json:"history"`
-		Redo    []entry `json:"redo"`
-	}
-	toEntries := func(changes []ChangeDesc) []entry {
-		var entries []entry
+	toEntries := func(changes []ChangeDesc) []undoEntry {
+		var entries []undoEntry
 		for _, c := range changes {
-			var extra []extraEntry
+			var extra []undoExtraEntry
 			for _, ef := range c.ExtraFiles {
-				extra = append(extra, extraEntry{
+				extra = append(extra, undoExtraEntry{
 					FilePath:    ef.FilePath,
 					NewFilePath: ef.NewFilePath,
 					OldContent:  string(us.redactContent(ef.OldContent, ef.FilePath, ef.NewFilePath)),
@@ -555,7 +593,7 @@ func (us *UndoStack) save() {
 					IsDirDelete: ef.IsDirDelete,
 				})
 			}
-			entries = append(entries, entry{
+			entries = append(entries, undoEntry{
 				Time:        c.Time,
 				Description: c.Description,
 				FilePath:    c.FilePath,
@@ -571,7 +609,7 @@ func (us *UndoStack) save() {
 		}
 		return entries
 	}
-	data := saveData{
+	data := undoFile{
 		History: toEntries(us.history),
 		Redo:    toEntries(us.redo),
 	}
@@ -708,38 +746,12 @@ func (us *UndoStack) load() {
 	if err != nil {
 		return
 	}
-	type extraEntry struct {
-		FilePath    string `json:"file_path"`
-		NewFilePath string `json:"new_file_path,omitempty"`
-		OldContent  string `json:"old_content"`
-		NewContent  string `json:"new_content"`
-		IsDelete    bool   `json:"is_delete,omitempty"`
-		IsCreate    bool   `json:"is_create,omitempty"`
-		IsDirDelete bool   `json:"is_dir_delete,omitempty"`
-	}
-	type entry struct {
-		Time        string       `json:"time"`
-		Description string       `json:"description"`
-		FilePath    string       `json:"file_path"`
-		NewFilePath string       `json:"new_file_path,omitempty"`
-		OldContent  string       `json:"old_content"`
-		NewContent  string       `json:"new_content"`
-		IsDelete    bool         `json:"is_delete"`
-		IsCreate    bool         `json:"is_create"`
-		IsDirDelete bool         `json:"is_dir_delete,omitempty"`
-		ExtraFiles  []extraEntry `json:"extra_files,omitempty"`
-		IDMap       map[int]int  `json:"id_map,omitempty"`
-	}
-	type saveData struct {
-		History []entry `json:"history"`
-		Redo    []entry `json:"redo"`
-	}
-	var sd saveData
+	var sd undoFile
 	if err := json.Unmarshal(data, &sd); err != nil {
 		log.Printf("undo: failed to load history: %v", err)
 		return
 	}
-	toChanges := func(entries []entry) []ChangeDesc {
+	toChanges := func(entries []undoEntry) []ChangeDesc {
 		out := make([]ChangeDesc, 0, len(entries))
 		for _, e := range entries {
 			c := ChangeDesc{
diff --git a/internal/admin/yaml_util.go b/internal/admin/yaml_util.go
index 8cd3252..e1d4942 100644
--- a/internal/admin/yaml_util.go
+++ b/internal/admin/yaml_util.go
@@ -157,6 +157,21 @@ func snapshotFile(path string) ([]byte, error) {
 	return os.ReadFile(path)
 }
 
+// snapshotBefore reads a file's content for an undo record and reports
+// whether it existed. A missing file returns (nil, false, nil) — a create. A
+// read failure on an EXISTING file returns an error: treating it as a create
+// would make the undo record delete the real file, so the caller must abort.
+func snapshotBefore(path string) ([]byte, bool, error) {
+	data, err := os.ReadFile(path)
+	if err == nil {
+		return data, true, nil
+	}
+	if os.IsNotExist(err) {
+		return nil, false, nil
+	}
+	return nil, false, err
+}
+
 func findEntityPath(dataDir, subdir, id string) (string, error) {
 	_, path, err := readYAMLFile(dataDir, subdir, id)
 	if err == nil {
@@ -167,6 +182,9 @@ func findEntityPath(dataDir, subdir, id string) (string, error) {
 }
 
 func moveEntityFile(dataDir, subdir, id, targetDir string) (string, string, []byte, error) {
+	if !validEntityID(id) {
+		return "", "", nil, fmt.Errorf("invalid id: %q", id)
+	}
 	if targetDir != "" && !validEntityID(targetDir) {
 		return "", "", nil, fmt.Errorf("invalid directory: %q", targetDir)
 	}
@@ -219,6 +237,9 @@ func createEntityDir(dataDir, subdir, name string) error {
 }
 
 func renameEntityFile(dataDir, subdir, oldID, newID string) (string, string, []byte, error) {
+	if !validEntityID(oldID) {
+		return "", "", nil, fmt.Errorf("invalid id: %q", oldID)
+	}
 	if !validEntityID(newID) {
 		return "", "", nil, fmt.Errorf("invalid new id: %q", newID)
 	}