feat(web): a remote session expires, and a wildcard bind must say what it expects (#648)

docs/remote-access.md

@@ -3,9 +3,9 @@
 `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.
+**Status: incomplete. Nothing remote should be exposed yet.** Four of #648's five requirements
+are met. The last one — re-verifying the PWA's secure-context behaviour over a real HTTPS
+origin — cannot be done from a checkout, and it is named at the bottom rather than waved past.
 
 ## What the defence actually defends
 
@@ -62,21 +62,47 @@ 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.
 
+## The session, once a remote host is configured
+
+**There is no brute-force threat, and saying so is the point.** The token carries 256 bits of
+entropy (`secrets.token_urlsafe(32)`), a space of about 1.2 × 10⁷⁷. An attacker managing a
+billion guesses a second — which no `http.server` on a laptop will serve — needs on the order of
+10⁶⁰ years to cover a meaningful fraction. A rate limiter installed to stop guessing would be
+theatre: state, a failure mode, and a false sense that something was closed. The arithmetic is
+recorded beside the constant (`TOKEN_ENTROPY_BITS`) so it travels with the claim. If a limiter is
+ever added it must be justified by bounding log volume or making probing visible — never by
+brute force.
+
+**What a remote origin does change is who can use a token that leaked.** On loopback that
+population is software already running as the operator, and no session lifetime helps against
+it. Through a tunnel it becomes anyone who can reach the origin — and the token has been in a
+URL, in terminal scrollback, and in whatever got pasted while asking for help. The 30-day cookie
+`Max-Age` is a *browser* hint that such an attacker ignores entirely.
+
+So a server configured with `--external-host` enforces a **12-hour session lifetime on its own
+side of the wire**, checked before the token so an expired session cannot be told apart from a
+wrong one by which refusal comes back. A loopback-only server has none, and that asymmetry is
+the argument rather than an exemption. Restarting `keel serve` remains the instant revocation
+gesture in both postures.
+
+## Binding every interface
+
+`--host 0.0.0.0` (or `::`) used to produce a server that refused **every** request: `HostPolicy`
+would then expect `Host: 0.0.0.0`, which no browser sends. It failed closed — the right
+direction, the wrong explanation, and the operator's conclusion was "keel is broken" rather than
+"keel does not know which name to expect".
+
+A wildcard bind is precisely the case where the name cannot be derived, because every interface
+has a different one. So it is the one bind that requires stating it, and `keel serve` now refuses
+to start on a wildcard with no `--external-host` — once, at the moment it can be acted on.
+
 ## ⛔ Not done — do not expose the console yet
 
-Three of #648's requirements remain, and each is a real gap rather than a formality:
+One requirement remains, and it is not 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.
+  than assumed from the loopback behaviour. That needs a real deployed origin, so it cannot be
+  closed from a checkout — and until it is, an installed console reached through a tunnel is
+  untested, not merely unsupported.

keel/commands/serve.py

@@ -29,6 +29,11 @@
 
 DEFAULT_HOST = "127.0.0.1"
 
+#: Binds that answer on EVERY interface, where no single hostname can be derived from the
+#: address (#648). Not a blocklist -- each is permitted the moment `--external-host` says what
+#: to expect.
+_WILDCARD_BINDS = frozenset({"0.0.0.0", "::", "[::]", "*"})
+
 
 @click.command("serve")
 @click.option("--host", default=DEFAULT_HOST, show_default=True, help="Address to bind.")
@@ -92,6 +97,27 @@ def serve_cmd(
         HostPolicy(bound_host=host, port=port, external_hosts=cleaned)
     except ValueError as exc:
         raise click.BadParameter(str(exc), param_hint="--external-host") from exc
+
+    # #648, and this is a DECISION about a confusing failure rather than a new restriction.
+    # Binding a wildcard produced a server that refused every request: `HostPolicy` then expects
+    # `Host: 0.0.0.0`, which no browser ever sends, so the bind succeeded, the URL printed, and
+    # nothing worked with a 403 that blamed the address. It failed CLOSED, which is the right
+    # direction and the wrong explanation -- the operator's conclusion is "keel is broken", not
+    # "keel does not know what name to expect".
+    #
+    # A wildcard bind is exactly the case where the name cannot be derived, because the server
+    # is answering on every interface and each has a different one. So it is the one bind that
+    # REQUIRES the name to be stated. Refusing here says that, once, at the moment it can be
+    # acted on.
+    if host in _WILDCARD_BINDS and not cleaned:
+        raise click.BadParameter(
+            f"binding {host} answers on every interface, so keel cannot derive which hostname "
+            "to expect in Host: -- and a server that expects the wrong one refuses every "
+            "request with a 403 that looks like a fault. Name the hostname browsers will use, "
+            "e.g. `--external-host keel.example.com`, or bind that address directly. See "
+            "docs/remote-access.md before exposing this server at all.",
+            param_hint="--host",
+        )
     cfg = ServeConfig(
         host=host,
         port=port,

keel/web/security.py

@@ -91,6 +91,36 @@
 #: does not accumulate an entry with no end at all.
 SESSION_COOKIE_MAX_AGE_SECONDS = 30 * 24 * 60 * 60
 
+#: How long a session may authenticate once a REMOTE origin is configured (#648). Twelve hours:
+#: long enough that a working day does not end in a re-authorisation, short enough that a token
+#: which leaked this morning does not still work tomorrow.
+#:
+#: ⚠️ This does NOT apply to a loopback-only server, and the asymmetry is the argument rather
+#: than an exemption. `SESSION_COOKIE_MAX_AGE_SECONDS`'s reasoning above is sound and unchanged:
+#: the token's power is bounded by a process the browser does not control, and on loopback the
+#: population who could USE a leaked copy is "software already running as this operator" --
+#: against which a shorter session buys nothing at all.
+#:
+#: A remote origin changes that population to "anyone who can reach the tunnel", and the token
+#: has been in a URL, in terminal scrollback, and in whatever the operator pasted while asking
+#: for help. The 30-day cookie is a BROWSER hint an attacker with the token ignores entirely, so
+#: the bound has to be enforced on this side of the wire or it is not a bound.
+REMOTE_SESSION_MAX_AGE_SECONDS = 12 * 60 * 60
+
+#: ⛔ THE BRUTE-FORCE ARITHMETIC, WRITTEN DOWN SO NOBODY ADDS A RATE LIMITER FOR THE WRONG
+#: REASON (#648). `_TOKEN_BYTES = 32` is 256 bits, so `secrets.token_urlsafe` draws from a space
+#: of 2**256 ~ 1.2e77. An attacker managing a billion guesses per second -- which no HTTP server
+#: on a laptop will serve -- needs on the order of 1e60 years to cover a meaningful fraction.
+#:
+#: Guessing is therefore NOT a threat this server has, and a rate limiter installed to stop it
+#: would be theatre: it would add state, a failure mode, and a false sense that something was
+#: closed. What bounds risk here is entropy, which is already past any margin that matters.
+#:
+#: What a limiter WOULD buy is unrelated to guessing -- bounding log volume from a scanner, and
+#: making probing visible. Those are real, and if one is ever added it must be justified by
+#: those and not by brute force. Recorded as a constant so the number travels with the claim.
+TOKEN_ENTROPY_BITS = 256
+
 #: The request header carrying the CSRF token on a write (#540).
 #:
 #: A HEADER rather than a body field, and the difference is the whole reason this layer still

keel/web/server.py

@@ -43,8 +43,9 @@
 import json
 import socket
 import sys
+import time
 from collections.abc import Callable
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
 from pathlib import Path
 from typing import Any
@@ -53,6 +54,7 @@
 from keel.web import api, events, staticfiles
 from keel.web.security import (
     CSRF_HEADER,
+    REMOTE_SESSION_MAX_AGE_SECONDS,
     SESSION_COOKIE,
     HostPolicy,
     csrf_token,
@@ -105,6 +107,23 @@ class ServeConfig:
     #: 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()
+    #: When this run began, for the remote session lifetime (#648). Defaults to the moment the
+    #: config is built, which is the moment `keel serve` mints the token it bounds.
+    started_at: float = field(default_factory=time.time)
+
+    @property
+    def session_expired_at(self) -> float | None:
+        """When this session stops authenticating, or `None` if it does not expire.
+
+        `None` on a loopback-only server, and that is a decision rather than an omission --
+        `REMOTE_SESSION_MAX_AGE_SECONDS` carries the argument. The population who could use a
+        leaked token on loopback is software already running as this operator, and no session
+        lifetime helps against that; a remote origin changes the population to anyone who can
+        reach the tunnel, and the 30-day cookie is a browser hint such an attacker ignores.
+        """
+        if not self.external_hosts:
+            return None
+        return self.started_at + REMOTE_SESSION_MAX_AGE_SECONDS
 
     @property
     def host_policy(self) -> HostPolicy:
@@ -485,6 +504,20 @@ def _admitted(self) -> bool:
                 "This request did not come from the address keel is serving on.",
             )
             return False
+        # #648: an ENFORCED lifetime, checked before the token so an expired session cannot be
+        # distinguished from a wrong one by which refusal comes back. Only a remote-configured
+        # server has one -- see `ServeConfig.session_expired_at`.
+        expires = self.cfg.session_expired_at
+        if expires is not None and time.time() >= expires:
+            self._refuse(
+                403,
+                "Session expired",
+                "This session has reached its lifetime. keel enforces one when it is configured "
+                "to answer a remote hostname, because a token that leaked into a URL, a "
+                "screenshot or terminal scrollback should not still work tomorrow. Restart "
+                "`keel serve` and open the address it prints.",
+            )
+            return False
         cookies = parse_cookie_header(self.headers.get("Cookie"))
         if not tokens_match(cookies.get(SESSION_COOKIE), self.cfg.token):
             self._refuse(

tests/web/test_serve_command.py

@@ -9,6 +9,7 @@
 from click.testing import CliRunner
 
 from keel.cli import cli
+from keel.web import security
 from keel.web import server as web_server
 from keel.web.security import new_session_token
 
@@ -239,3 +240,108 @@ def test_an_empty_flag_value_is_dropped_rather_than_refused(
     assert result.exit_code == 0, result.output
     assert cfg is not None
     assert cfg.external_hosts == frozenset()
+
+
+# -- a wildcard bind must name what it expects (#648) ---------------------------------------------
+
+
+@pytest.mark.parametrize("wildcard", ["0.0.0.0", "::", "[::]"])
+def test_a_wildcard_bind_without_a_named_host_refuses_to_start(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path, wildcard: str
+) -> None:
+    """The confusing failure, turned into an explanation.
+
+    Binding a wildcard used to produce a server that refused EVERY request: `HostPolicy` then
+    expects `Host: 0.0.0.0`, which no browser sends, so the bind succeeded, the URL printed, and
+    nothing worked behind a 403 that blamed the address. It failed closed -- right direction,
+    wrong explanation. The operator concludes "keel is broken", not "keel does not know which
+    name to expect".
+    """
+    result, _cfg = _policy_from_cli(monkeypatch, tmp_path, "--host", wildcard)
+
+    assert result.exit_code != 0, result.output
+    assert "every interface" in result.output
+
+
+def test_a_wildcard_bind_is_permitted_once_a_host_is_named(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """Not a blocklist. A wildcard bind is exactly the case where the name cannot be derived --
+    every interface has a different one -- so it is the one bind that requires stating it."""
+    result, cfg = _policy_from_cli(
+        monkeypatch, tmp_path, "--host", "0.0.0.0", "--external-host", "keel.example.com"
+    )
+
+    assert result.exit_code == 0, result.output
+    assert cfg is not None
+    assert cfg.host_policy.permits("keel.example.com:8765")
+
+
+def test_a_specific_non_loopback_bind_is_untouched(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """`--host 10.0.0.5` derives its own name and always worked. This must not become collateral
+    damage of the wildcard refusal."""
+    result, cfg = _policy_from_cli(monkeypatch, tmp_path, "--host", "10.0.0.5")
+
+    assert result.exit_code == 0, result.output
+    assert cfg is not None
+    assert cfg.host_policy.permits("10.0.0.5:8765")
+
+
+# -- the remote session lifetime (#648) -----------------------------------------------------------
+
+
+def test_a_loopback_server_has_no_session_expiry(tmp_path: Path) -> None:
+    """A decision, not an omission. On loopback the population who could use a leaked token is
+    software already running as this operator, and no session lifetime helps against that."""
+    cfg = web_server.ServeConfig(
+        host="127.0.0.1",
+        port=8765,
+        token=new_session_token(),
+        db_path=str(tmp_path / "keel.db"),
+        config_path=str(tmp_path / "config.yaml"),
+    )
+
+    assert cfg.session_expired_at is None
+
+
+def test_configuring_a_remote_host_bounds_the_session(tmp_path: Path) -> None:
+    """A remote origin changes the population to anyone who can reach the tunnel, and the
+    30-day cookie is a BROWSER hint an attacker holding the token ignores entirely -- so the
+    bound has to be enforced on this side of the wire or it is not a bound."""
+    cfg = web_server.ServeConfig(
+        host="127.0.0.1",
+        port=8765,
+        token=new_session_token(),
+        db_path=str(tmp_path / "keel.db"),
+        config_path=str(tmp_path / "config.yaml"),
+        external_hosts=frozenset({"keel.example.com"}),
+        started_at=1_000_000.0,
+    )
+
+    assert cfg.session_expired_at == 1_000_000.0 + security.REMOTE_SESSION_MAX_AGE_SECONDS
+
+
+def test_the_remote_lifetime_is_bounded_at_both_ends() -> None:
+    """A lifetime is only useful between two limits, and both are claims the docstring makes.
+
+    **Shorter than the cookie** the browser keeps, or the enforced bound never bites before the
+    browser stops sending the cookie anyway and the mechanism is decorative.
+
+    **Long enough for a working day**, or it is not a session lifetime but an outage: a value
+    small enough to expire mid-use would be indistinguishable, to the operator, from the
+    "refuses every request" failure the wildcard-bind refusal exists to prevent. A mutation run
+    set this to one second and every test still passed -- which is why the floor is here.
+    """
+    assert security.REMOTE_SESSION_MAX_AGE_SECONDS < security.SESSION_COOKIE_MAX_AGE_SECONDS
+    assert security.REMOTE_SESSION_MAX_AGE_SECONDS >= 8 * 60 * 60
+
+
+def test_the_entropy_arithmetic_is_recorded_beside_the_token() -> None:
+    """#648 asked for a brute-force posture and the honest answer is that there is no brute-force
+    threat: 256 bits is ~1.2e77 values. A rate limiter added to stop guessing would be theatre,
+    and this constant exists so the number travels with that claim instead of being re-derived
+    by whoever proposes one."""
+    assert security.TOKEN_ENTROPY_BITS == security._TOKEN_BYTES * 8
+    assert security.TOKEN_ENTROPY_BITS >= 128