fix: harden engine across casino, economy, concurrency, security, and content

1 files changed, 262 insertions, 0 deletions

diff --git a/internal/admin/correct_undo_account_image_test.go b/internal/admin/correct_undo_account_image_test.go
new file mode 100644
index 0000000..51fe037
--- /dev/null
+++ b/internal/admin/correct_undo_account_image_test.go
@@ -0,0 +1,262 @@
+package admin
+
+// Regression tests for the undo-history account-file paths re-verified in the
+// infra pass:
+//   - a history image whose password hash was redacted on disk cannot be
+//     written over an account after a restart (login could never repair the
+//     placeholder, and the raw editor copies it back)
+//   - undo/redo refuses a character/account file whose owner is online, the
+//     same refusal the raw file editor applies
+//   - the refusal is scoped to player files: other data files undo normally
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const (
+	testAccountHash    = "argon2id:1:65536:2:c2FsdHNhbHRzYWx0c2E:deadbeef"
+	testAccountLive    = "name: bob\npassword_hash: " + testAccountHash + "\nadmin: false\n"
+	testAccountPromote = "name: bob\npassword_hash: " + testAccountHash + "\nadmin: true\n"
+)
+
+func writeAccountFile(t *testing.T, path, content string) {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(path, []byte(content), 0600); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// TestUndoOfRedactedAccountImageDoesNotClobberAccount drives the restart the
+// history file exists to survive: Push redacts the persisted image, a fresh
+// UndoStack loads that redacted image, and the undo must then fail rather than
+// write "[redacted]" into a live account.
+func TestUndoOfRedactedAccountImageDoesNotClobberAccount(t *testing.T) {
+	t.Run("delete of a removed account fails without creating it", func(t *testing.T) {
+		dir := t.TempDir()
+		accPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+		writeAccountFile(t, accPath, testAccountLive)
+
+		us := NewUndoStack(dir)
+		us.Push(ChangeDesc{
+			Description: "delete bob",
+			FilePath:    accPath,
+			OldContent:  []byte(testAccountLive),
+			IsDelete:    true,
+		})
+		if err := os.Remove(accPath); err != nil {
+			t.Fatal(err)
+		}
+		raw, err := os.ReadFile(filepath.Join(dir, ".admin_history.json"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !strings.Contains(string(raw), redactedAccountHash) {
+			t.Fatalf("history file did not redact the hash:\n%s", raw)
+		}
+
+		restarted := NewUndoStack(dir)
+		if ch := restarted.Undo(); ch != nil {
+			t.Fatalf("undo wrote a redacted account image: %s", ch.ShortDesc())
+		}
+		if got, err := os.ReadFile(accPath); err == nil {
+			t.Fatalf("undo recreated the account from the redacted image:\n%s", got)
+		} else if !os.IsNotExist(err) {
+			t.Fatalf("stat recreated account: %v", err)
+		}
+	})
+
+	t.Run("rename restores the real hash from the renamed file", func(t *testing.T) {
+		dir := t.TempDir()
+		oldPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+		newPath := filepath.Join(dir, "players", "accounts", "robert.yaml")
+		writeAccountFile(t, oldPath, testAccountLive)
+
+		us := NewUndoStack(dir)
+		us.Push(ChangeDesc{
+			Description: "rename bob",
+			FilePath:    oldPath,
+			NewFilePath: newPath,
+			OldContent:  []byte(testAccountLive),
+			NewContent:  []byte(testAccountLive),
+		})
+		if err := os.Rename(oldPath, newPath); err != nil {
+			t.Fatal(err)
+		}
+
+		restarted := NewUndoStack(dir)
+		if ch := restarted.Undo(); ch == nil {
+			t.Fatal("undo of a rename must restore the account from its live renamed copy")
+		}
+		got, err := os.ReadFile(oldPath)
+		if err != nil {
+			t.Fatalf("undo did not restore the old path: %v", err)
+		}
+		if !strings.Contains(string(got), testAccountHash) {
+			t.Fatalf("restored account lost its hash:\n%s", got)
+		}
+		if strings.Contains(string(got), redactedAccountHash) {
+			t.Fatalf("restored account carries the redaction placeholder:\n%s", got)
+		}
+		if _, err := os.Stat(newPath); !os.IsNotExist(err) {
+			t.Fatalf("undo left the renamed file behind: %v", err)
+		}
+	})
+
+	t.Run("rename whose only copy is gone fails before removing anything", func(t *testing.T) {
+		dir := t.TempDir()
+		oldPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+		newPath := filepath.Join(dir, "players", "accounts", "robert.yaml")
+
+		us := NewUndoStack(dir)
+		us.Push(ChangeDesc{
+			Description: "rename bob",
+			FilePath:    oldPath,
+			NewFilePath: newPath,
+			OldContent:  []byte(testAccountLive),
+			NewContent:  []byte(testAccountLive),
+		})
+
+		restarted := NewUndoStack(dir)
+		if ch := restarted.Undo(); ch != nil {
+			t.Fatalf("undo wrote a redacted account image: %s", ch.ShortDesc())
+		}
+		if _, err := os.Stat(oldPath); !os.IsNotExist(err) {
+			t.Fatalf("failed undo created the account from the redacted image: %v", err)
+		}
+	})
+
+	t.Run("edit keeps the live hash", func(t *testing.T) {
+		dir := t.TempDir()
+		accPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+		writeAccountFile(t, accPath, testAccountPromote)
+
+		us := NewUndoStack(dir)
+		us.Push(ChangeDesc{
+			Description: "make bob an admin",
+			FilePath:    accPath,
+			OldContent:  []byte(testAccountLive),
+			NewContent:  []byte(testAccountPromote),
+		})
+		// The live file still holds the real hash after the restart below, so
+		// the undo has one to merge and must keep working.
+		restarted := NewUndoStack(dir)
+		if ch := restarted.Undo(); ch == nil {
+			t.Fatal("undo of a redacted edit must still apply while the live account file exists")
+		}
+		got, err := os.ReadFile(accPath)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !strings.Contains(string(got), testAccountHash) {
+			t.Fatalf("undo did not merge the live hash into the restored image:\n%s", got)
+		}
+		if !strings.Contains(string(got), "admin: false") {
+			t.Fatalf("undo did not restore the pre-edit image:\n%s", got)
+		}
+	})
+}
+
+// TestUndoRedoRefusesOnlinePlayerFile wires the online check the way main.go
+// does and pins that undo and redo both consult it for a live account, and
+// that the check receives the data-relative path form the game expects.
+func TestUndoRedoRefusesOnlinePlayerFile(t *testing.T) {
+	dir := t.TempDir()
+	accPath := filepath.Join(dir, "players", "accounts", "alice.yaml")
+	writeAccountFile(t, accPath, testAccountLive)
+
+	s := &AdminServer{dataDir: dir, undoStack: NewUndoStack(dir)}
+	var seen []string
+	online := true
+	s.SetPlayerFileOnlineCheck(func(relPath string) bool {
+		seen = append(seen, relPath)
+		return online && relPath == "players/accounts/alice.yaml"
+	})
+
+	s.undoStack.Push(ChangeDesc{
+		Description: "make alice an admin",
+		FilePath:    accPath,
+		OldContent:  []byte(testAccountLive),
+		NewContent:  []byte(testAccountPromote),
+	})
+	writeAccountFile(t, accPath, testAccountPromote)
+
+	if ch := s.undoStack.Undo(); ch != nil {
+		t.Fatalf("undo overwrote an online account file: %s", ch.ShortDesc())
+	}
+	if got, _ := os.ReadFile(accPath); string(got) != testAccountPromote {
+		t.Fatalf("online account file was modified:\n%s", got)
+	}
+	if len(seen) == 0 || seen[0] != "players/accounts/alice.yaml" {
+		t.Fatalf("online check paths = %v, want the data-relative account path first", seen)
+	}
+
+	// Logging the character out clears the refusal, and the redo of the same
+	// change is refused when it is online again. A successful undo rewrites
+	// the mapping (so its key order differs), hence the byte snapshot rather
+	// than a comparison against the original text.
+	online = false
+	if ch := s.undoStack.Undo(); ch == nil {
+		t.Fatal("undo still refused after the character logged out")
+	}
+	undone, err := os.ReadFile(accPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, want := range []string{"admin: false", testAccountHash} {
+		if !strings.Contains(string(undone), want) {
+			t.Fatalf("undo did not apply %q after logout:\n%s", want, undone)
+		}
+	}
+
+	online = true
+	if ch := s.undoStack.Redo(); ch != nil {
+		t.Fatalf("redo overwrote an online account file: %s", ch.ShortDesc())
+	}
+	if got, _ := os.ReadFile(accPath); string(got) != string(undone) {
+		t.Fatalf("redo modified an online account file:\n%s", got)
+	}
+	online = false
+	if ch := s.undoStack.Redo(); ch == nil {
+		t.Fatal("redo still refused after the character logged out")
+	}
+}
+
+// TestUndoOfNonPlayerFileIgnoresOnlineCheck: the refusal is scoped to player
+// files, so a hook that reports every path online cannot block room or item
+// undo.
+func TestUndoOfNonPlayerFileIgnoresOnlineCheck(t *testing.T) {
+	dir := t.TempDir()
+	roomPath := filepath.Join(dir, "rooms", "1.yaml")
+	if err := os.MkdirAll(filepath.Dir(roomPath), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(roomPath, []byte("id: 1\nname: edited\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	us := NewUndoStack(dir)
+	us.SetPlayerFileOnlineCheck(func(string) bool { return true })
+	us.Push(ChangeDesc{
+		Description: "edit room",
+		FilePath:    roomPath,
+		OldContent:  []byte("id: 1\nname: old\n"),
+		NewContent:  []byte("id: 1\nname: edited\n"),
+	})
+
+	if ch := us.Undo(); ch == nil {
+		t.Fatal("undo of a non-player file was refused by the online check")
+	}
+	got, err := os.ReadFile(roomPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(got), "name: old") {
+		t.Fatalf("undo did not revert the room file:\n%s", got)
+	}
+}