mirror of
https://github.com/cliffe/BreakEscape.git
synced 2026-02-21 11:18:08 +00:00
Finalize implementation plan for VM and CTF Flag Integration following Review 4
- Updated last modified date and added new review document reference. - Added policy methods for `GamePolicy#submit_flag?` and `MissionPolicy#create_game?`. - Fixed authorization in `games#create` to use `:create_game?` and removed redundant `VmSet` authorization. - Clarified loading of `hacktivity-cable.js` with CSP nonce. - Included fallback for non-Hacktivity mode in VM set validation. - Documented minor amendments and verification checklist for implementation readiness.
This commit is contained in:
@@ -1,11 +1,12 @@
|
||||
# VM and CTF Flag Integration - Implementation Plan
|
||||
|
||||
**Last Updated**: After Console Integration Deep-Dive (2025-11-28)
|
||||
**Last Updated**: After Review 4 Amendments (2025-11-28)
|
||||
**Review Documents**:
|
||||
- `planning_notes/vms_and_flags/review1/REVIEW.md`
|
||||
- `planning_notes/vms_and_flags/review2/REVIEW.md`
|
||||
- `planning_notes/vms_and_flags/review3/REVIEW.md`
|
||||
- `planning_notes/vms_and_flags/review3/HACKTIVITY_COMPATIBILITY.md`
|
||||
- `planning_notes/vms_and_flags/review4/REVIEW.md` ✅ **Ready for Implementation**
|
||||
|
||||
## Overview
|
||||
|
||||
@@ -582,6 +583,9 @@ end
|
||||
# - The `scenario` field contains the XML path (e.g., "scenarios/ctf/foo.xml")
|
||||
# - VmSet doesn't have a `display_name` method - use sec_gen_batch.title instead
|
||||
# - Always eager-load :vms and :sec_gen_batch to avoid N+1 queries
|
||||
#
|
||||
# NOTE: Update the existing `hacktivity_mode?` method (line 60-62 in current code)
|
||||
# to use the new definition that checks for ::VmSet and ::FlagService instead of just ::Cybok.
|
||||
def valid_vm_sets_for_user(user)
|
||||
return [] unless self.class.hacktivity_mode? && requires_vms?
|
||||
|
||||
@@ -821,19 +825,25 @@ end
|
||||
# Add to app/controllers/break_escape/games_controller.rb
|
||||
def create
|
||||
@mission = Mission.find(params[:mission_id])
|
||||
authorize @mission if defined?(Pundit)
|
||||
authorize @mission, :create_game? if defined?(Pundit)
|
||||
|
||||
# Build initial player_state with VM/flag context
|
||||
initial_player_state = {}
|
||||
|
||||
# Hacktivity mode with VM set
|
||||
if params[:vm_set_id].present? && defined?(::VmSet)
|
||||
vm_set = ::VmSet.find(params[:vm_set_id])
|
||||
authorize vm_set, :use? if defined?(Pundit)
|
||||
vm_set = ::VmSet.find_by(id: params[:vm_set_id])
|
||||
return render json: { error: 'VM set not found' }, status: :not_found unless vm_set
|
||||
|
||||
# Validate VM set belongs to user and matches mission
|
||||
unless @mission.valid_vm_sets_for_user(current_user).include?(vm_set)
|
||||
return render json: { error: 'Invalid VM set for this mission' }, status: :forbidden
|
||||
if BreakEscape::Mission.hacktivity_mode?
|
||||
unless @mission.valid_vm_sets_for_user(current_user).include?(vm_set)
|
||||
return render json: { error: 'Invalid VM set for this mission' }, status: :forbidden
|
||||
end
|
||||
else
|
||||
# Standalone mode - vm_set_id shouldn't be used
|
||||
Rails.logger.warn "[BreakEscape] vm_set_id provided but not in Hacktivity mode, ignoring"
|
||||
params.delete(:vm_set_id)
|
||||
end
|
||||
|
||||
initial_player_state['vm_set_id'] = vm_set.id
|
||||
@@ -1080,7 +1090,34 @@ end
|
||||
1. Add `:new` to `only: [:new, :show, :create]`
|
||||
2. Add `post :flags` to the member block
|
||||
|
||||
### 2. Missions Controller (`app/controllers/break_escape/missions_controller.rb`)
|
||||
### 3. Update Policies
|
||||
|
||||
#### GamePolicy
|
||||
|
||||
Add to `app/policies/break_escape/game_policy.rb`:
|
||||
|
||||
```ruby
|
||||
def submit_flag?
|
||||
show?
|
||||
end
|
||||
|
||||
def container?
|
||||
show?
|
||||
end
|
||||
```
|
||||
|
||||
#### MissionPolicy
|
||||
|
||||
Add to `app/policies/break_escape/mission_policy.rb`:
|
||||
|
||||
```ruby
|
||||
def create_game?
|
||||
# Anyone authenticated can create a game for a mission
|
||||
user.present?
|
||||
end
|
||||
```
|
||||
|
||||
### 4. Missions Controller (`app/controllers/break_escape/missions_controller.rb`)
|
||||
|
||||
#### Update `show` Action (Critical Change)
|
||||
|
||||
@@ -1467,25 +1504,15 @@ export { setupHacktivityActionCable, teardownHacktivityActionCable };
|
||||
|
||||
**Loading this module:**
|
||||
|
||||
Include in the game's main entry point or load via script tag:
|
||||
Add to `app/views/break_escape/games/show.html.erb` after the Phaser script loads:
|
||||
|
||||
```html
|
||||
<!-- In app/views/break_escape/games/show.html.erb -->
|
||||
```erb
|
||||
<% if BreakEscape::Mission.hacktivity_mode? %>
|
||||
<script type="module" src="/break_escape/js/systems/hacktivity-cable.js"></script>
|
||||
<script type="module" src="/break_escape/js/systems/hacktivity-cable.js" nonce="<%= content_security_policy_nonce %>"></script>
|
||||
<% end %>
|
||||
```
|
||||
|
||||
Or import in `js/main.js`:
|
||||
|
||||
```javascript
|
||||
// Only import in Hacktivity mode
|
||||
if (window.breakEscapeConfig?.hacktivityMode) {
|
||||
import('./systems/hacktivity-cable.js').then(module => {
|
||||
module.setupHacktivityActionCable();
|
||||
});
|
||||
}
|
||||
```
|
||||
**Note**: The module self-initializes on load. No additional setup required.
|
||||
|
||||
### 1. VM Launcher Minigame
|
||||
|
||||
@@ -2459,6 +2486,8 @@ end
|
||||
- [ ] 2.11 Update GamesController `scenario` action: Include `submittedFlags`
|
||||
- [ ] 2.12 Add CSS link tags for new minigame styles to show.html.erb
|
||||
- [ ] 2.13 Write controller tests
|
||||
- [ ] 2.14 Add GamePolicy#submit_flag? method
|
||||
- [ ] 2.15 Add MissionPolicy#create_game? method (for authorization in games#create)
|
||||
|
||||
### Phase 3: Client-Side Minigames
|
||||
- [ ] 3.0 Create `public/break_escape/js/systems/hacktivity-cable.js` (ActionCable FilePush subscription)
|
||||
@@ -2583,3 +2612,4 @@ This plan provides a complete, actionable roadmap for integrating VMs and CTF fl
|
||||
| 2025-11-27 | Review 3 (`review3/REVIEW.md`) | **Critical**: Discovered `games#create` action does NOT exist (routes declared it but controller didn't implement it). Updated plan to implement action from scratch. Added `games#new` action and view for VM set selection. Added MissionsController#show update. Validated callback timing approach is correct. Added revised implementation order prioritizing controller infrastructure. |
|
||||
| 2025-11-28 | Hacktivity Compatibility (`review3/HACKTIVITY_COMPATIBILITY.md`) | **Reviewed Hacktivity codebase** for compatibility. Key findings: (1) Use `FlagService.process_flag()` instead of direct model update for proper scoring/streaks/notifications, (2) VmSet uses `sec_gen_batch` (with underscore) not `secgen_batch`, (3) VmSet has no `display_name` method - use `sec_gen_batch.title` instead, (4) Added `event_id` and `sec_gen_batch_id` to VM context for console URLs, (5) Added case-insensitive flag matching to match Hacktivity behavior, (6) Added eager loading with `.includes(:vms, :sec_gen_batch)`, (7) Filter out pending/error build_status VM sets. |
|
||||
| 2025-11-28 | Console Integration Update | **Deep-dive into Hacktivity's console mechanism**: Discovered console file delivery is async via ActionCable, not HTTP response. (1) Console endpoint dispatches background job that generates SPICE `.vv` file, (2) Job broadcasts file via `ActionCable.server.broadcast "file_push:#{user_id}"` with Base64-encoded content, (3) Updated plan to single approach: subscribe to `FilePushChannel`, POST to trigger job, receive file via ActionCable. (4) Created `hacktivity-cable.js` module specification, (5) Updated VM launcher minigame to use AJAX POST + ActionCable callback pattern, (6) Removed alternative "open Hacktivity page" approach - now single definitive approach. |
|
||||
| 2025-11-28 | Review 4 (`review4/REVIEW.md`) | **Final validation - Plan ready for implementation**. Minor fixes applied: (1) Added policy checklist items for `GamePolicy#submit_flag?` and `MissionPolicy#create_game?`, (2) Fixed authorization in `games#create` to use `authorize @mission, :create_game?`, (3) Removed redundant VmSet authorization (replaced with `find_by` + nil check), (4) Added Hacktivity mode check for VmSet validation fallback, (5) Added note about updating existing `hacktivity_mode?` method, (6) Added policy code examples section (GamePolicy + MissionPolicy), (7) Simplified hacktivity-cable.js loading to single recommended approach with CSP nonce. |
|
||||
|
||||
@@ -0,0 +1,184 @@
|
||||
# Implementation Ready Checklist
|
||||
|
||||
**Status**: ✅ READY FOR IMPLEMENTATION
|
||||
**Date**: November 28, 2025
|
||||
|
||||
---
|
||||
|
||||
## Pre-Implementation Fixes (Minor)
|
||||
|
||||
Before starting implementation, apply these minor fixes to the plan:
|
||||
|
||||
### 1. Policy Methods
|
||||
|
||||
Add to `app/policies/break_escape/game_policy.rb`:
|
||||
```ruby
|
||||
def submit_flag?
|
||||
show?
|
||||
end
|
||||
```
|
||||
|
||||
Add to `app/policies/break_escape/mission_policy.rb` (or create if doesn't exist):
|
||||
```ruby
|
||||
def create_game?
|
||||
user.present?
|
||||
end
|
||||
```
|
||||
|
||||
### 2. Update `hacktivity_mode?` Method
|
||||
|
||||
In `games#create`, change authorization from:
|
||||
```ruby
|
||||
authorize @mission if defined?(Pundit)
|
||||
```
|
||||
To:
|
||||
```ruby
|
||||
authorize @mission, :create_game? if defined?(Pundit)
|
||||
```
|
||||
|
||||
### 3. Remove Redundant Authorization
|
||||
|
||||
In `games#create`, remove this line:
|
||||
```ruby
|
||||
authorize vm_set, :use? if defined?(Pundit) # REMOVE - valid_vm_sets_for_user handles this
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Implementation Order
|
||||
|
||||
### Phase 1: Controller Infrastructure (Day 1)
|
||||
|
||||
| Step | File | Action |
|
||||
|------|------|--------|
|
||||
| 1.1 | `app/policies/break_escape/game_policy.rb` | Add `submit_flag?` method |
|
||||
| 1.2 | `app/policies/break_escape/mission_policy.rb` | Add `create_game?` method |
|
||||
| 1.3 | `config/routes.rb` | Add `:new` to games resource, add `post :flags` |
|
||||
| 1.4 | `app/controllers/break_escape/games_controller.rb` | Add `new`, `create`, `submit_flag` actions |
|
||||
| 1.5 | `app/controllers/break_escape/missions_controller.rb` | Update `show` for VM missions |
|
||||
| 1.6 | `app/views/break_escape/games/new.html.erb` | Create VM set selection view |
|
||||
|
||||
### Phase 2: Database Migration (Day 1)
|
||||
|
||||
| Step | File | Action |
|
||||
|------|------|--------|
|
||||
| 2.1 | `db/migrate/[timestamp]_remove_unique_game_constraint.rb` | Create migration |
|
||||
| 2.2 | Terminal | Run `rails db:migrate` |
|
||||
|
||||
### Phase 3: Model Changes (Day 2)
|
||||
|
||||
| Step | File | Action |
|
||||
|------|------|--------|
|
||||
| 3.1 | `app/models/break_escape/mission.rb` | Add `requires_vms?`, `valid_vm_sets_for_user` |
|
||||
| 3.2 | `app/models/break_escape/mission.rb` | Extend `ScenarioBinding` with `vm_context` |
|
||||
| 3.3 | `app/models/break_escape/mission.rb` | Update `generate_scenario_data` signature |
|
||||
| 3.4 | `app/models/break_escape/mission.rb` | Update `hacktivity_mode?` to check VmSet/FlagService |
|
||||
| 3.5 | `app/models/break_escape/game.rb` | Add `build_vm_context` method |
|
||||
| 3.6 | `app/models/break_escape/game.rb` | Extend `initialize_player_state` for VM/flag fields |
|
||||
| 3.7 | `app/models/break_escape/game.rb` | Add flag submission methods |
|
||||
| 3.8 | `app/models/break_escape/game.rb` | Update `generate_scenario_data` callback |
|
||||
|
||||
### Phase 4: Client-Side (Day 3-4)
|
||||
|
||||
| Step | File | Action |
|
||||
|------|------|--------|
|
||||
| 4.1 | `public/break_escape/js/systems/hacktivity-cable.js` | Create ActionCable integration |
|
||||
| 4.2 | `public/break_escape/js/minigames/vm-launcher/` | Create minigame directory and files |
|
||||
| 4.3 | `public/break_escape/js/minigames/flag-station/` | Create minigame directory and files |
|
||||
| 4.4 | `public/break_escape/js/minigames/index.js` | Register new minigames |
|
||||
| 4.5 | `public/break_escape/js/systems/interactions.js` | Add handlers for new object types |
|
||||
| 4.6 | `public/break_escape/css/minigames/vm-launcher.css` | Create styles |
|
||||
| 4.7 | `public/break_escape/css/minigames/flag-station.css` | Create styles |
|
||||
| 4.8 | `app/views/break_escape/games/show.html.erb` | Add CSS links, extend config, add script tag |
|
||||
|
||||
### Phase 5: Testing (Day 5)
|
||||
|
||||
| Step | Test | Validation |
|
||||
|------|------|------------|
|
||||
| 5.1 | Model unit tests | `submit_flag`, `build_vm_context` |
|
||||
| 5.2 | Controller tests | `create`, `submit_flag` actions |
|
||||
| 5.3 | Standalone mode | Create game without VMs, submit manual flags |
|
||||
| 5.4 | Hacktivity mode | Create game with VmSet, test console + flags |
|
||||
| 5.5 | Minigame tests | Open test HTML files for VM launcher and flag station |
|
||||
|
||||
---
|
||||
|
||||
## Verification Checklist
|
||||
|
||||
After implementation, verify:
|
||||
|
||||
- [ ] Visiting mission with `secgen_scenario` redirects to `games#new`
|
||||
- [ ] VM set selection page shows available VM sets
|
||||
- [ ] Creating game with VM set stores `vm_set_id` in player_state
|
||||
- [ ] Scenario ERB has access to `vm_context`
|
||||
- [ ] Clicking `type: "vm-launcher"` object opens VM launcher minigame
|
||||
- [ ] Clicking `type: "flag-station"` object opens flag station minigame
|
||||
- [ ] Flag submission validates server-side
|
||||
- [ ] Flag submission calls `FlagService.process_flag` in Hacktivity mode
|
||||
- [ ] Console button triggers SPICE file download via ActionCable
|
||||
- [ ] Multiple games per mission work (unique index removed)
|
||||
- [ ] Standalone mode works without VMs
|
||||
|
||||
---
|
||||
|
||||
## Key Code Locations for Reference
|
||||
|
||||
### Existing Patterns to Follow
|
||||
|
||||
| Pattern | File | Line |
|
||||
|---------|------|------|
|
||||
| Minigame registration | `public/break_escape/js/minigames/index.js` | 80-94 |
|
||||
| Object type handling | `public/break_escape/js/systems/interactions.js` | 455-512 |
|
||||
| Server unlock validation | `app/models/break_escape/game.rb` | 184-301 |
|
||||
| ERB scenario generation | `app/models/break_escape/mission.rb` | 65-76 |
|
||||
| Player state initialization | `app/models/break_escape/game.rb` | 549-582 |
|
||||
| Policy authorization | `app/controllers/break_escape/games_controller.rb` | 8 |
|
||||
|
||||
### Config Object (Current)
|
||||
|
||||
```javascript
|
||||
// app/views/break_escape/games/show.html.erb:115-120
|
||||
window.breakEscapeConfig = {
|
||||
gameId: <%= @game.id %>,
|
||||
apiBasePath: '<%= game_path(@game) %>',
|
||||
assetsPath: '/break_escape/assets',
|
||||
csrfToken: '<%= form_authenticity_token %>'
|
||||
};
|
||||
```
|
||||
|
||||
### Config Object (After Changes)
|
||||
|
||||
```javascript
|
||||
window.breakEscapeConfig = {
|
||||
gameId: <%= @game.id %>,
|
||||
apiBasePath: '<%= game_path(@game) %>',
|
||||
assetsPath: '/break_escape/assets',
|
||||
csrfToken: '<%= form_authenticity_token %>',
|
||||
// NEW FIELDS:
|
||||
hacktivityMode: <%= BreakEscape::Mission.hacktivity_mode? %>,
|
||||
vmSetId: <%= @game.player_state['vm_set_id'] || 'null' %>
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Command Quick Reference
|
||||
|
||||
```bash
|
||||
# Generate migration
|
||||
rails generate migration RemoveUniqueGameConstraint
|
||||
|
||||
# Run migrations
|
||||
rails db:migrate
|
||||
|
||||
# Test routes
|
||||
rails routes | grep break_escape
|
||||
|
||||
# Compile Ink scripts (if adding VM-related NPC dialogs)
|
||||
./scripts/compile-ink.sh
|
||||
|
||||
# Run tests
|
||||
rails test test/controllers/break_escape/
|
||||
rails test test/models/break_escape/
|
||||
```
|
||||
|
||||
145
planning_notes/vms_and_flags/review4/PLAN_AMENDMENTS.md
Normal file
145
planning_notes/vms_and_flags/review4/PLAN_AMENDMENTS.md
Normal file
@@ -0,0 +1,145 @@
|
||||
# Plan Amendments - Review 4
|
||||
|
||||
These are the specific amendments to apply to `IMPLEMENTATION_PLAN.md` based on Review 4 findings.
|
||||
|
||||
---
|
||||
|
||||
## 1. Add Policy Requirements to Implementation Checklist
|
||||
|
||||
**Location**: Lines 2448-2462 (Phase 2 checklist)
|
||||
|
||||
**Add after line 2.13**:
|
||||
```markdown
|
||||
- [ ] 2.14 Add GamePolicy#submit_flag? method
|
||||
- [ ] 2.15 Add MissionPolicy#create_game? method (for authorization in games#create)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Fix Authorization in `games#create` Action
|
||||
|
||||
**Location**: Lines 820-864 (games#create action code)
|
||||
|
||||
**Change line 821**:
|
||||
```ruby
|
||||
# FROM:
|
||||
authorize @mission if defined?(Pundit)
|
||||
|
||||
# TO:
|
||||
authorize @mission, :create_game? if defined?(Pundit)
|
||||
```
|
||||
|
||||
**Remove lines 831-832**:
|
||||
```ruby
|
||||
# REMOVE THESE LINES (redundant - valid_vm_sets_for_user already validates ownership):
|
||||
vm_set = ::VmSet.find(params[:vm_set_id])
|
||||
authorize vm_set, :use? if defined?(Pundit)
|
||||
```
|
||||
|
||||
**Replace with**:
|
||||
```ruby
|
||||
vm_set = ::VmSet.find_by(id: params[:vm_set_id])
|
||||
return render json: { error: 'VM set not found' }, status: :not_found unless vm_set
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Update `hacktivity_mode?` Reference in Model Changes
|
||||
|
||||
**Location**: Lines 580-600 (Mission model changes)
|
||||
|
||||
**Add note after line 585**:
|
||||
```markdown
|
||||
**NOTE**: Update the existing `hacktivity_mode?` method (line 60-62 in current code) to use this new definition. The current definition only checks for `::Cybok`.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 4. Add Policy Code Examples
|
||||
|
||||
**Location**: Add new section after line 1080 (after controller routes section)
|
||||
|
||||
```markdown
|
||||
### 3. Update Policies
|
||||
|
||||
#### GamePolicy
|
||||
|
||||
Add to `app/policies/break_escape/game_policy.rb`:
|
||||
|
||||
```ruby
|
||||
def submit_flag?
|
||||
show?
|
||||
end
|
||||
|
||||
def container?
|
||||
show?
|
||||
end
|
||||
```
|
||||
|
||||
#### MissionPolicy
|
||||
|
||||
Add to `app/policies/break_escape/mission_policy.rb`:
|
||||
|
||||
```ruby
|
||||
def create_game?
|
||||
# Anyone authenticated can create a game for a mission
|
||||
user.present?
|
||||
end
|
||||
```
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 5. Clarify `hacktivity-cable.js` Script Loading
|
||||
|
||||
**Location**: Lines 1472-1488 (loading hacktivity-cable.js)
|
||||
|
||||
**Replace the two options with single recommended approach**:
|
||||
```markdown
|
||||
**Loading this module:**
|
||||
|
||||
Add to `app/views/break_escape/games/show.html.erb` after the Phaser script loads:
|
||||
|
||||
```erb
|
||||
<% if BreakEscape::Mission.hacktivity_mode? %>
|
||||
<script type="module" src="/break_escape/js/systems/hacktivity-cable.js" nonce="<%= content_security_policy_nonce %>"></script>
|
||||
<% end %>
|
||||
```
|
||||
|
||||
**Note**: The module self-initializes on load. No additional setup required.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 6. Add Fallback for Non-Hacktivity VmSet Authorization
|
||||
|
||||
**Location**: Lines 833-839 (games#create validation)
|
||||
|
||||
**Update the validation block**:
|
||||
```ruby
|
||||
# Validate VM set belongs to user and matches mission
|
||||
if BreakEscape::Mission.hacktivity_mode?
|
||||
unless @mission.valid_vm_sets_for_user(current_user).include?(vm_set)
|
||||
return render json: { error: 'Invalid VM set for this mission' }, status: :forbidden
|
||||
end
|
||||
else
|
||||
# Standalone mode - vm_set_id shouldn't be used
|
||||
Rails.logger.warn "[BreakEscape] vm_set_id provided but not in Hacktivity mode, ignoring"
|
||||
params.delete(:vm_set_id)
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| Amendment | Severity | Reason |
|
||||
|-----------|----------|--------|
|
||||
| Add policy methods | Minor | Pundit will fail without these |
|
||||
| Fix authorization action | Minor | `authorize @mission` uses `show?` not `create_game?` |
|
||||
| Remove redundant VmSet auth | Cleanup | Query already validates ownership |
|
||||
| Add `hacktivity-cable.js` load | Clarification | Two options were confusing |
|
||||
| Add Hacktivity mode check | Robustness | Prevents errors in standalone mode |
|
||||
|
||||
These amendments can be applied incrementally during implementation without blocking the start of development.
|
||||
|
||||
325
planning_notes/vms_and_flags/review4/REVIEW.md
Normal file
325
planning_notes/vms_and_flags/review4/REVIEW.md
Normal file
@@ -0,0 +1,325 @@
|
||||
# VM and CTF Flag Integration - Plan Review 4
|
||||
|
||||
**Reviewer**: AI Review
|
||||
**Date**: November 28, 2025
|
||||
**Document Reviewed**: `planning_notes/vms_and_flags/IMPLEMENTATION_PLAN.md` (post-Reviews 1, 2, 3 + Hacktivity Compatibility)
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The implementation plan has matured significantly through four review cycles. The plan is now **ready for implementation** with minor clarifications needed. All critical issues from previous reviews have been addressed, and the Hacktivity compatibility deep-dive has resolved API integration concerns.
|
||||
|
||||
**Status**: ✅ Ready for Implementation
|
||||
**Remaining Issues**: 2 Minor, 2 Recommendations
|
||||
**Estimated Risk**: Low
|
||||
|
||||
---
|
||||
|
||||
## Validation Summary
|
||||
|
||||
### ✅ Issues Resolved from Previous Reviews
|
||||
|
||||
| Issue | Status | Notes |
|
||||
|-------|--------|-------|
|
||||
| `games#create` action missing | ✅ Resolved | Plan now explicitly shows full implementation |
|
||||
| `games#new` action missing | ✅ Resolved | Plan includes action and view |
|
||||
| `secgen_scenario` column | ✅ Verified | Exists in migration `20251125000001` |
|
||||
| Hacktivity association naming | ✅ Fixed | Uses `sec_gen_batch` (with underscore) |
|
||||
| Flag submission method | ✅ Fixed | Uses `FlagService.process_flag()` |
|
||||
| VmSet `display_name` | ✅ Fixed | Uses `sec_gen_batch.title` instead |
|
||||
| Console URL construction | ✅ Complete | Stores `event_id`, `sec_gen_batch_id` in VM context |
|
||||
| ActionCable integration | ✅ Complete | Uses FilePushChannel subscription |
|
||||
| ERB null-safety patterns | ✅ Verified | Uses `&.`, `dig`, `|| 'null'` |
|
||||
| `window.breakEscapeConfig` | ✅ Verified | Plan extends, not replaces |
|
||||
|
||||
### ✅ Codebase Alignment Verified
|
||||
|
||||
| Component | Plan Assumption | Actual Codebase | Status |
|
||||
|-----------|-----------------|-----------------|--------|
|
||||
| Routes structure | `BreakEscape::Engine.routes.draw` | ✅ Matches | OK |
|
||||
| Game model callbacks | `before_create` for scenario/state | ✅ Matches | OK |
|
||||
| Player state structure | JSONB with defaults | ✅ Matches | OK |
|
||||
| Minigame registration | `MinigameFramework.registerScene()` | ✅ Matches | OK |
|
||||
| Policy pattern | Pundit with `authorize` | ✅ Matches | OK |
|
||||
| Object interaction | `handleObjectInteraction()` via type | ✅ Matches | OK |
|
||||
| Unique index on games | Exists, needs removal | ✅ Verified | OK |
|
||||
|
||||
---
|
||||
|
||||
## Minor Issues to Address
|
||||
|
||||
### 1. 📝 Policy Methods Missing for New Actions
|
||||
|
||||
The `GamePolicy` currently defines policies for existing actions but will need new policy methods for `create`, `new`, and `submit_flag` actions.
|
||||
|
||||
**Current** (`app/policies/break_escape/game_policy.rb`):
|
||||
```ruby
|
||||
def show?
|
||||
record.player == user || user&.admin? || user&.account_manager?
|
||||
end
|
||||
```
|
||||
|
||||
**Required Additions**:
|
||||
```ruby
|
||||
# For games#create - check if user can create games for this mission
|
||||
def create?
|
||||
# The record here is the Mission, not a Game
|
||||
# Anyone authenticated can create games
|
||||
user.present?
|
||||
end
|
||||
|
||||
# For games#new - same as create
|
||||
def new?
|
||||
create?
|
||||
end
|
||||
|
||||
# For games#submit_flag - owner can submit flags
|
||||
def submit_flag?
|
||||
record.player == user || user&.admin? || user&.account_manager?
|
||||
end
|
||||
```
|
||||
|
||||
**Note**: The `create` action in the plan authorizes `@mission`, not `@game`:
|
||||
```ruby
|
||||
authorize @mission if defined?(Pundit)
|
||||
```
|
||||
|
||||
This means you need a `MissionPolicy#create?` OR change the authorization approach to use `Game.new.authorize` pattern.
|
||||
|
||||
**Recommendation**: Use `authorize @mission, :create_game?` with:
|
||||
```ruby
|
||||
# mission_policy.rb
|
||||
def create_game?
|
||||
user.present? # Anyone authenticated can start a game
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 2. 📝 `hacktivity_mode?` Detection Method Location
|
||||
|
||||
The plan defines `hacktivity_mode?` in two places:
|
||||
|
||||
1. In `Mission` model (lines 465-468):
|
||||
```ruby
|
||||
def self.hacktivity_mode?
|
||||
defined?(::VmSet) && defined?(::SecGenBatch) && defined?(::FlagService)
|
||||
end
|
||||
```
|
||||
|
||||
2. Reference in `show.html.erb` (line 92):
|
||||
```ruby
|
||||
hacktivityMode: <%= BreakEscape::Mission.hacktivity_mode? %>,
|
||||
```
|
||||
|
||||
**Issue**: The Mission model currently only checks `defined?(::Cybok)` (line 60-62), not the VM/Flag classes.
|
||||
|
||||
**Current** (`app/models/break_escape/mission.rb`):
|
||||
```ruby
|
||||
def self.hacktivity_mode?
|
||||
defined?(::Cybok)
|
||||
end
|
||||
```
|
||||
|
||||
**Recommendation**: The plan's more comprehensive check is better - update the existing method:
|
||||
```ruby
|
||||
def self.hacktivity_mode?
|
||||
defined?(::VmSet) && defined?(::FlagService)
|
||||
end
|
||||
```
|
||||
|
||||
This is a minor fix but important for consistent behavior.
|
||||
|
||||
---
|
||||
|
||||
## Recommendations (Nice to Have)
|
||||
|
||||
### 1. 💡 Add `container` Action Authorization
|
||||
|
||||
The plan's `submit_flag` action calls `@game.submit_flag(flag_key)` which accesses scenario data. The policy should ensure this is the game owner.
|
||||
|
||||
The plan shows:
|
||||
```ruby
|
||||
def submit_flag
|
||||
authorize @game if defined?(Pundit)
|
||||
# ...
|
||||
end
|
||||
```
|
||||
|
||||
This is correct - the existing pattern `show?` → `record.player == user` will work. Just add:
|
||||
```ruby
|
||||
def submit_flag?
|
||||
show?
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 2. 💡 Consider Route Namespacing for VmSet Authorization
|
||||
|
||||
When authorizing `vm_set` in `games#create`:
|
||||
```ruby
|
||||
vm_set = ::VmSet.find(params[:vm_set_id])
|
||||
authorize vm_set, :use? if defined?(Pundit)
|
||||
```
|
||||
|
||||
The `::VmSet` is a Hacktivity model with its own policy. The `use?` policy method may not exist.
|
||||
|
||||
**Recommendation**: Check Hacktivity's VmSetPolicy or skip direct VmSet authorization and rely on the `valid_vm_sets_for_user` method's implicit authorization (it only returns VMs the user owns).
|
||||
|
||||
The plan's current approach:
|
||||
```ruby
|
||||
unless @mission.valid_vm_sets_for_user(current_user).include?(vm_set)
|
||||
return render json: { error: 'Invalid VM set' }, status: :forbidden
|
||||
end
|
||||
```
|
||||
|
||||
This is sufficient - the `valid_vm_sets_for_user` query already filters to user's own VMs. The extra `authorize vm_set, :use?` line can be removed.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Readiness Checklist
|
||||
|
||||
| Phase | Ready | Notes |
|
||||
|-------|-------|-------|
|
||||
| Database Changes | ✅ | Migration file outlined, `secgen_scenario` already exists |
|
||||
| Model Changes | ✅ | All methods documented with code |
|
||||
| Controller Changes | ✅ | Full action implementations provided |
|
||||
| Views | ✅ | `new.html.erb` template included |
|
||||
| Routes | ✅ | Changes clearly specified |
|
||||
| Client JS - ActionCable | ✅ | `hacktivity-cable.js` fully documented |
|
||||
| Client JS - Minigames | ✅ | Both minigames fully implemented |
|
||||
| Client JS - Interactions | ✅ | Handler code provided |
|
||||
| CSS Styling | ✅ | Complete stylesheets included |
|
||||
| ERB Templates | ✅ | Example scenario with null-safety patterns |
|
||||
| Policies | ⚠️ | Need to add `submit_flag?` and `create_game?` |
|
||||
| Tests | 📝 | Testing plan outlined but not implemented |
|
||||
|
||||
---
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
### Low Risk ✅
|
||||
- Database migration is straightforward (index removal)
|
||||
- New endpoints follow existing patterns
|
||||
- Minigame framework integration is well-established
|
||||
- ERB patterns are documented with null-safety
|
||||
|
||||
### Medium Risk ⚠️ (Mitigated)
|
||||
- **Hacktivity integration** - mitigated by FlagService usage and compatibility review
|
||||
- **Console file delivery** - mitigated by ActionCable subscription approach
|
||||
|
||||
### No High-Risk Items
|
||||
The plan has addressed all critical concerns through the review process.
|
||||
|
||||
---
|
||||
|
||||
## Deployment Considerations
|
||||
|
||||
### Order of Operations
|
||||
|
||||
1. **Deploy controller changes** (disabled/hidden)
|
||||
- Add `games#new`, `games#create`, `games#submit_flag`
|
||||
- Update `MissionsController#show` redirect logic
|
||||
- Add policy methods
|
||||
|
||||
2. **Run migration**
|
||||
- Remove unique index on games table
|
||||
|
||||
3. **Deploy frontend changes**
|
||||
- Add minigame JS files
|
||||
- Add CSS files
|
||||
- Update `show.html.erb` config
|
||||
- Add ActionCable integration
|
||||
|
||||
4. **Enable new functionality**
|
||||
- Update scenarios with VM/flag ERB syntax
|
||||
- Test end-to-end
|
||||
|
||||
### Rollback Strategy
|
||||
|
||||
- Controller changes are additive (new actions)
|
||||
- Migration can be reversed (re-add unique index)
|
||||
- Frontend JS/CSS can be reverted
|
||||
- Existing games unaffected (no schema changes to player_state)
|
||||
|
||||
---
|
||||
|
||||
## Suggested Minor Updates to Plan
|
||||
|
||||
### 1. Add Policy Code to Phase 2
|
||||
|
||||
Insert after line 2.13 in the Implementation Checklist:
|
||||
```
|
||||
- [ ] 2.14 Add policy methods: GamePolicy#submit_flag?, MissionPolicy#create_game?
|
||||
```
|
||||
|
||||
### 2. Update `hacktivity_mode?` in Implementation
|
||||
|
||||
Update Phase 1.5 to explicitly note updating the existing method:
|
||||
```
|
||||
- [ ] 1.5 Update Mission.hacktivity_mode? to check for ::VmSet and ::FlagService
|
||||
```
|
||||
|
||||
### 3. Remove Redundant VmSet Authorization
|
||||
|
||||
In the `games#create` action code (lines 830-833), simplify:
|
||||
```ruby
|
||||
# REMOVE THIS LINE:
|
||||
authorize vm_set, :use? if defined?(Pundit)
|
||||
|
||||
# The valid_vm_sets_for_user check below is sufficient
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
The implementation plan is **ready for implementation**. The three previous reviews and Hacktivity compatibility analysis have addressed all critical issues. The plan provides:
|
||||
|
||||
- ✅ Complete code examples for all changes
|
||||
- ✅ Clear implementation order
|
||||
- ✅ Comprehensive error handling
|
||||
- ✅ Both Hacktivity and standalone mode support
|
||||
- ✅ Proper security patterns (server-side flag validation)
|
||||
- ✅ Async console file handling via ActionCable
|
||||
|
||||
**Recommendation**: Proceed with implementation following the revised implementation order in the plan (Phase 1: Controller Infrastructure first).
|
||||
|
||||
---
|
||||
|
||||
## Appendix: Quick Reference
|
||||
|
||||
### Files to Create (New)
|
||||
```
|
||||
db/migrate/[timestamp]_remove_unique_game_constraint.rb
|
||||
app/views/break_escape/games/new.html.erb
|
||||
public/break_escape/js/systems/hacktivity-cable.js
|
||||
public/break_escape/js/minigames/vm-launcher/vm-launcher-minigame.js
|
||||
public/break_escape/js/minigames/flag-station/flag-station-minigame.js
|
||||
public/break_escape/css/minigames/vm-launcher.css
|
||||
public/break_escape/css/minigames/flag-station.css
|
||||
```
|
||||
|
||||
### Files to Modify (Existing)
|
||||
```
|
||||
app/models/break_escape/mission.rb
|
||||
app/models/break_escape/game.rb
|
||||
app/controllers/break_escape/games_controller.rb
|
||||
app/controllers/break_escape/missions_controller.rb
|
||||
app/policies/break_escape/game_policy.rb
|
||||
app/policies/break_escape/mission_policy.rb
|
||||
app/views/break_escape/games/show.html.erb
|
||||
public/break_escape/js/minigames/index.js
|
||||
public/break_escape/js/systems/interactions.js
|
||||
config/routes.rb
|
||||
```
|
||||
|
||||
### Key API Endpoints (New)
|
||||
```
|
||||
GET /break_escape/games/new?mission_id=:id
|
||||
POST /break_escape/games
|
||||
POST /break_escape/games/:id/flags
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user