TL;DR
- ある会員制対戦プラットフォームに「マルチプレイヤー形式」の機能を追加するPRをAI主導で実装した結果、+3278/-29 行・60 ファイル超の巨大PRが生まれた
- セルフレビュー無しでマージ寸前まで到達したが、人間レビュアーが見たら 致命的問題が 4 件・ロジックバグが 10 件・設計問題が 7 件 あった
- 致命的問題の代表は 「全 mutation API に認可が一切無い」（誰でも他人のリソースを改ざん可能）
- AI レビュアー（CodeRabbit）はサマリしか出さず、これらを 1 件も検出しなかった
- 根本原因は「実装フェーズに入る前に、認可マトリクス・失敗モード・冪等性を仕様化していなかった」こと
- この記事では実例を匿名化して紹介し、仕様段階で防ぐためのテンプレまで示す
この記事でできること
| やりたいこと | この記事で得られるもの |
|---|---|
| AI が書いた大規模 PR の典型的な穴を知りたい | 致命的問題 4 件 + ロジックバグ 10 件のカテゴリ別実例 |
| 認可漏れを構造的に防ぎたい | Authorization Matrix のテンプレと埋め方 |
| AI レビュアーが見逃すパターンを理解したい | CodeRabbit 等が苦手な指摘の種類 |
| 実装者にレビュー前のセルフチェックを習慣化させたい | レビュー前 5 ステップ |
「ブラウザで動いたから OK だと思いました」
ある日、ジュニアエンジニアから上がってきた PR のレビューを始めて、30 秒で背筋が凍りました。新しい mutation API が 5 本追加されているのに、全部認可ゼロ。誰でも他人のリソースを改ざんできる状態で、本人もレビュアー（自称）の AI ボットも気付いていなかったのです。
この記事では、その PR で見つかった問題を 実例ベース で分類し、なぜそれが起きたのか、どうすれば仕様段階で防げるのかを書きます。機能名・組織名は全て匿名化していますが、起きていることは現場でよく見る話だと思います。
舞台設定（匿名化済み）
- ある会員制対戦プラットフォーム
- 既存は 1 対 1 のスクリムマッチのみ対応
- 新機能として「マルチプレイヤー形式」（最大 20 チームが同時参加するイベント）を追加
- 実装者: AI 駆動開発に慣れていないジュニア
- AI: Claude Code / Cursor 系
- 既存スタック: Python + FastAPI + SQLAlchemy + PostgreSQL + 外部チャットツール（Discord）+ 画像解析 AI（Gemini）
- PR サイズ: +3278 / -29 行、60+ ファイル
- レビュアー: 人間 1 名 + AI コードレビュアー（CodeRabbit）
PR の見た目は綺麗で、テストも書かれていて、CI も緑でした。ぱっと見「動いてるなら OK」と思いそうな PR です。
致命的問題 4 件（マージブロッカー）
1. 全 mutation API に認可が一切無い 🚨
これが一番ヤバいやつです。
@router.post("/scrim-matches/{scrim_match_id}/items")
async def create_item(
scrim_match_id: UUID,
result_image: UploadFile = File(None),
db: AsyncSession = Depends(deps.get_db),
) -> Any:
"""
Create a new item by uploading a result image.
"""
# ↑ Depends(deps.get_current_active_user) すら無い
Depends(deps.get_current_active_user) も Depends(deps.get_current_active_superuser) も無い。未認証で叩けるし、他人の試合結果を任意に書き換え・削除できる。
5 本追加された mutation エンドポイント (POST / PUT / DELETE / 管理者用) のうち、明示的に superuser ガードがあったのは 1 本だけ。残り 4 本はノーガード。
実装者の誤解:
- 「画面からは admin 経由でしか叩けないから大丈夫」
- → API は 画面と独立した攻撃面。直接 curl で叩けば誰でも到達する
- 「テストではちゃんと動いた」
- → テストは admin として叩いていたから、認可漏れがあっても通る
2. ユーザーアップロード画像 18 枚がリポジトリにコミットされていた
Gemini で解析テストをした際の 実際のスクリーンショット 18 枚がリポジトリの static/ 配下に commit されていました。プレイヤー名・ID・戦績が映り込んでいる可能性があり、個人情報インシデント一歩手前。
static/
└── result_images/
├── 05d43229-e066-4e63-88a6-aedd56b21ab4.png
├── 102fdef9-c0b6-4f8e-8bd2-caef89b0e0f7.png
... (18 枚)
根本原因:
- ローカル開発で
upload_image()関数が S3 ではなくstatic/フォルダに落とす実装になっていた -
git add .で全部入った -
.gitignoreにstatic/result_images/が無かった - pre-commit hook で大ファイルを弾く設定も無かった
S3 にアップする想定の実装が、本番だと複数インスタンスで分散してアクセス不能になる構造的バグでもあります。
3. .DS_Store がコミット（しかも更新までされている）
macOS のゴミファイル。.gitignore に入っていない時点で、開発環境の整備状況が分かります。
4. lint ルール 20 個を新規 disable
- --disable=C0111,E1102,R0903,E1101,W0107,C0103,R0901,E0213,R0913
+ --disable=C0111,E1102,R0903,E1101,W0107,C0103,R0901,E0213,R0913,
+ C0415,C0302,W0613,W0707,W0404,R1702,W0621,W0612,W0611,W0212,
+ R0912,R0904,R0915,E0015,C0301,R0914,W1203,W0603
無効化された内容:
-
C0415(関数内 import) — 本当はファイル冒頭に書くべき -
W0212(private 属性アクセス) — 外部ライブラリの_stateを直叩きしていた -
R0915(関数行数超過) — 100 行超のハンドラがある -
W0707(raise from 省略) — 例外チェーンを切っていた -
R0914(too-many-locals) — 関数内変数が多すぎる
「コードを直す」ではなく「警告を消す」。これは典型的な "まずいシグナル" です。
ロジックバグ 10 件のうち、特にヤバい 5 つ
5. 採番ジョブの冪等性が破綻
team_number = len(existing_participants) + 1 # ← 既存数+1から始める
for entry in entries:
if entry.id in existing_entry_ids:
continue # ← 既存はスキップ
participant = Participant(team_number=team_number, ...)
team_number += 1
仕様: 「チェックイン時刻順に 1, 2, 3 ... と採番」
実装: 「既存 5 件あって、新規 3 件来たら 新規には 6, 7, 8 を振る」
→ 新規エントリーが時刻順を無視して後ろにくっつく。リトライで二重実行されると、後から来た人が早い時刻でチェックインしていても、後ろの番号になります。
「冪等にしたつもり」だが実質的に冪等ですらない。本番の運用 1 回目に絶対事故るタイプ。
6. ポイント計算で int() 切り捨てバグ
team_stats[result.team_id]["games"].append(
GameStanding(
...
points=int(game_points) # ← 切り捨て
)
)
team_stats[result.team_id]["total_points"] += game_points # ← float のまま
kill_multiplier=1.5、kills=3 → game_points=4.5
- 各ゲームの points = 4 (int)
- total_points = 4.5 (float)
→ 「ゲーム1: 4pt」「ゲーム2: 4pt」「合計: 9pt」と表示される。ユーザーから「点数おかしい」とクレーム不可避。
7. 60 秒の同期 API ブロック
response = await asyncio.wait_for(
asyncio.to_thread(model.generate_content, ...),
timeout=60.0
)
画像解析 AI の呼び出しを HTTP リクエスト内で 60 秒間 await。フロントエンドは 60 秒スピナー → ユーザーがリロード → 二重 API コール → AI 課金 2 倍。本来は 202 Accepted + job_id を返して polling すべき。
8. 外部ライブラリの private 属性直叩き
role = await guild._state.http.get_role(guild.id, role_id)
if role:
role = discord.Role(guild=guild, state=guild._state, data=role)
_state は discord.py の private 属性。ライブラリのバージョンアップで即破綻する時限爆弾。pylint の W0212 を disable した本当の理由はこれでした。
9. SDK の誤用で API コスト 33% 増
image_part = {
"mime_type": mime_type,
"data": base64.b64encode(image_data).decode('utf-8')
}
画像解析 SDK は bytes をそのまま受け取れるのに、わざわざ base64 経由で送っていました。base64 はデータサイズが約 33% 増えるので、API 課金も 33% 増、帯域も 33% 増。SDK のドキュメントを読まずに書いた典型。
10. 同じ計算ロジックが 2 箇所にコピペ
「順位表 API」と「ランキング API」で、ポイント計算ロジックが完全コピペで存在。一方を直したらもう一方も直さないとならない。修正漏れバグの温床。
なぜこれらが見つからなかったのか
実装者の検証手段は「ブラウザで動かす」だけだった
ブラウザで触ると気付けるのは ハッピーパスが動くこと だけです。
ブラウザで触っても 絶対に気付かないもの:
- 認可の有無（自分が admin で叩いてるから当たり前に通る）
- 小数切り捨て（自分のテストデータが整数だから）
- pylint disable（コードを見ないと分からない）
-
.DS_Store（git status 見ないと分からない） - 冪等性バグ（再実行しないと分からない）
- N+1 / 性能（負荷かけないと分からない）
- セキュリティ（攻撃しないと分からない）
つまり、実装者の検証手段は致命的問題を 1 個も検出できない手段でした。
CodeRabbit も見逃した
AI コードレビュアーは差分サイズに弱いです。6000 行を超えると、内部的にチャンク分割してサマリしか出さなくなります。
検出できなかった理由:
- ファイル横断の問題（認可は他のエンドポイントとの相対比較で初めて分かる）
- 設定ファイルの diff に深い意味づけをしない（pylint disable がスルーされた）
- バイナリファイル は中身を見ない（画像 18 枚混入を疑問視しない）
- 数行の意味的な誤り が大量の他コードに埋もれる（int 切り捨て）
CodeRabbit が出したのはリリースノートの自動生成だけ。実質的な指摘ゼロでした。
根本原因: 仕様書の 70% が無かった
このプロジェクトには flow.md というドキュメントがありました。フロー図と通知一覧表とポイント計算式が書かれています。一見、仕様書に見えます。
でも、Plan に必要な 70% が抜けていました:
| あった項目 | 無かった項目（= PR で起きた問題と完全対応） |
|---|---|
| ✅ Mermaid フロー図（ハッピーパス） | ❌ アクター × 権限マトリクス → 認可ゼロが起きた |
| ✅ 通知一覧表 | ❌ エラー仕様 → 例外握りつぶしが起きた |
| ✅ ポイント計算ルール | ❌ 冪等性要件 → 採番破綻が起きた |
| ✅ チャンネル種別 | ❌ データ分類 → 画像コミットが起きた |
| ✅ 関連ファイル一覧 | ❌ バリデーション仕様 → 型変換バグが起きた |
| ❌ パフォーマンス予算 → 60秒同期待ちが起きた | |
| ❌ 受け入れ条件 → テストが偽物になった | |
| ❌ Out of Scope → スコープクリープが起きた |
「無かった項目 = 起きた問題」が因果でほぼ完全に対応しています。これは偶然ではなく、AI は spec に書かれたものは作るが、書かれていないものは作らないからです。
予防: 認可マトリクスを必須化する
仕様段階で「全 mutation API に認可が必要」を構造的に強制するなら、Authorization Matrix を埋めるまで実装に進めない ルールにするのが効きます。
テンプレ
## Authorization Matrix（必須）
| 操作 | 未認証 | 一般ユーザー | リソース所有者 | 同組織者 | 管理者 | superuser |
|---|---|---|---|---|---|---|
| GET /resources | ❌ 401 | ✅ | ✅ | ✅ | ✅ | ✅ |
| POST /resources | ❌ 401 | ✅ | - | - | ✅ | ✅ |
| PUT /resources/{id} | ❌ 401 | ❌ 403 | ✅ | ❌ 403 | ✅ | ✅ |
| DELETE /resources/{id} | ❌ 401 | ❌ 403 | ✅ | ❌ 403 | ✅ | ✅ |
凡例:
✅ = 許可
❌ 401 = 未認証拒否
❌ 403 = 認可拒否
- = 該当なし
ルール
- 全セルを必ず埋める。空欄禁止
- 「公開」と書いた項目には 理由 を必ず明記する
- 未認証で叩けるべきもの は ✅ (公開) と書いて理由を下に書く
- リソース所有者と同組織者の挙動が同じなら、その判断根拠を明記する
- 既存の類似 API があれば、その認可パターンと整合させる
これを書く瞬間に、実装者の頭は「あ、未認証で叩かれたら？」「他組織のユーザーが叩いたら？」と動き始めます。マトリクスを埋める時間は 10 分。たった 10 分の儀式で、本番セキュリティホールが防げる。投資対効果が良すぎます。
IDOR / Mass Assignment チェックも併記
## IDOR / Mass Assignment チェック
### POST /resources
- **クライアントから受け取るフィールド**: name, description
- **サーバー側で設定するフィールド**: id, owner_id (= current_user.id), created_at
- **クライアントから受け取ってはいけないフィールド**: id, owner_id, role, organization_id
- **検証方法**: Pydantic schema で許可フィールドを明示し、それ以外を拒否
### PUT /resources/{id}
- **path の {id} と current_user の関係**: owner_id == current_user.id を必ず DB で確認
- **クライアントが id を改竄したら**: 404 or 403
これも書くと「Mass Assignment 怖いな」と実装者が自覚できます。
レビュー前の 5 ステップ（実装者向け）
セルフレビューを構造的にやらせるなら、この 5 ステップを PR テンプレに組み込みます。
Step 1: 差分を頭から最後まで自分の目で読む（AI に頼らない）
git diff main...HEAD
1 hunk ずつ「これは何のためのコードか、自分の言葉で説明できるか」を確認。説明できないコードがあったら AI に書かせたけど自分が理解してない = レビュー前に消す or 理解する。
これだけで .DS_Store と static/*.png は 100% 気付きます。
Step 2: AI に「敵対的レビュー」をさせる
新しい会話で:
あなたはセキュリティとコード品質に厳しいシニアエンジニアです。以下の PR を「絶対に通したくない」前提で、マージブロッカーを探してください。特に以下を重点的に見てください:
- 認可・認証の漏れ
- SQL インジェクション、N+1
- 例外握りつぶし
- リソースリーク・トランザクション境界
- 個人情報の混入
- 設定ファイルの変更（.gitignore, pre-commit, lint disable）
- ロジックバグ（オフバイワン、型変換、丸め、冪等性）
コツ: 新しい会話でやること。同じ会話で「レビューして」と言うと AI は自分が書いたコードを擁護します。文脈リセットが必須。
Step 3: 設定ファイル変更を全部リストアップして説明する
git diff main...HEAD -- '*.yaml' '*.yml' '*.toml' '.env*' '.gitignore' '.pre-commit-config*'
ここに変更がある場合、全部 PR 本文に「なぜ変えたか」を書く。「警告が出たから」は理由になりません。これだけで pylint 20 個 disable は通らなくなります。
Step 4: テストが守れていない領域をリストアップ
「私のテストは X, Y, Z をカバーしています」じゃなくて、
「私のテストでは以下を カバーできていません: 認可 / 並行実行 / エラーパス / N+1 / 性能 / ...」
カバーできていないものを列挙させる方が、抜けに気付きやすいです。
Step 5: 「夜中の 3 時に壊れたら」を想像する
- 壊れたとき、自分は何を見れば原因が分かるか（ログ・メトリクス）
- 壊れたとき、ユーザーは何が見えるか（エラーメッセージ・空画面）
- ロールバックは可能か
これに答えられないなら本番投入してはいけない。
まとめ
- 6000 行の PR は人間も AI レビュアーも全部読めない。差分サイズを 300〜500 行に抑える文化が必要
- ブラウザで動く ≠ 検証完了。ブラウザでは絶対に検出できない問題（認可・冪等性・型変換・性能）が無数にある
- AI は仕様に書かれたものは作るが、書かれていないものは作らない。flow 図しかない仕様で AI を走らせると、認可・エラー処理・冪等性が全部抜ける
- Authorization Matrix を必須化するだけで、セキュリティホールの大半は仕様段階で防げる
- 設定ファイルの diff は赤信号。PR テンプレで変更理由の記載を必須にする
- AI に敵対的レビューさせるには、必ず新しい会話で文脈をリセットする
仕様書に何を書くべきかは、次の記事「フロー図は Plan ではない ― 仕様書に必要な 4 つの軸」で詳しく書きます。
おわりに
この記事の出来事は実話ベースですが、機能名・組織名は全て匿名化しています。同じような事故は AI 駆動開発のあらゆる現場で起きうるし、おそらく今この瞬間にも誰かの PR で起きています。
「ジュニアが悪い」「AI が悪い」「レビュアーが悪い」と犯人を探す前に、仕様段階で構造的に防ぐ仕組みを整えるのが先だと思います。Authorization Matrix を 10 分埋める儀式を導入するだけで、チームの未来の事故が何件か消えるはずです。
次の記事: Threat Modeling を仕様段階で組み込む
この記事で紹介した Authorization Matrix は、実は Threat Modeling の一部 です。SFAD の最新版では、仕様ファイルを 4 つに分割して threat.md という脅威モデリング専用ファイルを必須化しました。
-
functional.md— 機能要件・ハッピーパス -
threat.md— 攻撃者視点・OWASP 対応・認可マトリクス ★本記事のテーマ -
resilience.md— 障害シナリオ・リトライ・タイムアウト -
plan.md— 実装計画
「認可が無い」「IDOR が通る」「Mass Assignment できる」を仕様段階で構造的に潰すには、Three Amigos に 攻撃者視点 を加えた 4 人目のペルソナが必要です。この仕組みは次回の記事で詳しく書きます。
次回: 「Threat Modeling を AI と自動化する ― threat.md 入門」 (5/28 公開予定)