Skip to content

Commit f37bd19

Browse files
eaitbrahimclaude
andauthored
Console: serve the static shell past the remote lifetime to its own session (#1006) (#1008)
* Console: serve the static shell past the remote lifetime to its own session (#1006) On `keel serve --external-host`, a browser with no cached console was refused the page itself after the 12 h lifetime (#648), so it never reached the expired-Pulse HALT that #985 serves. A GET/HEAD of a shell path -- a shipped file under the static root or a client route, by the same resolution `_serve_static` performs -- carrying THIS session's cookie now skips the lifetime (`KeelHandler._is_a_sessioned_shell_read`, called from `_admitted`; `do_GET` is untouched). The shell is the package's files unaltered: no data, no token. Every `/api/` read (events included), `/switch/`, unmapped paths and every write keep the lifetime; a request without the cookie is refused the shell as before, so an expired session is never more open to a stranger than a live one. The token exchange already ran before admission, so a first visit via the printed address gets the cookie and then the page. Docstrings, docs/remote-access.md and the prose test that used `/` to show the lifetime are updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): exempt the shell by listing its paths, not resolving the request (#1006) CodeQL counted a second user-controlled flow into resolve_static_asset. _is_a_sessioned_shell_read now compares the unquoted request path against staticfiles.shell_request_paths, enumerated from the package side, so the request path never reaches a filesystem call. Adds the HEAD pin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent f8e80b4 commit f37bd19

7 files changed

Lines changed: 345 additions & 33 deletions

File tree

‎docs/remote-access.md‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,20 @@ URL, in terminal scrollback, and in whatever got pasted while asking for help. T
8181

8282
So a server configured with `--external-host` enforces a **12-hour session lifetime on its own
8383
side of the wire**. It is checked before the token, so an expired session and a wrong one get the
84-
same refusal. There are two exceptions, and both are for the stop. A halt that carries the
85-
session cookie and the halt token is accepted past the lifetime (#983): a console left running
86-
overnight must still be able to stop trading. And a read of Pulse (`/api/pulse`) that carries
87-
this session's cookie is refused as expired, but the refusal carries the HALT control's inputs,
88-
so a console reloaded after the lifetime still shows HALT (#985). Only the holder of the real
89-
session token sees the difference. Nothing else is reopened: no release, no setup write, no other
90-
read, and the release token is still served nowhere. A browser with no cached console is refused
91-
the page itself; it can stop trading with `keel kill` at the terminal. A loopback-only server has
92-
no lifetime, and that asymmetry is the argument rather than an exemption. Restarting `keel serve`
93-
remains the instant revocation gesture in both postures.
84+
same refusal. There are three exceptions, and all of them are for the stop. A halt that carries
85+
the session cookie and the halt token is accepted past the lifetime (#983): a console left running
86+
overnight must still be able to stop trading. A read of Pulse (`/api/pulse`) that carries this
87+
session's cookie is refused as expired, but the refusal carries the HALT control's inputs, so a
88+
console reloaded after the lifetime still shows HALT (#985). And the console's own files (the
89+
page, its scripts, stylesheet and icons) are served past the lifetime to a request carrying this
90+
session's cookie (#1006), so a browser with no cached console — or one that opens the address
91+
`keel serve` printed, which sets the cookie — loads the page and reaches that HALT. Those files
92+
are the package's own, unaltered: no data and no token. Only the holder of the real session token
93+
sees any of the difference. Nothing else is reopened: no release, no setup write, no other read,
94+
and the release token is still served nowhere. A browser without the cookie is refused the page
95+
exactly as before the lifetime ended; it can stop trading with `keel kill` at the terminal. A
96+
loopback-only server has no lifetime, and that asymmetry is the argument rather than an
97+
exemption. Restarting `keel serve` remains the instant revocation gesture in both postures.
9498

9599
## Capability-increasing actions are refused unless the bind is loopback
96100

‎keel/web/gates.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,11 @@ class UngatedAction:
203203
#: serves a console still showing a Pulse from before the lifetime ended, whose memory holds the
204204
#: token, or any client that holds it. A RELOADED console gets the token from `/api/pulse`'s
205205
#: expiry refusal, which carries `{halt_token, resume_command}` to a request bearing this
206-
#: session's cookie and to nothing else (`server._expired_pulse_halt`, #985); that refusal still
207-
#: names `keel kill`. A leaked halt token can at worst halt, which `keel resume` at
208-
#: a terminal undoes, and it opens no release (#968).
206+
#: session's cookie and to nothing else (`server._expired_pulse_halt`, #985); the shell that
207+
#: reads it is served past the lifetime to that cookie too, so a browser with no cached console
208+
#: reaches it (`server._is_a_sessioned_shell_read`, #1006). That refusal still names
209+
#: `keel kill`. A leaked halt token can at worst halt, which `keel resume` at a terminal
210+
#: undoes, and it opens no release (#968).
209211
HALT = UngatedAction(operation=engage_kill_switch, done=KILL_ENGAGED_LINE)
210212

211213

‎keel/web/security.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,10 @@
130130
#: the token. A reloaded console gets the token from the expiry refusal itself: on `/api/pulse`,
131131
#: and for a request carrying this session's cookie only, that refusal carries the HALT control's
132132
#: `{halt_token, resume_command}` (`server.KeelHandler._expired_pulse_halt`, #985). It re-opens
133-
#: no read and no other endpoint serves the token. The refusal still names `keel kill`, for a
134-
#: browser with no cached shell, which is refused the page that would draw the button.
133+
#: no read and no other endpoint serves the token. A browser with no cached shell is served the
134+
#: shell's static files past the lifetime, for this session's cookie only, so it reaches that
135+
#: refusal (`server.KeelHandler._is_a_sessioned_shell_read`, #1006); they carry no data and no
136+
#: token. The refusal still names `keel kill`, for a browser without the cookie.
135137
REMOTE_SESSION_MAX_AGE_SECONDS = 12 * 60 * 60
136138

137139
#: ⛔ THE BRUTE-FORCE ARITHMETIC, WRITTEN DOWN SO NOBODY ADDS A RATE LIMITER FOR THE WRONG

‎keel/web/server.py‎

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
5858
from pathlib import Path
5959
from typing import Any
60-
from urllib.parse import parse_qs, urlsplit
60+
from urllib.parse import parse_qs, unquote, urlsplit
6161

6262
from keel.web import api, events, runtime, staticfiles
6363
from keel.web.security import (
@@ -708,7 +708,10 @@ def _admitted(self, *, outlives_the_lifetime: bool = False) -> bool:
708708
709709
`outlives_the_lifetime` skips the LIFETIME and nothing else, and exactly one caller sets
710710
it: `_post`, for a halt that carries this session's halt token (`_is_a_tokened_halt`,
711-
#983). Every other request -- a release, a setup write, every read -- keeps it.
711+
#983). The one other request that skips it is decided here rather than by `do_GET`, so
712+
that handler is untouched: a read of the shell carrying this session's cookie
713+
(`_is_a_sessioned_shell_read`, #1006). Every other request -- a release, a setup write,
714+
every `/api/` read -- keeps it.
712715
"""
713716
if not self.cfg.host_policy.permits(self.headers.get("Host")):
714717
# DNS rebinding lands exactly here: the packet arrived on loopback, so the bind check
@@ -721,13 +724,16 @@ def _admitted(self, *, outlives_the_lifetime: bool = False) -> bool:
721724
)
722725
return False
723726
# #648: an ENFORCED lifetime, checked before the token so an expired session cannot be
724-
# distinguished from a wrong one by which refusal comes back -- with one exception since
725-
# #985: on a GET or HEAD of `/api/pulse` a request bearing THIS session's cookie gets a
726-
# refusal that carries `halt`, and a wrong or missing cookie gets `halt: null`. That is
727-
# acceptable because only the holder of the real 256-bit session token can see the
728-
# difference, and it tells them nothing they could not already use (`_is_a_tokened_halt`
729-
# admits that halt anyway). Every other request still cannot tell the two apart. Only a
730-
# remote-configured server has a lifetime -- see `ServeConfig.session_expired_at`.
727+
# distinguished from a wrong one by which refusal comes back -- with two exceptions. Since
728+
# #985, on a GET or HEAD of `/api/pulse` a request bearing THIS session's cookie gets a
729+
# refusal that carries `halt`, and a wrong or missing cookie gets `halt: null`. Since
730+
# #1006, a GET or HEAD of the shell bearing THIS session's cookie is served it, and a
731+
# wrong or missing cookie is refused as expired. Both are acceptable because only the
732+
# holder of the real 256-bit session token can see the difference, and it tells them
733+
# nothing they could not already use (`_is_a_tokened_halt` admits that halt anyway, and
734+
# the shell is the package's files). Every other request still cannot tell the two
735+
# apart. Only a remote-configured server has a lifetime -- see
736+
# `ServeConfig.session_expired_at`.
731737
#
732738
# #983, the owner's decision: the halt is exempt. A console left running past the lifetime
733739
# refused its own stop, and refusing a stop is the one failure mode this surface must not
@@ -738,10 +744,17 @@ def _admitted(self, *, outlives_the_lifetime: bool = False) -> bool:
738744
# It serves a console still showing a Pulse from before the lifetime ended (the token is
739745
# in its memory), or any client that holds the token. A RELOADED console gets the token
740746
# back from this refusal itself, on `/api/pulse` and for this session's cookie only
741-
# (#985, `_expired_pulse_halt`). The refusal still names `keel kill`: a browser with no
742-
# cached shell is refused the page that would draw the button.
747+
# (#985, `_expired_pulse_halt`). A browser with no cached shell gets the page from this
748+
# server past the lifetime too, for this session's cookie only (#1006,
749+
# `_is_a_sessioned_shell_read`), so it reaches that refusal. The refusal still names
750+
# `keel kill`, for a browser without the cookie, which is refused the page as before.
743751
expires = self.cfg.session_expired_at
744-
if expires is not None and not outlives_the_lifetime and time.time() >= expires:
752+
if (
753+
expires is not None
754+
and not outlives_the_lifetime
755+
and time.time() >= expires
756+
and not self._is_a_sessioned_shell_read()
757+
):
745758
self._refuse(
746759
403,
747760
"Session expired",
@@ -778,6 +791,52 @@ def _admitted(self, *, outlives_the_lifetime: bool = False) -> bool:
778791
return False
779792
return True
780793

794+
def _is_a_sessioned_shell_read(self) -> bool:
795+
"""Whether this request is a GET (or HEAD) of the client shell carrying this session's
796+
cookie -- the read the remote session lifetime does not bind (#1006).
797+
798+
**Why.** Since #985 an expired `/api/pulse` refusal carries the HALT control to this
799+
session's cookie, but only a console that is already loaded can read it. A browser with no
800+
cached console -- a first visit after the lifetime, through the printed `?token=` address,
801+
or a browser whose cached files were cleared -- was refused the page itself, so it never
802+
drew the button and its only stop was `keel kill` at a terminal.
803+
804+
**What it re-opens: files, never data.** The shell is `staticfiles.STATIC_ROOT` byte for
805+
byte -- `_serve_static` reads and sends it unaltered, there is no template -- so it carries
806+
no reading of the ledger and no token; the halt token still comes only from the
807+
cookie-matched Pulse refusal (#985), and `gates_token` is served nowhere (#968). The test
808+
is membership in `staticfiles.shell_request_paths`, the paths `_serve_static` answers with
809+
a file: a shipped asset or a name in `CLIENT_ROUTES` / `RETIRED_CLIENT_ROUTES`. The request
810+
path is only compared against that package-side list, never passed to a filesystem call,
811+
so this adds no second user-controlled path flow. The list is walked per call, which is
812+
acceptable because this is reached only once the lifetime has expired. A spelling
813+
`_serve_static` would still resolve (`/js/../js/main.js`) is not in the list, so it is
814+
NOT exempted and is refused as expired: stricter, and fine. Everything under `API_PREFIX`
815+
(`EVENTS_PATH` included) and `SWITCH_PREFIX` is excluded by name first, so no later file
816+
under the static root can shadow one; an unmapped path keeps the lifetime and is refused as
817+
expired, not 404ed.
818+
819+
**Why this session's cookie, and not any browser (#1006's held question, ruled here).**
820+
Before the lifetime, a request without the cookie is refused the shell
821+
(`test_static_assets_are_behind_the_same_admission_as_every_other_page`: "not exempted
822+
... just because it holds no secrets today"). Serving it to anyone after expiry would make
823+
an EXPIRED session more open to a stranger than a live one, and a browser without the
824+
cookie gains nothing from the page: the Pulse refusal draws HALT only for the cookie. A
825+
browser that has only the printed address gets the cookie from the token exchange, which
826+
`do_GET` performs before admission, so the first visit is covered. So, as for #985: only
827+
the holder of the real session token can see the difference, and it tells them nothing
828+
they could not already use.
829+
"""
830+
if self.command not in ("GET", "HEAD"):
831+
return False
832+
path = urlsplit(self.path).path
833+
if path.startswith((API_PREFIX, SWITCH_PREFIX)):
834+
return False
835+
if unquote(path) not in staticfiles.shell_request_paths(staticfiles.STATIC_ROOT):
836+
return False
837+
cookies = parse_cookie_header(self.headers.get("Cookie"))
838+
return tokens_match(cookies.get(SESSION_COOKIE), self.cfg.token)
839+
781840
def _expired_pulse_halt(self) -> dict[str, str] | None:
782841
"""The HALT control's inputs for the expired-session refusal, or `None` (#985).
783842

‎keel/web/staticfiles.py‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,12 @@ def resolve_client_route(root: Path, url_path: str) -> Path | None:
193193
**This is a read, and it widens nothing.** It maps the names in `CLIENT_ROUTES` and
194194
`RETIRED_CLIENT_ROUTES` onto one file that is already served at
195195
`STATIC_PREFIX + CLIENT_ENTRY`; the same session cookie and the same `Host` check gate it,
196-
because `server.do_GET` checks both before any static path is looked at.
196+
because `server.do_GET` checks both before any static path is looked at. The one difference
197+
from an `/api/` read is the remote session lifetime: a path this function or
198+
`resolve_static_asset` answers is served past it to this session's cookie (#1006,
199+
`server.KeelHandler._is_a_sessioned_shell_read`, which asks `shell_request_paths`, the list of
200+
the paths these two functions answer), because the shell is files and the expired Pulse
201+
refusal it reads is what carries the HALT (#985).
197202
"""
198203
if not url_path.startswith(STATIC_PREFIX):
199204
return None
@@ -209,3 +214,28 @@ def resolve_client_route(root: Path, url_path: str) -> Path | None:
209214
if not shell.is_file():
210215
return None
211216
return shell
217+
218+
219+
def shell_request_paths(root: Path) -> frozenset[str]:
220+
"""Every request path the shell answers, spelled out (#1006).
221+
222+
`STATIC_PREFIX + relpath` for each file under `root` that `content_type_for` can label (the
223+
files `resolve_static_asset` would serve), `STATIC_PREFIX` itself, and `STATIC_PREFIX` plus
224+
each name in `CLIENT_ROUTES` and `RETIRED_CLIENT_ROUTES` (the paths `resolve_client_route`
225+
answers with the shell).
226+
227+
**Why enumerate rather than resolve.** `server.KeelHandler._is_a_sessioned_shell_read` asks
228+
"is this request path one the shell answers" and compares against this set, so the request
229+
path is only ever COMPARED, never handed to a filesystem call: what is built here comes from
230+
the package's own directory listing. Resolving the path instead would be a second
231+
user-controlled flow into `resolve_static_asset`. The walk is per call, which is cheap
232+
enough because its one caller is reached only once the remote session lifetime has expired.
233+
"""
234+
names = {
235+
STATIC_PREFIX + path.relative_to(root).as_posix()
236+
for path in root.rglob("*")
237+
if path.is_file() and content_type_for(path) is not None
238+
}
239+
names.add(STATIC_PREFIX)
240+
names.update(STATIC_PREFIX + name for name in (*CLIENT_ROUTES, *RETIRED_CLIENT_ROUTES))
241+
return frozenset(names)

0 commit comments

Comments
 (0)