From e44848e16b1f407259d6b77710f87b67d730823e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 23:38:57 +0000 Subject: [PATCH 1/3] Key rate limits on client IP only; make trusted proxy count configurable The throttling identifier included a hash of User-Agent and Accept-Language, so varying a header gave an attacker a fresh failure counter. ProxyFix also always trusted one X-Forwarded-For hop, letting anyone who can reach the port directly forge their IP. docker-compose now publishes on 127.0.0.1 by default. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01PemRAtiBKjoZV6HkBCGDBC --- .env.example | 5 +++++ README.md | 2 ++ app.py | 23 ++++++++++++++-------- config.ini.example | 4 ++++ docker-compose.yml | 5 ++++- tests/test_ip_rate_limit.py | 39 +++++++++++++++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 tests/test_ip_rate_limit.py diff --git a/.env.example b/.env.example index bffa1b6..7b0d8a5 100644 --- a/.env.example +++ b/.env.example @@ -10,6 +10,11 @@ DOOROPENER_PORT=6532 FLASK_SECRET_KEY=your-secret-key-here FLASK_DEBUG=false +# Interface the container port is published on (docker-compose). Keep 127.0.0.1 behind a reverse proxy. +DOOROPENER_BIND=127.0.0.1 +# Number of reverse proxies in front of the app whose X-Forwarded-For is trusted (0 = none, direct access) +DOOROPENER_TRUSTED_PROXIES=1 + # When running behind HTTPS (reverse proxy), keep this true so cookies are secure. # For local HTTP testing only, set to false so the browser will send session cookies. SESSION_COOKIE_SECURE=true diff --git a/README.md b/README.md index 21fe48c..4ce2099 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,8 @@ python app.py ```bash FLASK_SECRET_KEY=change-me-to-something-long-and-random # required DOOROPENER_PORT=6532 # default 6532 +DOOROPENER_BIND=127.0.0.1 # interface docker-compose publishes the port on (keep local behind a reverse proxy) +DOOROPENER_TRUSTED_PROXIES=1 # reverse proxies whose X-Forwarded-For is trusted; 0 if clients connect directly TZ=Europe/Amsterdam # default UTC PUID=1000 # aligns container user to your host user PGID=1000 diff --git a/app.py b/app.py index 554b478..6281507 100644 --- a/app.py +++ b/app.py @@ -97,7 +97,6 @@ def get_current_time(): # --- Flask App Setup --- app = Flask(__name__) -app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1) # Prefer fixed secret from environment; fallback to temporary random (will be overridden by config.ini later if present) _env_secret = os.environ.get("FLASK_SECRET_KEY") if _env_secret: @@ -173,6 +172,16 @@ def get_effective_user_pins() -> dict: # Server Configuration server_port = int(os.environ.get("DOOROPENER_PORT", config.getint("server", "port", fallback=6532))) test_mode = config.getboolean("server", "test_mode", fallback=False) + +# Number of reverse proxies in front of the app whose X-Forwarded-* headers we trust. Every rate +# limit keys on the client IP, so trusting more hops than actually exist lets any caller who can +# reach the port directly pick their own IP via a forged X-Forwarded-For. 0 = no proxy (use the +# socket address). Default 1 matches a single reverse proxy such as Traefik/nginx/Caddy. +TRUSTED_PROXIES = int( + os.environ.get("DOOROPENER_TRUSTED_PROXIES", config.getint("server", "trusted_proxies", fallback=1)) +) +if TRUSTED_PROXIES > 0: + app.wsgi_app = ProxyFix(app.wsgi_app, x_for=TRUSTED_PROXIES, x_proto=TRUSTED_PROXIES, x_host=TRUSTED_PROXIES) if test_mode: logging.getLogger("dooropener").warning( "TEST MODE ENABLED — the door will NOT open. " @@ -314,7 +323,8 @@ def manifest_file(): def get_client_identifier(): """Get client identifier using multiple factors for better security""" - # Use request.remote_addr as primary (can't be spoofed easily) + # request.remote_addr is the only attacker-independent factor (given a correct + # TRUSTED_PROXIES setting); everything else a client sends can be varied per request. primary_ip = request.remote_addr # Create session-based identifier if available @@ -323,12 +333,9 @@ def get_client_identifier(): session_id = secrets.token_hex(16) session["_session_id"] = session_id - # Combine multiple factors for identifier - user_agent = request.headers.get("User-Agent", "")[:100] # Limit length - accept_lang = request.headers.get("Accept-Language", "")[:50] - - # Create composite identifier (harder to spoof than just IP) - identifier = f"{primary_ip}:{hash(user_agent + accept_lang) % 10000}" + # The throttling identifier must NOT include client-controlled headers (User-Agent, + # Accept-Language): rotating them would hand an attacker a fresh failure counter per request. + identifier = primary_ip # Record activity so idle rate-limit state for these keys can be evicted later. now_mono = time.monotonic() diff --git a/config.ini.example b/config.ini.example index 8e0737c..d218d8f 100644 --- a/config.ini.example +++ b/config.ini.example @@ -26,6 +26,10 @@ admin_password = admin123 # Can be overridden by DOOROPENER_PORT environment variable port = 6532 +# Reverse proxies in front of the app whose X-Forwarded-For is trusted (0 = direct access). +# Can be overridden by DOOROPENER_TRUSTED_PROXIES. +trusted_proxies = 1 + # Test mode - when true, shows success message but doesn't actually open door test_mode = false diff --git a/docker-compose.yml b/docker-compose.yml index c2ce245..b8b4af2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -13,7 +13,10 @@ services: - PUID=${PUID:-1000} - PGID=${PGID:-1000} ports: - - "${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" + # Bound to localhost by default so only your reverse proxy can reach it. Rate limits key on the + # client IP taken from X-Forwarded-For, which anyone who can reach this port directly could forge. + # Set DOOROPENER_BIND=0.0.0.0 only if you accept that (and set DOOROPENER_TRUSTED_PROXIES=0). + - "${DOOROPENER_BIND:-127.0.0.1}:${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" volumes: - ./config.ini:/app/config.ini:rw - ./logs:/app/logs diff --git a/tests/test_ip_rate_limit.py b/tests/test_ip_rate_limit.py new file mode 100644 index 0000000..62c21b9 --- /dev/null +++ b/tests/test_ip_rate_limit.py @@ -0,0 +1,39 @@ +"""Rate limits must key on the client IP, not on headers the client controls.""" + +import pytest + +HEADERS = {"User-Agent": "pytest-client/1.0 (+https://example.test)", "Content-Type": "application/json"} + + +@pytest.fixture +def app_module(): + import app as app_module + + return app_module + + +@pytest.fixture +def client(app_module): + app_module.app.config["TESTING"] = True + with app_module.app.test_client() as c: + yield c + + +def test_identifier_ignores_user_agent_and_language(app_module): + seen = set() + for i in range(5): + with app_module.app.test_request_context( + "/", headers={"User-Agent": f"agent-number-{i}", "Accept-Language": f"xx-{i}"} + ): + seen.add(app_module.get_client_identifier()[2]) + assert len(seen) == 1 + + +def test_rotating_user_agent_still_gets_blocked(client, app_module): + app_module.test_mode = True + statuses = [] + for i in range(app_module.MAX_ATTEMPTS + 1): + client.delete_cookie("session") # also drop the cookie so only the IP can throttle + h = {**HEADERS, "User-Agent": f"rotating-agent-{i}-padding", "Accept-Language": f"l{i}"} + statuses.append(client.post("/open-door", json={"pin": "0000"}, headers=h).status_code) + assert statuses[-1] == 429 From ea1d70892ae73b2eef89b98fb6ecddc0501aae92 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 23:46:02 +0000 Subject: [PATCH 2/3] Address review: parse trusted_proxies from the selected source; bind README port to localhost - Read the INI value only when DOOROPENER_TRUSTED_PROXIES is unset, so an invalid INI entry can't break startup when the env override is valid. - README Quick Start compose snippet, access instructions and docker run example now publish on 127.0.0.1 (DOOROPENER_BIND), matching docker-compose.yml. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01PemRAtiBKjoZV6HkBCGDBC --- README.md | 6 +++--- app.py | 7 +++++-- tests/test_ip_rate_limit.py | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 4ce2099..5c82083 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ services: container_name: dooropener env_file: .env ports: - - "${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" + - "${DOOROPENER_BIND:-127.0.0.1}:${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" volumes: - ./config.ini:/app/config.ini:ro - ./users.json:/app/users.json @@ -70,7 +70,7 @@ cp .env.example .env # set FLASK_SECRET_KEY at minimum docker compose up -d ``` -Then open `http://your-server:6532`. +Then open `http://localhost:6532` on the Docker host, or the URL configured on your reverse proxy. The port is published on `127.0.0.1` by default; see `DOOROPENER_BIND` below if you need it reachable directly. ### Build locally @@ -80,7 +80,7 @@ docker run -d --env-file .env \ -v $(pwd)/config.ini:/app/config.ini:ro \ -v $(pwd)/users.json:/app/users.json \ -v $(pwd)/logs:/app/logs \ - -p 6532:6532 dooropener:latest + -p 127.0.0.1:6532:6532 dooropener:latest ``` ### Without Docker diff --git a/app.py b/app.py index 6281507..4b2cb48 100644 --- a/app.py +++ b/app.py @@ -177,8 +177,11 @@ def get_effective_user_pins() -> dict: # limit keys on the client IP, so trusting more hops than actually exist lets any caller who can # reach the port directly pick their own IP via a forged X-Forwarded-For. 0 = no proxy (use the # socket address). Default 1 matches a single reverse proxy such as Traefik/nginx/Caddy. -TRUSTED_PROXIES = int( - os.environ.get("DOOROPENER_TRUSTED_PROXIES", config.getint("server", "trusted_proxies", fallback=1)) +# Select the source first and parse only that value, so a bad INI entry can't break startup when +# the environment override is valid. +_env_proxies = os.environ.get("DOOROPENER_TRUSTED_PROXIES") +TRUSTED_PROXIES = ( + int(_env_proxies) if _env_proxies is not None else config.getint("server", "trusted_proxies", fallback=1) ) if TRUSTED_PROXIES > 0: app.wsgi_app = ProxyFix(app.wsgi_app, x_for=TRUSTED_PROXIES, x_proto=TRUSTED_PROXIES, x_host=TRUSTED_PROXIES) diff --git a/tests/test_ip_rate_limit.py b/tests/test_ip_rate_limit.py index 62c21b9..4c6c1b6 100644 --- a/tests/test_ip_rate_limit.py +++ b/tests/test_ip_rate_limit.py @@ -37,3 +37,39 @@ def test_rotating_user_agent_still_gets_blocked(client, app_module): h = {**HEADERS, "User-Agent": f"rotating-agent-{i}-padding", "Accept-Language": f"l{i}"} statuses.append(client.post("/open-door", json={"pin": "0000"}, headers=h).status_code) assert statuses[-1] == 429 + + +def test_env_override_wins_even_if_ini_value_is_invalid(tmp_path): + """DOOROPENER_TRUSTED_PROXIES must be selected before the INI value is parsed. + + conftest mocks ConfigParser, so run the real thing in a subprocess against a throwaway copy. + """ + import os + import shutil + import subprocess + import sys + + root = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) + work = tmp_path / "app" + work.mkdir() + for name in os.listdir(root): + if name.endswith(".py"): + shutil.copy(os.path.join(root, name), work / name) + for d in ("templates", "static"): + shutil.copytree(os.path.join(root, d), work / d) + (work / "config.ini").write_text( + "[HomeAssistant]\nurl = http://x\ntoken = t\nswitch_entity = switch.d\n" + "[admin]\nadmin_password = a-real-password\n[server]\ntrusted_proxies = not-a-number\n" + ) + env = {**os.environ, "DOOROPENER_LOG_DIR": str(tmp_path / "logs"), "USERS_STORE_PATH": str(tmp_path / "u.json")} + code = f"import sys; sys.path.insert(0, {str(work)!r}); import app; print(app.TRUSTED_PROXIES)" + + ok = subprocess.run( + [sys.executable, "-I", "-c", code], + cwd=work, + env={**env, "DOOROPENER_TRUSTED_PROXIES": "0"}, + capture_output=True, + text=True, + ) + assert ok.returncode == 0, ok.stderr[-1500:] + assert ok.stdout.strip().endswith("0") From 83f2e8a8573abb7f8246e61f14446dc6db8225ae Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 23:56:34 +0000 Subject: [PATCH 3/3] Fix CI coverage gate and test isolation in IP rate-limit tests - The subprocess test inherited pytest-cov's env vars, so coverage measured a throwaway copy of the app and dropped total coverage below the 75% gate. Strip COV_CORE*/COVERAGE*. - Restore test_mode and app.config['TESTING'] after each test via monkeypatch. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01PemRAtiBKjoZV6HkBCGDBC --- tests/test_ip_rate_limit.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/test_ip_rate_limit.py b/tests/test_ip_rate_limit.py index 4c6c1b6..726553b 100644 --- a/tests/test_ip_rate_limit.py +++ b/tests/test_ip_rate_limit.py @@ -13,8 +13,8 @@ def app_module(): @pytest.fixture -def client(app_module): - app_module.app.config["TESTING"] = True +def client(app_module, monkeypatch): + monkeypatch.setitem(app_module.app.config, "TESTING", True) with app_module.app.test_client() as c: yield c @@ -29,8 +29,8 @@ def test_identifier_ignores_user_agent_and_language(app_module): assert len(seen) == 1 -def test_rotating_user_agent_still_gets_blocked(client, app_module): - app_module.test_mode = True +def test_rotating_user_agent_still_gets_blocked(client, app_module, monkeypatch): + monkeypatch.setattr(app_module, "test_mode", True) statuses = [] for i in range(app_module.MAX_ATTEMPTS + 1): client.delete_cookie("session") # also drop the cookie so only the IP can throttle @@ -61,7 +61,10 @@ def test_env_override_wins_even_if_ini_value_is_invalid(tmp_path): "[HomeAssistant]\nurl = http://x\ntoken = t\nswitch_entity = switch.d\n" "[admin]\nadmin_password = a-real-password\n[server]\ntrusted_proxies = not-a-number\n" ) - env = {**os.environ, "DOOROPENER_LOG_DIR": str(tmp_path / "logs"), "USERS_STORE_PATH": str(tmp_path / "u.json")} + # Drop pytest-cov's env vars: otherwise its .pth hook measures this throwaway copy of the app + # and its uncovered lines drag the project's coverage below the CI gate. + env = {k: v for k, v in os.environ.items() if not k.startswith(("COV_CORE", "COVERAGE"))} + env.update(DOOROPENER_LOG_DIR=str(tmp_path / "logs"), USERS_STORE_PATH=str(tmp_path / "u.json")) code = f"import sys; sys.path.insert(0, {str(work)!r}); import app; print(app.TRUSTED_PROXIES)" ok = subprocess.run(