Reviewing Backend API

reviewing-backend-api

バックエンドAPIのコード変更をレビューします。API設計、セキュリティ、データベース操作、エラーハンドリング、パフォーマンスに焦点を当てます。

いつこのスキルを使うか

以下の場合に使用:

• バックエンド/API変更を含むプルリクエストのレビュー
• 新しいAPIエンドポイントまたは修正の分析
• データベースクエリとマイグレーションの評価
• セキュリティと認証実装の確認

用語定義

レビューで使用する重要な概念の定義:

RESTful API設計原則

• リソース志向: URLはリソース（名詞）を表現、動詞はHTTPメソッドで表現

  • OK: GET /api/v1/users/123
  • NG: GET /api/v1/getUser?id=123

• HTTPメソッドの使い分け:

  • GET: リソース取得（副作用なし、冪等）
  • POST: リソース作成（非冪等）
  • PUT: リソース全体更新（冪等）
  • PATCH: リソース部分更新
  • DELETE: リソース削除（冪等）

ステータスコードの使い分け

• 2xx成功: 200 (成功), 201 (作成), 204 (成功・レスポンスなし)
• 4xxクライアントエラー: 400 (不正リクエスト), 401 (未認証), 403 (認可エラー), 404 (未存在)
• 5xxサーバーエラー: 500 (内部エラー), 503 (一時的利用不可)

RLS (Row Level Security)

データベース行レベルでのアクセス制御ポリシー

• 判断基準: ユーザーが自分のデータのみアクセス可能かどうかをSQLレベルで制御
• 例: user_id = auth.uid() でログインユーザーの行のみ参照可能

N+1クエリ問題

ループ内でクエリを発行し、パフォーマンスが劣化する問題

• NG:

  for (const user of users) {
    const posts = await db.query('SELECT * FROM posts WHERE user_id = $1', [user.id]);
  }
  

• OK:

  const posts = await db.query('SELECT * FROM posts WHERE user_id = ANY($1)', [userIds]);
  

レビュープロセス

このチェックリストに従って体系的にレビュー:

1. API設計

• RESTful原則に従っている（上記定義参照）

  • 判断基準: URLがリソース名（名詞）で構成されている
  • 判断基準: 操作はHTTPメソッドで表現されている

• エンドポイント命名が明確で一貫している

  • 判断基準: 複数形を使用（/users, /posts）
  • 判断基準: ケバブケースまたはスネークケース統一
  • 判断基準: 階層構造が論理的（/users/123/posts）

• HTTPメソッドが正しく使用されている（上記定義参照）

  • 判断基準: GET は副作用なし、冪等性あり
  • 判断基準: POST は作成、PUT/PATCH は更新、DELETE は削除

• ステータスコードが適切（上記定義参照）

  • 判断基準: 成功は 2xx、クライアントエラーは 4xx、サーバーエラーは 5xx
  • 判断基準: リソース作成時は 201、削除成功は 204

• リクエスト/レスポンス形式が一貫している

  • 判断基準: すべてのエンドポイントでJSON形式統一
  • 判断基準: エラーレスポンス形式が統一（例: { error: { message, code } }）

• APIバージョニング戦略がある

  • 判断基準: URLパスに /v1/, /v2/ を含む
  • または: Accept ヘッダーでバージョン指定

• リストエンドポイントにページネーション実装

  • 判断基準: limit, offset または page, per_page パラメータ
  • 判断基準: レスポンスに総件数（total）を含む

2. セキュリティ

• 認証が正しく実装されている

  • 判断基準: Authorizationヘッダーで JWT または Bearer トークン検証
  • 判断基準: トークンの有効期限チェック
  • 判断基準: ログアウト時のトークン無効化

• 認可チェックが存在する

  • 判断基準: ユーザーが他人のデータにアクセスできない
  • 判断基準: ロールベースまたはリソースベースの権限チェック
  • 判断基準: RLSポリシーまたはアプリケーションレベルで実装

• RLSポリシーが正確（上記定義参照）

  • 判断基準: user_id = auth.uid() 等で所有者のみアクセス可能
  • 判断基準: INSERT/UPDATE/DELETE/SELECT すべてにポリシー設定
  • 判断基準: テストでポリシー違反が403/404を返すことを確認

• すべてのユーザー入力にバリデーション

  • 判断基準: 型チェック（文字列、数値、列挙型）
  • 判断基準: 範囲チェック（最小/最大長、最小/最大値）
  • 判断基準: フォーマットチェック（メール、URL、日付）
  • 判断基準: 必須フィールドチェック

• SQLインジェクション対策済み

  • OK: パラメータ化クエリ query('SELECT * FROM users WHERE id = $1', [id])
  • NG: 文字列連結 query('SELECT * FROM users WHERE id = ' + id)
  • 判断基準: ORMまたはクエリビルダー使用

• エラーメッセージに機密データがない

  • NG: スタックトレース、SQL文、内部パス露出
  • OK: 一般的なエラーメッセージ（"無効なリクエスト"）
  • 判断基準: 詳細はサーバーログ、クライアントには概要のみ

• CORSが適切に設定されている

  • 判断基準: 許可するオリジンが明示的（* は本番環境で使用しない）
  • 判断基準: 認証情報を含むリクエストは credentials: true

• レート制限を考慮

  • 判断基準: ログイン、パスワードリセット等の認証エンドポイントに実装
  • 判断基準: IP単位または ユーザー単位で制限（例: 10回/分）

• シークレットがハードコードされていない

  • 判断基準: API キー、DB パスワード等は環境変数から読み込み
  • 判断基準: .env ファイルが .gitignore に含まれる
  • 判断基準: コード内に process.env.SECRET_KEY 等の参照のみ

3. データベース操作

• N+1クエリ問題がない（上記定義参照）

  • 判断基準: ループ内でクエリ発行していない
  • 判断基準: JOIN または IN句 で一括取得

• 適切なインデックスが存在

  • 判断基準: WHERE句で頻繁に使用される列にインデックス
  • 判断基準: 外部キー列にインデックス
  • 判断基準: ユニーク制約が必要な列にユニークインデックス

• トランザクション使用が適切

  • 判断基準: 複数テーブルへの書き込みはトランザクション内
  • 判断基準: 整合性が必要な操作はトランザクション内
  • 判断基準: 読み取り専用操作にはトランザクション不要

• データベース制約が適用されている

  • 判断基準: 外部キー制約でデータ整合性を保証
  • 判断基準: ユニーク制約で重複防止
  • 判断基準: NOT NULL制約で必須フィールド保証

• マイグレーションが可逆的

  • 判断基準: up と down の両方が定義されている
  • 判断基準: down実行でup前の状態に戻る

• コネクションプーリング設定

  • 判断基準: 最大接続数が適切（CPU数 × 2 + ディスク数 が目安）
  • 判断基準: アイドルタイムアウト設定

• クエリパフォーマンス分析

  • 判断基準: EXPLAIN ANALYZE で実行計画確認
  • 判断基準: 大量データ（10万件以上）で性能測定

4. エラーハンドリング

• 十分なコンテキストでエラーをログ記録
• エラーレスポンスの構造が一貫している
• ユーザー向けエラーメッセージが有用
• クライアントにスタックトレースが公開されない
• 適切なレベルでエラーをキャッチ
• 非同期エラーが適切に処理されている

5. パフォーマンス

• 大きな結果セットにページネーション
• 有益な場所でキャッシング戦略を実装
• 不要なデータベースクエリを回避
• クエリの最適化（必要なフィールドのみSELECT）
• ループの代わりに一括操作
• レスポンスペイロードサイズが妥当

6. データバリデーション & 型

• 入力バリデーションが包括的
• 型定義がデータベーススキーマと一致
• Enum値がバリデートされている
• 日時処理がタイムゾーンを考慮
• ファイルアップロードサイズ制限が適用されている
• アップロードのContent-Typeバリデーション

7. テスト

• ビジネスロジックの単体テスト
• APIエンドポイントの統合テスト
• 成功とエラーケースをカバー
• 認証/認可がテストされている
• データベーストランザクションのテスト
• 外部依存関係を適切にモック

8. コード品質

• 明確な関数名と変数名
• 一貫したエラーハンドリングパターン
• コメントアウトされたコードがない
• コメントが複雑なロジックを説明
• Lintチェックが通過
• 型安全性が維持されている

出力形式

以下の構造でレビューフィードバックを提供:

## 🎯 概要
[全体的な評価: 承認、変更要求、または議論が必要]

## ✅ 良い点
- [よく設計されたAPI]
- [優れたセキュリティプラクティス]

## 🚨 セキュリティ問題
- [認証/認可の問題]
- [インジェクション脆弱性]
- [データ露出リスク]

## ⚠️ その他の問題

### 重大
- [データ破損リスク、破壊的変更]

### 重要
- [パフォーマンス問題、バリデーション不足]

### 軽微
- [コードスタイル、最適化の機会]

## 💡 推奨事項
- [具体的な改善提案]
- [採用すべきベストプラクティス]

## 📝 詳細コメント
[エンドポイント毎またはファイル毎のフィードバック]

使用するツール

1. Grep: セキュリティパターン検索（ハードコードされたシークレット、SQLクエリ）
2. Read: APIルートハンドラーとデータベースクエリを調査
3. Bash: git diff mainを実行して変更を確認
4. Glob: マイグレーションファイル、テストファイルを検索

フラグすべきセキュリティアンチパターン

• 認証チェックの欠如
• 不十分な認可（ユーザーが他人のデータにアクセス可能）
• パラメータ化クエリの代わりに生SQLの連結
• 平文で保存されたパスワード
• 有効期限のないJWTトークン
• 認証エンドポイントでのレート制限の欠如
• 過度に許可的なRLSポリシー
• ログ内の機密データ

フラグすべきパフォーマンスアンチパターン

• N+1クエリ（ループ内のデータベース呼び出し）
• フィールド制限なしのSELECT *
• 頻繁にクエリされる列にインデックスがない
• 大規模データセットにページネーションがない
• 非同期であるべき同期操作
• 不要なデータベース接続

データベースレビューポイント

• マイグレーションファイルの命名規則に従っている
• アップとダウンマイグレーションの両方が存在
• 外部キー制約が定義されている
• クエリパターン用のインデックスが作成されている
• データに適した列タイプ
• 必要に応じてデフォルト値が設定されている

コンテキスト質問

レビュー開始前に必要に応じて明確化:

• 予想されるトラフィック/負荷は？
• 特定のセキュリティ要件はあるか？
• データ保持ポリシーは？
• パフォーマンスSLAはあるか？
• 使用しているデータベースシステムは？