diff --git a/docs/changes/unreleased/1771-web-fetch-cached-snapshot.md b/docs/changes/unreleased/1771-web-fetch-cached-snapshot.md new file mode 100644 index 0000000000..e0eae318f1 --- /dev/null +++ b/docs/changes/unreleased/1771-web-fetch-cached-snapshot.md @@ -0,0 +1,14 @@ +--- +kind: fixed +title: web fetch asks the reader for a fresh copy and rejects its cached snapshot +pr: 1771 +surface: [chat, engine] +invalidates: + - "`codeaf web fetch https://example.com` returned r.jina.ai's stale cached snapshot of the page — Jina's own 'cached snapshot' page — because the reader serves its 3600s page cache by default. It now sends the reader's no-cache header and, when the reader still marks its answer a cache hit, fetches the target directly, so what comes back is the page at that address now." +--- + +The Jina reader r.jina.ai serves a 3600s page cache by default, and codeaf's +fetch passed its answer through verbatim — so a model asked to read a page +could be handed someone else's cache of it without knowing. The fetch now +asks for a fresh copy on every request, and discards an answer the reader +still marks as cached (X-Cache: HIT), fetching the target itself instead. diff --git a/internal/search/jina.go b/internal/search/jina.go index d6de6c5660..b777e70138 100644 --- a/internal/search/jina.go +++ b/internal/search/jina.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "regexp" "strings" ) @@ -162,6 +163,11 @@ func (j jina) Fetch(ctx context.Context, target string) (string, error) { req.Header.Set("Authorization", "Bearer "+key) } req.Header.Set("Accept", "text/plain") + // The reader keeps a 3600s page cache and answers from it by default; + // X-No-Cache asks it not to. The ask is not a guarantee — a HIT can still + // come back (see [cacheHit] below), but without the header every answer + // would be the cache's. + req.Header.Set("X-No-Cache", "true") resp, err := j.opts.client().Do(req) if err != nil { @@ -177,6 +183,18 @@ func (j jina) Fetch(ctx context.Context, target string) (string, error) { return "", fmt.Errorf("jina: %s%s", resp.Status, detail(body)) } + // A HIT is the reader serving its OWN 3600s page cache, which is someone + // else's snapshot of the page, not the page. It is deliberately not + // passed through: an agent that was handed it would believe it had read + // the page. Discard it and fetch the target itself. + if cacheHit(resp.Header) { + text, err := jinaDirectFetch(ctx, j.opts, target) + if err != nil { + return "", fmt.Errorf("jina: cached snapshot rejected; direct fetch of %s: %w", target, err) + } + return text, nil + } + // Returned whole. Capping is the RENDERER's job ([RenderFetch]) so that a // caller doing something other than prompting a model — a summariser, a // grep — is not handed a page someone already cut for a different budget. @@ -186,3 +204,75 @@ func (j jina) Fetch(ctx context.Context, target string) (string, error) { } return text, nil } + +// cacheHit reports whether the reader answered from its own page cache: the +// X-Cache response header is HIT. Trimmed and case-insensitive, because a +// header's casing and whitespace are the answerer's, not ours. +func cacheHit(header http.Header) bool { + return strings.EqualFold(strings.TrimSpace(header.Get("X-Cache")), "HIT") +} + +// jinaDirectFetch is the freshness fallback: GET the target itself when the +// reader answered from its cache, and reduce its HTML to text so the result +// has the same readable shape a fresh reader answer would have had. +func jinaDirectFetch(ctx context.Context, opts Options, target string) (string, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + if err != nil { + return "", err + } + req.Header.Set("User-Agent", "Mozilla/5.0 (compatible; codeaf/1.0)") + + resp, err := opts.client().Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, 8<<20)) + if err != nil { + return "", fmt.Errorf("read response: %w", err) + } + if resp.StatusCode >= http.StatusBadRequest { + return "", fmt.Errorf("http %d%s", resp.StatusCode, detail(body)) + } + + text := htmlToText(string(body)) + if text == "" { + return "", fmt.Errorf("empty body") + } + return text, nil +} + +// HTML stripping for the direct fetch, modeled on the resident's helper of +// the same shape: crude on purpose, because the consumer is a language model +// that tolerates ragged text far better than it tolerates paying for markup. +var ( + // Written out per element rather than with a backreference: Go's regexp is + // RE2, which has none, and a pattern that compiles everywhere except here + // would fail at init rather than at review. + dropElements = regexp.MustCompile(`(?is)]*>.*?` + + `|]*>.*?` + + `|]*>.*?` + + `|]*>.*?` + + `|]*>.*?`) + blockBreaks = regexp.MustCompile(`(?i)]*>`) + anyTag = regexp.MustCompile(`(?s)<[^>]+>`) + manyBlanks = regexp.MustCompile(`\n{3,}`) + manySpaces = regexp.MustCompile(`[ \t]{2,}`) +) + +// htmlToText strips markup well enough to read. It is intentionally crude: +// the consumer is a language model, which tolerates ragged text far better +// than it tolerates paying for markup it will ignore. +func htmlToText(page string) string { + page = dropElements.ReplaceAllString(page, " ") + page = blockBreaks.ReplaceAllString(page, "\n") + page = anyTag.ReplaceAllString(page, " ") + page = strings.NewReplacer( + " ", " ", "&", "&", "<", "<", ">", ">", + """, `"`, "'", "'", "—", "—", "–", "–", + ).Replace(page) + page = manySpaces.ReplaceAllString(page, " ") + page = manyBlanks.ReplaceAllString(page, "\n\n") + return strings.TrimSpace(page) +} diff --git a/internal/search/search_test.go b/internal/search/search_test.go index e2087b1126..fd1b9583b1 100644 --- a/internal/search/search_test.go +++ b/internal/search/search_test.go @@ -448,6 +448,140 @@ func TestJinaFetchIsCappedByTheRenderer(t *testing.T) { } } +// TestJinaFetchAsksForAFreshCopy: the reader keeps a 3600s page cache and +// serves it by default, so every fetch must ask it not to, key or no key. +func TestJinaFetchAsksForAFreshCopy(t *testing.T) { + const page = "# Go 1.25\n\nThe release notes, as markdown." + var ( + gotNoCache string + gotAuth string + ) + server := serve(t, &jinaBaseURL, func(w http.ResponseWriter, r *http.Request) { + gotNoCache = r.Header.Get("X-No-Cache") + gotAuth = r.Header.Get("Authorization") + fmt.Fprint(w, page) + }) + jinaBaseURL = server.URL + "/" + + fetcher := jina{}.Bind(Options{}) + if _, err := fetcher.Fetch(context.Background(), "https://go.dev/doc/go1.25"); err != nil { + t.Fatalf("Fetch error = %v", err) + } + if gotNoCache != "true" { + t.Errorf("X-No-Cache = %q, want the no-cache ask", gotNoCache) + } + if gotAuth != "" { + t.Errorf("Authorization = %q, want none without a key", gotAuth) + } + + // A key only raises the rate ceiling, but it must ride the same ask. + if _, err := (jina{}).Bind(Options{JinaKey: "jina-key"}).Fetch(context.Background(), "https://go.dev"); err != nil { + t.Fatalf("keyed Fetch error = %v", err) + } + if gotNoCache != "true" { + t.Errorf("keyed X-No-Cache = %q, want the no-cache ask", gotNoCache) + } + if gotAuth != "Bearer jina-key" { + t.Errorf("Authorization = %q, want Bearer jina-key", gotAuth) + } +} + +// TestJinaFetchFallsBackWhenTheReaderServedItsCache: when the reader marks +// its answer a HIT it is serving someone ELSE's snapshot, so the fetch must +// discard it and GET the target directly, reducing the page to text. +// The handler tells the two routes apart by path: the target is the server's +// own /page, while the reader route prefixes it with the reader base URL. +func TestJinaFetchFallsBackWhenTheReaderServedItsCache(t *testing.T) { + const page = `Example Domain

Example Domain

This domain is for use in illustrative examples in documents.

Ignore & forget me
` + server := serve(t, &jinaBaseURL, func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/http") { + // The reader route: a cached snapshot of the page. + w.Header().Set("X-Cache", "HIT") + fmt.Fprint(w, "# stale cached snapshot, not the page") + return + } + if r.URL.Path != "/page" { + t.Errorf("unexpected route %q", r.URL.Path) + return + } + w.Header().Set("Content-Type", "text/html") + fmt.Fprint(w, page) + }) + jinaBaseURL = server.URL + "/" + + target := server.URL + "/page" + text, err := jina{}.Bind(Options{}).Fetch(context.Background(), target) + if err != nil { + t.Fatalf("Fetch error = %v", err) + } + if strings.Contains(text, "stale cached snapshot") { + t.Errorf("Fetch = %q, returned the reader's snapshot instead of the page", text) + } + if !strings.Contains(text, "Example Domain") || !strings.Contains(text, "illustrative examples") { + t.Errorf("Fetch = %q, want the direct page's own text", text) + } + if strings.Contains(text, "

") || strings.Contains(text, "&") { + t.Errorf("Fetch = %q, want the markup stripped and entities decoded", text) + } +} + +// TestJinaFetchNamesTheFallbackWhenTheDirectFetchFails: a HIT must never be +// returned, so when the direct fetch itself fails the error has to name that +// fallback rather than hand the snapshot over silently. +func TestJinaFetchNamesTheFallbackWhenTheDirectFetchFails(t *testing.T) { + server := serve(t, &jinaBaseURL, func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/http") { + w.Header().Set("X-Cache", "HIT") + fmt.Fprint(w, "# stale cached snapshot") + return + } + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, "boom") + }) + jinaBaseURL = server.URL + "/" + + target := server.URL + "/page" + _, err := jina{}.Bind(Options{}).Fetch(context.Background(), target) + if err == nil { + t.Fatal("Fetch of a HIT whose direct fetch 500s returned no error") + } + msg := err.Error() + if !strings.Contains(msg, "cached snapshot rejected") || !strings.Contains(msg, "direct fetch of "+target) { + t.Errorf("Fetch error = %q, want the rejected snapshot and its direct fallback named", msg) + } + if strings.Contains(msg, "stale cached snapshot") { + t.Errorf("Fetch error = %q, leaked the snapshot body", msg) + } +} + +// TestJinaFetchEmptyDirectPageIsAnError: an empty direct response means the +// fallback found nothing to read, and the error must say so — never fall back +// to the snapshot it just rejected. +func TestJinaFetchEmptyDirectPageIsAnError(t *testing.T) { + server := serve(t, &jinaBaseURL, func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/http") { + w.Header().Set("X-Cache", "HIT") + fmt.Fprint(w, "# stale cached snapshot") + return + } + w.Header().Set("Content-Type", "text/html") + }) + jinaBaseURL = server.URL + "/" + + target := server.URL + "/page" + _, err := jina{}.Bind(Options{}).Fetch(context.Background(), target) + if err == nil { + t.Fatal("Fetch of a HIT whose direct body is empty returned no error") + } + msg := err.Error() + if !strings.Contains(msg, "cached snapshot rejected") || !strings.Contains(msg, "direct fetch of "+target) { + t.Errorf("Fetch error = %q, want the rejected snapshot and its direct fallback named", msg) + } + if strings.Contains(msg, "stale cached snapshot") { + t.Errorf("Fetch error = %q, leaked the snapshot body", msg) + } +} + // --- resolution ----------------------------------------------------------- func TestResolveAuto(t *testing.T) {