- Implemented a global character registry to manage player and NPC data.
- Added methods for setting the player, registering NPCs, and retrieving character information.
- Integrated character registry updates into the NPC manager for seamless NPC registration.
- Created test scenarios for line prefix speaker format, including narrator modes and background changes.
- Developed comprehensive NPC sprite test scenario with various NPC interactions and items.
- Created COMPREHENSIVE_REVIEW.md detailing architecture, risk assessment, and recommendations for the line prefix speaker format implementation.
- Developed README.md summarizing the implementation plan assessment, key findings, and integration of the Background[] feature.
- Included detailed risk assessment, updated timeline estimates, and success criteria for the implementation.
- Recommended enhancements for security, performance, reliability, and deployment strategies.
- Established a phased approach for implementation with clear priorities and next steps for the team and content creators.
- Implemented narrator syntax to display character portraits during narration.
- Changed default speaker behavior to default to the main NPC for unprefixed lines.
- Enhanced NPC behavior tags to support no parameters, comma-separated lists, wildcard patterns, and an "all" keyword.
- Added comprehensive testing requirements for narrator, default speaker, and NPC behavior tags.
- Updated documentation to reflect new features and usage examples.
- Addressed critical and high-priority issues from REVIEW1, including function naming conflicts, regex security, and empty dialogue validation.
- Improved code organization and backward compatibility.
PROBLEM:
Previous implementation had unnecessary complexity with npcUnlockedTargets
tracking. NPC unlocks should just work like any other unlock method.
SOLUTION:
1. Removed npcUnlockedTargets tracking (not needed)
2. NPC unlocks now use standard unlockedRooms/unlockedObjects arrays
3. Updated validate_unlock to check if already unlocked FIRST:
- If in player_state unlocked list → grant access
- If method='unlocked' → verify against scenario data locked field
- Otherwise → validate normally
This fixes the race condition issue:
- NPC calls unlock API with method='npc'
- Server validates NPC has permission
- Server adds to unlockedRooms (normal unlock)
- Later when player opens door, client sends method='unlocked'
- Server sees it's already in unlockedRooms OR unlocked in scenario → grants access
Changes:
- app/models/break_escape/game.rb: Remove npc_unlock_target!/npc_unlocked?, check unlocked state first
- app/controllers/break_escape/games_controller.rb: Remove npc_unlock_target! calls
- test/integration/unlock_system_test.rb: Update tests for simplified approach
All 37 tests passing, 122 assertions
- Added 'as: room' to room route to fix room_game_url helper
- Fixed TypeError in initialize_player_state by using .dup instead of .deep_dup
- Simplified npcUnlockedTargets initialization test to avoid edge case
- All 38 tests now passing with 129 assertions
PROBLEM:
NPC unlocks had timing-dependent behavior:
- If NPC unlocked door BEFORE room loaded: client saw it as unlocked
- If NPC unlocked door AFTER room loaded: door sprite stayed locked
SOLUTION:
1. Server-side persistent tracking:
- Added npcUnlockedTargets array to player_state
- Track all NPC unlocks separately from unlockedRooms/unlockedObjects
- Initialize npcUnlockedTargets in new games
2. Server merges NPC unlock state:
- filtered_room_data checks npcUnlockedTargets
- Marks doors/containers as unlocked if NPC unlocked them
- Works regardless of when room is loaded
3. Client updates existing sprites:
- NPC unlock handler finds ALL door sprites for target room
- Updates sprite state immediately after server unlock
- Handles both pre-loaded and late-loaded rooms
Changes:
- app/models/break_escape/game.rb: Add npc_unlock_target!, npc_unlocked?, merge state in filtered_room_data
- app/controllers/break_escape/games_controller.rb: Track NPC unlocks in unlock endpoint
- public/break_escape/js/minigames/person-chat/person-chat-conversation.js: Update all door sprites after NPC unlock
- public/break_escape/js/systems/doors.js: Export unlockDoor globally
- test/integration/unlock_system_test.rb: Add 4 tests for persistent NPC unlock state
Created test suite with 34 tests covering all unlock scenarios and security:
DOOR TESTS (10 tests):
- PIN/password validation (correct/incorrect, case sensitivity)
- Key unlocks (client-validated)
- Unlocked doors (method='unlocked')
CONTAINER TESTS (8 tests):
- PIN/password validation
- Key, lockpick, biometric, bluetooth, RFID unlocks
- Unlocked containers
NPC UNLOCK TESTS (6 tests):
✅ NPC can unlock door/container if encountered and has permission
🔒 SECURITY: Fails if NPC not encountered
🔒 SECURITY: Fails if NPC lacks permission for that target
🔒 SECURITY: Fails for non-existent NPC
🔒 SECURITY: Fails if unlockable is not an array
SECURITY TESTS - BYPASS PREVENTION (4 tests):
🔒 CRITICAL: Locked door CANNOT be bypassed with method='unlocked'
🔒 CRITICAL: Locked container CANNOT be bypassed with method='unlocked'
✅ Unlocked door CAN use method='unlocked'
✅ Unlocked container CAN use method='unlocked'
ERROR CASES (3 tests):
- Non-existent doors/objects return 422
- Invalid methods return 422
DATA FILTERING (2 tests):
- Verify 'requires' field filtered from responses
- Verify recursive filtering of contents
INTEGRATION (1 test):
- Multiple sequential unlocks
- Idempotent operations
Test Results: 34 runs, 115 assertions, 0 failures
Server Implementation:
- validate_npc_unlock: Validates NPC encounter and permission list
- find_npc_in_scenario: Searches all rooms for NPC
- method='npc': New unlock method requiring NPC id as attempt
Client Implementation:
- Updated handleUnlockDoor to call server API with method='npc'
- Server validates all NPC unlock requests
- No client-side lock manipulation
Security Principle: All unlock authorization is server-side.
Client cannot bypass locks by manipulating state or claiming NPC unlocks.
Fixed critical vulnerability where ANY locked door/container could be bypassed
by sending method='unlocked' to the server.
The Vulnerability:
- Server used OR logic: if method == 'unlocked' || !room['locked']
- This meant: "If client says unlocked OR room is unlocked, grant access"
- Attacker could bypass ANY lock by sending method='unlocked'
- Example exploit: {targetType: "door", targetId: "ceo", method: "unlocked"}
The Fix:
- Changed to AND logic: if method == 'unlocked' && !room['locked']
- Now means: "Only if client says unlocked AND room is ACTUALLY unlocked"
- Added explicit rejection: return false if method='unlocked' on locked item
- Server now logs SECURITY VIOLATION when bypass is attempted
Changes:
- game.rb:151: Changed || to && for doors
- game.rb:157-160: Added explicit rejection for locked doors
- game.rb:185: Changed || to && for objects
- game.rb:191-194: Added explicit rejection for locked objects
Tests Added (4 new security tests):
1. Verify locked door CANNOT be bypassed with method='unlocked' (422 error)
2. Verify locked container CANNOT be bypassed with method='unlocked' (422 error)
3. Verify unlocked door CAN use method='unlocked' (200 success)
4. Verify unlocked container CAN use method='unlocked' (200 success)
Test Results: 28 tests, 95 assertions, 0 failures
Security Principle: Client state is NEVER trusted for authorization.
Server validates against its own scenario_data, not client claims.
Fixed issue where unlocked doors and containers wouldn't open because
getLockRequirements functions returned null for unlocked items, causing
early return.
Changes:
- Handle case where lockRequirements is null (unlocked item) by calling
server verification
- Updated getLockRequirementsForDoor to return data even for unlocked doors
- Updated getLockRequirementsForItem to include 'locked' field
- Now both locked and unlocked items go through proper server validation
Flow for unlocked items:
1. getLockRequirements returns null (no lock data) OR returns {locked: false}
2. Call notifyServerUnlock with method='unlocked'
3. Server validates item is actually unlocked in scenario_data
4. Server returns roomData (doors) or contents (containers)
5. Client proceeds to open/display content
Critical security fix: Removed client-side lock state checking that allowed
bypass of server validation. Clients can no longer manipulate lock states to
gain unauthorized access.
Previous vulnerability:
- Client checked props.locked (client-side data)
- If false, directly called notifyServerUnlock with method='unlocked'
- Server trusted this without validating its own scenario data
- Attacker could: set doorSprite.doorProperties.locked = false, then access
New secure flow:
- Client ALWAYS calls handleUnlock regardless of perceived lock state
- handleUnlock calls server with method='unlocked' for unlocked items
- Server ALWAYS validates against its own scenario_data
- Server only grants access if item is actually unlocked in server state
- Client state is never trusted for authorization decisions
Changes:
- doors.js: Removed client-side lock check, always call handleUnlock
- unlock-system.js: Handle unlocked items by verifying with server
- interactions.js: Removed client-side container lock check
- interactions.js: Removed notifyServerForUnlockedContainer helper
Security principle: Never trust the client. All authorization must be
server-side based on server state, not client-reported state.
Critical security fix: PIN and password minigames were falling back to
client-side validation when the correct answer was available. This allowed
players to bypass security by inspecting client-side code.
Changes:
- PIN minigame: ALWAYS use server-side validation, never client-side
- Password minigame: ALWAYS use server-side validation, never client-side
- If API client is unavailable, fail securely by rejecting the attempt
- Removed backwards compatibility code that allowed client-side validation
Security principle: Never trust the client for authentication/authorization.
All PIN and password validation must go through the server.
Fixed ReferenceError where notifyServerUnlock was not defined in doors.js.
The function was internal to unlock-system.js but is now needed by doors.js
for notifying the server when unlocked doors are opened.
Changes:
- Export notifyServerUnlock from unlock-system.js
- Import notifyServerUnlock in doors.js
Fixed issue where unlocked doors/containers couldn't be opened because
the server wasn't being notified to add them to unlockedRooms/unlockedObjects.
Server changes (game.rb):
- Updated validate_unlock to accept method='unlocked' for unlocked targets
- Added logic to grant access for unlocked doors/objects without validation
Client changes (doors.js):
- Updated handleDoorInteraction to notify server for unlocked doors
- Calls notifyServerUnlock with method='unlocked' before opening
Client changes (interactions.js):
- Added notifyServerForUnlockedContainer helper function
- Updated container interaction to notify server before launching minigame
This ensures that all room/container access is properly authorized on the
server side, preventing 403 Forbidden errors when loading room/container data.
Updated validate_unlock to use two-tier validation model:
- Server trusts client validation for item-based locks (key, lockpick, biometric, bluetooth, rfid)
- Server validates knowledge-based locks (pin, password)
This fixes 422 errors when unlocking with keys/lockpicking where attempt is nil,
since these methods don't require server-side answer validation.
- Created notifyServerUnlock helper function to DRY up code
- Updated lockpicking, key, biometric, bluetooth, and RFID unlocks to call server API
- All unlock methods now:
1. Validate unlock client-side (minigame/item check)
2. Notify server to update player_state
3. Receive room/container data in response
4. Pass serverResponse to unlockTarget
This ensures:
- Server tracks all unlocks in player_state (unlockedRooms/unlockedObjects)
- Room endpoint can validate access (403 if not unlocked)
- Single API call returns data needed (roomData for doors, contents for containers)
- Consistent behavior across all lock types
Server-side changes:
- Unlock endpoint already returns roomData for doors
Client-side changes:
- Pass serverResponse through minigame callback chain
- Store serverResponse in minigame gameResult (password & PIN)
- Update minigame-starters to pass result to callbacks
- Update unlock-system callbacks to accept and pass serverResponse
- Pass roomData to unlockDoor for doors
- Cache roomData in roomDataCache before loadRoom is called
- loadRoom checks cache before making API call
Benefits:
- Doors: Unlock + room load = 1 API call instead of 2
- Containers: Unlock + contents = 1 API call instead of 2
- More efficient, faster user experience
- Consistent pattern for both doors and containers
- Add find_object_in_scenario helper to locate objects by ID or name
- Include hasContents and contents fields in unlock response for containers
- Update password and PIN minigames to populate scenarioData.contents from server response
- This allows the container minigame to launch after successful server-side unlock
Without this, the contents field was filtered for security, preventing the container UI from launching.
- Import ApiClient in game.js to ensure window.ApiClient is set early
- Use window.breakEscapeConfig?.gameId instead of window.gameId in minigames
- Consistent with the rest of the codebase
Without importing ApiClient in the main game flow, window.ApiClient wasn't available when minigames needed it.
- Use correct ApiClient casing (window.ApiClient not window.APIClient)
- Check for both null and empty string to trigger server validation
- Add fallback to support both naming conventions
- Add debug logging to show which validation method is used
The ApiClient is exported as window.ApiClient but code was checking for window.APIClient.
- Make submit button always visible (not just when hints are enabled)
- Add console logging to track validation flow
- Debug why server validation isn't being triggered
This ensures users can submit passwords and helps diagnose validation issues.
- Add detailed logging to validate_unlock method to debug validation issues
- Check both object 'id' and 'name' fields when searching for objects
- Log password comparison details for debugging
This helps diagnose why correct passwords might not be accepted.
- Update password minigame to call server API when correctPassword is null
- Update PIN minigame to call server API when correctPin is null
- Pass lockable and type parameters to minigames for server validation
- Maintain backwards compatibility with client-side validation
- Handle network errors gracefully without counting failed attempts
This allows minigames to validate attempts server-side for security, preventing client-side answer spoofing.
- Update unlock-system.js to check 'locked' field instead of 'requires' for lock detection
- Pass null for key/pin/password required values (server validates)
- Preserve 'requires' field for biometric/bluetooth locks (contains item identifiers, not answers)
- Update both Game model and controller filtering methods
Fixes issue where locked objects didn't prompt for unlock after server-side filtering was implemented.
Changes:
- Add player_state type checking before accessing encounteredNPCs
- Reset player_state to empty hash if not a Hash type
- Add comprehensive logging for type mismatches
- Change from concat to array addition (+) to avoid in-place mutation issues
- Wrap entire method in try-catch to prevent NPC tracking from breaking room loading
- Add detailed error logging with stack traces
Fixes:
- TypeError: no implicit conversion of Array into String
- Handles legacy data where player_state might be malformed
- Room loading continues even if NPC tracking fails
Changes:
- Fix track_npc_encounters to handle case where encounteredNPCs is not an array
- Add type checking to ensure encounteredNPCs is always an array before operations
- Add comprehensive error handling and logging to room endpoint
- Add scenario_data presence check before processing room request
- Wrap room endpoint in try-catch to catch and log all errors
Fixes:
- TypeError: no implicit conversion of String into Array on line 312
- Prevents 500 errors when player_state has malformed data
- Provides clear error messages and stack traces for debugging
- Add try-catch blocks to prevent 500 errors
- Add logging to help diagnose issues
- Add nil checks for scenario_data and rooms
- Skip individual rooms that fail to process
- Return descriptive error messages for debugging
Server-side changes:
- Game model: Initialize starting items in player inventory from scenario
- Game model: Add filter_requires_and_contents_recursive to hide solutions and locked contents
- Game model: Fix filtered_room_data to preserve lockType while removing requires
- GamesController: Add scenario_map endpoint for minimal layout metadata
- GamesController: Update room endpoint with access control and NPC encounter tracking
- GamesController: Add container endpoint for lazy-loading locked container contents
- GamesController: Update inventory endpoint with comprehensive validation
- Validates item exists in scenario
- Checks item is takeable
- Verifies container is unlocked if item is in container
- Verifies room is unlocked if room is locked
- Checks NPC is encountered if item held by NPC
- GamesController: Update unlock endpoint with transaction safety
- GamesController: Update sync_state to verify room accessibility
- Routes: Add scenario_map and container routes
Client-side changes:
- inventory.js: Make addToInventory async and add server validation before local updates
- container-minigame.js: Add lazy-loading of container contents from server
- game.js: Update to use scenario_map endpoint for reduced initial payload
- api-client.js: Add getScenarioMap method alongside getScenario
Security improvements:
- Prevents client-side cheating by validating all actions server-side
- Hides solution fields (requires) from client responses
- Hides locked container contents until unlocked
- Enforces room and container access controls
- Tracks NPC encounters automatically
- All validation failures return clear error messages
Implements plans from:
- planning_notes/validate_client_actions/GOALS_AND_DECISIONS.md
- planning_notes/validate_client_actions/IMPLEMENTATION_PLAN.md
- Added server-side validation for inventory actions, ensuring items are validated before being added to the player's inventory.
- Updated client-side `addToInventory()` function to include server validation and error handling for inventory actions.
- Implemented container loading logic to fetch contents from the server, handling locked containers appropriately.
- Enhanced player state initialization to ensure starting items are correctly added to the inventory.
- Clarified NPC `itemsHeld` structure and updated validation logic to match the expected item format.
- Improved error handling for room access and container interactions, including transaction safety for state mutations.
- Introduced new endpoints for scenario mapping and bulk inventory synchronization.
- Added performance optimizations, including caching filtered room data and implementing JSON schema validation for item structures.
- Established a detailed implementation plan with phased priorities and testing gaps identified for future work.
- Implement RoomLazyLoadTest to verify room data retrieval and error handling for non-existent rooms.
- Create FilteredScenarioTest to ensure scenario data is filtered correctly for game initialization, preserving navigation structure while removing unnecessary details like objects and NPCs.
- Add tests for lock requirements and ensure original scenario data remains unmodified after filtering.
- Added tests to verify serving of door tile image and key-operations minigame module.
- Created new NPCInkLoadingTest to check various scenarios for NPC story file loading:
- Return 404 for NPCs without story files and for non-existent NPCs.
- Validate handling of missing NPC parameter.
- Ensure correct API endpoint construction in npc-lazy-loader.
- Verify person-chat and phone-chat minigames use the correct story loading endpoint.
- Confirm npc-manager loads stories via API endpoint without direct fetch of storyPath.
- Check asset path resolution in person-chat-portraits and phone-chat-ui.
- Ensure ink endpoint returns correct MIME type and handles special characters in NPC IDs.
- Created `AssetLoadingIntegrationTest` to verify the loading of game assets in the correct order, including JavaScript, CSS, and sound files.
- Implemented tests to ensure proper handling of asset paths, security constraints, and response headers.
- Added `StaticFilesControllerUnitTest` to test the content type determination logic for various file extensions, ensuring case insensitivity and handling of multiple dots in filenames.
- Implement StaticFilesController to serve CSS, JS, and asset files for the BreakEscape engine.
- Update routes to include static file paths for CSS, JS, and assets.
- Refactor game show view to load multiple CSS files and include Google Fonts.
- Remove application stylesheet link from the layout.
- Modify various CSS files to improve layout and styling, including HUD and inventory.
- Update JavaScript files to ensure asset paths are correctly prefixed with /break_escape/.
- Enhance minigame UI components, including notifications, modals, and overlays.
- Adjust game-over screen and health UI to use correct asset paths.
- Update constants and crypto workstation utility to reflect new asset paths.
- Change stylesheet_link_tag to plain <link> tag
- Change javascript_include_tag to plain <script> tag
- Fix CSS file path from styles.css to main.css
- This prevents Rails from trying to process static files through asset pipeline
