[Feature] Add web-fetch as build-in tool - #27
Merged
Conversation
…tput Two user-visible defects, each from reading or configuring the wrong thing rather than from the display code. 1. The status bar stayed at 0/1M for a whole session. Conversation state lives on the per-session engines built by SessionRegistry. ctx.engine is only the template they are cloned from and never accumulates turns, so every metadata path reading it reported zero context. Three call sites did exactly that: the per-chunk stream metadata (the status bar's main source), status(), and the config-reload notification. This is the third time this root cause has surfaced — it produced the empty LeapBoard earlier, and I fixed that one call site without converging the others, so it reappeared here. Fixed as a single entry point this time: _active_engine() resolves through SessionCoordinator, all three sites use it, and the streaming path passes the engine that produced the event so concurrent sessions cannot be cross-reported. Link protection, since a wrong read is invisible in review: a contract test scans for engine_context_metadata() calls that take ctx.engine directly, and AGENTS.md now states the session engine is the only reporting source. 2. Long answers were clipped at the right edge. LeapConsole was built with soft_wrap=True, which makes Rich emit one long line and defer wrapping to whoever owns the screen; prompt_toolkit's renderer clips instead of reflowing. Now soft_wrap=False so Rich wraps to the console width, which it re-derives from the terminal file descriptors on every access and therefore still follows resizes. Verified: a long CJK paragraph goes from one 166-char line to properly wrapped lines. banner.py keeps soft_wrap for fixed-width art but sets an explicit width, so it is unaffected. AGENTS.md gains three contracts: session engine as the only reporting source, client-visible runtime state must be pushed rather than inferred, and wrapping belongs to the console layer. 12 new tests; reintroducing either defect fails 7 of them. Suite green.
Reading a URL previously meant shell_run with a hand-written `curl | python3 -c` pipeline. That path is classified external_side_effect, so a plain GET inherited side-effect batch stopping, session-scoped dedup, and "this may already have taken effect" retry guidance — none of which describe reading a web page. HTTP failures also surfaced as whatever the improvised pipeline printed, typically a Python traceback rather than a status code. web_fetch is declared read_only and disclosed every turn: retries are safe, the batch gate does not fire, and failures return structured status information. Two transports are kept deliberately — httpx as the async-native default, curl as the fallback for environments without it. JSON responses support a dotted `select` so only the needed branch enters context. Egress is gated. URL classification lives in security/network.py rather than in risk.py, because deciding whether a host is internal needs DNS resolution: that is I/O and must not run inside an approval decision. The tool resolves the target off the event loop and passes the verdict as action metadata for the synchronous classifier to judge. Resolution — not just literal addresses — is what matters, since a public hostname can point at 127.0.0.1 or a cloud metadata endpoint. httpx is now an explicit dependency instead of being borrowed transitively through openai; trafilatura is an optional `web` extra that improves main-content extraction without gating the feature, since the stdlib extractor always ships. Suite green (1374 passed), ruff clean. Note: this change came from a separate working session, not the status-bar / wrapping work in the preceding commit. It is committed as requested with the suite verified; the web transport behavior itself has not been exercised against live endpoints here.
…ssification gaps Review pass over the web_fetch commit that precedes this one. Four of the six findings are security defects in that change; one is a regression it introduced in the shell path. 1. The egress gate only ever saw the first hop. classify_url ran once per fetch while the transports were configured to follow redirects themselves (follow_redirects=True, curl --location). Any server able to answer 302 could therefore bounce a request from an approved public URL to loopback or a cloud metadata endpoint, and the gate never inspected the target whose body was returned. Transports are now single-hop and report the Location header instead of acting on it; the orchestrator follows the chain and re-classifies and re-gates every hop. Redirect loops and the hop budget are enforced there too, and the chain is reported so the model can see where it was sent. A source-level test pins follow_redirects=False so the auto-follow cannot quietly come back. 2. Alibaba Cloud's metadata service was classified as public. The classifier decided "public" with `not is_private`, but 100.64.0.0/10 is shared address space rather than RFC1918, so ipaddress reports is_private=False for 100.100.100.200 and the address was treated as ordinary internet. The test is now `is_global`, which asks the question that actually matters — is this routable on the public internet — and fails closed for every unroutable range we have not enumerated (CGNAT, benchmarking, documentation). The endpoint is also named explicitly so the approval prompt can state the consequence. 3. Approval details carried URL query strings into the audit log. ApprovalAuditLog persists an action's detail and the prompt displays it, while a query string routinely holds an API key or a signed token. The detail is now origin plus path; query, fragment, and userinfo are dropped before the action is built. config_tools had already established this rule for config values and this path did not follow it. 4. Expanding shell variables blocked ordinary commands. The workspace gate expands variables so `$HOME/x` cannot evade it, but `$PATH` expands to an os.pathsep-joined list that begins with `/` and names no file, so `echo $PATH` and `PATH=$PATH:./node_modules/.bin npm test` were refused. An expansion that is not a single filesystem operand is now ignored; single-path variables stay gated. 5. Compacted HTTP failures lost their explanation. _compact_error dropped `status` and `body_excerpt`, leaving the model only the prose error. Both are preserved now, the body excerpt head/tail trimmed. 6. A completion with no matching start printed nothing. Correlating tool lines by tool_call_id improved batch rendering but traded a wrong line for no line at all. An unmatched completion now still prints, since silence hides a tool the turn actually ran. Also: cache storage moved out of web_fetch.py into tools/web_cache.py (that module had grown to ~890 lines carrying transport, gating, caching and result shaping); the text cap now derives from web.max_bytes instead of a private constant; the per-call timeout and size ceilings are named; a hardcoded Accept-Language that biased fetched pages toward English is removed; and FetchRequest.max_redirects is dropped now that no transport follows redirects. Verified against live endpoints this time, not only the suite: a JSON API with select, an http->https redirect chain followed to completion, and refusals for the AWS and Alibaba metadata addresses, loopback, and file://. Suite green (1384 passed), ruff clean. The TUI batch rendering from the preceding commit has now been confirmed by hand.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.