From e42952469dadfb3535633c66c881050ac128703c Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Mon, 5 Oct 2026 17:05:14 -0400 Subject: fix: gather verbs, walk oom, admin path bypass, roulette refund --- internal/admin/api_entity.go | 3 ++- internal/admin/api_files.go | 22 ++++++++++++++++------ internal/admin/api_rooms.go | 26 ++++++++++++++++++++++++++ internal/admin/correct_audit_fixes_test.go | 22 ++++++++++++++++++++++ internal/admin/server.go | 12 +++++++++++- internal/admin/undo.go | 17 +++++++++++++++-- 6 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 internal/admin/correct_audit_fixes_test.go (limited to 'internal/admin') diff --git a/internal/admin/api_entity.go b/internal/admin/api_entity.go index b32ab89..204c59e 100644 --- a/internal/admin/api_entity.go +++ b/internal/admin/api_entity.go @@ -38,7 +38,8 @@ var ( refresh: func(s *AdminServer) { s.invalidateEntityCaches("drops") }} mobSpec = entityCRUD{route: "mobs", noun: "mob", searchSubdirs: true, refresh: func(s *AdminServer) { s.invalidateEntityCaches("mobs") }} - objectSpec = entityCRUD{route: "objects", noun: "object", searchSubdirs: true} + objectSpec = entityCRUD{route: "objects", noun: "object", searchSubdirs: true, + refresh: func(s *AdminServer) { s.invalidateEntityCaches("objects") }} hazardSpec = entityCRUD{route: "hazards", noun: "hazard", refresh: func(s *AdminServer) { s.world.ClearHazardCache() }} techSpec = entityCRUD{route: "techs", noun: "tech", searchSubdirs: true, upsert: true, bodyErr: true, okResponse: true} diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go index b9f54c5..a19cb3b 100644 --- a/internal/admin/api_files.go +++ b/internal/admin/api_files.go @@ -23,6 +23,20 @@ func resolveDataPath(dataDir, relPath string) (string, error) { return filepath.Join(dataDir, clean), nil } +// canonicalDataRelPath normalizes a client-supplied data-relative path so that +// repeated leading "/" or "data/" prefixes cannot make the security +// classifiers (isPlayerDataFilePath / isAccountFilePath) disagree with the +// path resolveDataPath actually reads. Without this, "//players/..." resolved +// to the real player file while the classifiers saw a non-player path. +func canonicalDataRelPath(relPath string) string { + clean := filepath.ToSlash(filepath.Clean("/" + strings.TrimPrefix(relPath, "/"))) + clean = strings.TrimPrefix(clean, "/") + for strings.HasPrefix(clean, "data/") { + clean = strings.TrimPrefix(clean, "data/") + } + return clean +} + // evictDeletedRoomFile drops cached room state and any cached in-room games // when the raw file manager deletes a room YAML (rooms//.yaml). // Best-effort: only numeric .yaml stems under rooms/ count as room files. @@ -75,8 +89,7 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { } } - relPath := strings.TrimPrefix(body.Path, "/") - relPath = strings.TrimPrefix(relPath, "data/") + relPath := canonicalDataRelPath(body.Path) oldAbs, err := resolveDataPath(s.dataDir, relPath) if err != nil { failJSON(w, err.Error()) @@ -145,9 +158,7 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { // is currently online is refused: overwriting it would clobber the in-memory // player the game will later save. func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { - relPath := r.URL.Query().Get("path") - relPath = strings.TrimPrefix(relPath, "/") - relPath = strings.TrimPrefix(relPath, "data/") + relPath := canonicalDataRelPath(r.URL.Query().Get("path")) fullPath, pathErr := resolveDataPath(s.dataDir, relPath) if pathErr != nil { failJSON(w, pathErr.Error()) @@ -173,7 +184,6 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { return } - fullPath := filepath.Join(s.dataDir, relPath) info, err := os.Stat(fullPath) if err != nil { failJSON(w, "not found: "+relPath) diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index efc466c..586bfff 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -159,6 +159,16 @@ func (s *AdminServer) handleRooms(w http.ResponseWriter, r *http.Request) { // source room's new exit first, so a failed destination write left // a dangling exit pointing at a room that never existed, with no // undo entry to repair it. + // Validate before writing, exactly like the PUT and bulk-edit paths: + // otherwise a create can persist a room the loader/validator reject. + if content, marshalErr := marshalMapYAML(m); marshalErr != nil { + failJSON(w, fmt.Sprintf("marshal room: %v", marshalErr)) + return + } else if err := validateRoomSchema(content); err != nil { + writeJSONError(w, "room failed schema validation: "+err.Error(), http.StatusBadRequest) + return + } + path := filepath.Join(subdir, strconv.Itoa(id)+".yaml") newContent, writeErr := writeMapAsYAML(path, m) if writeErr != nil { @@ -1244,6 +1254,14 @@ func (s *AdminServer) handleRoomSwap(w http.ResponseWriter, r *http.Request) { return } + // The files now declare each other's content; refresh both rooms' live + // runtime state or each keeps its old objects/mobs alongside the new ones + // (seeding is add-only). + if s.reconcileRoom != nil { + s.reconcileRoom(aID) + s.reconcileRoom(bID) + } + s.undoStack.Push(ChangeDesc{ Description: fmt.Sprintf("swap room %d with %d (%s)", aID, bID, dir), FilePath: aPath, @@ -1433,6 +1451,14 @@ func (s *AdminServer) handleCreateEmptyRoom(w http.ResponseWriter, r *http.Reque "name": roomName, "description": []map[string]any{{"text": "An empty room."}}, } + // Validate before writing, exactly like the main create path. + if content, marshalErr := marshalMapYAML(m); marshalErr != nil { + failJSON(w, "marshal room: "+marshalErr.Error()) + return + } else if err := validateRoomSchema(content); err != nil { + writeJSONError(w, "room failed schema validation: "+err.Error(), http.StatusBadRequest) + return + } newContent, writeErr := writeMapAsYAML(path, m) if writeErr != nil { failJSON(w, "write room: "+writeErr.Error()) diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go new file mode 100644 index 0000000..381fb5c --- /dev/null +++ b/internal/admin/correct_audit_fixes_test.go @@ -0,0 +1,22 @@ +package admin + +import "testing" + +// Regression: the raw file API must canonicalize repeated "/" and "data/" +// prefixes, or "//players/..." reaches the real file while the classifiers +// see a non-player path and skip the redaction / online refusal. +func TestCanonicalDataRelPathStripsRepeatedPrefixes(t *testing.T) { + cases := map[string]string{ + "players/accounts/x.yaml": "players/accounts/x.yaml", + "/players/accounts/x.yaml": "players/accounts/x.yaml", + "//players/accounts/x.yaml": "players/accounts/x.yaml", + "data/players/accounts/x.yaml": "players/accounts/x.yaml", + "data/data/players/accounts/x.yaml": "players/accounts/x.yaml", + "../etc/passwd": "etc/passwd", + } + for in, want := range cases { + if got := canonicalDataRelPath(in); got != want { + t.Errorf("canonicalDataRelPath(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/admin/server.go b/internal/admin/server.go index 96b5981..8fcba77 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -1092,6 +1092,12 @@ func (s *AdminServer) rebuildAfterUndo(change *ChangeDesc) { if s.evictRoom != nil { s.evictRoom(id) } + } else if s.reconcileRoom != nil { + // The file still exists but its content changed: refresh the + // live room so a removed spawn/object stops lingering and a + // restored one appears. Without this, undo of a content edit + // is not reflected until restart. + s.reconcileRoom(id) } } } @@ -1126,11 +1132,15 @@ func (s *AdminServer) invalidateEntityCaches(entityType string) { if s.rebuildCraftIndex != nil { s.rebuildCraftIndex() } + case "objects": + if s.objectStore != nil { + s.objectStore.Reload(s.dataDir) + } } } func (s *AdminServer) invalidateEntityCacheForPath(path string) { - for _, subdir := range []string{"mobs", "drops", "items"} { + for _, subdir := range []string{"mobs", "drops", "items", "objects"} { rel, err := filepath.Rel(filepath.Join(s.dataDir, subdir), path) if err == nil && !strings.HasPrefix(rel, "..") && rel != "." { s.invalidateEntityCaches(subdir) diff --git a/internal/admin/undo.go b/internal/admin/undo.go index 3001c8b..5683f13 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -754,12 +754,21 @@ func (us *UndoStack) load() { toChanges := func(entries []undoEntry) []ChangeDesc { out := make([]ChangeDesc, 0, len(entries)) for _, e := range entries { + // A before-image recorded as nil is persisted as "" and would + // reload as a non-nil empty slice, defeating requireSnapshot and + // letting undo truncate the file to zero bytes. Map it back to nil + // so the guard fires. An intentionally empty before-image is also + // refused, which is safer than zeroing a live file. + oldContent := []byte(e.OldContent) + if e.OldContent == "" { + oldContent = nil + } c := ChangeDesc{ Time: e.Time, Description: e.Description, FilePath: e.FilePath, NewFilePath: e.NewFilePath, - OldContent: []byte(e.OldContent), + OldContent: oldContent, NewContent: []byte(e.NewContent), IsDelete: e.IsDelete, IsCreate: e.IsCreate, @@ -767,10 +776,14 @@ func (us *UndoStack) load() { IDMap: e.IDMap, } for _, ef := range e.ExtraFiles { + oldExtra := []byte(ef.OldContent) + if ef.OldContent == "" { + oldExtra = nil + } c.ExtraFiles = append(c.ExtraFiles, ExtraFile{ FilePath: ef.FilePath, NewFilePath: ef.NewFilePath, - OldContent: []byte(ef.OldContent), + OldContent: oldExtra, NewContent: []byte(ef.NewContent), IsDelete: ef.IsDelete, IsCreate: ef.IsCreate, -- cgit v1.2.3