Skip to content

keel serve: socket timeout, GET drain, halt never creates a DB, halt outlives the remote lifetime; stale citations (#981 #982 #983) - #987

Merged
eaitbrahim merged 4 commits into
mainfrom
fix/981-982-serve-hygiene
Oct 13, 2026
Merged

eaitbrahim merged 4 commits into
mainfrom
fix/981-982-serve-hygiene

Conversation

@eaitbrahim

@eaitbrahim eaitbrahim commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Closes #981
Closes #982
Closes #983

#981 — keel serve hygiene

  • Socket timeout. REQUEST_TIMEOUT_SECONDS = 60 is set on the bound handler by build_server, so StreamRequestHandler.setup applies it to every accepted socket. A refused POST that declares Content-Length: 100 and sends nothing now gets its refusal and is closed; an idle kept-alive connection is dropped instead of parking a thread in readline.
  • The halt is not slowed. The timeout is per socket operation. The halt reads no body and answers before do_POST's drain, so only the drain that comes after the answer waits on a body that never arrives. A test pins this under a 0.5 s timeout.
  • GET bodies are drained. A KeelHandler.handle_one_request override drains a GET's or HEAD's body at the request boundary once the response is out, unless the connection is already closing. This is server: /api/gates/halt leaves an unread request body on a kept-alive connection #955's shape on the read path. do_GET is deliberately left byte-for-byte as on main, so CodeQL's existing self.path dataflow alerts are not re-attributed to this PR.
  • The timeout bounds silence, not holding. A client that trickles a byte inside every window is not cut off (slowloris). That is accepted: threads are unpooled, so an attacker can open more connections anyway.
  • The halt never creates a database. api.open_repo_to_write opens through the new db.connect_existing (mode=rw URI, otherwise the same as connect), which refuses a missing file. A halt that can't be recorded now returns a stated 500 ("The halt could not be recorded … Run keel kill") instead of a reset connection.

#982 — stale citations

engine._assemble_cts_context, backtest._closed_trade (x2) and simulate._slippage_for_products are corrected. A new test resolves every backticked engine. / backtest. / simulate. citation in keel/ against an allowlist of modules. A citation is excused only when that exact string is a constant in the cited module's source (its event kinds, engine.no_signal / engine.setup_detected). A guard test checks that each allowlisted stem names exactly one file in keel/.

#983 — HALT outlives the remote session lifetime (owner decision, option 2)

_post passes outlives_the_lifetime=self._is_a_tokened_halt(path) to _admitted. The flag is true only for /api/gates/halt carrying this session's halt token. The halt still needs the session cookie, and _gated_action still checks the token. If an expired session sends the halt without the token, it still gets "Session expired". Releases, setup writes and every read keep the lifetime. These docstrings are updated: security.REMOTE_SESSION_MAX_AGE_SECONDS, session_expired_at, _admitted, do_POST, _gated_action, gates.HALT, gates.run_gated_action.

The "Session expired" refusal now also says "To stop trading now, run keel kill at the terminal." The exemption serves a console still showing a Pulse painted before expiry, or any client that holds the token. The client half is filed as #985. The console's refusedView draws no HALT, so the next Pulse repaint after expiry still hides the button that the server would now accept.

Tests

TDD: every new pin was seen red for the reported reason (the timeout tests hung into the client's 5 s timeout, the GET tests got a 501, the halt reset the connection and created the file, the expired halt got a 403). Nine mutants were applied to saved copies, each confirmed to change the source, and all nine were killed.

🤖 Generated with Claude Code

eaitbrahim and others added 2 commits October 12, 2026 17:11
Correct engine._assemble_cts_context, backtest._closed_trade and
simulate._slippage_for_products, and resolve backticked citations of
those three modules against an allowlist, excusing only strings the
cited module emits verbatim (its event kinds).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…halt outlives the remote lifetime (#981, #983)

- REQUEST_TIMEOUT_SECONDS (60 s) on every accepted socket via build_server,
  so a refused POST that declares a body it never sends is answered and
  closed, and an idle kept-alive connection is dropped.
- do_GET drains a request body in a finally, as do_POST does (#955).
- gates._perform writes through db.connect_existing (mode=rw URI), which
  refuses a missing database instead of creating one; a halt that cannot
  be recorded is a stated 500.
- #983 (owner decision, option 2): a halt carrying the session cookie and
  the halt token is admitted past the remote session lifetime; releases,
  setup writes and reads keep it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@eaitbrahim eaitbrahim self-assigned this Oct 12, 2026

@eaitbrahim eaitbrahim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 1 review. I read the framing (_declared_body_length/_drain_request_body against the new GET drain and the timeout), the #983 exemption (/api/gates/halt only, halt token plus cookie, the same urlsplit path for the exemption and the routing, HEAD and GET unaffected), the halt's 500 path, connect_existing, and the new tests (175 pass locally). I found one prose inaccuracy, below.

Comment thread keel/web/server.py Resolved
…981)

do_GET is restored to its origin/main body so the request-target dataflow
is unchanged (CodeQL flagged the moved body). The drain moves to
handle_one_request. Also corrects the REQUEST_TIMEOUT_SECONDS comment: it
bounds silence, not how long a connection is held.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@eaitbrahim eaitbrahim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 2 review. The round-1 timeout comment is fixed accurately. The handle_one_request override held up against every edge case I probed on a live scratch server: pipelined GET+body+GET, chunked GET, HTTP/1.0 keep-alive, HTTP/0.9, 414, a bad version, the 303 exchange with a body, a refused GET with a body, and HEAD /api/events. One finding, below.

Comment thread keel/web/gates.py Resolved
…iry refusal

The halt token is served only by /api/pulse, which keeps the lifetime, so the
exemption serves a console still showing a pre-expiry Pulse or a client holding
the token; a reloaded console cannot halt until #985. The refusal now names keel kill.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread keel/web/server.py Resolved
@eaitbrahim
eaitbrahim merged commit c53ad0e into main Oct 13, 2026
4 checks passed
@eaitbrahim
eaitbrahim deleted the fix/981-982-serve-hygiene branch October 13, 2026 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment