feat: Enhance inventory management with server-side validation and CSRF protection

app/controllers/break_escape/games_controller.rb

@@ -100,11 +100,13 @@ module BreakEscape
           return render_error("Room not accessible: #{room_id}", :forbidden)
         end
 
-        # Auto-add start room if missing (defensive programming)
-        if is_start_room && !is_unlocked
+        # Auto-add room to unlockedRooms when accessed
+        # This ensures items in the room can be collected
+        if !is_unlocked
           @game.player_state['unlockedRooms'] ||= []
-          @game.player_state['unlockedRooms'] << room_id
+          @game.player_state['unlockedRooms'] << room_id unless @game.player_state['unlockedRooms'].include?(room_id)
           @game.save!
+          Rails.logger.info "[BreakEscape] Auto-unlocked room #{room_id} on access"
         end
 
         # Get and filter room data
@@ -284,16 +286,21 @@ module BreakEscape
       action_type = params[:action_type] || params[:actionType]
       item = params[:item]
 
+      Rails.logger.info "[BreakEscape] inventory endpoint: action=#{action_type}, item=#{item.inspect}"
+
       case action_type
       when 'add'
         # Validate item exists and is collectible
         validation_error = validate_item_collectible(item)
         if validation_error
+          Rails.logger.warn "[BreakEscape] inventory validation failed: #{validation_error}"
           return render json: { success: false, message: validation_error },
                        status: :unprocessable_entity
         end
 
+        Rails.logger.info "[BreakEscape] Adding item to inventory: #{item['type']} / #{item['name']}"
         @game.add_inventory_item!(item.to_unsafe_h)
+        Rails.logger.info "[BreakEscape] Item added successfully. Current inventory: #{@game.player_state['inventory'].inspect}"
         render json: { success: true, inventory: @game.player_state['inventory'] }
 
       when 'remove'
@@ -437,56 +444,122 @@ module BreakEscape
 
     def validate_item_collectible(item)
       item_type = item['type']
-      item_id = item['id']
+      # Use key_id for keys (more unique), fall back to id for other items
+      item_id = item['key_id'] || item['id']
+      item_name = item['name']
 
-      # Search scenario for this item
-      found_item = find_item_in_scenario(item_type, item_id)
-      return "Item not found in scenario: #{item_type}" unless found_item
+      Rails.logger.info "[BreakEscape] validate_item_collectible: type=#{item_type}, id=#{item_id}, name=#{item_name}"
+
+      # Check if this is a starting item first (if so, skip all other checks)
+      is_starting_item = @game.scenario_data['startItemsInInventory']&.any? do |start_item|
+        start_item['type'] == item_type && (start_item['id'] == item_id || start_item['name'] == item_name)
+      end
+
+      if is_starting_item
+        Rails.logger.info "[BreakEscape] Item is a starting item, skipping room/container checks"
+        return nil # Starting items are always valid
+      end
+
+      # Search for item, prioritizing accessible locations (not locked containers/rooms)
+      found_item_info = find_accessible_item(item_type, item_id, item_name)
+
+      unless found_item_info
+        error_msg = "Item not found in scenario: #{item_type}"
+        Rails.logger.warn "[BreakEscape] #{error_msg}"
+        return error_msg
+      end
+
+      found_item = found_item_info[:item]
+      location = found_item_info[:location]
 
       # Check if item is takeable
       unless found_item['takeable']
-        return "Item is not takeable: #{found_item['name']}"
+        error_msg = "Item is not takeable: #{found_item['name']}"
+        Rails.logger.warn "[BreakEscape] #{error_msg}"
+        return error_msg
       end
 
-      # If item is in locked container, check if container is unlocked
-      container_info = find_item_container(item_type, item_id)
-      if container_info.present?
-        container_id = container_info[:id]
+      # Check access based on location type
+      if location[:type] == 'container'
+        container_id = location[:container_id]
         unless @game.player_state['unlockedObjects'].include?(container_id)
-          return "Container not unlocked: #{container_id}"
+          error_msg = "Container not unlocked: #{container_id}"
+          Rails.logger.warn "[BreakEscape] #{error_msg}"
+          return error_msg
         end
-      end
-
-      # If item is in locked room, check if room is unlocked
-      room_info = find_item_room(item_type, item_id)
-      if room_info.present?
-        room_id = room_info[:id]
-        if room_info[:locked] && !@game.player_state['unlockedRooms'].include?(room_id)
-          return "Room not unlocked: #{room_id}"
+      elsif location[:type] == 'room'
+        room_id = location[:room_id]
+        room_info = @game.scenario_data['rooms'][room_id]
+        if room_info && room_info['locked'] && !@game.player_state['unlockedRooms'].include?(room_id)
+          error_msg = "Room not unlocked: #{room_id}"
+          Rails.logger.warn "[BreakEscape] #{error_msg}"
+          return error_msg
         end
-      end
-
-      # Check if NPC holds this item and if NPC encountered
-      npc_info = find_npc_holding_item(item_type, item_id)
-      if npc_info.present?
-        npc_id = npc_info[:id]
+      elsif location[:type] == 'npc'
+        npc_id = location[:npc_id]
         unless @game.player_state['encounteredNPCs'].include?(npc_id)
-          return "NPC not encountered: #{npc_id}"
+          error_msg = "NPC not encountered: #{npc_id}"
+          Rails.logger.warn "[BreakEscape] #{error_msg}"
+          return error_msg
         end
       end
 
+      Rails.logger.info "[BreakEscape] Item collection valid: #{item_type}"
       nil # No error
     end
 
-    def find_item_in_scenario(item_type, item_id)
+    def find_accessible_item(item_type, item_id, item_name)
+      # Priority 1: Items in unlocked rooms (most accessible)
+      @game.scenario_data['rooms'].each do |room_id, room_data|
+        if room_data['locked'] == false || @game.player_state['unlockedRooms'].include?(room_id)
+          room_data['objects']&.each do |obj|
+            if obj['type'] == item_type && (obj['key_id'] == item_id || obj['id'] == item_id || obj['name'] == item_name || obj['name'] == item_id)
+              return { item: obj, location: { type: 'room', room_id: room_id } }
+            end
+          end
+        end
+      end
+
+      # Priority 2: Items in any room (including locked ones - will validate in main method)
+      @game.scenario_data['rooms'].each do |room_id, room_data|
+        room_data['objects']&.each do |obj|
+          if obj['type'] == item_type && (obj['key_id'] == item_id || obj['id'] == item_id || obj['name'] == item_name || obj['name'] == item_id)
+            return { item: obj, location: { type: 'room', room_id: room_id } }
+          end
+
+          # Search nested contents in room objects
+          obj['contents']&.each do |content|
+            if content['type'] == item_type && (content['key_id'] == item_id || content['id'] == item_id || content['name'] == item_name || content['name'] == item_id)
+              return { item: content, location: { type: 'container', container_id: obj['id'] || obj['name'] } }
+            end
+          end
+        end
+      end
+
+      nil
+    end
+
+    def find_item_in_scenario(item_type, item_id, item_name = nil)
+      # First check startItemsInInventory (items the player begins with)
+      @game.scenario_data['startItemsInInventory']&.each do |item|
+        if item['type'] == item_type && (item['key_id'] == item_id || item['id'] == item_id || item['name'] == item_name || item['name'] == item_id)
+          return item
+        end
+      end
+
+      # Then search room objects
       @game.scenario_data['rooms'].each do |room_id, room_data|
         # Search room objects
         room_data['objects']&.each do |obj|
-          return obj if obj['type'] == item_type && (obj['id'] == item_id || obj['name'] == item_id)
+          if obj['type'] == item_type && (obj['key_id'] == item_id || obj['id'] == item_id || obj['name'] == item_name || obj['name'] == item_id)
+            return obj
+          end
 
           # Search nested contents
           obj['contents']&.each do |content|
-            return content if content['type'] == item_type && (content['id'] == item_id || content['name'] == item_id)
+            if content['type'] == item_type && (content['key_id'] == item_id || content['id'] == item_id || content['name'] == item_name || content['name'] == item_id)
+              return content
+            end
           end
         end
       end

app/models/break_escape/game.rb

@@ -58,6 +58,47 @@ module BreakEscape
       save!
     end
 
+    # Check if player has a specific key in inventory
+    def has_key_in_inventory?(key_id)
+      inventory = player_state['inventory'] || []
+      
+      Rails.logger.info "[BreakEscape] Checking for key #{key_id} in inventory (#{inventory.length} items)"
+      
+      # Check for key with matching key_id
+      found = inventory.any? do |item|
+        is_match = item['scenarioData']&.dig('key_id') == key_id || 
+                   item['scenarioData']&.dig('id') == key_id ||
+                   item['key_id'] == key_id ||
+                   item['id'] == key_id
+        
+        item_key_id = item['scenarioData']&.dig('key_id') || item['key_id']
+        item_name = item['scenarioData']&.dig('name') || item['name']
+        Rails.logger.debug "[BreakEscape] Inventory item: name=#{item_name}, key_id=#{item_key_id}, is_match=#{is_match}"
+        is_match
+      end
+      
+      Rails.logger.info "[BreakEscape] Key #{key_id} found in inventory: #{found}"
+      found
+    end
+
+    # Check if player has a lockpick in inventory
+    def has_lockpick_in_inventory?
+      inventory = player_state['inventory'] || []
+      
+      Rails.logger.info "[BreakEscape] Checking for lockpick in inventory (#{inventory.length} items)"
+      
+      # Check for lockpick item in scenarioData or at top level
+      found = inventory.any? do |item|
+        is_lockpick = item['scenarioData']&.dig('type') == 'lockpick' ||
+                      item['type'] == 'lockpick'
+        Rails.logger.debug "[BreakEscape] Inventory item: type=#{item['type']}, scenarioData.type=#{item['scenarioData']&.dig('type')}, is_lockpick=#{is_lockpick}"
+        is_lockpick
+      end
+      
+      Rails.logger.info "[BreakEscape] Lockpick found in inventory: #{found}"
+      found
+    end
+
     # NPC tracking
     def encounter_npc!(npc_id)
       player_state['encounteredNPCs'] ||= []
@@ -113,9 +154,10 @@ module BreakEscape
       if filtered['rooms'].present?
         filtered['rooms'].each do |room_id, room_data|
           # Keep only essential fields for navigation and metadata
-          # Build new hash with only the fields we want
+          # keyPins MUST be included: Door locks need pin configuration at interaction time,
+          # before the connected room is lazy-loaded. Without keyPins here, lockpicking uses random pins.
           kept_fields = {}
-          %w[type connections locked lockType requires difficulty door_sign].each do |field|
+          %w[type connections locked lockType requires difficulty door_sign keyPins].each do |field|
             kept_fields[field] = room_data[field] if room_data.key?(field)
           end
           
@@ -152,33 +194,62 @@ module BreakEscape
         room = room_data(target_id)
         return false unless room
 
-        # Handle method='unlocked' - verify against scenario data
-        if method == 'unlocked'
-          if !room['locked']
-            Rails.logger.info "[BreakEscape] Door is unlocked in scenario data, granting access"
-            return true
-          else
+        Rails.logger.debug "[BreakEscape] Room data: locked=#{room['locked']}, lockType=#{room['lockType']}, requires=#{room['requires']}"
+
+        # If room is LOCKED, it requires validation
+        if room['locked']
+          Rails.logger.info "[BreakEscape] Room is LOCKED, method must be valid: #{method}"
+          
+          # Handle method='unlocked' - REJECT for locked doors
+          if method == 'unlocked'
             Rails.logger.warn "[BreakEscape] SECURITY VIOLATION: Client sent method='unlocked' for LOCKED door: #{target_id}"
             return false
           end
-        end
 
-        # NPC unlock: Validate NPC has been encountered and has permission to unlock this door
-        if method == 'npc'
-          npc_id = attempt  # NPC id is passed as 'attempt'
-          return validate_npc_unlock(npc_id, target_id)
-        end
+          # NPC unlock: Validate NPC has been encountered and has permission to unlock this door
+          if method == 'npc'
+            npc_id = attempt  # NPC id is passed as 'attempt'
+            return validate_npc_unlock(npc_id, target_id)
+          end
 
-        case method
-        when 'key', 'lockpick', 'biometric', 'bluetooth', 'rfid'
-          # Client validated the unlock - trust it
-          # (player had correct key, picked lock, had fingerprint, had bluetooth device, had RFID card)
-          true
-        when 'pin', 'password'
-          # Server validates password/PIN attempts
-          room['requires'].to_s == attempt.to_s
+          result = case method
+          when 'key'
+            # Server validates player has the correct key in inventory
+            is_valid = room['requires'].present? && has_key_in_inventory?(room['requires'])
+            Rails.logger.info "[BreakEscape] Key validation result: #{is_valid}"
+            is_valid
+          when 'lockpick'
+            # Server validates player has lockpick in inventory
+            # Lockpick can bypass any key-based lock
+            is_valid = has_lockpick_in_inventory?
+            Rails.logger.info "[BreakEscape] Lockpick validation result: #{is_valid}"
+            is_valid
+          when 'biometric', 'bluetooth', 'rfid'
+            # Client validated these - trust it
+            # (player had fingerprint, had bluetooth device, had RFID card)
+            Rails.logger.info "[BreakEscape] #{method} validation passed (trusted client)"
+            true
+          when 'pin', 'password'
+            # Server validates password/PIN attempts
+            is_valid = room['requires'].to_s == attempt.to_s
+            Rails.logger.info "[BreakEscape] #{method} validation result: #{is_valid}"
+            is_valid
+          else
+            Rails.logger.warn "[BreakEscape] SECURITY VIOLATION: No valid unlock method for LOCKED door: #{target_id}, method=#{method}"
+            false
+          end
+
+          Rails.logger.info "[BreakEscape] validate_unlock returning: #{result}"
+          return result
         else
-          false
+          # Room is unlocked
+          if method == 'unlocked'
+            Rails.logger.info "[BreakEscape] Door is unlocked in scenario data, granting access"
+            return true
+          else
+            Rails.logger.warn "[BreakEscape] Client sent method='#{method}' for UNLOCKED door: #{target_id}, but room has no lock"
+            return true # Still allow access since room is unlocked
+          end
         end
       else
         # Check if already unlocked in player state (grants access regardless of method)

public/break_escape/js/systems/inventory.js

@@ -3,6 +3,7 @@
 
 import { rooms } from '../core/rooms.js';
 import InkEngine from './ink/ink-engine.js?v=1';
+import { CSRF_TOKEN } from '../config.js';
 
 // Helper function to create a unique identifier for an item
 export function createItemIdentifier(scenarioData) {
@@ -214,18 +215,23 @@ export async function addToInventory(sprite) {
         }
 
         // NEW: Validate with server before adding
-        const gameId = window.gameId;
+        const gameId = window.breakEscapeConfig?.gameId;
         if (gameId) {
             try {
+                // Create item data with ID from scenario if available
+                const itemData = sprite.scenarioData;
+
                 const response = await fetch(`/break_escape/games/${gameId}/inventory`, {
                     method: 'POST',
+                    credentials: 'same-origin',
                     headers: {
                         'Content-Type': 'application/json',
-                        'Accept': 'application/json'
+                        'Accept': 'application/json',
+                        'X-CSRF-Token': CSRF_TOKEN
                     },
                     body: JSON.stringify({
                         action_type: 'add',
-                        item: sprite.scenarioData
+                        item: itemData
                     })
                 });