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

1 files changed, 162 insertions, 0 deletions

diff --git a/internal/game/hardening_owner_panic_test.go b/internal/game/hardening_owner_panic_test.go
new file mode 100644
index 0000000..aa1070b
--- /dev/null
+++ b/internal/game/hardening_owner_panic_test.go
@@ -0,0 +1,162 @@
+package game
+
+// Crash-safety regressions for the owner goroutine and the account-menu
+// handlers. The owner goroutine is the single execution authority for live
+// game state, so an unrecovered panic there (from command parsing, a tick
+// system, or a session whose account/player has gone away) takes down every
+// session and all unsaved state. These tests pin both halves of the fix: the
+// loop contains panics, and the menu handlers refuse to dereference a nil
+// account.
+
+import (
+	"strings"
+	"testing"
+
+	"thehouseoficarus/internal/net"
+	"thehouseoficarus/internal/player"
+)
+
+// TestOwnerLoopSurvivesPanickingTask proves a panicking task is contained and
+// counted, and that the loop keeps draining the queue afterwards.
+func TestOwnerLoopSurvivesPanickingTask(t *testing.T) {
+	h := newHarness(t)
+
+	before := h.g.owner.panics.Load()
+	h.g.RunOnOwner(func() { panic("boom") })
+	// Game.run releases its caller from the task's OWN defer (close(task)),
+	// which unwinds before runTask's deferred recover runs. The panic counter
+	// is incremented by that recover, so reading it right here could observe
+	// before+0. Submit a no-op task and wait for it: the owner drains tasks in
+	// FIFO order, so the panicking task's runTask — recover and counter
+	// increment included — has fully settled before the no-op task executes.
+	h.barrier()
+	if got := h.g.owner.panics.Load(); got != before+1 {
+		t.Fatalf("panic counter = %d, want %d", got, before+1)
+	}
+
+	ran := false
+	h.g.RunOnOwner(func() { ran = true })
+	if !ran {
+		t.Fatal("owner loop did not execute a task submitted after a panic")
+	}
+}
+
+// TestOwnerSessionInputPanicDoesNotKillLoop reproduces the production crash
+// path: a session in StateGame with no Player (a departed session whose
+// connection goroutine is still dispatching buffered input) dereferences a nil
+// Player. The owner must log and contain the panic instead of taking the
+// process down with it.
+func TestOwnerSessionInputPanicDoesNotKillLoop(t *testing.T) {
+	h := newHarness(t)
+	sess := &net.Session{Conn: newChanConn()}
+	sess.SetState(net.StateGame)
+
+	h.g.HandleSessionOnOwner(sess, "look")
+
+	if got := h.g.owner.panics.Load(); got == 0 {
+		t.Fatal("expected the nil-Player dispatch to panic and be contained")
+	}
+	h.barrier() // the owner loop is still alive and serving tasks
+}
+
+// assertResetToAccountName requires a session to be back at the account-name
+// prompt with no account bound.
+func assertResetToAccountName(t *testing.T, sess *net.Session, conn *chanConn) {
+	t.Helper()
+	if got := sess.State(); got != net.StateAccountName {
+		t.Fatalf("state = %v, want StateAccountName", got)
+	}
+	if sess.Account != nil {
+		t.Fatalf("account not cleared: %+v", sess.Account)
+	}
+	if out := string(conn.Bytes()); !strings.Contains(out, "Account name: ") {
+		t.Fatalf("missing account-name prompt; output = %q", out)
+	}
+}
+
+// TestNilAccountHandlersResetToAccountName drives every menu-state handler with
+// a session whose account is nil (purged, or republished from another session)
+// and requires each to fall back to the account-name prompt instead of
+// dereferencing the nil account. The handlers run on the test goroutine so an
+// unguarded dereference fails loudly here rather than being absorbed by the
+// owner loop's panic barrier.
+func TestNilAccountHandlersResetToAccountName(t *testing.T) {
+	h := newHarness(t)
+
+	cases := []struct {
+		name  string
+		state net.SessionState
+		input string
+		run   func(*Game, *net.Session, string)
+	}{
+		{"menu-connect", net.StateMenu, "c", (*Game).handleMenu},
+		{"menu-list", net.StateMenu, "l", (*Game).handleMenu},
+		{"menu-new", net.StateMenu, "n", (*Game).handleMenu},
+		{"menu-rename-account", net.StateMenu, "a", (*Game).handleMenu},
+		{"menu-rename-char", net.StateMenu, "r", (*Game).handleMenu},
+		{"menu-delete-char", net.StateMenu, "d", (*Game).handleMenu},
+		{"menu-purge", net.StateMenu, "p", (*Game).handleMenu},
+		{"menu-default", net.StateMenu, "zzz", (*Game).handleMenu},
+		{"color-choice", net.StateColorChoice, "y", (*Game).handleColorChoice},
+		{"new-account-confirm", net.StateNewAccount, "pw", (*Game).handleNewAccount},
+		{"new-char-name", net.StateNewCharName, "bob", (*Game).handleNewCharName},
+		{"rename-account", net.StateRenameAccount, "bob", (*Game).handleRenameAccount},
+		{"rename-char", net.StateRenameChar, "1", (*Game).handleRenameChar},
+		{"rename-char-name", net.StateRenameCharName, "bob", (*Game).handleRenameCharName},
+		{"delete-char", net.StateDeleteChar, "1", (*Game).handleDeleteChar},
+		{"purge-account", net.StatePurgeAccount, "PURGE bob", (*Game).handlePurgeAccount},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			conn := newChanConn()
+			sess := &net.Session{Conn: conn}
+			sess.SetState(tc.state)
+			tc.run(h.g, sess, tc.input)
+			assertResetToAccountName(t, sess, conn)
+		})
+	}
+
+	t.Run("show-menu", func(t *testing.T) {
+		conn := newChanConn()
+		sess := &net.Session{Conn: conn}
+		sess.SetState(net.StateGame)
+		h.g.showMenu(sess)
+		assertResetToAccountName(t, sess, conn)
+	})
+
+	t.Run("connect-character", func(t *testing.T) {
+		conn := newChanConn()
+		sess := &net.Session{Conn: conn}
+		sess.SetState(net.StateMenu)
+		h.g.connectCharacter(sess, "bob")
+		assertResetToAccountName(t, sess, conn)
+	})
+
+	t.Run("apply-password-auth", func(t *testing.T) {
+		conn := newChanConn()
+		sess := &net.Session{Conn: conn}
+		sess.SetState(net.StatePassword)
+		h.g.applyPasswordAuth(sess, passwordAuth{ok: true, stored: "stale-hash"})
+		assertResetToAccountName(t, sess, conn)
+	})
+}
+
+// TestCommandsTolerateNilAccount covers the in-game command handlers that
+// persist to the account: with the account gone they must still work for the
+// live player instead of faulting on the nil account.
+func TestCommandsTolerateNilAccount(t *testing.T) {
+	h := newHarness(t)
+	sess := &net.Session{Conn: newChanConn(), Player: player.New("alice")}
+	sess.SetState(net.StateGame)
+
+	h.g.doOption(sess, "unicode off")
+	if sess.Player.Options["unicode"] != false {
+		t.Fatalf("unicode option not applied to the player: %v", sess.Player.Options["unicode"])
+	}
+
+	h.g.doColor(sess, "room_name D0")
+	h.g.saveAccountColor(sess, "room_name", "D0")
+	h.g.doAlias(sess, []string{"x", "look"})
+	h.g.doUnalias(sess, []string{"x"})
+}