From aeefbeb25a5e5e289f86959e3f2d937791fcb23c Mon Sep 17 00:00:00 2001 From: Wada Yusuke Date: Sat, 30 May 2026 22:54:53 +0900 Subject: [PATCH 1/3] infra refacter --- .claude/skills/SEC_apply/SKILL.md | 218 +++++++++++++++ .claude/skills/SEC_review/SKILL.md | 257 ++++++++++++++++++ .github/workflows/ci.yml | 60 ++-- .github/workflows/opentofu-ci.yml | 20 +- backend/app/routers/internal.py | 44 ++- .../intelligence/github/api_client.py | 37 +++ backend/app/services/tasks/cloud_tasks.py | 7 +- backend/requirements.txt | 6 +- .../security/test_admin_authorization.py | 75 ++++- .../tests/security/test_mass_assignment.py | 120 ++++++++ backend/tests/security/test_rate_limit.py | 53 ++++ backend/tests/security/test_ssrf_github.py | 76 ++++++ docs/api.md | 4 +- frontend/package-lock.json | 17 ++ frontend/package.json | 1 + .../forms/MarkdownTextarea.test.tsx | 61 +++++ .../src/components/forms/MarkdownTextarea.tsx | 6 +- 17 files changed, 1012 insertions(+), 50 deletions(-) create mode 100644 .claude/skills/SEC_apply/SKILL.md create mode 100644 .claude/skills/SEC_review/SKILL.md create mode 100644 backend/tests/security/test_mass_assignment.py create mode 100644 backend/tests/security/test_rate_limit.py create mode 100644 backend/tests/security/test_ssrf_github.py create mode 100644 frontend/src/components/forms/MarkdownTextarea.test.tsx diff --git a/.claude/skills/SEC_apply/SKILL.md b/.claude/skills/SEC_apply/SKILL.md new file mode 100644 index 00000000..738addb6 --- /dev/null +++ b/.claude/skills/SEC_apply/SKILL.md @@ -0,0 +1,218 @@ +--- +name: SEC_apply +description: Use when applying the security findings produced by the SEC_review skill. Reads `report/SEC_report_.md` (latest by default, or a path passed as argument), confirms scope with the user, implements the fixes against backend / frontend / infra (including design-level authorization/SSRF/OAuth fixes and dependency supply-chain hardening), writes exploit-style regression unit tests for the findings, runs lint/test/audit, then writes a result report to `report/SEC_pr_.md`. Trigger on requests such as "SEC_apply 実行", "セキュリティ指摘を修正", "SEC レポートを適用", "脆弱性を直して", "exploit テストを追加". +--- + +# Security Fix Apply + +`SEC_review` skill が生成したレビュー (`report/SEC_report_*.md`) を入力にして、実際のセキュリティ修正・検証・PR 用レポート作成までを行う skill。 + +## 先に読む + +- `.claude/CLAUDE.md` +- `.claude/skills/SEC_review/SKILL.md`（出力フォーマットの参照元） +- `.claude/rules/security.md` +- `.claude/rules/backend/auth-security.md` +- 影響領域に応じて `.claude/rules/{backend,frontend,infra}/` の該当ルール +- `backend/app/core/env_keys.py`（env 参照の正本） +- `.claude/rules/frontend/messages.md`（frontend メッセージの正本） + +## 入力（対象レポートの選択） + +引数で明示パスが渡されていればそれを使う。なければ `report/SEC_report_*.md` の中で最新（mtime 降順）を採用する。 + +```bash +# 明示 +/SEC_apply report/SEC_report_20260530_1042.md + +# 省略時 = 最新を自動採用 +/SEC_apply +``` + +最初に必ずユーザーへ「採用したレポートのパス」を 1 行返す。 + +```text +採用レポート: report/SEC_report_20260530_1042.md +``` + +`report/` 配下に `SEC_report_*.md` が一つも無い場合はここで停止し、`/SEC_review` の実行を促す。 + +## 実装スコープの確認（必須） + +レポートを読み込んだら、Findings を Critical / High / Medium / Low の件数で集計してユーザーへ提示し、どこまで修正するかを **毎回必ず聞く**。勝手に全件着手してはいけない。 + +提示例: + +```text +採用レポート: report/SEC_report_20260530_1042.md +Findings: Critical 1 / High 3 / Medium 5 / Low 2 +Design-level: 2（IDOR 1 / SSRF 1） +Dependency & Supply Chain: High CVE 2 / SHA 未固定 1 +Secrets Scan: 混入 1 +Missing Exploit Tests: 4 + +どこまで適用しますか？ + 1) Critical のみ + 2) Critical + High + 3) 設計観点(IDOR/SSRF/認可)も含める + 4) 依存(CVE 更新 / 固定 / SHA ピン留め)も含める + 5) Secrets 対応(rotate/除去)も含める + 6) Missing Exploit Tests(攻撃者視点テスト追加)も含める + 7) 全部 + 8) 個別に選ぶ（番号で指定） +``` + +`AskUserQuestion` で選ばせるのが望ましい。「個別」が選ばれた場合は、Findings の見出しを番号付きで列挙して再選択させる。 + +## 実装の進め方 + +1. **タスク化**: 採用した各 Finding を `TaskCreate` で 1 タスクずつ切り、`in_progress` → `completed` を必ず更新する。 +2. **小さく分ける**: 「秘密情報除去」「env_keys 化」「認証ガード追加」「XSS 修正」「依存更新」を別タスクにする。1 タスクあたりの diff は読める範囲に保つ。 +3. **DevForge コーディング規約を厳守**: + - コメント・docstring は日本語、HTTPException の `detail` は日本語 + - frontend のユーザー向け文言はリテラル禁止 → `frontend/src/constants/messages.ts` 経由（`.claude/rules/frontend/messages.md`） + - 環境変数は `os.getenv(env_keys.XXX)` 経由。リテラル `os.getenv("XXX")` 禁止。新規 env 追加時は env_keys.py の 4 箇所同期手順を踏む + - `except SomeException: pass` 禁止。最低限 `logger.warning` + - 黙って return 禁止。失敗パスは適切な例外を `raise` + - 統合テストで DB をモックしない（実 DB セッション） +4. **秘密情報の扱い（重要）**: + - すでに git にコミット済みの秘密情報は、ファイル削除だけでは履歴に残る。**該当クレデンシャルの rotate（再発行）が必要**であることを警告し、git 履歴からの除去（filter-repo 等）は破壊的なのでユーザー判断を仰ぐ。勝手に履歴を書き換えない。 + - 今後の混入防止として `.gitignore` への追加は行ってよい。 +5. **認証 / IAM 変更**: + - 認証ガード追加・IAM ロール変更は影響が大きい。変更理由をコメントで残し、最小権限を守る。 +6. **設計観点の修正（IDOR / SSRF / OAuth / マスアサインメント）**: + - **IDOR**: repository / service のクエリに `user_id` 境界フィルタを追加し、所有者不一致を 403/404 にする。エンドポイント単独でなく repository 層まで直す。 + - **SSRF**: 外部 fetch（GitHub クライアント / collector）の宛先を許可スキーム・ホストで検証してから叩く。 + - **OAuth / state**: `state` の backend Cookie 検証、`redirect_uri` の許可リスト固定を確認・補強。 + - **マスアサインメント**: 入力 Pydantic スキーマから `user_id` / 権限系フィールドを除外し、サーバ側で固定。 + - 設計変更は必ず後述の exploit テストとセットで固定する。 +7. **依存 / サプライチェーン対応**: + - CVE 解消のバージョン更新は `--force` で無視せず該当パッケージを更新（`.claude/rules/security.md` の依存関係節）。更新後に再度 audit を回して解消を確認。 + - 緩いバージョン指定は pin（`requirements.txt` の `==` / lockfile commit）に寄せる。 + - GitHub Actions の `uses:` がタグ参照なら commit SHA 固定へ（直近 commit「GitHub Actions のサプライチェーン保護」の方針を踏襲）。 + - 不審な新規依存（typosquatting / dependency confusion / postinstall）は導入を止め、代替を提案。 +8. **Missing Exploit Tests（攻撃者視点の回帰テスト）**: + - レポートの「Missing Exploit Tests」と、今回直した設計穴を **攻撃が失敗することを assert** するテストで固定する。 + - `.claude/rules/backend/test.md` 準拠: DB はモックせず実 SQLite セッション、外部 API（GitHub / LLM / Cloud Tasks / Redis）はモック。失敗パスは `pytest.raises` か HTTP status assert で必ず明示検証（silent return を許容しない）。 + - テスト名に守る仕様を書く（例: `test_他人のresumeはget_404` / `test_internal_secret欠落は拒否` / `test_state不一致のoauth_callbackは拒否`）。 + - 配置先は既存方針: 認可/エンドポイントは `tests/test_.py`、認証は `tests/test_auth.py` / `tests/test_oauth_flow.py`、タスクは `tests/test_worker_*.py`。 + +## 検証（必須） + +修正範囲に応じて検証を回す。スキップ禁止。 + +```bash +# backend を触ったら +make lint-backend +make test-backend + +# frontend を触ったら +make lint-frontend +make lint-frontend-messages +make test-frontend + +# infra を触ったら +make infra-fmt-check +make infra-validate + +# まとめて +make ci +``` + +- 認証 / ナビゲーション / 新規ルート / サイドバー / API フロー影響がある場合は E2E も回す: + `nix develop --command bash -c "cd frontend && npm run test:e2e"` +- **追加した exploit テストが「修正前は落ち、修正後に通る」ことを確認する**（回帰防止として機能しているかの確認。可能なら修正を一時 revert して赤を見る、難しければレビューで論理を担保）。 +- 依存更新をした場合は再度 audit を回し、CVE 解消を確認する。SHA 固定・pin 化をした場合は CI の audit ステップ（`.github/workflows/ci.yml`）が通る前提を崩していないか確認。 +- sandbox が `~/.cache/nix/fetcher-locks/*.lock` で落ちる場合は `dangerouslyDisableSandbox: true` で再実行する（CLAUDE.md の既知の例外）。 +- lint / test / audit に失敗したら、原因を直してから次へ。失敗を残したまま PR レポートを書かない。`--no-verify` で hook を skip しない。 + +## 成果物の出力先（必須） + +実装と検証が完了したら、PR 用のサマリを必ずファイルへ書く。assistant メッセージに本文を貼らない。 + +- 保存先: `report/SEC_pr_.md` + - 例: `report/SEC_pr_20260530_1530.md` + - `report/` が無ければ作成する (`mkdir -p report`) + - タイムスタンプは **PR レポート書き出し時刻**（ローカル）。`date +%Y%m%d_%H%M` +- 既存 `SEC_pr_*.md` は履歴として残す。上書き禁止 +- 採用元レポートのパスを冒頭に明記する + +### ターミナルへの出力ルール + +- 詳細はファイルにだけ書く +- ターミナルへ返すのは以下のみ: + 1. 採用レポートのパス + 2. PR レポートのパス（`report/SEC_pr_YYYYMMDD_HHMM.md`） + 3. `Summary` セクションの 3-5 行サマリ + 4. 検証結果（lint / test / audit の pass / fail） + 5. 残タスク・要ユーザー判断（rotate 必要など 1-2 行） + +## PR レポートのフォーマット + +下記テンプレートを `report/SEC_pr_.md` に書き込む。 + +````markdown +# Security Fix PR Report + +- 採用レポート: report/SEC_report_YYYYMMDD_HHMM.md +- 実装ブランチ: +- 適用スコープ: + +## Summary +- 何を直したかを 3-5 行で要約 + +## Applied Fixes +### Critical +- [path/to/file:line] 何を直したか。元レポートの Finding を引用。（違反ルール: security.md「§...」） + +### High +- ... + +### Medium +- ... + +### Low +- ... + +## Design-level Fixes +- [path:line] 直した設計穴（IDOR / SSRF / OAuth / マスアサインメント）。塞いだ攻撃シナリオ。対応する exploit テスト名。 + +## Dependency & Supply Chain +- CVE 更新: → （解消した CVE） +- バージョン固定 / lockfile: pin / lockfile commit した箇所 +- GitHub Actions SHA 固定: <対応した uses> +- 拒否した不審依存（typosquatting / confusion など）があれば記載 + +## Exploit Tests Added +- [tests/...::test_名] 守る仕様 / 期待結果（401 / 403 / 422 / 429 / 拒否）。修正前に赤・修正後に緑を確認したか。 + +## Secrets Remediation +- 除去したファイル / .gitignore 追加 +- **rotate 要否**: 要 / 不要（要の場合、どのクレデンシャルを再発行すべきか。git 履歴除去はユーザー判断待ち） + +## Skipped +- 採用しなかった指摘と理由（影響範囲が大きい / 後続 PR / 要仕様判断） + +## Validation +- `make lint-backend`: pass / fail（fail なら抜粋） +- `make test-backend`: pass / fail +- `make lint-frontend` / `lint-frontend-messages` / `test-frontend`: pass / fail +- `make infra-validate`: pass / fail +- 追加 exploit テスト: 件数 / pass。修正前赤・修正後緑の確認: 済 / 未 +- 依存 audit 再実行: 解消 / 残あり + +## Follow-ups +- 次の PR で対応すべき項目 +- 要ユーザー判断で保留にした項目（秘密情報の履歴除去 など） +```` + +## 進め方の流れ（チェックリスト） + +1. 採用レポートを決定し、パスをユーザーへ提示 +2. Findings 集計を提示し、`AskUserQuestion` で適用スコープを選ばせる +3. 採用項目を `TaskCreate` で 1 つずつ切る +4. 各タスクを `in_progress` にして実装、終わったら `completed` +5. 影響範囲に応じた lint / test / audit（または `make ci`）を回す +6. fail があれば直す。pass まで PR レポートを書かない +7. `report/SEC_pr_.md` を書く +8. ターミナルにはパスとサマリだけ返す（rotate 等の要判断事項は明示） diff --git a/.claude/skills/SEC_review/SKILL.md b/.claude/skills/SEC_review/SKILL.md new file mode 100644 index 00000000..705a4f23 --- /dev/null +++ b/.claude/skills/SEC_review/SKILL.md @@ -0,0 +1,257 @@ +--- +name: SEC_review +description: Use when running a security review / vulnerability check against the DevForge codebase based on `.claude/rules/security.md` — secrets leakage, env literal references, input validation, XSS, token storage, auth guards, rate limit, file upload, IAM least-privilege, Secret Manager. Also covers design-level security holes (IDOR / authorization model / trust boundaries / SSRF / OAuth / mass assignment), dependency supply-chain risk (CVE audit, lockfile/version pinning, GitHub Actions SHA pinning, typosquatting / dependency confusion), and missing exploit-style unit tests (attacker-perspective regression tests). Produces a report under `report/SEC_report_.md`. Trigger on requests such as "セキュリティチェック", "security review", "脆弱性を見て", "設計のセキュリティホール", "サプライチェーン", "PR 前のセキュリティ確認", "SEC_review 実行". +--- + +# Security Review + +`.claude/rules/security.md`（全領域横断セキュリティルール）を走査軸に、コードへセキュリティ観点のレビューを当てる skill。 +組み込みの汎用 `/security-review` と違い、DevForge 固有の正本（`env_keys.py` / `messages.ts` / Secret Manager 運用 / auth-security）に紐づいて判定する。 + +実装変更は `SEC_apply` skill が担う。**本 skill はレビューと提案までで止める。** + +## 先に読む + +- `.claude/CLAUDE.md` +- `.claude/rules/security.md`（**正本**。末尾のチェックリストが走査軸） +- `.claude/rules/backend/auth-security.md`（JWT / Cookie / rate limit / CORS / INTERNAL_SECRET） +- `backend/app/core/env_keys.py`（環境変数名の正本。リテラル参照検出の基準） +- `.claude/rules/frontend/messages.md`（誤検知除外の基準。テスト・英語開発者向け・console は許容） +- `report/dupe/jscpd-report.json` が存在すれば参考程度に（重複は本 skill の主目的ではない） + +## スキャン範囲の決定 + +引数で範囲を切り替える。**最初に必ず採用した範囲を 1 行でユーザーへ返す。** + +```bash +# 省略時 = 現ブランチ差分（PR 前チェック向け・デフォルト） +/SEC_review + +# 全体スキャン +/SEC_review full +``` + +- **差分（デフォルト）**: `git diff --name-only origin/main...HEAD`。`origin/main` が無ければ `origin/dev`、それも無ければ `HEAD~1` にフォールバック。検出されたファイルのうち `backend/** frontend/** infra/**` に該当するものを対象にする。 +- **全体（`full`）**: `backend/** frontend/** infra/**` 全体を security.md 全項目で走査する。 +- 差分が空（コミットなし）なら全体スキャンに自動フォールバックし、その旨を 1 行で告げる。 + +```text +スキャン範囲: 差分 (origin/main...HEAD) — 対象 7 ファイル +``` + +## 成果物の出力先（必須） + +レビュー本文はターミナルに垂れ流さず、必ずファイルへ保存する。スクロールで流れて読み返せなくなるのを防ぐためのルール。 + +- 保存先: `report/SEC_report_.md` + - 例: `report/SEC_report_20260530_1042.md` + - `report/` が無ければ作成する (`mkdir -p report`) + - タイムスタンプはレビュー開始時刻のローカルタイム (`date +%Y%m%d_%H%M`) +- ファイル中身は末尾の「推奨出力フォーマット」に従う +- 既存の `SEC_report_*.md` は削除しない（履歴として残す） + +### ターミナルへの出力ルール + +- レポート本文を assistant メッセージへ貼らない（ファイルにだけ書く） +- ターミナルには以下だけを返す: + 1. スキャン範囲（差分 / 全体） + 2. 保存先パス（`report/SEC_report_YYYYMMDD_HHMM.md`） + 3. `Verdict` セクションの 3-5 行サマリ（Critical / High の件数を含める） + 4. 次に取るべきアクション 1-2 行（例: `/SEC_apply` を促す） +- Findings などの詳細セクションはファイル参照に留める + +## 目的 + +「security.md のどのルールに、どこで違反しているか」を、根拠（ファイル:行 + 引用ルール）付きで列挙する。 +単なる「なんとなく危なそう」ではなく、各 Finding に **どのルール違反か** と **どう直すか** を必ず添える。 + +加えて、grep で機械検出できる規約違反（後述の「検査項目」）だけでなく、**設計レベルのセキュリティホール**（認可モデルの欠陥・信頼境界の取り違え）と **依存サプライチェーンのリスク** を必ず観点に含める。これらは grep だけでは出ないため、コードフローを追って判断する。 + +## 設計観点レビュー（Security-by-Design） + +「個々の行は規約に沿っているが、設計として穴がある」類を拾う。DevForge のアーキテクチャ（`.claude/rules/backend/architecture.md` / `auth-security.md`）を踏まえて以下を見る。 + +- **オブジェクトレベル認可 / IDOR**: エンドポイントが `user_id` 境界を必ず効かせているか。`resumes` / `blog_accounts` 等のリソースを ID 直指定で取得・更新する経路で「他人のリソースを取れてしまう」穴がないか（`get_current_user` で認証だけ通して認可（所有者一致）を確認していないケースが典型）。repository のクエリに `user_id` フィルタが入っているかまで追う。 +- **信頼境界（Trust boundary）**: Cloudflare Pages → Cloud Run の `INTERNAL_SECRET` ヘッダ検証が `routers/internal.py`（Cloud Tasks → backend）で実際に効いているか。内部 API が認証なしで外部公開されていないか。 +- **SSRF / 外部フェッチ**: `services/intelligence/github/api_client.py` や blog collector がユーザー指定の URL / リポジトリ名を使って外部へ fetch する経路で、宛先を検証せず任意 URL を叩けないか（内部メタデータエンドポイントへの到達など）。 +- **OAuth フローの設計**: GitHub OAuth の `state` が backend Cookie で検証されているか（frontend のみ検証は不可）、`redirect_uri` が許可リスト内に固定されているか（オープンリダイレクト防止）。 +- **トークンライフサイクル**: アクセス/リフレッシュトークンの失効・ローテーション、ログアウト時の Cookie 破棄、リフレッシュトークン再利用検知の有無。 +- **マスアサインメント**: Pydantic スキーマが更新系で「ユーザーが書き換えてはいけないフィールド」（`user_id` / `role` / `is_admin` 相当 / タイムスタンプ）まで受け付けていないか。入力スキーマと DB モデルのフィールド差を確認。 +- **ビジネスロジック濫用**: rate limit のない高コスト経路（LLM / 外部 API / PDF 生成）を繰り返し叩くコスト増幅、冪等性のない副作用の二重実行。 +- **エラー / 例外からの情報漏洩**: スタックトレースや内部パス・SQL をユーザー向けレスポンスに返していないか（`detail` に生例外を載せていないか）。 +- **暗号設計**: `FIELD_ENCRYPTION_KEY`（Fernet）で暗号化すべき機微フィールドが平文保存されていないか。鍵の取り違え・固定 IV 等。 + +各設計 Finding には「どのフローのどの前提が崩れると何が起きるか（攻撃シナリオ）」を 1-2 行で添える。 + +## 検査項目（security.md チェックリストを走査軸にする） + +各項目に grep ベースの検出コマンドを付ける。差分モードでは対象ファイルに絞って実行する。 + +### 1. 秘密情報の混入（Secrets Management） + +- `.env` / `.env.*` / `*.pem` / `*.key` / GCP サービスアカウント鍵 / トークンをハードコードした設定が **git 追跡対象に入っていないか**: + - `git ls-files | rg -n '\.(env|pem|key)$|service.?account.*\.json'` + - `.gitignore` に上記が入っているかも確認 +- ソース中のハードコードされた認証情報: `rg -n '(api[_-]?key|secret|token|password)\s*[:=]\s*["'"'"'][A-Za-z0-9_\-]{16,}' backend frontend infra` +- 実ツールがあれば併用: `gitleaks detect --no-banner`（無ければ「未実行・要手動」と Secrets Scan に記録） + +### 2. 環境変数のリテラル直接参照 + +- `rg -n 'os\.getenv\("' backend/app`（`env_keys.XXX` 経由でないリテラルは違反） +- `rg -n 'os\.environ\[' backend/app` +- frontend は `import.meta.env.VITE_...` の散在を確認 + +### 3. ログへの秘密情報出力 + +- `rg -n 'logger\.(debug|info|warning|error).*(token|secret|password|email|api[_-]?key)' backend/app`（目視で文脈確認、定数名だけなら誤検知） + +### 4. 入力バリデーション / SQL インジェクション + +- 新規 router の `Any` / `dict` 素通し: `rg -n ': (Any|dict)\b' backend/app/routers` +- SQL 文字列連結: `rg -n 'text\(f["'"'"']|\.execute\(f["'"'"']|".*SELECT.*"\s*\+' backend/app` +- API エンドポイントの引数が Pydantic モデルで受けているか + +### 5. LLM プロンプトのサニタイズ + +- ユーザー由来文字列を `services/llm/sanitizer.py` を通さずプロンプトに埋め込んでいないか: `rg -n 'prompt|messages=' backend/app/services` 周辺を目視 + +### 6. Frontend XSS + +- `rg -n 'dangerouslySetInnerHTML|\.innerHTML|eval\(|new Function\(' frontend/src` +- `target="_blank"` の `rel="noopener noreferrer"` 欠落: `rg -n 'target=["'"'"']_blank' frontend/src` → 各箇所で rel を確認 +- Markdown レンダラの sanitize 無効化（`react-markdown` の `rehype-raw` 等） + +### 7. トークン保管（Frontend） + +- `rg -n '(localStorage|sessionStorage)\.(set|get)Item' frontend/src` でトークン保存がないか +- Redux store に生のトークン文字列を載せていないか + +### 8. 認証ガード + +- 新規エンドポイントに `get_current_user` 依存が付いているか: `rg -n '@router\.(get|post|put|patch|delete)' backend/app/routers` の各関数で `Depends(get_current_user)` 有無を確認（公開エンドポイントは理由を Info に） + +### 9. Rate limit + +- 高コスト処理（外部 API / LLM 実行）に `slowapi` の `@limiter.limit` が付いているか + +### 10. ファイルアップロード + +- アップロード機能があれば: MIME(magic bytes)検証 / サイズ上限 / ファイル名サニタイズ(UUID リネーム) / 保存先（本番は GCS）の 4 点を確認 + +### 11. Infra: IAM 最小権限 + +- `rg -n 'roles/(owner|editor)' infra/` が Cloud Run SA に付いていないか +- 新規ロール付与に「なぜ必要か」のコメントがあるか（`infra/modules/service_account/`） + +### 12. Infra: Secret Manager + +- シークレットを扱う resource に `sensitive = true` が付いているか: `rg -n 'sensitive\s*=\s*true' infra/`（欠落候補を目視） +- 平文シークレットの埋め込みがないか + +### 13. 依存監査 / サプライチェーン攻撃リスク（Dependency & Supply Chain） + +既知 CVE だけでなく「依存経路そのものが攻撃面になる」観点で見る。CI には既に audit が配線済み（`.github/workflows/ci.yml`: `npm audit --audit-level=high` / `uv tool run pip-audit -r requirements.txt`）なので、ローカルでは差分の妥当性確認に重点を置く。 + +- **既知 CVE スキャン**（実行可能なら）: + - `nix develop --command bash -c "cd frontend && npm audit --audit-level=high"` + - `nix develop --command bash -c "cd backend && uv tool run pip-audit -r requirements.txt"`（実行できなければ「未実行・要手動」と記録。**本 skill で新規ツール導入はしない**） + - High / Critical を Findings に取り込む +- **バージョン固定 / lockfile 整合**: 直接依存にレンジ指定（`^` / `~` / `*` / `>=` のみ）が無いか、lockfile（`package-lock.json` / `uv.lock` 等）が commit され integrity hash を持つか。`requirements.txt` が pin（`==`）されているか。 +- **GitHub Actions のピン留め**: `uses:` がタグ（`@v4`）ではなく commit SHA で固定されているか（直近 commit「GitHub Actions のサプライチェーン保護」で対応済みの方針を維持。`rg -n 'uses:.*@v[0-9]' .github/workflows` で SHA 未固定を検出）。 +- **新規・更新依存の素性**: 差分で追加された依存があれば、メンテ状況・ダウンロード規模・typosquatting（正規パッケージ名との1文字違い）・dependency confusion（社内名と公開名の衝突）を確認。`postinstall` / ビルドスクリプトを持つ npm パッケージは特に注視。 +- **取得元の信頼性**: パッケージ取得が公式レジストリ（npm / PyPI）以外（任意 git URL / 直リンク tarball）を指していないか。 +- **transitive 依存の急増**: 1 パッケージ追加で推移的依存が大量に増えていないか（攻撃面の拡大）。 +- 上記は Findings の重大度に「サプライチェーン」観点を明記して取り込む。 + +## 重大度分類 + +- **Critical**: 秘密情報の git 混入、認証バイパス、SQL インジェクション、本番 SA への owner/editor +- **High**: env リテラル参照、認証ガード欠落、`dangerouslySetInnerHTML` 新規使用、High CVE +- **Medium**: rate limit 欠落、`rel` 欠落、`sensitive` 欠落、ログ漏洩の疑い +- **Low**: 軽微な hardening 余地 +- **Info / Allowed**: 誤検知・許容例外（テストコード / `constants/messages.ts` / 英語開発者向けメッセージ / `console.*` / 意図的な公開エンドポイント）。**理由を必ず残す**（`.claude/rules/frontend/messages.md` の例外節に準拠）。 + +各 Finding には **security.md のどのルール違反か** を必ず引用する。 + +## 脆弱性を突いた unittest の観点（Exploit-style Tests） + +「攻撃が失敗することを assert する」テストが存在するかをレビューする。設計観点で見つけた穴は、回帰防止として **攻撃者視点のテスト** で固定すべき。不足しているケースを Findings とは別に列挙する（実際に書くのは `SEC_apply`）。 + +確認・提案する観点（`.claude/rules/backend/test.md` の方針に沿い、DB はモックせず実 SQLite セッション、外部 API はモック）: + +- **認可（IDOR）**: ユーザー A がユーザー B のリソース（resume / blog_account / notification）を ID 直指定で取得・更新・削除 → **403 / 404 を返す**ことを assert。所有者一致を破る試みが通らないこと。 +- **認証ガード**: トークン無し / 期限切れ / 改竄トークンで保護エンドポイントを叩く → **401**。`get_current_user` 依存の欠落を検知する。 +- **入力境界**: 過大長・型不正・想定外フィールド（マスアサインメント）を投げる → **422 / 無視**。`user_id` 上書きが効かないこと。 +- **CSRF / OAuth state**: `state` 不一致・欠落の OAuth コールバック → 拒否。CSRF トークン不正 → 拒否。 +- **内部 API 境界**: `INTERNAL_SECRET` 無し / 不正で `routers/internal.py` を叩く → 拒否。 +- **SSRF**: collector / GitHub クライアントに内部アドレス・スキーム不正な URL を渡す → 拒否（モックで宛先検証ロジックを通す）。 +- **rate limit**: 高コスト経路を上限超で連打 → **429**。 +- **暗号**: 機微フィールドが DB 上で平文でないこと（保存後に raw 値が読めない）を assert。 + +各観点は「守る仕様」がテスト名から読めること（例: `test_他人のresumeはget_404`）。既にカバー済みなら Info に、未カバーなら「Missing Exploit Tests」に挙げる。 + +## 推奨出力フォーマット + +下記テンプレートを `report/SEC_report_.md` に書き込む。ターミナルには貼らない。 + +````markdown +# Security Review + +- スキャン範囲: 差分 (origin/main...HEAD) / 全体 +- 対象ファイル数: N + +## Verdict +- セキュリティ総評を 3-5 行で。Critical / High の件数、設計観点の穴の有無、サプライチェーンリスク、不足する exploit テスト数を含める。 + +## Findings +### Critical +- [path/to/file:line] 何が問題か。なぜ危険か。どう直すか。（違反ルール: security.md「§秘密情報管理」など） + +### High +- ... + +### Medium +- ... + +### Low +- ... + +## Design-level Findings +- **観点**: +- [path:line] 設計上の穴。**攻撃シナリオ**（何が崩れると何が起きるか 1-2 行）。修正方針。 + +## Dependency & Supply Chain +- npm audit: <結果 or 未実行理由> +- pip-audit: <結果 or 未実行理由> +- バージョン固定 / lockfile integrity: +- GitHub Actions SHA 固定: +- 新規・更新依存の素性（typosquatting / dependency confusion / postinstall）: <なし / 懸念箇所> + +## Secrets Scan +- git 追跡対象の秘密ファイル: <なし / 検出パス> +- ハードコード認証情報: <なし / 検出箇所> +- gitleaks: <結果 or 未実行> + +## Missing Exploit Tests +- [対象エンドポイント / モジュール] 追加すべき攻撃者視点テスト。守る仕様（例: `test_他人のresumeはget_404`）。期待結果（401 / 403 / 422 / 429 / 拒否）。 + +## False Positives / Allowed +- [path:line] 検出されたが許容する理由（テスト / messages.ts / 英語開発者向け 等） + +## Remediation Plan +1. まず直すべき Critical / High +2. 次に対応する Medium +3. 最後に検討する Low + +## Validation +- 実行したコマンド（grep / git / audit） +- 未実行のものとその理由 +```` + +## 最低限の検証コマンド + +- スキャンは grep / git ベースで破壊なし。差分対象は `git diff --name-only` で取得。 +- 依存監査は nix wrap 経由で実行（生シェルで `.venv/bin/` を直接叩かない。WeasyPrint の動的ライブラリ解決に失敗する）。 +- `make lint-*` / `make test-*` は本 skill では必須としない（修正検証は `SEC_apply` 側で回す）。 + +実装変更は `SEC_apply` skill が担う。本 skill はレビューと提案までで止める。 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c2ec8be..1bfbe17a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,12 +32,12 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 - name: Detect changed application files - uses: dorny/paths-filter@v3 + uses: dorny/paths-filter@d1c1ffe0248fe513906c8e24db8ea791d46f8590 # v3 id: filter with: filters: | @@ -68,12 +68,12 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" cache: npm @@ -83,7 +83,7 @@ jobs: run: npx --yes jscpd@4 --config .jscpd.json - name: Upload duplication report - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 if: always() with: name: jscpd-report @@ -98,10 +98,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" cache: npm @@ -135,10 +135,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" cache: npm @@ -162,7 +162,7 @@ jobs: CI: true - name: Upload Playwright report - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 if: failure() with: name: playwright-report @@ -177,10 +177,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Install uv - uses: astral-sh/setup-uv@v4 + uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4 with: version: "latest" enable-cache: true @@ -298,7 +298,7 @@ jobs: timeout-minutes: 10 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 @@ -308,7 +308,7 @@ jobs: echo "APP_VERSION=$VERSION" >> $GITHUB_ENV - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" cache: npm @@ -324,7 +324,7 @@ jobs: CLOUD_RUN_URL: ${{ secrets.CLOUD_RUN_URL_DEV }} - name: Deploy to Cloudflare Pages (dev) - uses: cloudflare/wrangler-action@v3 + uses: cloudflare/wrangler-action@9acf94ace14e7dc412b076f2c5c20b8ce93c79cd # v3 with: apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -351,7 +351,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 @@ -361,12 +361,12 @@ jobs: echo "APP_VERSION=$VERSION" >> $GITHUB_ENV - name: Authenticate to GCP - uses: google-github-actions/auth@v2 + uses: google-github-actions/auth@c200f3691d83b41bf9bbd8638997a462592937ed # v2 with: credentials_json: ${{ secrets.GCP_SA_KEY }} - name: Setup gcloud - uses: google-github-actions/setup-gcloud@v2 + uses: google-github-actions/setup-gcloud@e427ad8a34f8676edf47cf7d7925499adf3eb74f # v2 - name: Configure Docker for Artifact Registry run: gcloud auth configure-docker ${{ env.REGION }}-docker.pkg.dev --quiet @@ -400,7 +400,7 @@ jobs: timeout-minutes: 10 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 @@ -410,7 +410,7 @@ jobs: echo "APP_VERSION=$VERSION" >> $GITHUB_ENV - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" cache: npm @@ -426,7 +426,7 @@ jobs: CLOUD_RUN_URL: ${{ secrets.CLOUD_RUN_URL_STG }} - name: Deploy to Cloudflare Pages (stg) - uses: cloudflare/wrangler-action@v3 + uses: cloudflare/wrangler-action@9acf94ace14e7dc412b076f2c5c20b8ce93c79cd # v3 with: apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -454,7 +454,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 @@ -464,12 +464,12 @@ jobs: echo "APP_VERSION=$VERSION" >> $GITHUB_ENV - name: Authenticate to GCP - uses: google-github-actions/auth@v2 + uses: google-github-actions/auth@c200f3691d83b41bf9bbd8638997a462592937ed # v2 with: credentials_json: ${{ secrets.GCP_SA_KEY_STG }} - name: Setup gcloud - uses: google-github-actions/setup-gcloud@v2 + uses: google-github-actions/setup-gcloud@e427ad8a34f8676edf47cf7d7925499adf3eb74f # v2 - name: Configure Docker for Artifact Registry run: gcloud auth configure-docker ${{ env.REGION }}-docker.pkg.dev --quiet @@ -503,7 +503,7 @@ jobs: timeout-minutes: 10 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 @@ -513,7 +513,7 @@ jobs: echo "APP_VERSION=$VERSION" >> $GITHUB_ENV - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" cache: npm @@ -529,7 +529,7 @@ jobs: CLOUD_RUN_URL: ${{ secrets.CLOUD_RUN_URL_PROD }} - name: Deploy to Cloudflare Pages (prod) - uses: cloudflare/wrangler-action@v3 + uses: cloudflare/wrangler-action@9acf94ace14e7dc412b076f2c5c20b8ce93c79cd # v3 with: apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -557,7 +557,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 @@ -567,12 +567,12 @@ jobs: echo "APP_VERSION=$VERSION" >> $GITHUB_ENV - name: Authenticate to GCP - uses: google-github-actions/auth@v2 + uses: google-github-actions/auth@c200f3691d83b41bf9bbd8638997a462592937ed # v2 with: credentials_json: ${{ secrets.GCP_SA_KEY_PROD }} - name: Setup gcloud - uses: google-github-actions/setup-gcloud@v2 + uses: google-github-actions/setup-gcloud@e427ad8a34f8676edf47cf7d7925499adf3eb74f # v2 - name: Configure Docker for Artifact Registry run: gcloud auth configure-docker ${{ env.REGION }}-docker.pkg.dev --quiet diff --git a/.github/workflows/opentofu-ci.yml b/.github/workflows/opentofu-ci.yml index d7dfd71b..9a35bbac 100644 --- a/.github/workflows/opentofu-ci.yml +++ b/.github/workflows/opentofu-ci.yml @@ -35,12 +35,12 @@ jobs: prod: ${{ steps.filter.outputs.prod }} steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 - name: Detect changed environments - uses: dorny/paths-filter@v3 + uses: dorny/paths-filter@d1c1ffe0248fe513906c8e24db8ea791d46f8590 # v3 id: filter with: filters: | @@ -69,11 +69,11 @@ jobs: - name: Checkout if: needs.detect-changes.outputs.infra == 'true' - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup OpenTofu if: needs.detect-changes.outputs.infra == 'true' - uses: opentofu/setup-opentofu@v1 + uses: opentofu/setup-opentofu@9d84900f3238fab8cd84ce47d658d25dd008be2f # v1 with: tofu_version: 1.8.5 @@ -93,11 +93,11 @@ jobs: - name: Checkout if: needs.detect-changes.outputs.dev == 'true' - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup OpenTofu if: needs.detect-changes.outputs.dev == 'true' - uses: opentofu/setup-opentofu@v1 + uses: opentofu/setup-opentofu@9d84900f3238fab8cd84ce47d658d25dd008be2f # v1 with: tofu_version: 1.8.5 @@ -121,11 +121,11 @@ jobs: - name: Checkout if: needs.detect-changes.outputs.stg == 'true' - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup OpenTofu if: needs.detect-changes.outputs.stg == 'true' - uses: opentofu/setup-opentofu@v1 + uses: opentofu/setup-opentofu@9d84900f3238fab8cd84ce47d658d25dd008be2f # v1 with: tofu_version: 1.8.5 @@ -149,11 +149,11 @@ jobs: - name: Checkout if: needs.detect-changes.outputs.prod == 'true' - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup OpenTofu if: needs.detect-changes.outputs.prod == 'true' - uses: opentofu/setup-opentofu@v1 + uses: opentofu/setup-opentofu@9d84900f3238fab8cd84ce47d658d25dd008be2f # v1 with: tofu_version: 1.8.5 diff --git a/backend/app/routers/internal.py b/backend/app/routers/internal.py index 7f1b7a6b..355a0c0b 100644 --- a/backend/app/routers/internal.py +++ b/backend/app/routers/internal.py @@ -12,6 +12,8 @@ import os from fastapi import APIRouter, HTTPException, Request +from google.auth.transport.requests import Request as GoogleAuthRequest +from google.oauth2 import id_token from ..core import env_keys from ..core.messages import get_error @@ -24,16 +26,54 @@ router = APIRouter(prefix="/internal/tasks", tags=["internal"]) +def _get_bearer_token(request: Request) -> str: + """Authorization ヘッダーから Bearer token を取り出す。""" + authorization = request.headers.get("Authorization", "") + scheme, _, token = authorization.partition(" ") + if scheme.lower() != "bearer" or not token: + return "" + return token.strip() + + +def _verify_cloud_tasks_oidc(request: Request) -> bool: + """Cloud Tasks OIDC トークンの audience と発行元サービスアカウントを検証する。""" + expected_audience = os.environ.get(env_keys.CLOUD_TASKS_SERVICE_URL, "").strip() + expected_service_account = os.environ.get(env_keys.CLOUD_TASKS_SERVICE_ACCOUNT, "").strip() + token = _get_bearer_token(request) + if not expected_audience or not expected_service_account or not token: + return False + + try: + claims = id_token.verify_oauth2_token( + token, + GoogleAuthRequest(), + audience=expected_audience, + ) + except ValueError: + logger.warning("Cloud Tasks OIDC トークン検証に失敗しました", exc_info=True) + return False + + issuer = claims.get("iss") + email = claims.get("email") + email_verified = claims.get("email_verified") + allowed_issuers = {"https://accounts.google.com", "accounts.google.com"} + return ( + issuer in allowed_issuers + and email == expected_service_account + and email_verified is True + ) + + def _verify_request(request: Request) -> bool: """Cloud Tasks からのリクエストか検証する。 - TASK_RUNNER=cloud_tasks の場合のみ X-CloudTasks-QueueName ヘッダーを必須とする。 + TASK_RUNNER=cloud_tasks の場合は X-CloudTasks-QueueName と OIDC を必須とする。 未設定（空文字含む）はローカル／テスト環境とみなし無条件で許可する。 """ if os.environ.get(env_keys.TASK_RUNNER, "").strip() != "cloud_tasks": return True queue_name = request.headers.get("X-CloudTasks-QueueName") - return bool(queue_name) + return bool(queue_name) and _verify_cloud_tasks_oidc(request) def _get_max_attempts() -> int: diff --git a/backend/app/services/intelligence/github/api_client.py b/backend/app/services/intelligence/github/api_client.py index 8cd3a560..940bf122 100644 --- a/backend/app/services/intelligence/github/api_client.py +++ b/backend/app/services/intelligence/github/api_client.py @@ -5,6 +5,7 @@ """ import logging +import re import time from typing import Any, Dict, List, Optional @@ -16,6 +17,32 @@ GITHUB_API = "https://api.github.com" +# GitHub の owner（ユーザー/Organization）名と repo 名の許容パターン。 +# owner: 英数字とハイフンのみ。repo: 英数字 . _ - のみ。 +# API パスへ補間する前にこのパターンで検証し、不正文字によるパス操作・SSRF を防ぐ（多層防御）。 +_OWNER_PATTERN = re.compile(r"^[A-Za-z0-9-]{1,39}$") +_REPO_PATTERN = re.compile(r"^[A-Za-z0-9._-]{1,100}$") + + +def _ensure_valid_owner(owner: str) -> None: + """owner（GitHub ユーザー/Org 名）が許容パターンに合致するか検証する。""" + if not _OWNER_PATTERN.fullmatch(owner or ""): + raise NonRetryableError(f"不正な GitHub ユーザー名: {owner!r}") + + +def _ensure_valid_repo(repo: str) -> None: + """repo 名が許容パターンに合致するか検証する。""" + if not _REPO_PATTERN.fullmatch(repo or ""): + raise NonRetryableError(f"不正な GitHub リポジトリ名: {repo!r}") + + +def _is_valid_owner_repo(owner: str, repo: str) -> bool: + """owner / repo が両方とも許容パターンに合致するか（多層防御の軽量判定）。""" + return bool(_OWNER_PATTERN.fullmatch(owner or "")) and bool( + _REPO_PATTERN.fullmatch(repo or "") + ) + + # 一時障害とみなす HTTP ステータスコード _RETRYABLE_STATUS_CODES = {408, 425, 429, 500, 502, 503, 504} @@ -90,6 +117,7 @@ async def fetch_repos_raw( - 5xx も ``RetryableError`` を raise する - その他の 4xx は ``NonRetryableError`` を raise する """ + _ensure_valid_owner(username) raw_repos: List[Dict[str, Any]] = [] for page in range(1, max_pages + 1): resp = await client.get( @@ -163,6 +191,9 @@ async def fetch_languages( repo: str, ) -> Dict[str, int]: """リポジトリの言語バイト数を取得する。""" + if not _is_valid_owner_repo(owner, repo): + logger.warning("不正な owner/repo をスキップ: %s/%s", owner, repo) + return {} try: resp = await client.get(f"/repos/{owner}/{repo}/languages") if resp.status_code == 403: @@ -181,6 +212,9 @@ async def fetch_root_files( repo: str, ) -> List[str]: """リポジトリのルートレベルの注目すべきファイル名/ディレクトリ名を取得する。""" + if not _is_valid_owner_repo(owner, repo): + logger.warning("不正な owner/repo をスキップ: %s/%s", owner, repo) + return [] try: resp = await client.get(f"/repos/{owner}/{repo}/contents/") if resp.status_code in (403, 404): @@ -208,6 +242,9 @@ async def fetch_file_content( path: str, ) -> Optional[str]: """リポジトリから生のファイルコンテンツをダウンロードする。""" + if not _is_valid_owner_repo(owner, repo): + logger.warning("不正な owner/repo をスキップ: %s/%s", owner, repo) + return None try: resp = await client.get( f"/repos/{owner}/{repo}/contents/{path}", diff --git a/backend/app/services/tasks/cloud_tasks.py b/backend/app/services/tasks/cloud_tasks.py index 275955cd..01803727 100644 --- a/backend/app/services/tasks/cloud_tasks.py +++ b/backend/app/services/tasks/cloud_tasks.py @@ -25,16 +25,21 @@ def __init__(self): ) self._service_url = os.environ[env_keys.CLOUD_TASKS_SERVICE_URL] self._service_account = os.environ.get(env_keys.CLOUD_TASKS_SERVICE_ACCOUNT, "") + self._internal_secret = os.environ.get(env_keys.INTERNAL_SECRET, "") async def dispatch(self, task_type: TaskType, payload: dict) -> None: task = tasks_v2.Task( http_request=tasks_v2.HttpRequest( http_method=tasks_v2.HttpMethod.POST, url=f"{self._service_url}/internal/tasks/{task_type.value}", - headers={"Content-Type": "application/json"}, + headers={ + "Content-Type": "application/json", + "X-Internal-Secret": self._internal_secret, + }, body=json.dumps(payload).encode(), oidc_token=tasks_v2.OidcToken( service_account_email=self._service_account, + audience=self._service_url, ), ), dispatch_deadline={"seconds": 1800}, diff --git a/backend/requirements.txt b/backend/requirements.txt index 59953e86..78919d7a 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -20,11 +20,11 @@ autopep8==2.3.2 markdown==3.8.1 weasyprint==68.0 pydyf==0.12.1 -google-genai>=1.0.0 +google-genai==1.46.0 redis==5.0.1 isort==5.12.0 black==26.3.1 pytest-cov==6.1.0 google-cloud-tasks>=2.16,<3 -pyasn1>=0.6.3 -python-multipart>=0.0.27 +pyasn1==0.6.3 +python-multipart==0.0.27 diff --git a/backend/tests/security/test_admin_authorization.py b/backend/tests/security/test_admin_authorization.py index ed1aef07..9a857950 100644 --- a/backend/tests/security/test_admin_authorization.py +++ b/backend/tests/security/test_admin_authorization.py @@ -47,7 +47,7 @@ def test_wrong_bearer_token_returns_403(self, client: TestClient) -> None: class TestInternalSecret: - """Cloud Tasks コールバックには X-CloudTasks-QueueName を要求する。""" + """Cloud Tasks コールバックにはキューヘッダーと OIDC を要求する。""" def test_unknown_task_type_returns_400(self, client: TestClient) -> None: resp = client.post("/internal/tasks/totally-unknown-type", json={}) @@ -60,3 +60,76 @@ def test_missing_cloud_tasks_header_returns_403( monkeypatch.setenv("TASK_RUNNER", "cloud_tasks") resp = client.post("/internal/tasks/github_link", json={"user_id": "x"}) assert resp.status_code == 403 + + def test_missing_cloud_tasks_oidc_returns_403( + self, client: TestClient, monkeypatch: pytest.MonkeyPatch + ) -> None: + """TASK_RUNNER=cloud_tasks では OIDC Bearer token が無いと 403。""" + monkeypatch.setenv("TASK_RUNNER", "cloud_tasks") + monkeypatch.setenv("CLOUD_TASKS_SERVICE_URL", "https://backend.example.com") + monkeypatch.setenv( + "CLOUD_TASKS_SERVICE_ACCOUNT", + "tasks@example.iam.gserviceaccount.com", + ) + resp = client.post( + "/internal/tasks/github_link", + json={"user_id": "x"}, + headers={"X-CloudTasks-QueueName": "queue"}, + ) + assert resp.status_code == 403 + + def test_invalid_cloud_tasks_oidc_claims_return_403( + self, client: TestClient, monkeypatch: pytest.MonkeyPatch + ) -> None: + """OIDC の email が期待 SA と違う場合は 403。""" + monkeypatch.setenv("TASK_RUNNER", "cloud_tasks") + monkeypatch.setenv("CLOUD_TASKS_SERVICE_URL", "https://backend.example.com") + monkeypatch.setenv( + "CLOUD_TASKS_SERVICE_ACCOUNT", + "tasks@example.iam.gserviceaccount.com", + ) + monkeypatch.setattr( + "app.routers.internal.id_token.verify_oauth2_token", + lambda token, request, audience: { + "iss": "https://accounts.google.com", + "email": "attacker@example.iam.gserviceaccount.com", + "email_verified": True, + }, + ) + resp = client.post( + "/internal/tasks/github_link", + json={"user_id": "x"}, + headers={ + "X-CloudTasks-QueueName": "queue", + "Authorization": "Bearer token", + }, + ) + assert resp.status_code == 403 + + def test_valid_cloud_tasks_oidc_reaches_handler( + self, client: TestClient, monkeypatch: pytest.MonkeyPatch + ) -> None: + """正しいキューヘッダーと OIDC なら内部ハンドラまで到達する。""" + monkeypatch.setenv("TASK_RUNNER", "cloud_tasks") + monkeypatch.setenv("CLOUD_TASKS_SERVICE_URL", "https://backend.example.com") + monkeypatch.setenv( + "CLOUD_TASKS_SERVICE_ACCOUNT", + "tasks@example.iam.gserviceaccount.com", + ) + monkeypatch.setattr( + "app.routers.internal.id_token.verify_oauth2_token", + lambda token, request, audience: { + "iss": "https://accounts.google.com", + "email": "tasks@example.iam.gserviceaccount.com", + "email_verified": True, + }, + ) + resp = client.post( + "/internal/tasks/totally-unknown-type", + json={}, + headers={ + "X-CloudTasks-QueueName": "queue", + "Authorization": "Bearer token", + }, + ) + assert resp.status_code == 400 diff --git a/backend/tests/security/test_mass_assignment.py b/backend/tests/security/test_mass_assignment.py new file mode 100644 index 00000000..30d116c0 --- /dev/null +++ b/backend/tests/security/test_mass_assignment.py @@ -0,0 +1,120 @@ +"""mass-assignment 対策テスト。 + +作成・更新系の入力に `user_id` 等のサーバ管理フィールドを混ぜても、所有者が +攻撃者の指定どおりに移らない（サーバ側が認証ユーザーに固定する）ことを保証する。 +""" + +from unittest.mock import AsyncMock, patch + +from app.repositories import BlogAccountRepository, ResumeRepository, UserRepository +from fastapi.testclient import TestClient + +from conftest import auth_header, make_resume_payload + +_ACCOUNT_VERIFY_PATCH = "app.routers.blog.accounts.verify_user_exists" +_SERVICE_VERIFY_PATCH = "app.services.blog.account_service.verify_user_exists" + + +def test_resume_create_does_not_transfer_ownership(client: TestClient) -> None: + """POST /api/resumes に他人の user_id を混ぜても、その他人は所有者にならない。""" + db = client._db_session + UserRepository(db).create("victim-a", hashed_password=None, email="victim-a@example.com") + victim = UserRepository(db).get_by_username("victim-a") + + headers = auth_header(client, "attacker-a") + attacker = UserRepository(db).get_by_username("attacker-a") + + payload = make_resume_payload(user_id=victim.id) + resp = client.post("/api/resumes", json=payload, headers=headers) + # extra フィールドは無視(201)または拒否(422)のいずれでも所有権移転は起きてはならない + assert resp.status_code in (201, 422), f"unexpected {resp.status_code}: {resp.text}" + + # 被害者 B は決して所有者にならない + assert ResumeRepository(db, victim.id).get_latest() is None + if resp.status_code == 201: + # 作成された場合は攻撃者自身に紐づく + assert ResumeRepository(db, attacker.id).get_latest() is not None + + +def test_resume_update_does_not_transfer_ownership(client: TestClient) -> None: + """PUT /api/resumes/{id} に他人の user_id を混ぜても所有者が移らない。""" + db = client._db_session + UserRepository(db).create("victim-b", hashed_password=None, email="victim-b@example.com") + victim = UserRepository(db).get_by_username("victim-b") + + headers = auth_header(client, "attacker-b") + attacker = UserRepository(db).get_by_username("attacker-b") + + created = client.post("/api/resumes", json=make_resume_payload(), headers=headers) + assert created.status_code == 201 + resume = ResumeRepository(db, attacker.id).get_latest() + assert resume is not None + + upd = client.put( + f"/api/resumes/{resume.id}", + json=make_resume_payload(user_id=victim.id, self_pr="更新後の自己PR"), + headers=headers, + ) + assert upd.status_code in (200, 422) + + # 所有権は攻撃者のまま。被害者には渡らない。 + assert ResumeRepository(db, victim.id).get_latest() is None + assert ResumeRepository(db, attacker.id).get_latest() is not None + + +def test_blog_account_create_does_not_transfer_ownership(client: TestClient) -> None: + """POST /api/blog/accounts に他人の user_id を混ぜても、その他人は所有者にならない。""" + db = client._db_session + UserRepository(db).create("victim-blog-a", hashed_password=None, email="victim-blog-a@example.com") + victim = UserRepository(db).get_by_username("victim-blog-a") + + headers = auth_header(client, "attacker-blog-a") + attacker = UserRepository(db).get_by_username("attacker-blog-a") + + with patch(_ACCOUNT_VERIFY_PATCH, new_callable=AsyncMock, return_value=True): + resp = client.post( + "/api/blog/accounts", + json={"platform": "zenn", "username": "attacker", "user_id": victim.id}, + headers=headers, + ) + assert resp.status_code in (201, 422), f"unexpected {resp.status_code}: {resp.text}" + + assert BlogAccountRepository(db, victim.id).list_by_user() == [] + if resp.status_code == 201: + accounts = BlogAccountRepository(db, attacker.id).list_by_user() + assert len(accounts) == 1 + assert accounts[0].username == "attacker" + + +def test_blog_account_update_does_not_transfer_ownership(client: TestClient) -> None: + """PATCH /api/blog/accounts/{platform} に他人の user_id を混ぜても所有者が移らない。""" + db = client._db_session + UserRepository(db).create("victim-blog-b", hashed_password=None, email="victim-blog-b@example.com") + victim = UserRepository(db).get_by_username("victim-blog-b") + + headers = auth_header(client, "attacker-blog-b") + attacker = UserRepository(db).get_by_username("attacker-blog-b") + + with patch(_ACCOUNT_VERIFY_PATCH, new_callable=AsyncMock, return_value=True), patch( + _SERVICE_VERIFY_PATCH, + new_callable=AsyncMock, + return_value=True, + ): + created = client.post( + "/api/blog/accounts", + json={"platform": "zenn", "username": "before"}, + headers=headers, + ) + assert created.status_code == 201 + updated = client.patch( + "/api/blog/accounts/zenn", + json={"username": "after", "user_id": victim.id}, + headers=headers, + ) + assert updated.status_code in (200, 422), f"unexpected {updated.status_code}: {updated.text}" + + assert BlogAccountRepository(db, victim.id).list_by_user() == [] + accounts = BlogAccountRepository(db, attacker.id).list_by_user() + assert len(accounts) == 1 + if updated.status_code == 200: + assert accounts[0].username == "after" diff --git a/backend/tests/security/test_rate_limit.py b/backend/tests/security/test_rate_limit.py new file mode 100644 index 00000000..d10cf094 --- /dev/null +++ b/backend/tests/security/test_rate_limit.py @@ -0,0 +1,53 @@ +"""高コスト業務経路の rate limit (429) 回帰テスト。 + +外部 API / タスク起動を伴う経路に `@limiter.limit` が付いていることを、 +上限超過で 429 が返ることで保証する（security.md §rate limit）。 +""" + +from app.main import limiter +from app.repositories import UserRepository +from fastapi.testclient import TestClient + +from conftest import auth_header + + +def test_github_link_run_rate_limited(client: TestClient) -> None: + """POST /api/github-link/run が 5/分の上限超で 429 を返す。""" + headers = auth_header(client, "rl-gh-user") + # /run は連携済み GitHub アカウント（github_login）を要求するため事前にセットする + db = client._db_session + user = UserRepository(db).get_by_username("rl-gh-user") + user.github_login = "octocat" + db.commit() + + limiter.reset() + statuses: list[int] = [] + for _ in range(8): + resp = client.post( + "/api/github-link/run", + json={"include_forks": False}, + headers=headers, + ) + statuses.append(resp.status_code) + if resp.status_code == 429: + break + assert 429 in statuses, f"429 が観測されなかった: {statuses}" + limiter.reset() + + +def test_blog_sync_rate_limited(client: TestClient) -> None: + """POST /api/blog/accounts/{id}/sync が 10/分の上限超で 429 を返す。 + + 存在しない account_id でも rate limit はハンドラ本体より前に評価されるため、 + 上限超過で 429 になることを検証できる。 + """ + headers = auth_header(client, "rl-blog-user") + limiter.reset() + statuses: list[int] = [] + for _ in range(13): + resp = client.post("/api/blog/accounts/nonexistent/sync", headers=headers) + statuses.append(resp.status_code) + if resp.status_code == 429: + break + assert 429 in statuses, f"429 が観測されなかった: {statuses}" + limiter.reset() diff --git a/backend/tests/security/test_ssrf_github.py b/backend/tests/security/test_ssrf_github.py new file mode 100644 index 00000000..f2612f25 --- /dev/null +++ b/backend/tests/security/test_ssrf_github.py @@ -0,0 +1,76 @@ +"""GitHub API クライアントの SSRF / パス操作対策テスト。 + +`api_client` は username / owner / repo を API パスへ補間する。許容パターン外の +値を渡した場合に HTTP リクエストを発行せず弾くこと（多層防御）を検証する。 + +非同期関数は pytest-asyncio に依存せず専用 event loop で同期的に実行する +（本リポジトリは asyncio_mode=auto を設定していないため）。 +""" + +import asyncio +from unittest.mock import AsyncMock + +import pytest +from app.services.intelligence.github.api_client import ( + _ensure_valid_owner, + _ensure_valid_repo, + fetch_languages, + fetch_repos_raw, +) +from app.services.tasks.exceptions import NonRetryableError + +# パス操作・SSRF を狙う不正なユーザー名/owner 名 +_MALICIOUS_OWNERS = [ + "../../etc/passwd", + "owner/extra-segment", + "has space", + "user@evil.com", + "name%2f..%2f", + "a" * 40, # 39 文字上限超過 + "", +] + + +def _run(coro): + """既存テストの event loop 前提を壊さず非同期関数を実行する。""" + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + asyncio.set_event_loop(asyncio.new_event_loop()) + + +@pytest.mark.parametrize("bad", _MALICIOUS_OWNERS) +def test_ensure_valid_owner_rejects_malicious(bad: str) -> None: + """不正な owner はバリデーションで NonRetryableError を上げる。""" + with pytest.raises(NonRetryableError): + _ensure_valid_owner(bad) + + +def test_ensure_valid_repo_rejects_path_traversal() -> None: + """repo 名のパストラバーサルを弾く。""" + with pytest.raises(NonRetryableError): + _ensure_valid_repo("../../secret") + + +def test_ensure_valid_owner_accepts_normal() -> None: + """正当な GitHub ユーザー名は通す。""" + _ensure_valid_owner("octocat") + _ensure_valid_repo("my-repo.v2_final") + + +def test_fetch_repos_raw_rejects_bad_username_without_http() -> None: + """不正な username では HTTP リクエストを一切発行せず弾く。""" + client = AsyncMock() + with pytest.raises(NonRetryableError): + _run(fetch_repos_raw(client, "../../evil")) + client.get.assert_not_called() + + +def test_fetch_languages_skips_bad_owner_without_http() -> None: + """不正な owner/repo では HTTP を発行せず空を返す（パイプラインは継続）。""" + client = AsyncMock() + result = _run(fetch_languages(client, "evil/../x", "repo")) + assert result == {} + client.get.assert_not_called() diff --git a/docs/api.md b/docs/api.md index 902a03e0..40677876 100644 --- a/docs/api.md +++ b/docs/api.md @@ -125,8 +125,8 @@ REST API エンドポイント一覧と、バックエンド／フロントエ | `GCP_PROJECT_ID` | Cloud Tasks の GCP プロジェクト ID | | `CLOUD_TASKS_QUEUE` | Cloud Tasks のキュー名 | | `CLOUD_TASKS_LOCATION` | Cloud Tasks のロケーション（例: `asia-northeast1`） | -| `CLOUD_TASKS_SERVICE_URL` | Cloud Tasks → Cloud Run コールバック先 URL | -| `CLOUD_TASKS_SERVICE_ACCOUNT` | Cloud Tasks 実行用サービスアカウント | +| `CLOUD_TASKS_SERVICE_URL` | Cloud Tasks → Cloud Run コールバック先 URL（OIDC audience としても検証） | +| `CLOUD_TASKS_SERVICE_ACCOUNT` | Cloud Tasks 実行用サービスアカウント（OIDC `email` として検証） | | `TASK_MAX_ATTEMPTS` | タスク最大試行回数（リトライ判定で参照） | ### Redis（レートリミット等） diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 8780e1e5..ab197200 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -9,6 +9,7 @@ "version": "0.1.0", "dependencies": { "@reduxjs/toolkit": "^2.11.2", + "dompurify": "^3.4.7", "marked": "^17.0.4", "react": "^18.3.1", "react-dom": "^18.3.1", @@ -2896,6 +2897,13 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/trusted-types": { + "version": "2.0.7", + "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz", + "integrity": "sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==", + "license": "MIT", + "optional": true + }, "node_modules/@types/use-sync-external-store": { "version": "0.0.6", "resolved": "https://registry.npmjs.org/@types/use-sync-external-store/-/use-sync-external-store-0.0.6.tgz", @@ -4274,6 +4282,15 @@ "license": "MIT", "peer": true }, + "node_modules/dompurify": { + "version": "3.4.7", + "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.7.tgz", + "integrity": "sha512-2jBxDJY4RR06tQNy4w5FlFH7kfxsQZlufd0sbv+chfHCxeJwrFw2baUDsSwvBISD4K4RDbd0PTfy3uNXsR6siA==", + "license": "(MPL-2.0 OR Apache-2.0)", + "optionalDependencies": { + "@types/trusted-types": "^2.0.7" + } + }, "node_modules/dunder-proto": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz", diff --git a/frontend/package.json b/frontend/package.json index bc960ee4..1eb1842e 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -21,6 +21,7 @@ }, "dependencies": { "@reduxjs/toolkit": "^2.11.2", + "dompurify": "^3.4.7", "marked": "^17.0.4", "react": "^18.3.1", "react-dom": "^18.3.1", diff --git a/frontend/src/components/forms/MarkdownTextarea.test.tsx b/frontend/src/components/forms/MarkdownTextarea.test.tsx new file mode 100644 index 00000000..22883add --- /dev/null +++ b/frontend/src/components/forms/MarkdownTextarea.test.tsx @@ -0,0 +1,61 @@ +import { render } from "@testing-library/react"; +import { describe, it, expect } from "vitest"; +import { MarkdownTextarea } from "./MarkdownTextarea"; + +function getPreview(container: HTMLElement): HTMLElement { + const preview = container.querySelector("textarea + div"); + expect(preview).not.toBeNull(); + return preview as HTMLElement; +} + +/** + * XSS 回帰テスト。 + * MarkdownTextarea は marked の出力を dangerouslySetInnerHTML で描画するため、 + * DOMPurify を通して危険な要素・属性が除去されることを保証する。 + * （security.md §Frontend: Markdown レンダラーは sanitize を有効化） + */ +describe("MarkdownTextarea XSS サニタイズ", () => { + it("script タグとその中身を除去する", () => { + const { container } = render( + window.__xss = 1通常テキスト"} + onChange={() => {}} + />, + ); + const preview = getPreview(container); + expect(preview.querySelector("script")).toBeNull(); + expect(preview.innerHTML).not.toContain("window.__xss"); + }); + + it("img の onerror 属性を除去する", () => { + const { container } = render( + '} + onChange={() => {}} + />, + ); + const preview = getPreview(container); + expect(preview.innerHTML.toLowerCase()).not.toContain("onerror"); + }); + + it("javascript: スキームのリンクを無害化する", () => { + const { container } = render( + click'} + onChange={() => {}} + />, + ); + const anchor = getPreview(container).querySelector("a"); + expect(anchor?.getAttribute("href") ?? "").not.toContain("javascript:"); + }); + + it("通常の Markdown は引き続き描画される", () => { + const { container } = render( + {}} />, + ); + expect(getPreview(container).querySelector("h1")?.textContent).toBe("見出し"); + }); +}); diff --git a/frontend/src/components/forms/MarkdownTextarea.tsx b/frontend/src/components/forms/MarkdownTextarea.tsx index b2ac5f09..fe6cfbe9 100644 --- a/frontend/src/components/forms/MarkdownTextarea.tsx +++ b/frontend/src/components/forms/MarkdownTextarea.tsx @@ -1,5 +1,6 @@ import { useMemo, type ReactNode } from "react"; import { marked } from "marked"; +import DOMPurify from "dompurify"; import shared from "../../styles/shared.module.css"; import styles from "./MarkdownTextarea.module.css"; @@ -26,7 +27,10 @@ type Props = { export function MarkdownTextarea({ label, value, onChange, rows = 3, placeholder, required, labelAdornment }: Props) { const renderedHtml = useMemo(() => { if (!value) return ""; - return marked.parse(value, { async: false }) as string; + // marked は HTML をサニタイズしないため（v5 以降 sanitize オプション廃止）、 + // DOMPurify を通して XSS（