Chapter 30: AI を使ったコードレビュー

以下のコードを**セキュリティの観点**でレビューしてほしい。
特に以下の点を確認してほしい:
- SQL インジェクションの可能性
- 認証・認可のバイパス
- センシティブデータの適切な取り扱い
- ユーザー入力のバリデーション

問題がある場合は、深刻度 (Critical / High / Medium / Low) もつけてほしい。

[コードを貼り付ける]

# PR の差分を取得
git diff main...feature/add-payment-system > pr-diff.txt

以下は PR の diff です。
総合的なコードレビューをしてほしい。

レビュー観点:
1. バグやロジックの誤り
2. セキュリティ上の懸念点
3. パフォーマンスの問題
4. コードの可読性・保守性
5. テストの十分さ

問題点は重要度順に並べてほしい。
修正が必要なものと、提案レベルのものを区別してほしい。

--- diff ---
[diff の内容を貼り付ける]

# feature ブランチをチェックアウトした状態で
claude

> この PR をレビューしてほしい。
> main ブランチとの差分を確認して、以下の観点でレビューしてほしい:
> - バグ・ロジックの誤り
> - セキュリティ
> - パフォーマンス
> - テストの充実度
>
> 問題があれば、具体的な修正案も提示してほしい。

以下のコードを OWASP Top 10 の観点でレビューしてほしい。
各カテゴリについて「問題なし」か「問題あり (具体的な箇所と修正方法)」を教えてほしい。

チェック項目:
- A01: アクセス制御の欠陥
- A02: 暗号化の失敗
- A03: インジェクション (SQL/XSS/コマンドインジェクション)
- A04: 安全でない設計
- A05: セキュリティの設定ミス
- A07: 認識・認証の失敗
- A09: セキュリティログとモニタリングの失敗

[コードを貼り付ける]

以下の API エンドポイントの認証・認可ロジックをレビューしてほしい。

特に以下を確認してほしい:
1. ログイン済みユーザーのみがアクセスできるか
2. 自分のリソースしか操作できないか (横断的アクセス制御)
3. 権限昇格の可能性がないか
4. JWTの検証が適切か

[コードを貼り付ける]

// 問題のあるコード (SQL インジェクションの脆弱性)
async function getUserByName(name: string) {
  const query = `SELECT * FROM users WHERE name = '${name}'`;
  return db.query(query);
}

// AI が提案する修正案
async function getUserByName(name: string) {
  // パラメータ化クエリを使う
  return db.query('SELECT * FROM users WHERE name = $1', [name]);
}

以下のコードで N+1 クエリが発生していないか確認してほしい。
発生している場合は、どの箇所で何クエリ発行されるかを説明し、
改善方法 (JOIN や include を使う方法) を提示してほしい。

[コードを貼り付ける]

// 問題: N+1 クエリが発生
async function getOrdersWithProducts() {
  const orders = await Order.findAll();           // 1クエリ
  for (const order of orders) {
    order.products = await Product.findByOrderId(order.id); // N クエリ
  }
  return orders;
}

// 改善: JOIN または include を使う
async function getOrdersWithProducts() {
  return Order.findAll({
    include: [{ model: Product }]  // 1クエリ (または 2クエリ)
  });
}

以下の関数の時間計算量を分析してほしい。
データ量が増えたときにパフォーマンスが問題になる可能性があるか教えてほしい。
改善できる場合は改善後のコードも提示してほしい。

[コードを貼り付ける]

以下のコードに問題を発見した。
この問題を指摘するレビューコメントを日本語で書いてほしい。
問題を明確に説明し、改善案も含めること。
批判的ではなく、建設的なトーンで書いてほしい。

【発見した問題】
getUserById 関数でユーザーが見つからない場合に null を返しているが、
呼び出し側で null チェックをしていないため実行時エラーが発生する可能性がある。

【問題のコード】
[コードを貼り付ける]

ご実装ありがとうございます。一点確認させてください。

`getUserById` で対象ユーザーが存在しない場合に `null` を返していますが、
呼び出し側の `updateUserProfile` ではその戻り値の null チェックがないため、
存在しないユーザー ID を指定した際に `TypeError: Cannot read properties of null` が
発生する可能性があります。

以下のいずれかの対応を検討していただけますでしょうか:

案 A: getUserById でユーザーが見つからない場合に例外を投げる
- if (!user) throw new UserNotFoundError(...)

案 B: 呼び出し側で null チェックを追加する
- const user = await getUserById(userId);
- if (!user) return res.status(404).json(...)

個人的には案 A のほうが、呼び出し側での書き忘れを防げるため好みです。
いかがでしょうか？

以下のレビューコメント候補がある。
それぞれを「必ず直すべき」「できれば直してほしい」「提案のみ」に分類してほしい。
分類の根拠も教えてほしい。

[レビューコメントの一覧を貼り付ける]

我々のプロジェクトのコードレビューガイドラインを作成してほしい。

プロジェクトの特性:
- Node.js + TypeScript の Web API
- チームメンバー5人
- 週2回リリース
- セキュリティが重要 (金融系アプリ)

以下のセクションを含めること:
1. 必ずチェックする項目 (Blocker)
2. 推奨する改善点 (Suggestion)
3. スタイルガイド
4. セキュリティチェックリスト