fix: close map OOM, admin traversal, transport safespot, room rekey, and legacy payout recovery

6 files changed, 174 insertions, 27 deletions

diff --git a/internal/game/cmd_map.go b/internal/game/cmd_map.go
index fc4ebf1..e84d1ce 100644
--- a/internal/game/cmd_map.go
+++ b/internal/game/cmd_map.go
@@ -2,14 +2,22 @@ package game
 
 import (
 	"thehouseoficarus/internal/net"
+	"thehouseoficarus/internal/player"
 )
 
 func (g *Game) executeMap(sess *net.Session, args []string, rawInput string) {
 	g.doMap(sess)
 }
 
-func (g *Game) doMap(sess *net.Session) {
-	p := sess.Player
+// maxMapDimension bounds the grid a player may request. buildFullMap
+// allocates mapHeight x mapWidth cells, so an unbounded option value would let
+// any authenticated player trigger a fatal (unrecoverable) out-of-memory
+// allocation that takes the whole server down. The option parser accepts any
+// int and the value persists in the account file, so the clamp must live here.
+const maxMapDimension = 200
+
+// mapDimensions returns the clamped map grid size requested by the player.
+func mapDimensions(p *player.Player) (int, int) {
 	mapWidth := p.OptionInt("map_width")
 	mapHeight := p.OptionInt("map_height")
 	if mapWidth < 5 {
@@ -18,6 +26,18 @@ func (g *Game) doMap(sess *net.Session) {
 	if mapHeight < 5 {
 		mapHeight = 5
 	}
+	if mapWidth > maxMapDimension {
+		mapWidth = maxMapDimension
+	}
+	if mapHeight > maxMapDimension {
+		mapHeight = maxMapDimension
+	}
+	return mapWidth, mapHeight
+}
+
+func (g *Game) doMap(sess *net.Session) {
+	p := sess.Player
+	mapWidth, mapHeight := mapDimensions(p)
 
 	lines := buildFullMap(g, sess, p.RoomID, mapWidth, mapHeight, mapGlyphsForPlayer(p.OptionBool("unicode")))
 
diff --git a/internal/game/cmd_trigger_transport.go b/internal/game/cmd_trigger_transport.go
index f6c9c77..1aa3089 100644
--- a/internal/game/cmd_trigger_transport.go
+++ b/internal/game/cmd_trigger_transport.go
@@ -44,6 +44,14 @@ func (g *Game) executeTransportEffect(sess *net.Session, p *player.Player, mod *
 		}
 	}
 
+	// A transport changes rooms just like a walk or teleport, so it must
+	// clear any safespot cover: the safespot predicates are room-agnostic,
+	// so leaving the state active would grant melee/hazard immunity in the
+	// destination and keep the player as a phantom occupant of the old cover.
+	if ss, ok := g.safespot.Get(p.Name); ok {
+		g.forceLeaveSafespot(sess, p, &ss, "")
+	}
+
 	sess.WriteLine(fmt.Sprintf("You activate %s...", mod.Name))
 
 	p.RoomID = mod.Destination
@@ -53,6 +61,12 @@ func (g *Game) executeTransportEffect(sess *net.Session, p *player.Player, mod *
 		g.Hub.EnterRoom(sess, p.RoomID)
 	}
 
+	// Seed the destination exactly as the other teleport paths do, or the
+	// room's mobs, objects and ground spawns never appear.
+	g.World.SeedGroundItems(p.RoomID)
+	g.seedRoomMobs(p.RoomID)
+	g.seedRoomObjects(p.RoomID)
+
 	room, _ := g.World.LoadRoom(p.RoomID)
 	destName := fmt.Sprintf("room %d", p.RoomID)
 	if room != nil {
@@ -61,4 +75,6 @@ func (g *Game) executeTransportEffect(sess *net.Session, p *player.Player, mod *
 	sess.WriteLine(fmt.Sprintf("You arrive at %s.", destName))
 	g.saveCharacter(p)
 	g.doLook(sess)
+	g.runEnterSteps(sess, p.RoomID)
+	g.writePrompt(sess)
 }
diff --git a/internal/game/economy.go b/internal/game/economy.go
index 3505a86..9ce4404 100644
--- a/internal/game/economy.go
+++ b/internal/game/economy.go
@@ -885,6 +885,24 @@ func pruneOpenPayouts(j *economyJournal, p *player.Player) bool {
 //     written later proves the round resolved (a win/push/refund already
 //     returned the stake inside that payout) → settled with no refund.
 //     Anything else stays open for operator inspection.
+// reanchorOwedPayout rewrites an open payout record's checkpoints to the
+// current durable balance so a later recovery reads it as still owed (the
+// balance sits at the record's before-checkpoint). It is used when a recovery
+// run could not pay every sibling in an identical-signature group: the paid
+// siblings moved the durable balance, so leaving the unpaid remainder at its
+// original checkpoints would let a later run mistake the balance for its own
+// durability and settle the liability without paying it. The post-checkpoint
+// respects the economy cap: when the credit cannot fit, before == after, so
+// the record keeps reading as owed at this balance instead of claiming an
+// impossible post-state.
+func reanchorOwedPayout(rec *economyRecord, credits, credit int) {
+	rec.CreditsBefore = credits
+	rec.CreditsAfter = credits
+	if after, err := addCreditsChecked(credits, credit); err == nil {
+		rec.CreditsAfter = after
+	}
+}
+
 func (g *Game) recoverEconomy() {
 	if g.Economy == nil {
 		return
@@ -1099,35 +1117,29 @@ func (g *Game) recoverEconomy() {
 				settled[other.ID] = true
 				continue
 			}
-			if capBlocked {
-				// The credit cap blocked re-application. Re-anchor the
-				// record's checkpoints to the current durable balance so
-				// a later recovery re-applies it against a balance it can
-				// actually be paid from — without this, the balance a PAID
-				// sibling moved reads as this record's own durability and
-				// the liability is silently settled instead of paid.
-				// The re-anchored post-state must respect the cap: when it
-				// would exceed it, no reachable balance can pay the record,
-				// so the current balance is recorded as both checkpoints —
-				// the record then keeps reading as owed at this balance
-				// (retrying the re-application) instead of claiming an
-				// impossible post-state that a coincidental balance could
-				// be mistaken for as this record's own durability.
-				other.CreditsBefore = p.Credits
-				other.CreditsAfter = p.Credits
-				if after, err := addCreditsChecked(p.Credits, other.Credit); err == nil {
-					other.CreditsAfter = after
-				}
+			if capBlocked || paid > 0 {
+				// Either the credit cap blocked re-application, or an
+				// earlier sibling WAS paid before this record's save failed
+				// transiently. In both cases the durable balance already
+				// moved away from this record's original before-checkpoint,
+				// so re-anchor the checkpoints to the current durable balance
+				// — without this, the balance a PAID sibling moved reads as
+				// this record's own durability and the liability is silently
+				// settled instead of paid (and compaction then erases the
+				// settled sibling evidence, so a later run sees len(group)==1
+				// and settles the remainder as "already durable").
+				reanchorOwedPayout(other, p.Credits, other.Credit)
 				settled[other.ID] = true // handled for the rest of this run
 				_ = g.Economy.append(*other)
 				continue
 			}
-			// A TRANSIENT save failure blocked re-application and the
-			// balance still has room below the cap: keep the record open
-			// with its ORIGINAL checkpoints. Re-anchoring here would make
-			// the record unreadable against the unchanged durable balance
-			// (before-checkpoint mismatch) and permanently ambiguous; the
-			// original checkpoints let a later recovery retry cleanly.
+			// A TRANSIENT save failure blocked the FIRST re-application
+			// (paid == 0) and the balance still sits at the original
+			// before-checkpoint: keep the record open with its ORIGINAL
+			// checkpoints so a later recovery reads it as owed and retries
+			// cleanly. Re-anchoring here would make the record unreadable
+			// against the unchanged durable balance (before-checkpoint
+			// mismatch) and permanently ambiguous.
 			settled[other.ID] = true // handled for the rest of this run
 		}
 	}
diff --git a/internal/game/economy_reanchor_test.go b/internal/game/economy_reanchor_test.go
new file mode 100644
index 0000000..86d7bf1
--- /dev/null
+++ b/internal/game/economy_reanchor_test.go
@@ -0,0 +1,31 @@
+package game
+
+import (
+	"testing"
+
+	"thehouseoficarus/internal/casino"
+)
+
+// TestReanchorOwedPayoutReadsAsOwed: when recovery cannot pay every sibling in
+// an identical-signature payout group, the unpaid remainder must be re-anchored
+// to the current durable balance. Otherwise a later run (after compaction drops
+// the settled sibling evidence) reads the balance as the record's own
+// durability and settles the liability without paying it.
+func TestReanchorOwedPayoutReadsAsOwed(t *testing.T) {
+	rec := &economyRecord{Credit: 20, CreditsBefore: 90, CreditsAfter: 110}
+	// One sibling was paid: the durable balance now sits at 110.
+	reanchorOwedPayout(rec, 110, rec.Credit)
+	if rec.CreditsBefore != 110 || rec.CreditsAfter != 130 {
+		t.Fatalf("re-anchored checkpoints = before %d after %d, want before 110 after 130", rec.CreditsBefore, rec.CreditsAfter)
+	}
+
+	// At the economy cap the post-checkpoint must stay reachable: before ==
+	// after so the record keeps reading as owed instead of claiming an
+	// impossible balance that a coincidental balance could be mistaken for.
+	capped := &economyRecord{Credit: 20}
+	reanchorOwedPayout(capped, casino.MaxCredits, capped.Credit)
+	if capped.CreditsBefore != casino.MaxCredits || capped.CreditsAfter != casino.MaxCredits {
+		t.Fatalf("cap re-anchor = before %d after %d, want both %d",
+			capped.CreditsBefore, capped.CreditsAfter, casino.MaxCredits)
+	}
+}
diff --git a/internal/game/map_bounds_regression_test.go b/internal/game/map_bounds_regression_test.go
new file mode 100644
index 0000000..dd3423b
--- /dev/null
+++ b/internal/game/map_bounds_regression_test.go
@@ -0,0 +1,34 @@
+package game
+
+import (
+	"testing"
+
+	"thehouseoficarus/internal/player"
+)
+
+// TestMapDimensionsClamped: the player-settable map size drives a direct
+// mapHeight x mapWidth allocation in buildFullMap, so an unbounded option
+// value is a fatal out-of-memory kill reachable by any authenticated player
+// (and persisted in the account file, so it repeats on login). The clamp must
+// cap both dimensions while leaving ordinary values alone.
+func TestMapDimensionsClamped(t *testing.T) {
+	p := player.New("mapper")
+
+	p.Options = map[string]any{"map_width": 100000, "map_height": 100000}
+	w, h := mapDimensions(p)
+	if w != maxMapDimension || h != maxMapDimension {
+		t.Fatalf("huge map dimensions = (%d,%d), want (%d,%d)", w, h, maxMapDimension, maxMapDimension)
+	}
+
+	p.Options = map[string]any{"map_width": -5, "map_height": 0}
+	w, h = mapDimensions(p)
+	if w != 5 || h != 5 {
+		t.Fatalf("undersized map dimensions = (%d,%d), want (5,5)", w, h)
+	}
+
+	p.Options = map[string]any{"map_width": 30, "map_height": 20}
+	w, h = mapDimensions(p)
+	if w != 30 || h != 20 {
+		t.Fatalf("normal map dimensions = (%d,%d), want (30,20)", w, h)
+	}
+}
diff --git a/internal/game/transport_safespot_regression_test.go b/internal/game/transport_safespot_regression_test.go
new file mode 100644
index 0000000..fe77994
--- /dev/null
+++ b/internal/game/transport_safespot_regression_test.go
@@ -0,0 +1,34 @@
+package game
+
+import "testing"
+
+// TestTransportClearsSafespot: the safespot predicates are room-agnostic, so a
+// transport that changes rooms without clearing cover grants melee/hazard
+// immunity in the destination and leaves a phantom occupant in the old room.
+// Every other teleport path clears it; the transport effect must too.
+func TestTransportClearsSafespot(t *testing.T) {
+	h := newHarness(t)
+	h.seedAccount("alice")
+	h.seedCharacter("alice", "alice")
+	sess, _ := h.connect("alice")
+
+	h.g.RunOnOwner(func() {
+		p := sess.Player
+		h.g.safespot.Set(p.Name, SafespotState{Active: true, ObjectDefID: "rock", ObjectIndex: 0, RoomID: p.RoomID})
+		if !h.g.isSafespotted(p.Name) {
+			t.Fatal("safespot not active before transport")
+		}
+
+		h.g.executeTransportEffect(sess, p, &ModDef{ID: "test_transport", Name: "test shuttle", Destination: 2})
+
+		if h.g.safespot.Has(p.Name) {
+			t.Fatal("safespot state survived transport; hazard/melee immunity persists in the destination")
+		}
+		if h.g.isSafespotted(p.Name) {
+			t.Fatal("player still counts as safespotted after transport")
+		}
+		if p.RoomID != 2 {
+			t.Fatalf("room after transport = %d, want 2", p.RoomID)
+		}
+	})
+}