From 48f88e3a2adbe173cd714efdaef52b8a08c19295 Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Wed, 29 Jul 2026 03:40:41 -0400 Subject: [PATCH] fix(http): apply SSRF guard to os.http.request Os.web.fetch already blocked private/internal addresses and pinned DNS; os.http.request did not, so model-controlled curl could hit metadata/LAN while web.fetch of the same URL failed. Share the guard, pin --resolve, and re-validate each redirect hop. Closes #45 Signed-off-by: Bartok9 --- src/tools/os/http-request-fetch.ts | 256 +++++++++++++++++++++++++++ src/tools/os/http-request.test.ts | 143 ++++++++++++++- src/tools/os/http-request.ts | 216 ++++++++++------------ src/tools/os/web-fetch-ssrf-guard.ts | 8 + 4 files changed, 493 insertions(+), 130 deletions(-) create mode 100644 src/tools/os/http-request-fetch.ts diff --git a/src/tools/os/http-request-fetch.ts b/src/tools/os/http-request-fetch.ts new file mode 100644 index 0000000..0828c0b --- /dev/null +++ b/src/tools/os/http-request-fetch.ts @@ -0,0 +1,256 @@ +import { + runCommand as defaultRunCommand, + type CommandResult, +} from "../../sandbox/command-runner.js"; +import { CurlUnavailableError, isCurlMissingError } from "./ensure-curl.js"; +import { + assertHostAllowed, + parseHttpUrl, + SsrfBlockedError, + type HostLookup, +} from "./web-fetch-ssrf-guard.js"; + +/** + * Marker appended to curl stdout via `-w` so the response body can be split + * from structured metadata without `curl -i` (which mixes redirect chains + * into the body). Deterministic for tests. + */ +export const CURL_META_MARKER = "__ATOMIC_CURL_META__"; + +const MAX_REDIRECTS = 5; +const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); + +export type HttpMethod = "GET" | "POST"; + +export interface HttpRequestArgs { + method: HttpMethod; + headers: Record; + body: string | undefined; + timeoutMs: number; + followRedirects: boolean; +} + +export interface GuardedCurlResponse { + finalUrl: string; + status: number; + contentType: string; + body: string; + sizeDownload: number; + timeTotal: number; + truncated: boolean; + redirectChain: string[]; + /** Last curl argv (for diagnostics). */ + command: string[]; +} + +export interface ExecuteGuardedHttpOptions { + runCommand?: typeof defaultRunCommand; + lookup?: HostLookup; + cwd: string; + signal: AbortSignal; + maxResponseBytes: number; +} + +/** + * Fetch `rawUrl` under the same SSRF rules as `os.web.fetch`: + * resolve every hop, reject private/internal addresses, pin curl with + * `--resolve`, never use bare `-L` (which would skip hop re-checks). + */ +export async function executeGuardedHttpRequest( + rawUrl: string, + args: HttpRequestArgs, + opts: ExecuteGuardedHttpOptions, +): Promise { + const runCommand = opts.runCommand ?? defaultRunCommand; + let currentUrl = parseHttpUrl(rawUrl); + let method = args.method; + let body = args.body; + const chain: string[] = []; + let lastCommand: string[] = []; + let truncated = false; + let totalTime = 0; + + for (let hop = 0; ; hop += 1) { + const pinnedIp = await assertHostAllowed(currentUrl, { + lookup: opts.lookup, + }); + const curlArgs = buildPinnedCurlArgs({ + url: currentUrl, + pinnedIp, + method, + headers: args.headers, + body, + timeoutMs: args.timeoutMs, + }); + lastCommand = ["curl", ...curlArgs]; + + let result: CommandResult; + try { + result = await runCommand("curl", curlArgs, { + cwd: opts.cwd, + timeoutMs: args.timeoutMs + 2_000, + signal: opts.signal, + maxOutputBytes: opts.maxResponseBytes + 1024, + input: body, + }); + } catch (err) { + if (isCurlMissingError(err)) throw new CurlUnavailableError(); + throw err; + } + + if (result.exitCode !== 0) { + const err = new Error(formatCurlError(result)); + (err as Error & { curlExit: true; command: string[] }).curlExit = true; + (err as Error & { command: string[] }).command = lastCommand; + (err as Error & { exitCode: number | null }).exitCode = + result.exitCode; + (err as Error & { stderr: string }).stderr = result.stderr.trim(); + throw err; + } + + const parsed = parseCurlOutput(result.stdout); + truncated = truncated || result.truncated; + totalTime += parsed.timeTotal; + chain.push(currentUrl.toString()); + + const shouldFollow = + args.followRedirects && + REDIRECT_STATUSES.has(parsed.status) && + parsed.redirectUrl.length > 0; + + if (shouldFollow) { + if (hop >= MAX_REDIRECTS) { + throw new Error( + `os.http.request: too many redirects (> ${MAX_REDIRECTS})`, + ); + } + // Curl -L semantics: 301/302/303 drop to GET without body; 307/308 keep method+body. + if (parsed.status === 301 || parsed.status === 302 || parsed.status === 303) { + method = "GET"; + body = undefined; + } + currentUrl = parseHttpUrl(parsed.redirectUrl); + continue; + } + + return { + finalUrl: currentUrl.toString(), + status: parsed.status, + contentType: parsed.contentType, + body: parsed.body, + sizeDownload: parsed.sizeDownload, + timeTotal: totalTime, + truncated, + redirectChain: chain, + command: lastCommand, + }; + } +} + +export function isCurlTransportError( + err: unknown, +): err is Error & { + curlExit: true; + command: string[]; + exitCode: number | null; + stderr: string; +} { + return ( + err instanceof Error && + (err as Error & { curlExit?: boolean }).curlExit === true + ); +} + +export { SsrfBlockedError }; + +interface BuildPinnedCurlArgs { + url: URL; + pinnedIp: string; + method: HttpMethod; + headers: Record; + body: string | undefined; + timeoutMs: number; +} + +function buildPinnedCurlArgs(input: BuildPinnedCurlArgs): string[] { + const host = input.url.hostname.replace(/^\[|\]$/g, ""); + const port = + input.url.port || (input.url.protocol === "https:" ? "443" : "80"); + const resolveTarget = input.pinnedIp.includes(":") + ? `[${input.pinnedIp}]` + : input.pinnedIp; + const argv: string[] = [ + "-sS", + "--max-time", + String(Math.ceil(input.timeoutMs / 1000)), + // Hop-by-hop follow is owned by executeGuardedHttpRequest so each + // Location can be re-validated. Never bare `-L` here. + "--max-redirs", + "0", + "--resolve", + `${host}:${port}:${resolveTarget}`, + ]; + if (input.method !== "GET") argv.push("-X", input.method); + for (const [key, value] of Object.entries(input.headers)) { + argv.push("-H", `${key}: ${value}`); + } + if (input.body !== undefined) { + argv.push("--data-binary", "@-"); + } + argv.push( + "-w", + `\n${CURL_META_MARKER}%{http_code}|%{content_type}|%{size_download}|%{time_total}|%{redirect_url}`, + ); + argv.push("--", input.url.toString()); + return argv; +} + +export interface CurlParsedOutput { + body: string; + status: number; + contentType: string; + sizeDownload: number; + timeTotal: number; + redirectUrl: string; +} + +export function parseCurlOutput(stdout: string): CurlParsedOutput { + const markerIdx = stdout.lastIndexOf(CURL_META_MARKER); + if (markerIdx === -1) { + return { + body: stdout, + status: 0, + contentType: "", + sizeDownload: stdout.length, + timeTotal: 0, + redirectUrl: "", + }; + } + const body = stdout.slice(0, markerIdx).replace(/\n$/, ""); + const meta = stdout.slice(markerIdx + CURL_META_MARKER.length).trim(); + const [ + statusStr = "", + contentType = "", + sizeStr = "", + timeStr = "", + redirectUrl = "", + ] = meta.split("|"); + const status = Number.parseInt(statusStr, 10); + const sizeDownload = Number.parseInt(sizeStr, 10); + const timeTotal = Number.parseFloat(timeStr); + return { + body, + status: Number.isFinite(status) ? status : 0, + contentType: contentType.trim(), + sizeDownload: Number.isFinite(sizeDownload) ? sizeDownload : body.length, + timeTotal: Number.isFinite(timeTotal) ? timeTotal : 0, + redirectUrl: redirectUrl.trim(), + }; +} + +function formatCurlError(result: CommandResult): string { + const stderr = result.stderr.trim(); + if (stderr.length > 0) return stderr; + if (result.timedOut) return "curl timed out"; + return `curl exited with code ${result.exitCode}`; +} diff --git a/src/tools/os/http-request.test.ts b/src/tools/os/http-request.test.ts index fdd87c6..0648e2d 100644 --- a/src/tools/os/http-request.test.ts +++ b/src/tools/os/http-request.test.ts @@ -11,6 +11,7 @@ import { hostAllowed, parseCurlOutput, } from "./http-request.js"; +import type { HostLookup } from "./web-fetch-ssrf-guard.js"; function makeCtx(): ToolContext { return { @@ -83,6 +84,25 @@ function fakeRun( }; } +/** Hermetic lookup: public unicast address for any hostname. */ +const publicLookup: HostLookup = async () => [ + { address: "93.184.216.34", family: 4 }, +]; + +function privateLookup(ip: string): HostLookup { + return async () => [{ address: ip, family: 4 }]; +} + +function meta( + status: number, + contentType: string, + size: number, + time = 0.01, + redirectUrl = "", +): string { + return `__ATOMIC_CURL_META__${status}|${contentType}|${size}|${time}|${redirectUrl}`; +} + describe("hostAllowed", () => { it("returns true when allowlist is null", () => { expect(hostAllowed("api.github.com", null)).toBe(true); @@ -105,13 +125,14 @@ describe("hostAllowed", () => { describe("parseCurlOutput", () => { it("splits body from meta marker", () => { const stdout = - 'hello world\n__ATOMIC_CURL_META__200|application/json; charset=utf-8|11|0.123'; + "hello world\n__ATOMIC_CURL_META__200|application/json; charset=utf-8|11|0.123|https://next.example/"; const parsed = parseCurlOutput(stdout); expect(parsed.body).toBe("hello world"); expect(parsed.status).toBe(200); expect(parsed.contentType).toBe("application/json; charset=utf-8"); expect(parsed.sizeDownload).toBe(11); expect(parsed.timeTotal).toBeCloseTo(0.123); + expect(parsed.redirectUrl).toBe("https://next.example/"); }); it("falls back to raw output if marker is missing", () => { @@ -124,6 +145,7 @@ describe("parseCurlOutput", () => { describe("os.http.request", () => { it("rejects when config.http.enabled is false", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ enabled: false }), @@ -136,6 +158,7 @@ describe("os.http.request", () => { it("rejects non-http(s) URLs", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig(), @@ -148,6 +171,7 @@ describe("os.http.request", () => { it("rejects hostnames not on the allowlist", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ hostAllowlist: ["api.github.com"] }), @@ -161,6 +185,7 @@ describe("os.http.request", () => { it("performs GET without approval when approvalMode=writes", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: denyAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "writes" }), @@ -174,13 +199,14 @@ describe("os.http.request", () => { ); expect(result.status).toBe("ok"); expect(result.details.status).toBe(200); - expect(capture.args).toContain("https://example.com"); + expect(capture.args!.some((a) => a.includes("example.com"))).toBe(true); expect(capture.args).not.toContain("-X"); }); it("returns status:error for a 404 while keeping the body in details", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: false, config: makeHttpConfig({ approvalMode: "never" }), @@ -198,6 +224,7 @@ describe("os.http.request", () => { it("returns status:error for a 500", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: false, config: makeHttpConfig({ approvalMode: "never" }), @@ -213,6 +240,7 @@ describe("os.http.request", () => { it("requires approval for POST when approvalMode=writes", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: denyAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "writes" }), @@ -228,6 +256,7 @@ describe("os.http.request", () => { it("requires approval for GET when approvalMode=always", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: denyAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "always" }), @@ -241,6 +270,7 @@ describe("os.http.request", () => { it("bypasses approval entirely when approvalMode=never", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: denyAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -259,6 +289,7 @@ describe("os.http.request", () => { it("serialises object body to JSON and auto-sets Content-Type", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -290,6 +321,7 @@ describe("os.http.request", () => { it("passes custom headers through to curl and does not override them", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -320,6 +352,7 @@ describe("os.http.request", () => { it("injects a default Accept header when none is provided", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -334,6 +367,7 @@ describe("os.http.request", () => { it("does not override an explicit Accept header", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -357,6 +391,7 @@ describe("os.http.request", () => { it("rejects headers containing CR/LF to prevent injection", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -375,6 +410,7 @@ describe("os.http.request", () => { it("rejects body on GET requests", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -390,6 +426,7 @@ describe("os.http.request", () => { it("returns structured error when curl fails", async () => { const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -416,6 +453,7 @@ describe("os.http.request", () => { "", ].join("\n"); const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -440,6 +478,7 @@ describe("os.http.request", () => { cursor: "abc", }); const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -461,6 +500,7 @@ describe("os.http.request", () => { it("returns an HTML-looking body unchanged when content-type is missing", async () => { const html = "

Sniffed

"; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -479,6 +519,7 @@ describe("os.http.request", () => { it("returns plain-text responses verbatim", async () => { const body = "if (a < b) return true; else return false;"; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -500,6 +541,7 @@ describe("os.http.request", () => { ); expect(big.length).toBeGreaterThan(2000); const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ approvalMode: "never" }), @@ -519,11 +561,12 @@ describe("os.http.request", () => { it("uses config.http.defaultTimeoutMs when timeoutMs is not provided", async () => { const capture: { cmd?: string; args?: string[]; opts?: CommandOptions } = {}; const tool = buildOsHttpRequestTool({ + lookup: publicLookup, approvals: approveAll(), approvalRequired: true, config: makeHttpConfig({ defaultTimeoutMs: 7_000, approvalMode: "never" }), runCommand: fakeRun(capture, { - stdout: 'ok\n__ATOMIC_CURL_META__200|text/plain|2|0.01', + stdout: "ok\n" + meta(200, "text/plain", 2), }), }); await tool.run({ url: "https://example.com" }, makeCtx()); @@ -531,4 +574,98 @@ describe("os.http.request", () => { expect(maxTimeIdx).toBeGreaterThan(-1); expect(capture.args![maxTimeIdx + 1]).toBe("7"); }); + + it("blocks private IP literals (SSRF parity with os.web.fetch)", async () => { + const capture: { cmd?: string; args?: string[] } = {}; + const tool = buildOsHttpRequestTool({ + // Hostname itself is the blocked IP — lookup must surface that address + // (real DNS does this for literal IPs; do not substitute a public pin). + lookup: async () => [{ address: "169.254.169.254", family: 4 }], + approvals: approveAll(), + approvalRequired: false, + config: makeHttpConfig({ approvalMode: "never" }), + runCommand: fakeRun(capture, { + stdout: "ok\n" + meta(200, "text/plain", 2), + }), + }); + const result = await tool.run( + { url: "http://169.254.169.254/latest/meta-data/" }, + makeCtx(), + ); + expect(result.status).toBe("error"); + expect(result.details.blocked).toBe(true); + expect(result.summary).toMatch(/private|internal|169\.254/i); + expect(capture.args).toBeUndefined(); + }); + + it("blocks hosts that DNS-resolve to a private address", async () => { + const capture: { cmd?: string; args?: string[] } = {}; + const tool = buildOsHttpRequestTool({ + lookup: privateLookup("10.0.0.5"), + approvals: approveAll(), + approvalRequired: false, + config: makeHttpConfig({ approvalMode: "never" }), + runCommand: fakeRun(capture), + }); + const result = await tool.run({ url: "https://evil.internal" }, makeCtx()); + expect(result.status).toBe("error"); + expect(result.details.blocked).toBe(true); + expect(result.summary).toContain("10.0.0.5"); + expect(capture.args).toBeUndefined(); + }); + + it("pins curl with --resolve and does not use bare -L", async () => { + const capture: { cmd?: string; args?: string[] } = {}; + const tool = buildOsHttpRequestTool({ + lookup: publicLookup, + approvals: approveAll(), + approvalRequired: false, + config: makeHttpConfig({ approvalMode: "never" }), + runCommand: fakeRun(capture, { + stdout: "ok\n" + meta(200, "text/plain", 2), + }), + }); + await tool.run({ url: "https://example.com/x" }, makeCtx()); + const resolveIdx = capture.args!.indexOf("--resolve"); + expect(resolveIdx).toBeGreaterThan(-1); + expect(capture.args![resolveIdx + 1]).toBe("example.com:443:93.184.216.34"); + expect(capture.args).toContain("--max-redirs"); + expect(capture.args).not.toContain("-L"); + }); + + it("re-validates redirect hops and blocks a private Location target", async () => { + const calls: string[][] = []; + const runCommand = async ( + _cmd: string, + args: string[], + _opts: CommandOptions, + ): Promise => { + calls.push(args); + if (calls.length === 1) { + return makeCommandResult({ + stdout: + "\n" + + meta(302, "text/plain", 0, 0.01, "http://169.254.169.254/meta"), + }); + } + return makeCommandResult({ + stdout: "leaked\n" + meta(200, "text/plain", 6), + }); + }; + // First hop is public; second hop would be private IP literal. + const tool = buildOsHttpRequestTool({ + lookup: publicLookup, + approvals: approveAll(), + approvalRequired: false, + config: makeHttpConfig({ approvalMode: "never" }), + runCommand, + }); + const result = await tool.run( + { url: "https://example.com/start", followRedirects: true }, + makeCtx(), + ); + expect(result.status).toBe("error"); + expect(result.details.blocked).toBe(true); + expect(calls).toHaveLength(1); + }); }); diff --git a/src/tools/os/http-request.ts b/src/tools/os/http-request.ts index 42f3309..40222d7 100644 --- a/src/tools/os/http-request.ts +++ b/src/tools/os/http-request.ts @@ -1,6 +1,7 @@ import { compressToolResult } from "../../compressor/result-compressor.js"; import { runCommand as defaultRunCommand, + type CommandOptions, type CommandResult, } from "../../sandbox/command-runner.js"; import { @@ -12,21 +13,29 @@ import type { HttpApprovalMode, } from "../../config/index.js"; import type { ToolDefinition } from "../tool-registry.js"; -import { CurlUnavailableError, isCurlMissingError } from "./ensure-curl.js"; +import { CurlUnavailableError } from "./ensure-curl.js"; +import { + executeGuardedHttpRequest, + isCurlTransportError, + type HttpMethod, + SsrfBlockedError, +} from "./http-request-fetch.js"; +import type { HostLookup } from "./web-fetch-ssrf-guard.js"; -/** - * Marker we append to curl stdout via `-w` so we can split the response - * body from the structured metadata without relying on `curl -i` (which - * mixes redirect chains into the body). The token is random-looking but - * deterministic so tests can assert on it. - */ -const CURL_META_MARKER = "__ATOMIC_CURL_META__"; +export type { HttpMethod } from "./http-request-fetch.js"; +export { parseCurlOutput } from "./http-request-fetch.js"; -export type HttpMethod = "GET" | "POST"; +type RunCommandFn = ( + cmd: string, + args: string[], + opts: CommandOptions, +) => Promise; export interface OsHttpRequestOptions extends DangerousToolOptions { config: Pick; - runCommand?: typeof defaultRunCommand; + runCommand?: RunCommandFn; + /** Injectable DNS lookup for SSRF tests (mirrors os.web.fetch). */ + lookup?: HostLookup; } interface HttpArgs { @@ -46,7 +55,7 @@ export function buildOsHttpRequestTool( return { name: "os.http.request", description: - "Raw HTTP GET or POST via the system `curl` binary for APIs and machine-readable endpoints (JSON, XML, plain text). Returns the response body verbatim — no HTML extraction or cleanup. To read a human web page as markdown/text, use `os.web.fetch` instead. Host allowlist and approval policy come from `config.http`. Body is capped at `config.http.maxResponseBytes`.", + "Raw HTTP GET or POST via the system `curl` binary for APIs and machine-readable endpoints (JSON, XML, plain text). Returns the response body verbatim — no HTML extraction or cleanup. To read a human web page as markdown/text, use `os.web.fetch` instead. Blocks private/internal addresses (SSRF) like `os.web.fetch`, pins DNS with curl `--resolve`, and re-validates each redirect hop. Host allowlist and approval policy come from `config.http`. Body is capped at `config.http.maxResponseBytes`.", readonly: false, async run(rawArgs, ctx) { const httpCfg = options.config.http; @@ -75,75 +84,84 @@ export function buildOsHttpRequestTool( ); } - const curlArgs = buildCurlArgs(args); - let commandResult: CommandResult; + let guarded; try { - commandResult = await runCommand("curl", curlArgs, { - cwd: ctx.workingDir, - timeoutMs: args.timeoutMs + 2_000, - signal: ctx.signal, - maxOutputBytes: httpCfg.maxResponseBytes + 1024, - input: args.body, - }); + guarded = await executeGuardedHttpRequest( + args.url, + { + method: args.method, + headers: args.headers, + body: args.body, + timeoutMs: args.timeoutMs, + followRedirects: args.followRedirects, + }, + { + runCommand, + lookup: options.lookup, + cwd: ctx.workingDir, + signal: ctx.signal, + maxResponseBytes: httpCfg.maxResponseBytes, + }, + ); } catch (err) { - if (isCurlMissingError(err)) { + if (err instanceof CurlUnavailableError) { return compressToolResult({ tool: "os.http.request", status: "error", - output: new CurlUnavailableError().message, + output: err.message, details: { url: args.url, method: args.method }, }); } + if (err instanceof SsrfBlockedError) { + return compressToolResult({ + tool: "os.http.request", + status: "error", + output: err.message, + details: { + url: args.url, + method: args.method, + blocked: true, + host: err.host, + }, + }); + } + if (isCurlTransportError(err)) { + return compressToolResult({ + tool: "os.http.request", + status: "error", + output: err.message, + details: { + exitCode: err.exitCode, + stderr: err.stderr, + url: args.url, + method: args.method, + command: err.command, + }, + }); + } throw err; } - if (commandResult.exitCode !== 0) { - return compressToolResult({ - tool: "os.http.request", - status: "error", - output: formatCurlError(commandResult), - details: { - exitCode: commandResult.exitCode, - stderr: commandResult.stderr.trim(), - url: args.url, - method: args.method, - command: ["curl", ...curlArgs], - }, - }); - } - - const parsed = parseCurlOutput(commandResult.stdout); - const truncated = commandResult.truncated; - // An HTTP status >= 400 is a real failure signal. Returning - // `status:"ok"` here masked erroring endpoints from the model and - // from the loop detector's semantic result hash, letting the agent - // hammer the same dead endpoint indefinitely. Surface it as an - // error while keeping the response body in `details.body` so the - // model can still inspect any error payload. - const isHttpError = parsed.status >= 400; - // Return the body verbatim — this tool is the raw-HTTP surface. HTML - // extraction (markdown/text) is the job of `os.web.fetch`. Lift the - // compressor caps so small JSON payloads that would otherwise be - // cropped to 400 chars / 12 lines survive intact for the LLM. The - // body is already bounded by curl via `maxResponseBytes`; the - // downstream rendering layer applies its own per-turn cap. + const isHttpError = guarded.status >= 400; return compressToolResult( { tool: "os.http.request", status: isHttpError ? "error" : "ok", output: isHttpError - ? `HTTP ${parsed.status} ${args.method} ${args.url}` - : parsed.body, + ? `HTTP ${guarded.status} ${args.method} ${guarded.finalUrl}` + : guarded.body, details: { url: args.url, + finalUrl: guarded.finalUrl, method: args.method, - status: parsed.status, - contentType: parsed.contentType, - sizeDownload: parsed.sizeDownload, - timeTotalSeconds: parsed.timeTotal, - truncated, - command: ["curl", ...curlArgs], - ...(isHttpError ? { body: parsed.body } : {}), + status: guarded.status, + contentType: guarded.contentType, + sizeDownload: guarded.sizeDownload, + timeTotalSeconds: guarded.timeTotal, + truncated: guarded.truncated, + redirectChain: guarded.redirectChain, + command: guarded.command, + ...(isHttpError ? { body: guarded.body } : {}), }, }, { @@ -201,8 +219,12 @@ function parseArgs( `os.http.request: header ${JSON.stringify(key)} must be a string`, ); } - if (key.includes("\n") || key.includes("\r") || value.includes("\n") || value.includes("\r")) { - // Block CRLF-injection into curl header args. + if ( + key.includes("\n") || + key.includes("\r") || + value.includes("\n") || + value.includes("\r") + ) { throw new Error( `os.http.request: header ${JSON.stringify(key)} contains CR/LF which is not allowed`, ); @@ -306,72 +328,12 @@ function buildApprovalPreview(args: HttpArgs): string { lines.push(`${k}: ${v}`); } if (args.body !== undefined) { - const snippet = args.body.length > 400 - ? `${args.body.slice(0, 400)}… [${args.body.length} bytes]` - : args.body; + const snippet = + args.body.length > 400 + ? `${args.body.slice(0, 400)}… [${args.body.length} bytes]` + : args.body; lines.push(""); lines.push(snippet); } return lines.join("\n"); } - -function buildCurlArgs(args: HttpArgs): string[] { - const argv: string[] = ["-sS", "--max-time", String(Math.ceil(args.timeoutMs / 1000))]; - if (args.followRedirects) argv.push("-L"); - if (args.method !== "GET") argv.push("-X", args.method); - for (const [key, value] of Object.entries(args.headers)) { - argv.push("-H", `${key}: ${value}`); - } - if (args.body !== undefined) { - // Use --data-binary to avoid curl munging newlines. - argv.push("--data-binary", "@-"); - } - argv.push( - "-w", - `\n${CURL_META_MARKER}%{http_code}|%{content_type}|%{size_download}|%{time_total}`, - ); - argv.push("--", args.url); - return argv; -} - -interface CurlParsedOutput { - body: string; - status: number; - contentType: string; - sizeDownload: number; - timeTotal: number; -} - -export function parseCurlOutput(stdout: string): CurlParsedOutput { - const markerIdx = stdout.lastIndexOf(CURL_META_MARKER); - if (markerIdx === -1) { - return { - body: stdout, - status: 0, - contentType: "", - sizeDownload: stdout.length, - timeTotal: 0, - }; - } - const body = stdout.slice(0, markerIdx).replace(/\n$/, ""); - const meta = stdout.slice(markerIdx + CURL_META_MARKER.length).trim(); - const [statusStr = "", contentType = "", sizeStr = "", timeStr = ""] = - meta.split("|"); - const status = Number.parseInt(statusStr, 10); - const sizeDownload = Number.parseInt(sizeStr, 10); - const timeTotal = Number.parseFloat(timeStr); - return { - body, - status: Number.isFinite(status) ? status : 0, - contentType: contentType.trim(), - sizeDownload: Number.isFinite(sizeDownload) ? sizeDownload : body.length, - timeTotal: Number.isFinite(timeTotal) ? timeTotal : 0, - }; -} - -function formatCurlError(result: CommandResult): string { - const stderr = result.stderr.trim(); - if (stderr.length > 0) return stderr; - if (result.timedOut) return "curl timed out"; - return `curl exited with code ${result.exitCode}`; -} diff --git a/src/tools/os/web-fetch-ssrf-guard.ts b/src/tools/os/web-fetch-ssrf-guard.ts index 2edc7ac..2f503f1 100644 --- a/src/tools/os/web-fetch-ssrf-guard.ts +++ b/src/tools/os/web-fetch-ssrf-guard.ts @@ -179,6 +179,14 @@ export async function assertHostAllowed( if (host.length === 0) { throw new SsrfBlockedError("empty host", host); } + // IP literals do not need DNS — reject them directly so a custom/test + // lookup cannot whitewash a private target by returning a public pin. + if (isBlockedIp(host)) { + throw new SsrfBlockedError( + `${host} is a private/internal address`, + host, + ); + } let addresses: readonly { address: string; family: number }[]; try { addresses = await lookup(host);