-
Notifications
You must be signed in to change notification settings - Fork 0
fix: セキュリティ強化(認証・SSRF防止・レート制限) #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
aec1d68
169170c
8695189
c7c5a60
eae64b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,4 +45,6 @@ public/generated/* | |
|
|
||
| tsconfig.tsbuildinfo | ||
| worklog/ | ||
| status/ | ||
| status/ | ||
| .playwright-mcp/ | ||
| .playwright-cli/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # LESSONS | ||
|
|
||
| 開発で得た教訓・パターンの記録。同じミスを繰り返さないためのルール集。 | ||
|
|
||
| ## PRレビュー・マージ運用 (2026-03-17) | ||
|
|
||
| ### Jules (AI) 生成PRは型安全性とロギングの品質チェックが必須 | ||
| - Jules が生成したPR #42 (logger) と #43 (バッチ最適化) で、`any` 型の乱用やスタックトレース消失など品質問題が複数発見された | ||
| - **ルール**: AI生成PRは特に以下を重点チェックする — (1) `any` 型が不必要に使われていないか (2) Error処理でスタック情報が失われていないか (3) テストが正しいランナー(Vitest)で実行されているか | ||
|
|
||
| ### logger で Error オブジェクトを加工する際はスタックトレースを保持する | ||
| - PR #42 の `formatArg` が `err.message` のみ返却し、スタックトレースが完全に消失していた。デバッグが極めて困難になる | ||
| - **ルール**: Error を文字列化する場合は `arg.stack || arg.message` を使う。`arg.message` 単体での変換は禁止 | ||
|
|
||
| ### 競合する複数PRのマージ順序を事前に計画する | ||
| - PR #42 (logger) と #43 (バッチ最適化) が `auto-adopt.ts` で競合。#43を先にマージ → #42をリベースして解決した | ||
| - **ルール**: 同じファイルを変更する複数PRがある場合、依存関係の少ない方を先にマージし、残りをリベースする。マージ前にファイル重複を `gh pr diff` で確認する | ||
|
|
||
| ### サブエージェントの worktree は権限不足で失敗することがある | ||
| - `isolation: "worktree"` でエージェントを起動したが、Edit/Bash 権限が拒否されて作業できなかった | ||
| - **ルール**: worktree エージェントが権限問題で失敗した場合は、メインプロセスで直接対応に切り替える。権限モードは `auto` または `bypassPermissions` を使う | ||
|
|
||
| ## セキュリティ修正 (2026-03-29) | ||
|
|
||
| ### adopt/reject 投票は「誰でも可」が意図された設計 | ||
| - スキーマにルームメンバーシップテーブルがなく、認証済みユーザーなら誰でも投票可能。IDORに見えるが仕様 | ||
| - **ルール**: adopt/reject ルートにルームメンバーシップ制限を追加しない。将来メンバーシップテーブルを追加する場合は vote ルートも合わせて変更する | ||
|
|
||
| ### 高コスト操作の新規エンドポイントにはレートリミッターを使う | ||
| - `src/lib/rate-limit.ts` にインメモリ Map ベースの `checkRateLimit(limiterId, key, max, windowMs)` が実装済み | ||
| - **ルール**: 画像生成など外部API費用が発生する操作には必ず `checkRateLimit()` を適用する。サーバー再起動でリセットされるが単一インスタンスには十分 | ||
|
|
||
| ### tile.imageUrl は常にローカルパス。HTTP URL を loadReferenceImage に渡さない | ||
| - SSRF防止のため `loadReferenceImage()` は HTTP/HTTPS URL を意図的に拒否する。DALL-E URLのダウンロードは `downloadAndUpload()` が直接行う | ||
| - **ルール**: `tile.imageUrl` に外部URLを格納しない。`/placeholder.png` または `/generated/{cuid}.png` のみ。HTTP対応をこの関数に追加しないこと |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # VISION | ||
|
|
||
| ## なぜ作るのか | ||
|
|
||
| 「複数人が一緒に使える AI アプリケーション」を作りたかった。 | ||
|
|
||
| 多くの AI ツールは一人で使うものだが、Gen-Jigsaw は複数人が同じキャンバスに集まり、それぞれが AI を使って世界を広げていく体験を目指している。AI は個人の道具ではなく、共同創造の素材。 | ||
|
|
||
| ## 何を作るのか | ||
|
|
||
| 参加者が協力して一つの世界を描き続けるアプリ。 | ||
|
|
||
| - 誰かがタイルを生成すると、その隣に別の誰かが続きを生成できる | ||
| - AI(アウトペインティング)が境界を自然につなぎ、世界は継ぎ目なく広がっていく | ||
| - 生成された候補はみんなで投票して採用・却下する | ||
|
|
||
| ## 大切にすること | ||
|
|
||
| - **参加すること自体が楽しい** — 生成結果の善し悪しより、誰かと一緒に作っている感覚 | ||
| - **世界の一貫性** — タイルがつながって見えること。隣接コンテキストを活用したシームレスな生成 | ||
| - **シンプルな操作** — AI や技術の知識がなくても、すぐに参加できる |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| interface RateLimitEntry { | ||
| count: number; | ||
| resetAt: number; | ||
| } | ||
|
|
||
| const stores = new Map<string, Map<string, RateLimitEntry>>(); | ||
|
|
||
| // 期限切れエントリを5分ごとに削除してメモリリークを防ぐ | ||
| setInterval(() => { | ||
| const now = Date.now(); | ||
| for (const store of stores.values()) { | ||
| for (const [key, entry] of store.entries()) { | ||
| if (now > entry.resetAt) { | ||
| store.delete(key); | ||
| } | ||
| } | ||
| } | ||
| }, 5 * 60 * 1000); | ||
|
|
||
| /** | ||
| * シンプルなインメモリレートリミッター。 | ||
| * @param limiterId リミッターの識別子(例: "image-gen") | ||
| * @param key ユーザーIDやIPアドレスなどのキー | ||
| * @param maxRequests ウィンドウ内の最大リクエスト数 | ||
| * @param windowMs ウィンドウのミリ秒 | ||
| * @returns true = 許可, false = 超過 | ||
| */ | ||
| export function checkRateLimit( | ||
| limiterId: string, | ||
| key: string, | ||
| maxRequests: number, | ||
| windowMs: number | ||
| ): boolean { | ||
| if (!stores.has(limiterId)) stores.set(limiterId, new Map()); | ||
| const store = stores.get(limiterId)!; | ||
| const now = Date.now(); | ||
| const entry = store.get(key); | ||
|
|
||
| if (!entry || now > entry.resetAt) { | ||
| store.set(key, { count: 1, resetAt: now + windowMs }); | ||
| return true; | ||
| } | ||
| if (entry.count >= maxRequests) return false; | ||
| entry.count++; | ||
| return true; | ||
| } | ||
|
Comment on lines
+28
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. このインメモリレートリミッターの実装は、エントリをクリーンアップする仕組みがないため、時間とともにメモリリークを引き起こす可能性があります。 この問題を解決するために、期限切れのエントリを定期的に削除するクリーンアップ処理を実装することをお勧めします。例えば、 // クリーンアップ処理の例(別途実装が必要)
const CLEANUP_INTERVAL_MS = 5 * 60 * 1000; // 5分ごと
setInterval(() => {
const now = Date.now();
for (const store of stores.values()) {
for (const [key, entry] of store.entries()) {
if (now > entry.resetAt) {
store.delete(key);
}
}
}
}, CLEANUP_INTERVAL_MS); |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new in-memory limiter never evicts old keys from
stores; it only overwrites entries when the same key is seen again. With endpoints that accept arbitrary caller keys (notably/api/users), repeated requests using unique keys will make this map grow indefinitely even after windows expire, which can become a memory-exhaustion path over time. Add expiration cleanup (sweep or bounded cache) so stale buckets are removed.Useful? React with 👍 / 👎.