From 14101a557200b9dc4681a8f213d10b51fed603b4 Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Sat, 16 May 2026 03:31:59 +0530 Subject: [PATCH 1/8] fix: set secure cookie default to True and restrict proxy trusted hosts - Change SESSION_COOKIE_SECURE default from False to True so session cookies require HTTPS by default. Developers on local HTTP must set SESSION_COOKIE_SECURE=false in their .env (documented in .env.example). - Replace ProxyHeadersMiddleware trusted_hosts=["*"] with an env- configurable list (TRUSTED_PROXY_IPS, defaults to 127.0.0.1) to prevent IP spoofing via forged X-Forwarded-For headers. - Add Security section to .env.example documenting both settings. Fix issue #498 --- .env.example | 9 +++++++++ finbot/config.py | 2 +- finbot/main.py | 6 +++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.env.example b/.env.example index 31526784..567208ba 100644 --- a/.env.example +++ b/.env.example @@ -25,3 +25,12 @@ RESEND_API_KEY=resend_api_key_here # ── Magic Link ─────────────────────────────────────────────────────── MAGIC_LINK_BASE_URL=http://localhost:8000 + +# ── Security ───────────────────────────────────────────────────────── +# Session cookie Secure flag — must be false for local http://localhost dev. +# Always leave as true (or unset) for any https deployment. +SESSION_COOKIE_SECURE=false + +# Comma-separated IPs of trusted reverse proxies (Railway, nginx, etc.) +# The app only reads X-Forwarded-For from these IPs to prevent IP spoofing. +# TRUSTED_PROXY_IPS=127.0.0.1 diff --git a/finbot/config.py b/finbot/config.py index df362f5c..9e68f833 100644 --- a/finbot/config.py +++ b/finbot/config.py @@ -77,7 +77,7 @@ class Settings(BaseSettings): # Cookie Config SESSION_COOKIE_NAME: str = "finbot_session" - SESSION_COOKIE_SECURE: bool = False # Set to True in production with https + SESSION_COOKIE_SECURE: bool = True # Set to False in .env for local HTTP dev only SESSION_COOKIE_HTTP_ONLY: bool = True # Always HTTP-only for security SESSION_COOKIE_SAMESITE: str = "Lax" diff --git a/finbot/main.py b/finbot/main.py index 8cd4f1a2..c03523da 100644 --- a/finbot/main.py +++ b/finbot/main.py @@ -145,11 +145,15 @@ async def lifespan(app: FastAPI): # Trust X-Forwarded-Proto/For from reverse proxies (Railway, etc.) # so url_for() generates https:// URLs and client IPs are correct. +import os # noqa: E402 — placed here to stay near usage from uvicorn.middleware.proxy_headers import ( ProxyHeadersMiddleware, # pylint: disable=ungrouped-imports ) -app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=["*"]) +# Trust X-Forwarded-For only from known reverse proxy IPs. +# Use TRUSTED_PROXY_IPS env var (comma-separated) for production deployments. +_trusted_proxies = [h.strip() for h in os.getenv("TRUSTED_PROXY_IPS", "127.0.0.1").split(",")] +app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=_trusted_proxies) # Register error handlers register_error_handlers(app) From 8f5916f04c55a7a827425124b72e5ea4e79b0e42 Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Tue, 19 May 2026 12:48:31 +0530 Subject: [PATCH 2/8] fix(security): revert proxy trusted hosts for PaaS support Reverts the ProxyHeadersMiddleware trusted_hosts configuration back to ["*"] to prevent logging failures and false positives on PaaS deployments (like Railway/Fly) that use dynamic internal proxies. Adds documentation clarifying that edge proxies MUST strip inbound X-Forwarded-For headers to prevent spoofing. --- .env.example | 6 +++--- finbot/main.py | 6 +----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.env.example b/.env.example index 567208ba..d52fe516 100644 --- a/.env.example +++ b/.env.example @@ -31,6 +31,6 @@ MAGIC_LINK_BASE_URL=http://localhost:8000 # Always leave as true (or unset) for any https deployment. SESSION_COOKIE_SECURE=false -# Comma-separated IPs of trusted reverse proxies (Railway, nginx, etc.) -# The app only reads X-Forwarded-For from these IPs to prevent IP spoofing. -# TRUSTED_PROXY_IPS=127.0.0.1 +# NOTE: The application trusts all upstream proxies by default (trusted_hosts=["*"]) +# to support zero-config PaaS deployments. To prevent IP spoofing, your edge proxy +# or WAF *must* be configured to strip inbound X-Forwarded-For headers from clients. diff --git a/finbot/main.py b/finbot/main.py index c03523da..8cd4f1a2 100644 --- a/finbot/main.py +++ b/finbot/main.py @@ -145,15 +145,11 @@ async def lifespan(app: FastAPI): # Trust X-Forwarded-Proto/For from reverse proxies (Railway, etc.) # so url_for() generates https:// URLs and client IPs are correct. -import os # noqa: E402 — placed here to stay near usage from uvicorn.middleware.proxy_headers import ( ProxyHeadersMiddleware, # pylint: disable=ungrouped-imports ) -# Trust X-Forwarded-For only from known reverse proxy IPs. -# Use TRUSTED_PROXY_IPS env var (comma-separated) for production deployments. -_trusted_proxies = [h.strip() for h in os.getenv("TRUSTED_PROXY_IPS", "127.0.0.1").split(",")] -app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=_trusted_proxies) +app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=["*"]) # Register error handlers register_error_handlers(app) From d93e959e8f451aa42cf27bf48db67dc0743a1f0f Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Tue, 19 May 2026 12:50:19 +0530 Subject: [PATCH 3/8] feat(security): make TRUSTED_PROXY_IPS an opt-in env var for strict hijack heuristics Per reviewer feedback, re-introduced TRUSTED_PROXY_IPS as an unset default. When unset, ProxyHeadersMiddleware trusts ["*"] (ensuring PaaS compatibility) and IP-based session hijack logs are downgraded to debug since the IPs are untrusted. When set, strict IP spoofing checks and INFO-level hijack heuristics are enabled. Also added a startup warning when it is unset to clearly communicate the degraded security state. --- .env.example | 4 ++++ finbot/config.py | 1 + finbot/core/auth/session.py | 20 ++++++++++++++------ finbot/main.py | 11 ++++++++++- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/.env.example b/.env.example index d52fe516..e96ed62c 100644 --- a/.env.example +++ b/.env.example @@ -34,3 +34,7 @@ SESSION_COOKIE_SECURE=false # NOTE: The application trusts all upstream proxies by default (trusted_hosts=["*"]) # to support zero-config PaaS deployments. To prevent IP spoofing, your edge proxy # or WAF *must* be configured to strip inbound X-Forwarded-For headers from clients. +# +# Opt-in: Set TRUSTED_PROXY_IPS to a comma-separated list of your proxy IPs (e.g., 10.0.0.0/8) +# to enable IP-based hijack detection in the application layer. +# TRUSTED_PROXY_IPS= diff --git a/finbot/config.py b/finbot/config.py index 9e68f833..220f72b7 100644 --- a/finbot/config.py +++ b/finbot/config.py @@ -53,6 +53,7 @@ class Settings(BaseSettings): # Security Config SECRET_KEY: str = DEFAULT_SECRET_KEY SESSION_SIGNING_KEY: str | None = None # Derived from SECRET_KEY + TRUSTED_PROXY_IPS: str | None = None # Session Config TEMP_SESSION_TIMEOUT: int = 86400 * 7 # 7 days diff --git a/finbot/core/auth/session.py b/finbot/core/auth/session.py index ce08f070..273af208 100644 --- a/finbot/core/auth/session.py +++ b/finbot/core/auth/session.py @@ -448,12 +448,20 @@ def get_session( # IP address monitoring (logging only, not strict validation) if current_ip and session_context.original_ip: if current_ip != session_context.original_ip: - logger.info( - "IP change detected for session %s: %s -> %s", - session_id[:8], - session_context.original_ip, - current_ip, - ) + if settings.TRUSTED_PROXY_IPS: + logger.info( + "IP change detected for session %s: %s -> %s", + session_id[:8], + session_context.original_ip, + current_ip, + ) + else: + logger.debug( + "Untrusted IP change detected for session %s (TRUSTED_PROXY_IPS unset): %s -> %s", + session_id[:8], + session_context.original_ip, + current_ip, + ) # Tiered fingerprint validation if settings.ENABLE_FINGERPRINT_VALIDATION: diff --git a/finbot/main.py b/finbot/main.py index 8cd4f1a2..5e1e191f 100644 --- a/finbot/main.py +++ b/finbot/main.py @@ -149,7 +149,16 @@ async def lifespan(app: FastAPI): ProxyHeadersMiddleware, # pylint: disable=ungrouped-imports ) -app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=["*"]) +if settings.TRUSTED_PROXY_IPS: + _trusted_proxies = [h.strip() for h in settings.TRUSTED_PROXY_IPS.split(",")] + app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=_trusted_proxies) +else: + import logging + logging.getLogger("uvicorn").warning( + "TRUSTED_PROXY_IPS is unset. Client IPs are untrusted and IP-based hijack detection is disabled. " + "Ensure your edge proxy strips inbound X-Forwarded-For headers." + ) + app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=["*"]) # Register error handlers register_error_handlers(app) From 5e8b5e6882edb838f5b9e70a08c08955dadb3e2a Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Tue, 19 May 2026 12:55:06 +0530 Subject: [PATCH 4/8] fix(security): improve untrusted IP log visibility and safe defaults Per reviewer feedback: 1) Downgraded untrusted IP changes back to INFO but flagged them as 'trusted_source=false' to maintain greppability without triggering incident alerts. 2) Commented out SESSION_COOKIE_SECURE in .env.example so production copy-pastes are secure-by-default. --- .env.example | 4 ++-- finbot/core/auth/session.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index e96ed62c..a9d676e2 100644 --- a/.env.example +++ b/.env.example @@ -28,8 +28,8 @@ MAGIC_LINK_BASE_URL=http://localhost:8000 # ── Security ───────────────────────────────────────────────────────── # Session cookie Secure flag — must be false for local http://localhost dev. -# Always leave as true (or unset) for any https deployment. -SESSION_COOKIE_SECURE=false +# For production https deployments, leave this commented out (defaults to true). +# SESSION_COOKIE_SECURE=false # NOTE: The application trusts all upstream proxies by default (trusted_hosts=["*"]) # to support zero-config PaaS deployments. To prevent IP spoofing, your edge proxy diff --git a/finbot/core/auth/session.py b/finbot/core/auth/session.py index 273af208..a19c211a 100644 --- a/finbot/core/auth/session.py +++ b/finbot/core/auth/session.py @@ -456,8 +456,8 @@ def get_session( current_ip, ) else: - logger.debug( - "Untrusted IP change detected for session %s (TRUSTED_PROXY_IPS unset): %s -> %s", + logger.info( + "IP change observed (trusted_source=false) for session %s: %s -> %s", session_id[:8], session_context.original_ip, current_ip, From 2effd4efab020fc26a40028d3ac63b32c19d3171 Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Tue, 19 May 2026 13:09:54 +0530 Subject: [PATCH 5/8] test(security): add test coverage for proxy startup warnings and session IP heuristics Per reviewer feedback: 1) Updated session IP heuristic log to use a structured extra={'trusted_source': False} argument instead of a string tag for better downstream parsing. 2) Added integration tests for the uvicorn startup warning and the IP change fallback logic. --- finbot/core/auth/session.py | 4 +- tests/unit/auth/test_session_ip_logging.py | 42 +++++++++++++++++++ .../core/test_proxy_middleware_startup.py | 35 ++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 tests/unit/auth/test_session_ip_logging.py create mode 100644 tests/unit/core/test_proxy_middleware_startup.py diff --git a/finbot/core/auth/session.py b/finbot/core/auth/session.py index a19c211a..8bd090cb 100644 --- a/finbot/core/auth/session.py +++ b/finbot/core/auth/session.py @@ -454,13 +454,15 @@ def get_session( session_id[:8], session_context.original_ip, current_ip, + extra={"trusted_source": True} ) else: logger.info( - "IP change observed (trusted_source=false) for session %s: %s -> %s", + "IP change observed for session %s: %s -> %s", session_id[:8], session_context.original_ip, current_ip, + extra={"trusted_source": False} ) # Tiered fingerprint validation diff --git a/tests/unit/auth/test_session_ip_logging.py b/tests/unit/auth/test_session_ip_logging.py new file mode 100644 index 00000000..e1c26054 --- /dev/null +++ b/tests/unit/auth/test_session_ip_logging.py @@ -0,0 +1,42 @@ +import pytest +import logging +from unittest.mock import patch +from finbot.core.auth.session import SessionManager + +@pytest.fixture +def session_manager(): + return SessionManager() + +@pytest.mark.parametrize("trusted_proxy_ips, expected_trusted_source", [ + (None, False), + ("10.0.0.1,127.0.0.1", True) +]) +def test_session_ip_change_logging( + session_manager, + trusted_proxy_ips, + expected_trusted_source, + caplog, + db +): + """ + Assert that IP-change heuristics correctly add structured `trusted_source` flags + based on the TRUSTED_PROXY_IPS configuration. + """ + with patch("finbot.core.auth.session.settings.TRUSTED_PROXY_IPS", trusted_proxy_ips): + with caplog.at_level(logging.INFO): + # Create a session with an initial IP + session_context = session_manager.create_session( + ip_address="192.168.1.1" + ) + + # Retrieve session with a different IP to trigger the heuristic + session_manager.get_session( + session_id=session_context.session_id, + current_ip="192.168.1.2", + _db=db + ) + + # Check log records + log_records = [r for r in caplog.records if "IP change" in r.getMessage()] + assert len(log_records) == 1 + assert getattr(log_records[0], "trusted_source") == expected_trusted_source diff --git a/tests/unit/core/test_proxy_middleware_startup.py b/tests/unit/core/test_proxy_middleware_startup.py new file mode 100644 index 00000000..0c5f319c --- /dev/null +++ b/tests/unit/core/test_proxy_middleware_startup.py @@ -0,0 +1,35 @@ +import pytest +import logging +import importlib +from unittest.mock import patch + +def test_proxy_middleware_startup_warning_when_unset(): + """ + Assert that the startup warning fires when TRUSTED_PROXY_IPS is unset. + """ + with patch("finbot.main.settings.TRUSTED_PROXY_IPS", None): + with patch("logging.Logger.warning") as mock_warning: + import finbot.main + importlib.reload(finbot.main) + + # Check if warning was called with the correct message + called_with_message = any( + "TRUSTED_PROXY_IPS is unset" in call_args[0][0] + for call_args in mock_warning.call_args_list + ) + assert called_with_message + +def test_proxy_middleware_startup_no_warning_when_set(): + """ + Assert that the startup warning does NOT fire when TRUSTED_PROXY_IPS is set. + """ + with patch("finbot.main.settings.TRUSTED_PROXY_IPS", "10.0.0.1"): + with patch("logging.Logger.warning") as mock_warning: + import finbot.main + importlib.reload(finbot.main) + + called_with_message = any( + "TRUSTED_PROXY_IPS is unset" in call_args[0][0] + for call_args in mock_warning.call_args_list + ) + assert not called_with_message From fcbb1d53a5cf6d5f0ce348f04e4ba34faac45ea2 Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Tue, 19 May 2026 13:16:09 +0530 Subject: [PATCH 6/8] fix(security): preserve structured logs and add rigorous security assertions Per reviewer feedback: 1) Added a custom ExtraFormatter to the standard library logging config so that `extra={}` fields actually survive in plain text standard output. 2) Added a test explicitly wiping the environment to prove SESSION_COOKIE_SECURE safely defaults to True in code. 3) Added a test asserting that ProxyHeadersMiddleware successfully mitigates IP spoofing from untrusted peers. --- finbot/logging_config.py | 25 +++++++++++- tests/unit/core/test_config_defaults.py | 17 +++++++++ .../core/test_proxy_middleware_behavior.py | 38 +++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 tests/unit/core/test_config_defaults.py create mode 100644 tests/unit/core/test_proxy_middleware_behavior.py diff --git a/finbot/logging_config.py b/finbot/logging_config.py index a804df69..deb46ead 100644 --- a/finbot/logging_config.py +++ b/finbot/logging_config.py @@ -37,7 +37,30 @@ def setup_logging(log_level: str | None = None) -> None: console_handler = logging.StreamHandler(sys.stdout) console_handler.setLevel(numeric_level) - formatter = logging.Formatter( + class ExtraFormatter(logging.Formatter): + def format(self, record): + # Format the base message + s = super().format(record) + + # Extract standard record attributes so we know what is "extra" + standard_attrs = { + 'name', 'msg', 'args', 'levelname', 'levelno', 'pathname', 'filename', + 'module', 'exc_info', 'exc_text', 'stack_info', 'lineno', 'funcName', + 'created', 'msecs', 'relativeCreated', 'thread', 'threadName', + 'processName', 'process', 'message', 'asctime', 'taskName' + } + + # Find any extra attributes added via logger.info(..., extra={"key": "val"}) + extras = {k: v for k, v in record.__dict__.items() if k not in standard_attrs} + + if extras: + # Append them in a structured way + extra_str = " ".join(f"{k}={v}" for k, v in extras.items()) + s += f" | {extra_str}" + + return s + + formatter = ExtraFormatter( fmt="%(asctime)s - %(levelname)s - %(name)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S", ) diff --git a/tests/unit/core/test_config_defaults.py b/tests/unit/core/test_config_defaults.py new file mode 100644 index 00000000..611f9c39 --- /dev/null +++ b/tests/unit/core/test_config_defaults.py @@ -0,0 +1,17 @@ +import pytest +from unittest.mock import patch +from finbot.config import Settings + +def test_session_cookie_secure_default_when_env_missing(monkeypatch): + """ + Assert that when SESSION_COOKIE_SECURE is completely absent from the environment, + the code-level default firmly evaluates to True. + """ + # Ensure it's totally removed from the environment + monkeypatch.delenv("SESSION_COOKIE_SECURE", raising=False) + + # Instantiate Settings directly (bypassing any cached singletons). + # Pass _env_file=None to ensure Pydantic doesn't read a stray .env file in the test dir. + test_settings = Settings(_env_file=None) + + assert test_settings.SESSION_COOKIE_SECURE is True diff --git a/tests/unit/core/test_proxy_middleware_behavior.py b/tests/unit/core/test_proxy_middleware_behavior.py new file mode 100644 index 00000000..9e06197b --- /dev/null +++ b/tests/unit/core/test_proxy_middleware_behavior.py @@ -0,0 +1,38 @@ +import pytest +from fastapi import FastAPI, Request +from fastapi.testclient import TestClient +from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware + +def create_test_app(trusted_hosts): + app = FastAPI() + + @app.get("/ip") + def get_ip(request: Request): + return {"client_ip": request.client.host} + + app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=trusted_hosts) + return app + +@pytest.mark.parametrize("trusted_hosts, peer_ip, x_forwarded_for, expected_ip", [ + # When unset, we pass ["*"], so ANY peer is trusted to provide X-Forwarded-For + (["*"], "192.168.1.100", "8.8.8.8", "8.8.8.8"), + + # When set to specific IPs, only those peers can provide X-Forwarded-For + (["10.0.0.1"], "10.0.0.1", "8.8.8.8", "8.8.8.8"), + + # Negative Branch: A peer NOT in the trusted list tries to spoof IP + (["10.0.0.1"], "192.168.1.100", "8.8.8.8", "192.168.1.100"), +]) +def test_proxy_middleware_spoofing_behavior(trusted_hosts, peer_ip, x_forwarded_for, expected_ip): + """ + Assert that X-Forwarded-For from a non-listed peer doesn't influence the application's + perceived client_ip when strict trusted hosts are configured. + """ + app = create_test_app(trusted_hosts) + + # We must explicitly set the client IP on the TestClient to simulate the physical peer IP + # that Uvicorn would see on the TCP socket. + with TestClient(app, client=(peer_ip, 12345)) as client: + response = client.get("/ip", headers={"X-Forwarded-For": x_forwarded_for}) + assert response.status_code == 200 + assert response.json()["client_ip"] == expected_ip From f677d7951ec3cbc582831419e2885ca35a3a1675 Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Tue, 19 May 2026 13:21:28 +0530 Subject: [PATCH 7/8] test(security): harden logging format and behavior tests Per reviewer feedback: 1) Updated config default test to chdir(tmp_path) to strictly prevent Pydantic from resolving stray .env files. 2) Updated structured logging fields to use the 'app.' prefix (e.g. app.trusted_source) to avoid stdlib logging attribute collisions. 3) Added a test for the 'chained header' attack simulating an edge proxy appending to a forged X-Forwarded-For. --- finbot/core/auth/session.py | 4 ++-- tests/unit/auth/test_session_ip_logging.py | 2 +- tests/unit/core/test_config_defaults.py | 5 ++++- tests/unit/core/test_proxy_middleware_behavior.py | 7 +++++++ 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/finbot/core/auth/session.py b/finbot/core/auth/session.py index 8bd090cb..49c0e570 100644 --- a/finbot/core/auth/session.py +++ b/finbot/core/auth/session.py @@ -454,7 +454,7 @@ def get_session( session_id[:8], session_context.original_ip, current_ip, - extra={"trusted_source": True} + extra={"app.trusted_source": True} ) else: logger.info( @@ -462,7 +462,7 @@ def get_session( session_id[:8], session_context.original_ip, current_ip, - extra={"trusted_source": False} + extra={"app.trusted_source": False} ) # Tiered fingerprint validation diff --git a/tests/unit/auth/test_session_ip_logging.py b/tests/unit/auth/test_session_ip_logging.py index e1c26054..f50724c5 100644 --- a/tests/unit/auth/test_session_ip_logging.py +++ b/tests/unit/auth/test_session_ip_logging.py @@ -39,4 +39,4 @@ def test_session_ip_change_logging( # Check log records log_records = [r for r in caplog.records if "IP change" in r.getMessage()] assert len(log_records) == 1 - assert getattr(log_records[0], "trusted_source") == expected_trusted_source + assert getattr(log_records[0], "app.trusted_source") == expected_trusted_source diff --git a/tests/unit/core/test_config_defaults.py b/tests/unit/core/test_config_defaults.py index 611f9c39..59660d4f 100644 --- a/tests/unit/core/test_config_defaults.py +++ b/tests/unit/core/test_config_defaults.py @@ -2,11 +2,14 @@ from unittest.mock import patch from finbot.config import Settings -def test_session_cookie_secure_default_when_env_missing(monkeypatch): +def test_session_cookie_secure_default_when_env_missing(monkeypatch, tmp_path): """ Assert that when SESSION_COOKIE_SECURE is completely absent from the environment, the code-level default firmly evaluates to True. """ + # Change into an empty temporary directory to ensure no local .env files can be discovered + monkeypatch.chdir(tmp_path) + # Ensure it's totally removed from the environment monkeypatch.delenv("SESSION_COOKIE_SECURE", raising=False) diff --git a/tests/unit/core/test_proxy_middleware_behavior.py b/tests/unit/core/test_proxy_middleware_behavior.py index 9e06197b..19751342 100644 --- a/tests/unit/core/test_proxy_middleware_behavior.py +++ b/tests/unit/core/test_proxy_middleware_behavior.py @@ -22,6 +22,13 @@ def get_ip(request: Request): # Negative Branch: A peer NOT in the trusted list tries to spoof IP (["10.0.0.1"], "192.168.1.100", "8.8.8.8", "192.168.1.100"), + + # Chained-header vulnerability scenario: + # If the edge proxy (10.0.0.1) is trusted but simply *appends* to an attacker-controlled header + # rather than stripping it, uvicorn's right-to-left traversal will pop 10.0.0.1, see 8.8.8.8, + # and resolve client_ip to the spoofed 8.8.8.8. This test pins this exact behavior, + # which is why our documentation explicitly mandates that edge proxies MUST STRIP inbound headers. + (["10.0.0.1"], "10.0.0.1", "8.8.8.8, 10.0.0.1", "8.8.8.8"), ]) def test_proxy_middleware_spoofing_behavior(trusted_hosts, peer_ip, x_forwarded_for, expected_ip): """ From 6362d8b110abddd5847b6a17fc9bd186db5a9a5c Mon Sep 17 00:00:00 2001 From: Prince Shakya Date: Tue, 19 May 2026 13:41:34 +0530 Subject: [PATCH 8/8] test(security): add explicit assertion for bare Settings() isolation Per reviewer feedback: Added an explicit assertion for the completely bare Settings() class default to crystal-clear pin the code-level default alongside the isolated Pydantic instantiation. --- tests/unit/core/test_config_defaults.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unit/core/test_config_defaults.py b/tests/unit/core/test_config_defaults.py index 59660d4f..8433f184 100644 --- a/tests/unit/core/test_config_defaults.py +++ b/tests/unit/core/test_config_defaults.py @@ -17,4 +17,8 @@ def test_session_cookie_secure_default_when_env_missing(monkeypatch, tmp_path): # Pass _env_file=None to ensure Pydantic doesn't read a stray .env file in the test dir. test_settings = Settings(_env_file=None) + # 1. Assert that without a loaded environment, the class-level default evaluates to True + assert Settings().SESSION_COOKIE_SECURE is True + + # 2. Assert that explicitly bypassing the .env file loading yields the same safe default assert test_settings.SESSION_COOKIE_SECURE is True