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

1 files changed, 289 insertions, 0 deletions

diff --git a/internal/admin/hardening_security_test.go b/internal/admin/hardening_security_test.go
new file mode 100644
index 0000000..a077983
--- /dev/null
+++ b/internal/admin/hardening_security_test.go
@@ -0,0 +1,289 @@
+package admin
+
+// Regression tests for the security-hardening stage:
+//   - undo history is written 0600 and never stores account password hashes
+//   - undo of a deleted account file recreates it 0600
+//   - the unauthenticated /login route bounds its request body
+//   - /static/ requires auth (except the login stylesheet) and never lists
+//   - saving colors preserves unrelated config keys/comments and writes a backup
+//   - a trigger kind change records a before-image at the old path
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"thehouseoficarus/internal/config"
+)
+
+func TestUndoHistoryRedactsSecretsAndIsOwnerOnly(t *testing.T) {
+	dir := t.TempDir()
+	accDir := filepath.Join(dir, "players", "accounts")
+	if err := os.MkdirAll(accDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+	accPath := filepath.Join(accDir, "alice.yaml")
+	oldContent := "name: alice\npassword_hash: argon2id:1:65536:2:c2FsdHNhbHRzYWx0c2E:deadbeef\nadmin: false\n"
+	newContent := "name: alice\npassword_hash: argon2id:1:65536:2:c2FsdHNhbHRzYWx0c2E:deadbeef\nadmin: true\n"
+	if err := os.WriteFile(accPath, []byte(newContent), 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	us := NewUndoStack(dir)
+	// A history file left world-readable by a previous version must be tightened.
+	if err := os.WriteFile(filepath.Join(dir, ".admin_history.json"), []byte("{}"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	us.Push(ChangeDesc{
+		Description: "edit account",
+		FilePath:    accPath,
+		OldContent:  []byte(oldContent),
+		NewContent:  []byte(newContent),
+	})
+
+	histPath := filepath.Join(dir, ".admin_history.json")
+	info, err := os.Stat(histPath)
+	if err != nil {
+		t.Fatalf("history file not written: %v", err)
+	}
+	if info.Mode().Perm() != 0600 {
+		t.Fatalf("history file mode = %o, want 0600", info.Mode().Perm())
+	}
+	raw, err := os.ReadFile(histPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(raw), "argon2id:") || strings.Contains(string(raw), "deadbeef") {
+		t.Fatalf("history file leaked the password hash:\n%s", raw)
+	}
+	if !strings.Contains(string(raw), redactedAccountHash) {
+		t.Fatalf("history file missing redaction placeholder:\n%s", raw)
+	}
+
+	// The in-memory change keeps the real bytes so an in-session undo still
+	// restores a working account.
+	ch := us.Undo()
+	if ch == nil {
+		t.Fatal("undo returned nil")
+	}
+	got, err := os.ReadFile(accPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(got), "argon2id:") {
+		t.Fatalf("undo lost the real password hash:\n%s", got)
+	}
+}
+
+func TestUndoRestoresDeletedAccountAsOwnerOnly(t *testing.T) {
+	dir := t.TempDir()
+	accPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+	if err := os.MkdirAll(filepath.Dir(accPath), 0755); err != nil {
+		t.Fatal(err)
+	}
+	content := []byte("name: bob\npassword_hash: argon2id:1:65536:2:c2FsdHNhbHRzYWx0c2E:deadbeef\n")
+	if err := os.WriteFile(accPath, content, 0600); err != nil {
+		t.Fatal(err)
+	}
+	us := NewUndoStack(dir)
+	us.Push(ChangeDesc{
+		Description: "delete bob",
+		FilePath:    accPath,
+		OldContent:  content,
+		IsDelete:    true,
+	})
+	if err := os.Remove(accPath); err != nil {
+		t.Fatal(err)
+	}
+	if us.Undo() == nil {
+		t.Fatal("undo of delete returned nil")
+	}
+	info, err := os.Stat(accPath)
+	if err != nil {
+		t.Fatalf("account not recreated: %v", err)
+	}
+	if info.Mode().Perm() != 0600 {
+		t.Fatalf("recreated account mode = %o, want 0600", info.Mode().Perm())
+	}
+}
+
+func TestLoginRejectsOversizedBody(t *testing.T) {
+	s := &AdminServer{}
+	big := strings.Repeat("a", maxLoginBodyBytes+1)
+	r := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(big))
+	r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	w := httptest.NewRecorder()
+	s.handleLogin(w, r)
+	if w.Code != http.StatusRequestEntityTooLarge {
+		t.Fatalf("oversized login body status = %d, want 413", w.Code)
+	}
+}
+
+func TestStaticRequiresAuthAndDoesNotList(t *testing.T) {
+	s := newSecurityTestServer(t)
+
+	// admin.css is referenced by the unauthenticated login page, so it stays
+	// public.
+	css := httptest.NewRecorder()
+	s.ServeHTTP(css, httptest.NewRequest(http.MethodGet, "/static/admin.css", nil))
+	if css.Code != http.StatusOK {
+		t.Fatalf("admin.css status = %d, want 200", css.Code)
+	}
+
+	// Every other asset requires a session.
+	js := httptest.NewRecorder()
+	s.ServeHTTP(js, httptest.NewRequest(http.MethodGet, "/static/admin.js", nil))
+	if js.Code != http.StatusUnauthorized {
+		t.Fatalf("unauthenticated admin.js status = %d, want 401", js.Code)
+	}
+
+	// A directory request must never emit a listing.
+	dir := httptest.NewRecorder()
+	s.ServeHTTP(dir, httptest.NewRequest(http.MethodGet, "/static/", nil))
+	if dir.Code == http.StatusOK {
+		t.Fatalf("GET /static/ served a directory listing:\n%s", dir.Body.String())
+	}
+}
+
+func newSecurityTestServer(t *testing.T) *AdminServer {
+	t.Helper()
+	h, _ := newEditorHarness(t)
+	s, err := NewServer(config.Default(), false, h.g.AccountStore, h.g.World, h.g.ItemStore, h.g.ObjectStore,
+		h.g.MobStore, h.dir, h.g.CourseStore.ReloadAll, h.g.MigrateRoomIDsOnOwner, h.g.EvictCasinoRoom,
+		h.g.RebuildCraftIndex, filepath.Join(h.dir, "config.yaml"), h.g.RunAdminOnOwner)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return s
+}
+
+func TestFileEditorRefusesOnlinePlayerFile(t *testing.T) {
+	dir := t.TempDir()
+	accPath := filepath.Join(dir, "players", "accounts", "alice.yaml")
+	if err := os.MkdirAll(filepath.Dir(accPath), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(accPath, []byte("name: alice\npassword_hash: x\n"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	s := &AdminServer{dataDir: dir, undoStack: NewUndoStack(dir)}
+	s.SetPlayerFileOnlineCheck(func(relPath string) bool {
+		return strings.HasPrefix(relPath, "players/accounts/")
+	})
+
+	r := httptest.NewRequest(http.MethodPut, "/api/files?path=players/accounts/alice.yaml",
+		strings.NewReader("name: alice\npassword_hash: y\n"))
+	w := httptest.NewRecorder()
+	s.handleFiles(w, r)
+	if !strings.Contains(w.Body.String(), "online") {
+		t.Fatalf("online player file edit not refused: %s", w.Body.String())
+	}
+	got, err := os.ReadFile(accPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(got), "password_hash: y") {
+		t.Fatalf("online account file was overwritten:\n%s", got)
+	}
+}
+
+func TestNewAccountFileIsOwnerOnly(t *testing.T) {
+	dir := t.TempDir()
+	s := &AdminServer{dataDir: dir, undoStack: NewUndoStack(dir)}
+	r := httptest.NewRequest(http.MethodPut, "/api/files?path=players/accounts/newbie.yaml",
+		strings.NewReader("name: newbie\npassword_hash: x\n"))
+	w := httptest.NewRecorder()
+	s.handleFiles(w, r)
+	info, err := os.Stat(filepath.Join(dir, "players", "accounts", "newbie.yaml"))
+	if err != nil {
+		t.Fatalf("account file not created: %v", err)
+	}
+	if info.Mode().Perm() != 0600 {
+		t.Fatalf("new account file mode = %o, want 0600", info.Mode().Perm())
+	}
+}
+
+func TestColorsSavePatchesOnlyColorMaps(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.yaml")
+	orig := "# operator note\ntick_length: 900\ndefault_colors:\n  room_name: \"74 bold\"\nconstant_colors:\n  hp_bar_low: \"A7\"\n"
+	if err := os.WriteFile(path, []byte(orig), 0644); err != nil {
+		t.Fatal(err)
+	}
+	loaded, err := config.Load(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := &AdminServer{cfg: loaded, configPath: path}
+
+	body := map[string]any{"colors": map[string]string{"room_name": "FF bold"}}
+	b, _ := json.Marshal(body)
+	req := httptest.NewRequest(http.MethodPost, "/api/colors", bytes.NewReader(b))
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+	s.saveColors(w, req)
+	if w.Code != http.StatusOK {
+		t.Fatalf("save status = %d: %s", w.Code, w.Body.String())
+	}
+
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	out := string(raw)
+	if !strings.Contains(out, "tick_length: 900") {
+		t.Fatalf("unrelated config key was reverted:\n%s", out)
+	}
+	if !strings.Contains(out, "# operator note") {
+		t.Fatalf("config comment was dropped:\n%s", out)
+	}
+	if !strings.Contains(out, "FF bold") {
+		t.Fatalf("color edit not persisted:\n%s", out)
+	}
+	if _, err := os.Stat(path + ".bak"); err != nil {
+		t.Fatalf("no config backup written: %v", err)
+	}
+}
+
+func TestTriggerKindChangeUndoRestoresOldPath(t *testing.T) {
+	dir := t.TempDir()
+	root := filepath.Join(dir, "triggers")
+	oldPath := filepath.Join(root, "player_flag", "t1.yaml")
+	if err := os.MkdirAll(filepath.Dir(oldPath), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(oldPath, []byte("on_player_flag: visited\nsteps: []\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	s := &AdminServer{dataDir: dir, undoStack: NewUndoStack(dir)}
+
+	body := `{"on_global_flag":"world_event","steps":[]}`
+	req := httptest.NewRequest(http.MethodPut, "/api/triggers/t1", strings.NewReader(body))
+	w := httptest.NewRecorder()
+	s.handleTriggerByID(w, req)
+	if w.Code != http.StatusOK {
+		t.Fatalf("put status = %d: %s", w.Code, w.Body.String())
+	}
+	newPath := filepath.Join(root, "global_flag", "t1.yaml")
+	if _, err := os.Stat(newPath); err != nil {
+		t.Fatalf("new trigger file missing: %v", err)
+	}
+	if _, err := os.Stat(oldPath); !os.IsNotExist(err) {
+		t.Fatalf("old trigger file not removed after kind change")
+	}
+
+	if s.undoStack.Undo() == nil {
+		t.Fatal("undo returned nil")
+	}
+	if _, err := os.Stat(oldPath); err != nil {
+		t.Fatalf("undo did not restore the old path: %v", err)
+	}
+	if _, err := os.Stat(newPath); !os.IsNotExist(err) {
+		t.Fatalf("undo left the new path behind")
+	}
+}