From 476384206ecb8f0ea250b5f6092cc35406d353f0 Mon Sep 17 00:00:00 2001
From: prosolis <5590409+prosolis@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:05:24 -0700
Subject: [PATCH] N4/E2 review fixes: close a gift-enabled equip dupe; harden
 vault writes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A high-effort code review of the gifting/vault work turned up three issues:

1. Item duplication (real exploit). The masterwork/arena equip confirmation
   captures an item at prompt time and equips it on "yes" without re-checking
   ownership. Since MasterworkGear/ArenaGear are now giftable, a player could
   !give the item away in the window, then confirm — minting a copy onto
   themselves while the recipient kept theirs. Fixed by taking the user lock
   (making gift-vs-confirm atomic) and re-loading the inventory to refuse an
   item that is no longer ours. Magic items are exempt (not giftable); other
   delete paths load-and-remove within one locked invocation.

2. clearAdvInventory read the vault-filtered list but its DELETE wiped every
   row including vaulted ones — no live caller today, but it would break the
   vault's "safe from !sell all" promise the instant sell-all routed through
   it. Delete now matches the read (in_vault = 0).

3. vault store/take took no per-user lock, so two near-simultaneous stores
   could both pass the capacity check and overfill the 10-slot vault. Now
   serialized on the same user lock the gift path uses.

go test ./... green; golden byte-identical.

Claude-Session: https://claude.ai/code/session_017mEwUmmS7aQTP2NQXj6rUa
---
 internal/plugin/adventure_character.go  |  5 ++++-
 internal/plugin/adventure_masterwork.go | 25 +++++++++++++++++++++++++
 internal/plugin/adventure_vault.go      |  8 ++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/internal/plugin/adventure_character.go b/internal/plugin/adventure_character.go
index d17f9a4..fcd3db1 100644
--- a/internal/plugin/adventure_character.go
+++ b/internal/plugin/adventure_character.go
@@ -613,7 +613,10 @@ func clearAdvInventory(userID id.UserID) ([]AdvItem, error) {
 		return nil, err
 	}
 	d := db.Get()
-	_, err = d.Exec(`DELETE FROM adventure_inventory WHERE user_id = ?`, string(userID))
+	// Delete only what loadAdvInventory returned — vaulted rows are out of play
+	// and must survive a bulk clear, or the vault's whole promise (safe from
+	// !sell all) breaks the moment any caller routes sell-all through here.
+	_, err = d.Exec(`DELETE FROM adventure_inventory WHERE user_id = ? AND in_vault = 0`, string(userID))
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/plugin/adventure_masterwork.go b/internal/plugin/adventure_masterwork.go
index f40d9b9..1320943 100644
--- a/internal/plugin/adventure_masterwork.go
+++ b/internal/plugin/adventure_masterwork.go
@@ -492,8 +492,33 @@ func (p *AdventurePlugin) handleMasterworkEquipConfirm(ctx MessageContext, inter
 		return p.SendDM(ctx.Sender, "Equip cancelled.")
 	}
 
+	// Serialize against the sender's own !give: MasterworkGear/ArenaGear are
+	// giftable (N4/E2), so the item captured at prompt time may have changed
+	// hands before this confirmation. The lock makes gift-vs-confirm atomic,
+	// and the re-load below refuses an item that is no longer ours — otherwise
+	// equipping the captured copy would mint a duplicate the gift already
+	// handed to the recipient.
+	userMu := p.advUserLock(ctx.Sender)
+	userMu.Lock()
+	defer userMu.Unlock()
+
 	selected := data.Item
 
+	inv, err := loadAdvInventory(ctx.Sender)
+	if err != nil {
+		return p.SendDM(ctx.Sender, "Failed to load your inventory.")
+	}
+	stillOwned := false
+	for _, it := range inv {
+		if it.ID == selected.ID {
+			stillOwned = true
+			break
+		}
+	}
+	if !stillOwned {
+		return p.SendDM(ctx.Sender, fmt.Sprintf("**%s** isn't in your inventory anymore. Equip cancelled.", selected.Name))
+	}
+
 	equip, err := loadAdvEquipment(ctx.Sender)
 	if err != nil {
 		return p.SendDM(ctx.Sender, "Failed to load equipment.")
diff --git a/internal/plugin/adventure_vault.go b/internal/plugin/adventure_vault.go
index 45f5f6d..97b56ff 100644
--- a/internal/plugin/adventure_vault.go
+++ b/internal/plugin/adventure_vault.go
@@ -48,9 +48,17 @@ func (p *AdventurePlugin) handleVaultCmd(ctx MessageContext, args string) error
 	case lower == "" || lower == "list":
 		reply = renderVault(ctx.Sender)
 	case strings.HasPrefix(lower, "store "):
+		// Serialize a player's own store/take so two near-simultaneous stores
+		// can't both pass the capacity check and overfill the vault.
+		userMu := p.advUserLock(ctx.Sender)
+		userMu.Lock()
 		reply = vaultStoreItem(ctx.Sender, strings.TrimSpace(args[len("store "):]))
+		userMu.Unlock()
 	case strings.HasPrefix(lower, "take "):
+		userMu := p.advUserLock(ctx.Sender)
+		userMu.Lock()
 		reply = vaultTakeItem(ctx.Sender, strings.TrimSpace(args[len("take "):]))
+		userMu.Unlock()
 	default:
 		reply = "Usage: `!adventure vault` to view · `!adventure vault store <item>` · `!adventure vault take <item>`."
 	}