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

1 files changed, 201 insertions, 0 deletions

diff --git a/internal/game/hardening_account_name_validation_test.go b/internal/game/hardening_account_name_validation_test.go
new file mode 100644
index 0000000..7fca0ab
--- /dev/null
+++ b/internal/game/hardening_account_name_validation_test.go
@@ -0,0 +1,201 @@
+package game
+
+// Regression coverage for name validation in the account/character lifecycle
+// transactions: every caller-supplied (or account-file-supplied) name is
+// joined into AccountPath/CharPath, so a path-like name must be rejected
+// before the first join instead of deleting or moving a file outside the
+// players/ tree.
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// writeTraversalCanary creates a file that a path-traversal join would target,
+// so a test can prove the transaction never touched it.
+func writeTraversalCanary(t *testing.T, path string) {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(path, []byte("canary\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func assertTraversalCanary(t *testing.T, path string) {
+	t.Helper()
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("canary %s was removed: %v", path, err)
+	}
+	if string(data) != "canary\n" {
+		t.Fatalf("canary %s was overwritten: %q", path, data)
+	}
+}
+
+func TestPurgeAccountTxnRejectsPathLikeNames(t *testing.T) {
+	h := newHarness(t)
+	h.seedAccount("alice")
+
+	// CharPath("../../escape") would resolve to <dataDir>/escape.yaml.
+	charCanary := filepath.Join(h.dir, "escape.yaml")
+	writeTraversalCanary(t, charCanary)
+
+	err := h.g.purgeAccountTxn("alice", []string{"../../escape"})
+	if err == nil {
+		t.Fatal("purge accepted a path-traversing character name")
+	}
+	if !strings.Contains(err.Error(), "invalid character name") {
+		t.Fatalf("unclear purge error: %v", err)
+	}
+	assertTraversalCanary(t, charCanary)
+	if !h.g.AccountStore.AccountExists("alice") {
+		t.Fatal("purge ran (account file removed) despite the invalid character name")
+	}
+
+	// AccountPath("../alice") would resolve to <dataDir>/players/alice.yaml.
+	accountCanary := filepath.Join(h.dir, "players", "alice.yaml")
+	writeTraversalCanary(t, accountCanary)
+
+	err = h.g.purgeAccountTxn("../alice", nil)
+	if err == nil {
+		t.Fatal("purge accepted a path-traversing account name")
+	}
+	if !strings.Contains(err.Error(), "invalid account name") {
+		t.Fatalf("unclear purge error: %v", err)
+	}
+	assertTraversalCanary(t, accountCanary)
+	if !h.g.AccountStore.AccountExists("alice") {
+		t.Fatal("purge removed the real account despite the invalid account name")
+	}
+}
+
+func TestRenameAccountTxnRejectsPathLikeNames(t *testing.T) {
+	h := newHarness(t)
+	h.seedAccount("dave")
+
+	// AccountPath("../dave") would resolve to <dataDir>/players/dave.yaml.
+	accountCanary := filepath.Join(h.dir, "players", "dave.yaml")
+	writeTraversalCanary(t, accountCanary)
+
+	err := h.g.renameAccountTxn("dave", "../dave")
+	if err == nil {
+		t.Fatal("rename accepted a path-traversing new account name")
+	}
+	if !strings.Contains(err.Error(), "invalid account name") {
+		t.Fatalf("unclear rename error: %v", err)
+	}
+	assertTraversalCanary(t, accountCanary)
+	if !h.g.AccountStore.AccountExists("dave") {
+		t.Fatal("rename moved the account file despite the invalid new name")
+	}
+
+	// A path-like entry in the account's own character list is rejected too:
+	// the list is joined into CharPath while re-pointing owners.
+	acc, err := h.g.AccountStore.LoadAccount("dave")
+	if err != nil {
+		t.Fatal(err)
+	}
+	acc.Characters = []string{"../../escape"}
+	if err := h.g.AccountStore.SaveAccount(acc); err != nil {
+		t.Fatal(err)
+	}
+	charCanary := filepath.Join(h.dir, "escape.yaml")
+	writeTraversalCanary(t, charCanary)
+
+	err = h.g.renameAccountTxn("dave", "dave2")
+	if err == nil {
+		t.Fatal("rename accepted an account whose character list is path-like")
+	}
+	if !strings.Contains(err.Error(), "invalid character name") {
+		t.Fatalf("unclear rename error: %v", err)
+	}
+	assertTraversalCanary(t, charCanary)
+	if !h.g.AccountStore.AccountExists("dave") {
+		t.Fatal("rename moved the account file despite the invalid character list")
+	}
+	if h.g.AccountStore.AccountExists("dave2") {
+		t.Fatal("rename created the new account name despite the invalid character list")
+	}
+}
+
+func TestCharacterTxnsRejectPathLikeNames(t *testing.T) {
+	h := newHarness(t)
+	h.seedAccount("alice")
+
+	// CharPath("../escape") resolves to <dataDir>/players/escape.yaml.
+	canary := filepath.Join(h.dir, "players", "escape.yaml")
+	writeTraversalCanary(t, canary)
+
+	if _, err := h.g.createCharacterTxn("alice", "../escape", 1); err == nil {
+		t.Fatal("create accepted a path-traversing character name")
+	} else if !strings.Contains(err.Error(), "invalid character name") {
+		t.Fatalf("unclear create error: %v", err)
+	}
+	assertTraversalCanary(t, canary)
+
+	if _, err := h.g.createCharacterTxn("../alice", "bob", 1); err == nil {
+		t.Fatal("create accepted a path-traversing account name")
+	} else if !strings.Contains(err.Error(), "invalid account name") {
+		t.Fatalf("unclear create error: %v", err)
+	}
+	assertTraversalCanary(t, canary)
+
+	if err := h.g.renameCharacterTxn("alice", "../escape", "bob"); err == nil {
+		t.Fatal("rename accepted a path-traversing old character name")
+	} else if !strings.Contains(err.Error(), "invalid character name") {
+		t.Fatalf("unclear rename error: %v", err)
+	}
+	assertTraversalCanary(t, canary)
+
+	if err := h.g.deleteCharacterTxn("alice", "../escape"); err == nil {
+		t.Fatal("delete accepted a path-traversing character name")
+	} else if !strings.Contains(err.Error(), "invalid character name") {
+		t.Fatalf("unclear delete error: %v", err)
+	}
+	assertTraversalCanary(t, canary)
+
+	// Valid names still work: the validation must not block the normal path.
+	p, err := h.g.createCharacterTxn("alice", "bob", 1)
+	if err != nil {
+		t.Fatalf("create with a valid name failed: %v", err)
+	}
+	if p.Name != "bob" {
+		t.Fatalf("created character name = %q, want bob", p.Name)
+	}
+	acc, err := h.g.AccountStore.LoadAccount("alice")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(acc.Characters) != 1 || acc.Characters[0] != "bob" {
+		t.Fatalf("account character list = %v, want [bob]", acc.Characters)
+	}
+	if err := h.g.deleteCharacterTxn("alice", "bob"); err != nil {
+		t.Fatalf("delete with a valid name failed: %v", err)
+	}
+	if _, err := h.g.AccountStore.LoadCharacter("bob"); err == nil {
+		t.Fatal("delete left the character file behind")
+	}
+}
+
+// TestPurgeAccountTxnAcceptsSeededNames is the positive control: the existing
+// login/session path (a genuinely registered account with its own character)
+// still purges.
+func TestPurgeAccountTxnAcceptsSeededNames(t *testing.T) {
+	h := newHarness(t)
+	h.seedAccount("alice")
+	h.seedCharacter("alice", "alice")
+
+	if err := h.g.purgeAccountTxn("alice", []string{"alice"}); err != nil {
+		t.Fatalf("purge with valid names failed: %v", err)
+	}
+	if h.g.AccountStore.AccountExists("alice") {
+		t.Fatal("purge left the account file behind")
+	}
+	if _, err := h.g.AccountStore.LoadCharacter("alice"); err == nil {
+		t.Fatal("purge left the character file behind")
+	}
+}