mize1978/rewardme — CodeLens AI

GOOD

改善余地があります

mize1978/rewardme

completed_countという単一真実源からゲーム全体を導出するアーキテクチャは秀逸だが、そのコインを守るトランザクション処理が存在しないため、並列リクエストで経済崩壊する「設計は天才、実装は未完成」なアプリ。

⭐ 2 Stars

🍴 0 Forks

📝 195 Commits

💬 5 Issues

🔵 CSS

Rails

🔍 重要ファイル 11 件を精査全 362 ファイル中 AI が優先度順に選択

✦ 品質

🛡 セキュリティ

⚙ 保守性

📋 概要

Ruby on Rails 7 + Hotwire(Turbo/Stimulus)で構築された育成型タスク管理Webアプリ。タスク完了によるキャラクター育成・ガチャ・ミニゲーム5種・マイルームカスタマイズなどゲーム要素をフルスクラッチ実装している。

🎯 今すぐ直すべき3点

app/models/user.rb open_box!メソッドおよびset_bg!メソッドにおけるコイン増減・BOX開封処理をActiveRecord::Base.transactionでラップし、競合状態（Race Condition）によるコイン二重取得を防ぐ。特にopen_box!はbox_available?チェックとupdate!が非アトミックなため、並列リクエストで複数回実行される危険がある。
↗
app/models/user.rb User モデルにemail/usernameのvalidates（presence, uniqueness, format）が確認できない。has_secure_passwordだけでは入力バリデーションが不十分で、空文字やSQL特殊文字を含む値が登録される可能性がある。validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP } を追加する。
↗
app/javascript/controllers/gacha_controller.js gacha_controller.jsのpullメソッドでfetchエラー時にalert()でエラー内容を表示しているが、サーバーからのエラーメッセージをそのままDOMに反映する箇所でXSSリスクがある。_buildCardメソッド等でinnerHTMLにサーバーデータを直接挿入している場合はtextContentまたはDOMPurifyによるサニタイズに置き換える。
↗

🔍 検出された問題 (8件)

🟥 Critical 📄 app/models/user.rb

open_box!のRace Conditionによるコイン二重取得

app/models/user.rbのopen_box!メソッドはbox_available?チェックとupdate!が別トランザクションで実行されるため、並列リクエストが同時に到達した場合に複数回BOXが開封されコインが不正に増加する。ゲーム内経済を破壊するリスクがある。

🟥 Critical 📄 app/models/user.rb

Userモデルに入力バリデーションが存在しない

app/models/user.rbにemail・username等のvalidatesが確認できず、has_secure_passwordのみでは空文字・重複・不正フォーマットの登録を防げない。不正なデータがDBに蓄積され、認証ロジックの誤動作や予期しないエラーを引き起こす危険がある。

🟧 Warning 📄 app/javascript/controllers/gacha_controller.js

JavaScriptコントローラーでinnerHTMLにサーバーデータを直接挿入している可能性

app/javascript/controllers/gacha_controller.jsの_buildCardメソッド（コードが途中で切れているが）でサーバーレスポンスのデータをinnerHTMLで挿入している場合、サーバー側でエスケープが不十分だとストアドXSSが成立する。textContentへの置き換えまたはDOMPurifyによるサニタイズが必要。

🟧 Warning 📄 app/javascript/controllers/gacha_controller.js

ガチャ・ゲーム報酬エンドポイントにレート制限がない

gacha_controller.jsのpullおよびpotion_game_controller.js/tap_game_controller.jsのresultUrl送信エンドポイントにレート制限が確認できない。連打・自動化ツールによりコインを無制限に取得できる可能性があり、ゲーム内経済が崩壊するリスクがある。

🟧 Warning

テストコードが存在しない（またはほぼ皆無）

Gemfileにrspec-rails/factory_bot_railsが宣言されているにもかかわらず、提供されたコードにspecファイルが存在しない。completed_countを起点とするゲームロジック（ribbon_stage/ribbon_exp等）はビジネスクリティカルな計算であり、テストなしでのリファクタリングは回帰バグのリスクが高い。

🟧 Warning 📄 app/models/user.rb

app/models/user.rbの責務過多（God Model）

app/models/user.rbにゲームロジック（ribbon_stage/exp/level）・部屋管理（set_bg!）・デイリーBOX（open_box!）・バッジ計算（badge）が混在しており、1ファイルに複数の責務が集中している。変更時の影響範囲が広く、バグの温床になりやすい。

🟨 Suggestion 📄 app/javascript/controllers/match_game_controller.js

match_game_controller.jsなど複数のStimulusコントローラーが500行超の巨大ファイルになっている可能性

match_game_controller.js・potion_game_controller.js・tap_game_controller.jsはゲームロジック・UI更新・アニメーション・API通信を1ファイルに集約しており、コードが途中で切れていることからも巨大化が推測される。責務ごとにモジュール分割しないと保守性が著しく低下する。

🟨 Suggestion 📄 app/models/letter.rb

app/models/letter.rbがPOROで静的データをハードコード

app/models/letter.rbはActiveRecordを継承せずCATALOGという定数配列にすべてのレターデータをハードコードしている。コンテンツ追加のたびにコードデプロイが必要になり、将来的な多言語化・管理画面対応が困難になる。

✅ 良い点

app/models/user.rbのribbon_stage/ribbon_exp/ribbon_exp_percentが completed_count という単一の永続化値から全ゲームステータスを純粋関数的に導出しており、状態の不整合が構造的に発生しにくい設計になっている
Dockerfileがマルチステージビルド（builder/runtime分離）を採用しており、ARG SECRET_KEY_BASE=dummyをビルドステージのみに限定することで本番シークレットが最終イメージに残らないよう配慮されている
Gemfileでmysql2をdevelopment/testグループ、pgをproductionグループに分離しており、環境ごとのDB切り替えが明示的に管理されている。またrspec-rails/factory_bot_railsがtest依存として宣言されている点も適切

作り切ることが第一歩。改善を続けることがエンジニアリング。

🔧 リファクタ提案

app/models/user.rbのゲームロジック（ribbon_stage/exp/level/badge）をapp/models/concerns/ribbon_progress.rbとしてConcernに抽出し、部屋管理はRoomManagerサービスオブジェクト、BOX処理はDailyBoxServiceとして分離することでGod Model問題を解消する
app/javascript/controllers/match_game_controller.jsなどの巨大Stimulusコントローラーを、ゲームロジック（スコア計算・タイマー）をgame_engine.jsモジュールとして分離し、コントローラーはDOM操作とイベントハンドリングのみに責務を絞る構成にリファクタリングする
app/models/letter.rbのCATALOGをYAMLファイル（config/letters.yml）またはSeedデータ（db/seeds/letters.rb）に移行し、コードとコンテンツを分離することでデプロイなしでのコンテンツ更新を可能にする

🔒 セキュリティ注意点

ガチャ・ミニゲーム報酬のサーバーサイド処理にRack::Attack等でレート制限を設定すること。クライアントサイドのゲームロジック（スコア計算）はすべてサーバー側で再検証し、クライアントから送られるスコア値をそのまま信用しないこと
open_box!・set_bg!・gacha pullの各メソッドはActiveRecord::Base.transactionとlockによる楽観的/悲観的ロックを実装し、並列リクエストによるコイン不正取得を防ぐこと。特にopen_box!のbox_available?チェックはwhere(...).lock(true).firstパターンで原子的に実行する必要がある

📤 PRコメント出力

このレビューをGitHub PRコメントとして貼り付けられるMarkdown形式で出力します。

## CodeLens AI Code Review

**mize1978/rewardme** の自動レビュー結果です。

### スコア
| 総合 | 品質 | セキュリティ | 保守性 |
|------|------|--------------|--------|
| **62** | 62 | 65 | 60 |

### 検出された問題
- 🟥 **[Critical] open_box!のRace Conditionによるコイン二重取得** — app/models/user.rbのopen_box!メソッドはbox_available?チェックとupdate!が別トランザクションで実行されるため、並列リクエストが同時に到達した場合に複数回BOXが開封されコインが不正に増加する。ゲーム内経済を破壊するリスクがある。
- 🟥 **[Critical] Userモデルに入力バリデーションが存在しない** — app/models/user.rbにemail・username等のvalidatesが確認できず、has_secure_passwordのみでは空文字・重複・不正フォーマットの登録を防げない。不正なデータがDBに蓄積され、認証ロジックの誤動作や予期しないエラーを引き起こす危険がある。
- 🟧 **[Warning] JavaScriptコントローラーでinnerHTMLにサーバーデータを直接挿入している可能性** — app/javascript/controllers/gacha_controller.jsの_buildCardメソッド（コードが途中で切れているが）でサーバーレスポンスのデータをinnerHTMLで挿入している場合、サーバー側でエスケープが不十分だとストアドXSSが成立する。textContentへの置き換えまたはDOMPurifyによるサニタイズが必要。
- 🟧 **[Warning] ガチャ・ゲーム報酬エンドポイントにレート制限がない** — gacha_controller.jsのpullおよびpotion_game_controller.js/tap_game_controller.jsのresultUrl送信エンドポイントにレート制限が確認できない。連打・自動化ツールによりコインを無制限に取得できる可能性があり、ゲーム内経済が崩壊するリスクがある。
- 🟧 **[Warning] テストコードが存在しない（またはほぼ皆無）** — Gemfileにrspec-rails/factory_bot_railsが宣言されているにもかかわらず、提供されたコードにspecファイルが存在しない。completed_countを起点とするゲームロジック（ribbon_stage/ribbon_exp等）はビジネスクリティカルな計算であり、テストなしでのリファクタリングは回帰バグのリスクが高い。
- 🟧 **[Warning] app/models/user.rbの責務過多（God Model）** — app/models/user.rbにゲームロジック（ribbon_stage/exp/level）・部屋管理（set_bg!）・デイリーBOX（open_box!）・バッジ計算（badge）が混在しており、1ファイルに複数の責務が集中している。変更時の影響範囲が広く、バグの温床になりやすい。
- 🟨 **[Suggestion] match_game_controller.jsなど複数のStimulusコントローラーが500行超の巨大ファイルになっている可能性** — match_game_controller.js・potion_game_controller.js・tap_game_controller.jsはゲームロジック・UI更新・アニメーション・API通信を1ファイルに集約しており、コードが途中で切れていることからも巨大化が推測される。責務ごとにモジュール分割しないと保守性が著しく低下する。
- 🟨 **[Suggestion] app/models/letter.rbがPOROで静的データをハードコード** — app/models/letter.rbはActiveRecordを継承せずCATALOGという定数配列にすべてのレターデータをハードコードしている。コンテンツ追加のたびにコードデプロイが必要になり、将来的な多言語化・管理画面対応が困難になる。

> 🤖 Generated with [CodeLens AI](http://localhost:3003) — crafted by Mize

← 新しいレビューを開始 📚 Review Archive

Generated with CodeLens AI v1.0

Designed & Developed by Mize

View Creator mize1978.github.io

本レビューは CodeLensAI による自動解析結果であり、リポジトリ所有者による投稿・承認を意味しません。公開リポジトリを対象に生成され、公開アーカイブに保存されます。