diff --git a/changelog.d/301.deprecated.md b/changelog.d/301.deprecated.md new file mode 100644 index 0000000..a081af6 --- /dev/null +++ b/changelog.d/301.deprecated.md @@ -0,0 +1,8 @@ +**Calling `add_routes()` without `dependencies`.** The monitoring routes have +no access control of their own and expose every cached key (including query +strings) and response previews, so leaving `dependencies` unset now emits a +`UserWarning`. 0.4.0 will require the parameter and turn +`include_content_preview` off by default +([#298](https://github.com/allen0099/FastAPI-CacheX/issues/298)). Pass a guard +such as `dependencies=[Depends(verify_admin)]`, or `dependencies=[]` to keep +the routes open on purpose without the warning. diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index ce5a8c3..fca331b 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -362,6 +362,12 @@ add_routes( > `include_content_preview=False`) and exposes your whole route structure. In > production always pass `dependencies=[Depends(your_auth)]`, or mount them on > an internal-only app. +> +> Calling `add_routes()` without `dependencies` emits a `UserWarning`. Version +> 0.4.0 will require the parameter and turn `include_content_preview` off by +> default ([#298](https://github.com/allen0099/FastAPI-CacheX/issues/298)). For +> a local or test app that should stay open, pass `dependencies=[]` to opt out +> deliberately without the warning. > [!NOTE] > On Memcached, which cannot enumerate keys, both routes return nothing. diff --git a/fastapi_cachex/routes.py b/fastapi_cachex/routes.py index 16e43ac..3bddacd 100644 --- a/fastapi_cachex/routes.py +++ b/fastapi_cachex/routes.py @@ -1,6 +1,7 @@ """Optional routes for cache monitoring and management.""" import time +import warnings from collections.abc import Sequence from dataclasses import dataclass from typing import TYPE_CHECKING @@ -261,7 +262,10 @@ def add_routes( Mounts two read-only routes that report what the configured backend currently holds. They inspect stored entries; nothing counts cache hits. The routes have no authentication of their own, so pass ``dependencies`` - in production. + in production. Leaving ``dependencies`` unset (``None``) emits a + ``UserWarning``: 0.4.0 will require it. Pass ``dependencies=[]`` to mount + the routes unguarded on purpose (local or test setups) without the + warning. Args: app: FastAPI application instance @@ -269,10 +273,12 @@ def add_routes( Defaults to "" (no prefix). include_in_schema: Whether to include routes in OpenAPI schema. Defaults to False. - dependencies: Optional list of FastAPI ``Depends`` objects applied to - all monitoring routes. Useful for adding authentication - or authorization guards (e.g. - ``[Depends(verify_api_key)]``). + dependencies: FastAPI ``Depends`` objects applied to all monitoring + routes, for authentication or authorization guards + (e.g. ``[Depends(verify_api_key)]``). ``None`` (the + default) mounts the routes unguarded and emits a + ``UserWarning``; an explicit ``[]`` does the same + without the warning. include_content_preview: Whether ``/cached-records`` includes the first bytes of each cached response body. When False, ``content_preview`` is ``null`` while keys, sizes and @@ -280,16 +286,27 @@ def add_routes( Example: ```python - from fastapi import FastAPI + from fastapi import Depends, FastAPI from fastapi_cachex import add_routes app = FastAPI() - add_routes(app) # Routes at /cached-hits and /cached-records - - # Or with a prefix: /api/cache/cached-hits and /api/cache/cached-records - add_routes(app, prefix="/api/cache") + # Routes at /api/cache/cached-hits and /api/cache/cached-records, + # guarded by your own dependency + add_routes(app, prefix="/api/cache", dependencies=[Depends(verify_admin)]) ``` """ + if dependencies is None: + warnings.warn( + "add_routes() is mounting the cache monitoring routes without access " + "control: anyone who can reach the app can read every cached key " + "(including query strings) and previews of the cached responses. " + "Pass dependencies=[Depends(your_auth)] to guard them, or " + "dependencies=[] to opt out deliberately. Version 0.4.0 will require " + "dependencies and turn include_content_preview off by default " + "(https://github.com/allen0099/FastAPI-CacheX/issues/298).", + UserWarning, + stacklevel=2, + ) @app.get( f"{prefix}/cached-hits", diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index 2a7c680..f67498c 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -267,6 +267,8 @@ add_routes( > [!WARNING] > **這些路由本身沒有任何身分驗證。** `include_in_schema=False` 只是讓它們不出現在 OpenAPI 文件中；任何猜到路徑的人都能讀取。`/cached-records` 含有快取內容的預覽（除非設定 `include_content_preview=False`），並會暴露整個路由結構。正式環境中請務必傳入 `dependencies=[Depends(your_auth)]`，或將它們掛載在僅供內部使用的應用程式上。 +> +> 呼叫 `add_routes()` 時若未傳入 `dependencies`，會發出 `UserWarning`。0.4.0 版將要求必須傳入此參數，並將 `include_content_preview` 預設改為關閉（[#298](https://github.com/allen0099/FastAPI-CacheX/issues/298)）。若本機或測試用的應用程式確實要保持開放，請傳入 `dependencies=[]` 明確選擇不設防護，這樣就不會出現警告。 > [!NOTE] > Memcached 無法列舉鍵，因此在 Memcached 上這兩個路由都不會回傳任何內容。 diff --git a/tests/test_routes.py b/tests/test_routes.py index 87caa6c..935dd26 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -1,6 +1,7 @@ """Tests for cache monitoring routes.""" import time +import warnings import pytest from fastapi import FastAPI @@ -43,7 +44,7 @@ class TestCachedHitsRoute: def test_cached_hits_without_backend(self, app, client): """Test /cached-hits returns empty when backend not configured.""" - add_routes(app) + add_routes(app, dependencies=[]) response = client.get("/cached-hits") assert response.status_code == 200 @@ -55,7 +56,7 @@ def test_cached_hits_without_backend(self, app, client): def test_cached_hits_empty_cache(self, app, client, setup_cache): """Test /cached-hits returns empty structure when cache is empty.""" - add_routes(app) + add_routes(app, dependencies=[]) response = client.get("/cached-hits") assert response.status_code == 200 @@ -67,7 +68,7 @@ def test_cached_hits_empty_cache(self, app, client, setup_cache): def test_cached_hits_with_entries(self, app, client, setup_cache): """Test /cached-hits returns cached entries when routes are cached.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/users") @cache(ttl=60) @@ -110,7 +111,7 @@ async def get_products(): def test_cached_hits_route_structure(self, app, client, setup_cache): """Test that cached hit records have correct structure.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/test") @cache(ttl=60) @@ -146,7 +147,7 @@ async def test_endpoint(): def test_cached_hits_with_prefix(self, app, client, setup_cache): """Test /cached-hits route with custom prefix.""" - add_routes(app, prefix="/admin/cache") + add_routes(app, prefix="/admin/cache", dependencies=[]) @app.get("/test") @cache(ttl=60) @@ -162,7 +163,7 @@ async def test_endpoint(): def test_cached_hits_multiple_query_variations(self, app, client, setup_cache): """Test /cached-hits shows different cache keys for query params.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/items") # type: ignore[untyped-decorator] @cache(ttl=60) @@ -192,7 +193,7 @@ class TestCachedRecordsRoute: def test_cached_records_without_backend(self, app, client): """Test /cached-records returns empty when backend not configured.""" - add_routes(app) + add_routes(app, dependencies=[]) response = client.get("/cached-records") assert response.status_code == 200 @@ -203,7 +204,7 @@ def test_cached_records_without_backend(self, app, client): def test_cached_records_empty_cache(self, app, client, setup_cache): """Test /cached-records returns empty structure when cache is empty.""" - add_routes(app) + add_routes(app, dependencies=[]) response = client.get("/cached-records") assert response.status_code == 200 @@ -215,7 +216,7 @@ def test_cached_records_empty_cache(self, app, client, setup_cache): def test_cached_records_with_entries(self, app, client, setup_cache): """Test /cached-records returns cached entries with content info.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/users") @cache(ttl=60) @@ -248,7 +249,7 @@ async def get_products(): def test_cached_records_structure(self, app, client, setup_cache): """Test that cached records have correct structure.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/test") @cache(ttl=60) @@ -286,7 +287,7 @@ async def test_endpoint(): def test_cached_records_reports_media_type(self, app, client, setup_cache): """``media_type`` is the stored response's, ``content_type`` stays "bytes".""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/json") @cache(ttl=60) @@ -310,7 +311,7 @@ async def text_endpoint(): def test_cached_records_media_type_null_when_unset(self, app, client, setup_cache): """An entry stored without a media type reports ``null``.""" - add_routes(app) + add_routes(app, dependencies=[]) setup_cache.cache["GET|||h|||/raw|||"] = CacheItem( value=CacheEntry(fingerprint="e", content=b"x"), expiry=None ) @@ -320,7 +321,7 @@ def test_cached_records_media_type_null_when_unset(self, app, client, setup_cach def test_cached_records_content_size_calculation(self, app, client, setup_cache): """Test that content size is calculated correctly.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/small") @cache(ttl=60) @@ -351,7 +352,7 @@ async def large_endpoint(): def test_cached_records_with_prefix(self, app, client, setup_cache): """Test /cached-records route with custom prefix.""" - add_routes(app, prefix="/api/cache") + add_routes(app, prefix="/api/cache", dependencies=[]) @app.get("/test") @cache(ttl=60) @@ -367,7 +368,7 @@ async def test_endpoint(): def test_cached_records_content_preview(self, app, client, setup_cache): """Test that content preview is limited to 100 bytes.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/large") @cache(ttl=60) @@ -388,7 +389,7 @@ async def large_endpoint(): def test_cached_records_can_omit_content_preview(self, app, client, setup_cache): """include_content_preview=False hides bodies but keeps the metadata.""" - add_routes(app, include_content_preview=False) + add_routes(app, include_content_preview=False, dependencies=[]) @app.get("/api/secret") @cache(ttl=60) @@ -411,7 +412,7 @@ async def get_secret(): def test_cached_records_summary_calculations(self, app, client, setup_cache): """Test that summary calculations are correct.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/test1") @cache(ttl=60) @@ -442,7 +443,7 @@ class TestRoutesIntegration: def test_routes_without_prefix(self, app, client, setup_cache): """Test that routes work without prefix.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/test") @cache(ttl=60) @@ -459,7 +460,7 @@ async def test_endpoint(): def test_routes_consistency(self, app, client, setup_cache): """Test that both routes show consistent data.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/consistent") @cache(ttl=60) @@ -485,7 +486,7 @@ async def consistent_endpoint(): def test_routes_not_cached_by_default(self, app, client, setup_cache): """Test that the monitoring routes themselves are not cached.""" - add_routes(app) + add_routes(app, dependencies=[]) @app.get("/api/test") @cache(ttl=60) @@ -504,7 +505,7 @@ async def test_endpoint(): def test_include_in_schema_parameter(self, app): """Test that include_in_schema parameter works.""" - add_routes(app, include_in_schema=True) + add_routes(app, include_in_schema=True, dependencies=[]) # Check if routes are included in OpenAPI schema openapi_schema = app.openapi() @@ -541,13 +542,52 @@ def require_api_key(x_api_key: str | None = Header(default=None)) -> None: r2 = dep_client.get("/cached-hits", headers={"x-api-key": "secret"}) assert r2.status_code == 200 - def test_add_routes_with_none_dependencies_no_error(self, app, client, setup_cache): - """Passing dependencies=None (default) must not raise errors.""" - add_routes(app, dependencies=None) + def test_add_routes_with_none_dependencies_warns_and_mounts( + self, app, client, setup_cache + ): + """dependencies=None warns but still mounts the routes unguarded.""" + with pytest.warns(UserWarning, match="dependencies"): + add_routes(app, dependencies=None) response = client.get("/cached-hits") assert response.status_code == 200 +class TestUnguardedWarning: + """add_routes() warns when mounted without dependencies (#301).""" + + def test_default_warns(self, app): + """Leaving dependencies unset emits a UserWarning naming it.""" + with pytest.warns(UserWarning, match="without access control") as record: + add_routes(app) + + assert len(record) == 1 + message = str(record[0].message) + assert "dependencies" in message + assert "dependencies=[]" in message + assert "0.4.0" in message + assert "include_content_preview" in message + assert "298" in message + # stacklevel points at the caller, not at routes.py + assert record[0].filename == __file__ + + def test_empty_dependencies_does_not_warn(self, app): + """An explicit dependencies=[] is a deliberate opt-out.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + add_routes(app, dependencies=[]) + + def test_guarded_does_not_warn(self, app): + """Passing a real guard does not warn.""" + from fastapi import Depends + + def guard() -> None: + return None + + with warnings.catch_warnings(): + warnings.simplefilter("error") + add_routes(app, dependencies=[Depends(guard)]) + + def _report_expired(backend: MemoryBackend, key: str, entry: CacheEntry) -> None: """Make ``get_cache_data`` return an entry whose expiry has passed. @@ -567,7 +607,7 @@ class TestExpiredEntryMonitoring: def test_cached_hits_shows_expired_entry(self, app, client, setup_cache): """/cached-hits marks is_expired=True for entries whose TTL has passed.""" - add_routes(app) + add_routes(app, dependencies=[]) # TestClient sends Host: testserver by default cache_key = "GET|||testserver|||/expired-route|||" @@ -589,7 +629,7 @@ def test_cached_hits_shows_expired_entry(self, app, client, setup_cache): def test_cached_records_shows_expired_entry(self, app, client, setup_cache): """/cached-records marks is_expired=True for entries whose TTL has passed.""" - add_routes(app) + add_routes(app, dependencies=[]) cache_key = "GET|||testserver|||/expired-data|||" expired_entry = CacheEntry( @@ -614,7 +654,7 @@ class TestMonitoringEdgeCases: def test_non_route_keys_are_skipped(self, app, client, setup_cache): """A CacheManager/state key has no method|||host|||path shape and must not be listed.""" - add_routes(app) + add_routes(app, dependencies=[]) setup_cache.cache["cache:plain-value"] = CacheItem( value=CacheEntry(fingerprint="x", content=b"1"), expiry=None ) @@ -629,7 +669,7 @@ def test_non_route_keys_are_skipped(self, app, client, setup_cache): assert [r["path"] for r in records["cached_records"]] == ["/route"] def test_routes_answer_empty_when_no_backend_is_configured(self, app, client): - add_routes(app) + add_routes(app, dependencies=[]) BackendProxy.set(None) hits = client.get("/cached-hits").json()