diff options
Diffstat (limited to 'internal')
39 files changed, 886 insertions, 81 deletions
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go index d32bda4..2b59526 100644 --- a/internal/admin/api_files.go +++ b/internal/admin/api_files.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strconv" "strings" + + "thehouseoficarus/internal/persistence" ) // resolveDataPath resolves a request-supplied path inside the admin data @@ -93,7 +95,7 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { return } - if err := os.WriteFile(newAbs, oldContent, 0644); err != nil { + if err := writeFilePreservingMode(newAbs, oldContent); err != nil { failJSON(w, "write error: "+err.Error()) return } @@ -174,6 +176,7 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { failJSON(w, "read error: "+err.Error()) return } + data = redactAccountContent(relPath, data) writeJSON(w, map[string]any{ "name": info.Name(), "content": string(data), @@ -190,6 +193,9 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { failJSON(w, "read body: "+err.Error()) 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) var oldContent []byte if existing, err := snapshotFile(fullPath); err == nil { @@ -202,7 +208,7 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { return } - if err := os.WriteFile(fullPath, body, 0644); err != nil { + if err := writeFilePreservingMode(fullPath, body); err != nil { failJSON(w, "write error: "+err.Error()) return } @@ -244,3 +250,72 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { http.Error(w, "method not allowed", http.StatusMethodNotAllowed) } } + +// writeFilePreservingMode writes data atomically without widening an existing +// file's permissions (persistence.WriteFile preserves the current mode and +// uses 0644 for a new file). +func writeFilePreservingMode(path string, data []byte) error { + return persistence.WriteFile(path, data, 0644) +} + +// isAccountFilePath reports whether a data-relative path is an account file +// whose password hash must not be exposed or overwritten through the raw file +// editor. +func isAccountFilePath(relPath string) bool { + clean := filepath.ToSlash(filepath.Clean(relPath)) + return strings.HasPrefix(clean, "players/accounts/") +} + +const redactedAccountHash = "[redacted]" + +// redactAccountContent replaces the password hash with a placeholder for GET +// responses. The placeholder round-trips through restoreAccountSecret on save. +func redactAccountContent(relPath string, data []byte) []byte { + if !isAccountFilePath(relPath) { + return data + } + m, err := yamlToMap(data) + if err != nil { + return data + } + if _, ok := m["password_hash"]; !ok { + return data + } + m["password_hash"] = redactedAccountHash + out, err := marshalMapYAML(m) + if err != nil { + return data + } + return out +} + +// 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 { + if !isAccountFilePath(relPath) { + return body + } + existing, err := os.ReadFile(fullPath) + if err != nil { + return body + } + em, err := yamlToMap(existing) + if err != nil { + return body + } + hash, ok := em["password_hash"] + if !ok { + return body + } + bm, err := yamlToMap(body) + if err != nil { + return body + } + bm["password_hash"] = hash + out, err := marshalMapYAML(bm) + if err != nil { + return body + } + return out +} diff --git a/internal/admin/api_helpers.go b/internal/admin/api_helpers.go index 11ad958..37cc4ae 100644 --- a/internal/admin/api_helpers.go +++ b/internal/admin/api_helpers.go @@ -3,10 +3,13 @@ package admin import ( "bytes" "fmt" - "os" + "path/filepath" "strings" "gopkg.in/yaml.v3" + + "thehouseoficarus/internal/persistence" + "thehouseoficarus/internal/world" ) // validEntityID reports whether an entity ID can be safely joined into a @@ -23,6 +26,45 @@ func validEntityID(id string) bool { return true } +// resolveRoomSubdir validates a client-supplied room subdirectory and returns +// the cleaned path relative to <dataDir>/rooms. Absolute paths, ".." escapes, +// drive-like prefixes, and backslashes are rejected; the empty string is the +// rooms root. Room-directory API handlers must route body.Dir/patch.Dir through +// this before joining, mirroring validEntityID/resolveDataPath elsewhere. +func (s *AdminServer) resolveRoomSubdir(dir string) (string, error) { + dir = strings.TrimSpace(dir) + if dir == "" { + return "", nil + } + if filepath.IsAbs(dir) || strings.ContainsRune(dir, '\\') || strings.ContainsRune(dir, 0) { + return "", fmt.Errorf("invalid directory") + } + clean := filepath.Clean(dir) + if clean == "." { + return "", nil + } + if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("directory escapes the rooms tree") + } + base := filepath.Join(s.dataDir, "rooms") + rel, err := filepath.Rel(base, filepath.Join(base, clean)) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("directory escapes the rooms tree") + } + return clean, nil +} + +// validateRoomSchema decodes a rendered room YAML into world.Room before it is +// written, so the editor cannot persist a room the runtime loader would later +// reject (for example a non-numeric exit target). KnownFields(true) also +// catches keys the runtime would silently ignore. +func validateRoomSchema(content []byte) error { + dec := yaml.NewDecoder(bytes.NewReader(content)) + dec.KnownFields(true) + var room world.Room + return dec.Decode(&room) +} + func yamlToMap(data []byte) (map[string]any, error) { var v any if err := yaml.Unmarshal(data, &v); err != nil { @@ -65,7 +107,7 @@ func writeMapAsYAML(path string, m map[string]any) ([]byte, error) { if err != nil { return nil, err } - if err := os.WriteFile(path, content, 0644); err != nil { + if err := persistence.WriteFile(path, content, 0644); err != nil { return nil, err } return content, nil diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index f9508f6..d84449b 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -298,9 +298,17 @@ func (s *AdminServer) handleRoomByID(w http.ResponseWriter, r *http.Request) { return } - newContent, writeErr := writeMapAsYAML(path, m) - if writeErr != nil { - failJSON(w, fmt.Sprintf("write room: %v", writeErr)) + newContent, marshalErr := marshalMapYAML(m) + if marshalErr != nil { + failJSON(w, fmt.Sprintf("marshal room: %v", marshalErr)) + return + } + if err := validateRoomSchema(newContent); err != nil { + writeJSONError(w, "room failed schema validation: "+err.Error(), http.StatusBadRequest) + return + } + if err := writeFilePreservingMode(path, newContent); err != nil { + failJSON(w, fmt.Sprintf("write room: %v", err)) return } @@ -455,14 +463,19 @@ func (s *AdminServer) handleRenameRoomDir(w http.ResponseWriter, r *http.Request failJSON(w, "cannot rename root") return } + dir, err := s.resolveRoomSubdir(body.Dir) + if err != nil || dir == "" { + failJSON(w, "invalid directory") + return + } newName := strings.TrimSpace(body.NewName) if newName == "" || strings.ContainsAny(newName, "/\\") || newName == "." || newName == ".." { failJSON(w, "invalid directory name") return } base := filepath.Join(s.dataDir, "rooms") - parent := filepath.Dir(body.Dir) - oldPath := filepath.Join(base, body.Dir) + parent := filepath.Dir(dir) + oldPath := filepath.Join(base, dir) newRel := filepath.Join(parent, newName) newPath := filepath.Join(base, newRel) if _, err := os.Stat(oldPath); os.IsNotExist(err) { @@ -825,7 +838,11 @@ func (s *AdminServer) handleRoomMove(w http.ResponseWriter, r *http.Request) { failJSON(w, "room not found") return } - dir := strings.TrimSpace(body.Dir) + dir, err := s.resolveRoomSubdir(body.Dir) + if err != nil { + failJSON(w, err.Error()) + return + } destDir := filepath.Join(s.dataDir, "rooms", dir) if _, err := os.Stat(destDir); os.IsNotExist(err) { failJSON(w, "directory does not exist: "+dir) @@ -1279,8 +1296,11 @@ func (s *AdminServer) handleCreateEmptyRoom(w http.ResponseWriter, r *http.Reque } base := filepath.Join(s.dataDir, "rooms") targetDir := base - if body.Dir != "" { - targetDir = filepath.Join(base, body.Dir) + if dir, err := s.resolveRoomSubdir(body.Dir); err != nil { + failJSON(w, err.Error()) + return + } else if dir != "" { + targetDir = filepath.Join(base, dir) if _, err := os.Stat(targetDir); os.IsNotExist(err) { failJSON(w, "directory not found") return diff --git a/internal/admin/api_rooms_bulk.go b/internal/admin/api_rooms_bulk.go index d10e0e3..b9cbb4a 100644 --- a/internal/admin/api_rooms_bulk.go +++ b/internal/admin/api_rooms_bulk.go @@ -108,7 +108,12 @@ func (s *AdminServer) handleBulkEdit(w http.ResponseWriter, r *http.Request) { newPath := path if patch.Dir != nil && *patch.Dir != "" { - destDir := filepath.Join(s.dataDir, "rooms", *patch.Dir) + dir, dirErr := s.resolveRoomSubdir(*patch.Dir) + if dirErr != nil { + failJSON(w, dirErr.Error()) + return + } + destDir := filepath.Join(s.dataDir, "rooms", dir) if _, statErr := os.Stat(destDir); os.IsNotExist(statErr) { failJSON(w, fmt.Sprintf("directory does not exist: %s", *patch.Dir)) return @@ -130,6 +135,10 @@ func (s *AdminServer) handleBulkEdit(w http.ResponseWriter, r *http.Request) { failJSON(w, fmt.Sprintf("marshal room %d: %v", id, marshalErr)) return } + if err := validateRoomSchema(newContent); err != nil { + failJSON(w, fmt.Sprintf("room %d failed schema validation: %v", id, err)) + return + } writes = append(writes, roomWrite{id: id, oldPath: path, newPath: newPath, oldContent: oldContent, newContent: newContent}) } diff --git a/internal/admin/api_rooms_bulk_delete.go b/internal/admin/api_rooms_bulk_delete.go index 5b525f6..6017eed 100644 --- a/internal/admin/api_rooms_bulk_delete.go +++ b/internal/admin/api_rooms_bulk_delete.go @@ -8,6 +8,7 @@ import ( "net/http" "os" + "thehouseoficarus/internal/persistence" "thehouseoficarus/internal/world" ) @@ -74,13 +75,14 @@ func (s *AdminServer) handleBulkDelete(w http.ResponseWriter, r *http.Request) { deletedSet := make(map[int]bool, len(req.IDs)) for _, snap := range snapshots { - s.detachRoomFromCourse(snap.id) if err := os.Remove(snap.path); err != nil { // All-or-nothing: restore every room deleted so far so a failed - // bulk delete never strands rooms without an undo record. + // bulk delete never strands rooms without an undo record. Course + // detaches are deferred until every file is gone, so a failed + // delete cannot leave course references already rewritten. for _, done := range snapshots { if deletedSet[done.id] { - if werr := os.WriteFile(done.path, done.oldContent, 0644); werr != nil { + if werr := persistence.WriteFile(done.path, done.oldContent, 0644); werr != nil { log.Printf("bulk-delete rollback of room %d: %v", done.id, werr) } } @@ -90,6 +92,10 @@ func (s *AdminServer) handleBulkDelete(w http.ResponseWriter, r *http.Request) { } deletedSet[snap.id] = true } + // Room files are gone; now detach the deleted rooms from any courses. + for _, snap := range snapshots { + s.detachRoomFromCourse(snap.id) + } s.world.RebuildRoomIndex(s.dataDir) for _, snap := range snapshots { diff --git a/internal/admin/fixes_regression_test.go b/internal/admin/fixes_regression_test.go new file mode 100644 index 0000000..3d7d19f --- /dev/null +++ b/internal/admin/fixes_regression_test.go @@ -0,0 +1,49 @@ +package admin + +import ( + "path/filepath" + "testing" +) + +// TestResolveRoomSubdirRejectsEscape covers the traversal guard used by the +// room-directory API handlers (rename dir, move room, create empty room, bulk +// edit). Every escaping form must be rejected. +func TestResolveRoomSubdirRejectsEscape(t *testing.T) { + s := &AdminServer{dataDir: t.TempDir()} + for _, bad := range []string{"..", "../items", "../../etc", "/etc", "a/../../b", `..\items`, "town/../../x"} { + if _, err := s.resolveRoomSubdir(bad); err == nil { + t.Fatalf("accepted escaping dir %q", bad) + } + } + for _, ok := range []string{"", ".", "town", "town/sub"} { + got, err := s.resolveRoomSubdir(ok) + if err != nil { + t.Fatalf("rejected valid dir %q: %v", ok, err) + } + if ok != "" && ok != "." && got != filepath.FromSlash(ok) { + t.Fatalf("dir %q resolved to %q", ok, got) + } + } +} + +// TestUndoHistorySurvivesReload: the editor documents that undo history is +// persisted across sessions; NewUndoStack must load what save wrote. +func TestUndoHistorySurvivesReload(t *testing.T) { + dir := t.TempDir() + us := NewUndoStack(dir) + us.Push(ChangeDesc{ + Description: "edit x", + FilePath: filepath.Join(dir, "x.yaml"), + OldContent: []byte("old"), + NewContent: []byte("new"), + }) + + reloaded := NewUndoStack(dir) + info := reloaded.Info() + if !info.CanUndo || info.StackSize != 1 { + t.Fatalf("history did not survive reload: %+v", info) + } + if info.UndoDesc == "" { + t.Fatal("undo description empty after reload") + } +} diff --git a/internal/admin/server.go b/internal/admin/server.go index 12d090d..d9a0386 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -324,19 +324,27 @@ func (s *AdminServer) checkAuth(r *http.Request) bool { if err != nil { return false } - parts := strings.SplitN(cookie.Value, ":", 2) - if len(parts) != 2 { + parts := strings.SplitN(cookie.Value, ":", 3) + if len(parts) != 3 { return false } account := parts[0] - sig := parts[1] + issued, err := strconv.ParseInt(parts[1], 10, 64) + if err != nil { + return false + } + issuedAt := time.Unix(issued, 0) + if time.Since(issuedAt) > adminSessionTTL || time.Until(issuedAt) > time.Minute { + return false + } + sig := parts[2] if !s.isAdminAccount(account) { return false } mac := hmac.New(sha256.New, s.cookieSecret) - mac.Write([]byte(account)) + mac.Write([]byte(account + ":" + parts[1])) expected := hex.EncodeToString(mac.Sum(nil)) return hmac.Equal([]byte(sig), []byte(expected)) } @@ -349,18 +357,24 @@ func (s *AdminServer) isAdminAccount(name string) bool { return acc.Admin } +// adminSessionTTL bounds how long a signed admin cookie is accepted. The +// signature covers the issue time, so a leaked cookie cannot authenticate +// indefinitely and the server can age it out without server-side state. +const adminSessionTTL = 24 * time.Hour + func (s *AdminServer) setAuthCookie(w http.ResponseWriter, account string) { + issued := strconv.FormatInt(time.Now().Unix(), 10) mac := hmac.New(sha256.New, s.cookieSecret) - mac.Write([]byte(account)) + mac.Write([]byte(account + ":" + issued)) sig := hex.EncodeToString(mac.Sum(nil)) http.SetCookie(w, &http.Cookie{ Name: "admin_session", - Value: account + ":" + sig, + Value: account + ":" + issued + ":" + sig, Path: "/", HttpOnly: true, Secure: s.useTLS, SameSite: http.SameSiteStrictMode, - MaxAge: 86400, + MaxAge: int(adminSessionTTL / time.Second), }) } @@ -409,7 +423,6 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) { log.Printf("admin login: password rehash save for %s: %v", account, err) } } - s.undoStack.Clear() }) { writeJSONError(w, "game is shutting down", http.StatusServiceUnavailable) return diff --git a/internal/admin/undo.go b/internal/admin/undo.go index 51610f8..1786295 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -17,6 +17,7 @@ type ExtraFile struct { NewFilePath string `json:"new_file_path,omitempty"` IsDelete bool `json:"is_delete,omitempty"` IsCreate bool `json:"is_create,omitempty"` + IsDirDelete bool `json:"is_dir_delete,omitempty"` } type ChangeDesc struct { @@ -28,6 +29,7 @@ type ChangeDesc struct { NewContent []byte `json:"new_content"` IsDelete bool `json:"is_delete"` IsCreate bool `json:"is_create"` + IsDirDelete bool `json:"is_dir_delete,omitempty"` ExtraFiles []ExtraFile `json:"extra_files,omitempty"` // IDMap records a room-ID change (oldID -> newID) included in the // change. Undo applies its inverse, redo the forward map, through the @@ -78,11 +80,13 @@ type UndoStack struct { } func NewUndoStack(dataDir string) *UndoStack { - return &UndoStack{ + us := &UndoStack{ dataDir: dataDir, maxSize: 100, filePath: filepath.Join(dataDir, ".admin_history.json"), } + us.load() + return us } // SetMigrateRoomIDs wires the runtime room-ID migration used by undo/redo @@ -174,11 +178,14 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error { } } if c.IsDelete { - if c.OldContent == nil { + if c.IsDirDelete { if err := os.MkdirAll(c.FilePath, 0755); err != nil { return fmt.Errorf("recreate dir %s: %w", c.FilePath, err) } } else { + if c.OldContent == nil { + return fmt.Errorf("restore deleted %s: no snapshot recorded", c.FilePath) + } if err := os.MkdirAll(filepath.Dir(c.FilePath), 0755); err != nil { return fmt.Errorf("mkdir: %w", err) } @@ -266,7 +273,14 @@ func applyUndoExtraFile(ef ExtraFile) error { if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("undo remove %s: %w", ef.FilePath, err) } + case ef.IsDelete && ef.IsDirDelete: + if err := os.MkdirAll(ef.FilePath, 0755); err != nil { + return fmt.Errorf("undo mkdir: %w", err) + } case ef.IsDelete: + if ef.OldContent == nil { + return fmt.Errorf("undo restore %s: no snapshot recorded", ef.FilePath) + } if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { return fmt.Errorf("undo mkdir: %w", err) } @@ -354,6 +368,7 @@ func (us *UndoStack) save() { 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"` @@ -364,6 +379,7 @@ func (us *UndoStack) save() { 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"` } @@ -383,6 +399,7 @@ func (us *UndoStack) save() { NewContent: string(ef.NewContent), IsDelete: ef.IsDelete, IsCreate: ef.IsCreate, + IsDirDelete: ef.IsDirDelete, }) } entries = append(entries, entry{ @@ -394,6 +411,7 @@ func (us *UndoStack) save() { NewContent: string(c.NewContent), IsDelete: c.IsDelete, IsCreate: c.IsCreate, + IsDirDelete: c.IsDirDelete, ExtraFiles: extra, IDMap: c.IDMap, }) @@ -414,6 +432,79 @@ func (us *UndoStack) save() { } } +// load restores persisted undo/redo history written by save. A missing or +// unreadable history file is not fatal: the stack simply starts empty. The +// editor documents that history survives restarts, so NewUndoStack calls this. +func (us *UndoStack) load() { + data, err := os.ReadFile(us.filePath) + 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 + if err := json.Unmarshal(data, &sd); err != nil { + log.Printf("undo: failed to load history: %v", err) + return + } + toChanges := func(entries []entry) []ChangeDesc { + out := make([]ChangeDesc, 0, len(entries)) + for _, e := range entries { + c := ChangeDesc{ + Time: e.Time, + Description: e.Description, + FilePath: e.FilePath, + NewFilePath: e.NewFilePath, + OldContent: []byte(e.OldContent), + NewContent: []byte(e.NewContent), + IsDelete: e.IsDelete, + IsCreate: e.IsCreate, + IsDirDelete: e.IsDirDelete, + IDMap: e.IDMap, + } + for _, ef := range e.ExtraFiles { + c.ExtraFiles = append(c.ExtraFiles, ExtraFile{ + FilePath: ef.FilePath, + NewFilePath: ef.NewFilePath, + OldContent: []byte(ef.OldContent), + NewContent: []byte(ef.NewContent), + IsDelete: ef.IsDelete, + IsCreate: ef.IsCreate, + IsDirDelete: ef.IsDirDelete, + }) + } + out = append(out, c) + } + return out + } + us.history = toChanges(sd.History) + us.redo = toChanges(sd.Redo) +} + func (us *UndoStack) Clear() { us.mu.Lock() defer us.mu.Unlock() diff --git a/internal/admin/yaml_util.go b/internal/admin/yaml_util.go index c042248..44712a3 100644 --- a/internal/admin/yaml_util.go +++ b/internal/admin/yaml_util.go @@ -9,6 +9,8 @@ import ( "strings" "gopkg.in/yaml.v3" + + "thehouseoficarus/internal/persistence" ) func readYAMLFile(dataDir, subdir, id string) ([]byte, string, error) { @@ -35,7 +37,7 @@ func writeYAMLFile(path string, data any) ([]byte, error) { if err != nil { return nil, err } - if err := os.WriteFile(path, content, 0644); err != nil { + if err := persistence.WriteFile(path, content, 0644); err != nil { return nil, err } return content, nil @@ -338,6 +340,7 @@ func deleteEntityDir(dataDir, subdir, dir string) ([]ChangeDesc, error) { Description: fmt.Sprintf("delete directory %s/%s", subdir, dir), FilePath: srcDir, IsDelete: true, + IsDirDelete: true, }) return changes, nil diff --git a/internal/behavior/store.go b/internal/behavior/store.go index 8112e1c..cd7a6b7 100644 --- a/internal/behavior/store.go +++ b/internal/behavior/store.go @@ -5,6 +5,7 @@ import ( "math/rand" "os" "path/filepath" + "strconv" "strings" "sync" @@ -31,10 +32,14 @@ func BuildRoomIndex(dir string) map[int]string { return nil } id := strings.TrimSuffix(d.Name(), ".yaml") - var n int - if _, scanErr := fmt.Sscanf(id, "%d", &n); scanErr == nil { - index[n] = path + n, convErr := strconv.Atoi(id) + if convErr != nil { + // A non-numeric filename (for example a "1001_copy.yaml" + // backup) must never shadow a real room via a numeric prefix; + // skip it here and let validation flag it. + return nil } + index[n] = path return nil }) return index @@ -150,7 +155,20 @@ func pickWeighted(drops []DropEntry) *DropEntry { return &drops[0] } +// maxDropTableDepth bounds sub-table recursion. A self- or mutual-reference +// in authored drop tables would otherwise recurse until the goroutine stack +// is exhausted (an unrecoverable fatal error); the validator also rejects +// cycles at startup. +const maxDropTableDepth = 32 + func ResolveDrop(dataDir string, drops []DropEntry) *DropEntry { + return resolveDrop(dataDir, drops, 0) +} + +func resolveDrop(dataDir string, drops []DropEntry, depth int) *DropEntry { + if depth > maxDropTableDepth { + return nil + } picked := pickWeighted(drops) if picked == nil { return nil @@ -160,7 +178,7 @@ func ResolveDrop(dataDir string, drops []DropEntry) *DropEntry { if err != nil { return nil } - resolved := ResolveDrop(dataDir, sub.Drops) + resolved := resolveDrop(dataDir, sub.Drops, depth+1) if resolved == nil { return nil } @@ -187,6 +205,13 @@ func ResolveDrop(dataDir string, drops []DropEntry) *DropEntry { // (default 1) — each roll may produce a different item — and all results are // returned. Tables are just weighted lists of items; this is skill-independent. func ResolveDropList(dataDir string, drops []DropEntry) []DropEntry { + return resolveDropList(dataDir, drops, 0) +} + +func resolveDropList(dataDir string, drops []DropEntry, depth int) []DropEntry { + if depth > maxDropTableDepth { + return nil + } picked := pickWeighted(drops) if picked == nil { return nil @@ -204,7 +229,7 @@ func ResolveDropList(dataDir string, drops []DropEntry) []DropEntry { } var out []DropEntry for i := 0; i < rolls; i++ { - if resolved := ResolveDrop(dataDir, sub.Drops); resolved != nil && resolved.ItemID != "" { + if resolved := resolveDrop(dataDir, sub.Drops, depth+1); resolved != nil && resolved.ItemID != "" { out = append(out, *resolved) } } diff --git a/internal/behavior/types.go b/internal/behavior/types.go index 86115a7..ad10ef4 100644 --- a/internal/behavior/types.go +++ b/internal/behavior/types.go @@ -210,5 +210,6 @@ type DropEntry struct { } type DropTableDef struct { + ID string `yaml:"id"` Drops []DropEntry `yaml:"drops"` } diff --git a/internal/cardart/store.go b/internal/cardart/store.go index 7e9b263..1a3621f 100644 --- a/internal/cardart/store.go +++ b/internal/cardart/store.go @@ -3,6 +3,7 @@ package cardart import ( + "fmt" "os" "gopkg.in/yaml.v3" @@ -15,9 +16,15 @@ type Store struct { func Load(path string) *Store { store := &Store{Cards: make(map[string]map[int][]string)} data, err := os.ReadFile(path) - if err != nil || yaml.Unmarshal(data, store) != nil { + if err != nil { + if !os.IsNotExist(err) { + fmt.Fprintf(os.Stderr, "cardart: read %s: %v\n", path, err) + } return store } + if err := yaml.Unmarshal(data, store); err != nil { + fmt.Fprintf(os.Stderr, "cardart: parse %s: %v\n", path, err) + } return store } diff --git a/internal/casino/config_validate.go b/internal/casino/config_validate.go index 13229bc..8e5e7b5 100644 --- a/internal/casino/config_validate.go +++ b/internal/casino/config_validate.go @@ -68,6 +68,9 @@ func validateIdentityAndBets(id string, minBet, maxBet int) error { if minBet < 0 || maxBet < 0 { return fmt.Errorf("min_bet and max_bet must not be negative") } + if minBet > 0 && maxBet == 0 && minBet > DefaultMaxBet { + return fmt.Errorf("min_bet (%d) exceeds the default maximum bet (%d); set a larger max_bet", minBet, DefaultMaxBet) + } if maxBet > 0 && maxBet < max(1, minBet) { return fmt.Errorf("max_bet must be at least min_bet") } diff --git a/internal/casino/machine.go b/internal/casino/machine.go index 0f137ec..844c5e9 100644 --- a/internal/casino/machine.go +++ b/internal/casino/machine.go @@ -228,8 +228,12 @@ func (m *Machine) finishSpin() []Event { events := []Event{} result := m.spin.Payout + m.payoutRemainder // Checked conversion: a runaway profile or overflow must clamp at the - // economy cap instead of wrapping through a machine-sized int. - if result > MaxCredits || result != result || result < 0 { + // economy cap instead of wrapping through a machine-sized int. A NaN or + // negative result is a corrupt computation, not a jackpot: clamp it to + // zero (fail closed) rather than up to the economy ceiling (fail open). + if math.IsNaN(result) || result < 0 { + result = 0 + } else if result > MaxCredits { result = MaxCredits } credits := int(math.Floor(result)) diff --git a/internal/casino/manager.go b/internal/casino/manager.go index 78e26b0..4798217 100644 --- a/internal/casino/manager.go +++ b/internal/casino/manager.go @@ -3,6 +3,7 @@ package casino import ( "fmt" "math/rand" + "reflect" "sort" "strings" "sync" @@ -41,7 +42,10 @@ func (m *Manager) EnsureMachine(roomID int, cfg MachineConfig) *Machine { m.mu.Lock() defer m.mu.Unlock() if machine := m.machines[key]; machine != nil { - if machine.Config == cfg { + // DeepEqual (not ==) so the *bool rule fields compare by value; + // == would treat every freshly parsed config as different and + // discard an idle machine on every room load. + if reflect.DeepEqual(machine.Config, cfg) { return machine } if machine.idle() { @@ -74,7 +78,7 @@ func (m *Manager) EnsureTable(roomID int, cfg TableConfig) *Table { m.mu.Lock() defer m.mu.Unlock() if table := m.tables[key]; table != nil { - if table.Config == cfg { + if reflect.DeepEqual(table.Config, cfg) { return table } if table.idle() { @@ -308,6 +312,10 @@ func (m *Manager) RefundAll() []Event { // (oldID -> newID): both the manager's map keys and each game's RoomID. // Game state (wagers, rounds) is preserved — the games continue in the // renamed room. Uses delete-all-then-set-all so a swap cannot clobber. +// +// Callers MUST run on the owner goroutine: each game's RoomID is mutated +// without taking that game's own lock, which is safe only because Tick and +// every command are owner-serialized too. func (m *Manager) RekeyRooms(idMap map[int]int) { if len(idMap) == 0 { return diff --git a/internal/casino/minbet_regression_test.go b/internal/casino/minbet_regression_test.go new file mode 100644 index 0000000..578c251 --- /dev/null +++ b/internal/casino/minbet_regression_test.go @@ -0,0 +1,25 @@ +package casino + +import "testing" + +// TestMinBetAboveDefaultMaxRejected guards the config/validate + Normalize +// interaction: an unset max_bet defaults to DefaultMaxBet, so a larger min_bet +// must be rejected (and Normalize must never leave MaxBet below MinBet), or the +// game would accept a table every wager is rejected from. +func TestMinBetAboveDefaultMaxRejected(t *testing.T) { + if err := (TableConfig{ID: "bj", Game: GameBlackjack, MinBet: DefaultMaxBet + 1}).Validate(); err == nil { + t.Fatal("table accepted min_bet above the default max") + } + if err := (MachineConfig{ID: "slots", Game: GameSlots, MinBet: DefaultMaxBet + 1}).Validate(); err == nil { + t.Fatal("machine accepted min_bet above the default max") + } + + m := MachineConfig{ID: "slots", Game: GameSlots, MinBet: DefaultMaxBet + 5}.Normalize() + if m.MaxBet < m.MinBet { + t.Fatalf("normalized machine left max %d below min %d", m.MaxBet, m.MinBet) + } + tc := TableConfig{ID: "bj", Game: GameBlackjack, MinBet: DefaultMaxBet + 5}.Normalize() + if tc.MaxBet < tc.MinBet { + t.Fatalf("normalized table left max %d below min %d", tc.MaxBet, tc.MinBet) + } +} diff --git a/internal/casino/types.go b/internal/casino/types.go index 283aa43..49e2cc4 100644 --- a/internal/casino/types.go +++ b/internal/casino/types.go @@ -128,6 +128,12 @@ func (c MachineConfig) Normalize() MachineConfig { if c.MaxBet <= 0 { c.MaxBet = DefaultMaxBet } + // Never leave a normalized config self-contradictory: a min_bet above + // the default cap must widen the effective maximum rather than produce + // a game every wager is rejected from. + if c.MaxBet < c.MinBet { + c.MaxBet = c.MinBet + } if c.SpinTicks <= 0 { c.SpinTicks = 2 } @@ -153,6 +159,9 @@ func (c TableConfig) Normalize() TableConfig { if c.MaxBet <= 0 { c.MaxBet = DefaultMaxBet } + if c.MaxBet < c.MinBet { + c.MaxBet = c.MinBet + } return c } diff --git a/internal/engine/tick.go b/internal/engine/tick.go index aa10a73..68dd54f 100644 --- a/internal/engine/tick.go +++ b/internal/engine/tick.go @@ -26,6 +26,7 @@ type Engine struct { ticker *time.Ticker running bool stopCh chan struct{} + loopDone chan struct{} } func New() *Engine { @@ -67,8 +68,11 @@ func (e *Engine) Start(tickLengthMs int) { // Capture this run's resources. A quick Stop/Start must not make the // old goroutine consume the replacement ticker or stop channel. ticker, stopCh := e.ticker, e.stopCh + loopDone := make(chan struct{}) + e.loopDone = loopDone go func() { + defer close(loopDone) for { select { case <-ticker.C: @@ -100,10 +104,17 @@ func (e *Engine) Stop() { // exited. Use this for graceful shutdown, where the final tick must finish // before owned state is saved. func (e *Engine) StopAndWait() { + e.mu.Lock() + loopDone := e.loopDone + e.mu.Unlock() e.Stop() - // Join any in-flight tick: processTick holds tickMu for its whole body, - // so acquiring the mutex blocks until the running callback completes; - // holding it until return keeps that guarantee for the caller. + // Wait for the ticker loop goroutine to exit. This is the real save + // barrier: the loop may already have received from the ticker and be + // about to call processTick, which locking tickMu alone would not catch. + if loopDone != nil { + <-loopDone + } + // Also join a direct Tick() that is mid-callback on another goroutine. e.tickMu.Lock() defer e.tickMu.Unlock() } diff --git a/internal/game/account_txn.go b/internal/game/account_txn.go index 2a420ca..cf31dea 100644 --- a/internal/game/account_txn.go +++ b/internal/game/account_txn.go @@ -39,15 +39,22 @@ func (g *Game) newTxn() *persistence.Transaction { // first connection and are re-saved so subsequent authorizations can use // them. Called from the owner goroutine only. func (g *Game) adoptCharacterIdentity(p *player.Player, accountName string) { + assignedID := "" if p.ID == "" { p.ID = newCharacterID() + assignedID = p.ID } if p.Account == "" { p.Account = accountName } if err := g.AccountStore.SaveCharacter(p); err != nil { - // Identity backfill is best-effort: the character remains usable, - // but membership validation falls back to the account list. + // Identity backfill is best-effort, but a NON-DURABLE ID must never + // become an economy key: a later startup's backfill would assign a + // different ID and strand every journal record written under this + // one. Fall back to the durable character name instead. + if assignedID != "" { + p.ID = "" + } log.Printf("identity backfill for %s: %v", p.Name, err) } } diff --git a/internal/game/act_combine.go b/internal/game/act_combine.go index b16cf7a..014dbef 100644 --- a/internal/game/act_combine.go +++ b/internal/game/act_combine.go @@ -134,6 +134,10 @@ func (g *Game) advanceCombine(sess *net.Session, p *player.Player) { for _, bp := range byproducts { if slot := p.FirstFreeSlot(); slot >= 0 { p.SetInvSlot(slot, &player.InventorySlot{ItemID: bp, Quantity: 1}) + } else { + // Never silently delete a byproduct: drop the remainder on + // the ground where the player can pick it up. + g.World.AddGroundItem(p.RoomID, bp, 1) } } diff --git a/internal/game/act_farm.go b/internal/game/act_farm.go index 91384a0..1e576c3 100644 --- a/internal/game/act_farm.go +++ b/internal/game/act_farm.go @@ -180,6 +180,13 @@ func (g *Game) advancePlant(sess *net.Session, p *player.Player) { seedID := data.SeedID xp := data.XP + if p.CountItem(seedID) < 1 { + sess.WriteLine("You no longer have that seed.") + p.Action = nil + return + } + p.RemoveItem(seedID, 1) + g.ensureFarmState(p, prefix) g.setPlayerFlag(p, prefix+"_weeds", false) diff --git a/internal/game/cmd_farm.go b/internal/game/cmd_farm.go index dec16c0..1c8f4c8 100644 --- a/internal/game/cmd_farm.go +++ b/internal/game/cmd_farm.go @@ -124,7 +124,8 @@ func (g *Game) doPlant(sess *net.Session, input string) { return } - p.RemoveItem(seedID, 1) + // The seed is consumed when the plant action COMPLETES (advancePlant), + // not here: an interrupted action must not destroy the seed. p.Action = &behavior.Action{ Type: "plant", diff --git a/internal/game/cmd_quit.go b/internal/game/cmd_quit.go index d6e3313..f8075c1 100644 --- a/internal/game/cmd_quit.go +++ b/internal/game/cmd_quit.go @@ -89,7 +89,11 @@ func (g *Game) beginQuitCountdown(sess *net.Session) { if g.Hub != nil { g.Hub.LeaveRoom(sess) } - sess.Player = nil + if g.Hub != nil { + g.Hub.SetSessionPlayer(sess, nil) + } else { + sess.Player = nil + } sess.SetState(net.StateMenu) g.showMenu(sess) return false diff --git a/internal/game/cmd_shutdown.go b/internal/game/cmd_shutdown.go index 0030649..9a57c6b 100644 --- a/internal/game/cmd_shutdown.go +++ b/internal/game/cmd_shutdown.go @@ -105,9 +105,16 @@ func (g *Game) broadcastLoggedIn(msg string) { if g.Hub == nil { return } - for _, other := range g.Hub.AllSessions() { - if other.Player != nil { - other.WriteLine(msg) + // Player pointers and option maps are owner-owned. Route the broadcast + // through the owner so it cannot race a login/logout or an option change + // from a raw shutdown goroutine (a concurrent map read/write would fatal + // the process before the final save). Drops harmlessly once the owner has + // stopped. + g.run(func() { + for _, other := range g.Hub.AllSessions() { + if other.Player != nil { + other.WriteLine(msg) + } } - } + }) } diff --git a/internal/game/cmd_trigger_combat.go b/internal/game/cmd_trigger_combat.go index f9a6bf1..0578c2b 100644 --- a/internal/game/cmd_trigger_combat.go +++ b/internal/game/cmd_trigger_combat.go @@ -64,9 +64,19 @@ func (g *Game) scienceAttack(sess *net.Session, p *player.Player, mob *world.Mob if mob.HP < 0 { mob.HP = 0 } + // Science damage obeys the same finishing-blow rule as melee/ranged: + // a mob with a required finisher cannot be killed by raw damage. + if mob.FinishingBlow != "" && mob.HP <= 0 { + mob.HP = 1 + } if mob.HP < mob.MaxHP && mob.HP > 0 { mob.StartRegen() } + if mob.FinishingBlow != "" && mob.HP == 1 { + sess.WriteLine(g.colorize(sess, "warning", + fmt.Sprintf("%s resists death! Use %s on it to finish it off.", + mobDisplayName(mob, false), g.itemDisplayName(mob.FinishingBlow)))) + } sciXP := mod.BaseXP hpXP := mod.BaseXP / 3 diff --git a/internal/game/cmd_trigger_enchant.go b/internal/game/cmd_trigger_enchant.go index 62a3c48..02c9755 100644 --- a/internal/game/cmd_trigger_enchant.go +++ b/internal/game/cmd_trigger_enchant.go @@ -72,23 +72,22 @@ func (g *Game) triggerChipBolts(sess *net.Session, p *player.Player, mod *ModDef return } + outputDef, _ := g.ItemStore.Load(chip.Output) + // Verify the output can be placed BEFORE consuming the bolts: never + // consume inputs into an inventory with nowhere to put the result. + if !craftOutputFits(p, chip.Output, outputDef) { + sess.WriteLine("Your inventory is too full!") + return + } + if p.Action != nil { g.cancelAction(p) } p.RemoveItem(chip.Input, chip.Qty) - placed := false - for i := 0; i < 28; i++ { - slot := p.InvSlot(i) - if slot != nil && slot.ItemID == chip.Output { - slot.Quantity += chip.Qty - placed = true - break - } - } - if !placed { - freeSlot := p.FirstFreeSlot() - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: chip.Output, Quantity: chip.Qty}) + if !placeCraftOutput(p, chip.Output, outputDef, chip.Qty) { + sess.WriteLine("Your inventory is too full!") + return } g.triggerModReward(sess, p, mod, 1.0) diff --git a/internal/game/cmd_trigger_utility.go b/internal/game/cmd_trigger_utility.go index 09eabb1..b5a168b 100644 --- a/internal/game/cmd_trigger_utility.go +++ b/internal/game/cmd_trigger_utility.go @@ -136,19 +136,52 @@ func (g *Game) triggerEmGrab(sess *net.Session, p *player.Player, mod *ModDef, t } qty := groundItems[matchedID] - g.World.RemoveGroundItem(p.RoomID, matchedID, qty) - def, _ := g.ItemStore.Load(matchedID) name := matchedID if def != nil { name = def.Name } - freeSlot := p.FirstFreeSlot() - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: matchedID, Quantity: qty}) + // Credits are currency, not an inventory item: collect them the same way + // the get command does. + if matchedID == "credits" { + g.World.RemoveGroundItem(p.RoomID, matchedID, qty) + g.grantCredits(p, qty) + g.triggerModReward(sess, p, mod, 1.0) + sess.WriteLine(fmt.Sprintf("You magnetically pull %d credits toward you.", qty)) + return + } - g.triggerModReward(sess, p, mod, 1.0) + if def != nil && def.Stackable { + g.World.RemoveGroundItem(p.RoomID, matchedID, qty) + if !placeCraftOutput(p, matchedID, def, qty) { + g.World.AddGroundItem(p.RoomID, matchedID, qty) + sess.WriteLine("Your inventory is full.") + return + } + } else { + // Non-stackable items take one slot each; leave any remainder on the + // ground instead of cramming N items into a single slot. + taken := 0 + for taken < qty { + freeSlot := p.FirstFreeSlot() + if freeSlot < 0 { + break + } + p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: matchedID, Quantity: 1}) + taken++ + } + if taken == 0 { + sess.WriteLine("Your inventory is full.") + return + } + g.World.RemoveGroundItem(p.RoomID, matchedID, taken) + if taken < qty { + sess.WriteLine(fmt.Sprintf("Your inventory fills up; %d %s remain on the ground.", qty-taken, name)) + } + } + g.triggerModReward(sess, p, mod, 1.0) sess.WriteLine(fmt.Sprintf("You magnetically pull the %s toward you.", g.itemColorize(sess, def, name))) } @@ -214,6 +247,13 @@ func (g *Game) triggerSuperheat(sess *net.Session, p *player.Player, mod *ModDef g.cancelAction(p) } + // Verify the output can be placed BEFORE consuming: a full inventory + // must never destroy ingredients without producing the bar. + if !craftOutputFits(p, match.ID, match) { + sess.WriteLine("Your inventory is too full!") + return + } + consumeIngredients(c, p.CountItem, p.RemoveItem) outputQty := c.OutputQty @@ -288,19 +328,30 @@ func (g *Game) triggerPlankMake(sess *net.Session, p *player.Player, mod *ModDef return } + plankDef, _ := g.ItemStore.Load(info.plankID) + // Check placement BEFORE consuming the log and credits: a full + // inventory must never delete materials without producing planks. + if !craftOutputFits(p, info.plankID, plankDef) { + sess.WriteLine("Your inventory is full.") + return + } + if p.Action != nil { g.cancelAction(p) } - p.RemoveItem(inv.ItemID, 1) + logID := inv.ItemID + p.RemoveItem(logID, 1) p.Credits -= info.cost - freeSlot := p.FirstFreeSlot() - if freeSlot == -1 { + if !placeCraftOutput(p, info.plankID, plankDef, 1) { + // Defensive: craftOutputFits guaranteed placement. Refund the + // log (to the ground, so it can never be lost) and the credits. + p.Credits += info.cost + g.World.AddGroundItem(p.RoomID, logID, 1) sess.WriteLine("Your inventory is full.") return } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: info.plankID, Quantity: 1}) g.triggerModReward(sess, p, mod, 1.0, xpGain{Skill: string(player.Construction), XP: 10}) diff --git a/internal/game/cmd_wear.go b/internal/game/cmd_wear.go index cea4d67..a02fd3b 100644 --- a/internal/game/cmd_wear.go +++ b/internal/game/cmd_wear.go @@ -94,13 +94,14 @@ func (g *Game) doWear(sess *net.Session, input string) { if slotOccupied { freeSlot := p.FirstFreeSlot() - if freeSlot == -1 && existingItemID != slot.ItemID { + // The equipped item must always be returned to the inventory before + // the new one is removed; with no free slot, refuse rather than + // silently destroy an identical equipped copy. + if freeSlot == -1 { sess.WriteLine("Your inventory is full.") return } - if freeSlot >= 0 { - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: existingItemID, Quantity: 1}) - } + p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: existingItemID, Quantity: 1}) } if slot.Quantity > 1 { diff --git a/internal/game/core_login_char.go b/internal/game/core_login_char.go index 70746fc..0f57d3c 100644 --- a/internal/game/core_login_char.go +++ b/internal/game/core_login_char.go @@ -104,7 +104,11 @@ func (g *Game) connectCharacter(sess *net.Session, name string) { } g.adoptCharacterIdentity(p, sess.Account.Name) - sess.Player = p + if g.Hub != nil { + g.Hub.SetSessionPlayer(sess, p) + } else { + sess.Player = p + } sess.SetState(net.StateGame) g.loggedInChars[name] = sess g.charsMu.Unlock() diff --git a/internal/game/economy.go b/internal/game/economy.go index 21e7721..b39dbe4 100644 --- a/internal/game/economy.go +++ b/internal/game/economy.go @@ -69,6 +69,10 @@ type economyRecord struct { Credit int `json:"credits"` Status string `json:"status"` // "open", "settled", "void" TS time.Time `json:"ts"` + // Seq is the record's monotonic creation order within the journal. It + // lets coverage checks distinguish a settlement that predates a stake + // (and therefore cannot return it) from one written after it. + Seq int64 `json:"seq,omitempty"` // Refund marks a payout record as a SHUTDOWN/EVICTION REFUND (the // stake being returned), not a game settlement. Recovery must not // treat the chip part of a refund as paid in credits: a refund's chip @@ -117,6 +121,9 @@ type economyJournal struct { // (stakeCovered). Same ownership discipline as stakes: all mutations // happen on the game owner or during startup recovery. openPayoutRecs map[string][]*economyRecord + // seq is the monotonic record counter (see economyRecord.Seq). It is + // advanced by startKind and raised to the journal maximum on every fold. + seq int64 } func newEconomyJournal(dataDir string) *economyJournal { @@ -198,6 +205,8 @@ func (j *economyJournal) startKind(kind string, refund bool, covers []string, p } // Retry a bounded number of times: a transient append failure must not // drop the record that makes the wager or owed payout durable. + j.seq++ + rec.Seq = j.seq if err := j.appendRetry(*rec, 5); err != nil { return nil, err } @@ -225,12 +234,16 @@ func (j *economyJournal) closePayoutTracking(rec *economyRecord) { } } -// settle marks a record settled (the balance mutation is durable). -// Fire-and-forget: callers whose recovery semantics depend on the marker -// being durable use settleDurable instead. +// settle marks a record settled (the balance mutation is durable). The +// marker is retried and the record is left OPEN (with its tracking index +// intact) if no append succeeds: a non-durable settle must never drop the +// evidence a later recovery needs, or the liability is paid twice. func (j *economyJournal) settle(rec *economyRecord) { rec.Status = "settled" - _ = j.append(*rec) + if err := j.appendRetry(*rec, 3); err != nil { + rec.Status = "open" + return + } j.closePayoutTracking(rec) } @@ -249,10 +262,15 @@ func (j *economyJournal) settleDurable(rec *economyRecord) bool { } // void marks a record void (the mutation was reverted in memory and never -// became durable; recovery must not compensate). +// became durable; recovery must not compensate). Like settle, the marker is +// retried and the record is left open on failure so its index/evidence is +// preserved for a later attempt. func (j *economyJournal) void(rec *economyRecord) { rec.Status = "void" - _ = j.append(*rec) + if err := j.appendRetry(*rec, 3); err != nil { + rec.Status = "open" + return + } j.closePayoutTracking(rec) } @@ -344,6 +362,13 @@ func (j *economyJournal) stakeCovered(stake *economyRecord) bool { if payout.Refund { continue } + // A payout written BEFORE the stake cannot be its settlement: without + // this ordering check an old open payout (for example a failed slots + // win) would mask a later, unrelated stake and RefundOutstanding + // would resolve it without returning the money. + if payout.Seq <= stake.Seq { + continue + } if len(payout.Covers) == 0 || containsString(payout.Covers, stake.ID) { return true } @@ -526,6 +551,9 @@ func (j *economyJournal) loadFolded() ([]foldedRecord, error) { if corruptPending { return nil, fmt.Errorf("economy journal line %d is malformed; later records exist and may be owed money (journal preserved for inspection)", corruptSeq) } + if rec.Seq > j.seq { + j.seq = rec.Seq + } if existing, ok := latest[rec.ID]; ok { existing.rec = rec existing.seq = seq diff --git a/internal/game/economy_seq_test.go b/internal/game/economy_seq_test.go new file mode 100644 index 0000000..2c5bbdb --- /dev/null +++ b/internal/game/economy_seq_test.go @@ -0,0 +1,24 @@ +package game + +import "testing" + +// TestStakeCoveredRespectsCreationOrder: an open payout created BEFORE a stake +// must not be treated as that stake's settlement, or RefundOutstanding would +// resolve the later stake without returning its money. +func TestStakeCoveredRespectsCreationOrder(t *testing.T) { + j := newEconomyJournal(t.TempDir()) + stake := &economyRecord{ID: "stake", Player: "alice", Kind: "wager", Status: "open", Seq: 2} + j.registerStake(stake) + + older := &economyRecord{ID: "older", Player: "alice", Kind: "payout", Status: "open", Seq: 1, Tracked: true} + j.openPayoutRecs[stake.key()] = []*economyRecord{older} + if j.stakeCovered(stake) { + t.Fatal("a payout created before the stake must not cover it") + } + + newer := &economyRecord{ID: "newer", Player: "alice", Kind: "payout", Status: "open", Seq: 3, Tracked: true} + j.openPayoutRecs[stake.key()] = []*economyRecord{newer} + if !j.stakeCovered(stake) { + t.Fatal("a payout created after the stake should cover it") + } +} diff --git a/internal/game/game.go b/internal/game/game.go index fc6914f..ef48b63 100644 --- a/internal/game/game.go +++ b/internal/game/game.go @@ -187,6 +187,13 @@ func (g *Game) SetHub(hub *net.Hub) { }) hub.OnDisconnect(func(sess *net.Session) { g.run(func() { + // During graceful shutdown the shutdown sequence owns final + // persistence and refunds outstanding wagers; a disconnect + // forfeit here would destroy a stake the shutdown policy + // promises to return. Mirrors the OnRemove guard. + if g.isShuttingDown() { + return + } if sess.Player != nil { g.emitCasinoEvents(g.Casino.MarkDisconnected(sess.Player.Name)) } diff --git a/internal/game/production_engine.go b/internal/game/production_engine.go index 2cd9b1d..035b7e2 100644 --- a/internal/game/production_engine.go +++ b/internal/game/production_engine.go @@ -253,6 +253,10 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { for _, bp := range byproducts { if slot := p.FirstFreeSlot(); slot >= 0 { p.SetInvSlot(slot, &player.InventorySlot{ItemID: bp, Quantity: 1}) + } else { + // Never silently delete a byproduct: drop the remainder on + // the ground where the player can pick it up. + g.World.AddGroundItem(p.RoomID, bp, 1) } } diff --git a/internal/net/server.go b/internal/net/server.go index 78eceb8..9d99acc 100644 --- a/internal/net/server.go +++ b/internal/net/server.go @@ -311,6 +311,16 @@ func (h *Hub) Add(s *Session) { h.sessions[s] = true } +// SetSessionPlayer assigns a session's live-character pointer while holding the +// Hub mutex. The owner goroutine writes Player here; Hub.Remove (a connection +// goroutine) reads it under the same mutex, so the pointer is synchronized +// instead of a data race. +func (h *Hub) SetSessionPlayer(s *Session, p *player.Player) { + h.mu.Lock() + s.Player = p + h.mu.Unlock() +} + func (h *Hub) Remove(s *Session) { h.mu.Lock() if s.Player != nil && !s.IsDisconnecting() { @@ -592,6 +602,23 @@ func (s *Server) Shutdown() { for _, sess := range s.hub.AllSessions() { sess.Close() } + // Give each writer goroutine a bounded window to flush its outbound + // queue before the process exits; otherwise shutdown and refund + // messages can be truncated for slower clients. + flushDeadline := time.Now().Add(2 * time.Second) + for _, sess := range s.hub.AllSessions() { + if sess.writeDone == nil { + continue + } + remaining := time.Until(flushDeadline) + if remaining <= 0 { + break + } + select { + case <-sess.writeDone: + case <-time.After(remaining): + } + } s.finish(nil) }) <-s.done diff --git a/internal/validate/checks.go b/internal/validate/checks.go index 9d09eef..5eb4288 100644 --- a/internal/validate/checks.go +++ b/internal/validate/checks.go @@ -736,6 +736,7 @@ func validateDropTables(s Source) []Issue { var issues []Issue dropIDs := dropTableIDSet(s.DataDir) itemIDs := s.Items.IDSet() + issues = append(issues, validateDropTableCycles(s, dropIDs)...) for id := range dropIDs { dt, err := behavior.LoadDropTable(s.DataDir, id) @@ -838,6 +839,72 @@ func validateDropTables(s Source) []Issue { return issues } +// validateDropTableCycles reports every table that participates in a +// reference cycle. ResolveDrop recurses at runtime, so a self- or +// mutual-reference would otherwise overflow the goroutine stack and kill the +// process; a cycle is therefore a hard startup error. +func validateDropTableCycles(s Source, dropIDs map[string]bool) []Issue { + edges := make(map[string][]string, len(dropIDs)) + for id := range dropIDs { + dt, err := behavior.LoadDropTable(s.DataDir, id) + if err != nil { + continue // reported by the main pass + } + for _, d := range dt.Drops { + if d.Table != "" && dropIDs[d.Table] { + edges[id] = append(edges[id], d.Table) + } + } + } + const ( + white = 0 + gray = 1 + black = 2 + ) + color := make(map[string]int, len(dropIDs)) + reported := make(map[string]bool) + var stack []string + var issues []Issue + var visit func(id string) + visit = func(id string) { + color[id] = gray + stack = append(stack, id) + for _, next := range edges[id] { + switch color[next] { + case gray: + start := 0 + for i, m := range stack { + if m == next { + start = i + break + } + } + for _, m := range stack[start:] { + if reported[m] { + continue + } + reported[m] = true + issues = append(issues, Issue{ + Level: "ERROR", + Type: "integrity", + Message: fmt.Sprintf("Drop table %q participates in a reference cycle; drop-table recursion would overflow the stack", m), + }) + } + case white: + visit(next) + } + } + stack = stack[:len(stack)-1] + color[id] = black + } + for id := range dropIDs { + if color[id] == white { + visit(id) + } + } + return issues +} + func validateCourses(s Source) []Issue { var issues []Issue roomIndex := s.World.RoomIndex() diff --git a/internal/validate/regression_test.go b/internal/validate/regression_test.go new file mode 100644 index 0000000..53475bd --- /dev/null +++ b/internal/validate/regression_test.go @@ -0,0 +1,45 @@ +package validate + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/behavior" +) + +// TestDropTableCycleIsAnError: a self/mutual drop-table reference must be a +// startup ERROR, because ResolveDrop recurses and would otherwise overflow the +// goroutine stack at runtime. +func TestDropTableCycleIsAnError(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "drops", "a.yaml"), "id: a\ndrops:\n - table: b\n weight: 1\n") + writeFile(t, filepath.Join(dir, "drops", "b.yaml"), "id: b\ndrops:\n - table: a\n weight: 1\n") + behavior.ClearDropIndex() + defer behavior.ClearDropIndex() + + issues := validateDropTableCycles(Source{DataDir: dir}, dropTableIDSet(dir)) + if len(issues) == 0 { + t.Fatal("expected a cycle error for a <-> b") + } + for _, issue := range issues { + if issue.Level != "ERROR" { + t.Fatalf("cycle issue must be ERROR: %+v", issue) + } + } +} + +// TestNonNumericRoomFilenameIsAnError: the room index only accepts numeric +// stems, so a backup file must be flagged instead of silently ignored. +func TestNonNumericRoomFilenameIsAnError(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "rooms", "1001.yaml"), "id: 1001\nname: real\n") + writeFile(t, filepath.Join(dir, "rooms", "1001_copy.yaml"), "id: 1001\nname: copy\n") + + issues := validateRoomFilenames(dir) + if len(issues) != 1 { + t.Fatalf("expected exactly one filename issue, got %+v", issues) + } + if issues[0].Level != "ERROR" { + t.Fatalf("expected ERROR, got %+v", issues[0]) + } +} diff --git a/internal/validate/shipped_data_test.go b/internal/validate/shipped_data_test.go new file mode 100644 index 0000000..9c5bef1 --- /dev/null +++ b/internal/validate/shipped_data_test.go @@ -0,0 +1,20 @@ +package validate + +import ( + "path/filepath" + "testing" +) + +// TestShippedDataHasNoUnknownFields guards the authored content tree against +// misspelled or stale YAML keys: every data file is decoded with +// KnownFields(true), so an unread key (like the old exit-level +// set_player_flags or the legacy triggers key) fails CI instead of being +// silently discarded at load time. +func TestShippedDataHasNoUnknownFields(t *testing.T) { + dataDir := filepath.Join("..", "..", "data") + for _, issue := range validateKnownFields(dataDir) { + if issue.Level == "ERROR" { + t.Errorf("%s", issue.Message) + } + } +} diff --git a/internal/validate/validate.go b/internal/validate/validate.go index 735c9ef..8ca4555 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -6,13 +6,18 @@ package validate import ( + "bytes" "fmt" "os" "path/filepath" "sort" + "strconv" "strings" + "gopkg.in/yaml.v3" + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/cardart" "thehouseoficarus/internal/item" "thehouseoficarus/internal/object" "thehouseoficarus/internal/world" @@ -67,6 +72,9 @@ func Run(s Source) []Issue { issues = append(issues, validateIDs(s.DataDir, dir.sub, dir.label)...) } + issues = append(issues, validateKnownFields(s.DataDir)...) + issues = append(issues, validateRoomFilenames(s.DataDir)...) + issues = append(issues, validateRooms(s)...) issues = append(issues, validateMobs(s)...) issues = append(issues, validateHazards(s)...) @@ -84,6 +92,77 @@ func Run(s Source) []Issue { return issues } +// validateRoomFilenames flags room files whose name is not a plain integer: +// the room index only accepts numeric stems, so a file like +// "1001_copy.yaml" would otherwise be silently ignored (and, before the +// strconv fix, could shadow the real room 1001). +func validateRoomFilenames(dataDir string) []Issue { + dir := filepath.Join(dataDir, "rooms") + var issues []Issue + _ = behavior.WalkYAMLDir(dir, func(path, id string, data []byte) error { + if _, err := strconv.Atoi(id); err != nil { + rel := strings.TrimPrefix(path, dataDir+"/") + issues = append(issues, Issue{ + Level: "ERROR", + Type: "integrity", + Message: fmt.Sprintf("room %s: filename is not a numeric room ID and the file is ignored", rel), + }) + } + return nil + }) + return issues +} + +// validateKnownFields decodes every authored data file with KnownFields(true) +// so a misspelled or stale YAML key is a startup ERROR instead of being +// silently discarded by yaml.Unmarshal. Only package-owned content types are +// covered (world/object/item/behavior); game-owned definitions are checked by +// their own loaders. +func validateKnownFields(dataDir string) []Issue { + targets := []struct { + subDir string + label string + newDef func() any + }{ + {"rooms", "room", func() any { return new(world.Room) }}, + {"mobs", "mob", func() any { return new(world.MobDef) }}, + {"objects", "object", func() any { return new(object.ObjectDef) }}, + {"items", "item", func() any { return new(item.ItemDef) }}, + {"drops", "drop table", func() any { return new(behavior.DropTableDef) }}, + {"hazards", "hazard", func() any { return new(world.HazardDef) }}, + } + var issues []Issue + for _, t := range targets { + dir := filepath.Join(dataDir, t.subDir) + _ = behavior.WalkYAMLDir(dir, func(path, id string, data []byte) error { + dec := yaml.NewDecoder(bytes.NewReader(data)) + dec.KnownFields(true) + if err := dec.Decode(t.newDef()); err != nil { + rel := strings.TrimPrefix(path, dataDir+"/") + issues = append(issues, Issue{ + Level: "ERROR", + Type: "unknown-field", + Message: fmt.Sprintf("%s %s: %v", t.label, rel, err), + }) + } + return nil + }) + } + // cards.yaml lives at the data root rather than a subdirectory. + if data, err := os.ReadFile(filepath.Join(dataDir, "cards.yaml")); err == nil { + dec := yaml.NewDecoder(bytes.NewReader(data)) + dec.KnownFields(true) + if err := dec.Decode(new(cardart.Store)); err != nil { + issues = append(issues, Issue{ + Level: "ERROR", + Type: "unknown-field", + Message: fmt.Sprintf("cards.yaml: %v", err), + }) + } + } + return issues +} + func validateIDs(dataDir, subDir, label string) []Issue { dir := filepath.Join(dataDir, subDir) dups := behavior.CheckDuplicateIDs(dir) diff --git a/internal/world/mob.go b/internal/world/mob.go index a794bdd..b004643 100644 --- a/internal/world/mob.go +++ b/internal/world/mob.go @@ -665,7 +665,15 @@ func (s *MobStore) SpawnTransient(def *MobDef, cfg *behavior.SpawnMobConfig, roo s.mu.Lock() defer s.mu.Unlock() - instID := fmt.Sprintf("trigger_%s_%d_%d", cfg.ID, roomID, len(s.instances)) + instID := "" + base := fmt.Sprintf("trigger_%s_%d", cfg.ID, roomID) + for n := 0; ; n++ { + candidate := fmt.Sprintf("%s_%d", base, n) + if _, exists := s.instances[candidate]; !exists { + instID = candidate + break + } + } inst := NewMobInstance(def, instID, roomID, nil, 0) inst.Owner = owner inst.OwnerOnly = cfg.OwnerOnly |