コードレビュー セキュリティ

使用するべき場合

以下の場合にこのスキルを有効化してください：

• セキュリティ脆弱性についてプルリクエストをレビューする場合
• 認証または認可コードの変更を監査する場合
• ユーザー入力、ファイルアップロード、または外部データを処理するコードをレビューする場合
• 新機能に OWASP Top 10 脆弱性がないかを確認する場合
• シークレットがリポジトリにコミットされていないことを検証する場合
• 既知の脆弱性についてデペンデンシーをスキャンする場合
• 機密データを公開する API エンドポイントをレビューする場合

出力： security-review.md に検査結果を記述してください。重大度、ファイル:行番号、説明、および推奨事項を含めます。

このスキルを使用しないでください：

• デプロイメントインフラストラクチャセキュリティの場合（docker-best-practices を使用）
• インシデント対応手順の場合（incident-response を使用）
• セキュリティフォーカスなしの一般的なコード品質レビューの場合（pre-merge-checklist を使用）
• 実装コードを作成する場合（python-backend-expert または react-frontend-expert を使用）

指示

OWASP Top 10 チェックリスト

すべてのプルリクエストを OWASP Top 10（2021 版）に対してレビューしてください。以下の各カテゴリには、Python/FastAPI および React コードベース向けの具体的なチェックが含まれています。

---

A01: アクセス制御の不備

確認すべき内容：

• エンドポイント上の認可チェック漏れ
• 所有権検証なしの直接オブジェクト参照
• ロールベースのフィルタリングなしにデータを公開するエンドポイント
• 新しいルート上の Depends() 認証漏れ

Python/FastAPI チェック：

# 悪い例: 認可チェックなし -- 認証されたユーザー全員が任意ユーザーにアクセス可能
@router.get("/users/{user_id}")
async def get_user(user_id: int, db: Session = Depends(get_db)):
    return await user_repo.get(user_id)

# 良い例: リクエストユーザーがリソースを所有しているか、または管理者であることを確認
@router.get("/users/{user_id}")
async def get_user(
    user_id: int,
    current_user: User = Depends(get_current_user),
    db: Session = Depends(get_db),
):
    if current_user.id != user_id and current_user.role != "admin":
        raise HTTPException(status_code=403, detail="Forbidden")
    return await user_repo.get(user_id)

レビューチェックリスト：

• すべてのルートに認証がある（Depends(get_current_user)）
• リソースアクセスがリクエストユーザーに対して検証されている
• 管理者のみのエンドポイントが role == "admin" をチェックしている
• リスト取得エンドポイントがユーザー所有権でフィルタリングされている（管理者を除く）
• IDOR（不正な直接オブジェクト参照）脆弱性がない

---

A02: 暗号化の失敗

確認すべき内容：

• パスワードが平文またはめゆい弱いハッシュで保存されている
• ログまたはエラーメッセージに機密データが含まれている
• ハードコードされたシークレット、API キー、またはトークン
• 弱い JWT 設定

Python チェック：

# 悪い例: 弱いパスワードハッシュ
import hashlib
password_hash = hashlib.md5(password.encode()).hexdigest()

# 良い例: passlib を経由して bcrypt を使用
from passlib.context import CryptContext
pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
password_hash = pwd_context.hash(password)

# 悪い例: コード内のシークレット
SECRET_KEY = "my-super-secret-key-123"

# 良い例: 環境からシークレットを読み込み
SECRET_KEY = os.environ["SECRET_KEY"]

レビューチェックリスト：

• パスワードが bcrypt でハッシュされている（MD5、SHA1、平文は絶対に使用しない）
• JWT シークレットが環境から読み込まれ、ハードコードされていない
• ログから機密データが除外されている（パスワード、トークン、個人情報）
• すべての外部通信に HTTPS が強制されている
• ソースコード内にシークレットがない（.env.example にはプレースホルダーのみ）

---

A03: インジェクション

確認すべき内容：

• 文字列補間を使用した生 SQL クエリ
• ユーザー入力を含む eval()、exec()、compile()
• shell=True の subprocess 呼び出し
• テンプレートインジェクション

Python チェック：

# 悪い例: 文字列フォーマットを使用した SQL インジェクション
query = f"SELECT * FROM users WHERE email = '{email}'"
db.execute(text(query))

# 良い例: パラメータ化クエリ
db.execute(text("SELECT * FROM users WHERE email = :email"), {"email": email})

# 良い例: SQLAlchemy ORM（常にパラメータ化）
user = db.query(User).filter(User.email == email).first()

# 悪い例: コマンドインジェクション
subprocess.run(f"convert {filename}", shell=True)

# 良い例: 引数をリストとして渡す
subprocess.run(["convert", filename], shell=False)

# 悪い例: ユーザー入力での コード実行
result = eval(user_input)

# 良い例: ユーザー入力を eval しない。安全な解析には ast.literal_eval を使用
result = ast.literal_eval(user_input)  # リテラル構造体のみ対応

レビューチェックリスト：

• 文字列補間を使用した生 SQL がない（ORM またはパラメータ化クエリを使用）
• 外部入力を使用した eval()、exec()、compile() がない
• 動的引数を使用した subprocess.run(..., shell=True) がない
• 信頼できないデータに対する pickle.loads() がない
• すべてのユーザー入力が使用前に Pydantic スキーマで検証されている

---

A04: 安全でない設計

確認すべき内容：

• 認証エンドポイント上のレート制限漏れ
• ログイン失敗後のアカウントロックアウト漏れ
• 公開フォーム上の CAPTCHA 漏れ
• ビジネスロジックの欠陥（例：負の金額、自身の権限昇格）

レビューチェックリスト：

• ログイン、登録、パスワード リセット上のレート制限がある
• 5 回以上の失敗後のアカウントロックアウトまたは指数バックオフがある
• ビジネスロジックが制約を検証している（正の金額、有効な遷移）
• 機密操作で再認証が必須である

---

A05: セキュリティ設定の不備

確認すべき内容：

• プロダクション内のデバッグモード有効化
• ワイルドカード * オリジンで設定された CORS
• デフォルト認証情報または管理者アカウント
• スタックトレースを公開する詳細なエラーメッセージ

Python/FastAPI チェック：

# 悪い例: 広くオープンな CORS
app.add_middleware(CORSMiddleware, allow_origins=["*"])

# 良い例: 明示的に許可されたオリジン
app.add_middleware(
    CORSMiddleware,
    allow_origins=["https://app.example.com"],
    allow_methods=["GET", "POST", "PUT", "DELETE"],
    allow_headers=["Authorization", "Content-Type"],
)

# 悪い例: プロダクション内のデバッグモード
app = FastAPI(debug=True)

# 良い例: 開発時のみデバッグモード
app = FastAPI(debug=settings.DEBUG)  # プロダクションでは DEBUG=False

レビューチェックリスト：

• CORS オリジンが明示的である（プロダクション内にワイルドカードなし）
• デバッグモードがプロダクション設定で無効化されている
• エラー応答がスタックトレースまたは内部詳細を公開していない
• デフォルト管理者認証情報が変更または削除されている
• セキュリティヘッダーが設定されている（X-Content-Type-Options、X-Frame-Options など）

---

A06: 脆弱で廃止されたコンポーネント

レビューチェックリスト：

• Python デペンデンシーに既知の CVE がない（pip-audit または safety check）
• npm デペンデンシーに既知の CVE がない（npm audit）
• デペンデンシーがロックファイル内の特定バージョンにピン留めされている
• 廃止されたパッケージがまだ使用されていない

---

A07: 識別・認証の失敗

確認すべき内容：

• 弱いパスワードポリシー
• 有効期限のないセッショントークン
• 管理者アクションのための多要素認証漏れ
• 有効期限のない JWT トークン

Python チェック：

# 悪い例: 有効期限なしの JWT
token = jwt.encode({"sub": user_id}, SECRET_KEY, algorithm="HS256")

# 良い例: 有効期限付きの JWT
token = jwt.encode(
    {"sub": user_id, "exp": datetime.utcnow() + timedelta(minutes=30)},
    SECRET_KEY,
    algorithm="HS256",
)

レビューチェックリスト：

• JWT トークンに有効期限がある（exp クレーム）
• リフレッシュトークンが安全に保存され、無効化可能である
• パスワードポリシーが最小長（12 文字以上）と複雑性を強制している
• パスワード変更またはログアウト時にセッションが無効化されている
• ログインエラーメッセージによるユーザー列挙がない

---

A08: ソフトウェアおよびデータ整合性の失敗

レビューチェックリスト：

• CI/CD パイプラインがアーティファクト整合性を検証している
• 署名されていない、または検証されていないパッケージがない
• 信頼できないデータの逆シリアル化が安全な方法を使用している（pickle.loads() なし）
• データベースマイグレーションが実行前にレビューされている

---

A09: セキュリティログと監視の失敗

レビューチェックリスト：

• 認証イベントがログされている（ログイン、ログアウト、失敗した試行）
• 認可失敗がコンテキストと共にログされている
• 機密データがログに含まれていない（パスワード、トークン、個人情報）
• ログエントリにタイムスタンプ、ユーザー ID、IP アドレス、アクションが含まれている
• 疑わしいパターン（ブルートフォース、異常なアクセス）に対するアラートが設定されている

---

A10: サーバー側リクエスト偽造（SSRF）

確認すべき内容：

• ユーザー指定の URL がサーバー側リクエストで使用されている
• 任意の URL を受け付けるリダイレクトエンドポイント

Python チェック：

# 悪い例: ユーザー入力から任意の URL をフェッチ
url = request.query_params["url"]
response = httpx.get(url)  # SSRF: 内部サービスにアクセス可能

# 良い例: URL を許可リストに対して検証
ALLOWED_HOSTS = {"api.example.com", "cdn.example.com"}
parsed = urlparse(url)
if parsed.hostname not in ALLOWED_HOSTS:
    raise HTTPException(400, "URL not allowed")
response = httpx.get(url)

レビューチェックリスト：

• ユーザーコントロール下の URL への検証なしのサーバー側リクエストがない
• 外部統合に URL 許可リストが使用されている
• 内部サービス URL がエラーメッセージで公開されていない

---

Python 固有のセキュリティチェック

OWASP を超えて、これらのパターンについて Python コードをレビューしてください：

パターン	リスク	修正
eval(user_input)	リモートコード実行	削除または ast.literal_eval を使用
pickle.loads(data)	任意のコード実行	JSON または msgpack を使用
subprocess.run(cmd, shell=True)	コマンドインジェクション	引数をリストで渡す、shell=False
yaml.load(data)	コード実行	yaml.safe_load(data) を使用
os.system(cmd)	コマンドインジェクション	subprocess.run([...]) を使用
生 SQL 文字列	SQL インジェクション	ORM またはパラメータ化クエリを使用
hashlib.md5(password)	弱いハッシュ	passlib を経由して bcrypt を使用
jwt.decode(token, options={"verify_signature": False})	認証バイパス	常に署名を検証
open(user_path)	パストラバーサル	パスを検証、pathlib.resolve() を使用
tempfile.mktemp()	競合状態	tempfile.mkstemp() を使用

React 固有のセキュリティチェック

パターン	リスク	修正
dangerouslySetInnerHTML	XSS	テキスト内容を使用するか、DOMPurify で消毒
href 内の javascript:	XSS	URL を検証、https: のみを許可
window.location = userInput	オープンリダイレクト	許可リストに対して検証
localStorage にトークンを保存	XSS によるトークン盗難	httpOnly クッキーを使用
データからのインラインイベントハンドラ	XSS	React イベントハンドラを使用
eval() または Function()	コード実行	完全に削除
ユーザー HTML をレンダリング	XSS	サニテーション ライブラリを使用

React コードレビュー：

// 悪い例: dangerouslySetInnerHTML 経由の XSS
<div dangerouslySetInnerHTML={{ __html: userBio }} />

// 良い例: 最初にサニタイズするか、テキスト内容を使用
import DOMPurify from "dompurify";
<div dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(userBio) }} />

// より良い例: HTML が不要な場合はテキスト内容を使用
<p>{userBio}</p>

// 悪い例: javascript: URL
<a href={userLink}>Click</a>  // userLink は "javascript:alert(1)" の可能性あり

// 良い例: プロトコルを検証
const safeHref = /^https?:\/\//.test(userLink) ? userLink : "#";
<a href={safeHref}>Click</a>

重大度分類

優先順位付けのため、各検査結果を重大度で分類してください：

重大度	説明	例	SLA
Critical	リモートから悪用可能、認証不要、データ漏洩	SQL インジェクション、RCE、認証バイパス	マージをブロック、即座に修正
High	認証ありで悪用可能、権限昇格	IDOR、アクセス制御不備、保存型 XSS	マージをブロック、リリース前に修正
Medium	悪用に特定の条件が必要	CSRF、反射型 XSS、オープンリダイレクト	スプリント内に修正
Low	多層防御、情報提供	ヘッダ漏れ、詳細なエラー	都合がつき次第修正
Info	ベストプラクティス推奨	デペンデンシー更新、コードスタイル	バックログで追跡

検査結果レポートフォーマット

セキュリティ検査結果をレポートする際は、一貫性のためこのフォーマットを使用してください：

## セキュリティ検査結果: [タイトル]

**重大度：** Critical | High | Medium | Low | Info
**カテゴリ：** OWASP A01-A10 またはカスタムカテゴリ
**ファイル：** path/to/file.py:42
**CWE：** CWE-89（該当する場合）

### 説明
脆弱性とその影響の簡潔な説明。

### 脆弱なコード
```python
# 問題のあるコード
vulnerable_function(user_input)

推奨される修正

# 安全な代替案
safe_function(sanitize(user_input))

影響

この脆弱性を悪用することで攻撃者が達成できることは何か。

参考資料

• 関連する OWASP ページへのリンク
• 関連する CWE エントリへのリンク

### 自動スキャン

`scripts/security-scan.py` を使用して、Python コード内の一般的な脆弱性パターンについて AST ベースのスキャンを実行してください。スクリプトは以下をスキャンします：
- `eval()` / `exec()` / `compile()` 呼び出し
- `shell=True` の `subprocess`
- 潜在的に信頼できないデータに対する `pickle.loads()`
- 生 SQL 文字列構築
- `Loader=SafeLoader` なしの `yaml.load()`
- ハードコードされたシークレットパターン（API キー、パスワード）
- 弱いハッシュ関数（パスワード用の MD5、SHA1）

実行：`python scripts/security-scan.py --path ./app --output-dir ./security-results`

**デペンデンシースキャン（別途実行）：**
```bash
# Python デペンデンシー
pip-audit --requirement requirements.txt --output json > dep-audit.json

# npm デペンデンシー
npm audit --json > npm-audit.json

例

コードレビューコメント例（Critical）

セキュリティ: SQL インジェクション（Critical、OWASP A03）

ファイル：app/repositories/user_repository.py:47

query = f"SELECT * FROM users WHERE name LIKE '%{search_term}%'"

このコードは文字列補間で生 SQL クエリを構築しており、SQL インジェクションが可能です。 攻撃者は '; DROP TABLE users; -- を入力してデータを破棄できます。

修正： SQLAlchemy ORM フィルタリングを使用してください：

users = db.query(User).filter(User.name.ilike(f"%{search_term}%")).all()

コードレビューコメント例（Medium）

セキュリティ: レート制限漏れ（Medium、OWASP A04）

ファイル：app/routes/auth.py:12

/auth/login エンドポイントにレート制限がありません。攻撃者は無制限の速度でブルートフォース パスワード攻撃を実行できます。

修正： レート制限ミドルウェアを追加してください：

from slowapi import Limiter
limiter = Limiter(key_func=get_remote_address)

@router.post("/login")
@limiter.limit("5/minute")
async def login(request: Request, ...):

出力ファイル

セキュリティ検査結果を security-review.md に記述してください：

# セキュリティレビュー: [機能/PR 名]

## サマリー
- Critical: 0 | High: 1 | Medium: 2 | Low: 1

## 検査結果

### [CRITICAL] ユーザー検索における SQL インジェクション
- **ファイル：** app/routes/users.py:45
- **OWASP：** A03 インジェクション
- **説明：** 文字列補間を使用した生 SQL
- **推奨事項：** SQLAlchemy ORM フィルタリングを使用

### [HIGH] 認可チェック漏れ
...

## 合格したチェック
- ハードコードされたシークレットが見つからない
- デペンデンシーが最新