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

2 files changed, 102 insertions, 0 deletions

diff --git a/internal/item/hardening_id_mismatch_test.go b/internal/item/hardening_id_mismatch_test.go
new file mode 100644
index 0000000..5123fc0
--- /dev/null
+++ b/internal/item/hardening_id_mismatch_test.go
@@ -0,0 +1,88 @@
+package item
+
+import (
+	"bytes"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Regression: an item file whose declared id: disagrees with its filename was
+// silently loaded under the filename with no diagnostic. ItemStore must warn,
+// because the author has no other sign that the id they wrote is ignored.
+
+func captureLog(t *testing.T, fn func()) string {
+	t.Helper()
+	var buf bytes.Buffer
+	old := log.Writer()
+	log.SetOutput(&buf)
+	defer log.SetOutput(old)
+	fn()
+	return buf.String()
+}
+
+func writeItemFile(t *testing.T, dir, name, body string) {
+	t.Helper()
+	path := filepath.Join(dir, "items", name)
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(path, []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestItemStoreWarnsOnDeclaredIDMismatch(t *testing.T) {
+	dir := t.TempDir()
+	writeItemFile(t, dir, "foo.yaml", "id: bar\nname: Foo\n")
+	s := NewItemStore(dir)
+
+	out := captureLog(t, func() {
+		def, err := s.Load("foo")
+		if err != nil {
+			t.Fatalf("load: %v", err)
+		}
+		if def.ID != "foo" {
+			t.Fatalf("filename must win: ID = %q, want foo", def.ID)
+		}
+	})
+	if !strings.Contains(out, "declares id") {
+		t.Fatalf("expected an id-mismatch warning, got log %q", out)
+	}
+}
+
+func TestItemStoreDoesNotWarnWhenIDMatchesOrAbsent(t *testing.T) {
+	dir := t.TempDir()
+	writeItemFile(t, dir, "foo.yaml", "id: foo\nname: Foo\n")
+	writeItemFile(t, dir, "baz.yaml", "name: Baz\n")
+	s := NewItemStore(dir)
+
+	out := captureLog(t, func() {
+		if _, err := s.Load("foo"); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := s.Load("baz"); err != nil {
+			t.Fatal(err)
+		}
+	})
+	if strings.Contains(out, "declares id") {
+		t.Fatalf("unexpected id-mismatch warning: %q", out)
+	}
+}
+
+func TestItemStoreLoadAllWarnsOnDeclaredIDMismatch(t *testing.T) {
+	dir := t.TempDir()
+	writeItemFile(t, dir, "foo.yaml", "id: bar\nname: Foo\n")
+	s := NewItemStore(dir)
+
+	out := captureLog(t, func() {
+		if _, err := s.LoadAll(); err != nil {
+			t.Fatal(err)
+		}
+	})
+	if !strings.Contains(out, "declares id") {
+		t.Fatalf("expected an id-mismatch warning from LoadAll, got %q", out)
+	}
+}
diff --git a/internal/item/store.go b/internal/item/store.go
index e50e6e9..12aa729 100644
--- a/internal/item/store.go
+++ b/internal/item/store.go
@@ -2,6 +2,7 @@ package item
 
 import (
 	"fmt"
+	"log"
 	"os"
 	"path/filepath"
 	"sync"
@@ -44,6 +45,7 @@ func (s *ItemStore) Load(id string) (*ItemDef, error) {
 	if err := yaml.Unmarshal(data, &def); err != nil {
 		return nil, fmt.Errorf("parse item %s: %w", id, err)
 	}
+	warnDeclaredIDMismatch("item", path, id, def.ID)
 	def.ID = id
 	s.cache[id] = &def
 	return &def, nil
@@ -63,6 +65,7 @@ func (s *ItemStore) LoadAll() ([]*ItemDef, error) {
 		if err := yaml.Unmarshal(data, &def); err != nil {
 			return fmt.Errorf("parse item %s: %w", id, err)
 		}
+		warnDeclaredIDMismatch("item", path, id, def.ID)
 		def.ID = id
 		s.cache[id] = &def
 		defs = append(defs, &def)
@@ -71,6 +74,17 @@ func (s *ItemStore) LoadAll() ([]*ItemDef, error) {
 	return defs, err
 }
 
+// warnDeclaredIDMismatch logs when a data file declares an id: that disagrees
+// with its filename. IDs come from the filename (def.ID = id above), so the
+// declared value is silently ignored at runtime; without this warning an author
+// sees no sign that the id they wrote is dead.
+func warnDeclaredIDMismatch(kind, path, filenameID, declaredID string) {
+	if declaredID != "" && declaredID != filenameID {
+		log.Printf("%s %s: file declares id %q but is loaded as %q; the filename is authoritative and the declared id is ignored",
+			kind, path, declaredID, filenameID)
+	}
+}
+
 // PathIndex returns a copy of the id→path index, safe to iterate concurrently.
 func (s *ItemStore) PathIndex() map[string]string {
 	s.mu.Lock()