0
✦ 品質
🛡 セキュリティ
⚙ 保守性
📋 概要
Ruby on Rails 7 + Hotwire(Turbo/Stimulus)で構築された育成型タスク管理Webアプリ。タスク完了数(completed_count)を唯一の永続データとし、レベル・EXP・ガチャ・ミニゲーム5種をクライアント側で演出する個人開発ポートフォリオ作品。
🎯 今すぐ直すべき3点
-
app/controllers/games_controller.rb tap_game_result アクションの score パラメータを .to_i.clamp(0, 999) でサニタイズしているが、クライアント側で計算したスコアをそのままサーバーに送信する設計自体を見直す。スコア検証ロジック(coins_for の換算テーブルとスコア上限)をモデル層(User もしくは GameScore モデル)に移し、Controller は受け取り口のみにする。また、同アクション内でコイン加算と last_played_at 更新を1つの update! でまとめているが、played_today? チェックとの間に TOCTOU 競合が発生しうるため、DB側にユニーク制約または Redis による排他ロックを追加する。↗
-
app/controllers/games_controller.rb GACHA_POOL をコントローラー定数として直書きしているため、重み・レアリティ・報酬種別がビジネスロジックとしてテスト不可能な状態になっている。GachaItem バリューオブジェクトまたは app/services/gacha_service.rb に切り出し、抽選ロジックに対するユニットテストを追加する。↗
-
Dockerfile ビルドステージで ARG SECRET_KEY_BASE=dummy を使ってアセットプリコンパイルしているが、最終イメージ(Runtime stage)に .env や credentials ファイルが COPY . . でまるごとコピーされるリスクがある。.dockerignore に .env, config/master.key, config/credentials.yml.enc を追加し、SECRET_KEY_BASE は Runtime stage では環境変数インジェクション(Render の環境変数設定)のみで供給する構成に変更する。↗
🔍 検出された問題 (8件)
🟥 Critical
📄 app/controllers/games_controller.rb
ゲームスコアのサーバーサイド検証欠如
tap_game_result, match_game_result(推定) などのゲーム結果エンドポイントが、クライアントから送信されたスコア値を .to_i.clamp(0, 999) のみで受け入れている。30秒のタップゲームで999点が物理的に達成可能かどうかをサーバーが検証していないため、curlコマンド一発でコインを上限まで取得できる。コイン経済圏の根幹を破壊するチート行為が無防備な状態。
🟥 Critical
📄 app/controllers/games_controller.rb
TOCTOU競合によるコイン二重付与の可能性
tap_game_result で tap_game_played_today? チェック後に update! でコイン加算するまでの間に、並行リクエストが同じチェックを通過できる。Read-Check-Update のパターンは非アトミックであり、高速で2回リクエストを送ると両方がチェックを通過してコインが二重付与される。DBのユニーク制約またはRedis排他ロック・楽観的ロックなどでアトミック性を保証する必要がある。
🟥 Critical
📄 Dockerfile
.dockerignore の欠如による機密ファイルのイメージへの混入リスク
Dockerfile の Runtime stage で COPY . . を実行しており、.dockerignore が適切に設定されていない場合、config/master.key・config/credentials.yml.enc・.env などの機密ファイルがDockerイメージに焼き込まれる。イメージをレジストリにpushした場合、秘密鍵が公開される致命的なリスク。
🟧 Warning
📄 app/controllers/games_controller.rb
GachaPoolとゲームロジックがControllerに直書き
GACHA_POOL 定数(約20エントリ)がGamesControllerに直接定義されており、抽選ロジックもコントローラー内に存在する(推定)。ビジネスロジックがコントローラー層にあるためユニットテストが書けず、将来のガチャ仕様変更時の影響範囲が不明確になる。Service層またはPORO(Plain Old Ruby Object)への切り出しが必要。
🟧 Warning
📄 app/models/letter.rb
app/models/letter.rb がActiveRecordを継承しないPORO設計でテスト不能
Letter クラスが CATALOG 配列をクラス内定数として保持するだけの設計で、テンプレートエンジンも持たない。trigger条件の判定ロジックがどこで行われているか不明だが、モデルとしてテストフックがなく、文字列コンテンツの変更が無検証で本番に反映される。YAMLやi18nファイルへの切り出しで管理しやすくすべき。
🟧 Warning
📄 app/javascript/controllers/match_game_controller.js
Stimulusコントローラー内のsetTimeout多用によるメモリリーク
match_game_controller.js・tap_game_controller.js・gacha_controller.js で setTimeout/setInterval が多数使われているが、disconnect() でのクリアアップが tap_game_controller.js でのみ確認できる。Turboによるページ遷移でコントローラーが disconnect されてもタイマーが残存し、DOM参照によるメモリリークやゴースト処理が発生するリスクがある。
🟨 Suggestion
📄 app/javascript/controllers/ribbon_controller.js
ribbon_controller.js のROOM_MESSAGES定数が肥大化
ribbon_controller.js 内にROOM_MESSAGES・SPECIAL_COMBOSが数百行規模でハードコードされており、JSファイルが単一責任を逸脱している。セリフデータをJSONファイルまたはRailsのI18nに移し、コントローラーはロジックのみに集約するとバンドルサイズと保守性が改善される。
🟨 Suggestion
📄 app/javascript/controllers/puzzle_controller.js
puzzle_controller.js の selectPuzzle で catch が silent
selectPuzzle の catch ブロックが /* silent */ でエラーを握りつぶしており、ネットワーク障害やCSRFトークン失効時にユーザーへのフィードバックが一切ない。最低限 console.error によるログ出力と、ユーザーへのエラー表示(トースト等)を追加する必要がある。
✅ 良い点
- completed_count を唯一のソース・オブ・トゥルースとし、レベル・EXP・進化ステージをすべて計算で導出するアーキテクチャ(README Architecture図)は、状態の不整合が原理的に発生しない設計で、個人開発の規模でも明確な設計方針が貫かれている点が優れている。
- app/javascript/controllers/tap_game_controller.js の _pickType() が累積確率(cumProb)で実装されており、RIBBON_TYPES の確率定義ミスがあっても最後の要素にフォールバックする実装になっているため、フロントエンドのゲームバランス崩壊リスクが抑えられている。
- Dockerfile がマルチステージビルド(builder / runtime の2ステージ)を採用し、アセットプリコンパイル用のダミーSECRET_KEY_BASEを最終イメージに残さない設計を意識している点は、コンテナイメージの肥大化と秘密情報の漏洩リスクを低減する良いアプローチである。
🔧 リファクタ提案
- app/controllers/games_controller.rb のガチャ抽選ロジック・GACHA_POOL定数を app/services/gacha_service.rb に切り出し、GachaService.new(user).pull(count) のインターフェースにまとめることでRSpecでのユニットテストを可能にする。
- app/models/letter.rb の CATALOG をYAML(config/letters.yml)に移行し、Letter.find(id)・Letter.for_trigger(trigger) のクラスメソッドでラップする。セリフ文言の変更がコードレビューなしで管理できるようになり、i18n対応への足がかりにもなる。
- 各Stimulusコントローラーで共通化されているカウントダウン処理(_countdown)・タイマー処理(_startTimer)・スコア送信処理(_submitResult)を app/javascript/utils/game_utils.js に切り出し、match_game/tap_game/potion_gameの各コントローラーからimportして再利用する。現状では同一パターンのコードが3ファイルに重複している。
🔒 セキュリティ注意点
- ゲーム結果エンドポイント(tap_game_result等)がクライアント申告のスコアをほぼ無検証で受け入れており、正規ユーザーでもDevToolsのfetchコマンドで任意コインを取得可能。スコアに対する物理的な上限値(TIME_LIMIT × MAX_SCORE_PER_SEC)をサーバーサイドで計算・検証するバリデーションを実装すること。
- Dockerfile の COPY . . が .dockerignore なしに実行された場合、config/master.key がイメージに混入しRails credentialsが復号可能になる。.dockerignore に config/master.key, config/credentials/*.key, .env を必ず追加し、CIパイプラインでイメージの秘密情報スキャン(truffleHog等)を組み込むこと。
📤 PRコメント出力
このレビューをGitHub PRコメントとして貼り付けられるMarkdown形式で出力します。
Generated with CodeLens AI v1.0
Designed & Developed by Mize
View Creator
mize1978.github.io
本レビューは CodeLensAI による自動解析結果であり、リポジトリ所有者による投稿・承認を意味しません。公開リポジトリを対象に生成され、公開アーカイブに保存されます。