Repository navigation
feat(ai): web_fetch tool — read direct URLs as markdown - #1386
Conversation
Adds a `web_fetch` AI tool alongside `web_search` that fetches the full content of a specific URL and returns it as clean markdown via TurndownService (already a project dependency). Gated by the same "Web" toggle as web_search — both tools are now grouped under WEB_SEARCH_TOOLS. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a new ChangesWeb Fetch Tool Addition
Sequence DiagramsequenceDiagram
participant Client
participant WebFetch as web_fetch Tool
participant Validator as URL & SSRF Validator
participant Fetcher as HTTP Fetch
participant Converter as HTML→Markdown
participant Response as Result
Client->>WebFetch: URL, userId
WebFetch->>Validator: Validate HTTPS
Validator->>Validator: Check isPrivateHost
Validator-->>WebFetch: Valid or Error
WebFetch->>Fetcher: Fetch with 15s timeout
Fetcher->>Converter: HTML (scripts/styles removed)
Converter->>Converter: Convert via TurndownService
Converter->>Converter: Truncate to maxLength
Converter-->>Response: Success/Error payload
Response-->>Client: Structured result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 49008499c3
ℹ️ 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".
| const response = await fetch(url, { | ||
| headers: { 'User-Agent': 'Mozilla/5.0 (compatible; PageSpace/1.0)' }, | ||
| signal: AbortSignal.timeout(15000), | ||
| }); |
There was a problem hiding this comment.
Calling fetch(url) directly on model/user-controlled input allows SSRF against services reachable only from the server network (for example http://127.0.0.1, RFC1918 ranges, or cloud metadata endpoints), which can leak sensitive internal data through tool output. This commit introduces web_fetch as a general URL reader but does not enforce a public-host allowlist/denylist or private-IP resolution checks before requesting the URL, so a prompt can intentionally or indirectly pivot this tool into internal infrastructure access.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the current implementation. validatePublicHost() now runs before any network I/O: it calls dns.promises.lookup(hostname, { all: true }) to resolve every IP the hostname can return, and passes each through isPrivateIp() which covers RFC1918, loopback (127.x, ::1), link-local (169.254.x, fe80::/10), and IPv6-mapped private ranges. DNS failure is fail-closed (throws). This catches both literal private IPs and public hostnames that DNS-rebind to private space.
Adds isPrivateHost() guard that rejects loopback (127.x, ::1, localhost), RFC1918 (10.x, 172.16-31.x, 192.168.x), link-local/cloud-metadata (169.254.x), and http:// before issuing fetch(). Addresses P1 SSRF finding from Codex review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
SSRF fix (e115fa8): Added |
- Reject non-HTML/text responses before running turndown to avoid binary garbage in tool output (e.g. PDFs, images, ZIPs) - Fix truncated flag: compare fullMarkdown.length > maxLength instead of markdown.length === maxLength to avoid false positives on exact-length content Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Quality fixes (4871f29):
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/web/src/lib/ai/tools/web-search-tools.ts`:
- Around line 253-268: Before calling response.text(), validate
response.headers: ensure Content-Type is text/html (or startsWith 'text/') and
if Content-Length exists reject if it exceeds a byteCap (e.g., 1MB). If
Content-Length is absent, read response.body as a stream (use
response.body.getReader()) and accumulate bytes up to the byteCap, aborting and
throwing if the cap is exceeded; then decode the accumulated bytes to a string
and pass that to TurndownService (instead of calling response.text()). Apply
these checks in the same block that currently creates `response` and where
`html`, `cleaned`, `td`, and `markdown` are produced, and throw clear errors for
non-text content or oversized responses.
- Line 251: Several logging calls (e.g., the webSearchLogger.debug at "Fetching
URL" and the similar calls around lines 270-275 and 290-292) are logging full
user-supplied URLs including query strings and fragments; change those to log a
redacted URL that strips search and hash. For each place where you call
webSearchLogger.debug/info with the url variable (e.g., the "Fetching URL"
call), construct a redactedUrl by parsing the input with the URL constructor in
a try/catch and using only urlObj.origin + urlObj.pathname (or fallback to the
original host/path if parsing fails), then pass redactedUrl in the log payload
(keep maskIdentifier(userId) as-is); apply the same change to all other logging
sites in this file that log url so no query or fragment is sent to logs.
- Around line 121-135: The current isPrivateHost(hostname) only checks the
literal hostname string; you must resolve the hostname to IP addresses before
allowing the fetch and reject requests whose resolved IPs are in
private/loopback/link-local/metadata or IPv6-local ranges. Update the request
path to perform a DNS resolution (e.g., using dns.promises.lookup or resolve
with all addresses for both A and AAAA) for the parsed.hostname (handling
bracketed IPv6), then create/replace with an isPrivateIp(ip: string) helper that
checks numeric ranges (IPv4: 10.0.0.0/8, 127.0.0.0/8, 169.254.0.0/16,
172.16.0.0/12, 192.168.0.0/16, etc.; IPv6: ::1, fe80::/10, fc00::/7, link-local,
etc.) and reject if any resolved address is private; if DNS resolution fails or
returns no addresses, treat as disallowed (fail closed). Ensure this change is
applied where parsed.hostname was previously passed to isPrivateHost (including
the other locations noted).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4441f936-2c7d-40c2-8e89-9946e2f8a843
📒 Files selected for processing (3)
apps/web/src/components/ui/floating-input/ToolsPopover.tsxapps/web/src/lib/ai/core/tool-filtering.tsapps/web/src/lib/ai/tools/web-search-tools.ts
… cap - Replace isPrivateHost() string check with validatePublicHost() which resolves hostname via dns.lookup and checks all returned IPs — catches public hostnames that point to private IPs (DNS rebinding mitigation); also extends IPv6 private coverage (fe80::/10, fc00::/7, ::ffff: mapped) - Add redactUrl() helper that strips query and fragment before logging to prevent signed token or secret leakage in server logs - Add Content-Length early-reject and stream body with 5 MB hard byte cap using ReadableStream reader instead of response.text() — prevents unbounded memory buffering of large responses Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Security hardening (b26e352) — addressing CodeRabbit review: Critical: DNS-based SSRF validation (replacing string-only
Major: URL redaction in logs
Major: Streaming body with byte cap
|
…happy path - SSRF: http rejection, private IPv4 literals, localhost, DNS rebinding (public hostname resolving to private IP), and DNS failure (fail-closed) - Content: non-HTML content-type rejection and Content-Length size limit - Happy path: markdown conversion, truncation flag accuracy Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ckLookupFn
vi.mock('dns') factory needs both default and named exports for ESM interop.
Variable must start with 'mock' to be hoisted before vi.mock factory runs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…itest 2.x Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
Adds `web_fetch` AI tool that fetches any public URL and returns its full content as clean markdown, controlled by the same toggle as `web_search`.
What changed
Security hardening
Test plan
🤖 Generated with Claude Code