Add middleware for managing duplicate headers

testdays/app.py

@@ -17,6 +17,66 @@ from . import config
 from . import extensions
 
 
+class _ForwardedHeaderFix:
+    """Normalize duplicated ``X-Forwarded-*`` values before ProxyFix.
+
+    When the OpenShift HAProxy router uses the default ``append`` mode for
+    ``haproxy.router.openshift.io/set-forwarded-headers``, every
+    ``X-Forwarded-*`` header gets its value doubled, e.g.
+    ``X-Forwarded-Proto: https,https`` or
+    ``X-Forwarded-Host: app.example.com, app.example.com``.
+
+    ProxyFix itself parses comma-separated values correctly via
+    ``parse_list_header``, but the raw environ keys are left untouched.
+    This causes problems for:
+
+    - **werkzeug >= 3.1.7** host validation: if ``x_host`` is enabled,
+      ProxyFix writes the clean value into ``HTTP_HOST``, but
+      ``host_is_trusted()`` may still see the raw duplicated value and
+      reject it (empty string in 3.1.8, ``SecurityError`` with
+      ``trusted_hosts``).
+    - **gunicorn** ``secure_scheme_headers``: does exact string matching
+      (``"https" != "https,https"``), silently downgrading to HTTP.
+
+    This middleware strips the duplication from all five headers and logs a
+    warning so the infra team knows the upstream proxy is misbehaving.
+    The proper upstream fix is setting the OpenShift route annotation
+    ``haproxy.router.openshift.io/set-forwarded-headers: Replace``.
+    """
+
+    # WSGI environ keys that correspond to X-Forwarded-* headers.
+    _FORWARDED_KEYS = (
+        "HTTP_X_FORWARDED_FOR",
+        "HTTP_X_FORWARDED_PROTO",
+        "HTTP_X_FORWARDED_HOST",
+        "HTTP_X_FORWARDED_PORT",
+        "HTTP_X_FORWARDED_PREFIX",
+    )
+
+    def __init__(self, app):
+        self.app = app
+        self._logger = logging.getLogger("testdays.proxy")
+
+    def __call__(self, environ, start_response):
+        for key in self._FORWARDED_KEYS:
+            raw = environ.get(key, "")
+            if "," in raw:
+                parts = [p.strip() for p in raw.split(",")]
+                if len(set(parts)) == 1:
+                    # All values are identical: HAProxy append-mode duplication.
+                    normalized = parts[0]
+                    self._logger.warning(
+                        "Received duplicated %s header: %r "
+                        "(normalized to %r)",
+                        key,
+                        raw,
+                        normalized,
+                    )
+                    environ[key] = normalized
+                # else: different values in the chain -- leave untouched.
+        return self.app(environ, start_response)
+
+
 class _WerkzeugRequestFilter(logging.Filter):
     """Drop Werkzeug's per-request log lines but keep startup/error messages."""
 
@@ -263,6 +323,9 @@ def create_app(config_obj=None):
 
     # --- Reverse proxy middleware ---
     if os.getenv("IS_OPENSHIFT"):
+        # Normalize duplicated X-Forwarded-* values (e.g. "https,https")
+        # before ProxyFix processes the headers.
+        app.wsgi_app = _ForwardedHeaderFix(app.wsgi_app)
         app.wsgi_app = ProxyFix(
             app.wsgi_app,
             x_for=int(app.config["PROXYFIX_X_FOR"]),

testdays/config.py

@@ -48,9 +48,14 @@ class Config(object):
     # Proxy fix settings -- trust levels for X-Forwarded-* headers.
     # Each value = number of proxies trusted for that header (0 = ignore).
     # Overridable via env vars or settings.py.
+    #
+    # Note: enabling x_host (setting to 1) requires _ForwardedHeaderFix to
+    # normalize duplicated X-Forwarded-Host values first.  Without it,
+    # werkzeug >= 3.1.7 rejects the comma-separated hostname as invalid
+    # (returns empty string or raises SecurityError with trusted_hosts).
     PROXYFIX_X_FOR = 1
     PROXYFIX_X_PROTO = 1
-    PROXYFIX_X_HOST = 0
+    PROXYFIX_X_HOST = 0  # see note above before enabling
     PROXYFIX_X_PORT = 0
     PROXYFIX_X_PREFIX = 0
 

tests/unit/test_proxy_fix.py

@@ -3,16 +3,18 @@ Unit tests for ProxyFix middleware integration.
 
 Verifies that werkzeug's ProxyFix is applied when IS_OPENSHIFT is set,
 config values are passed through, and it is not applied otherwise.
+Also tests that duplicated X-Forwarded-* headers are normalized.
 """
 
+import logging
 import os
 from unittest import mock
 
-from flask import Flask
 import pytest
+from jinjax.middleware import ComponentsMiddleware
 from werkzeug.middleware.proxy_fix import ProxyFix
 
-from testdays.app import create_app
+from testdays.app import create_app, _ForwardedHeaderFix
 from testdays.config import Testing
 from tests.conftest import TEST_SETTINGS_PATH
 
@@ -45,9 +47,10 @@ class TestProxyFixMiddleware:
         """
         GIVEN IS_OPENSHIFT is not set in environment
         WHEN the app is created
-        THEN the outermost wsgi_app layer should NOT be a ProxyFix instance
+        THEN neither ProxyFix nor _ForwardedHeaderFix should be in the stack
         """
         assert not isinstance(app.wsgi_app, ProxyFix)
+        assert not isinstance(app.wsgi_app, _ForwardedHeaderFix)
 
     def test_proxyfix_uses_config_defaults(self, openshift_app):
         """
@@ -83,3 +86,104 @@ class TestProxyFixMiddleware:
         assert pf.x_host == 1
         assert pf.x_port == 1
         assert pf.x_prefix == 1
+
+    def test_forwarded_header_fix_in_middleware_stack(self, openshift_app):
+        """
+        GIVEN IS_OPENSHIFT is set
+        WHEN the app is created
+        THEN the middleware stack should be:
+             ProxyFix -> _ForwardedHeaderFix -> ComponentsMiddleware (WhiteNoise)
+        """
+        proxy_fix = openshift_app.wsgi_app
+        assert isinstance(proxy_fix, ProxyFix)
+        header_fix = proxy_fix.app
+        assert isinstance(header_fix, _ForwardedHeaderFix)
+        assert isinstance(header_fix.app, ComponentsMiddleware)
+
+
+class TestForwardedHeaderFix:
+    """Tests for _ForwardedHeaderFix header normalization and logging."""
+
+    @pytest.fixture(autouse=True)
+    def _setup_middleware(self):
+        """Create a _ForwardedHeaderFix wrapping a stub WSGI app that
+        captures the (possibly rewritten) environ values."""
+        self.captured = {}
+
+        def inner_app(environ, start_response):
+            for key in _ForwardedHeaderFix._FORWARDED_KEYS:
+                self.captured[key] = environ.get(key)
+            start_response("200 OK", [])
+            return [b""]
+
+        self.mw = _ForwardedHeaderFix(inner_app)
+
+    @pytest.mark.parametrize(
+        "header_key, raw, expected",
+        [
+            # Identical duplicates (HAProxy append mode) -- normalized
+            ("HTTP_X_FORWARDED_PROTO", "https,https", "https"),
+            ("HTTP_X_FORWARDED_PROTO", "https , https", "https"),
+            ("HTTP_X_FORWARDED_PROTO", "http,http,http", "http"),
+            ("HTTP_X_FORWARDED_HOST", "app.example.com, app.example.com",
+             "app.example.com"),
+            ("HTTP_X_FORWARDED_FOR", "1.2.3.4, 1.2.3.4", "1.2.3.4"),
+            ("HTTP_X_FORWARDED_PORT", "443,443", "443"),
+            # Single values -- passed through unchanged
+            ("HTTP_X_FORWARDED_PROTO", "https", "https"),
+            ("HTTP_X_FORWARDED_HOST", "app.example.com", "app.example.com"),
+            # Legitimate multi-hop chain (different values) -- left untouched
+            ("HTTP_X_FORWARDED_FOR", "1.2.3.4, 10.0.0.1", "1.2.3.4, 10.0.0.1"),
+        ],
+        ids=[
+            "proto-duplicated",
+            "proto-duplicated-with-spaces",
+            "proto-tripled",
+            "host-duplicated",
+            "for-duplicated",
+            "port-duplicated",
+            "proto-single",
+            "host-single",
+            "for-multihop-passthrough",
+        ],
+    )
+    def test_normalizes_header(self, header_key, raw, expected):
+        """Identical duplicates are collapsed; distinct multi-hop values pass through."""
+        self.mw({header_key: raw}, lambda *a: None)
+        assert self.captured[header_key] == expected
+
+    @pytest.mark.parametrize(
+        "header_key, raw, expect_warning",
+        [
+            ("HTTP_X_FORWARDED_PROTO", "https,https", True),
+            ("HTTP_X_FORWARDED_HOST", "a.example.com, a.example.com", True),
+            ("HTTP_X_FORWARDED_PROTO", "https", False),
+            ("HTTP_X_FORWARDED_HOST", None, False),
+            # Legitimate multi-hop -- different values, no warning
+            ("HTTP_X_FORWARDED_FOR", "1.2.3.4, 10.0.0.1", False),
+        ],
+        ids=["proto-duplicated", "host-duplicated", "proto-single",
+             "host-absent", "for-multihop-no-warning"],
+    )
+    def test_warning_on_duplicated_header(self, caplog, header_key,
+                                          raw, expect_warning):
+        """A warning is logged only for identical duplicate values."""
+        env = {header_key: raw} if raw is not None else {}
+
+        # Attach caplog's handler directly to the logger so capture works
+        # regardless of root logger state (which _configure_logging may alter).
+        logger = logging.getLogger("testdays.proxy")
+        logger.addHandler(caplog.handler)
+        try:
+            with caplog.at_level(logging.WARNING, logger="testdays.proxy"):
+                self.mw(env, lambda *a: None)
+        finally:
+            logger.removeHandler(caplog.handler)
+
+        found = any(
+            f"Received duplicated {header_key} header" in msg
+            for msg in caplog.messages
+        )
+        assert found is expect_warning, (
+            f"expect_warning={expect_warning} but log messages: {caplog.messages}"
+        )