Merge pull request #2043 from harshmathurx/fix/2040-redis-auth-hello

fix(docker): authenticate rate limit Redis storage

Author: ntohidi

deploy/docker/redis_config.py

@@ -0,0 +1,75 @@
+"""Redis URL helpers for the Docker API server.
+
+The Docker entrypoint protects the in-container Redis with a password. These
+helpers keep every Redis client (the app and optional rate-limit storage) using
+the same authenticated URL shape.
+"""
+
+import os
+from urllib.parse import quote, urlsplit, urlunsplit
+
+
+def redis_auth_from_config(config: dict) -> tuple[str, str]:
+    """Return Redis ACL username/password from config and environment."""
+    rc = config.get("redis", {})
+    username = os.environ.get("REDIS_USERNAME", rc.get("username", "default")) or "default"
+    password = os.environ.get("REDIS_PASSWORD", rc.get("password", "")) or ""
+    return str(username), str(password)
+
+
+def redis_auth_netloc(username: str, password: str) -> str:
+    """Build a URL-safe Redis AUTH fragment for redis-py/limits clients.
+
+    Redis 6+ HELLO AUTH requires both username and password when negotiating
+    RESP3. Supplying the explicit default ACL username avoids clients issuing an
+    unauthenticated HELLO against a password-protected Redis instance.
+    """
+    if not password:
+        return ""
+    return f"{quote(username, safe='')}:{quote(password, safe='')}@"
+
+
+def build_redis_url(config: dict) -> str:
+    """Build Redis URL from config fields and environment variables."""
+    rc = config.get("redis", {})
+    host = os.environ.get("REDIS_HOST", rc.get("host", "localhost"))
+    port = os.environ.get("REDIS_PORT", rc.get("port", 6379))
+    username, password = redis_auth_from_config(config)
+    db = rc.get("db", 0)
+    scheme = "rediss" if rc.get("ssl", False) else "redis"
+    auth = redis_auth_netloc(username, password)
+    return f"{scheme}://{auth}{host}:{port}/{db}"
+
+
+def build_rate_limit_storage_uri(config: dict) -> str:
+    """Return a rate-limit storage URI that can auth to protected Redis.
+
+    Older/self-managed Docker configs may set rate_limiting.storage_uri to an
+    unauthenticated Redis URL such as redis://localhost:6379. Since the Docker
+    entrypoint now always protects the in-container Redis with REDIS_PASSWORD,
+    SlowAPI/limits would fail before route handlers with Redis HELLO auth errors.
+    If the storage URI is Redis-like and has no credentials, reuse the configured
+    Redis ACL credentials. Explicit credentials and non-Redis backends are left
+    unchanged.
+    """
+    storage_uri = config.get("rate_limiting", {}).get("storage_uri", "memory://")
+    if not isinstance(storage_uri, str):
+        return "memory://"
+    parsed = urlsplit(storage_uri)
+    if parsed.scheme not in {"redis", "rediss", "redis+sentinel"}:
+        return storage_uri
+    if parsed.username or parsed.password:
+        return storage_uri
+
+    username, password = redis_auth_from_config(config)
+    if not password:
+        return storage_uri
+
+    auth = redis_auth_netloc(username, password)
+    return urlunsplit((
+        parsed.scheme,
+        f"{auth}{parsed.netloc}",
+        parsed.path,
+        parsed.query,
+        parsed.fragment,
+    ))

deploy/docker/server.py

@@ -72,6 +72,10 @@
 from slowapi.util import get_remote_address
 from prometheus_fastapi_instrumentator import Instrumentator
 from redis import asyncio as aioredis
+from redis_config import (
+    build_rate_limit_storage_uri as _build_rate_limit_storage_uri,
+    build_redis_url as _build_redis_url,
+)
 
 # ── internal imports (after sys.path append) ─────────────────
 sys.path.append(os.path.dirname(os.path.realpath(__file__)))
@@ -289,23 +293,12 @@ async def root():
     return RedirectResponse("/playground")
 
 # ─────────────────── infra / middleware  ─────────────────────
-def _build_redis_url(config: dict) -> str:
-    """Build Redis URL from config fields and environment variables."""
-    rc = config.get("redis", {})
-    host = os.environ.get("REDIS_HOST", rc.get("host", "localhost"))
-    port = os.environ.get("REDIS_PORT", rc.get("port", 6379))
-    password = os.environ.get("REDIS_PASSWORD", rc.get("password", ""))
-    db = rc.get("db", 0)
-    scheme = "rediss" if rc.get("ssl", False) else "redis"
-    auth = f":{password}@" if password else ""
-    return f"{scheme}://{auth}{host}:{port}/{db}"
-
 redis = aioredis.from_url(_build_redis_url(config))
 
 limiter = Limiter(
     key_func=get_remote_address,
     default_limits=[config["rate_limiting"]["default_limit"]],
-    storage_uri=config["rate_limiting"]["storage_uri"],
+    storage_uri=_build_rate_limit_storage_uri(config),
 )
 
 

deploy/docker/tests/test_security_default_posture.py

@@ -25,6 +25,8 @@
 
 import pytest
 
+from redis_config import build_rate_limit_storage_uri, build_redis_url
+
 pytestmark = pytest.mark.posture
 
 HEALTH = "/health"
@@ -175,6 +177,52 @@ def test_redis_is_not_network_exposed(self, effective_redis_url, effective_confi
             "redis is neither loopback-bound nor password-protected"
         )
 
+    def test_redis_url_includes_acl_username_and_encoded_password(self, monkeypatch):
+        """Redis URLs must authenticate during HELLO when Redis is password-protected."""
+        monkeypatch.setenv("REDIS_PASSWORD", "p@ss/w:rd")
+        cfg = {
+            "redis": {
+                "host": "localhost",
+                "port": 6379,
+                "db": 0,
+            }
+        }
+        url = build_redis_url(cfg)
+        assert url.startswith("redis://default:")
+        assert "p%40ss%2Fw%3Ard" in url
+        assert url.endswith("@localhost:6379/0")
+
+    def test_rate_limit_redis_storage_reuses_redis_password(self, monkeypatch):
+        """SlowAPI must not connect to protected Redis without credentials."""
+        monkeypatch.setenv("REDIS_PASSWORD", "abc123")
+        cfg = {
+            "redis": {"host": "localhost", "port": 6379, "db": 0},
+            "rate_limiting": {"storage_uri": "redis://localhost:6379/0"},
+        }
+        url = build_rate_limit_storage_uri(cfg)
+        assert url.startswith("redis://default:")
+        assert "abc123" in url
+        assert url.endswith("@localhost:6379/0")
+
+    def test_rate_limit_storage_preserves_explicit_auth(self, monkeypatch):
+        """Do not rewrite operator-provided rate-limit Redis credentials."""
+        monkeypatch.setenv("REDIS_PASSWORD", "container-secret")
+        storage_uri = "redis://" + "alice" + ":" + "custom" + "@redis:6379/2"
+        cfg = {
+            "redis": {"host": "localhost", "port": 6379, "db": 0},
+            "rate_limiting": {"storage_uri": storage_uri},
+        }
+        assert build_rate_limit_storage_uri(cfg) == storage_uri
+
+    def test_rate_limit_storage_keeps_memory_backend(self, monkeypatch):
+        """Non-Redis rate-limit backends must not be rewritten."""
+        monkeypatch.setenv("REDIS_PASSWORD", "abc123")
+        cfg = {
+            "redis": {"host": "localhost", "port": 6379, "db": 0},
+            "rate_limiting": {"storage_uri": "memory://"},
+        }
+        assert build_rate_limit_storage_uri(cfg) == "memory://"
+
     def test_trusted_hosts_not_wildcard_when_exposed(self, effective_config):
         """A wildcard trusted_hosts on a non-loopback bind silently disables the host guard."""
         host = effective_config["app"]["host"]