feat(llm-client): harden HTTP provider transport

[bbednarski9]

What

Hardens the reusable switchyard-llm-client HTTP transport used for provider
dispatch. The PR establishes five generic safety guarantees:

• configured provider requests do not follow redirects;
• connections and inactive response reads have safe, configurable timeouts;
• buffered success and retained error bodies have explicit memory bounds;
• credentials and provider error bodies are redacted from ordinary diagnostics; and
• individual SSE frames are bounded before line-oriented decoding can allocate
  without limit.

This is intentionally independent of the NeMo Relay dynamic-plugin implementation.
It changes the shared client and translation behavior for every caller rather than
placing Relay-specific protections in #270.

Review guide

1. Redirect rejection at the provider authority boundary

Motivation: A configured provider request can contain credentials and a full
prompt. Following an upstream redirect would allow a provider—or a compromised
endpoint—to move that request to a different authority. Redirects are therefore
returned as ordinary non-success HTTP responses instead of being followed.

• Implementation: TranslatingLlmClient constructs a no-redirect shared client.
• Contract test: configured_provider_requests_do_not_follow_redirects verifies the redirect target receives no request.

2. Configurable connection and idle-read timeouts

Motivation: Without transport timeouts, an unreachable or silent provider can
retain request resources indefinitely. The defaults are a 10-second connection
timeout and a 120-second idle-read timeout. The read timeout resets after every
successful read, so an active long-running stream is not limited to 120 seconds.
Callers serving providers that may legitimately remain silent longer can override
the values or disable the idle-read timeout.

• Public configuration: HttpTransportConfig and new_with_transport_config.
• Error mapping: reqwest timeouts remain the typed LlmClientError::Timeout variant.
• Contract test: routed_llm_client_exposes_timeout_variant exercises a short timeout through the public configuration path.

3. Bounded buffered and error response bodies

Motivation: A provider controls both its response body and advertised
Content-Length. Relying on that metadata can reserve large amounts of memory
before any bytes arrive, while collecting an unbounded body allows concurrent
responses to exhaust the process. The client now grows buffers only from received
chunks, rejects buffered success bodies above 64 MiB, and retains at most 64 KiB
of an HTTP error body with an explicit truncation marker.

• Enforcement: buffered success and error handling and the shared collect_response_body collector.
• Error-body test: upstream_error_body_is_bounded_and_redacted_from_display.
• Allocation regression: response_body_does_not_preallocate_advertised_content_length advertises 64 MiB while yielding only two bytes.

4. Diagnostic redaction without weakening typed errors

Motivation: Provider credentials can be stored in static target headers, and
provider error bodies can echo prompts or secrets. Redacting only Display is
insufficient because structured tracing commonly records errors with ?error,
which uses Debug. Header values and sensitive provider fields are now redacted
from both ordinary formatting paths. The bounded body/message fields remain
available to code that explicitly matches the typed error.

• Static headers: HttpBackendConfig::Debug emits names but not values, covered by debug_redacts_static_header_values.
• Provider bodies: LlmClientError display and debug formatting, covered by the protocol redaction tests.

5. Bounded SSE frames before line buffering

Motivation: AsyncBufReadExt::lines() must accumulate a complete line before
returning it. A provider that never emits an SSE frame boundary could otherwise
grow that allocation without limit. The translation layer now checks raw transport
chunks before the line reader and rejects a frame above 8 MiB. The byte limiter and
parser share the same ASCII-whitespace boundary semantics so valid small frames do
not accumulate into a false oversized-frame failure.

• Enforcement: decode_stream_with_limit and SseFrameSizeTracker.
• Tests: chunk-spanning limit, oversized-frame, and whitespace-boundary coverage.

6. Public documentation

• switchyard-llm-client README documents credentials, retries, timeout overrides, body limits, redaction, and the SSE limit.
• The Unreleased changelog entry records the behavior change for all client consumers.

Why

switchyard-llm-client is the reusable provider-dispatch boundary for the
Switchyard server, libsy examples, and the external Relay plugin. It handles
credentials and untrusted network responses, so its defaults must protect:

• authority integrity: credentials and request bodies stay with the configured provider;
• availability: stalled connections and reads cannot retain resources indefinitely;
• memory safety: provider metadata, buffered responses, and SSE frames cannot cause unbounded allocation; and
• credential/content privacy: routine logs and spans do not copy target headers or provider-returned content.

These are generic client guarantees, not routing-algorithm behavior. PR #270 can
build and function independently, while consuming these safer defaults once this
PR lands.

Relates to: #270

How tested

• uv run ruff check . clean (N/A: Rust-only change)
• uv run mypy switchyard clean (N/A: Rust-only change)
• uv run pytest tests/ green (N/A: Rust-only change)
• cargo test -p switchyard-llm-client
• cargo test -p switchyard-translation
• cargo test -p switchyard-protocol -p switchyard-libsy
• cargo clippy --workspace --all-targets -- -D warnings
• cargo fmt --all -- --check
• Manual smoke (not needed: behavior is covered with local HTTP fixtures)

Checklist

• One class per file; filename = snake_case of the primary class. (N/A: Rust-only change.)
• New public symbols exported from switchyard/__init__.py.__all__ if intended for downstream use. (N/A: no Python symbols.)
• Unit tests added for new components / bug fixes.
• README / --help updated if customer-facing surface changed.
• Commits signed off (Signed-off-by: Your Name <email>) per the DCO.

Notes for reviewers

• TranslatingLlmClient::new remains source-compatible and applies the safe defaults. The only new public surface is HttpTransportConfig plus new_with_transport_config for callers that need different timeout behavior.
• The read timeout is an idle timeout, not a total request deadline; every successful read resets it.
• Redirects are not followed. A provider 3xx is returned through the existing typed HTTP-error path.
• Oversized successful buffered responses fail. Oversized HTTP error bodies are truncated because callers still need bounded provider context for typed classification and diagnostics.
• Raw provider bodies remain available through explicit typed-error field access. Only implicit Display and Debug rendering is redacted.
• The 8 MiB SSE limit is enforced in switchyard-translation, before its line reader. This is the allocation boundary and protects every caller of the shared decoder.
• This PR adds no Relay plugin, manifest, Relay SDK dependency, routing behavior, or Relay-specific configuration.
• This PR is non-blocking for the Relay plugin integration. It captures hardening that could be made before production, but it should not prevent the integration from working.

Summary by CodeRabbit

• Security
  • Sensitive header values are no longer exposed in diagnostic output.
  • HTTP error messages are sanitized to avoid displaying upstream response contents.
• Reliability
  • Added connection and read timeouts, redirect blocking, and response-size limits.
  • Oversized responses and streaming frames now fail safely with clear errors.
  • Large error responses are capped and marked as truncated.
• Documentation
  • Documented HTTP safety, timeout, redaction, and streaming limits.

[coderabbitai]

Walkthrough

The HTTP client now enforces redirects, timeouts, response limits, and redaction. Typed HTTP error displays omit body content. SSE decoding rejects frames larger than 8 MiB.

Changes

Response safety controls

Layer / File(s)	Summary
HTTP transport and body limits crates/libsy-llm-client/src/backend.rs, crates/libsy-llm-client/src/client.rs, crates/libsy-llm-client/README.md	The client rejects redirects, applies connect and read timeouts, bounds response bodies, truncates oversized error bodies, and redacts header values in debug output.
Sanitized upstream error display crates/protocol/src/client.rs, crates/libsy/src/core/algorithm.rs	HTTP error displays now show status only. Tests validate the structured status, body, and formatted error.
SSE frame-size enforcement crates/switchyard-translation/src/helpers.rs	The decoder tracks frame size across chunks, resets at frame boundaries, and rejects frames over 8 MiB.

Estimated code review effort: 4 (Complex) | ~45 minutes

Poem

I’m a rabbit guarding each byte,
Redirects stop before the night.
Headers hide their secret hue,
Big frames bounce from the queue.
Safe errors hop back bright and clear.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: hardening HTTP transport behavior in the reusable LLM client.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/libsy-llm-client/src/client.rs`:
- Around line 526-554: Update collect_response_body to avoid preallocating based
on response.content_length(); initialize bytes with an empty buffer or small
fixed capacity while preserving the existing limit and truncation behavior. Add
a regression test covering an inflated Content-Length with minimal body data,
verifying collection does not reserve the advertised size.

In `@crates/switchyard-translation/src/helpers.rs`:
- Around line 267-271: Align SseFrameSizeTracker’s line-boundary detection with
decode_stream’s line.trim_end().is_empty() behavior so whitespace-only lines
reset frame_bytes and do not mark current_line_has_content. Add a regression
test covering two frames separated by " \n", verifying the second frame starts
with a reset byte count.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d43299c3-4b1c-4fa3-a3a9-666fbac50409

📥 Commits

Reviewing files that changed from the base of the PR and between a9c04b3 and 41d03a9.

📒 Files selected for processing (6)

• crates/libsy-llm-client/README.md
• crates/libsy-llm-client/src/backend.rs
• crates/libsy-llm-client/src/client.rs
• crates/libsy/src/core/algorithm.rs
• crates/protocol/src/client.rs
• crates/switchyard-translation/src/helpers.rs

[bbednarski9]

switchyard-rust-review evidence

I reviewed PR #271 at 2ef51d16 against main at a9c04b31, following the repository's switchyard-rust-review skill. I completed separate passes for correctness, async/streaming behavior, memory allocation, HTTP and credential boundaries, public error contracts, comments/naming, and tests, followed by the required second pass over every changed hunk.

Verdict: changes requested

High

1. An untrusted Content-Length can reserve 64 MiB before any response bytes arrive. collect_response_body passes the provider-controlled content_length() directly to Vec::with_capacity, capped only by the 64 MiB body limit. A slow or malicious provider can therefore make every concurrent buffered call reserve 64 MiB while sending a tiny body; 256 calls can request 16 GiB of capacity before the limit protects anything. Start with an empty or small bounded capacity and grow only as bytes are received. Add a regression test with an inflated Content-Length and a minimal body. This independently confirms the existing CodeRabbit finding.

2. Provider bodies are still exposed through Debug. LlmClientError continues to derive Debug at protocol/src/client.rs:23-30, so both retained fields at protocol/src/client.rs:73-89 appear verbatim in format!("{error:?}") and structured tracing fields written with ?error. Redacting only Display does not support the README claim that provider content is not copied into ordinary logs or spans. Implement a redacted Debug representation for these sensitive fields and test both Display and Debug for ContextWindowExceeded and UpstreamHttp.

Medium

3. The new 120-second read timeout is a mandatory global behavior change. The timeout constants at client.rs:54-58 are installed by the only public constructor at client.rs:98-113. Existing callers with a provider that legitimately emits no bytes for more than two minutes—particularly long reasoning requests—will now fail with no override. Keep a secure default, but expose a bounded client/transport configuration path and add a short-duration timeout test rather than baking one operational policy into every library consumer.

4. The SSE limiter and parser disagree about whitespace-only frame boundaries. decode_stream treats line.trim_end().is_empty() as a boundary at helpers.rs:214-238, but SseFrameSizeTracker marks spaces/tabs as content and does not reset on " \n". A sequence of individually valid small frames separated by whitespace-only lines is therefore counted cumulatively and can eventually be rejected as an oversized frame. Use exactly the parser's boundary semantics. The test at helpers.rs:661-670 should exercise decode_stream, including a whitespace boundary, rather than only the private tracker. This independently confirms the existing CodeRabbit finding.

Low

5. The stream limiter adds an avoidable trait-object allocation. helpers.rs:194-206 boxes bounded_bytes, then immediately maps and boxes it again as io_bytes. Keep the try_stream! value concrete and erase the type only once at the io_bytes boundary.

Positive checks

• The PR is now focused on the reusable HTTP client and contains no Relay/plugin coupling.
• Production changes add no unwrap/expect, blocking executor work, unbounded channels, or new dependencies.
• Redirect rejection, bounded body reads, source-error preservation, and static-header-value redaction are placed in the correct client/translation boundaries.
• git diff --check and the required second-pass hunk review were clean apart from the findings above.

Validation performed at this head

• cargo fmt --all -- --check — passed.
• cargo clippy --workspace --all-targets -- -D warnings — passed.
• cargo test -p switchyard-protocol -p switchyard-translation -p switchyard-libsy -p switchyard-llm-client — passed (30 protocol, 25 translation unit plus integration suites, 222 libsy plus observability, and 48 LLM-client tests).
• cargo test --workspace — attempted; Rust compilation succeeded through the affected crates, but the local macOS link of switchyard-py failed because Python symbols were unavailable in this shell. GitHub's Rust and aggregate CI Success checks are green at 2ef51d16, including the Ubuntu workspace validation.

[bbednarski9]

Comment#1 above addressed in e8081d1

[bbednarski9]

Comment#2 addressed in a61a981

[bbednarski9]

comment#3 addressed in b63db0f
comment#4 addressed in ca0c1aa

[bbednarski9]

comment#5 addressed in 4fb0a2a

[bbednarski9]

Some small (potential) regressions to follow-up on:
SSE bare-CR line endings fail. The standard permits CRLF, LF, or CR line endings. sse_regressions.rs (line 67) reproduces this. WHATWG SSE rules.

Valid data:{...} fields without the optional space are silently ignored. sse_regressions.rs (line 75) reproduces this.

Backend debug output leaks credentials embedded in URL userinfo. redaction.rs (line 31) reproduces this. Static header and typed provider-error redaction otherwise passed.

[mnajafian-nv]

Confirmed on head 889ce95 with a \r\r-delimited SSE stream. This still fails with ResponseTranslation(...) instead of decoding the frame, so bare-CR line endings are not handled on this path yet.