diff --git a/src/main/skills/url-validation.ts b/src/main/skills/url-validation.ts index 0af74848..352683f5 100644 --- a/src/main/skills/url-validation.ts +++ b/src/main/skills/url-validation.ts @@ -1,4 +1,12 @@ import { resolve4, resolve6 } from "node:dns/promises"; +import { request, type RequestOptions } from "node:https"; +import type { IncomingHttpHeaders, IncomingMessage } from "node:http"; +import type { LookupFunction } from "node:net"; + +interface ResolvedIpRecord { + address: string; + family: 4 | 6; +} function isPrivateIPv4(ip: string): boolean { const parts = ip.split(".").map(Number); @@ -62,6 +70,12 @@ export function validateSkillUrl(url: string): void { * Resolve DNS and validate that all IPs are public. Returns the resolved IPv4 addresses. */ export async function resolveAndValidateIps(url: string): Promise { + return (await resolveAndValidateIpRecords(url)) + .filter((record) => record.family === 4) + .map((record) => record.address); +} + +async function resolveAndValidateIpRecords(url: string): Promise { validateSkillUrl(url); const { hostname } = new URL(url); const v4: string[] = []; @@ -69,25 +83,83 @@ export async function resolveAndValidateIps(url: string): Promise { try { v4.push(...(await resolve4(hostname))); } catch { /* no A records */ } try { v6.push(...(await resolve6(hostname))); } catch { /* no AAAA records */ } assertPublicIps([...v4, ...v6]); - return v4; + return [ + ...v4.map((address): ResolvedIpRecord => ({ address, family: 4 })), + ...v6.map((address): ResolvedIpRecord => ({ address, family: 6 })), + ]; +} + +function buildHeaders(headers: IncomingHttpHeaders): Headers { + const out = new Headers(); + for (const [key, value] of Object.entries(headers)) { + if (Array.isArray(value)) { + for (const entry of value) out.append(key, entry); + } else if (value !== undefined) { + out.set(key, value); + } + } + return out; +} + +function readBody(response: IncomingMessage): Promise { + return new Promise((resolve, reject) => { + const chunks: Buffer[] = []; + response.on("data", (chunk: Buffer | string) => { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + }); + response.on("end", () => resolve(Buffer.concat(chunks))); + response.on("error", reject); + }); +} + +export function buildPinnedHttpsRequestOptions( + parsed: URL, + ip: ResolvedIpRecord, +): RequestOptions { + const lookup: LookupFunction = (_hostname, _options, callback) => { + callback(null, ip.address, ip.family); + }; + return { + method: "GET", + headers: { Host: parsed.host }, + servername: parsed.hostname, + lookup, + }; +} + +async function fetchHttpsWithResolvedIp(url: string, ip: ResolvedIpRecord): Promise { + const parsed = new URL(url); + return new Promise((resolve, reject) => { + const req = request(parsed, buildPinnedHttpsRequestOptions(parsed, ip), async (res) => { + try { + const body = await readBody(res); + const arrayBuffer = new ArrayBuffer(body.byteLength); + new Uint8Array(arrayBuffer).set(body); + resolve(new Response(arrayBuffer, { + status: res.statusCode ?? 500, + statusText: res.statusMessage, + headers: buildHeaders(res.headers), + })); + } catch (error) { + reject(error); + } + }); + req.on("error", reject); + req.end(); + }); } /** - * Fetch a URL with SSRF protection: resolve DNS once, validate IPs, then fetch - * using a resolved IP to prevent DNS rebinding between validation and fetch. + * Fetch a URL with SSRF protection: resolve DNS once, validate IPs, then keep + * the original hostname for Host/SNI while pinning the socket lookup to a + * validated IP. */ export async function fetchWithSsrfProtection(url: string): Promise { - const resolvedIps = await resolveAndValidateIps(url); + const resolvedIps = await resolveAndValidateIpRecords(url); if (resolvedIps.length === 0) { - throw new Error("DNS resolution returned no usable IPv4 addresses"); + throw new Error("DNS resolution returned no usable addresses"); } - const parsed = new URL(url); - const resolvedUrl = new URL(url); - resolvedUrl.hostname = resolvedIps[0]!; - const response = await fetch(resolvedUrl.toString(), { - headers: { Host: parsed.host }, - redirect: "manual", - }); + const response = await fetchHttpsWithResolvedIp(url, resolvedIps[0]!); // Follow redirects manually to prevent redirect-to-private-IP attacks if (response.status >= 300 && response.status < 400) { const location = response.headers.get("location"); diff --git a/tests/unit/skill-url-validation.test.ts b/tests/unit/skill-url-validation.test.ts new file mode 100644 index 00000000..02525202 --- /dev/null +++ b/tests/unit/skill-url-validation.test.ts @@ -0,0 +1,130 @@ +import { EventEmitter } from "node:events"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { resolve4, resolve6 } from "node:dns/promises"; +import { request } from "node:https"; +import { + buildPinnedHttpsRequestOptions, + fetchWithSsrfProtection, + validateSkillUrl, +} from "../../src/main/skills/url-validation"; + +vi.mock("node:dns/promises", () => ({ + resolve4: vi.fn(), + resolve6: vi.fn(), +})); + +vi.mock("node:https", () => ({ + request: vi.fn(), +})); + +type RequestCall = { + url: URL; + options: Parameters[1]; +}; + +const requestCalls: RequestCall[] = []; + +function mockHttpsResponse(input: { + statusCode: number; + statusMessage?: string; + headers?: Record; + body?: string; +}) { + const requestMock = vi.mocked(request); + requestMock.mockImplementationOnce((url, options, callback) => { + requestCalls.push({ url: url as URL, options: options as RequestCall["options"] }); + const req = new EventEmitter() as ReturnType; + req.end = vi.fn(() => { + const res = new EventEmitter() as EventEmitter & { + statusCode: number; + statusMessage?: string; + headers: Record; + }; + res.statusCode = input.statusCode; + res.statusMessage = input.statusMessage; + res.headers = input.headers ?? {}; + queueMicrotask(() => { + callback?.(res as never); + if (input.body) res.emit("data", Buffer.from(input.body)); + res.emit("end"); + }); + return req; + }) as never; + return req; + }); +} + +describe("skill URL validation", () => { + beforeEach(() => { + requestCalls.length = 0; + vi.mocked(resolve4).mockReset(); + vi.mocked(resolve6).mockReset(); + vi.mocked(request).mockReset(); + }); + + it("pins the socket lookup while preserving Host and SNI", () => { + const parsed = new URL("https://example.com/skills/SKILL.md"); + const options = buildPinnedHttpsRequestOptions(parsed, { address: "93.184.216.34", family: 4 }); + + expect(options.servername).toBe("example.com"); + expect(options.headers).toEqual({ Host: "example.com" }); + + let resolved: { address: string; family: number } | null = null; + if (typeof options.lookup !== "function") throw new Error("Expected lookup function."); + options.lookup("example.com", {}, (error, address, family) => { + if (error) throw error; + resolved = { address: String(address), family: Number(family) }; + }); + + expect(resolved).toEqual({ address: "93.184.216.34", family: 4 }); + }); + + it("rejects non-HTTPS and private skill import URLs", () => { + expect(() => validateSkillUrl("http://example.com/SKILL.md")).toThrow("Only HTTPS URLs are allowed"); + expect(() => validateSkillUrl("https://127.0.0.1/SKILL.md")).toThrow("Private/reserved IP addresses"); + expect(() => validateSkillUrl("https://localhost/SKILL.md")).toThrow("Private/reserved IP addresses"); + }); + + it("fetchWithSsrfProtection keeps the original hostname while pinning lookup to validated IP", async () => { + vi.mocked(resolve4).mockResolvedValue(["93.184.216.34"]); + vi.mocked(resolve6).mockResolvedValue([]); + mockHttpsResponse({ statusCode: 200, headers: { "content-type": "text/plain" }, body: "ok" }); + + const response = await fetchWithSsrfProtection("https://example.com/skills/SKILL.md"); + + expect(response.status).toBe(200); + await expect(response.text()).resolves.toBe("ok"); + expect(requestCalls).toHaveLength(1); + expect(requestCalls[0]?.url.hostname).toBe("example.com"); + expect(requestCalls[0]?.options).toMatchObject({ + headers: { Host: "example.com" }, + servername: "example.com", + }); + + let resolved: { address: string; family: number } | null = null; + const lookup = requestCalls[0]?.options?.lookup; + if (typeof lookup !== "function") throw new Error("Expected lookup function."); + lookup("example.com", {}, (error, address, family) => { + if (error) throw error; + resolved = { address: String(address), family: Number(family) }; + }); + expect(resolved).toEqual({ address: "93.184.216.34", family: 4 }); + }); + + it("revalidates redirects before following them", async () => { + vi.mocked(resolve4) + .mockResolvedValueOnce(["93.184.216.34"]) + .mockResolvedValueOnce(["127.0.0.1"]); + vi.mocked(resolve6).mockResolvedValue([]); + mockHttpsResponse({ + statusCode: 302, + headers: { location: "https://redirect.example/SKILL.md" }, + }); + + await expect(fetchWithSsrfProtection("https://example.com/skills/SKILL.md")) + .rejects + .toThrow("DNS resolved to private IP 127.0.0.1"); + expect(requestCalls).toHaveLength(1); + expect(requestCalls[0]?.url.hostname).toBe("example.com"); + }); +});