refactor(web): build the logged origin with urlunsplit, not an f-string

Semgrep's directly-returned-format-string rule read the helper as a Flask route returning a formatted string. Same output.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -38,8 +38,22 @@ accepts both, but the store flags the old spelling as deprecated
   dashboard or userscript on another host) can no longer call the mutating
   API; call it server-side instead. Anyone posting a form body to
   `system/action` must switch to JSON. Behind a reverse proxy, forward the
-  original `Host` (`proxy_set_header Host $host;`); `X-Forwarded-Host` is
-  not trusted.
+  original `Host`, port included (`proxy_set_header Host $http_host;`;
+  nginx's `$host` drops the port); `X-Forwarded-Host` is not trusted. A
+  TLS-terminating proxy needs nothing more: a portless `Host` matches an
+  `https://` page.
+
+### Fixes
+
+- On-demand no longer restarts a running display. `POST
+  /display/on-demand/start` treated `start_service` (on by default, and what
+  "Preview on display", the on-demand dialog and the MQTT bridge all send) as
+  "restart": it stopped the service, waited 1.5s and started it again, so
+  every request reloaded every plugin and left the panel blank for seconds.
+  The running display already reads the request within a quarter of a second,
+  mid-screen and mid-Vegas included, so the route now only starts the service
+  when it is not running. `POST /display/on-demand/stop` reads
+  `stop_service` as a boolean, so `"false"` no longer stops the service.
 
 ## 3.7.0
 

docs/ARCHITECTURE.md

@@ -52,8 +52,10 @@ each other. They share three things:
 | Preview viewer marker | `/tmp/led_matrix_preview_viewer` | web, while a preview is open | display: writes full-rate snapshots only while it is fresh |
 | Hardware init status | `/tmp/led_matrix_hw_status.json` | display | web: `/api/v3/hardware/status` |
 
-The on-demand start route also restarts `ledmatrix.service` by default so the
-request takes effect straight away.
+The on-demand start route starts `ledmatrix.service` when it is not running
+(`start_service`, on by default) but never restarts a running one: the display
+reads the mailbox every `ON_DEMAND_POLL_INTERVAL` (0.25s), from its dwell
+sleep, its render loops and Vegas's interrupt check as well as the main loop.
 
 ## Display loop
 

docs/REST_API_REFERENCE.md

@@ -26,8 +26,8 @@ driving the Pi through a LAN user's browser. Scripts, curl, Home Assistant and
 the MQTT bridge send neither header and are unaffected. A browser page on
 another origin (a dashboard you host elsewhere, say) can no longer call the
 API; call it server-side instead. Behind a reverse proxy, pass the original
-`Host` through (nginx: `proxy_set_header Host $host;`) -- `X-Forwarded-Host`
-is not read.
+`Host` through, port included (nginx: `proxy_set_header Host $http_host;`;
+`$host` drops the port) -- `X-Forwarded-Host` is not read.
 
 ## Table of Contents
 
@@ -401,7 +401,7 @@ Request a specific plugin to display on-demand.
 - `mode` (string, optional): Display mode name (plugin_id inferred if not provided)
 - `duration` (number, optional): Duration in seconds (0 = until stopped)
 - `pinned` (boolean, optional): Pin display (pause rotation)
-- `start_service` (boolean, optional): (Re)start the display service so it picks the request up (default: true)
+- `start_service` (boolean, optional): Start the display service if it is not running (default: true). A running service is never restarted: it picks the request up within about a quarter of a second. When false and the service is stopped, the route returns 400.
 
 **Response**:
 ```json

docs/WEB_INTERFACE_GUIDE.md

@@ -419,7 +419,8 @@ The API blueprint (`web_interface/blueprints/api_v3/`) is registered at
   (403 `CROSS_SITE_REQUEST`), so use the interface from its own address.
 - Scripts, curl, Home Assistant and the MQTT bridge send no such header and
   keep working. Behind a reverse proxy, forward the original `Host` header
-  (nginx: `proxy_set_header Host $host;`).
+  with its port (nginx: `proxy_set_header Host $http_host;` -- `$host`
+  drops the port).
 
 **Best Practices:**
 1. Run on a private network (not exposed to internet)

test/test_api_v3_on_demand_restart.py

@@ -1,25 +1,27 @@
-"""Regression test: POST /display/on-demand/start restarting a running
-service must not import a name that does not exist.
+"""POST /display/on-demand/start and /stop must not restart a running display.
 
-display.py has `import web_interface.blueprints.api_v3 as _pkg` and reads
-mutable, test-patched attributes back through it (`_pkg.time.time()`,
-`_pkg._get_starlark_plugin()`, ...) rather than binding them by value, per
-the package's own docstring. One spot went further and wrote a genuine
-`import` *statement* against that alias --
+The start route used to treat ``start_service`` (default True, and what both
+the web UI and the MQTT bridge send) as "restart": with the service running it
+ran ``systemctl stop``, slept 1.5s and started it again. Every on-demand or
+"Preview on display" click therefore cold-restarted the display process --
+every plugin reloaded, the panel blank for seconds -- to deliver a request the
+running process polls for every ON_DEMAND_POLL_INTERVAL anyway (see
+test_on_demand_mailbox.py and test_display_pending_changes.py for the display
+side: the mailbox is read mid-dwell, mid-screen and mid-Vegas-iteration).
 
-    import _pkg.time as time_module
+The restart did not buy anything either: a freshly started display restores
+only the on-demand session it saved itself (``display_on_demand_config``), so
+the new request reached it through the same mailbox, one cold start later.
 
--- but `_pkg` is a local name bound by `import ... as _pkg` in this module,
-not a real top-level package, so `import _pkg.time` is not something Python
-can resolve; it raises ModuleNotFoundError. That line only runs when the
-display service is already running and the caller also asked to (re)start
-it, so this endpoint failed on exactly the restart path -- the one where a
-cache write recording the new on-demand request had already happened.
+This file previously pinned that restart path (it guarded a broken
+``import _pkg.time`` inside it). The path is gone; these tests pin its
+replacement: a running service is left alone, a stopped one is started (only
+when start_service is set), and the request lands in the mailbox either way.
 
-The route wraps its body in `except Exception`, so the failure reached the
-caller as a handled 500 with a generic message, not an unhandled crash --
-but a 500 all the same on a request that should have restarted the service
-and reported success.
+The service helpers are patched where they run. display.py binds
+_get_display_service_status by value, while _ensure_display_service_running
+(in the package __init__) looks it up in its own module, so both are patched;
+_run_systemctl_command is the one place a systemctl command is issued.
 """
 
 import sys
@@ -32,60 +34,137 @@ sys.path.insert(0, str(Path(__file__).parent.parent))
 
 from test._api_v3_test_helpers import api_v3_client, api_v3_module  # noqa: F401,E402
 
-URL = "/api/v3/display/on-demand/start"
+START_URL = "/api/v3/display/on-demand/start"
+STOP_URL = "/api/v3/display/on-demand/stop"
+MAILBOX = "display_on_demand_request"
 
 
 @pytest.fixture
-def restart_path(api_v3_module):
-    """Force the `service_was_running and start_service` branch.
+def service(api_v3_module):
+    """A display service whose state the test sets; records systemctl calls.
 
-    plugin_manager and config_manager are set to None so the route takes
-    the simplest path to that branch rather than tripping over unrelated
-    MagicMock plumbing. The cache is the blueprint's cache_manager, which
-    api_v3_module already set to a MagicMock. _get_display_service_status,
-    _stop_display_service and _ensure_display_service_running are bound by
-    value in display.py (see its own docstring), so they are patched on
-    that submodule rather than on the package.
+    plugin_manager and config_manager are None so the route skips plugin
+    resolution (not what is under test here). The cache is the blueprint's
+    MagicMock cache_manager, so mailbox writes are visible as set() calls.
     """
     api_v3_module.api_v3.plugin_manager = None
     api_v3_module.api_v3.config_manager = None
+    state = {"active": True}
 
-    with patch("web_interface.blueprints.api_v3.display._get_display_service_status") as get_status, \
-         patch("web_interface.blueprints.api_v3.display._stop_display_service") as stop_service, \
-         patch("web_interface.blueprints.api_v3.display._ensure_display_service_running") as ensure_running:
-        # Active before the request: service_was_running becomes True.
-        get_status.return_value = {"active": True}
-        ensure_running.return_value = {"active": True}
+    def status():
+        return {"active": state["active"]}
+
+    def systemctl(args):
+        if args[-2:] == ["start", "ledmatrix.service"]:
+            state["active"] = True
+        elif args[-2:] == ["stop", "ledmatrix.service"]:
+            state["active"] = False
+        return {"returncode": 0, "stdout": "", "stderr": ""}
+
+    with patch("web_interface.blueprints.api_v3._get_display_service_status",
+               side_effect=status), \
+         patch("web_interface.blueprints.api_v3.display._get_display_service_status",
+               side_effect=status), \
+         patch("web_interface.blueprints.api_v3._run_systemctl_command",
+               side_effect=systemctl) as run_systemctl, \
+         patch("web_interface.blueprints.api_v3.display._stop_display_service") as stop_service:
         yield {
-            "get_status": get_status,
+            "state": state,
+            "systemctl": run_systemctl,
             "stop_service": stop_service,
-            "ensure_running": ensure_running,
+            "cache": api_v3_module.api_v3.cache_manager,
         }
 
 
-class TestRestartingARunningService:
-    def test_it_does_not_500(self, api_v3_client, restart_path):
-        response = api_v3_client.post(
-            URL, json={"plugin_id": "weather", "start_service": True})
-        body = response.get_json()
-        assert response.status_code == 200, body
-        assert body["status"] == "success", body
+def _mailbox_writes(cache):
+    return [c.args[1] for c in cache.set.call_args_list if c.args and c.args[0] == MAILBOX]
 
-    def test_the_service_is_actually_stopped_and_restarted(
-            self, api_v3_client, restart_path):
-        api_v3_client.post(
-            URL, json={"plugin_id": "weather", "start_service": True})
-        restart_path["stop_service"].assert_called_once()
-        restart_path["ensure_running"].assert_called_once()
 
-    def test_a_service_that_was_not_running_is_not_stopped_first(
-            self, api_v3_client, restart_path):
-        # The buggy import sits inside `if service_was_running and
-        # start_service`, so it only ever fired on the restart path --
-        # this is the other side of that branch, unaffected either way,
-        # kept here so the branch condition itself stays covered.
-        restart_path["get_status"].return_value = {"active": False}
-        response = api_v3_client.post(
-            URL, json={"plugin_id": "weather", "start_service": True})
+def _systemctl_verbs(run_systemctl):
+    return [c.args[0][-2] for c in run_systemctl.call_args_list]
+
+
+class TestStartWhileTheServiceIsRunning:
+    @pytest.mark.parametrize("body", [
+        {"plugin_id": "weather"},                           # "Preview on display", MQTT
+        {"plugin_id": "weather", "start_service": True},    # on-demand modal, box ticked
+        {"plugin_id": "weather", "start_service": "true"},
+    ])
+    def test_the_service_is_not_stopped_or_restarted(self, api_v3_client, service, body):
+        response = api_v3_client.post(START_URL, json=body)
         assert response.status_code == 200, response.get_json()
-        restart_path["stop_service"].assert_not_called()
+        assert response.get_json()["status"] == "success"
+        service["stop_service"].assert_not_called()
+        assert _systemctl_verbs(service["systemctl"]) == [], (
+            "a running display service was sent a systemctl command")
+
+    def test_the_request_is_posted_for_the_running_display(self, api_v3_client, service):
+        response = api_v3_client.post(
+            START_URL, json={"plugin_id": "weather", "mode": "weather_current",
+                             "duration": 60, "pinned": True})
+        data = response.get_json()["data"]
+        writes = _mailbox_writes(service["cache"])
+        assert len(writes) == 1
+        assert writes[0]["action"] == "start"
+        assert writes[0]["request_id"] == data["request_id"]
+        assert writes[0]["plugin_id"] == "weather"
+        assert writes[0]["mode"] == "weather_current"
+        assert writes[0]["duration"] == 60
+        assert writes[0]["pinned"] is True
+
+    def test_the_response_reports_the_service_was_not_started(self, api_v3_client, service):
+        data = api_v3_client.post(START_URL, json={"plugin_id": "weather"}).get_json()["data"]
+        assert data["service"]["active"] is True
+        assert data["service"]["started"] is False
+
+    def test_it_answers_without_the_old_restart_pause(self, api_v3_client, service):
+        # The restart slept 1.5s; nothing here should sleep at all.
+        with patch("time.sleep") as sleep:
+            api_v3_client.post(START_URL, json={"plugin_id": "weather"})
+        sleep.assert_not_called()
+
+
+class TestStartWhileTheServiceIsStopped:
+    def test_start_service_starts_it_once_and_never_stops_it(self, api_v3_client, service):
+        service["state"]["active"] = False
+        response = api_v3_client.post(START_URL, json={"plugin_id": "weather"})
+        assert response.status_code == 200, response.get_json()
+        assert _systemctl_verbs(service["systemctl"]) == ["start"]
+        service["stop_service"].assert_not_called()
+        # Written before the start, so the new process finds it on its first poll.
+        assert len(_mailbox_writes(service["cache"])) == 1
+
+    def test_without_start_service_it_is_left_stopped(self, api_v3_client, service):
+        service["state"]["active"] = False
+        response = api_v3_client.post(
+            START_URL, json={"plugin_id": "weather", "start_service": "false"})
+        assert response.status_code == 400
+        assert _systemctl_verbs(service["systemctl"]) == []
+
+    def test_a_start_that_fails_is_reported(self, api_v3_client, service):
+        service["state"]["active"] = False
+        service["systemctl"].side_effect = lambda args: {
+            "returncode": 1, "stdout": "", "stderr": "denied"}
+        response = api_v3_client.post(START_URL, json={"plugin_id": "weather"})
+        assert response.status_code == 500
+        assert response.get_json()["status"] == "error"
+
+
+class TestStop:
+    def test_stop_posts_a_stop_request_and_leaves_the_service_running(
+            self, api_v3_client, service):
+        response = api_v3_client.post(STOP_URL, json={})
+        assert response.status_code == 200, response.get_json()
+        writes = _mailbox_writes(service["cache"])
+        assert [w["action"] for w in writes] == ["stop"]
+        service["stop_service"].assert_not_called()
+        assert _systemctl_verbs(service["systemctl"]) == []
+
+    def test_a_string_false_stop_service_does_not_stop_it(self, api_v3_client, service):
+        # bool("false") is True: the flag was read raw and stopped the service.
+        api_v3_client.post(STOP_URL, json={"stop_service": "false"})
+        service["stop_service"].assert_not_called()
+
+    def test_stop_service_true_still_stops_it(self, api_v3_client, service):
+        api_v3_client.post(STOP_URL, json={"stop_service": True})
+        service["stop_service"].assert_called_once()

test/test_web_origin_guard.py

@@ -51,7 +51,9 @@ def test_a_cross_site_origin_is_refused_for_every_changing_method(probe, method)
     body = resp.get_json()
     assert body['status'] == 'error'
     assert body['error_code'] == 'CROSS_SITE_REQUEST'
-    assert 'evil.example' in body['details']
+    assert body['details'].startswith('Origin ')
+    # The attacker-chosen origin is logged, never echoed back in the body.
+    assert 'evil.example' not in resp.get_data(as_text=True)
 
 
 @pytest.mark.parametrize('origin', [
@@ -88,6 +90,57 @@ def test_the_same_host_on_another_port_is_another_site(probe):
     assert resp.status_code == 403
 
 
+def test_an_https_page_behind_a_tls_terminating_proxy_passes(probe):
+    # nginx terminates TLS and forwards a portless Host to the plain-http
+    # upstream: the browser's Origin is https (443), Flask sees http (80).
+    resp = probe.post('/change', headers={
+        'Host': 'pi.example', 'Origin': 'https://pi.example'})
+    assert resp.status_code == 200
+    resp = probe.post('/change', headers={
+        'Host': 'pi.example', 'Referer': 'https://pi.example/v3'})
+    assert resp.status_code == 200
+
+
+def test_a_portless_host_still_refuses_a_nondefault_port(probe):
+    # Only the standard port of either scheme counts as "no port".
+    for origin in ('https://pi.example:8443', 'http://pi.example:5000',
+                   'http://pi.example:443', 'https://evil.example'):
+        resp = probe.post('/change', headers={
+            'Host': 'pi.example', 'Origin': origin})
+        assert resp.status_code == 403, origin
+
+
+def test_an_explicit_host_port_must_match_exactly(probe):
+    # A Host with a port (the proxy forwards $http_host) is compared as is.
+    assert probe.post('/change', headers={
+        'Host': 'pi.example:8443',
+        'Origin': 'https://pi.example:8443'}).status_code == 200
+    assert probe.post('/change', headers={
+        'Host': 'pi.example:8443',
+        'Origin': 'https://pi.example'}).status_code == 403
+
+
+def test_a_refusal_logs_only_the_site_never_the_referer_path(probe, caplog):
+    # A Referer's path and query can carry tokens.
+    with caplog.at_level('WARNING', logger='web_interface.origin_guard'):
+        resp = probe.post('/change', headers={
+            'Referer': EVIL + '/page?token=s3cret#frag'})
+    assert resp.status_code == 403
+    logged = caplog.text
+    assert 'evil.example' in logged
+    assert 's3cret' not in logged
+    assert '/page' not in logged
+
+
+def test_a_refusal_log_cannot_be_forged_with_newlines(probe, caplog):
+    with caplog.at_level('WARNING', logger='web_interface.origin_guard'):
+        probe.post('/change%0D%0AFAKE', headers={'Origin': EVIL})
+    assert len(caplog.records) == 1
+    message = caplog.records[0].getMessage()
+    assert '\n' not in message and '\r' not in message
+    assert 'FAKE' in message  # the path was logged, escaped
+
+
 def test_no_origin_and_no_referer_passes(probe):
     # curl, Home Assistant, the MQTT bridge: not a browser.
     assert probe.post('/change').status_code == 200

web_interface/blueprints/api_v3/display.py

@@ -192,8 +192,9 @@ def start_on_demand_display():
                 resolved_plugin,
             )
 
-    # Set the on-demand request in cache FIRST (before starting service)
-    # This ensures the request is available when the service starts/restarts
+    # Post the request to the mailbox the display process polls
+    # (DisplayController._poll_on_demand_requests). Written before any
+    # service start, so a freshly started display finds it on its first poll.
     cache = _cache_manager()
     request_id = data.get('request_id') or str(uuid.uuid4())
     request_payload = {
@@ -207,18 +208,7 @@ def start_on_demand_display():
     }
     cache.set('display_on_demand_request', request_payload)
 
-    # Check if display service is running (or will be started)
     service_status = _get_display_service_status()
-    service_was_running = service_status.get('active', False)
-        
-    # Stop the display service first to ensure clean state when we will restart it
-    if service_was_running and start_service:
-        import time as time_module
-        logger.debug("Stopping display service before starting on-demand mode")
-        _stop_display_service()
-        # Wait a brief moment for the service to fully stop
-        time_module.sleep(1.5)
-        logger.debug("Display service stopped, now starting with on-demand request")
 
     if not service_status.get('active') and not start_service:
         return jsonify({
@@ -227,6 +217,18 @@ def start_on_demand_display():
             'service_status': service_status
         }), 400
 
+    # start_service means "start it if it is not running", as the UI's
+    # checkbox says; _ensure_display_service_running leaves a running service
+    # alone. This used to stop a running service, sleep 1.5s and start it
+    # again, so every on-demand or "Preview on display" click -- and every
+    # MQTT on-demand command, which posts here with the default -- cold-
+    # restarted the display process: every plugin reloaded and the panel was
+    # blank for seconds. The restart bought nothing. The running process
+    # reads this mailbox every ON_DEMAND_POLL_INTERVAL (0.25s), from its
+    # dwell sleep, its render loops and Vegas's interrupt check as well as
+    # the main loop, and a restarted one got the request the same way: the
+    # startup path only restores a session the display itself saved
+    # (display_on_demand_config), so it loaded nothing it would not have had.
     service_result = None
     if start_service:
         service_result = _ensure_display_service_running()
@@ -237,9 +239,6 @@ def start_on_demand_display():
                 'message': 'Failed to start display service. Please check service logs or start it manually.',
                 'service_result': service_result
             }), 500
-            
-        # Service was restarted (or started fresh) with on-demand request in cache
-        # The display controller will read the request during initialization or when it polls
 
     response_data = {
         'request_id': request_id,
@@ -254,10 +253,12 @@ def start_on_demand_display():
 def stop_on_demand_display():
     """Request the display controller to stop on-demand mode."""
     data = request.get_json(silent=True) or {}
-    stop_service = data.get('stop_service', False)
+    # _coerce_to_bool: bool("false") is True, which stopped the service.
+    stop_service = _coerce_to_bool(data.get('stop_service', False))
 
-    # Set the stop request in cache FIRST
-    # The display controller will poll this and restart without the on-demand filter
+    # The running display reads the stop from the mailbox within
+    # ON_DEMAND_POLL_INTERVAL and resumes normal rotation in place
+    # (_clear_on_demand); nothing is restarted.
     cache = _cache_manager()
     request_id = data.get('request_id') or str(uuid.uuid4())
     request_payload = {
@@ -266,10 +267,7 @@ def stop_on_demand_display():
         'timestamp': _pkg.time.time()
     }
     cache.set('display_on_demand_request', request_payload)
-        
-    # Note: The display controller's _clear_on_demand() will handle the restart
-    # to restore normal operation with all plugins
-        
+
     service_result = None
     if stop_service:
         service_result = _stop_display_service()

web_interface/origin_guard.py

@@ -29,21 +29,25 @@ it follows whatever name or address the user typed: ``ledpi.local:5000``,
 portal's port 80 -> 5000 redirect keeps the Host the browser sent, and the
 setup page's fetches go back to that same host).
 
-The scheme is deliberately not compared, only host and port (with each side's
-default port filled in from its own scheme). A TLS-terminating reverse proxy
-that passes ``Host`` through makes the browser say ``https://pi.example`` while
-Flask sees ``http``; an attacker cannot use that gap, because to match they
-would need to serve a page from this same host and port. The app does not use
-``ProxyFix`` and so does not trust ``X-Forwarded-Host``: a proxy that rewrites
-``Host`` to the upstream address (nginx's default ``proxy_pass`` does) must
-be configured to pass the original one (``proxy_set_header Host $host;``).
+The scheme is deliberately not compared, only host and port. The claimed
+value's default port comes from its own scheme. A ``Host`` without a port
+means "the default port of whatever scheme the browser used", and that scheme
+is not always the one Flask sees: a TLS-terminating reverse proxy makes the
+browser say ``https://pi.example`` (443) while Flask sees ``http`` (80). So a
+portless ``Host`` accepts either default. An attacker cannot use that gap,
+because to match they would need to serve a page from this same host on its
+standard port. The app does not use ``ProxyFix`` and so does not trust
+``X-Forwarded-Host`` or ``X-Forwarded-Proto``: a proxy that rewrites ``Host``
+to the upstream address (nginx's default ``proxy_pass`` does) must be
+configured to pass the original one, port included
+(``proxy_set_header Host $http_host;`` -- nginx's ``$host`` drops the port).
 
 Not covered: DNS rebinding (an attacker's hostname re-pointed at the Pi is
 "same origin" to the browser), and anyone who can reach the port directly.
 Neither is new; the interface is still meant for a trusted network.
 """
 import logging
-from urllib.parse import urlsplit
+from urllib.parse import urlsplit, urlunsplit
 
 from flask import Flask, jsonify, request
 
@@ -55,14 +59,14 @@ STATE_CHANGING_METHODS = frozenset({'POST', 'PUT', 'PATCH', 'DELETE'})
 _DEFAULT_PORTS = {'http': 80, 'https': 443}
 
 
-def _host_port(scheme: str, netloc: str):
-    """``(hostname, port)`` for a URL's authority, or None if it has none.
+def _authority(netloc: str):
+    """``(hostname, port)`` for an authority; port is None when it has none.
 
-    Lower-cases the host and fills in the scheme's default port, so
-    ``http://Pi.local`` and a ``Host: pi.local:80`` header compare equal.
+    Lower-cases the host and drops a trailing dot, so ``Pi.local.`` and
+    ``pi.local`` compare equal. None if the authority is unreadable.
     """
     try:
-        parts = urlsplit(f'{scheme}://{netloc}')
+        parts = urlsplit(f'//{netloc}')
         hostname = parts.hostname
         port = parts.port
     except ValueError:
@@ -70,25 +74,63 @@ def _host_port(scheme: str, netloc: str):
         return None
     if not hostname:
         return None
-    if port is None:
-        port = _DEFAULT_PORTS.get(scheme.lower())
     return hostname.lower().rstrip('.'), port
 
 
 def _url_host_port(url: str):
-    """``(hostname, port)`` for an Origin or Referer value, or None."""
+    """``(hostname, port, default_port)`` for an Origin or Referer, or None.
+
+    ``port`` is the explicit port or, failing that, the URL scheme's default,
+    which is also returned as ``default_port``.
+    """
     try:
         parts = urlsplit(url.strip())
     except ValueError:
         return None
-    if parts.scheme.lower() not in _DEFAULT_PORTS or not parts.netloc:
+    scheme = parts.scheme.lower()
+    if scheme not in _DEFAULT_PORTS or not parts.netloc:
         return None
-    return _host_port(parts.scheme.lower(), parts.netloc.rsplit('@', 1)[-1])
+    authority = _authority(parts.netloc.rsplit('@', 1)[-1])
+    if authority is None:
+        return None
+    hostname, port = authority
+    default_port = _DEFAULT_PORTS[scheme]
+    return hostname, default_port if port is None else port, default_port
 
 
-def _request_host_port():
-    """``(hostname, port)`` this request was addressed to, per its Host."""
-    return _host_port(request.scheme, request.host)
+def _names_this_server(claimed) -> bool:
+    """Whether a claimed ``(hostname, port, default_port)`` is this request's
+    own ``Host``."""
+    own = _authority(request.host)
+    if own is None:
+        return False
+    hostname, port = own
+    claimed_host, claimed_port, claimed_default = claimed
+    if claimed_host != hostname:
+        return False
+    if port is not None:
+        return claimed_port == port
+    # A portless Host is the default port of the scheme the browser used.
+    # Behind a TLS-terminating proxy that is https/443 while Flask sees
+    # http/80, so accept the default of either scheme.
+    return claimed_port in (claimed_default,
+                            _DEFAULT_PORTS.get(request.scheme))
+
+
+def _loggable(value: str) -> str:
+    """Just the ``scheme://host[:port]`` of an Origin/Referer, for the log.
+
+    A Referer's path and query can carry tokens or other private data, and
+    only the site matters when reading a refusal.
+    """
+    try:
+        parts = urlsplit(value.strip())
+        netloc = parts.netloc.rsplit('@', 1)[-1]
+    except ValueError:
+        return '<unreadable>'
+    if not parts.scheme or not netloc:
+        return '<unreadable>'
+    return urlunsplit((parts.scheme, netloc, '', '', ''))
 
 
 def check_request_origin():
@@ -115,8 +157,10 @@ def check_request_origin():
     claimed = _url_host_port(value)
     if claimed is None:
         return f'{header} header is not a valid http(s) URL'
-    if claimed != _request_host_port():
-        return f'{header} {value!r} is not this interface ({request.host!r})'
+    if not _names_this_server(claimed):
+        # The claimed value is attacker-chosen: the hook logs it, but the
+        # reason (echoed in the 403 body) never repeats it.
+        return header + ' names a different host than this interface'
     return None
 
 
@@ -128,8 +172,14 @@ def init_app(app: Flask) -> None:
         reason = check_request_origin()
         if reason is None:
             return None
-        logger.warning("Refused cross-site %s %s: %s",
-                       request.method, request.path, reason)
+        # Only the site each header names, never a Referer's path or query
+        # (which can carry tokens); %r keeps CR/LF from forging log lines.
+        origin = request.headers.get('Origin')
+        referer = request.headers.get('Referer')
+        logger.warning("Refused cross-site %s %r: %s (Origin=%r, Referer=%r)",
+                       request.method, request.path, reason,
+                       None if origin is None else _loggable(origin),
+                       None if referer is None else _loggable(referer))
         return jsonify({
             'status': 'error',
             'error_code': 'CROSS_SITE_REQUEST',