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 --- .../game/hardening_account_name_validation_test.go | 201 +++++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 internal/game/hardening_account_name_validation_test.go (limited to 'internal/game/hardening_account_name_validation_test.go') 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 /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 /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 /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 /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") + } +} -- cgit v1.2.3