Halt and keel serve hardening: drain every POST, HALT on a failed Pulse, serve keeps --external-host (#955, #958, #971)

[eaitbrahim]

Closes #955
Closes #958
Closes #971
Closes #976

Hardens the halt and keel serve. There are three server-side fixes, plus a client view for a Pulse report that fails.

#955: every POST ends at a request boundary

• do_POST now runs its routing in _post() and calls _drain_request_body() in a finally. The halt answers without reading a body, so before this a body sent anyway stayed on a kept-alive socket and the next request was parsed as {}GET … (501).
• Ruling: drain, not close. The drain is bounded the same way as before: an oversized or unreadable Content-Length closes the connection instead of reading. Why drain: the console's next read rides the same connection, and the test the issue asks for (a GET on the same socket answers 200) requires it. Cost if wrong: a client that declares a body and never sends it stalls only its own connection.
• Ruling: the drain runs AFTER the answer. The halt has already engaged and been reported by then, so a slow body never delays the stop (gates.py: "stopping never slower than starting"). Cost if wrong: none for well-formed clients.
• _drain_request_body now sets body_consumed, so the closing drain never reads twice after _refuse already drained.
• Tests: one keep-alive socket. A halt POST with {}, a 2-byte body, non-JSON, or nested JSON each answers 200 with the switch engaged, and then a GET on the same socket answers 200. An oversized body still halts. Together these pin server-side that the halt reads and validates no body: the console: HALT gets its own token; the release token is served nowhere (#968, #969) #970 review's mutant (halt calls _read_json_object and answers 400) is now killed.

#958: HALT survives a Pulse that cannot be built

• When the read fails, /api/pulse's error envelope carries halt: {halt_token, resume_command}. The token is security.halt_token, never the gates token. The kill switch is read on its own path (api.pulse_halt → pulse.halt_resume_command), not through gather_pulse. Every other failure, and every pre-admission refusal, carries halt: null, so the key set stays constant.
• Ruling: a kill switch that cannot be read shows the button, not the HALTED line. Halting a halted engine is idempotent and harmless, but a HALTED line over a running engine would hide the only stop. Cost if wrong: an operator may press HALT on an engine that is already halted.
• Client: readingFrom forwards halt off a failure document. In paint, a Pulse failure that carries halt goes to rememberHaltToken (the one holder) and then to the new pulseFailedView, which is the same haltControl followed by the error. There is no extra confirm and no phrase.
• Tests: with gather_pulse forced to raise, the response carries the halt token, the resume command follows the switch, and the served token halts (200). An unreadable switch shows the button. With every route's read made to raise, only Pulse carries halt. A refused Pulse carries halt: null. Structural client pins cover: Reading literals pair halt with attestations, only the failure arm reads document_.halt, the order of the paint branches, and that pulseFailedView is haltControl({ halt: reading.halt }) plus the error, with no report section.

#971: serve() keeps every config field

• running = replace(cfg, port=int(bound_port)) replaces the hand-listed constructor, which dropped external_hosts and started_at.
• Tests go THROUGH the real serve(), run in a thread with build_server wrapped only to capture the server:
  • the installed config equals the given one apart from the port, compared as whole dataclasses with every optional field set off its default;
  • a declared external Host is admitted;
  • the remote session lifetime is enforced;
  • a release on a declared-remote deployment is refused by the locality check. Before the fix this released the switch: 200.

Review hardening (rounds 1-5)

• Framing that cannot be sized closes the connection. One helper, _declared_body_length, serves the drain, _read_json_object and _refuse. It treats the following as untrustworthy: any Transfer-Encoding, more than one Content-Length, a value that is not plain digits (-2, +2, 1_0), more than 18 digits, and a header block the stdlib could not parse (for example Content-Length : 2). In each case it closes the connection instead of draining. The halt still engages and answers 200 first. Other POSTs get the existing 400 refusal. Honest shapes stay on keep-alive: no length, 0, lowercase names, leading zeros, and the full header sets a browser sends with the halt POST and a JSON setup POST.
• Ruling: close, not guess. A body whose length can't be trusted can't be drained to a request boundary, and guessing is the precondition for request smuggling behind a proxy. Cost if wrong: a client sending chunked POSTs loses its keep-alive (no browser does this).
• The console reader never checkpoints. connect_read_only sets SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE, so a console read that closes last leaves the trading database file byte-identical. The WAL then persists until the agent's own next write checkpoints it. keel update's backup API reads the WAL, so backups are unaffected.
• Read-only, stated honestly. query_only is per-connection and reversible, and it does not block ATTACH or checkpoint pragmas. A structural (AST) pin, _forbidden_sql, therefore scans every non-docstring string constant, f-string parts included, in every console reader module plus keel/data/repository.py, audit.py and db.py. It bans query_only, wal_checkpoint, vacuum and a whole-word attach. Its floors are at least 50 SQL calls and 1500 strings, so it can't pass vacuously. Its only allow-listed statement is connect_read_only's own PRAGMA query_only = ON. The pin's self-test runs that same function on planted sources. A multipart/* or message/* POST also closes after it is answered, by design, because the console sends only JSON or no body.
• The read-only URI is built by a pure helper, _read_only_uri. It handles POSIX paths, Windows drive letters and UNC paths, and percent-encodes #, ?, % and spaces.
• Filed, not fixed:
  • keel serve: request handler has no socket timeout; a refused POST that declares a body it never sends holds a thread forever #981: the handler has no socket timeout, and the halt's writable open creates a missing database.
  • Remote session lifetime (#648) refuses HALT too: a keel serve --external-host console can't halt after 12h until restarted #983, for the owner: now that keel serve drops --external-host after binding: remote Host refused, session lifetime and release locality check hollow #971 enforces the keel link prerequisite: the remote-exposure security pass — bind opt-in, tunnel Host validation, token issuance off loopback #648 remote session lifetime, an --external-host console refuses everything after 12 h, HALT included, until it is restarted.

Verification (final head)

• ruff check keel tests packages and ruff format --check . are clean. Bare mypy is clean (509 files). node --check passes on every static/js/*.js and on sw.js.

• pytest: 8369 passed, 3 skipped, run with logs/keel.log removed first.

• Mutation:

  • the original build killed 13 of 13;
  • Console reads 'keel isn't running' after another process closes the WAL database last (setup.inspect mode=ro) #976 killed 8 of 8;
  • review rounds 1 to 5 killed 13 of 13. Round 5 added a pin that the halt answers before it drains a body that never arrives; it kills a drain-first mutant and a read-first mutant.

  Each mutant was applied with its sha256 checked, then restored from a saved copy.

• Browser check: Playwright against my own keel serve on :8911, with a scratch database and the service worker unregistered first.

  • Normal Pulse: HALT asks for one confirm, the POST to /api/gates/halt answers 200, and the HALTED line appears with no button.
  • Pulse forced to fail (a garbage open-position row): /api/pulse answers 500 with halt: {halt_token, resume_command: ""}. The page shows the "Pulse" heading, the HALT button and the error. HALT asks for one confirm, the switch engages, and the repaint shows the HALTED line.
  • Console reads 'keel isn't running' after another process closes the WAL database last (setup.inspect mode=ro) #976: the scratch WAL was checkpointed and -wal/-shm removed, so a mode=ro open fails ("unable to open database file"). The full Pulse with its HALT button still shows, not the stopped view.
  • Console errors: only the deliberate 500s on /api/pulse.

• CI is green on the final head.

#976: console reads open a sidecar-less WAL database, and still cannot write

• A WAL database whose -wal/-shm are gone (a checkpointing last close, any copied backup) cannot be opened mode=ro. setup.inspect opened it that way, so a running deployment read as "keel isn't running here", and the stopped view hides HALT.
• New keel.data.db.connect_read_only: mode=rw (it never creates a database file) plus PRAGMA query_only = ON set before any read, so SQLite refuses every write on the connection. Every console read goes through it: api.open_repo (which was a read-WRITE connect until now), setup.inspect, setup._sleeve_sell_kind and venue_readiness._read_only_trade_scope. The halt and the gated releases use their own writable opener, api.open_repo_to_write.
• Ruling: use query_only, not a mode=ro retry fallback. A fallback would need a read-write open anyway, and query_only puts the read-only guarantee in SQLite on every read rather than only on the retry. Cost if wrong: a console read may re-create the -wal/-shm sidecars, which hold no data.
• Tests use a scratch WAL copy with its sidecars removed (the fixture proves that mode=ro fails there):
  • inspect, Pulse, the promote lookup and the venue scope all read it, and Pulse's HALT answers 200.
  • A row write, a schema write and a header write through open_repo are each refused, and the switch is unchanged.
  • Every connection opened by a sweep of all JSON read routes is query_only. The halt's own connection is not.
  • Mutation: 8 of 8 killed.

🤖 Generated with Claude Code

[eaitbrahim]

Gatekeeper: READY at 64c3a68. The owner merges.

This PR now also fixes #976. Console reads open the database with mode=rw + PRAGMA query_only = ON + SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE. A write through any console read connection is refused by SQLite, and the halt is the only writer.

Review: 5 rounds, each by a fresh reviewer. Findings per round: 4, 4, 3, 2, 1. All are fixed and test-first, with each mutant applied and then restored from a saved copy.

• The round-5 finding (nothing pinned that the halt answers before draining its body) is fixed in 64c3a68.
• That fix was verified by mutation (2 of 2 killed) rather than by a sixth review round.

Checks on 64c3a68:

• CI: 4 of 4 green.
• pytest: 8369 passed, 3 skipped.
• ruff, format and bare mypy: clean.
• node --check: clean on all static JS and on sw.js.

Browser check (own keel serve, scratch DB, service worker cleared):

• HALT takes one confirm and answers 200, and the HALTED line appears.
• With Pulse forced to a 500, the page still shows the HALT button, and it halts.
• With a sidecar-less WAL database that mode=ro cannot open, the page shows the full Pulse with HALT, not "keel isn't running".

Filed:

• keel serve: request handler has no socket timeout; a refused POST that declares a body it never sends holds a thread forever #981: no socket timeout on the handler; the halt's open creates a missing database; GET bodies are not drained.
• Remote session lifetime (#648) refuses HALT too: a keel serve --external-host console can't halt after 12h until restarted #983, owner decision: the now-enforced remote session lifetime also refuses HALT after 12 h.