Skip to content

Commit 28c7607

Browse files
author
SqlRush
committed
Filter unsafe WebFetch HTML hrefs
1 parent a054fca commit 28c7607

3 files changed

Lines changed: 23 additions & 5 deletions

File tree

docs/first-second-parity-audit.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ Covered behavior:
123123
- `Bash` initial shell execution, including command/timeout/description validation, `/bin/sh -c` execution, stdout/stderr/exit-code/timeout structured results, dynamic read-only/concurrency-safe/destructive classification, Git diff/log/show/status/ls-files/grep/rev-parse/branch/tag/ls-remote safe-flag validation, Git remote/push/reflog/stash/worktree/merge-base/describe/cat-file/for-each-ref/rev-list/blame/shortlog/config-get argument-aware safety classification, conservative read-only `sed` and `awk` classification for safe print/delete/quit/line-number or field-print scripts over relative paths, safe relative-path `head`/`tail`/`cut`/`uniq` reads with `tail` follow modes excluded from read-only auto-allow, simple safe `sort` reads, destructive `find -delete/-exec rm` and `xargs rm` classification, safe-wrapper/env prefix normalization for Bash read-only/destructive classification (`time`/`nohup`/`timeout`/`nice`/`stdbuf`/`env`), destructive command detection after temporary environment assignments, permission-rule matching through the existing tool permission adapter, background command start, same-session `BashOutput` reads, and `KillBash` cancellation.
124124
- `TodoWrite` initial todo state, including full-list writes, status/priority validation, duplicate-id rejection, one `in_progress` guard, structured result payloads, tool metadata state storage, and session-scoped local persistence/restore.
125125
- `WebFetch` initial URL fetch behavior, including URL/timeout/max-byte validation, HTTP GET, HEAD preflight, metadata/raw `skipWebFetchPreflight` skip-preflight, binary preflight GET skipping from content type or attachment filename, text/binary detection, truncation, non-2xx error marking, structured result payloads, HTML-to-text rendering, prompt-focused excerpts, prompt phrase scoring/metadata, and `WebFetch(domain:...)` permission-rule adaptation.
126-
- `WebFetch` HTML-to-text rendering now preserves anchor `href` values as link context and emits visible image text from `img` `alt`/`title`/`aria-label` plus `src`, the first usable `img srcset` candidate, common lazy image `data-src`/`data-srcset` candidates, or a `<picture><source srcset>` candidate, allowing prompt-focused excerpts to match image descriptions while avoiding duplicate URL link text and `javascript:` hrefs plus `data:` image placeholders.
126+
- `WebFetch` HTML-to-text rendering now preserves anchor `href` values as link context and emits visible image text from `img` `alt`/`title`/`aria-label` plus `src`, the first usable `img srcset` candidate, common lazy image `data-src`/`data-srcset` candidates, or a `<picture><source srcset>` candidate, allowing prompt-focused excerpts to match image descriptions while avoiding duplicate URL link text, unsafe `javascript:`/`data:`/`blob:`/`vbscript:` hrefs, and `data:` image placeholders.
127127
- `WebFetch` GET records the redirect-resolved `final_url`, and HTML rendering resolves relative anchor/image URLs against that final URL or the document's first valid `<base href>` so redirected pages expose browser-visible link and image targets.
128128
- `WebFetch` text bodies now decode common web charsets from BOM, `Content-Type`, or HTML `<meta charset>`/`http-equiv` declarations, including UTF-8/UTF-16LE/UTF-16BE, Latin-1, and Windows-1252, and structured results expose the normalized `charset`.
129129
- `WebSearch` initial HTML/JSON-search adapter, including query/max-result/timeout/domain-filter validation, injectable search endpoint, DuckDuckGo HTML link parsing, HTML JSON-LD ItemList extraction, DuckDuckGo subdomain redirect unwrapping, common JSON result shapes plus nested backend wrappers such as `web`/`response`/`hits`/`documents`, DuckDuckGo result snippet extraction, domain allow/block filtering, structured result payloads, and query-based permission-rule matching.

internal/tools/web/web_fetch.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ func webFetchHTMLBaseURL(body string, fallback string) string {
610610

611611
func resolveWebFetchBaseURL(raw string, fallback string) string {
612612
raw = strings.TrimSpace(raw)
613-
if raw == "" || strings.HasPrefix(strings.ToLower(raw), "javascript:") {
613+
if raw == "" || hasUnsafeWebFetchHTMLURLScheme(raw) {
614614
return ""
615615
}
616616
parsed, err := url.Parse(raw)
@@ -642,7 +642,7 @@ func appendHTMLWebFetchAnchorHref(b *strings.Builder, anchors []htmlWebFetchAnch
642642
anchor := anchors[len(anchors)-1]
643643
anchors = anchors[:len(anchors)-1]
644644
href := strings.TrimSpace(anchor.Href)
645-
if href == "" || strings.HasPrefix(href, "#") || strings.HasPrefix(strings.ToLower(href), "javascript:") {
645+
if href == "" || strings.HasPrefix(href, "#") || hasUnsafeWebFetchHTMLURLScheme(href) {
646646
return anchors, false
647647
}
648648
text := ""
@@ -766,7 +766,7 @@ func resolveWebFetchHTMLURL(raw string, baseURL string) string {
766766
if raw == "" || strings.HasPrefix(raw, "#") {
767767
return raw
768768
}
769-
if strings.HasPrefix(strings.ToLower(raw), "javascript:") {
769+
if hasUnsafeWebFetchHTMLURLScheme(raw) {
770770
return ""
771771
}
772772
parsed, err := url.Parse(raw)
@@ -783,6 +783,20 @@ func resolveWebFetchHTMLURL(raw string, baseURL string) string {
783783
return base.ResolveReference(parsed).String()
784784
}
785785

786+
func hasUnsafeWebFetchHTMLURLScheme(raw string) bool {
787+
idx := strings.IndexByte(raw, ':')
788+
if idx <= 0 {
789+
return false
790+
}
791+
scheme := strings.ToLower(strings.TrimSpace(raw[:idx]))
792+
switch scheme {
793+
case "javascript", "data", "blob", "vbscript":
794+
return true
795+
default:
796+
return false
797+
}
798+
}
799+
786800
func htmlWebFetchAttr(rawTag string, name string) string {
787801
name = strings.ToLower(strings.TrimSpace(name))
788802
for _, match := range htmlWebFetchAttrRe.FindAllStringSubmatch(rawTag, -1) {

internal/tools/web/web_fetch_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,9 @@ func TestWebFetchHTMLRenderingPreservesLinksAndImageText(t *testing.T) {
242242
<img alt="Picture data placeholder" src="/assets/picture-fallback.png">
243243
</picture>
244244
<a href="javascript:alert(1)">ignored script link</a>
245+
<a href="data:text/plain;base64,SGVsbG8=">ignored data link</a>
246+
<a href="blob:https://example.com/asset-id">ignored blob link</a>
247+
<a href="vbscript:msgbox(1)">ignored vbscript link</a>
245248
</main>
246249
</body>
247250
</html>`))
@@ -290,7 +293,8 @@ func TestWebFetchHTMLRenderingPreservesLinksAndImageText(t *testing.T) {
290293
if !strings.Contains(rendered, "Image: Picture data placeholder ("+server.URL+"/assets/picture-real.png)") {
291294
t.Fatalf("rendered body missing data-placeholder picture source text: %#v", rendered)
292295
}
293-
if strings.Contains(rendered, "javascript:alert") || strings.Contains(rendered, "data:image") || strings.Contains(rendered, "R0lGOD") {
296+
if strings.Contains(rendered, "javascript:alert") || strings.Contains(rendered, "data:image") || strings.Contains(rendered, "R0lGOD") ||
297+
strings.Contains(rendered, "data:text/plain") || strings.Contains(rendered, "blob:https://") || strings.Contains(rendered, "vbscript:") {
294298
t.Fatalf("rendered body kept unsafe href: %#v", rendered)
295299
}
296300
excerpt, ok := result.StructuredContent["prompt_excerpt"].(string)

0 commit comments

Comments
 (0)