From 153acb7f644d755c45533b7c5311e3ffa47e8973 Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Fri, 18 Sep 2026 05:31:09 -0400 Subject: fix: harden engine across casino, economy, concurrency, security, and content --- internal/admin/undo.go | 329 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 277 insertions(+), 52 deletions(-) (limited to 'internal/admin/undo.go') diff --git a/internal/admin/undo.go b/internal/admin/undo.go index 1786295..7bfbe09 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -6,6 +6,7 @@ import ( "log" "os" "path/filepath" + "strings" "sync" "time" ) @@ -77,6 +78,11 @@ type UndoStack struct { // migrateRoomIDs applies a room-ID map to runtime state (players, Hub // membership, casino room keys) during undo/redo of room-ID changes. migrateRoomIDs func(map[int]int) + // playerFileOnlineCheck reports whether the character/account addressed + // by a data-relative player file path is currently online. undo/redo + // writes those files directly, so it must refuse the same files the raw + // file editor refuses. Optional; nil disables the refusal. + playerFileOnlineCheck func(relPath string) bool } func NewUndoStack(dataDir string) *UndoStack { @@ -97,6 +103,16 @@ func (us *UndoStack) SetMigrateRoomIDs(fn func(map[int]int)) { us.migrateRoomIDs = fn } +// SetPlayerFileOnlineCheck wires the online-session check used by undo/redo to +// refuse touching a character/account file the game still owns. It takes the +// same data-relative path form as AdminServer.SetPlayerFileOnlineCheck, which +// passes its own hook through here. +func (us *UndoStack) SetPlayerFileOnlineCheck(fn func(relPath string) bool) { + us.mu.Lock() + defer us.mu.Unlock() + us.playerFileOnlineCheck = fn +} + func (us *UndoStack) Push(desc ChangeDesc) { us.mu.Lock() defer us.mu.Unlock() @@ -172,6 +188,22 @@ func (us *UndoStack) Redo() *ChangeDesc { // Extra-file failures propagate: a multi-file undo that restored only part // of a change must be reported as failed, never as success. func (us *UndoStack) applyUndo(c ChangeDesc) error { + if err := us.checkPlayerFilesOffline(c); err != nil { + return err + } + // Resolve every secret-bearing image BEFORE touching the filesystem: the + // rename path below removes its target before the restored file is + // written, so a redacted history image that cannot be repaired must fail + // while both files are still intact. + restored, err := us.restoreSecret(c.OldContent, c.FilePath, c.NewFilePath) + if err != nil { + return err + } + for _, ef := range c.ExtraFiles { + if _, err := us.restoreSecret(ef.OldContent, ef.FilePath, ef.NewFilePath); err != nil { + return fmt.Errorf("undo restore %s: %w", ef.FilePath, err) + } + } if c.NewFilePath != "" { if err := os.Remove(c.NewFilePath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("remove %s: %w", c.NewFilePath, err) @@ -186,10 +218,7 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error { 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) - } - if err := os.WriteFile(c.FilePath, c.OldContent, 0644); err != nil { + if err := us.writeSnapshot(c.FilePath, restored); err != nil { return fmt.Errorf("restore deleted %s: %w", c.FilePath, err) } } @@ -198,15 +227,12 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error { return fmt.Errorf("remove created %s: %w", c.FilePath, err) } } else { - if err := os.MkdirAll(filepath.Dir(c.FilePath), 0755); err != nil { - return fmt.Errorf("mkdir: %w", err) - } - if err := os.WriteFile(c.FilePath, c.OldContent, 0644); err != nil { + if err := us.writeSnapshot(c.FilePath, restored); err != nil { return fmt.Errorf("revert %s: %w", c.FilePath, err) } } for _, ef := range c.ExtraFiles { - if err := applyUndoExtraFile(ef); err != nil { + if err := us.applyUndoExtraFile(ef); err != nil { return err } } @@ -216,6 +242,21 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error { // applyRedo re-applies a change: the primary file, then every extra file. // Extra-file failures propagate like undo. func (us *UndoStack) applyRedo(c ChangeDesc) error { + if err := us.checkPlayerFilesOffline(c); err != nil { + return err + } + // See applyUndo: every image's secret is resolved before any file is + // removed, so a redacted image fails the redo without destroying either + // end of a rename. + restored, err := us.restoreSecret(c.NewContent, c.NewFilePath, c.FilePath) + if err != nil { + return err + } + for _, ef := range c.ExtraFiles { + if _, err := us.restoreSecret(ef.NewContent, ef.NewFilePath, ef.FilePath); err != nil { + return fmt.Errorf("redo restore %s: %w", ef.FilePath, err) + } + } if c.NewFilePath != "" { if err := os.Remove(c.FilePath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("remove %s: %w", c.FilePath, err) @@ -226,14 +267,11 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error { if c.NewFilePath != "" { target = c.NewFilePath } - if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil { - return fmt.Errorf("mkdir: %w", err) - } - if err := os.WriteFile(target, c.NewContent, 0644); err != nil { + if err := us.writeSnapshot(target, restored); err != nil { return fmt.Errorf("recreate %s: %w", target, err) } for _, ef := range c.ExtraFiles { - if err := applyRedoExtraFile(ef); err != nil { + if err := us.applyRedoExtraFile(ef); err != nil { return err } } @@ -242,24 +280,20 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error { return fmt.Errorf("re-delete %s: %w", c.FilePath, err) } for _, ef := range c.ExtraFiles { - if err := applyRedoExtraFile(ef); err != nil { + if err := us.applyRedoExtraFile(ef); err != nil { return err } } } else { target := c.FilePath - content := c.NewContent if c.NewFilePath != "" { target = c.NewFilePath } - if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil { - return fmt.Errorf("mkdir: %w", err) - } - if err := os.WriteFile(target, content, 0644); err != nil { + if err := us.writeSnapshot(target, restored); err != nil { return fmt.Errorf("reapply %s: %w", target, err) } for _, ef := range c.ExtraFiles { - if err := applyRedoExtraFile(ef); err != nil { + if err := us.applyRedoExtraFile(ef); err != nil { return err } } @@ -267,7 +301,7 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error { return nil } -func applyUndoExtraFile(ef ExtraFile) error { +func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error { switch { case ef.IsCreate: if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { @@ -281,67 +315,143 @@ func applyUndoExtraFile(ef ExtraFile) error { 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) - } - if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil { + if err := us.writeRestored(ef.FilePath, ef.OldContent, ef.NewFilePath); err != nil { return fmt.Errorf("undo restore %s: %w", ef.FilePath, err) } case ef.NewFilePath != "": if err := os.Remove(ef.NewFilePath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("undo remove %s: %w", ef.NewFilePath, err) } - if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { - return fmt.Errorf("undo mkdir: %w", err) - } - if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil { + if err := us.writeRestored(ef.FilePath, ef.OldContent, ef.NewFilePath); err != nil { return fmt.Errorf("undo restore %s: %w", ef.FilePath, err) } default: - if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { - return fmt.Errorf("undo mkdir: %w", err) - } - if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil { + if err := us.writeRestored(ef.FilePath, ef.OldContent); err != nil { return fmt.Errorf("undo revert %s: %w", ef.FilePath, err) } } return nil } -func applyRedoExtraFile(ef ExtraFile) error { +func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error { switch { case ef.IsDelete: if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("redo remove %s: %w", ef.FilePath, err) } case ef.IsCreate: - if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { - return fmt.Errorf("redo mkdir: %w", err) - } - if err := os.WriteFile(ef.FilePath, ef.NewContent, 0644); err != nil { + if err := us.writeRestored(ef.FilePath, ef.NewContent, ef.NewFilePath); err != nil { return fmt.Errorf("redo create %s: %w", ef.FilePath, err) } case ef.NewFilePath != "": if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("redo remove %s: %w", ef.FilePath, err) } - if err := os.MkdirAll(filepath.Dir(ef.NewFilePath), 0755); err != nil { - return fmt.Errorf("redo mkdir: %w", err) - } - if err := os.WriteFile(ef.NewFilePath, ef.NewContent, 0644); err != nil { + if err := us.writeRestored(ef.NewFilePath, ef.NewContent, ef.FilePath); err != nil { return fmt.Errorf("redo create %s: %w", ef.NewFilePath, err) } default: - if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { - return fmt.Errorf("redo mkdir: %w", err) - } - if err := os.WriteFile(ef.FilePath, ef.NewContent, 0644); err != nil { + if err := us.writeRestored(ef.FilePath, ef.NewContent); err != nil { return fmt.Errorf("redo reapply %s: %w", ef.FilePath, err) } } return nil } +// writeRestored writes content to path after merging the live on-disk player +// secret back into it (see restoreSecret). A history image whose secret was +// redacted and has no live file left to take it from fails here instead of +// being written, so undo/redo reports the failure. Further paths are the +// change's counterparts (a rename's other end). +func (us *UndoStack) writeRestored(path string, content []byte, others ...string) error { + restored, err := us.restoreSecret(content, append([]string{path}, others...)...) + if err != nil { + return err + } + return us.writeSnapshot(path, restored) +} + +// checkPlayerFilesOffline refuses an undo/redo touching a character/account +// file whose owner is online. The raw file editor refuses those files before +// writing them; undo/redo writes them through this stack, so it must refuse +// them too or a live character can be clobbered behind the editor's back. +func (us *UndoStack) checkPlayerFilesOffline(c ChangeDesc) error { + if us.playerFileOnlineCheck == nil { + return nil + } + for _, p := range changePaths(c) { + rel, ok := us.playerDataRelPath(p) + if !ok { + continue + } + if us.playerFileOnlineCheck(rel) { + return fmt.Errorf("%s is online; it must be logged out before this change can be undone or redone", rel) + } + } + return nil +} + +// changePaths lists every file a change touches: the primary path, a rename's +// new path, and every extra file with its own new path. +func changePaths(c ChangeDesc) []string { + paths := []string{c.FilePath, c.NewFilePath} + for _, ef := range c.ExtraFiles { + paths = append(paths, ef.FilePath, ef.NewFilePath) + } + return paths +} + +// writeSnapshot writes content to path, creating the parent directory and +// choosing the file mode from the path: player/account files are owner-only. +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 { + return err + } + if us.isPlayerDataPath(path) { + // Tighten a file that was created world-readable before this fix. + if err := os.Chmod(path, 0600); err != nil { + return err + } + } + return nil +} + +// fileMode returns the mode an undo/redo write should use for path. +func (us *UndoStack) fileMode(path string) os.FileMode { + if us.isPlayerDataPath(path) { + return 0600 + } + return 0644 +} + +// isPlayerDataPath reports whether path is an account or character file under +// the data directory. These carry the password hash and live player state. +func (us *UndoStack) isPlayerDataPath(path string) bool { + _, ok := us.playerDataRelPath(path) + return ok +} + +// playerDataRelPath returns the data-relative, slash-separated form of path +// when it is an account or character file, which is the form the game's +// online-session check (AdminPlayerFileOnline) accepts. +func (us *UndoStack) playerDataRelPath(path string) (string, bool) { + if path == "" { + return "", false + } + rel, err := filepath.Rel(us.dataDir, path) + if err != nil { + return "", false + } + rel = filepath.ToSlash(rel) + if strings.HasPrefix(rel, "players/accounts/") || strings.HasPrefix(rel, "players/characters/") { + return rel, true + } + return "", false +} + func (us *UndoStack) Info() UndoInfo { us.mu.Lock() defer us.mu.Unlock() @@ -395,8 +505,8 @@ func (us *UndoStack) save() { extra = append(extra, extraEntry{ FilePath: ef.FilePath, NewFilePath: ef.NewFilePath, - OldContent: string(ef.OldContent), - NewContent: string(ef.NewContent), + OldContent: string(us.redactContent(ef.OldContent, ef.FilePath, ef.NewFilePath)), + NewContent: string(us.redactContent(ef.NewContent, ef.FilePath, ef.NewFilePath)), IsDelete: ef.IsDelete, IsCreate: ef.IsCreate, IsDirDelete: ef.IsDirDelete, @@ -407,8 +517,8 @@ func (us *UndoStack) save() { Description: c.Description, FilePath: c.FilePath, NewFilePath: c.NewFilePath, - OldContent: string(c.OldContent), - NewContent: string(c.NewContent), + OldContent: string(us.redactContent(c.OldContent, c.FilePath, c.NewFilePath)), + NewContent: string(us.redactContent(c.NewContent, c.FilePath, c.NewFilePath)), IsDelete: c.IsDelete, IsCreate: c.IsCreate, IsDirDelete: c.IsDirDelete, @@ -427,9 +537,124 @@ func (us *UndoStack) save() { log.Printf("undo: failed to marshal history: %v", err) return } - if err := os.WriteFile(us.filePath, b, 0644); err != nil { + if err := os.WriteFile(us.filePath, b, 0600); err != nil { log.Printf("undo: failed to write history file: %v", err) + return + } + // os.WriteFile does not change an existing file's mode: tighten a history + // file that a previous version created world-readable. + if err := os.Chmod(us.filePath, 0600); err != nil { + log.Printf("undo: failed to secure history file: %v", err) + } +} + +// playerSecretKeys are the top-level YAML keys whose values must never be +// written to the on-disk undo history. Account password hashes are the +// primary case: the raw file editor exposes account files and an unredacted +// history file would leak every hash to any local reader (and be stageable +// into the repository). +var playerSecretKeys = []string{"password_hash"} + +// redactContent replaces secret values with a placeholder before the content +// is serialized into the history file. The in-memory ChangeDesc keeps the +// real bytes; only the persisted copy is redacted. A redacted value cannot +// restore a deleted account's hash across a restart, which is an accepted +// trade for never storing credentials in the history. +func (us *UndoStack) redactContent(content []byte, paths ...string) []byte { + if content == nil { + return nil + } + for _, p := range paths { + if us.isPlayerDataPath(p) { + return redactPlayerSecrets(content) + } + } + return content +} + +// redactPlayerSecrets replaces every known secret value in a player/account +// document with the editor's placeholder, returning content unchanged when +// the document does not parse or carries no secret. +func redactPlayerSecrets(content []byte) []byte { + m, err := yamlToMap(content) + if err != nil { + return content + } + changed := false + for _, key := range playerSecretKeys { + if _, ok := m[key]; ok { + m[key] = redactedAccountHash + changed = true + } + } + if !changed { + return content + } + out, err := marshalMapYAML(m) + if err != nil { + return content + } + return out +} + +// restoreSecret merges the live on-disk password hash into content before an +// undo/redo overwrites an account file, so a hash that was redacted in the +// persisted history can never blank a live account after a restart. It looks +// at the primary target path and then the change's other path (a rename). +// +// When no path holds a live hash and content still carries the redaction +// placeholder, it returns an error: writing the placeholder would replace the +// account's real hash and lock both the owner and the raw editor out of it +// permanently, so undo/redo must report the failure instead of overwriting. +func (us *UndoStack) restoreSecret(content []byte, paths ...string) ([]byte, error) { + if content == nil { + return nil, nil + } + playerFile := false + for _, p := range paths { + if us.isPlayerDataPath(p) { + playerFile = true + break + } + } + if !playerFile { + return content, nil + } + cm, err := yamlToMap(content) + if err != nil { + return content, nil + } + hash, ok := cm["password_hash"] + if !ok { + return content, nil + } + for _, p := range paths { + if !us.isPlayerDataPath(p) { + continue + } + existing, err := os.ReadFile(p) + if err != nil { + continue + } + em, err := yamlToMap(existing) + if err != nil { + continue + } + live, ok := em["password_hash"] + if !ok { + continue + } + cm["password_hash"] = live + out, err := marshalMapYAML(cm) + if err != nil { + return content, nil + } + return out, nil + } + if fmt.Sprint(hash) == redactedAccountHash { + return nil, fmt.Errorf("the stored history image has a redacted password hash and no live account file to take it from") } + return content, nil } // load restores persisted undo/redo history written by save. A missing or -- cgit v1.2.3