feat(web): expect a tunnel's own hostname, one name at a time (#648, partial)

docs/architecture.md

@@ -73,9 +73,13 @@ is a load-bearing property across the codebase.
 
 ## 4. Security architecture
 
-- **Loopback bind by design** with Host/bind checks against DNS rebinding; remote
-  exposure is deliberately absent until #648 lands the opt-in bind, tunnel Host
-  allowlist, and off-loopback token posture.
+- **Loopback bind by design** with Host/bind checks against DNS rebinding. The tunnel
+  Host allowlist is in (`keel serve --external-host`, one specific name each, wildcards
+  refused at startup) and the threat model for each transport is written down in
+  [`docs/remote-access.md`](remote-access.md) — including that a Cloudflare Tunnel
+  terminates TLS at its edge. **Remote exposure is still deliberately absent**: #648's
+  off-loopback token posture and secure-context re-verification are not done, and that
+  document says so at the top rather than leaving it to be discovered.
 - **One-time session token** URLs; CSP `default-src 'self'` with no `unsafe-inline`
   (pinned by test); the key-parity scanner ensures the payload emits every key the
   client reads.

docs/remote-access.md

@@ -0,0 +1,82 @@
+# Reaching the console from another device
+
+`keel serve` binds loopback. That is the posture, and this document exists so that widening it
+is a decision someone made on purpose rather than a flag someone found.
+
+**Status: incomplete. Nothing remote should be exposed yet.** Two of #648's five requirements
+are met — the bind is configurable and a reverse proxy's hostname can be expected explicitly —
+and three are not. They are named at the bottom, and they are not paperwork.
+
+## What the defence actually defends
+
+`keel serve` answers on `127.0.0.1`, and a browser treats `http://127.0.0.1` as a secure
+context by specification, which is what makes the service worker and the manifest work over
+plain HTTP. Binding loopback is not, by itself, protection: any page in the operator's browser
+can issue requests to `127.0.0.1`, and an attacker who points `evil.example` at `127.0.0.1`
+gets a connection that arrives on loopback with every network-level check satisfied. That is
+DNS rebinding, and the `Host:` header is the only place the lie is visible.
+
+So `HostPolicy` refuses any `Host:` the bind does not account for. `evil.example` is refused
+even though the packet came from loopback, because the name is not one this server bound.
+
+## Why a tunnel is refused by default, and why that is correct
+
+A Cloudflare Tunnel forwards to loopback and presents the app's **public domain** in `Host:`.
+From inside the process that is indistinguishable from the rebinding attempt above: both are
+names that resolve to a machine this server did not bind. Nothing in the request tells them
+apart, and nothing can — only the operator knows which name is theirs.
+
+That is why the name has to be typed:
+
+```
+keel serve --external-host keel.example.com
+```
+
+One specific name, repeatable for more than one. Wildcards are refused **when the server
+starts**, not per request: `*`, `*.example.com`, `.example.com`, `0.0.0.0`, `::`, `any`, `all`
+and the empty string all raise. A wildcard is not a wider expectation, it is the absence of
+one — `*` answers `evil.example` exactly as readily as the operator's own domain, which is the
+attack itself. A leading-dot suffix is refused for the same reason: it admits every subdomain
+an attacker can provision, and a tunnel presents one name.
+
+Naming a host **extends** the defence and never replaces it. Every loopback spelling still
+answers, every unlisted name is still refused, and the port must still match — a proxied
+request is not exempt from being addressed to this server.
+
+## What each transport actually guarantees
+
+Stated plainly, because "it's encrypted" is not a threat model.
+
+| | what protects the traffic | who can read it | what the operator is trusting |
+| :-- | :-- | :-- | :-- |
+| **Loopback only** | nothing leaves the machine | anything running as the operator | the machine itself |
+| **WireGuard mesh** (Tailscale/Headscale) | end-to-end between the operator's own devices | only those devices | the mesh's coordination server for key distribution — not for content |
+| **Cloudflare Tunnel** | TLS from the browser to Cloudflare, then Cloudflare to the machine | **Cloudflare, in the clear at its edge** | Cloudflare with the plaintext of every page and every session token |
+
+The third row is the one that needs saying out loud. Cloudflare terminates TLS. In that mode
+the operator is not merely trusting Cloudflare to route traffic; they are handing it the
+readable contents of a console that displays positions, balances and attestations. That may be
+an acceptable trade for convenience. It is not a neutral one, and a document that let someone
+discover it later would have failed.
+
+A mesh has no such property: WireGuard is end-to-end between devices the operator enrolled, and
+the coordination server distributes keys without being able to read what they protect.
+
+## ⛔ Not done — do not expose the console yet
+
+Three of #648's requirements remain, and each is a real gap rather than a formality:
+
+- **Session tokens over a remote origin.** The token is generated per `keel serve` run and never
+  written to disk, which is right for loopback. Nothing yet states its entropy against an
+  attacker who can reach the origin from the open internet, and there is no issuance
+  rate-limiting or brute-force posture — on loopback there was no attacker to rate-limit.
+- **Secure-context re-verification.** The service worker and manifest work today because
+  `http://127.0.0.1` is a secure context *by specification*. Over an external origin that
+  property comes from HTTPS instead, and the PWA behaviours have to be re-verified there rather
+  than assumed from the loopback behaviour.
+- **Bind opt-in beyond a mesh address.** `--host` accepts any address, and binding `0.0.0.0`
+  currently produces a server that refuses every request — `HostPolicy` then expects
+  `Host: 0.0.0.0`, which no browser sends. It fails closed, which is the safe direction, but it
+  fails confusingly and needs its own decision rather than this footnote.
+
+Until those land, `--external-host` is the mechanism waiting for the pass, not the pass.

keel/commands/serve.py

@@ -20,7 +20,7 @@
 import click
 
 from keel.commands._common import default_config_path, default_db_path
-from keel.web.security import new_session_token
+from keel.web.security import HostPolicy, new_session_token
 from keel.web.server import ServeConfig, serve
 
 #: Not 8080. Freqtrade's FreqUI and Jesse's dashboard both sit there, and an operator running one
@@ -40,8 +40,25 @@
     show_default=True,
     help="Open the URL in your default browser.",
 )
+@click.option(
+    "--external-host",
+    "external_hosts",
+    multiple=True,
+    metavar="HOSTNAME",
+    help=(
+        "A hostname a reverse proxy (e.g. a Cloudflare Tunnel) may present in Host:. "
+        "Repeatable. One specific name each -- wildcards are refused. Default: none, and "
+        "leaving it that way keeps this server loopback-only."
+    ),
+)
 @click.pass_context
-def serve_cmd(ctx: click.Context, host: str, port: int, open_browser: bool) -> None:
+def serve_cmd(
+    ctx: click.Context,
+    host: str,
+    port: int,
+    open_browser: bool,
+    external_hosts: tuple[str, ...],
+) -> None:
     """Serve keel's read-only view on localhost and open it in your browser.
 
     Read-only, by construction: the server implements GET and HEAD and nothing else, so there is
@@ -63,9 +80,22 @@ def serve_cmd(ctx: click.Context, host: str, port: int, open_browser: bool) -> N
     # forks a subprocess to answer "which build is this" would make the cheapest question on the
     # server the most expensive one.
     build = _build_info()
+    # #648. Normalised HERE rather than in `HostPolicy`, so the policy compares two values that
+    # are already in the same case and the comparison stays a plain set membership.
+    cleaned = frozenset(name.strip().lower() for name in external_hosts if name.strip())
+    # BUILT EAGERLY, and the reason is a bug this caught in its own first draft: `host_policy` is
+    # a lazy property, so `HostPolicy.__post_init__`'s wildcard guard fired on the first REQUEST
+    # rather than at startup -- a `--external-host '*.example.com'` server started cleanly, said
+    # nothing, and would have raised somewhere inside a handler. Constructing one here turns that
+    # into the command refusing to start, which is what the guard was written to mean.
+    try:
+        HostPolicy(bound_host=host, port=port, external_hosts=cleaned)
+    except ValueError as exc:
+        raise click.BadParameter(str(exc), param_hint="--external-host") from exc
     cfg = ServeConfig(
         host=host,
         port=port,
+        external_hosts=cleaned,
         token=new_session_token(),
         db_path=obj.get("db_path") or default_db_path(),
         config_path=obj.get("config_path") or default_config_path(),

keel/web/security.py

@@ -108,6 +108,29 @@
 #: 127.0.0.1 -- is rejected, which is the entire point of checking the header at all.
 _LOOPBACK_NAMES = frozenset({"127.0.0.1", "localhost", "::1", "[::1]"})
 
+#: Spellings that mean "stop checking". Refused at construction rather than at request time so a
+#: configuration that would disable the defence fails when the server STARTS -- visibly, once --
+#: instead of quietly answering everything for as long as it runs.
+_WILDCARD_NAMES = frozenset({"*", "", "any", "all", "0.0.0.0", "::", "[::]"})
+
+
+def _reject_wildcard(name: str) -> None:
+    """Refuse an external host that is not one specific name.
+
+    The DNS-rebinding defence works by naming what is expected. A wildcard is not a wider
+    expectation, it is the absence of one, and a `*` here would answer `evil.example` exactly as
+    readily as the operator's own domain -- which is the whole attack. A leading-dot suffix
+    (`.example.com`) is refused for the same reason: it admits every subdomain an attacker can
+    provision, and a tunnel presents ONE name.
+    """
+    cleaned = name.strip().lower()
+    if cleaned in _WILDCARD_NAMES or "*" in cleaned or cleaned.startswith("."):
+        raise ValueError(
+            f"external host {name!r} is a wildcard, not a name. The DNS-rebinding defence works "
+            "by naming exactly what is expected; a wildcard removes the check rather than "
+            "widening it. List the tunnel's own hostname."
+        )
+
 
 def new_session_token() -> str:
     """A fresh token for one `keel serve` run. Never written to disk: a token that outlives the
@@ -192,6 +215,23 @@ class HostPolicy:
 
     bound_host: str
     port: int
+    #: Names a REVERSE PROXY may legitimately present that this server never bound (#648).
+    #:
+    #: A Cloudflare Tunnel forwards to loopback and passes the app's PUBLIC domain in `Host:`, so
+    #: the rebinding check refuses it -- correctly, and for exactly the same reason it refuses
+    #: `evil.example`. From inside the process the two are indistinguishable: both are names that
+    #: resolve to a machine this server did not bind. Only the OPERATOR can tell them apart, so
+    #: only the operator can name one, one at a time, in configuration.
+    #:
+    #: ⚠️ This EXTENDS the defence and never replaces it. Empty by default; a name is admitted
+    #: only by being listed; there is no wildcard and no "any" -- `_reject_wildcard` refuses the
+    #: spellings someone reaches for when a specific name is inconvenient. The port check still
+    #: applies to a proxied request the same as to a direct one.
+    external_hosts: frozenset[str] = frozenset()
+
+    def __post_init__(self) -> None:
+        for name in self.external_hosts:
+            _reject_wildcard(name)
 
     @property
     def is_loopback(self) -> bool:
@@ -207,6 +247,11 @@ def permits(self, host_header: str | None) -> bool:
         name, port = split_host_header(host_header)
         if port is not None and port != str(self.port):
             return False
+        # Checked BEFORE the bind-derived rules, and it changes neither: an allowlisted name is
+        # an addition to what the bind permits, so the loopback and explicit-address branches
+        # below answer exactly what they answered before this field existed.
+        if name.lower() in self.external_hosts:
+            return True
         if self.is_loopback:
             return name in _LOOPBACK_NAMES
         return name == self.bound_host

keel/web/server.py

@@ -102,10 +102,15 @@ class ServeConfig:
     #: `Any` rather than the real type for the same reason `api.load_config` returns `Any` -- this
     #: module names service objects loosely so that importing `keel/web/` stays cheap.
     build_info: Any = None
+    #: Hostnames a reverse proxy may present that this server never bound (#648). Empty by
+    #: default -- loopback-only is the posture, and remaining the posture is the point.
+    external_hosts: frozenset[str] = frozenset()
 
     @property
     def host_policy(self) -> HostPolicy:
-        return HostPolicy(bound_host=self.host, port=self.port)
+        return HostPolicy(
+            bound_host=self.host, port=self.port, external_hosts=self.external_hosts
+        )
 
     def url(self) -> str:
         host = f"[{self.host}]" if ":" in self.host else self.host

tests/web/test_security.py

@@ -182,3 +182,86 @@ def test_the_cookie_outlives_the_browser_and_dies_with_the_run() -> None:
         "two serve runs handed out the same cookie -- the session token is being reused across "
         "processes, which is the persisted-secret design #634 declined"
     )
+
+
+# -- external hosts: a tunnel's own name, and nothing wider (#648) --------------------------------
+
+
+def test_an_allowlisted_external_host_is_admitted() -> None:
+    """A Cloudflare Tunnel forwards to loopback and presents the app's PUBLIC domain.
+
+    From inside the process that is indistinguishable from a rebinding attempt -- both are names
+    that resolve to a machine this server did not bind. Only the operator can tell them apart,
+    which is why the name has to be configured rather than inferred.
+    """
+    policy = HostPolicy(
+        bound_host="127.0.0.1", port=8765, external_hosts=frozenset({"keel.example.com"})
+    )
+    assert policy.permits("keel.example.com:8765")
+    assert policy.permits("keel.example.com")
+
+
+def test_allowlisting_one_name_admits_no_other() -> None:
+    """**The pin that matters.** The defence is not weakened, it is extended by exactly one name."""
+    policy = HostPolicy(
+        bound_host="127.0.0.1", port=8765, external_hosts=frozenset({"keel.example.com"})
+    )
+    assert not policy.permits("evil.example:8765")
+    assert not policy.permits("keel.example.com.evil.example:8765")
+    assert not policy.permits("sub.keel.example.com:8765")
+
+
+def test_the_loopback_rules_are_unchanged_by_an_allowlist() -> None:
+    """Adding an external name must not disturb what the bind already permitted, in either
+    direction: every loopback spelling still answers, and a rebinding attempt still does not."""
+    policy = HostPolicy(
+        bound_host="127.0.0.1", port=8765, external_hosts=frozenset({"keel.example.com"})
+    )
+    for host in ("127.0.0.1:8765", "localhost:8765", "[::1]:8765"):
+        assert policy.permits(host), host
+    assert not policy.permits("evil.example:8765")
+    assert not policy.permits(None)
+
+
+def test_the_port_check_still_applies_to_a_proxied_request() -> None:
+    """A proxied request is not exempt from being addressed to THIS server."""
+    policy = HostPolicy(
+        bound_host="127.0.0.1", port=8765, external_hosts=frozenset({"keel.example.com"})
+    )
+    assert not policy.permits("keel.example.com:9000")
+
+
+def test_an_external_host_is_matched_case_insensitively() -> None:
+    """DNS is case-insensitive and a proxy may present any casing. Refusing on case would be a
+    defence that fails open in the operator's head -- they configured the name, it looks right,
+    and requests are refused for a reason nothing reports."""
+    policy = HostPolicy(
+        bound_host="127.0.0.1", port=8765, external_hosts=frozenset({"keel.example.com"})
+    )
+    assert policy.permits("KEEL.Example.COM:8765")
+
+
+@pytest.mark.parametrize(
+    "wildcard", ["*", "*.example.com", ".example.com", "0.0.0.0", "::", "any", "all", ""]
+)
+def test_a_wildcard_external_host_is_refused_at_construction(wildcard: str) -> None:
+    """A wildcard is not a wider expectation -- it is the ABSENCE of one.
+
+    The rebinding defence works by naming what is expected, so `*` would answer `evil.example`
+    exactly as readily as the operator's own domain, which is the attack itself. A leading-dot
+    suffix is refused for the same reason: it admits every subdomain an attacker can provision,
+    and a tunnel presents one name.
+
+    Refused at CONSTRUCTION, not per request, so a configuration that would disable the defence
+    fails when the server starts -- visibly, once -- instead of quietly answering everything for
+    as long as it runs.
+    """
+    with pytest.raises(ValueError, match="wildcard"):
+        HostPolicy(bound_host="127.0.0.1", port=8765, external_hosts=frozenset({wildcard}))
+
+
+def test_no_external_hosts_is_the_default() -> None:
+    """Loopback-only is the posture, and remaining the posture unless someone types a name is
+    the point. A default that admitted anything would make every other test here decoration."""
+    assert HostPolicy(bound_host="127.0.0.1", port=8765).external_hosts == frozenset()
+    assert not HostPolicy(bound_host="127.0.0.1", port=8765).permits("keel.example.com:8765")