diff options
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/admin/api_files.go | 53 | ||||
| -rw-r--r-- | internal/admin/correct_audit_fixes_test.go | 16 | ||||
| -rw-r--r-- | internal/game/correct_audit_fixes_test.go | 40 | ||||
| -rw-r--r-- | internal/game/production_menu.go | 12 |
4 files changed, 118 insertions, 3 deletions
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)?: ") } |
