0
✦ 品質
🛡 セキュリティ
⚙ 保守性
📋 概要
Ruby on Rails 7 + Stimulus.js で構築された育成型タスク管理Webアプリ「RewardMe」。ミニゲーム5種・ガチャ・マイルーム・リボンちゃん育成など、ゲームロジックをフルスクラッチ実装している個人開発ポートフォリオ作品。
🎯 今すぐ直すべき3点
-
app/javascript/controllers/match_game_controller.js デバッグ用URLパラメータ（?fx=nostar,nozoom 等）を本番コードから削除する。現状 `document.documentElement.dataset.mgFx = fx` でユーザー入力をDOMに直接書き込んでおり、XSSの踏み台になりうる。`connect()` 内の `const fx = ...` ブロックごと削除し、本番リリース前に必ず除去すること。↗
-
app/javascript/controllers/match_game_controller.js 24,289文字の巨大コントローラを責務ごとに分割する。ゲームロジック（盤面生成・マッチ判定・連鎖処理）・アニメーション制御・スコア管理・UI更新を別クラスまたはモジュールに切り出し、Stimulus コントローラは画面遷移とイベント委譲のみに絞ること。↗
-
app/models/user.rb 11,029文字のUserモデルに集中しているゲームドメインロジック（ribbon_stage / ribbon_exp / ribbon_exp_percent / next_stage_tasks / badge 等）をPORO（例: RibbonProgressCalculator）に抽出する。現状Userモデルが永続化・認証・ゲームロジック・表示ロジックを全て担っており、単一責任原則に違反している。↗
🔍 検出された問題 (6件)
🟧 Warning
📄 app/javascript/controllers/match_game_controller.js
デバッグ用URLパラメータがユーザー入力をDOMに直接書き込む（潜在的XSS）
`match_game_controller.js` の `connect()` 内で `const fx = new URLSearchParams(location.search).get('fx') || ''` を取得し、`document.documentElement.dataset.mgFx = fx` としてDOMに直接セットしている。data属性への代入自体はスクリプト実行を引き起こさないが、CSSセレクタや他のJS処理がこの値を参照する場合にインジェクションの踏み台となりうる。加えて本番コードにデバッグ用分岐が残存しており、攻撃面を不必要に広げている。
🟧 Warning
📄 app/javascript/controllers/match_game_controller.js
Stimulusコントローラが巨大すぎて責務が分離されていない
`match_game_controller.js`（24,289文字）・`potion_game_controller.js`（22,552文字）・`flight_game_controller.js`（13,900文字）がそれぞれゲームロジック・アニメーション・UI更新・通信処理を1ファイルに詰め込んでいる。Stimulusコントローラは本来DOM接続の薄いグルーコードであるべきで、この規模になるとテスト不能・変更困難・バグ混入リスクが高い。
🟧 Warning
📄 app/models/user.rb
Userモデルへのゲームロジック集中による単一責任原則違反
`app/models/user.rb`（11,029文字）が `has_secure_password` による認証・タスク関連付け・ゲスト管理・ribbon_stage/ribbon_exp/badge/room_bg_image などのゲームドメインロジックを全て担っている。モデルが肥大化するとテストの粒度が粗くなり、ゲームバランス変更時に認証コードへの影響範囲が読めなくなる危険がある。
🟧 Warning
📄 app/javascript/controllers/puzzle_controller.js
puzzle_controller.js の fetch エラーハンドリングがサイレント握り潰し
`puzzle_controller.js` の `selectPuzzle()` メソッドで `catch { /* silent */ }` としており、ネットワークエラー・CSRF失敗・サーバーエラーをすべて無視している。ユーザーにはパズル選択が成功したように見えるが実際には失敗しており、ゲーム状態の不整合（選択済み画像と実際の保存値のズレ）が発生しうる。
🟨 Suggestion
📄 app/javascript/controllers/tap_game_controller.js
RIBBON_TYPES の累積確率設計が不完全でランダム選択ロジックが壊れやすい
`tap_game_controller.js` の `RIBBON_TYPES` は `prob` フィールドを持つが、`_pickType()` は `cum += t.prob` で累積加算しており、最後の要素の `prob: 1.00` は「残り全部」ではなく「+1.00加算」を意味する。現状は最後の要素がフォールバックになっているが、配列の順序変更や要素追加で確率分布が壊れる。累積確率（cumProb）を事前計算して定数化するか、正規化処理を明示すべき。
🟨 Suggestion
📄 app/models/letter.rb
app/models/letter.rb のコンテンツがハードコードされており国際化・変更が困難
`Letter` クラスが `CATALOG` 定数にゲーム内メッセージ全文をRubyコードとして埋め込んでいる。文言変更のたびにコードデプロイが必要になり、将来的な多言語対応やA/Bテストが不可能。YAMLファイルやDBテーブルへの移行を検討すべき。
✅ 良い点
- app/models/user.rb の `completed_count` を唯一のソース・オブ・トゥルースとし、レベル・EXP・ステージ・称号をすべて計算で導出する設計は、状態の二重管理による不整合バグを構造的に排除しており、データ整合性の観点で優れている。
- Dockerfile がマルチステージビルド（builder / runtime 分離）を採用しており、`ARG SECRET_KEY_BASE=dummy` をビルドステージのみに限定することで最終イメージにダミーシークレットが残らない点は、セキュリティ意識の高い実装といえる。
- app/javascript/controllers/match_game_controller.js がアニメーション時間を定数 `T`（swap/revert/clear/fall/settle）に、ゲーム状態を定数 `S`（idle/swapping/resolving）に集約しており、マジックナンバーの散在を防いで変更コストを局所化している。
🔧 リファクタ提案
- match_game_controller.js・potion_game_controller.js のゲームロジックをStimulusコントローラから分離し、純粋なJSクラス（例: MatchGameEngine, PotionGameEngine）として切り出す。Stimulusコントローラはイベント受信とエンジン呼び出しのみに絞ることで、Stimulusなしでのユニットテストが可能になる。
- app/models/user.rb のリボン進化ロジック（ribbon_stage / ribbon_exp / ribbon_exp_percent / next_stage_tasks / ribbon_title）を `RibbonProgress` POROwrap（例: app/models/ribbon_progress.rb）に抽出し、`User#ribbon_progress` で委譲する。これによりUserモデルのテストとゲームロジックのテストを独立させられる。
- puzzle_controller.js の `catch { /* silent */ }` を `catch(err) { this._showError('通信エラーが発生しました') }` に置き換え、ユーザーへのフィードバックとコンソールへの `console.error(err)` ログを追加する。fetch レスポンスの `resp.ok` チェックも `if (!resp.ok) throw new Error(resp.status)` として明示的に例外化する。
🔒 セキュリティ注意点
- match_game_controller.js の `document.documentElement.dataset.mgFx = new URLSearchParams(location.search).get('fx')` はURLパラメータをDOMに直接反映している。現時点でのXSSリスクは限定的だが、本番環境では不要なデバッグコードであり即時削除が必要。
- puzzle_controller.js の fetch で `document.querySelector('meta[name="csrf-token"]').content` を使ったCSRFトークン送信は適切だが、`querySelector` が null を返した場合（CSPやDOM操作による欠落）に `.content` でTypeErrorが発生しリクエストが中断する。`?.content ?? ''` でnullガードを追加し、トークン欠落時はユーザーに再ログインを促すべき。
📤 PRコメント出力
このレビューをGitHub PRコメントとして貼り付けられるMarkdown形式で出力します。
Generated with CodeLens AI v1.0
Designed & Developed by Mize
View Creator
mize1978.github.io
本レビューは CodeLensAI による自動解析結果であり、リポジトリ所有者による投稿・承認を意味しません。公開リポジトリを対象に生成され、公開アーカイブに保存されます。