0
✦ 品質
🛡 セキュリティ
⚙ 保守性
📋 概要
Ruby on Rails 7 + Stimulus.js で構築された育成型タスク管理Webアプリ「RewardMe」。タスク完了によるキャラクター育成・ガチャ・ミニゲーム5種・マイルーム18種などゲーム要素をフルスクラッチ実装している。
🎯 今すぐ直すべき3点
-
app/javascript/controllers/gacha_controller.js fetch の .then チェーンで data.error チェック後も処理が続く可能性がある構造を修正し、コイン不足時に return を確実に実行していることを確認。加えて、サーバー側でガチャ結果の整合性(コイン残高の二重消費)を防ぐ冪等性キーまたはトランザクションロックを実装する↗
-
app/javascript/controllers/match_game_controller.js ファイル全体が16180文字に対し3000文字しか確認できていないが、確認範囲内でゲームスコア・コイン付与がクライアント側で計算されている可能性が高い。サーバー側でスコア上限バリデーションを追加し、クライアントから送信されるスコア値を信頼しない実装に変更する↗
-
app/models/user.rb open_box! メソッドのコメントに「行ロックを取り」と記載があるが、truncated のため実装が確認できない。SELECT FOR UPDATE(lock!)が実際に使われているか全文を確認し、楽観的ロックまたは悲観的ロックが正しく機能していることをテストで保証する↗
🔍 検出された問題 (7件)
🟥 Critical
📄 app/javascript/controllers/match_game_controller.js
ゲームスコア・コイン付与がクライアント主導で行われている疑い
match_game_controller.js・tap_game_controller.js・puzzle_controller.js 等の複数ゲームコントローラーがスコアをクライアント側で計算し、結果URLへ POST している構造が確認できる。サーバー側でスコアの妥当性検証(時間内に達成可能な最大値チェック等)がなければ、任意のコイン量をリクエスト改ざんで付与できる。ゲーム経済圏全体が崩壊するリスクがある。
🟧 Warning
📄 app/javascript/controllers/match_game_controller.js
複数のStimulusコントローラーが50行を大幅に超える巨大ファイルになっている
match_game_controller.js(16180文字)・potion_game_controller.js(10741文字)・puzzle_controller.js(9969文字)・tap_game_controller.js(9486文字)はいずれも単一ファイルにゲームロジック・UI操作・アニメーション・サーバー通信を混在させている。責務分離がなく、バグ修正時の影響範囲が広く、テストも困難になる。
🟧 Warning
📄 app/javascript/controllers/ribbon_controller.js
ribbon_controller.js に大量のハードコードされた文字列定数が存在する
ribbon_controller.js(7251文字)の確認範囲内だけで ROOM_MESSAGES・SPECIAL_COMBOS に数十件のセリフ文字列がハードコードされている。コンテンツ変更のたびにJSファイルを修正・デプロイが必要になり、多言語化・A/Bテスト・コンテンツ管理が困難。サーバーサイドまたはJSONファイルで管理すべきデータがJSに埋め込まれている。
🟧 Warning
📄 app/models/letter.rb
app/models/letter.rb が ActiveRecord を継承しないPORO設計でDBに永続化されていない
Letter クラスが CATALOG 定数にすべてのレター内容をハードコードしており、received_at が文字列リテラルで埋め込まれている。新しいレターの追加・既存レターの修正にデプロイが必要で、ユーザーごとの既読管理や動的なコンテンツ配信が困難になる設計上の制約がある。
🟧 Warning
📄 app/javascript/controllers/gacha_controller.js
gacha_controller.js のエラーハンドリングが alert() に依存している
gacha_controller.js の pull メソッドでエラー時に alert(data.error === 'coins_insufficient' ? ... : 'エラーが発生したよ') を使用している。alert() はブラウザのデフォルトUIでUXが悪く、また .catch() ブロックがネットワークエラーのみを捕捉しHTTPエラーステータス(4xx/5xx)を捕捉しない構造になっている可能性がある。
🟨 Suggestion
📄 app/javascript/controllers/puzzle_controller.js
puzzle_controller.js の selectPuzzle メソッドが catch { /* silent */ } でエラーを握りつぶしている
fetch 失敗時に catch { /* silent */ } としており、ネットワークエラーや認証エラーが発生してもユーザーへのフィードバックが一切ない。ユーザーは絵柄選択が成功したと誤認したままゲームを開始し、意図しない画像でプレイする可能性がある。
🟨 Suggestion
📄 README.md
README.md が14714文字中3000文字しか確認できていないが、セットアップ手順に .env の設定例が必要
確認できた範囲のREADMEにはセットアップセクションへの目次リンクはあるが、必要な環境変数の一覧(.env.example)が存在するかどうかが未確認。SECRET_KEY_BASE・DB接続情報等の環境変数が文書化されていない場合、新規開発者のオンボーディングコストが高くなる。
✅ 良い点
- app/models/user.rb の open_box! メソッドがコメントで並列リクエスト対策(行ロック+二重取得防止)を明示しており、デイリーBOXの複数回取得という典型的な競合状態を設計レベルで意識している
- Dockerfile がマルチステージビルド(builder / runtime 分離)を採用しており、ビルド時のみ必要な ARG SECRET_KEY_BASE=dummy を最終イメージに残さない構成になっている点がセキュリティ上適切
- app/models/user.rb の completed_count を唯一の永続化ソースとし、level・EXP・stage・称号をすべて純粋関数的メソッド(ribbon_stage, ribbon_exp, ribbon_exp_percent 等)で導出する設計が、状態の不整合を構造的に排除している
🔧 リファクタ提案
- match_game_controller.js・tap_game_controller.js・puzzle_controller.js のゲームロジック部分(スコア計算・タイマー・スポーン処理)を GameEngine クラスとして切り出し、StimulusコントローラーはイベントハンドリングとDOM操作のみに責務を限定する
- app/models/user.rb の ribbon_stage・ribbon_exp・ribbon_exp_percent 等の育成計算メソッド群を RibbonProgress バリューオブジェクト(Plain Ruby Object)に抽出し、User モデルの肥大化を防ぐとともに単体テストを容易にする
- ribbon_controller.js の ROOM_MESSAGES・SPECIAL_COMBOS をサーバーサイドのJSONエンドポイントまたはdata属性経由で受け渡す設計に変更し、コンテンツ更新をデプロイなしで行えるようにする
🔒 セキュリティ注意点
- ゲームスコアをクライアント側で計算してサーバーに送信する設計の場合、リクエスト改ざんによる不正コイン取得が可能になる。サーバー側でセッション開始時刻・ゲーム種別・理論上の最大スコアを記録し、受信スコアの妥当性を検証するバリデーション層を追加すること
- gacha_controller.js の fetch POST でCSRFトークンを正しく付与しているのは適切だが、ガチャ結果(レアリティ・アイテム)の決定はサーバー側のみで行い、クライアントには結果のみを返す設計になっているか確認が必要。クライアント側でレアリティを操作できる余地がないかコードレビューを実施すること
📤 PRコメント出力
このレビューをGitHub PRコメントとして貼り付けられるMarkdown形式で出力します。