Skip to content

Commit de5c326

Browse files
eaitbrahimclaude
andauthored
feat(web): delete the renderer -- the server serves files and JSON, and nothing else (#540) (#553)
`keel/web/render.py` is gone: 987 lines of server-side HTML, the seven page handlers that called it, and its inline stylesheet. The client moved from `/static/` to `/` and is the application now. Net -896 lines. ── THE WRITE SURFACE MOVED, AND THAT IS WHAT MADE A LAYER REAL ──────────────── `/setup/*` was an HTML form. It is `POST /api/setup/<key>` now, and the action SET is a 0-line diff: `keel.commands.setup.ACTIONS`, still only idempotent, non-destructive, `MECHANICAL` steps, still asserted disjoint from the eleven capability-increasing actions. A browser can set a deployment up; it still cannot arm a rule, attest an asset or enable autonomy. What changed is that `X-Keel-Client` now covers the write path. `_api_client_header_ok` recorded that widening as "#536's call to make, once (and only once) the forms it replaces are gone" -- because a `<form>` cannot set a header, and gating it would have refused every legitimate submission with no terminal to fall back to on the desktop bundle. The forms are gone. The CSRF token moved into `X-Keel-CSRF` for the same reason: in a body it would prove only that the sender could READ it; in a header it also proves the sender could set one, which a cross-origin form cannot do at all. ── A REVERSAL, RECORDED RATHER THAN QUIETLY APPLIED ─────────────────────────── `payload.setup_payload` used to argue against ever putting the token in a GET response: "minting a live write credential into a GET would put it into every cached copy, every proxy log and every paste". Two of those were already answered -- `/api/*` is `no-store` and #538's worker refuses to cache it, and the server binds loopback. The third does not survive the observation that settles it: this token authorises NOTHING without the session cookie, and anyone holding that cookie can mint it themselves. The old argument is quoted in place. ── TWO BUGS THE SUITE COULD NOT HAVE FOUND ──────────────────────────────────── **A refused write poisoned the next request on the same connection.** HTTP/1.1 keep-alive; a POST refused before its body was read left those bytes in the socket, and the stdlib parsed them as the next request line -- 501, on the request AFTER the one that was correctly refused. It became possible in this commit: moving the token to a header put the refusal in front of the body read. Every test in the suite opens a fresh connection, so none could see it. Found by driving a browser through five refusals in a row. `_drain_request_body` fixes it and disconnects rather than draining an oversized body, and a test now drives two requests down one socket. **An action's outcome was never visible.** The first version wrote the server's message into the card and then repainted the view, destroying it. The second restored it after the action's own repaint -- and the 15-second poll wiped it mid-sentence. Outcomes are session state now, re-applied in `rebuildInto`, which every path that replaces the view goes through. ── THE PIN TRANSFERRED, AND IT IS BIGGER THAN BEFORE ────────────────────────── `test_console_thinness` named `render` explicitly so its deletion would fail LOUDLY rather than shrink the scanned set. It did exactly that. The property -- "a THIRD renderer over the same reports, never a second place that computes them" -- belongs to the API layer now: `api`, `events`, `security`, `server` and `staticfiles` are pinned where three were. `keel/web/security.py`'s five layers are untouched. Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8a973e4 commit de5c326

22 files changed

Lines changed: 1188 additions & 2084 deletions

‎keel/web/api.py‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
from typing import TYPE_CHECKING, Any
4141

4242
from keel.web import payload
43+
from keel.web.security import csrf_token
4344

4445
if TYPE_CHECKING: # pragma: no cover - typing only
4546
from keel.web.server import ServeConfig
@@ -157,15 +158,24 @@ def read_status(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> di
157158
def read_setup(cfg: ServeConfig, _query: Query, state: Any, _now_ts: int) -> dict[str, Any]:
158159
"""The first-run checklist -- the ONE deployment-reading endpoint that must answer when there
159160
is no deployment, because it is the thing that says how to make one. `needs_database=False`
160-
for that reason, and `server.needs_database` serves the same page in HTML for the same one.
161+
for that reason: an endpoint that refused to answer until a deployment existed would refuse
162+
precisely the operator who has none.
161163
162164
`state` is the `DeploymentState` the envelope already read for `engine`, passed in rather than
163165
re-inspected: one 3.6 ms probe per response, not two."""
164166
from keel.commands import jobs
165167
from keel.commands.setup import ACTIONS, NOT_AUTOMATED_YET
166168

167169
return payload.setup_payload(
168-
state, actions=ACTIONS, not_automated=NOT_AUTOMATED_YET, job=jobs.status()
170+
state,
171+
actions=ACTIONS,
172+
not_automated=NOT_AUTOMATED_YET,
173+
job=jobs.status(),
174+
# The write token for this session, on the one endpoint whose view performs writes. Scoped
175+
# to that endpoint rather than put on `/api/config` (which every view reads at boot) so it
176+
# travels only to the page that needs it -- see `payload.setup_payload` for why it is in a
177+
# body at all, which was a reversal.
178+
csrf=csrf_token(cfg.token),
169179
)
170180

171181

@@ -607,6 +617,38 @@ def refusal_document(status: int, title: str, detail: str) -> dict[str, Any]:
607617
return payload.error_envelope(int(time.time()), status=status, title=title, detail=detail)
608618

609619

620+
def action_document(result: Any) -> dict[str, Any]:
621+
"""One completed setup action, as JSON (#540).
622+
623+
**`changed` is the field the client actually needs**, and it is not a success flag: it is the
624+
difference between "created" and "already there". `keel.commands.setup`'s own note on it calls
625+
it "the property that makes a double-click safe" -- every action is idempotent, so a repeated
626+
submission succeeds and reports `changed: false`, which is a true statement about the
627+
deployment rather than a soft failure.
628+
629+
Judged here, in the serialiser, exactly as every other payload value is: the client is handed
630+
`display` and `state` and never decides what a result means.
631+
"""
632+
return payload.envelope(
633+
int(time.time()),
634+
# `running=True` is a statement of fact rather than a probe: this document is built only
635+
# after an action has RUN in this process, so the engine's presence is not in question and
636+
# re-inspecting the deployment to say so would be one disk probe spent on a known answer.
637+
running=True,
638+
data={
639+
"step_key": str(getattr(result, "step_key", "")),
640+
"changed": payload.flag(
641+
bool(getattr(result, "changed", False)),
642+
on="done",
643+
off="already done — nothing to change",
644+
on_state=payload.GOOD,
645+
off_state=payload.NEUTRAL,
646+
),
647+
"message": payload.label(str(getattr(result, "message", ""))),
648+
},
649+
)
650+
651+
610652
def sortable_columns() -> Mapping[str, Sequence[str]]:
611653
"""The declared sort surface, for a test to read rather than restate."""
612654
return {path: route.sortable for path, route in API_ROUTES.items() if route.sortable}

‎keel/web/payload.py‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,16 @@ def _action_input_payload(field: Any) -> dict[str, Any]:
14791479
# a form that can be submitted safely and one that leaks its own contents into browser
14801480
# history. The client is told the answer rather than deriving it from the field's name.
14811481
"secret": flag(field.secret, on="never echoed back", off="shown as typed"),
1482+
# The SAME fact as `secret`, in the form the client needs to act on rather than to show.
1483+
#
1484+
# Two keys for one boolean looks like duplication and is the opposite: `secret` is a
1485+
# `Field`, and a `Field` is a thing the client DISPLAYS -- Rule 3 of the client's pins
1486+
# forbids `render.js` from reading `.value` at all, precisely so no view can start
1487+
# deciding what a payload means. Choosing between `type="password"` and `type="text"` is
1488+
# not a judgement about a value, it is a rendering instruction, and the server is the one
1489+
# that gives it. Without this the client would have had to read `secret.value`, which is
1490+
# the whole rule.
1491+
"kind": "secret" if field.secret else "text",
14821492
# A closed set of answers, rendered with NOTHING pre-selected: an action that could fill in
14831493
# a field the operator left blank is one that could record something they never supplied.
14841494
"choices": list(field.choices),
@@ -1526,22 +1536,42 @@ def setup_payload(
15261536
actions: Sequence[Any] = (),
15271537
not_automated: Mapping[str, str] | None = None,
15281538
job: Any = None,
1539+
csrf: str = "",
15291540
) -> dict[str, Any]:
15301541
"""`keel.commands.setup.inspect`'s `DeploymentState`, as JSON.
15311542
1532-
**No CSRF token, and that is the point rather than an omission.** `render_setup` takes one
1533-
because it emits `<form method=post>`; this issue ships reads only, and minting a live write
1534-
credential into a GET response would put it into every cached copy, every proxy log and every
1535-
paste of "here is what the API returned". The token is `csrf_token(session)` and it stays where
1536-
the write is.
1543+
**`csrf` is here since #540, and an earlier revision of this docstring argued it should not
1544+
be.** That argument is recorded rather than deleted, because reversing it was a decision:
1545+
1546+
"No CSRF token, and that is the point rather than an omission. `render_setup` takes one
1547+
because it emits `<form method=post>`; this issue ships reads only, and minting a live
1548+
write credential into a GET response would put it into every cached copy, every proxy log
1549+
and every paste of 'here is what the API returned'."
1550+
1551+
Two of those three concerns were already answered by the time the form was deleted: `/api/*`
1552+
is `Cache-Control: no-store` and #538's service worker refuses to cache it, so there is no
1553+
cached copy; and the server binds loopback, so there is no proxy. The third -- an operator
1554+
pasting API output into an issue -- is real, and it is what settles the question rather than
1555+
what blocks it: **this token is not a credential on its own.** It authorises nothing without
1556+
the session cookie, and anyone holding that cookie can read this endpoint and mint the same
1557+
token for themselves. A paste of it grants exactly what a paste of a random hex string grants.
1558+
1559+
What the alternative would have cost is the reason not to be clever here: delivering it in a
1560+
second, script-readable cookie would take a live token out of a body and put it into
1561+
`document.cookie`, which is a strictly worse place for it on an origin whose session cookie is
1562+
deliberately `HttpOnly`.
15371563
15381564
`actions` and `not_automated` are read from `keel.commands.setup`'s own closed registries and
15391565
copied, never filtered here: an action appears only where the registry carries one, which is
15401566
what stops "attest this asset" appearing because somebody edited a front-end.
1567+
15411568
"""
15421569
from keel.commands.setup import Stage
15431570

15441571
return {
1572+
# A bare string, not a `Field`: it is a credential the client SENDS, never a value it
1573+
# displays, and giving it a `display` would invite exactly that.
1574+
"csrf": str(csrf),
15451575
"root": str(state.root),
15461576
"config_path": str(state.config_path),
15471577
"db_path": str(state.db_path),

0 commit comments

Comments
 (0)