feat: sym-link containment, batch count prompt for crafting

5 files changed, 128 insertions, 8 deletions

diff --git a/FIXES.md b/FIXES.md
index b4c9198..45e0fab 100644
--- a/FIXES.md
+++ b/FIXES.md
@@ -100,16 +100,21 @@ Nothing was staged or committed. The audit artifacts (`AUDIT.md`, `reports/`) ar
 - **Validation**: craft `type`/`subtype` is checked against `item.KnownCraftKeys` — an unknown key left the action set but never advancing (player stuck). Test: `TestValidateCraftBlocksRejectsUnknownType`.
 - **Validation**: the mob/safespot size vocabulary moved to `object.MobSizeRank`; `validateObjects` rejects an unknown `safespot.max_block_size` (silently disables cover) and out-of-range chances, and `validateMobCombatStats` rejects an unknown mob `size`. Test: `TestValidateObjectsRejectsUnknownSafespotSize`.
 
+## Tranche 7 (this pass)
+
+- **Security**: `resolveDataPath` now resolves symlinks on the longest existing prefix and re-checks containment, so a symlink inside `data/` cannot redirect a raw-editor read or write outside the data directory. Test: `TestResolveDataPathRejectsSymlinkEscape`.
+- **UX**: the production "how many" prompt now says "How many batches (each makes N, return for all)?" so a batch count cannot silently mean N× the materials/units.
+- **Test coverage**: added `TestGatherYieldsDrop`, an end-to-end gather test (seed object + pickaxe, start the action, tick to a drop) — the first coverage for the gather system the audit flagged.
+- **Investigated (no change)**: the craps Fire bet. Settling it at the 4th/5th point as the audit suggested would cap the payout at the lower tier and under-pay; the bet must ride to the seven-out to pay the highest tier reached. Left as-is and noted here.
+
 ## Still open
 
 - **Explicit undo snapshot-existence flag** (the current fix safely refuses an intentionally empty before-image).
-- **Production "how many" batches vs units**.
 - **Persistence** `Recover` mid-game misuse.
-- **Casino**: craps Fire/ATS settlement deferral.
-- **Admin**: symlink handling in the raw file API.
+- **Casino**: craps Fire bet forfeiture on disconnect (design decision; shutdown refunds).
 - **Structural refactors** remaining from `AUDIT.md` §3–4 (`ui.Table` shrink consolidation, `tableCore` for pai gow, `refs` struct in validate, `remapInt`/`remapScalar`).
-- **Test infrastructure**: CI wiring for `test-race`, a master-tick harness option, and coverage for steal/gather/burn/talk.
+- **Test infrastructure**: CI wiring for `test-race`, a master-tick harness option, and coverage for steal/burn/talk.
 
 ## Suggested next step
 
-Tranche 7: the remaining structural refactors (`ui.Table`, `tableCore` for pai gow, `refs` struct, `remapInt`/`remapScalar`) and the test-infrastructure work. Keep `make lint`, `go test ./...`, and `go test -race` green.
+Tranche 8: the remaining structural refactors and the steal/burn/talk test coverage. Keep `make lint`, `go test ./...`, and `go test -race` green.
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go
index b1b3fff..9745b70 100644
--- a/internal/admin/api_files.go
+++ b/internal/admin/api_files.go
@@ -13,14 +13,61 @@ import (
 )
 
 // resolveDataPath resolves a request-supplied path inside the admin data
-// directory, rejecting paths that would escape it via .. segments. The
-// returned path is always filepath.Join(dataDir, cleaned relative path).
+// directory, rejecting paths that would escape it via .. segments or a symlink.
+// The returned path is always filepath.Join(dataDir, cleaned relative path).
 func resolveDataPath(dataDir, relPath string) (string, error) {
 	clean := filepath.Clean(strings.TrimPrefix(strings.TrimPrefix(relPath, "/"), "data/"))
 	if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) {
 		return "", fmt.Errorf("path escapes the data directory: %s", relPath)
 	}
-	return filepath.Join(dataDir, clean), nil
+	full := filepath.Join(dataDir, clean)
+	// Defence in depth: a symlink inside data/ could redirect a read or write
+	// outside it, so resolve the longest existing prefix and re-check
+	// containment. The root is resolved too, since the data dir itself may be
+	// a symlink (e.g. a mounted volume).
+	resolved, err := evalExisting(full)
+	if err != nil {
+		return "", fmt.Errorf("resolve path %s: %w", relPath, err)
+	}
+	root := dataDir
+	if r, err := filepath.EvalSymlinks(dataDir); err == nil {
+		root = r
+	}
+	if !withinDir(root, resolved) {
+		return "", fmt.Errorf("path escapes the data directory via a symlink: %s", relPath)
+	}
+	return full, nil
+}
+
+// evalExisting resolves symlinks on the longest existing prefix of path and
+// re-appends the not-yet-existing suffix, so a create target is still checked.
+func evalExisting(path string) (string, error) {
+	p := path
+	var suffix []string
+	for {
+		resolved, err := filepath.EvalSymlinks(p)
+		if err == nil {
+			for i := len(suffix) - 1; i >= 0; i-- {
+				resolved = filepath.Join(resolved, suffix[i])
+			}
+			return resolved, nil
+		}
+		if !os.IsNotExist(err) {
+			return "", err
+		}
+		parent := filepath.Dir(p)
+		if parent == p {
+			return path, nil
+		}
+		suffix = append(suffix, filepath.Base(p))
+		p = parent
+	}
+}
+
+// withinDir reports whether path lies inside dir.
+func withinDir(dir, path string) bool {
+	rel, err := filepath.Rel(dir, path)
+	return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
 }
 
 // canonicalDataRelPath normalizes a client-supplied data-relative path so that
diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go
index d97732d..d7b713e 100644
--- a/internal/admin/correct_audit_fixes_test.go
+++ b/internal/admin/correct_audit_fixes_test.go
@@ -73,3 +73,19 @@ func TestUndoFailureIsReportedDistinctly(t *testing.T) {
 		t.Fatal("empty history must be a clean nil change and nil error")
 	}
 }
+
+// Regression: a symlink inside data/ pointing outside it must not let the raw
+// file API read or write outside the data directory.
+func TestResolveDataPathRejectsSymlinkEscape(t *testing.T) {
+	dir := t.TempDir()
+	outside := t.TempDir()
+	if err := os.Symlink(outside, filepath.Join(dir, "link")); err != nil {
+		t.Skipf("symlinks unsupported: %v", err)
+	}
+	if _, err := resolveDataPath(dir, "link/secret.yaml"); err == nil {
+		t.Fatal("a symlink escaping the data directory was accepted")
+	}
+	if _, err := resolveDataPath(dir, "rooms/1.yaml"); err != nil {
+		t.Fatalf("normal path rejected: %v", err)
+	}
+}
diff --git a/internal/game/correct_audit_fixes_test.go b/internal/game/correct_audit_fixes_test.go
index 6177eac..c32b1e5 100644
--- a/internal/game/correct_audit_fixes_test.go
+++ b/internal/game/correct_audit_fixes_test.go
@@ -1,6 +1,7 @@
 package game
 
 import (
+	"path/filepath"
 	"testing"
 
 	"thehouseoficarus/internal/behavior"
@@ -126,6 +127,45 @@ func TestUsableCraftIndexSelectsSatisfiedRecipe(t *testing.T) {
 	}
 }
 
+// End-to-end gather: the missing coverage the audit called out. Start a mine
+// action on a seeded rock with a pickaxe and tick it to a drop.
+func TestGatherYieldsDrop(t *testing.T) {
+	h := newHarness(t)
+	h.seedAccount("alice")
+	p := h.seedCharacter("alice", "alice")
+	writeSeedFile(t, filepath.Join(h.dir, "objects", "copper_rock.yaml"),
+		[]byte("id: copper_rock\nname: copper rock\ngather:\n  skill: mining\n  tools: [pickaxe]\n  success: {base: 1, cap: 1}\n  gather_message: You mine the rock.\n  drops:\n    - item_id: copper_ore\n      weight: 100\n"))
+	writeSeedFile(t, filepath.Join(h.dir, "items", "pickaxe.yaml"),
+		[]byte("id: pickaxe\nname: pickaxe\ntool:\n  type: pickaxe\n  speed: 1\n"))
+	writeSeedFile(t, filepath.Join(h.dir, "items", "copper_ore.yaml"),
+		[]byte("id: copper_ore\nname: copper ore\nstackable: true\n"))
+	sess := &net.Session{Conn: newChanConn(), Player: p}
+	sess.SetState(net.StateGame)
+	h.g.RunOnOwner(func() {
+		h.g.ItemStore.Reload(h.dir)
+		h.g.ObjectStore.Reload(h.dir)
+		h.g.World.EnsureObjectStates(1, []string{"copper_rock"})
+		p.SetInvSlot(0, &player.InventorySlot{ItemID: "pickaxe", Quantity: 1})
+		obj, _, err := h.g.resolveObjectDef(1, "copper_rock")
+		if err != nil {
+			t.Fatalf("resolve object: %v", err)
+		}
+		st := h.g.World.GetObjState(1, "copper_rock", 0)
+		if st == nil {
+			t.Fatal("no object state after EnsureObjectStates")
+		}
+		h.g.startGather(sess, p, obj, st)
+		if p.Action == nil {
+			t.Fatalf("gather action did not start: %s", sess.Conn.(*chanConn).Bytes())
+		}
+		h.g.advanceGather(sess, p) // step 0: announce
+		h.g.advanceGather(sess, p) // step 1: resolve the drop
+		if got := p.CountItem("copper_ore"); got == 0 {
+			t.Fatalf("gather yielded no ore (output: %s)", sess.Conn.(*chanConn).Bytes())
+		}
+	})
+}
+
 func TestPlayerDataFileNameCanonicalizesPrefixes(t *testing.T) {
 	for _, in := range []string{
 		"players/characters/alice.yaml",
diff --git a/internal/game/production_menu.go b/internal/game/production_menu.go
index 5d712da..ad44853 100644
--- a/internal/game/production_menu.go
+++ b/internal/game/production_menu.go
@@ -290,6 +290,18 @@ func itemIDs(defs []*item.ItemDef) []string {
 func (g *Game) promptHowMany(sess *net.Session, itemID string) {
 	sess.PendingItemID = itemID
 	sess.SetState(net.StateHowMany)
+	yield := 1
+	if item := g.loadCraftItem(itemID); item != nil {
+		if c := g.usableCraft(sess.Player, item); c != nil && c.OutputQty > 0 {
+			yield = c.OutputQty
+		}
+	}
+	// The count is batches, not units: say so and show the per-batch yield so
+	// "15" cannot silently mean 225 arrow shafts.
+	if yield > 1 {
+		sess.Write(fmt.Sprintf("How many batches (each makes %d, return for all)?: ", yield))
+		return
+	}
 	sess.Write("How many (return for all)?: ")
 }