fix(security): outbound HTTP timeouts, streamed body cap, UTF-8-safe truncation (M1/M2/M3) - #16
Conversation
…truncation (M1/M2/M3) PR-E of the security-hardening plan. - M2: new core::http::client(Profile) builds a reqwest client with connect + total timeouts (Default 5s/30s; Llm 10s/180s). Replace every ad-hoc Client::new() in adapters, LLM providers, token refresh, and CLI OAuth so a stalled upstream can't pin a task. Build failure (TLS init) fails closed, not a silent no-timeout client. - M3: the LLM fallback fetch rejects an oversized Content-Length and otherwise streams the body with an 8 MiB running cap (decode lossily), so a huge/chunked response can't exhaust memory. Adds reqwest `stream` + futures-util. - M1: the extractor truncates HTML on a UTF-8 char boundary (floor_char_boundary) instead of a raw byte index, fixing a panic on multibyte content. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b14d0d717b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| buf.extend_from_slice(&chunk); | ||
| } | ||
| Ok(String::from_utf8_lossy(&buf).into_owned()) |
There was a problem hiding this comment.
When an allowed page serves non-UTF-8 HTML (for example Content-Type: text/html; charset=windows-1252), this now decodes the capped bytes as UTF-8 regardless of the response charset. The previous resp.text().await? path honored the declared charset, so pages with high-bit characters now reach the extractor/LLM as replacement characters or mojibake, which can corrupt extracted user-visible data. Consider retaining the response charset before streaming and decoding the capped buffer with it.
Useful? React with 👍 / 👎.
1 participant
PR-E of the security-hardening plan (robustness findings M1/M2/M3).
M2 — outbound timeouts
New
core::http::client(Profile)builds areqwest::Clientwith connect + total timeouts (Default5s/30s;Llm10s/180s for slow provider completions). Every ad-hocClient::new()is replaced — adapters (github/hackernews/scs/scs-legacy), LLM providers (openai/anthropic/ollama), token refresh, and CLI OAuth — so a stalled upstream fails by timeout instead of pinning a task forever. Build failure (TLS init) fails closed (no silent no-timeout client). The SSRF-guardedbrowser.rskeeps its own pinned/no-proxy/no-redirect client.M3 — bounded response body
The LLM fallback fetch rejects an oversized
Content-Length, then streamsbytes_stream()with an 8 MiB running cap (decode lossily), so a huge/chunked body can't exhaust memory. Adds reqweststream+futures-util.M1 — UTF-8-safe truncation
The extractor truncates HTML on a char boundary (
floor_char_boundary) instead of a raw byte index, fixing a panic on multibyte content crossing the 50 KB cap.Tests
floor_char_boundarymultibyte test;cargo test --workspacegreen;cargo clippy -p relais-core --all-targets -D warningsclean.Codex-reviewed; the two flagged items (untracked
http.rs, silent no-timeout fallback) are fixed.🤖 Generated with Claude Code