Skip to content

serve: reverse-proxy Host allowlist + quiet prefill keepalive (#597) - #606

Merged
JustVugg merged 1 commit into
devfrom
fix/597-serve-allowlist-keepalive
Jul 25, 2026
Merged

serve: reverse-proxy Host allowlist + quiet prefill keepalive (#597)#606
JustVugg merged 1 commit into
devfrom
fix/597-serve-allowlist-keepalive

Conversation

@JustVugg

Copy link
Copy Markdown
Owner

Addresses two self-contained items from @mkelcb's serving report (#597), both in the Python gateway, both explicit opt-ins so default behaviour is unchanged.

#597 item 2 — reverse-proxy Host allowlist

The DNS-rebinding guard (_check_host) accepted only loopback names + the bind address, so a legitimate reverse proxy / Tailscale MagicDNS hostname in front of the loopback bind was rejected with 403 Host header not allowed and there was no supported way to trust it (the reporter had to hard-code it into LOOPBACK_HOSTS).

  • New --allowed-host (repeatable) and COLI_ALLOWED_HOSTS (comma-separated) extend the allowlist.
  • Default unchanged: loopback + bind address only. No wildcard, each trusted Host is explicit, orthogonal to CORS and API-key auth.
./coli serve ... --allowed-host my-host.example.ts.net
# or
COLI_ALLOWED_HOSTS=my-host.example.ts.net,internal.example ./coli serve ...

#597 item 5 — quiet prefill keepalive

The cold-prefill keepalive streamed a visible reasoning_content: "." every 10s, painting hundreds of dots in the client's reasoning panel over a minutes-long prefill. Now defaults to an empty delta (still a valid SSE data event, still resets the client idle timer). COLI_VISIBLE_KEEPALIVE=1 restores the old visible dot for diagnosing keepalive delivery.

Scope / relationship to the rest of #597

  • item 1 (GLM <|user|> not terminating) — already fixed on dev by feat: support OpenAI stop sequences #535 (DEFAULT_CHAT_STOP_SEQUENCES).
  • item 4 non-streaming (reasoning_content) — already handled (split_thinking_reply); Inkling streaming by serve: split Anthropic thinking response blocks #568.
  • item 3 (HTTP/1.1 desync) — read_json consumes exactly Content-Length and the streaming path already sets close_connection; likely already mitigated on dev — asked reporter to retest.
  • item 4 streaming (GLM) and item 6 (ACCEPT-frame context ordering, touches the native C protocol) — tracked as separate follow-ups.

Tests

4 new AllowedHostsTest cases (allowlist normalisation/filtering; trusted host accepted with port + case-insensitively; loopback still works; untrusted rejected by default and with an allowlist). Full server suite 68/68.

Two self-contained gaps from mkelcb's serving report (#597), both in the
Python gateway, both behind explicit opt-ins:

- #597 item 2 — the DNS-rebinding Host guard accepted only loopback names and
  the bind address, so a legitimate reverse proxy / Tailscale MagicDNS host in
  front of the loopback bind was rejected with 403 and had no supported way to
  be trusted. Add `--allowed-host` (repeatable) and `COLI_ALLOWED_HOSTS`
  (comma-separated) that extend the allowlist. Default is unchanged
  (loopback + bind address only); no wildcard, each trusted Host is explicit,
  and this is orthogonal to CORS and API-key auth.

- #597 item 5 — the cold-prefill keepalive streamed a visible `reasoning_content: "."`
  every 10s, painting hundreds of dots in the client's reasoning panel over a
  minutes-long prefill. Default to an empty delta (still a valid SSE data event,
  still resets the client idle timer); COLI_VISIBLE_KEEPALIVE=1 restores the old
  visible dot for diagnosing keepalive delivery.

Items 1 and 4 (non-streaming) of the same report are already fixed on dev by
#535 and #568; items 3, 4-streaming and 6 are tracked separately.

Tests: 4 new AllowedHostsTest cases (normalisation, trusted host accepted with
port/case, untrusted rejected by default and with an allowlist). Full server
suite 68/68.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JustVugg
JustVugg merged commit cc0fd12 into dev Jul 25, 2026
10 checks passed
@JustVugg
JustVugg deleted the fix/597-serve-allowlist-keepalive branch July 25, 2026 11:38
NeuralNotwerk pushed a commit to NeuralNotwerk/colibri that referenced this pull request Jul 25, 2026
…en 200 stream (JustVugg#597 item 6)

A streaming chat request committed its HTTP 200 (SSE headers) before the engine
tokenised the prompt and checked it against CTX. An over-long prompt was only then
found -- ERROR CONTEXT_EXCEEDED -- far too late to send the 400 the client needs, so
the client saw a broken/empty stream instead of a clean context_length_exceeded.
(Non-streaming was already correct: it sends its 200 only after generation.)

Add an acceptance frame to the engine<->gateway protocol (mkelcb's design in JustVugg#597):

- c/colibri.c mux_submit(): after all cheap validation (encode, non-empty, fits CTX)
  and BEFORE prefill, emit `ACCEPT <id> <ntok>`. Every early failure already returns
  first with an ERROR, so a request yields exactly one of ACCEPT or an early ERROR.
- Engine._dispatch_stdout(): parse ACCEPT -> ("accept", {...}) without popping pending.
- Engine.generate(): new on_accept callback; commit exactly once on the first of
  ACCEPT / DATA / DONE. DATA/DONE before an ACCEPT still commit (an older engine with
  no ACCEPT frame keeps working); a second ACCEPT is a protocol error.
- APIHandler.generation() streaming: defer the SSE 200 + keepalive into start_stream(),
  fired via on_accept. An ERROR that arrives before ACCEPT now propagates as an APIError
  with nothing committed, so do_POST returns a normal JSON 400.

Tests: 4 Engine-level cases over a fake engine process (ACCEPT-before-DATA fires
on_accept first; ERROR-before-ACCEPT raises 400 and never commits; DATA-before-ACCEPT
still commits for an old engine; duplicate ACCEPT is a protocol error) + 1 end-to-end
HTTP case (streaming context-exceeded returns a clean 400 with context_length_exceeded,
not a 200 stream). Full server suite 83/83; the C engine builds clean.

Completes the JustVugg#597 series (items 1/2/4/5 already on dev via JustVugg#535/JustVugg#606/JustVugg#607).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant