From 10bb59ecb4b1af107e92335fb623e83f02c95089 Mon Sep 17 00:00:00 2001 From: hideyukiMORI Date: Wed, 20 May 2026 00:48:56 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20ThrottleMiddleware=20X-Forwarded-For?= =?UTF-8?q?=20=E3=83=AA=E3=82=B9=E3=82=AF=E6=98=8E=E8=A8=98=20+=20?= =?UTF-8?q?=E3=83=86=E3=82=B9=E3=83=88=20fixture=20=E6=95=B4=E7=90=86=20(#?= =?UTF-8?q?111=20#113)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#111] ThrottleMiddleware の X-Forwarded-For 偽装リスクをドキュメント化 - ADR-0006 に Known Limitations セクションを追加 - docs/reference/framework-modules.md (EN / JA) の ThrottleMiddleware 説明に注意書きを追加 [#113] HTTP テストの _client() を conftest fixture に移動 - tests/example/conftest.py を新規作成 — function スコープ client fixture - test_list_notes.py, test_tags.py, test_comment_http.py を pytest fixture スタイルに移行 - AppSettings(throttle_enabled=False) に統一してフレイクテスト防止 Co-Authored-By: Claude Sonnet 4.6 --- docs/adr/0006-rate-limiting.md | 6 +++ docs/ja/reference/framework-modules.md | 2 + docs/reference/framework-modules.md | 2 + tests/example/comment/test_comment_http.py | 32 +++++---------- tests/example/conftest.py | 13 +++++++ tests/example/note/test_list_notes.py | 45 +++++++++------------- tests/example/tag/test_tags.py | 42 ++++++++------------ 7 files changed, 66 insertions(+), 76 deletions(-) create mode 100644 tests/example/conftest.py diff --git a/docs/adr/0006-rate-limiting.md b/docs/adr/0006-rate-limiting.md index a6048f2..dcde396 100644 --- a/docs/adr/0006-rate-limiting.md +++ b/docs/adr/0006-rate-limiting.md @@ -20,3 +20,9 @@ We need to protect endpoints from abuse and runaway clients. PHP NENE2's `Thrott - Suitable for single-process deployments (uvicorn workers share no state between processes) - For multi-process / multi-node deployments, replace the in-memory store with Redis (implement `ThrottleStoreInterface` — future work) - Fixed-window is vulnerable to burst at window boundary; sliding-log or token-bucket can be added later without changing the interface + +## Known Limitations + +**`X-Forwarded-For` spoofing**: The client key is derived from the first entry in the `X-Forwarded-For` header, which can be set to an arbitrary value by the client. A malicious actor can send different forged IPs on every request to bypass the rate limit. + +**Mitigation**: In production, place the application behind a trusted reverse proxy (nginx, Caddy, AWS ALB, etc.) that strips and rewrites `X-Forwarded-For` before the request reaches the application. Do not expose the app directly to the internet without a reverse proxy when rate limiting is required. diff --git a/docs/ja/reference/framework-modules.md b/docs/ja/reference/framework-modules.md index 1c74843..310cf82 100644 --- a/docs/ja/reference/framework-modules.md +++ b/docs/ja/reference/framework-modules.md @@ -120,6 +120,8 @@ cfg_test = AppSettings(throttle_enabled=False) # テスト用オーバー `ThrottleMiddleware` には `enabled` フラグがありません。`if settings.throttle_enabled:` でラップして制御します。 +> **注意 — `X-Forwarded-For` 偽装**: レートリミットのキーは `X-Forwarded-For` ヘッダーの最初のエントリから生成されますが、クライアントがこのヘッダーを偽装することで制限を回避できます。本番環境では、信頼できるリバースプロキシ（nginx、Caddy、AWS ALB 等）の背後にアプリを配置し、リバースプロキシが `X-Forwarded-For` を上書きするよう設定してください。詳細は [ADR-0006](../../adr/0006-rate-limiting.md) を参照してください。 + #### 完全な登録順（任意ミドルウェア含む） ```python diff --git a/docs/reference/framework-modules.md b/docs/reference/framework-modules.md index fd1b34f..aef49cf 100644 --- a/docs/reference/framework-modules.md +++ b/docs/reference/framework-modules.md @@ -151,6 +151,8 @@ Starlette applies middleware in **reverse registration order** — the last regi `ThrottleMiddleware` has no `enabled` flag — wrap with `if settings.throttle_enabled:` to disable it. +> **Note — `X-Forwarded-For` spoofing**: The rate limit key is derived from the first entry of the `X-Forwarded-For` header, which clients can forge. In production, always place the application behind a trusted reverse proxy (nginx, Caddy, AWS ALB, etc.) that rewrites `X-Forwarded-For` before the request reaches the app. See [ADR-0006](../../adr/0006-rate-limiting.md) for details. + #### Full registration order with optional middleware ```python diff --git a/tests/example/comment/test_comment_http.py b/tests/example/comment/test_comment_http.py index f81e5d9..1582e66 100644 --- a/tests/example/comment/test_comment_http.py +++ b/tests/example/comment/test_comment_http.py @@ -2,25 +2,16 @@ from fastapi.testclient import TestClient -from example.app import create_app -from nene2.config import AppSettings - -def _client() -> TestClient: - cfg = AppSettings(throttle_enabled=False) - return TestClient(create_app(cfg)) - - -def test_list_comments_empty() -> None: - response = _client().get("/notes/1/comments") +def test_list_comments_empty(client: TestClient) -> None: + response = client.get("/notes/1/comments") assert response.status_code == 200 body = response.json() assert body["total"] == 0 assert body["items"] == [] -def test_create_and_list_comments() -> None: - client = _client() +def test_create_and_list_comments(client: TestClient) -> None: create_response = client.post("/notes/1/comments", json={"body": "first comment"}) assert create_response.status_code == 201 data = create_response.json() @@ -31,29 +22,26 @@ def test_create_and_list_comments() -> None: assert list_response.json()["total"] == 1 -def test_get_comment() -> None: - client = _client() +def test_get_comment(client: TestClient) -> None: created = client.post("/notes/1/comments", json={"body": "get me"}).json() response = client.get(f"/notes/1/comments/{created['id']}") assert response.status_code == 200 assert response.json()["body"] == "get me" -def test_get_comment_not_found() -> None: - response = _client().get("/notes/1/comments/9999") +def test_get_comment_not_found(client: TestClient) -> None: + response = client.get("/notes/1/comments/9999") assert response.status_code == 404 -def test_update_comment() -> None: - client = _client() +def test_update_comment(client: TestClient) -> None: created = client.post("/notes/1/comments", json={"body": "original"}).json() response = client.put(f"/notes/1/comments/{created['id']}", json={"body": "updated"}) assert response.status_code == 200 assert response.json()["body"] == "updated" -def test_delete_comment() -> None: - client = _client() +def test_delete_comment(client: TestClient) -> None: created = client.post("/notes/1/comments", json={"body": "to delete"}).json() delete_response = client.delete(f"/notes/1/comments/{created['id']}") assert delete_response.status_code == 204 @@ -61,6 +49,6 @@ def test_delete_comment() -> None: assert get_response.status_code == 404 -def test_create_comment_empty_body_returns_422() -> None: - response = _client().post("/notes/1/comments", json={"body": " "}) +def test_create_comment_empty_body_returns_422(client: TestClient) -> None: + response = client.post("/notes/1/comments", json={"body": " "}) assert response.status_code == 422 diff --git a/tests/example/conftest.py b/tests/example/conftest.py new file mode 100644 index 0000000..111f4c4 --- /dev/null +++ b/tests/example/conftest.py @@ -0,0 +1,13 @@ +"""Shared fixtures for example HTTP integration tests.""" + +import pytest +from fastapi.testclient import TestClient + +from example.app import create_app +from nene2.config import AppSettings + + +@pytest.fixture +def client() -> TestClient: + """Fresh TestClient with in-memory DB and throttle disabled (per-test isolation).""" + return TestClient(create_app(AppSettings(throttle_enabled=False))) diff --git a/tests/example/note/test_list_notes.py b/tests/example/note/test_list_notes.py index 5e78436..67b17fe 100644 --- a/tests/example/note/test_list_notes.py +++ b/tests/example/note/test_list_notes.py @@ -1,17 +1,11 @@ """HTTP-level tests for the Note endpoints.""" +import pytest from fastapi.testclient import TestClient -from nene2.config import AppSettings -from src.example.app import create_app - -def _client() -> TestClient: - return TestClient(create_app(AppSettings())) - - -def test_list_notes_empty() -> None: - r = _client().get("/notes") +def test_list_notes_empty(client: TestClient) -> None: + r = client.get("/notes") assert r.status_code == 200 body = r.json() assert body["items"] == [] @@ -20,8 +14,7 @@ def test_list_notes_empty() -> None: assert body["total"] == 0 -def test_create_and_get_note() -> None: - client = _client() +def test_create_and_get_note(client: TestClient) -> None: r = client.post("/notes", json={"title": "Hello", "body": "World"}) assert r.status_code == 201 note_id = r.json()["id"] @@ -31,19 +24,18 @@ def test_create_and_get_note() -> None: assert r2.json()["title"] == "Hello" -def test_create_note_empty_title_returns_422() -> None: - r = _client().post("/notes", json={"title": "", "body": "b"}) +def test_create_note_empty_title_returns_422(client: TestClient) -> None: + r = client.post("/notes", json={"title": "", "body": "b"}) assert r.status_code == 422 assert r.json()["errors"][0]["field"] == "title" -def test_get_nonexistent_note_returns_404() -> None: - r = _client().get("/notes/9999") +def test_get_nonexistent_note_returns_404(client: TestClient) -> None: + r = client.get("/notes/9999") assert r.status_code == 404 -def test_update_note_returns_200() -> None: - client = _client() +def test_update_note_returns_200(client: TestClient) -> None: r = client.post("/notes", json={"title": "Old", "body": "Old body"}) note_id = r.json()["id"] @@ -53,21 +45,19 @@ def test_update_note_returns_200() -> None: assert r2.json()["body"] == "New body" -def test_update_nonexistent_note_returns_404() -> None: - r = _client().put("/notes/9999", json={"title": "T", "body": "B"}) +def test_update_nonexistent_note_returns_404(client: TestClient) -> None: + r = client.put("/notes/9999", json={"title": "T", "body": "B"}) assert r.status_code == 404 -def test_update_note_empty_title_returns_422() -> None: - client = _client() +def test_update_note_empty_title_returns_422(client: TestClient) -> None: r = client.post("/notes", json={"title": "T", "body": "B"}) note_id = r.json()["id"] r2 = client.put(f"/notes/{note_id}", json={"title": "", "body": "B"}) assert r2.status_code == 422 -def test_delete_note_returns_204() -> None: - client = _client() +def test_delete_note_returns_204(client: TestClient) -> None: r = client.post("/notes", json={"title": "T", "body": "B"}) note_id = r.json()["id"] @@ -78,12 +68,13 @@ def test_delete_note_returns_204() -> None: assert r3.status_code == 404 -def test_delete_nonexistent_note_returns_404() -> None: - r = _client().delete("/notes/9999") +def test_delete_nonexistent_note_returns_404(client: TestClient) -> None: + r = client.delete("/notes/9999") assert r.status_code == 404 -def test_health() -> None: - r = _client().get("/health") +@pytest.mark.usefixtures("client") +def test_health(client: TestClient) -> None: + r = client.get("/health") assert r.status_code == 200 assert r.json()["status"] == "ok" diff --git a/tests/example/tag/test_tags.py b/tests/example/tag/test_tags.py index ea4d2f5..609ceee 100644 --- a/tests/example/tag/test_tags.py +++ b/tests/example/tag/test_tags.py @@ -2,24 +2,16 @@ from fastapi.testclient import TestClient -from nene2.config import AppSettings -from src.example.app import create_app - -def _client() -> TestClient: - return TestClient(create_app(AppSettings())) - - -def test_list_tags_empty() -> None: - r = _client().get("/tags") +def test_list_tags_empty(client: TestClient) -> None: + r = client.get("/tags") assert r.status_code == 200 body = r.json() assert body["items"] == [] assert body["total"] == 0 -def test_create_and_get_tag() -> None: - client = _client() +def test_create_and_get_tag(client: TestClient) -> None: r = client.post("/tags", json={"name": "python"}) assert r.status_code == 201 tag_id = r.json()["id"] @@ -29,19 +21,18 @@ def test_create_and_get_tag() -> None: assert r2.json()["name"] == "python" -def test_create_tag_empty_name_returns_422() -> None: - r = _client().post("/tags", json={"name": ""}) +def test_create_tag_empty_name_returns_422(client: TestClient) -> None: + r = client.post("/tags", json={"name": ""}) assert r.status_code == 422 assert r.json()["errors"][0]["field"] == "name" -def test_get_nonexistent_tag_returns_404() -> None: - r = _client().get("/tags/9999") +def test_get_nonexistent_tag_returns_404(client: TestClient) -> None: + r = client.get("/tags/9999") assert r.status_code == 404 -def test_update_tag_returns_200() -> None: - client = _client() +def test_update_tag_returns_200(client: TestClient) -> None: r = client.post("/tags", json={"name": "old"}) tag_id = r.json()["id"] @@ -50,21 +41,19 @@ def test_update_tag_returns_200() -> None: assert r2.json()["name"] == "new" -def test_update_nonexistent_tag_returns_404() -> None: - r = _client().put("/tags/9999", json={"name": "x"}) +def test_update_nonexistent_tag_returns_404(client: TestClient) -> None: + r = client.put("/tags/9999", json={"name": "x"}) assert r.status_code == 404 -def test_update_tag_empty_name_returns_422() -> None: - client = _client() +def test_update_tag_empty_name_returns_422(client: TestClient) -> None: r = client.post("/tags", json={"name": "t"}) tag_id = r.json()["id"] r2 = client.put(f"/tags/{tag_id}", json={"name": ""}) assert r2.status_code == 422 -def test_delete_tag_returns_204() -> None: - client = _client() +def test_delete_tag_returns_204(client: TestClient) -> None: r = client.post("/tags", json={"name": "temp"}) tag_id = r.json()["id"] @@ -75,13 +64,12 @@ def test_delete_tag_returns_204() -> None: assert r3.status_code == 404 -def test_delete_nonexistent_tag_returns_404() -> None: - r = _client().delete("/tags/9999") +def test_delete_nonexistent_tag_returns_404(client: TestClient) -> None: + r = client.delete("/tags/9999") assert r.status_code == 404 -def test_list_tags_pagination() -> None: - client = _client() +def test_list_tags_pagination(client: TestClient) -> None: for name in ["a", "b", "c"]: client.post("/tags", json={"name": name}) From c822a3e3e0cb8bd40b509ce27664207135059028 Mon Sep 17 00:00:00 2001 From: hideyukiMORI Date: Wed, 20 May 2026 00:49:28 +0900 Subject: [PATCH 2/2] =?UTF-8?q?docs:=20todo/current.md=20=E3=82=92?= =?UTF-8?q?=E5=93=81=E8=B3=AA=E6=94=B9=E5=96=84=20PR=20=E3=81=AE=E7=8A=B6?= =?UTF-8?q?=E6=85=8B=E3=81=AB=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 2026-05-20 のコード品質評価で作成した PR #114〜#119 を一覧化。 Co-Authored-By: Claude Sonnet 4.6 --- docs/todo/current.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/todo/current.md b/docs/todo/current.md index b1c541d..c3008d3 100644 --- a/docs/todo/current.md +++ b/docs/todo/current.md @@ -1,14 +1,27 @@ # TODO — current 最終更新: 2026-05-20 -現状: **v1.x 完了済み** +現状: **v1.x 完了済み / コード品質改善 PR レビュー中** --- ## 状態サマリー v0.1.0〜v1.x のすべてのマイルストーンが完了しています。 -ロードマップの詳細は [roadmap.md](../roadmap.md) を参照してください。 +2026-05-20 のコード品質評価に基づき、以下の改善 PR を作成中です。 + +--- + +## オープン PR（マージ待ち） + +| PR | Issue | 優先度 | 内容 | +|---|---|---|---| +| [#114](https://github.com/hideyukiMORI/nene2-python/pull/114) | #107 | 高 | `McpHttpResponse.body` 型誤記修正（docs） | +| [#115](https://github.com/hideyukiMORI/nene2-python/pull/115) | #108 | 中 | `PaginationQueryParser` 非整数パラメータで 500→ValidationException | +| [#116](https://github.com/hideyukiMORI/nene2-python/pull/116) | #112 | 中低 | `SecurityHeadersMiddleware` CSP が `/docs` を壊す問題を修正 | +| [#117](https://github.com/hideyukiMORI/nene2-python/pull/117) | #110 | 低 | note/tag Input dataclass に `slots=True` 追加 | +| [#118](https://github.com/hideyukiMORI/nene2-python/pull/118) | #109 | 低 | Get UseCase を typed Input DTO パターンに統一 | +| [#119](https://github.com/hideyukiMORI/nene2-python/pull/119) | #111/#113 | 低 | ThrottleMiddleware X-Forwarded-For リスク明記 + テスト fixture 整理 | ---