Skip to content

fix(server): add 30s connection timeout to prevent slow-client thread exhaustion - #198

Merged
nesquena-hermes merged 1 commit into
nesquena:masterfrom
iRonin:security/add-connection-timeout
Apr 10, 2026
Merged

fix(server): add 30s connection timeout to prevent slow-client thread exhaustion#198
nesquena-hermes merged 1 commit into
nesquena:masterfrom
iRonin:security/add-connection-timeout

Conversation

@iRonin

@iRonin iRonin commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a 30-second read timeout to the HTTP handler by setting timeout = 30 on the Handler class. Python BaseHTTPRequestHandler.setup() calls self.request.settimeout(self.timeout), which causes rfile.read() to raise socket.timeout after the configured duration on idle or slow connections.

Why

The ThreadingHTTPServer spawns one thread per connection. With no timeout:

  • A slow-client (Slowloris) attack can exhaust all threads
  • Hung connections from crashed clients hold threads open forever
  • No upper bound on thread count under sustained abuse

The Fix

One line in server.py:

class Handler(BaseHTTPRequestHandler):
    timeout = 30  # seconds

This is the standard Python stdlib approach — no new dependencies, no architectural change.

Fixes #194

…d exhaustion

Set Handler.timeout = 30. Python's BaseHTTPRequestHandler.setup()
calls self.request.settimeout(timeout), which raises socket.timeout
on idle or slow connections after the configured duration.

This defends against Slowloris-style attacks where a client holds
connections open indefinitely, exhausting threads in ThreadingHTTPServer.
Also recovers threads from crashed clients with hung TCP connections.

Addresses nesquena#194.
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Minimal and correct. Handler.timeout = 30 is exactly how the stdlib docs recommend setting a read timeout on BaseHTTPRequestHandlersetup() calls self.request.settimeout(self.timeout) if timeout is set.

What this protects against:

  • Slowloris-style slow-header attacks (connection held open, data trickled in)
  • Hung TCP connections from crashed clients that never send a FIN
  • Unbounded thread accumulation under ThreadingHTTPServer

30 seconds is a reasonable default for a local/LAN tool. It's long enough that no normal browser request will be affected, and short enough to reclaim threads from hung connections within half a minute.

One thing to note:
This timeout applies to the initial read phase (headers + body). It does not limit how long an SSE stream can stay open — that's governed by the streaming write loop, which is correct behavior. Long-running agent streams should not be cut off by this.

The single-file, single-line change is easy to audit. Ready for maintainer review.

@nesquena

nesquena commented Apr 9, 2026

Copy link
Copy Markdown
Owner

Full Review: PR #198 — 30s connection timeout

Thanks @iRonin! Minimal and correct.

Security Audit

Clean. One line added — timeout = 30 on the Handler class. This is the stdlib-documented approach; BaseHTTPRequestHandler.setup() calls self.request.settimeout(self.timeout).

Code Review

Protects against:

  • Slowloris-style slow-header attacks (connection held open indefinitely)
  • Hung connections from crashed clients holding threads open
  • Unbounded thread growth under sustained abuse

30 seconds is a reasonable balance — long enough for slow legitimate connections, short enough to reclaim resources from stalled ones. SSE streams are unaffected because they actively write data (the timeout is on idle read, not on the connection lifetime).

Test Results

506 passed, 0 failed, 41 skipped. No regressions.

Verdict

Approved. Ready to merge.

@nesquena-hermes
nesquena-hermes merged commit 04678b7 into nesquena:master Apr 10, 2026
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
…d exhaustion (nesquena#198)

Set Handler.timeout = 30. Python's BaseHTTPRequestHandler.setup()
calls self.request.settimeout(timeout), which raises socket.timeout
on idle or slow connections after the configured duration.

This defends against Slowloris-style attacks where a client holds
connections open indefinitely, exhausting threads in ThreadingHTTPServer.
Also recovers threads from crashed clients with hung TCP connections.

Addresses nesquena#194.
roadhero added a commit to fox-in-the-box-ai/hermes-webui that referenced this pull request May 17, 2026
…ore 6 upstream tests with FOX_OVERLAY skipif (#30)

Phase 7a (fork side) of v0.6.0 upstream-separation migration
(fox-in-the-box-ai/fox-in-the-box#155). Closes nesquena#197.

## What this removes

* `api/onboarding.py` (283 LOC) — Fox's wholesale-replaced 3-step
  setup wizard. Moves to fox_overlay/webui_modules/onboarding.py
  in the monorepo (P7b nesquena#198).
* `static/setup.html`, `static/setup.css`, `static/setup.js` —
  deferred from Phase 2 because they were coupled to
  api/onboarding.py's `REPO_ROOT / "static" / "setup.html"` path
  access. P7b moves them to the overlay's webui_static/.

## What this restores

6 upstream onboarding tests Fox previously deleted (the deletion
predates the v0.6.0 migration). All restored from merge-base 9e31a2a
with a module-level `pytestmark = pytest.mark.skipif(FOX_OVERLAY)`
decorator so they:

* Pass fork CI today (skip cleanly when FOX_OVERLAY=1, which is the
  fork test env post-overlay)
* Run normally against virgin upstream content (post-Phase-8 re-point)
  where Fox doesn't replace onboarding

Restored:
* `tests/test_issue1499_keyless_onboarding.py`
* `tests/test_issue1499_onboarding_probe.py`
* `tests/test_onboarding_existing_config.py`
* `tests/test_onboarding_mvp.py`
* `tests/test_onboarding_network.py`
* `tests/test_onboarding_static.py`

## What this does NOT change

`api/routes.py` keeps its 6 inline onboarding handler blocks
(lines 1641-1648 + 2310-2326) following the Option E pattern from
Phase 4. The dispatcher hook (Phase 4) pre-empts /setup and
/api/setup/* before those lazy imports trigger — so they become
dead code after P7b ships, but stay in source until a later cleanup
pass (Phase 8+).

## Critical preservation

Fox's `_write_env_key` (line 93 of deleted onboarding.py) is used
by overlay's `webui_modules/hostname.py` — re-exported in P7b's
overlay onboarding module + hostname.py import updated to point at
the new location. **P7b must ship simultaneously with this PR's
submodule bump** to avoid hostname module-load ImportError.

## Sequencing

This PR merges FIRST. Monorepo DRAFT (P7b) bumps submodule + ships
overlay onboarding + .fox-removals consumer wiring.

## Diff summary

```
 api/onboarding.py                            | 283 ----- (deleted)
 static/setup.css                             | (deleted)
 static/setup.html                            | (deleted)
 static/setup.js                              | (deleted)
 tests/test_issue1499_keyless_onboarding.py   | + (restored)
 tests/test_issue1499_onboarding_probe.py     | + (restored)
 tests/test_onboarding_existing_config.py     | + (restored)
 tests/test_onboarding_mvp.py                 | + (restored)
 tests/test_onboarding_network.py             | + (restored)
 tests/test_onboarding_static.py              | + (restored)
```
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…d exhaustion (nesquena#198)

Set Handler.timeout = 30. Python's BaseHTTPRequestHandler.setup()
calls self.request.settimeout(timeout), which raises socket.timeout
on idle or slow connections after the configured duration.

This defends against Slowloris-style attacks where a client holds
connections open indefinitely, exhausting threads in ThreadingHTTPServer.
Also recovers threads from crashed clients with hung TCP connections.

Addresses nesquena#194.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reliability: No connection timeout — vulnerable to slow-client thread exhaustion

3 participants