stacklok / toolhive / 29424050116

68%

Build Type

push

github

Committed by web-flow

Commit Message

Fix tool-filter terminal drain dropping error bodies (#5816)

* Stop tool-filter terminal drain from dropping error bodies

The unconditional Flush() added in #5809 to drain the tool-filter
buffer after ServeHTTP returns shares its buffer-processing logic
with the streaming Flush() path. For any body whose Content-Type
isn't application/json or text/event-stream (e.g. a plain-text
error from http.Error), processBuffer() rejects it as unsupported
and the shared logic discarded the original bytes, sending an
empty body to the client despite the correct status code.

Split the terminal drain into its own method that passes an
unprocessable body through unchanged instead of discarding it,
since there's no further chance to flush it once ServeHTTP has
returned. Flush() keeps its existing streaming semantics.

* Fail closed on tools-list filtering failures, not raw pass-through

Review feedback on the terminal-drain fix (PR #5816) found three gaps:

- drainBuffer() passed the raw body through on ANY processing error,
  including a malformed tools/list (e.g. a tool missing its required
  name) that failed filtering -- leaking the unfiltered list instead
  of failing closed.
- A missing/mislabeled Content-Type on an otherwise-valid, successful
  tools/list response bypassed filtering entirely, since the "no
  content type" path wrote the buffer through unconditionally.
- Flush() unconditionally forwarded the transport-level flush after
  drainBuffer(false), even when the buffer was incomplete and nothing
  new had been written, whereas the original implementation withheld
  it in that case.

Distinguish "unrecognized/error body, safe to pass through" from "a
tools list we failed to filter, must not leak" via a sentinel error.
For a 2xx response with an unsupported/missing Content-Type, sniff the
raw body the same way a correctly-labeled response would be processed
(mirrors the disguised-result handling in pkg/authz for #5257) so a
backend can't smuggle an unf... (continued)

Coverage Stats

90 of 95 new or added lines in 1 file covered. (94.74%)

13 existing lines in 6 files now uncovered.

73664 of 108178 relevant lines covered (68.1%)

63.95 hits per line