docs(roadmap): add stream text-tool ordering gap

This commit is contained in:
Yeachan-Heo
2026-05-22 12:00:54 +00:00
parent d8864ff151
commit 9f762b26fa

View File

@@ -6709,3 +6709,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
577. **`WebFetch` upgrades only the initial HTTP URL, but follows redirects without revalidating the final target, so HTTPS pages can redirect the tool into localhost/private HTTP endpoints** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 11:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@571b4c2`. Active tmux sessions at probe time: none. Code inspection: `rust/crates/tools/src/lib.rs::normalize_fetch_url` upgrades an initial `http://` URL to HTTPS unless the host is exactly `localhost`, `127.0.0.1`, or `::1`. But `build_http_client` enables `redirect(reqwest::redirect::Policy::limited(10))`, and `execute_web_fetch` sends the request once, then trusts `response.url()` as the final URL. There is no redirect policy that re-runs scheme/host/IP checks for each hop, no block for redirects from public HTTPS to `http://localhost`, `http://127.0.0.1`, RFC1918/link-local/metadata IPs, or non-HTTPS final URLs. As a result, a model-requested public URL can legally redirect the tool into local/private network resources even though direct non-local HTTP is upgraded and no SSRF boundary is documented. **Required fix shape:** (a) implement a custom redirect policy that validates every `Location` target before following it; (b) reject redirects to localhost, loopback, link-local, RFC1918/private ranges, cloud metadata hosts/IPs, and/or downgrade-to-HTTP unless explicitly allowed; (c) resolve hostnames before fetch or after redirect to prevent DNS-based private IP hops; (d) add tests with public HTTPS -> localhost/private/metadata redirects and safe HTTPS->HTTPS redirects; (e) expose final URL plus redirect-block reason in the `WebFetch` output/error without leaking response bodies. **Why this matters:** WebFetch is an external content tool. Redirects are part of the fetch boundary, not a postscript; without per-hop validation, a harmless-looking public URL can become an internal network probe or local service read. Source: gaebal-gajae dogfood response to Clawhip message `1507337256107642961` on 2026-05-22.
578. **`WebSearch` accepts `CLAWD_WEB_SEARCH_BASE_URL` with any scheme/host and follows the shared redirect policy, so a local environment variable can turn search into a private-network fetch without guardrails** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 11:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@bff67c2`. Active tmux session at probe time: `gajae-issue-339-backlog-zero-candidate-selection`; no active claw-code implementation session. Code inspection: `rust/crates/tools/src/lib.rs::build_search_url` checks `CLAWD_WEB_SEARCH_BASE_URL`, parses it with `reqwest::Url::parse`, appends `q`, and returns it unchanged. Unlike `WebFetch`, there is no `normalize_fetch_url`-style scheme upgrade or host validation at all. `execute_web_search` then uses the same `build_http_client` as WebFetch, which follows up to 10 redirects. A poisoned or stale local env var can point search at `http://localhost`, RFC1918 services, metadata endpoints, or a redirector to those targets; the tool will fetch and parse generic links before domain allow/block filters are applied to extracted result URLs, not to the search endpoint itself. Existing tests/logic focus on hit filtering, not search-provider endpoint validation. **Required fix shape:** (a) validate `CLAWD_WEB_SEARCH_BASE_URL` at parse time against allowed schemes/hosts or require an explicit unsafe/local-test opt-in; (b) apply the same per-redirect target validation required by #577 to WebSearch; (c) distinguish configured test search providers from production search with a typed config/source field in output; (d) add regressions for env base URLs pointing to localhost/private/metadata and safe HTTPS test endpoints; (e) document the env var as test-only or constrain it to trusted public search domains. **Why this matters:** search is often treated as a low-risk public-web tool. An unvalidated base URL makes it an environment-controlled internal fetch surface, and result-domain filters do not protect the initial request or redirects. Source: gaebal-gajae dogfood response to Clawhip message `1507344805167108257` on 2026-05-22.
579. **Streaming flushes pending markdown at `ContentBlockStop` before pending tool calls are rendered, so text immediately followed by a tool call can be displayed before the tool-use event that actually interrupted/ended the content block** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 12:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@d8864ff`. Active tmux sessions at probe time: none. Channel context included Jobdori's #587 finding that `MarkdownStreamState::push` can hold prose until flush; this probe inspected the caller ordering. Code inspection: in `rust/crates/rusty-claude-cli/src/main.rs` stream handling, `ContentBlockDelta::TextDelta` buffers text through `markdown_stream.push`. On `ApiStreamEvent::ContentBlockStop`, the code first calls `markdown_stream.flush(&renderer)` and writes any pending prose, then only afterward checks `pending_tool.take()` and renders `format_tool_call_start`. If a provider emits a text block that has no safe boundary (single paragraph or unclosed markdown) followed by a tool-use block, the held text is flushed at the stop event just before the pending tool call display. Operators watching the terminal can see the assistant's prose burst immediately before the tool start marker, even though the next actionable event is the tool call; any timing/ordering cue that the model paused to call a tool is blurred by the delayed text flush. There is no test asserting streamed text/tool display ordering when markdown buffering holds content until block stop. **Required fix shape:** (a) make stream rendering preserve block/event order explicitly, perhaps flushing text at the exact text block stop and rendering tool-use starts at their own block starts/stops with clear separators; (b) when a text block is followed by a tool block, include a newline/phase boundary so delayed text does not visually merge into the tool call; (c) add streaming tests with single-paragraph text immediately followed by a tool call and with safe-boundary text, asserting terminal output order and separators; (d) consider the #587 word-boundary fallback so prose is not entirely held until the tool boundary; (e) keep persisted `AssistantEvent` ordering aligned with displayed output. **Why this matters:** streaming UI is also an event log. If buffered prose appears only when the next block stops and visually collides with a tool-use marker, users cannot tell whether the model is still speaking, has switched to tool execution, or the stream stalled. Source: gaebal-gajae dogfood response to Clawhip message `1507352360379482193` on 2026-05-22.