From 3cbe05d67c345e3a25f490f1024e8f77836cede7 Mon Sep 17 00:00:00 2001 From: George Pu <19347973+thegeorgepu@users.noreply.github.com> Date: Sat, 5 Sep 2026 11:37:51 -0400 Subject: [PATCH 1/2] fix(search): validate every redirect hop --- vinci/extensions/vinci-search.test.mjs | 194 ++++++++++++++++++++++++- vinci/extensions/vinci-search.ts | 117 ++++++++++++--- 2 files changed, 286 insertions(+), 25 deletions(-) diff --git a/vinci/extensions/vinci-search.test.mjs b/vinci/extensions/vinci-search.test.mjs index 7c90b384b..b6fa459bb 100644 --- a/vinci/extensions/vinci-search.test.mjs +++ b/vinci/extensions/vinci-search.test.mjs @@ -3,7 +3,64 @@ // isPrivateIp must decode and classify the embedded address in BOTH spellings. import { test } from "node:test"; import assert from "node:assert/strict"; -import { isPrivateIp, preflightUrl } from "./vinci-search.ts"; +import registerVinciSearch, { + fetchPublicPage, + isPrivateIp, + preflightUrl, + WEB_FETCH_MAX_REDIRECTS, +} from "./vinci-search.ts"; + +const publicAddress = "93.184.216.34"; +const signal = new AbortController().signal; + +function response(status, location, body = "public page") { + const values = new Map([ + ["content-type", "text/plain"], + ...(location === undefined ? [] : [["location", location]]), + ]); + return { + ok: status >= 200 && status < 300, + status, + headers: { get: (name) => values.get(name.toLowerCase()) ?? null }, + text: async () => body, + }; +} + +function dependencies(responses, addresses = new Map()) { + const requests = []; + const resolutions = []; + return { + requests, + resolutions, + value: { + fetch: async (url, init) => { + requests.push({ url: url.href, redirect: init?.redirect }); + const next = responses.shift(); + if (next instanceof Error) throw next; + assert.ok(next, `unexpected fetch of ${url.href}`); + return next; + }, + lookup: async (hostname) => { + resolutions.push(hostname); + const next = addresses.get(hostname); + if (next instanceof Error) throw next; + return next ?? [{ address: publicAddress }]; + }, + }, + }; +} + +function registeredWebFetch() { + const tools = []; + registerVinciSearch({ registerTool: (tool) => tools.push(tool) }); + const webFetch = tools.find((tool) => tool.name === "web_fetch"); + assert.ok(webFetch, "production extension must register web_fetch"); + return webFetch; +} + +function resultText(result) { + return result.content.map((part) => part.text ?? "").join("\n"); +} // Negative: every private / loopback / link-local / NAT64 form must be classified PRIVATE. const PRIVATE = [ @@ -116,3 +173,138 @@ test("preflightUrl still allows public mapped, NAT64, IPv4, and IPv6 hosts", () assert.equal(preflightUrl("http://[2606:2800:220:1:248:1893:25c8:1946]/").ok, true); assert.equal(preflightUrl("http://[::ffff:ffff]/").ok, true); }); + +test("web_fetch production registration refuses a private redirect before a second request", async () => { + const originalFetch = globalThis.fetch; + const requests = []; + globalThis.fetch = async (url, init) => { + requests.push({ url: url.href, redirect: init?.redirect }); + return response(302, "http://127.0.0.1/private"); + }; + try { + const result = await registeredWebFetch().execute("call", { url: `https://${publicAddress}/start` }, signal); + assert.match(resultText(result), /internal server/); + assert.deepEqual(requests, [{ url: `https://${publicAddress}/start`, redirect: "manual" }]); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("public relative and absolute redirects are fetched after URL and DNS validation at every hop", async () => { + const deps = dependencies([ + response(302, "/relative"), + response(307, "https://final.example/page"), + response(200, undefined, "arrived"), + ]); + const result = await fetchPublicPage("https://start.example/root", signal, deps.value); + assert.equal(result.ok, true); + assert.equal(result.url.href, "https://final.example/page"); + assert.deepEqual(deps.resolutions, ["start.example", "start.example", "final.example"]); + assert.deepEqual(deps.requests, [ + { url: "https://start.example/root", redirect: "manual" }, + { url: "https://start.example/relative", redirect: "manual" }, + { url: "https://final.example/page", redirect: "manual" }, + ]); +}); + +test("a later-hop hostname resolving private is refused before that hop is fetched", async () => { + const addresses = new Map([ + ["start.example", [{ address: publicAddress }]], + ["private.example", [{ address: "10.0.0.8" }]], + ]); + const deps = dependencies([response(302, "https://private.example/secret")], addresses); + const result = await fetchPublicPage("https://start.example/", signal, deps.value); + assert.equal(result.ok, false); + assert.match(result.reason, /resolves to an internal server/); + assert.deepEqual(deps.resolutions, ["start.example", "private.example"]); + assert.deepEqual(deps.requests, [{ url: "https://start.example/", redirect: "manual" }]); +}); + +test("all redirect Location status and shape failures are typed and mechanism-reaching", async (t) => { + const badLocations = [ + ["missing", undefined, /without a valid Location/], + ["null", null, /without a valid Location/], + ["empty", " ", /without a valid Location/], + ["wrong type", 42, /without a valid Location/], + ["malformed", "http://[", /malformed Location/], + ["unsupported protocol", "file:///etc/passwd", /Only http\(s\)/], + ]; + for (const [name, location, expected] of badLocations) { + await t.test(name, async () => { + const deps = dependencies([response(302, location)]); + const result = await fetchPublicPage("https://start.example/", signal, deps.value); + assert.equal(result.ok, false); + assert.match(result.reason, expected); + assert.equal(deps.requests.length, 1, "the redirect target must not be requested"); + }); + } +}); + +test("only defined redirect statuses consume Location", async () => { + for (const status of [200, 201, 300, 304, 305, 306]) { + const deps = dependencies([response(status, "http://127.0.0.1/private")]); + const result = await fetchPublicPage("https://start.example/", signal, deps.value); + assert.equal(result.ok, true, `HTTP ${status} is not a followed redirect`); + assert.equal(result.response.status, status); + assert.equal(deps.requests.length, 1); + } + for (const status of [301, 302, 303, 307, 308]) { + const deps = dependencies([response(status, "http://127.0.0.1/private")]); + const result = await fetchPublicPage("https://start.example/", signal, deps.value); + assert.equal(result.ok, false, `HTTP ${status} must enter redirect validation`); + assert.match(result.reason, /internal server/); + assert.equal(deps.requests.length, 1); + } +}); + +test("redirect loops and redirects beyond the explicit cap are refused", async () => { + const loopDeps = dependencies([response(302, "/b"), response(302, "/")]); + const loop = await fetchPublicPage("https://loop.example/", signal, loopDeps.value); + assert.equal(loop.ok, false); + assert.match(loop.reason, /redirect loop/); + assert.equal(loopDeps.requests.length, 2); + + const excessResponses = Array.from({ length: WEB_FETCH_MAX_REDIRECTS + 1 }, (_, index) => response(302, `/hop-${index + 1}`)); + const excessDeps = dependencies(excessResponses); + const excess = await fetchPublicPage("https://many.example/", signal, excessDeps.value); + assert.equal(excess.ok, false); + assert.match(excess.reason, new RegExp(`more than ${WEB_FETCH_MAX_REDIRECTS} times`)); + assert.equal(excessDeps.requests.length, WEB_FETCH_MAX_REDIRECTS + 1); +}); + +test("the redirect cap still permits a public final response at its boundary", async () => { + const responses = Array.from({ length: WEB_FETCH_MAX_REDIRECTS }, (_, index) => response(302, `/hop-${index + 1}`)); + responses.push(response(200)); + const deps = dependencies(responses); + const result = await fetchPublicPage("https://bounded.example/", signal, deps.value); + assert.equal(result.ok, true); + assert.equal(deps.requests.length, WEB_FETCH_MAX_REDIRECTS + 1); +}); + +test("DNS and fetch failures return stable tool errors", async () => { + const dnsDeps = dependencies([], new Map([["dns-fails.example", new Error("offline")]])); + const dns = await fetchPublicPage("https://dns-fails.example/", signal, dnsDeps.value); + assert.equal(dns.ok, false); + assert.equal(dns.reason, "Couldn't resolve that web address."); + assert.equal(dnsDeps.requests.length, 0); + + const fetchDeps = dependencies([new Error("offline")]); + const unreachable = await fetchPublicPage("https://fetch-fails.example/", signal, fetchDeps.value); + assert.equal(unreachable.ok, false); + assert.equal(unreachable.reason, "Couldn't reach that page right now."); + + const timeout = new Error("slow"); + timeout.name = "TimeoutError"; + const timeoutDeps = dependencies([timeout]); + const timedOut = await fetchPublicPage("https://timeout.example/", signal, timeoutDeps.value); + assert.equal(timedOut.ok, false); + assert.equal(timedOut.reason, "That page took too long to load."); +}); + +test("web_fetch handles missing, null, empty, malformed, and wrong-type input without reaching fetch", async () => { + const webFetch = registeredWebFetch(); + for (const params of [{}, { url: null }, { url: "" }, { url: "not a URL" }, { url: 7 }, { url: [] }]) { + const result = await webFetch.execute("call", params, signal); + assert.match(resultText(result), /valid web address/); + } +}); diff --git a/vinci/extensions/vinci-search.ts b/vinci/extensions/vinci-search.ts index 44a352dfd..569e5e309 100644 --- a/vinci/extensions/vinci-search.ts +++ b/vinci/extensions/vinci-search.ts @@ -207,6 +207,17 @@ function formatDocs(library: string, topic: string | undefined, docs: string, tr const FETCH_MAX_CHARS = 30000; // cap the extracted text — a docs page can be huge; more just bloats context const FETCH_MAX_BYTES = 5_000_000; // refuse to download more than ~5MB of HTML +export const WEB_FETCH_MAX_REDIRECTS = 5; + +type LookupAddress = { address: string }; +type WebFetchDependencies = { + fetch: typeof globalThis.fetch; + lookup: (hostname: string, options: { all: true }) => Promise; +}; + +export type PublicPageFetchResult = + | { ok: true; response: Response; url: URL } + | { ok: false; reason: string; url?: URL }; /** Decode the 32 bits after an IPv4-mapped / NAT64 /96 prefix as a dotted-quad IPv4 string. * Accepts both spellings: ``::ffff:127.0.0.1`` (dotted) and ``::ffff:7f00:1`` (hex). @@ -289,6 +300,81 @@ export function preflightUrl(raw: string): { ok: true; url: URL } | { ok: false; return { ok: true, url }; } +const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); + +async function resolvePublicHost(url: URL, lookupHost: WebFetchDependencies["lookup"]): Promise { + const host = url.hostname.replace(/^\[|\]$/g, ""); + if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host) || host.includes(":")) return undefined; + try { + const addrs = await lookupHost(host, { all: true }); + if (addrs.some((address) => isPrivateIp(address.address))) { + return "That address resolves to an internal server — not reading it."; + } + } catch { + return "Couldn't resolve that web address."; + } + return undefined; +} + +/** Fetch one public page while applying the complete SSRF boundary before every network hop. + * Redirects are handled manually so the runtime cannot follow a newly supplied private target between + * the one-time URL/DNS checks and the final response. Five redirects are allowed; a sixth is refused. */ +export async function fetchPublicPage( + raw: string, + signal: AbortSignal, + dependencies: WebFetchDependencies = { fetch: globalThis.fetch, lookup }, +): Promise { + const initial = preflightUrl(raw); + if (!initial.ok) return initial; + + let current = initial.url; + let redirects = 0; + const seen = new Set([current.href]); + + while (true) { + const resolutionError = await resolvePublicHost(current, dependencies.lookup); + if (resolutionError) return { ok: false, reason: resolutionError, url: current }; + + let response: Response; + try { + response = await dependencies.fetch(current, { + redirect: "manual", + signal, + headers: { "User-Agent": "Mozilla/5.0 (compatible; VinciCode/1.0; +https://getsimpledirect.com)", Accept: "text/html,text/plain,*/*" }, + }); + } catch (error) { + const reason = error instanceof Error && error.name === "TimeoutError" ? "That page took too long to load." : "Couldn't reach that page right now."; + return { ok: false, reason, url: current }; + } + + if (!REDIRECT_STATUSES.has(response.status)) return { ok: true, response, url: current }; + if (redirects >= WEB_FETCH_MAX_REDIRECTS) { + return { ok: false, reason: `That page redirected more than ${WEB_FETCH_MAX_REDIRECTS} times — not following it.`, url: current }; + } + + const location: unknown = response.headers.get("location"); + if (typeof location !== "string" || location.trim().length === 0) { + return { ok: false, reason: "That page returned a redirect without a valid Location address.", url: current }; + } + + let resolved: URL; + try { + resolved = new URL(location, current); + } catch { + return { ok: false, reason: "That page returned a redirect with a malformed Location address.", url: current }; + } + const next = preflightUrl(resolved.href); + if (!next.ok) return { ok: false, reason: next.reason, url: current }; + if (seen.has(next.url.href)) { + return { ok: false, reason: "That page entered a redirect loop — not following it.", url: current }; + } + + redirects += 1; + current = next.url; + seen.add(current.href); + } +} + /** Strip an HTML document down to readable text (headings/paragraphs kept as newlines). */ export function htmlToText(html: string): string { return html @@ -449,33 +535,16 @@ export default function (pi: ExtensionAPI) { }, async execute(_toolCallId, params: { url: string }, signal) { const details: { tool: string; url: string; words: number; truncated: boolean } = { tool: "web_fetch", url: "", words: 0, truncated: false }; - const pre = preflightUrl((params.url || "").trim()); - if (!pre.ok) return { content: [{ type: "text", text: pre.reason }], details }; - details.url = pre.url.href; - - // Resolve the hostname and confirm EVERY IP is public (SSRF: a public name must not resolve to - // an internal address). IP-literal hosts were already checked in preflight. - const host = pre.url.hostname.replace(/^\[|\]$/g, ""); - if (!/^\d{1,3}(\.\d{1,3}){3}$/.test(host) && !host.includes(":")) { - try { - const addrs = await lookup(host, { all: true }); - if (addrs.some((a) => isPrivateIp(a.address))) { - return { content: [{ type: "text", text: "That address resolves to an internal server — not reading it." }], details }; - } - } catch { - return { content: [{ type: "text", text: "Couldn't resolve that web address." }], details }; - } - } - + const rawUrl = typeof params?.url === "string" ? params.url.trim() : ""; const timeout = AbortSignal.timeout(20000); const combined = typeof AbortSignal.any === "function" && signal instanceof AbortSignal ? AbortSignal.any([signal, timeout]) : timeout; + const fetched = await fetchPublicPage(rawUrl, combined); + if (!fetched.ok) return { content: [{ type: "text", text: fetched.reason }], details }; + + details.url = fetched.url.href; + const res = fetched.response; try { - const res = await fetch(pre.url, { - redirect: "follow", - signal: combined, - headers: { "User-Agent": "Mozilla/5.0 (compatible; VinciCode/1.0; +https://getsimpledirect.com)", Accept: "text/html,text/plain,*/*" }, - }); if (!res.ok) { // A 404/410 usually means the model GUESSED a plausible-but-fake URL. Coach it to search for // the real page rather than guess another URL — the observed failure mode (invented @@ -497,7 +566,7 @@ export default function (pi: ExtensionAPI) { const text = truncated ? extracted.slice(0, FETCH_MAX_CHARS) : extracted; details.words = text.split(/\s+/).filter(Boolean).length; details.truncated = truncated; - return { content: [{ type: "text", text: formatPage(pre.url.href, text, truncated) }], details }; + return { content: [{ type: "text", text: formatPage(fetched.url.href, text, truncated) }], details }; } catch (e) { const msg = e instanceof Error && e.name === "TimeoutError" ? "That page took too long to load." : "Couldn't reach that page right now."; return { content: [{ type: "text", text: msg }], details }; From 3d2dbc04ef2606a88edcbefbff61712c90392484 Mon Sep 17 00:00:00 2001 From: George Pu <19347973+thegeorgepu@users.noreply.github.com> Date: Sat, 5 Sep 2026 11:48:37 -0400 Subject: [PATCH 2/2] fix(search): validate complete DNS answers --- vinci/extensions/vinci-search.test.mjs | 79 +++++++++++++++++++++++++- vinci/extensions/vinci-search.ts | 22 ++++++- 2 files changed, 97 insertions(+), 4 deletions(-) diff --git a/vinci/extensions/vinci-search.test.mjs b/vinci/extensions/vinci-search.test.mjs index b6fa459bb..86412d5d7 100644 --- a/vinci/extensions/vinci-search.test.mjs +++ b/vinci/extensions/vinci-search.test.mjs @@ -44,7 +44,7 @@ function dependencies(responses, addresses = new Map()) { resolutions.push(hostname); const next = addresses.get(hostname); if (next instanceof Error) throw next; - return next ?? [{ address: publicAddress }]; + return addresses.has(hostname) ? next : [{ address: publicAddress }]; }, }, }; @@ -220,6 +220,66 @@ test("a later-hop hostname resolving private is refused before that hop is fetch assert.deepEqual(deps.requests, [{ url: "https://start.example/", redirect: "manual" }]); }); +test("a later-hop mixed public/private DNS answer is refused before that hop is fetched", async () => { + const addresses = new Map([ + ["start.example", [{ address: publicAddress }]], + ["mixed.example", [{ address: publicAddress }, { address: "10.0.0.8" }]], + ]); + const deps = dependencies([response(302, "https://mixed.example/secret")], addresses); + const result = await fetchPublicPage("https://start.example/", signal, deps.value); + assert.equal(result.ok, false); + assert.match(result.reason, /resolves to an internal server/); + assert.deepEqual(deps.resolutions, ["start.example", "mixed.example"]); + assert.deepEqual(deps.requests, [{ url: "https://start.example/", redirect: "manual" }]); +}); + +test("missing, empty, and wrong-shaped DNS evidence is refused before fetch", async (t) => { + const invalidAnswers = [ + ["missing", undefined, /Couldn't resolve/], + ["null", null, /Couldn't resolve/], + ["empty array", [], /Couldn't resolve/], + ["object instead of array", { address: publicAddress }, /Couldn't resolve/], + ["string instead of array", publicAddress, /Couldn't resolve/], + ["null row", [null], /Couldn't resolve/], + ["array row", [[{ address: publicAddress }]], /Couldn't resolve/], + ["missing address", [{}], /Couldn't resolve/], + ["null address", [{ address: null }], /Couldn't resolve/], + ["empty address", [{ address: "" }], /Couldn't resolve/], + ["wrong-type address", [{ address: 7 }], /Couldn't resolve/], + ["malformed IPv4 address", [{ address: "999.1.1.1" }], /invalid DNS address/], + ["malformed IPv6 address", [{ address: "2001:::1" }], /invalid DNS address/], + ]; + for (const [name, answer, expected] of invalidAnswers) { + await t.test(name, async () => { + const deps = dependencies([], new Map([["invalid-dns.example", answer]])); + const result = await fetchPublicPage("https://invalid-dns.example/", signal, deps.value); + assert.equal(result.ok, false); + assert.match(result.reason, expected); + assert.deepEqual(deps.resolutions, ["invalid-dns.example"]); + assert.equal(deps.requests.length, 0, "invalid resolver evidence must be rejected before fetch"); + }); + } +}); + +test("a nonempty all-public DNS answer is accepted", async () => { + const addresses = new Map([ + [ + "public.example", + [ + { address: publicAddress }, + { address: "2606:2800:220:1:248:1893:25c8:1946" }, + { address: "::ffff:5db8:d822" }, + { address: "64:ff9b::5db8:d822" }, + ], + ], + ]); + const deps = dependencies([response(200)], addresses); + const result = await fetchPublicPage("https://public.example/", signal, deps.value); + assert.equal(result.ok, true); + assert.deepEqual(deps.resolutions, ["public.example"]); + assert.deepEqual(deps.requests, [{ url: "https://public.example/", redirect: "manual" }]); +}); + test("all redirect Location status and shape failures are typed and mechanism-reaching", async (t) => { const badLocations = [ ["missing", undefined, /without a valid Location/], @@ -272,6 +332,23 @@ test("redirect loops and redirects beyond the explicit cap are refused", async ( assert.equal(excessDeps.requests.length, WEB_FETCH_MAX_REDIRECTS + 1); }); +test("a non-initial start-to-a-to-b-to-a redirect cycle is refused", async () => { + const deps = dependencies([ + response(302, "https://a.example/one"), + response(302, "https://b.example/two"), + response(302, "https://a.example/one"), + ]); + const result = await fetchPublicPage("https://start.example/", signal, deps.value); + assert.equal(result.ok, false); + assert.match(result.reason, /redirect loop/); + assert.deepEqual(deps.resolutions, ["start.example", "a.example", "b.example"]); + assert.deepEqual(deps.requests, [ + { url: "https://start.example/", redirect: "manual" }, + { url: "https://a.example/one", redirect: "manual" }, + { url: "https://b.example/two", redirect: "manual" }, + ]); +}); + test("the redirect cap still permits a public final response at its boundary", async () => { const responses = Array.from({ length: WEB_FETCH_MAX_REDIRECTS }, (_, index) => response(302, `/hop-${index + 1}`)); responses.push(response(200)); diff --git a/vinci/extensions/vinci-search.ts b/vinci/extensions/vinci-search.ts index 569e5e309..246ebcaff 100644 --- a/vinci/extensions/vinci-search.ts +++ b/vinci/extensions/vinci-search.ts @@ -23,6 +23,7 @@ * Additive: no core edit. */ import { lookup } from "node:dns/promises"; +import { isIP } from "node:net"; import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; import { Text } from "@earendil-works/pi-tui"; import { Type } from "typebox"; @@ -209,10 +210,9 @@ const FETCH_MAX_CHARS = 30000; // cap the extracted text — a docs page can be const FETCH_MAX_BYTES = 5_000_000; // refuse to download more than ~5MB of HTML export const WEB_FETCH_MAX_REDIRECTS = 5; -type LookupAddress = { address: string }; type WebFetchDependencies = { fetch: typeof globalThis.fetch; - lookup: (hostname: string, options: { all: true }) => Promise; + lookup: (hostname: string, options: { all: true }) => Promise; }; export type PublicPageFetchResult = @@ -307,7 +307,23 @@ async function resolvePublicHost(url: URL, lookupHost: WebFetchDependencies["loo if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host) || host.includes(":")) return undefined; try { const addrs = await lookupHost(host, { all: true }); - if (addrs.some((address) => isPrivateIp(address.address))) { + if (!Array.isArray(addrs) || addrs.length === 0) return "Couldn't resolve that web address."; + if ( + !addrs.every( + (entry): entry is { address: string } => + typeof entry === "object" && + entry !== null && + !Array.isArray(entry) && + typeof (entry as { address?: unknown }).address === "string" && + (entry as { address: string }).address.length > 0, + ) + ) { + return "Couldn't resolve that web address."; + } + if (addrs.some(({ address }) => isIP(address) === 0)) { + return "That web address returned an invalid DNS address — not reading it."; + } + if (addrs.some(({ address }) => isPrivateIp(address))) { return "That address resolves to an internal server — not reading it."; } } catch {