aboutsummaryrefslogtreecommitdiff
path: root/internal/admin
diff options
context:
space:
mode:
Diffstat (limited to 'internal/admin')
-rw-r--r--internal/admin/api_colors.go143
-rw-r--r--internal/admin/api_files.go61
-rw-r--r--internal/admin/api_triggers.go13
-rw-r--r--internal/admin/correct_colors_merge_limiter_test.go170
-rw-r--r--internal/admin/correct_undo_account_image_test.go262
-rw-r--r--internal/admin/hardening_login_limiter_test.go164
-rw-r--r--internal/admin/hardening_security_test.go289
-rw-r--r--internal/admin/server.go220
-rw-r--r--internal/admin/undo.go329
9 files changed, 1589 insertions, 62 deletions
diff --git a/internal/admin/api_colors.go b/internal/admin/api_colors.go
index b57ad82..7989e35 100644
--- a/internal/admin/api_colors.go
+++ b/internal/admin/api_colors.go
@@ -1,6 +1,8 @@
package admin
import (
+ "bytes"
+ "fmt"
"net/http"
"os"
"strings"
@@ -575,14 +577,145 @@ func (s *AdminServer) handleColorsReset(w http.ResponseWriter, r *http.Request)
writeJSON(w, map[string]any{"ok": true, "colors": live, "constant_colors": liveConst})
}
-// writeConfigFile marshals the entire live *config.Config back to config.yaml,
-// overwriting whatever was on disk. A full re-marshal drops the file's manual
-// comments but produces a valid, reloadable file with the edited colors.
+// writeConfigFile persists the two edited color maps back to config.yaml
+// without overwriting unrelated operator edits made after startup. The file
+// is re-read and only the default_colors / constant_colors mappings are
+// patched (comments and every other key are preserved), after writing a
+// backup. A missing/unreadable config falls back to a full marshal so a fresh
+// install still gets a valid file.
func (s *AdminServer) writeConfigFile() error {
- _ = backupFile(s.configPath) // best-effort snapshot for safety
- out, err := yaml.Marshal(s.cfg)
+ if err := s.backupConfigFile(); err != nil {
+ return err
+ }
+ data, err := os.ReadFile(s.configPath)
+ if err != nil {
+ // No config on disk yet (fresh install or removed): write the live
+ // config whole so the file exists and reloads.
+ out, merr := yaml.Marshal(s.cfg)
+ if merr != nil {
+ return merr
+ }
+ return os.WriteFile(s.configPath, out, 0644)
+ }
+ out, err := patchColorMaps(data, s.cfg.DefaultColors, s.cfg.ConstantColors)
if err != nil {
return err
}
return os.WriteFile(s.configPath, out, 0644)
}
+
+// backupConfigFile copies the current config.yaml next to itself before an
+// edit, so an operator can recover from a bad color save. A fresh install
+// (no file yet) has nothing to back up.
+func (s *AdminServer) backupConfigFile() error {
+ data, err := os.ReadFile(s.configPath)
+ if err != nil {
+ if os.IsNotExist(err) {
+ return nil
+ }
+ return err
+ }
+ return os.WriteFile(s.configPath+".bak", data, 0600)
+}
+
+// patchColorMaps updates the default_colors and constant_colors mappings in a
+// config.yaml document, preserving every other key and all comments. Only the
+// keys present in the live maps are written: keys an operator added inside
+// those sections, and every comment around them, survive the save.
+func patchColorMaps(data []byte, defaults, constants map[string]string) ([]byte, error) {
+ var doc yaml.Node
+ if err := yaml.Unmarshal(data, &doc); err != nil {
+ return nil, fmt.Errorf("parse config: %w", err)
+ }
+ if len(doc.Content) == 0 || doc.Content[0].Kind != yaml.MappingNode {
+ return nil, fmt.Errorf("config is not a mapping")
+ }
+ root := doc.Content[0]
+ for _, patch := range []struct {
+ key string
+ m map[string]string
+ }{
+ {"default_colors", defaults},
+ {"constant_colors", constants},
+ } {
+ if err := patchMapping(root, patch.key, patch.m); err != nil {
+ return nil, err
+ }
+ }
+ var buf bytes.Buffer
+ enc := yaml.NewEncoder(&buf)
+ enc.SetIndent(2)
+ if err := enc.Encode(&doc); err != nil {
+ return nil, err
+ }
+ if err := enc.Close(); err != nil {
+ return nil, err
+ }
+ return buf.Bytes(), nil
+}
+
+// patchMapping writes the entries of m into the mapping at root[key], creating
+// the mapping (and the key) when absent. Entries already on disk are updated
+// in place so their comments survive; entries on disk that m does not name are
+// left exactly as the operator wrote them.
+func patchMapping(root *yaml.Node, key string, m map[string]string) error {
+ fresh, err := colorMapNode(m)
+ if err != nil {
+ return err
+ }
+ for i := 0; i+1 < len(root.Content); i += 2 {
+ if root.Content[i].Value != key {
+ continue
+ }
+ if existing := root.Content[i+1]; existing.Kind == yaml.MappingNode {
+ mergeMapNode(existing, fresh)
+ return nil
+ }
+ root.Content[i+1] = fresh
+ return nil
+ }
+ root.Content = append(root.Content,
+ &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: key}, fresh)
+ return nil
+}
+
+// mergeMapNode copies src's entries into dst: a key dst already has is updated
+// in place (keeping the operator's comments on it), a key it lacks is appended
+// with the node from src.
+func mergeMapNode(dst, src *yaml.Node) {
+ for i := 0; i+1 < len(src.Content); i += 2 {
+ key, value := src.Content[i], src.Content[i+1]
+ merged := false
+ for j := 0; j+1 < len(dst.Content); j += 2 {
+ if dst.Content[j].Value != key.Value {
+ continue
+ }
+ if dst.Content[j+1].Kind == yaml.ScalarNode && value.Kind == yaml.ScalarNode {
+ dst.Content[j+1].Value = value.Value
+ dst.Content[j+1].Tag = value.Tag
+ } else {
+ dst.Content[j+1] = value
+ }
+ merged = true
+ break
+ }
+ if !merged {
+ dst.Content = append(dst.Content, key, value)
+ }
+ }
+}
+
+func colorMapNode(m map[string]string) (*yaml.Node, error) {
+ b, err := yaml.Marshal(m)
+ if err != nil {
+ return nil, err
+ }
+ var n yaml.Node
+ if err := yaml.Unmarshal(b, &n); err != nil {
+ return nil, err
+ }
+ if len(n.Content) == 0 {
+ return &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"}, nil
+ }
+ return n.Content[0], nil
+}
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go
index 2b59526..96b5e81 100644
--- a/internal/admin/api_files.go
+++ b/internal/admin/api_files.go
@@ -82,6 +82,10 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) {
failJSON(w, err.Error())
return
}
+ if isPlayerDataFilePath(relPath) && s.playerFileOnline(relPath) {
+ failJSON(w, "cannot rename a player/account file while it is online; log the character out first")
+ return
+ }
oldContent, err := snapshotFile(oldAbs)
if err != nil {
@@ -118,6 +122,12 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) {
writeJSON(w, map[string]any{"ok": true, "path": newRelPath})
}
+// handleFiles is only reachable through ownerMiddleware (see server.go), so
+// every player/account mutation below executes on the game owner and cannot
+// interleave with a live save. This is the same single-owner guarantee the
+// lifecycle transactions rely on. Additionally, a file whose character/account
+// is currently online is refused: overwriting it would clobber the in-memory
+// player the game will later save.
func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
relPath := r.URL.Query().Get("path")
relPath = strings.TrimPrefix(relPath, "/")
@@ -188,6 +198,10 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
failJSON(w, "missing path")
return
}
+ if isPlayerDataFilePath(relPath) && s.playerFileOnline(relPath) {
+ failJSON(w, "cannot edit a player/account file while it is online; log the character out first")
+ return
+ }
body, err := io.ReadAll(r.Body)
if err != nil {
failJSON(w, "read body: "+err.Error())
@@ -232,6 +246,10 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
failJSON(w, "missing path")
return
}
+ if isPlayerDataFilePath(relPath) && s.playerFileOnline(relPath) {
+ failJSON(w, "cannot delete a player/account file while it is online; log the character out first")
+ return
+ }
oldContent, _ := snapshotFile(fullPath)
if err := os.Remove(fullPath); err != nil {
failJSON(w, "delete error: "+err.Error())
@@ -253,9 +271,48 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
// writeFilePreservingMode writes data atomically without widening an existing
// file's permissions (persistence.WriteFile preserves the current mode and
-// uses 0644 for a new file).
+// uses the caller's perm for a new file). Player/account files are treated as
+// owner-only: they carry password hashes and live player state, so a newly
+// created file is 0600 and an existing wider mode is tightened.
func writeFilePreservingMode(path string, data []byte) error {
- return persistence.WriteFile(path, data, 0644)
+ perm := os.FileMode(0644)
+ if isPlayerDataAbsPath(path) {
+ perm = 0600
+ }
+ if err := persistence.WriteFile(path, data, perm); err != nil {
+ return err
+ }
+ if perm == 0600 {
+ return os.Chmod(path, 0600)
+ }
+ return nil
+}
+
+// isPlayerDataFilePath reports whether a data-relative path is a player or
+// account YAML file (password hash / live character state).
+func isPlayerDataFilePath(relPath string) bool {
+ clean := filepath.ToSlash(filepath.Clean(relPath))
+ return strings.HasPrefix(clean, "players/accounts/") || strings.HasPrefix(clean, "players/characters/")
+}
+
+// isPlayerDataAbsPath is the absolute-path form of isPlayerDataFilePath: it
+// matches .../players/accounts/<name>.yaml and .../players/characters/<name>.yaml.
+func isPlayerDataAbsPath(path string) bool {
+ parent := filepath.Base(filepath.Dir(path))
+ if parent != "accounts" && parent != "characters" {
+ return false
+ }
+ return filepath.Base(filepath.Dir(filepath.Dir(path))) == "players"
+}
+
+// playerFileOnline reports whether the character/account addressed by a
+// data-relative player file path has a live session. The game wires this via
+// SetPlayerFileOnlineCheck; nil (unwired) disables the refusal.
+func (s *AdminServer) playerFileOnline(relPath string) bool {
+ if s.playerFileOnlineCheck == nil {
+ return false
+ }
+ return s.playerFileOnlineCheck(relPath)
}
// isAccountFilePath reports whether a data-relative path is an account file
diff --git a/internal/admin/api_triggers.go b/internal/admin/api_triggers.go
index 5b82465..08e2855 100644
--- a/internal/admin/api_triggers.go
+++ b/internal/admin/api_triggers.go
@@ -147,12 +147,21 @@ func (s *AdminServer) handleTriggerByID(w http.ResponseWriter, r *http.Request)
os.Remove(oldPath)
}
- s.undoStack.Push(ChangeDesc{
+ desc := ChangeDesc{
Description: "Update trigger " + id,
FilePath: newPath,
OldContent: oldContent,
NewContent: triggerData,
- })
+ }
+ if findErr == nil && newSubdir != oldDir {
+ // A kind change moves the file between subdirectories. Record the
+ // old path as the before-image location so undo restores the old
+ // content there (and removes the new file) instead of writing the
+ // old content into the new directory and orphaning the old path.
+ desc.FilePath = oldPath
+ desc.NewFilePath = newPath
+ }
+ s.undoStack.Push(desc)
s.rebuildTriggerIndex()
writeJSON(w, map[string]any{"ok": true, "id": id, "kind": newKind})
diff --git a/internal/admin/correct_colors_merge_limiter_test.go b/internal/admin/correct_colors_merge_limiter_test.go
new file mode 100644
index 0000000..7ee9071
--- /dev/null
+++ b/internal/admin/correct_colors_merge_limiter_test.go
@@ -0,0 +1,170 @@
+package admin
+
+// Regression tests for two infra re-verification findings:
+// - saving colors merges the live maps' keys into config.yaml instead of
+// replacing the default_colors/constant_colors nodes, so keys and comments
+// an operator added inside those sections survive the save
+// - the /login limiter never evicts an address with an active cooldown, so a
+// blocked source cannot unblock itself by filling the tracked-address map
+
+import (
+ "bytes"
+ "encoding/json"
+ "fmt"
+ "net/http"
+ "net/http/httptest"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+ "time"
+
+ "thehouseoficarus/internal/config"
+)
+
+func TestColorsSavePreservesOperatorKeysInsideColorSections(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "config.yaml")
+ orig := "# operator note\n" +
+ "tick_length: 900\n" +
+ "default_colors:\n" +
+ " # keep this: hand-added\n" +
+ " room_name: \"74 bold\" # tweaked\n" +
+ " my_custom_color: \"FF\"\n" +
+ "# constant section\n" +
+ "constant_colors:\n" +
+ " hp_bar_low: \"A7\"\n" +
+ " my_custom_constant: \"1E\"\n"
+ if err := os.WriteFile(path, []byte(orig), 0644); err != nil {
+ t.Fatal(err)
+ }
+
+ // The live maps hold the values the process loaded, so the two hand-added
+ // keys are not in them: the save must not drop them from the file.
+ cfg := config.Default()
+ cfg.DefaultColors = config.ColorsConfig{"room_name": "74 bold"}
+ cfg.ConstantColors = config.ColorsConfig{"hp_bar_low": "A7"}
+ s := &AdminServer{cfg: cfg, configPath: path}
+
+ body, _ := json.Marshal(map[string]any{
+ "colors": map[string]string{"room_name": "FF bold", "say": "FF"},
+ "constant_colors": map[string]string{"hp_bar_low": "A7 bold"},
+ })
+ req := httptest.NewRequest(http.MethodPost, "/api/colors", bytes.NewReader(body))
+ 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)
+ for _, want := range []string{
+ "# operator note",
+ "tick_length: 900",
+ "# keep this: hand-added",
+ "# tweaked",
+ "my_custom_color",
+ "# constant section",
+ "my_custom_constant",
+ } {
+ if !strings.Contains(out, want) {
+ t.Errorf("saved config lost %q:\n%s", want, out)
+ }
+ }
+
+ reloaded, err := config.Load(path)
+ if err != nil {
+ t.Fatalf("saved config no longer loads: %v\n%s", err, out)
+ }
+ for key, want := range map[string]string{
+ "room_name": "FF bold",
+ "say": "FF",
+ "my_custom_color": "FF",
+ "my_custom_constant": "1E",
+ } {
+ source := reloaded.DefaultColors
+ if key == "my_custom_constant" {
+ source = reloaded.ConstantColors
+ }
+ if got := source[key]; got != want {
+ t.Errorf("default/constant color %s = %q, want %q", key, got, want)
+ }
+ }
+ if got := reloaded.ConstantColors["hp_bar_low"]; got != "A7 bold" {
+ t.Errorf("constant color hp_bar_low = %q, want %q", got, "A7 bold")
+ }
+}
+
+// TestLoginLimiterNeverEvictsAnAddressInCooldown: with the tracked-address map
+// full of cooling-down addresses, a flood from fresh addresses must not drop
+// one of them. Its failure count is reset when the block starts, so it fails
+// the idle test while still being the oldest entry: evicting it would hand a
+// blocked source a fresh allowance.
+func TestLoginLimiterNeverEvictsAnAddressInCooldown(t *testing.T) {
+ base := time.Now()
+ l := newLoginLimiter()
+ l.now = func() time.Time { return base }
+
+ blocked := make([]string, 0, adminLoginMaxTracked)
+ for i := 0; i < adminLoginMaxTracked; i++ {
+ ip := fmt.Sprintf("198.51.100.%d", i)
+ for j := 0; j < adminLoginMaxFailures; j++ {
+ l.fail(ip)
+ }
+ blocked = append(blocked, ip)
+ }
+ if got := len(l.entries); got != adminLoginMaxTracked {
+ t.Fatalf("tracked %d addresses, want the cap %d", got, adminLoginMaxTracked)
+ }
+
+ for i := 0; i < adminLoginMaxTracked*2; i++ {
+ l.fail(fmt.Sprintf("203.0.113.%d", i))
+ }
+ if got := len(l.entries); got > adminLoginMaxTracked {
+ t.Fatalf("tracked %d addresses, want at most %d", got, adminLoginMaxTracked)
+ }
+ for _, ip := range blocked {
+ if l.allow(ip) {
+ t.Fatalf("address %s was evicted while cooling down and may log in again", ip)
+ }
+ }
+ if _, ok := l.entries["203.0.113.0"]; ok {
+ t.Fatal("a fresh address was tracked by evicting an address in cooldown")
+ }
+}
+
+// TestLoginLimiterEvictsAnAddressWithNoCooldown: an address with failures but
+// no cooldown is still evictable, so the map keeps accepting new addresses
+// while any evictable entry exists.
+func TestLoginLimiterEvictsAnAddressWithNoCooldown(t *testing.T) {
+ base := time.Now()
+ l := newLoginLimiter()
+ l.now = func() time.Time { return base }
+
+ l.fail("192.0.2.1")
+ for i := 0; i < adminLoginMaxTracked-1; i++ {
+ ip := fmt.Sprintf("198.51.100.%d", i)
+ for j := 0; j < adminLoginMaxFailures; j++ {
+ l.fail(ip)
+ }
+ }
+ if _, ok := l.entries["192.0.2.1"]; !ok {
+ t.Fatal("setup: the single-failure address was not tracked")
+ }
+
+ l.fail("192.0.2.99")
+ if _, ok := l.entries["192.0.2.1"]; ok {
+ t.Fatal("the address with no cooldown was not evicted to make room")
+ }
+ if _, ok := l.entries["192.0.2.99"]; !ok {
+ t.Fatal("the fresh address was not tracked after evicting an evictable entry")
+ }
+ if got := len(l.entries); got != adminLoginMaxTracked {
+ t.Fatalf("tracked %d addresses after eviction, want %d", got, adminLoginMaxTracked)
+ }
+}
diff --git a/internal/admin/correct_undo_account_image_test.go b/internal/admin/correct_undo_account_image_test.go
new file mode 100644
index 0000000..51fe037
--- /dev/null
+++ b/internal/admin/correct_undo_account_image_test.go
@@ -0,0 +1,262 @@
+package admin
+
+// Regression tests for the undo-history account-file paths re-verified in the
+// infra pass:
+// - a history image whose password hash was redacted on disk cannot be
+// written over an account after a restart (login could never repair the
+// placeholder, and the raw editor copies it back)
+// - undo/redo refuses a character/account file whose owner is online, the
+// same refusal the raw file editor applies
+// - the refusal is scoped to player files: other data files undo normally
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+const (
+ testAccountHash = "argon2id:1:65536:2:c2FsdHNhbHRzYWx0c2E:deadbeef"
+ testAccountLive = "name: bob\npassword_hash: " + testAccountHash + "\nadmin: false\n"
+ testAccountPromote = "name: bob\npassword_hash: " + testAccountHash + "\nadmin: true\n"
+)
+
+func writeAccountFile(t *testing.T, path, content string) {
+ t.Helper()
+ if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(path, []byte(content), 0600); err != nil {
+ t.Fatal(err)
+ }
+}
+
+// TestUndoOfRedactedAccountImageDoesNotClobberAccount drives the restart the
+// history file exists to survive: Push redacts the persisted image, a fresh
+// UndoStack loads that redacted image, and the undo must then fail rather than
+// write "[redacted]" into a live account.
+func TestUndoOfRedactedAccountImageDoesNotClobberAccount(t *testing.T) {
+ t.Run("delete of a removed account fails without creating it", func(t *testing.T) {
+ dir := t.TempDir()
+ accPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+ writeAccountFile(t, accPath, testAccountLive)
+
+ us := NewUndoStack(dir)
+ us.Push(ChangeDesc{
+ Description: "delete bob",
+ FilePath: accPath,
+ OldContent: []byte(testAccountLive),
+ IsDelete: true,
+ })
+ if err := os.Remove(accPath); err != nil {
+ t.Fatal(err)
+ }
+ raw, err := os.ReadFile(filepath.Join(dir, ".admin_history.json"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(string(raw), redactedAccountHash) {
+ t.Fatalf("history file did not redact the hash:\n%s", raw)
+ }
+
+ restarted := NewUndoStack(dir)
+ if ch := restarted.Undo(); ch != nil {
+ t.Fatalf("undo wrote a redacted account image: %s", ch.ShortDesc())
+ }
+ if got, err := os.ReadFile(accPath); err == nil {
+ t.Fatalf("undo recreated the account from the redacted image:\n%s", got)
+ } else if !os.IsNotExist(err) {
+ t.Fatalf("stat recreated account: %v", err)
+ }
+ })
+
+ t.Run("rename restores the real hash from the renamed file", func(t *testing.T) {
+ dir := t.TempDir()
+ oldPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+ newPath := filepath.Join(dir, "players", "accounts", "robert.yaml")
+ writeAccountFile(t, oldPath, testAccountLive)
+
+ us := NewUndoStack(dir)
+ us.Push(ChangeDesc{
+ Description: "rename bob",
+ FilePath: oldPath,
+ NewFilePath: newPath,
+ OldContent: []byte(testAccountLive),
+ NewContent: []byte(testAccountLive),
+ })
+ if err := os.Rename(oldPath, newPath); err != nil {
+ t.Fatal(err)
+ }
+
+ restarted := NewUndoStack(dir)
+ if ch := restarted.Undo(); ch == nil {
+ t.Fatal("undo of a rename must restore the account from its live renamed copy")
+ }
+ got, err := os.ReadFile(oldPath)
+ if err != nil {
+ t.Fatalf("undo did not restore the old path: %v", err)
+ }
+ if !strings.Contains(string(got), testAccountHash) {
+ t.Fatalf("restored account lost its hash:\n%s", got)
+ }
+ if strings.Contains(string(got), redactedAccountHash) {
+ t.Fatalf("restored account carries the redaction placeholder:\n%s", got)
+ }
+ if _, err := os.Stat(newPath); !os.IsNotExist(err) {
+ t.Fatalf("undo left the renamed file behind: %v", err)
+ }
+ })
+
+ t.Run("rename whose only copy is gone fails before removing anything", func(t *testing.T) {
+ dir := t.TempDir()
+ oldPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+ newPath := filepath.Join(dir, "players", "accounts", "robert.yaml")
+
+ us := NewUndoStack(dir)
+ us.Push(ChangeDesc{
+ Description: "rename bob",
+ FilePath: oldPath,
+ NewFilePath: newPath,
+ OldContent: []byte(testAccountLive),
+ NewContent: []byte(testAccountLive),
+ })
+
+ restarted := NewUndoStack(dir)
+ if ch := restarted.Undo(); ch != nil {
+ t.Fatalf("undo wrote a redacted account image: %s", ch.ShortDesc())
+ }
+ if _, err := os.Stat(oldPath); !os.IsNotExist(err) {
+ t.Fatalf("failed undo created the account from the redacted image: %v", err)
+ }
+ })
+
+ t.Run("edit keeps the live hash", func(t *testing.T) {
+ dir := t.TempDir()
+ accPath := filepath.Join(dir, "players", "accounts", "bob.yaml")
+ writeAccountFile(t, accPath, testAccountPromote)
+
+ us := NewUndoStack(dir)
+ us.Push(ChangeDesc{
+ Description: "make bob an admin",
+ FilePath: accPath,
+ OldContent: []byte(testAccountLive),
+ NewContent: []byte(testAccountPromote),
+ })
+ // The live file still holds the real hash after the restart below, so
+ // the undo has one to merge and must keep working.
+ restarted := NewUndoStack(dir)
+ if ch := restarted.Undo(); ch == nil {
+ t.Fatal("undo of a redacted edit must still apply while the live account file exists")
+ }
+ got, err := os.ReadFile(accPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(string(got), testAccountHash) {
+ t.Fatalf("undo did not merge the live hash into the restored image:\n%s", got)
+ }
+ if !strings.Contains(string(got), "admin: false") {
+ t.Fatalf("undo did not restore the pre-edit image:\n%s", got)
+ }
+ })
+}
+
+// TestUndoRedoRefusesOnlinePlayerFile wires the online check the way main.go
+// does and pins that undo and redo both consult it for a live account, and
+// that the check receives the data-relative path form the game expects.
+func TestUndoRedoRefusesOnlinePlayerFile(t *testing.T) {
+ dir := t.TempDir()
+ accPath := filepath.Join(dir, "players", "accounts", "alice.yaml")
+ writeAccountFile(t, accPath, testAccountLive)
+
+ s := &AdminServer{dataDir: dir, undoStack: NewUndoStack(dir)}
+ var seen []string
+ online := true
+ s.SetPlayerFileOnlineCheck(func(relPath string) bool {
+ seen = append(seen, relPath)
+ return online && relPath == "players/accounts/alice.yaml"
+ })
+
+ s.undoStack.Push(ChangeDesc{
+ Description: "make alice an admin",
+ FilePath: accPath,
+ OldContent: []byte(testAccountLive),
+ NewContent: []byte(testAccountPromote),
+ })
+ writeAccountFile(t, accPath, testAccountPromote)
+
+ if ch := s.undoStack.Undo(); ch != nil {
+ t.Fatalf("undo overwrote an online account file: %s", ch.ShortDesc())
+ }
+ if got, _ := os.ReadFile(accPath); string(got) != testAccountPromote {
+ t.Fatalf("online account file was modified:\n%s", got)
+ }
+ if len(seen) == 0 || seen[0] != "players/accounts/alice.yaml" {
+ t.Fatalf("online check paths = %v, want the data-relative account path first", seen)
+ }
+
+ // Logging the character out clears the refusal, and the redo of the same
+ // change is refused when it is online again. A successful undo rewrites
+ // the mapping (so its key order differs), hence the byte snapshot rather
+ // than a comparison against the original text.
+ online = false
+ if ch := s.undoStack.Undo(); ch == nil {
+ t.Fatal("undo still refused after the character logged out")
+ }
+ undone, err := os.ReadFile(accPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, want := range []string{"admin: false", testAccountHash} {
+ if !strings.Contains(string(undone), want) {
+ t.Fatalf("undo did not apply %q after logout:\n%s", want, undone)
+ }
+ }
+
+ online = true
+ if ch := s.undoStack.Redo(); ch != nil {
+ t.Fatalf("redo overwrote an online account file: %s", ch.ShortDesc())
+ }
+ if got, _ := os.ReadFile(accPath); string(got) != string(undone) {
+ t.Fatalf("redo modified an online account file:\n%s", got)
+ }
+ online = false
+ if ch := s.undoStack.Redo(); ch == nil {
+ t.Fatal("redo still refused after the character logged out")
+ }
+}
+
+// TestUndoOfNonPlayerFileIgnoresOnlineCheck: the refusal is scoped to player
+// files, so a hook that reports every path online cannot block room or item
+// undo.
+func TestUndoOfNonPlayerFileIgnoresOnlineCheck(t *testing.T) {
+ dir := t.TempDir()
+ roomPath := filepath.Join(dir, "rooms", "1.yaml")
+ if err := os.MkdirAll(filepath.Dir(roomPath), 0755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(roomPath, []byte("id: 1\nname: edited\n"), 0644); err != nil {
+ t.Fatal(err)
+ }
+
+ us := NewUndoStack(dir)
+ us.SetPlayerFileOnlineCheck(func(string) bool { return true })
+ us.Push(ChangeDesc{
+ Description: "edit room",
+ FilePath: roomPath,
+ OldContent: []byte("id: 1\nname: old\n"),
+ NewContent: []byte("id: 1\nname: edited\n"),
+ })
+
+ if ch := us.Undo(); ch == nil {
+ t.Fatal("undo of a non-player file was refused by the online check")
+ }
+ got, err := os.ReadFile(roomPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(string(got), "name: old") {
+ t.Fatalf("undo did not revert the room file:\n%s", got)
+ }
+}
diff --git a/internal/admin/hardening_login_limiter_test.go b/internal/admin/hardening_login_limiter_test.go
new file mode 100644
index 0000000..1ee08ed
--- /dev/null
+++ b/internal/admin/hardening_login_limiter_test.go
@@ -0,0 +1,164 @@
+package admin
+
+// Regression coverage for the admin HTTP /login failed-login limiter: after a
+// handful of failures an IP is refused for a cooldown (even with the right
+// password), another client is unaffected, a success clears the state, and the
+// tracked-address map stays bounded. Telnet login throttling is out of scope
+// (net.Conn/Session expose no client identity), so only the HTTP route is
+// exercised here.
+
+import (
+ "fmt"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "sync/atomic"
+ "testing"
+ "time"
+)
+
+// loginStatus posts the login form through the real HTTP route and returns the
+// raw status: the client must not follow the success redirect, because both a
+// rendered error page and a successful login otherwise report 200.
+func loginStatus(t *testing.T, srv *httptest.Server, account, password string) int {
+ t.Helper()
+ client := &http.Client{
+ CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },
+ }
+ resp, err := client.PostForm(srv.URL+"/login", url.Values{"account": {account}, "password": {password}})
+ if err != nil {
+ t.Fatalf("login request: %v", err)
+ }
+ defer resp.Body.Close()
+ return resp.StatusCode
+}
+
+// loginStatusFromIP drives the same handler with an explicit client address, so
+// a test can act as a second IP without opening a second listener.
+func loginStatusFromIP(t *testing.T, s *AdminServer, remoteAddr, account, password string) int {
+ t.Helper()
+ body := url.Values{"account": {account}, "password": {password}}.Encode()
+ req := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(body))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.RemoteAddr = remoteAddr
+ rec := httptest.NewRecorder()
+ s.ServeHTTP(rec, req)
+ return rec.Code
+}
+
+func TestLoginLimiterBlocksFailedLoginsPerIP(t *testing.T) {
+ s := newSecurityTestServer(t)
+ lim := s.limitLogins()
+ base := time.Now()
+ var clockOffset atomic.Int64
+ lim.now = func() time.Time { return base.Add(time.Duration(clockOffset.Load())) }
+
+ srv := httptest.NewServer(s)
+ defer srv.Close()
+
+ fail := func() int { return loginStatus(t, srv, "alice", "wrong") }
+ succeed := func() int { return loginStatus(t, srv, "alice", "pw") }
+
+ // Below the threshold a failure still renders the login page with the
+ // invalid-credentials message: unchanged response content.
+ for i := 0; i < adminLoginMaxFailures-1; i++ {
+ if got := fail(); got != http.StatusOK {
+ t.Fatalf("attempt %d status = %d, want 200", i+1, got)
+ }
+ }
+
+ // A successful login clears the failure count, so the next batch starts
+ // over instead of tripping the threshold partway through.
+ if got := succeed(); got != http.StatusFound {
+ t.Fatalf("valid login status = %d, want 302", got)
+ }
+ for i := 0; i < adminLoginMaxFailures; i++ {
+ if got := fail(); got != http.StatusOK {
+ t.Fatalf("post-reset attempt %d status = %d, want 200 (failure count was not reset)", i+1, got)
+ }
+ }
+
+ // Threshold reached: this IP is refused, even with the right password.
+ if got := fail(); got != http.StatusTooManyRequests {
+ t.Fatalf("status after %d failures = %d, want 429", adminLoginMaxFailures, got)
+ }
+ if got := succeed(); got != http.StatusTooManyRequests {
+ t.Fatalf("valid credentials while blocked = %d, want 429", got)
+ }
+
+ // Another client is not punished for this one.
+ if got := loginStatusFromIP(t, s, "203.0.113.7:5555", "alice", "pw"); got != http.StatusFound {
+ t.Fatalf("valid login from another IP = %d, want 302", got)
+ }
+
+ // Once the cooldown lapses the address is served again.
+ clockOffset.Store(int64(adminLoginCooldown + time.Minute))
+ if got := succeed(); got != http.StatusFound {
+ t.Fatalf("valid login after the cooldown = %d, want 302", got)
+ }
+}
+
+func TestLoginLimiterCooldownAndReset(t *testing.T) {
+ base := time.Now()
+ now := base
+ l := newLoginLimiter()
+ l.now = func() time.Time { return now }
+
+ for i := 0; i < adminLoginMaxFailures; i++ {
+ l.fail("10.0.0.1")
+ }
+ if l.allow("10.0.0.1") {
+ t.Fatal("address allowed immediately after reaching the failure threshold")
+ }
+ if !l.allow("10.0.0.2") {
+ t.Fatal("an unrelated address was blocked")
+ }
+
+ // A blocked address cannot extend its own block, and the cooldown lapses.
+ l.fail("10.0.0.1")
+ now = base.Add(adminLoginCooldown - time.Second)
+ if l.allow("10.0.0.1") {
+ t.Fatal("address allowed before the cooldown lapsed")
+ }
+ now = base.Add(adminLoginCooldown)
+ if !l.allow("10.0.0.1") {
+ t.Fatal("address still blocked after the cooldown lapsed")
+ }
+
+ // A success clears both the count and any remaining cooldown.
+ for i := 0; i < adminLoginMaxFailures-1; i++ {
+ l.fail("10.0.0.3")
+ }
+ l.succeed("10.0.0.3")
+ for i := 0; i < adminLoginMaxFailures-1; i++ {
+ l.fail("10.0.0.3")
+ }
+ if !l.allow("10.0.0.3") {
+ t.Fatal("a successful login did not reset the failure count")
+ }
+}
+
+func TestLoginLimiterBoundsTrackedAddresses(t *testing.T) {
+ base := time.Now()
+ l := newLoginLimiter()
+ l.now = func() time.Time { return base }
+
+ for i := 0; i < adminLoginMaxTracked*3; i++ {
+ l.fail(fmt.Sprintf("198.51.100.%d", i))
+ }
+ if got := len(l.entries); got > adminLoginMaxTracked {
+ t.Fatalf("limiter tracked %d addresses, want at most %d", got, adminLoginMaxTracked)
+ }
+}
+
+// TestLoginLimiterKeysOnClientIPNotPort pins the key: two ports of one host
+// share a bucket, which is what r.RemoteAddr keying is meant to achieve.
+func TestLoginLimiterKeysOnClientIPNotPort(t *testing.T) {
+ if got := clientIP("127.0.0.1:54321"); got != "127.0.0.1" {
+ t.Fatalf("clientIP = %q, want 127.0.0.1", got)
+ }
+ if got := clientIP("no-port"); got != "no-port" {
+ t.Fatalf("clientIP without a port = %q, want the value verbatim", got)
+ }
+}
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")
+ }
+}
diff --git a/internal/admin/server.go b/internal/admin/server.go
index d9a0386..4a54c78 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -4,6 +4,7 @@
package admin
import (
+ "bytes"
"crypto/hmac"
"crypto/rand"
"crypto/rsa"
@@ -14,6 +15,7 @@ import (
"embed"
"encoding/hex"
"encoding/json"
+ "errors"
"fmt"
"html/template"
"io"
@@ -26,6 +28,7 @@ import (
"path/filepath"
"strconv"
"strings"
+ "sync"
"time"
"thehouseoficarus/internal/behavior"
@@ -74,6 +77,18 @@ type AdminServer struct {
// change (create/update/delete/rename/move/undo/redo). Wired from the
// Game so the web editor's item edits stay consistent with gameplay.
rebuildCraftIndex func()
+
+ // playerFileOnlineCheck reports whether the character/account addressed
+ // by a data-relative player file path is currently online. Wired from the
+ // game (SetPlayerFileOnlineCheck) so the raw file editor refuses to
+ // overwrite a live character/account's file. Optional; nil disables it.
+ playerFileOnlineCheck func(relPath string) bool
+
+ // loginLimiterOnce/loginLimiter hold the bounded per-IP failed-login
+ // throttle for /login. The limiter is created on first use so an
+ // AdminServer built directly (tests) behaves like one from NewServer.
+ loginLimiterOnce sync.Once
+ loginLimiter *loginLimiter
}
func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), evictRoom func(roomID int), rebuildCraftIndex func(), configPath string, runOnOwner func(func()) bool) (*AdminServer, error) {
@@ -139,7 +154,23 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
if err != nil {
return nil, fmt.Errorf("static subfs: %w", err)
}
- mux.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.FS(staticFS))))
+ // Serve embedded assets by exact name: this both requires auth (except
+ // for the stylesheet the unauthenticated login page needs) and never
+ // emits the http.FileServer directory listing that leaked the UI's
+ // module names.
+ mux.HandleFunc("/static/", func(w http.ResponseWriter, r *http.Request) {
+ name := strings.TrimPrefix(r.URL.Path, "/static/")
+ if name != "admin.css" && !s.checkAuth(r) {
+ http.Error(w, "unauthorized", http.StatusUnauthorized)
+ return
+ }
+ data, err := fs.ReadFile(staticFS, name)
+ if err != nil {
+ http.NotFound(w, r)
+ return
+ }
+ http.ServeContent(w, r, name, time.Time{}, bytes.NewReader(data))
+ })
mux.HandleFunc("/login", s.handleLogin)
mux.HandleFunc("/logout", s.handleLogout)
@@ -292,6 +323,21 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
return s, nil
}
+// SetPlayerFileOnlineCheck wires the online-session check used by the raw
+// file editor to refuse editing/deleting/renaming a character or account file
+// while that character/account is online. It is optional: when unset the
+// editor behaves as before. main.go wires it to a game-side lookup.
+//
+// Undo/redo writes the same files through the undo stack, so the hook is
+// handed to it as well: undoing an edit to a live character's file must be
+// refused like the edit itself.
+func (s *AdminServer) SetPlayerFileOnlineCheck(fn func(relPath string) bool) {
+ s.playerFileOnlineCheck = fn
+ if s.undoStack != nil {
+ s.undoStack.SetPlayerFileOnlineCheck(fn)
+ }
+}
+
// ServeHTTP exposes the same authenticated routes used by ListenAndServe.
func (s *AdminServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
s.httpServer.Handler.ServeHTTP(w, r)
@@ -362,6 +408,149 @@ func (s *AdminServer) isAdminAccount(name string) bool {
// indefinitely and the server can age it out without server-side state.
const adminSessionTTL = 24 * time.Hour
+// maxLoginBodyBytes bounds the unauthenticated /login request body before any
+// form or multipart parsing. 64 KiB is far above any real login form.
+const maxLoginBodyBytes = 64 << 10
+
+// Failed-login throttling for the unauthenticated /login route. The telnet
+// login path is deliberately NOT covered: net.Conn/Session expose no client
+// identity, so there is nothing to key a limit on.
+const (
+ // adminLoginMaxFailures is the number of failed attempts an IP may make
+ // before it is refused.
+ adminLoginMaxFailures = 5
+ // adminLoginCooldown is how long a refused IP stays refused.
+ adminLoginCooldown = 5 * time.Minute
+ // adminLoginMaxTracked caps the tracked client IPs, so a source that
+ // cycles addresses cannot grow server memory without bound.
+ adminLoginMaxTracked = 1024
+)
+
+// loginLimiter is a small, bounded, in-memory failed-login limiter keyed by
+// client IP. After adminLoginMaxFailures consecutive failures an IP is
+// refused for adminLoginCooldown; a successful login clears its state. The
+// entry map holds at most adminLoginMaxTracked keys (the least recently seen
+// entry without an active cooldown is evicted first), and every method is
+// safe for concurrent use: /login is unauthenticated and served by many
+// goroutines at once.
+//
+// now is the limiter's clock, read under mu on every call. Tests replace it
+// once, before the server serves a request, to drive the cooldown
+// deterministically; production leaves it at time.Now.
+type loginLimiter struct {
+ mu sync.Mutex
+ entries map[string]loginAttempts
+ now func() time.Time
+}
+
+type loginAttempts struct {
+ failures int
+ blockedTill time.Time
+ lastSeen time.Time
+}
+
+func newLoginLimiter() *loginLimiter {
+ return &loginLimiter{entries: make(map[string]loginAttempts), now: time.Now}
+}
+
+// allow reports whether ip may attempt a login now.
+func (l *loginLimiter) allow(ip string) bool {
+ l.mu.Lock()
+ defer l.mu.Unlock()
+ e, ok := l.entries[ip]
+ if !ok {
+ return true
+ }
+ return !l.now().Before(e.blockedTill)
+}
+
+// fail records a failed attempt for ip and starts the cooldown once the
+// threshold is reached. A still-blocked IP keeps its existing cooldown: a
+// blocked source cannot extend its own block by hammering the route, and an
+// address that cannot be tracked while every tracked address is cooling down
+// is left untracked rather than evicting a live cooldown.
+func (l *loginLimiter) fail(ip string) {
+ l.mu.Lock()
+ defer l.mu.Unlock()
+ now := l.now()
+ e, ok := l.entries[ip]
+ if !ok && !l.makeRoomLocked(now) {
+ return
+ }
+ e.lastSeen = now
+ if now.Before(e.blockedTill) {
+ l.entries[ip] = e
+ return
+ }
+ e.failures++
+ if e.failures >= adminLoginMaxFailures {
+ e.blockedTill = now.Add(adminLoginCooldown)
+ e.failures = 0
+ }
+ l.entries[ip] = e
+}
+
+// succeed clears ip's failure state after a successful login.
+func (l *loginLimiter) succeed(ip string) {
+ l.mu.Lock()
+ defer l.mu.Unlock()
+ delete(l.entries, ip)
+}
+
+// makeRoomLocked makes room for one new tracked address once the map is at
+// its cap, reporting false when it cannot. Fully expired entries (no cooldown
+// left and no failures counted) go first; if the map is still full the least
+// recently seen entry without an active cooldown is dropped.
+//
+// An entry with an active cooldown is NEVER evicted: a blocked source must not
+// be able to unblock itself by filling the map with other addresses, which is
+// exactly what dropping its (oldest) entry would hand it. When every tracked
+// address is still cooling down the map stays bounded and the new address is
+// simply not tracked.
+func (l *loginLimiter) makeRoomLocked(now time.Time) bool {
+ if len(l.entries) < adminLoginMaxTracked {
+ return true
+ }
+ for ip, e := range l.entries {
+ if e.failures == 0 && !now.Before(e.blockedTill) {
+ delete(l.entries, ip)
+ }
+ }
+ for len(l.entries) >= adminLoginMaxTracked {
+ oldest, found := "", false
+ for ip, e := range l.entries {
+ if now.Before(e.blockedTill) {
+ continue
+ }
+ if !found || e.lastSeen.Before(l.entries[oldest].lastSeen) {
+ oldest, found = ip, true
+ }
+ }
+ if !found {
+ return false
+ }
+ delete(l.entries, oldest)
+ }
+ return true
+}
+
+// limitLogins returns the server's login limiter, creating it on first use.
+func (s *AdminServer) limitLogins() *loginLimiter {
+ s.loginLimiterOnce.Do(func() { s.loginLimiter = newLoginLimiter() })
+ return s.loginLimiter
+}
+
+// clientIP keys the login limiter by the request's client address. RemoteAddr
+// is "host:port" for TCP transports; anything without a port (synthetic
+// requests, unix sockets) is used verbatim. Forwarded headers are ignored:
+// they are client-controlled and would let one source pose as many.
+func clientIP(remoteAddr string) string {
+ if host, _, err := net.SplitHostPort(remoteAddr); err == nil {
+ return host
+ }
+ return remoteAddr
+}
+
func (s *AdminServer) setAuthCookie(w http.ResponseWriter, account string) {
issued := strconv.FormatInt(time.Now().Unix(), 10)
mac := hmac.New(sha256.New, s.cookieSecret)
@@ -389,16 +578,44 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) {
return
}
+ // Refuse a cooling-down source before reading its body: the limit exists
+ // to make password guessing expensive, so it must not cost a parse.
+ limiter := s.limitLogins()
+ ip := clientIP(r.RemoteAddr)
+ if !limiter.allow(ip) {
+ http.Error(w, "too many failed login attempts; try again later", http.StatusTooManyRequests)
+ return
+ }
+
+ // /login is registered before authMiddleware, so bound and buffer its body
+ // here before r.FormValue can parse it: without the cap a multipart POST
+ // spills unbounded data to temp files (ParseMultipartForm's own 32 MiB cap
+ // only bounds memory).
+ body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, maxLoginBodyBytes))
+ r.Body.Close()
+ if err != nil {
+ var tooLarge *http.MaxBytesError
+ if errors.As(err, &tooLarge) {
+ http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
+ return
+ }
+ http.Error(w, "cannot read request body", http.StatusBadRequest)
+ return
+ }
+ r.Body = io.NopCloser(bytes.NewReader(body))
+
account := strings.TrimSpace(r.FormValue("account"))
password := r.FormValue("password")
acc, err := s.accountStore.LoadAccount(account)
if err != nil || !player.CheckPassword(password, acc.PasswordHash) {
+ limiter.fail(ip)
s.renderPage(w, r, "login.html", map[string]any{"Error": "Invalid account name or password."})
return
}
if !acc.Admin {
+ limiter.fail(ip)
s.renderPage(w, r, "login.html", map[string]any{"Error": "Account is not authorized as admin."})
return
}
@@ -432,6 +649,7 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) {
return
}
s.setAuthCookie(w, account)
+ limiter.succeed(ip)
http.Redirect(w, r, "/", http.StatusFound)
}
diff --git a/internal/admin/undo.go b/internal/admin/undo.go
index 1786295..7bfbe09 100644
--- a/internal/admin/undo.go
+++ b/internal/admin/undo.go
@@ -6,6 +6,7 @@ import (
"log"
"os"
"path/filepath"
+ "strings"
"sync"
"time"
)
@@ -77,6 +78,11 @@ type UndoStack struct {
// migrateRoomIDs applies a room-ID map to runtime state (players, Hub
// membership, casino room keys) during undo/redo of room-ID changes.
migrateRoomIDs func(map[int]int)
+ // playerFileOnlineCheck reports whether the character/account addressed
+ // by a data-relative player file path is currently online. undo/redo
+ // writes those files directly, so it must refuse the same files the raw
+ // file editor refuses. Optional; nil disables the refusal.
+ playerFileOnlineCheck func(relPath string) bool
}
func NewUndoStack(dataDir string) *UndoStack {
@@ -97,6 +103,16 @@ func (us *UndoStack) SetMigrateRoomIDs(fn func(map[int]int)) {
us.migrateRoomIDs = fn
}
+// SetPlayerFileOnlineCheck wires the online-session check used by undo/redo to
+// refuse touching a character/account file the game still owns. It takes the
+// same data-relative path form as AdminServer.SetPlayerFileOnlineCheck, which
+// passes its own hook through here.
+func (us *UndoStack) SetPlayerFileOnlineCheck(fn func(relPath string) bool) {
+ us.mu.Lock()
+ defer us.mu.Unlock()
+ us.playerFileOnlineCheck = fn
+}
+
func (us *UndoStack) Push(desc ChangeDesc) {
us.mu.Lock()
defer us.mu.Unlock()
@@ -172,6 +188,22 @@ func (us *UndoStack) Redo() *ChangeDesc {
// Extra-file failures propagate: a multi-file undo that restored only part
// of a change must be reported as failed, never as success.
func (us *UndoStack) applyUndo(c ChangeDesc) error {
+ if err := us.checkPlayerFilesOffline(c); err != nil {
+ return err
+ }
+ // Resolve every secret-bearing image BEFORE touching the filesystem: the
+ // rename path below removes its target before the restored file is
+ // written, so a redacted history image that cannot be repaired must fail
+ // while both files are still intact.
+ restored, err := us.restoreSecret(c.OldContent, c.FilePath, c.NewFilePath)
+ if err != nil {
+ return err
+ }
+ for _, ef := range c.ExtraFiles {
+ if _, err := us.restoreSecret(ef.OldContent, ef.FilePath, ef.NewFilePath); err != nil {
+ return fmt.Errorf("undo restore %s: %w", ef.FilePath, err)
+ }
+ }
if c.NewFilePath != "" {
if err := os.Remove(c.NewFilePath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("remove %s: %w", c.NewFilePath, err)
@@ -186,10 +218,7 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error {
if c.OldContent == nil {
return fmt.Errorf("restore deleted %s: no snapshot recorded", c.FilePath)
}
- if err := os.MkdirAll(filepath.Dir(c.FilePath), 0755); err != nil {
- return fmt.Errorf("mkdir: %w", err)
- }
- if err := os.WriteFile(c.FilePath, c.OldContent, 0644); err != nil {
+ if err := us.writeSnapshot(c.FilePath, restored); err != nil {
return fmt.Errorf("restore deleted %s: %w", c.FilePath, err)
}
}
@@ -198,15 +227,12 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error {
return fmt.Errorf("remove created %s: %w", c.FilePath, err)
}
} else {
- if err := os.MkdirAll(filepath.Dir(c.FilePath), 0755); err != nil {
- return fmt.Errorf("mkdir: %w", err)
- }
- if err := os.WriteFile(c.FilePath, c.OldContent, 0644); err != nil {
+ if err := us.writeSnapshot(c.FilePath, restored); err != nil {
return fmt.Errorf("revert %s: %w", c.FilePath, err)
}
}
for _, ef := range c.ExtraFiles {
- if err := applyUndoExtraFile(ef); err != nil {
+ if err := us.applyUndoExtraFile(ef); err != nil {
return err
}
}
@@ -216,6 +242,21 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error {
// applyRedo re-applies a change: the primary file, then every extra file.
// Extra-file failures propagate like undo.
func (us *UndoStack) applyRedo(c ChangeDesc) error {
+ if err := us.checkPlayerFilesOffline(c); err != nil {
+ return err
+ }
+ // See applyUndo: every image's secret is resolved before any file is
+ // removed, so a redacted image fails the redo without destroying either
+ // end of a rename.
+ restored, err := us.restoreSecret(c.NewContent, c.NewFilePath, c.FilePath)
+ if err != nil {
+ return err
+ }
+ for _, ef := range c.ExtraFiles {
+ if _, err := us.restoreSecret(ef.NewContent, ef.NewFilePath, ef.FilePath); err != nil {
+ return fmt.Errorf("redo restore %s: %w", ef.FilePath, err)
+ }
+ }
if c.NewFilePath != "" {
if err := os.Remove(c.FilePath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("remove %s: %w", c.FilePath, err)
@@ -226,14 +267,11 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error {
if c.NewFilePath != "" {
target = c.NewFilePath
}
- if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil {
- return fmt.Errorf("mkdir: %w", err)
- }
- if err := os.WriteFile(target, c.NewContent, 0644); err != nil {
+ if err := us.writeSnapshot(target, restored); err != nil {
return fmt.Errorf("recreate %s: %w", target, err)
}
for _, ef := range c.ExtraFiles {
- if err := applyRedoExtraFile(ef); err != nil {
+ if err := us.applyRedoExtraFile(ef); err != nil {
return err
}
}
@@ -242,24 +280,20 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error {
return fmt.Errorf("re-delete %s: %w", c.FilePath, err)
}
for _, ef := range c.ExtraFiles {
- if err := applyRedoExtraFile(ef); err != nil {
+ if err := us.applyRedoExtraFile(ef); err != nil {
return err
}
}
} else {
target := c.FilePath
- content := c.NewContent
if c.NewFilePath != "" {
target = c.NewFilePath
}
- if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil {
- return fmt.Errorf("mkdir: %w", err)
- }
- if err := os.WriteFile(target, content, 0644); err != nil {
+ if err := us.writeSnapshot(target, restored); err != nil {
return fmt.Errorf("reapply %s: %w", target, err)
}
for _, ef := range c.ExtraFiles {
- if err := applyRedoExtraFile(ef); err != nil {
+ if err := us.applyRedoExtraFile(ef); err != nil {
return err
}
}
@@ -267,7 +301,7 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error {
return nil
}
-func applyUndoExtraFile(ef ExtraFile) error {
+func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error {
switch {
case ef.IsCreate:
if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) {
@@ -281,67 +315,143 @@ func applyUndoExtraFile(ef ExtraFile) error {
if ef.OldContent == nil {
return fmt.Errorf("undo restore %s: no snapshot recorded", ef.FilePath)
}
- if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil {
- return fmt.Errorf("undo mkdir: %w", err)
- }
- if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil {
+ if err := us.writeRestored(ef.FilePath, ef.OldContent, ef.NewFilePath); err != nil {
return fmt.Errorf("undo restore %s: %w", ef.FilePath, err)
}
case ef.NewFilePath != "":
if err := os.Remove(ef.NewFilePath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("undo remove %s: %w", ef.NewFilePath, err)
}
- if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil {
- return fmt.Errorf("undo mkdir: %w", err)
- }
- if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil {
+ if err := us.writeRestored(ef.FilePath, ef.OldContent, ef.NewFilePath); err != nil {
return fmt.Errorf("undo restore %s: %w", ef.FilePath, err)
}
default:
- if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil {
- return fmt.Errorf("undo mkdir: %w", err)
- }
- if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil {
+ if err := us.writeRestored(ef.FilePath, ef.OldContent); err != nil {
return fmt.Errorf("undo revert %s: %w", ef.FilePath, err)
}
}
return nil
}
-func applyRedoExtraFile(ef ExtraFile) error {
+func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error {
switch {
case ef.IsDelete:
if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("redo remove %s: %w", ef.FilePath, err)
}
case ef.IsCreate:
- if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil {
- return fmt.Errorf("redo mkdir: %w", err)
- }
- if err := os.WriteFile(ef.FilePath, ef.NewContent, 0644); err != nil {
+ if err := us.writeRestored(ef.FilePath, ef.NewContent, ef.NewFilePath); err != nil {
return fmt.Errorf("redo create %s: %w", ef.FilePath, err)
}
case ef.NewFilePath != "":
if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("redo remove %s: %w", ef.FilePath, err)
}
- if err := os.MkdirAll(filepath.Dir(ef.NewFilePath), 0755); err != nil {
- return fmt.Errorf("redo mkdir: %w", err)
- }
- if err := os.WriteFile(ef.NewFilePath, ef.NewContent, 0644); err != nil {
+ if err := us.writeRestored(ef.NewFilePath, ef.NewContent, ef.FilePath); err != nil {
return fmt.Errorf("redo create %s: %w", ef.NewFilePath, err)
}
default:
- if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil {
- return fmt.Errorf("redo mkdir: %w", err)
- }
- if err := os.WriteFile(ef.FilePath, ef.NewContent, 0644); err != nil {
+ if err := us.writeRestored(ef.FilePath, ef.NewContent); err != nil {
return fmt.Errorf("redo reapply %s: %w", ef.FilePath, err)
}
}
return nil
}
+// writeRestored writes content to path after merging the live on-disk player
+// secret back into it (see restoreSecret). A history image whose secret was
+// redacted and has no live file left to take it from fails here instead of
+// being written, so undo/redo reports the failure. Further paths are the
+// change's counterparts (a rename's other end).
+func (us *UndoStack) writeRestored(path string, content []byte, others ...string) error {
+ restored, err := us.restoreSecret(content, append([]string{path}, others...)...)
+ if err != nil {
+ return err
+ }
+ return us.writeSnapshot(path, restored)
+}
+
+// checkPlayerFilesOffline refuses an undo/redo touching a character/account
+// file whose owner is online. The raw file editor refuses those files before
+// writing them; undo/redo writes them through this stack, so it must refuse
+// them too or a live character can be clobbered behind the editor's back.
+func (us *UndoStack) checkPlayerFilesOffline(c ChangeDesc) error {
+ if us.playerFileOnlineCheck == nil {
+ return nil
+ }
+ for _, p := range changePaths(c) {
+ rel, ok := us.playerDataRelPath(p)
+ if !ok {
+ continue
+ }
+ if us.playerFileOnlineCheck(rel) {
+ return fmt.Errorf("%s is online; it must be logged out before this change can be undone or redone", rel)
+ }
+ }
+ return nil
+}
+
+// changePaths lists every file a change touches: the primary path, a rename's
+// new path, and every extra file with its own new path.
+func changePaths(c ChangeDesc) []string {
+ paths := []string{c.FilePath, c.NewFilePath}
+ for _, ef := range c.ExtraFiles {
+ paths = append(paths, ef.FilePath, ef.NewFilePath)
+ }
+ return paths
+}
+
+// writeSnapshot writes content to path, creating the parent directory and
+// choosing the file mode from the path: player/account files are owner-only.
+func (us *UndoStack) writeSnapshot(path string, content []byte) error {
+ if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
+ return err
+ }
+ if err := os.WriteFile(path, content, us.fileMode(path)); err != nil {
+ return err
+ }
+ if us.isPlayerDataPath(path) {
+ // Tighten a file that was created world-readable before this fix.
+ if err := os.Chmod(path, 0600); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// fileMode returns the mode an undo/redo write should use for path.
+func (us *UndoStack) fileMode(path string) os.FileMode {
+ if us.isPlayerDataPath(path) {
+ return 0600
+ }
+ return 0644
+}
+
+// isPlayerDataPath reports whether path is an account or character file under
+// the data directory. These carry the password hash and live player state.
+func (us *UndoStack) isPlayerDataPath(path string) bool {
+ _, ok := us.playerDataRelPath(path)
+ return ok
+}
+
+// playerDataRelPath returns the data-relative, slash-separated form of path
+// when it is an account or character file, which is the form the game's
+// online-session check (AdminPlayerFileOnline) accepts.
+func (us *UndoStack) playerDataRelPath(path string) (string, bool) {
+ if path == "" {
+ return "", false
+ }
+ rel, err := filepath.Rel(us.dataDir, path)
+ if err != nil {
+ return "", false
+ }
+ rel = filepath.ToSlash(rel)
+ if strings.HasPrefix(rel, "players/accounts/") || strings.HasPrefix(rel, "players/characters/") {
+ return rel, true
+ }
+ return "", false
+}
+
func (us *UndoStack) Info() UndoInfo {
us.mu.Lock()
defer us.mu.Unlock()
@@ -395,8 +505,8 @@ func (us *UndoStack) save() {
extra = append(extra, extraEntry{
FilePath: ef.FilePath,
NewFilePath: ef.NewFilePath,
- OldContent: string(ef.OldContent),
- NewContent: string(ef.NewContent),
+ OldContent: string(us.redactContent(ef.OldContent, ef.FilePath, ef.NewFilePath)),
+ NewContent: string(us.redactContent(ef.NewContent, ef.FilePath, ef.NewFilePath)),
IsDelete: ef.IsDelete,
IsCreate: ef.IsCreate,
IsDirDelete: ef.IsDirDelete,
@@ -407,8 +517,8 @@ func (us *UndoStack) save() {
Description: c.Description,
FilePath: c.FilePath,
NewFilePath: c.NewFilePath,
- OldContent: string(c.OldContent),
- NewContent: string(c.NewContent),
+ OldContent: string(us.redactContent(c.OldContent, c.FilePath, c.NewFilePath)),
+ NewContent: string(us.redactContent(c.NewContent, c.FilePath, c.NewFilePath)),
IsDelete: c.IsDelete,
IsCreate: c.IsCreate,
IsDirDelete: c.IsDirDelete,
@@ -427,9 +537,124 @@ func (us *UndoStack) save() {
log.Printf("undo: failed to marshal history: %v", err)
return
}
- if err := os.WriteFile(us.filePath, b, 0644); err != nil {
+ if err := os.WriteFile(us.filePath, b, 0600); err != nil {
log.Printf("undo: failed to write history file: %v", err)
+ return
+ }
+ // os.WriteFile does not change an existing file's mode: tighten a history
+ // file that a previous version created world-readable.
+ if err := os.Chmod(us.filePath, 0600); err != nil {
+ log.Printf("undo: failed to secure history file: %v", err)
+ }
+}
+
+// playerSecretKeys are the top-level YAML keys whose values must never be
+// written to the on-disk undo history. Account password hashes are the
+// primary case: the raw file editor exposes account files and an unredacted
+// history file would leak every hash to any local reader (and be stageable
+// into the repository).
+var playerSecretKeys = []string{"password_hash"}
+
+// redactContent replaces secret values with a placeholder before the content
+// is serialized into the history file. The in-memory ChangeDesc keeps the
+// real bytes; only the persisted copy is redacted. A redacted value cannot
+// restore a deleted account's hash across a restart, which is an accepted
+// trade for never storing credentials in the history.
+func (us *UndoStack) redactContent(content []byte, paths ...string) []byte {
+ if content == nil {
+ return nil
+ }
+ for _, p := range paths {
+ if us.isPlayerDataPath(p) {
+ return redactPlayerSecrets(content)
+ }
+ }
+ return content
+}
+
+// redactPlayerSecrets replaces every known secret value in a player/account
+// document with the editor's placeholder, returning content unchanged when
+// the document does not parse or carries no secret.
+func redactPlayerSecrets(content []byte) []byte {
+ m, err := yamlToMap(content)
+ if err != nil {
+ return content
+ }
+ changed := false
+ for _, key := range playerSecretKeys {
+ if _, ok := m[key]; ok {
+ m[key] = redactedAccountHash
+ changed = true
+ }
+ }
+ if !changed {
+ return content
+ }
+ out, err := marshalMapYAML(m)
+ if err != nil {
+ return content
+ }
+ return out
+}
+
+// restoreSecret merges the live on-disk password hash into content before an
+// undo/redo overwrites an account file, so a hash that was redacted in the
+// persisted history can never blank a live account after a restart. It looks
+// at the primary target path and then the change's other path (a rename).
+//
+// When no path holds a live hash and content still carries the redaction
+// placeholder, it returns an error: writing the placeholder would replace the
+// account's real hash and lock both the owner and the raw editor out of it
+// permanently, so undo/redo must report the failure instead of overwriting.
+func (us *UndoStack) restoreSecret(content []byte, paths ...string) ([]byte, error) {
+ if content == nil {
+ return nil, nil
+ }
+ playerFile := false
+ for _, p := range paths {
+ if us.isPlayerDataPath(p) {
+ playerFile = true
+ break
+ }
+ }
+ if !playerFile {
+ return content, nil
+ }
+ cm, err := yamlToMap(content)
+ if err != nil {
+ return content, nil
+ }
+ hash, ok := cm["password_hash"]
+ if !ok {
+ return content, nil
+ }
+ for _, p := range paths {
+ if !us.isPlayerDataPath(p) {
+ continue
+ }
+ existing, err := os.ReadFile(p)
+ if err != nil {
+ continue
+ }
+ em, err := yamlToMap(existing)
+ if err != nil {
+ continue
+ }
+ live, ok := em["password_hash"]
+ if !ok {
+ continue
+ }
+ cm["password_hash"] = live
+ out, err := marshalMapYAML(cm)
+ if err != nil {
+ return content, nil
+ }
+ return out, nil
+ }
+ if fmt.Sprint(hash) == redactedAccountHash {
+ return nil, fmt.Errorf("the stored history image has a redacted password hash and no live account file to take it from")
}
+ return content, nil
}
// load restores persisted undo/redo history written by save. A missing or
generated by cgit v1.2.3 (git 2.25.1) at 2026-10-24 12:39:12 +0000