diff --git a/packages/bcode-browser/src/fetch-use.ts b/packages/bcode-browser/src/fetch-use.ts index 83562b982..73f648398 100644 --- a/packages/bcode-browser/src/fetch-use.ts +++ b/packages/bcode-browser/src/fetch-use.ts @@ -6,7 +6,7 @@ import { Context, Effect, Layer } from "effect" import { HttpClient, HttpClientRequest } from "effect/unstable/http" -const ENDPOINT = "https://fetch.browser-use.com/fetch" +const DEFAULT_ENDPOINT = "https://fetch.browser-use.com/fetch" export interface FetchResult { readonly body: ArrayBuffer @@ -32,11 +32,12 @@ export const layer = Layer.effect( Effect.gen(function* () { const http = yield* HttpClient.HttpClient const apiKey = process.env.BROWSER_USE_API_KEY ?? "" + const endpoint = resolveEndpoint() return Service.of({ enabled: apiKey.length > 0, fetch: (url, { timeoutMs }) => Effect.gen(function* () { - const request = yield* HttpClientRequest.post(ENDPOINT).pipe( + const request = yield* HttpClientRequest.post(endpoint).pipe( HttpClientRequest.setHeaders({ "Content-Type": "application/json", "X-Browser-Use-API-Key": apiKey }), HttpClientRequest.bodyJson({ url, timeout_ms: timeoutMs }), ) @@ -56,4 +57,42 @@ export const layer = Layer.effect( }), ) +// Overridable so a caller can mediate the request and keep the real key out of +// this process entirely. The default endpoint is a general-purpose URL fetcher, +// so anything holding the key can send it to an arbitrary host -- an untrusted +// or injectable agent should be given a mediating endpoint and a throwaway +// credential instead of the real one. +// +// Every rejection below is an operator mistake at startup, and each one would +// otherwise put X-Browser-Use-API-Key somewhere it should not go. Set-but-empty +// is a mistake rather than a default, because the default is the direct fetcher +// -- the exact path someone setting this variable is trying to leave. +function resolveEndpoint() { + const configured = process.env.BCODE_FETCH_USE_ENDPOINT + if (configured === undefined) return DEFAULT_ENDPOINT + if (configured.trim() === "") + throw new Error("BCODE_FETCH_USE_ENDPOINT is set but empty; unset it to use the default fetcher") + // The messages below name the variable and at most the destination's origin, + // never the value: it can carry userinfo or a token in its query, and writing + // that to stderr is the same leak this override exists to close. The operator + // can read back their own environment variable. + if (!URL.canParse(configured)) throw new Error("BCODE_FETCH_USE_ENDPOINT is not a valid url") + const url = new URL(configured) + // Checked before the loopback exemption below, which would otherwise wave + // through ftp://localhost and defer the failure to the first webfetch. + if (url.protocol !== "https:" && url.protocol !== "http:") + throw new Error(`BCODE_FETCH_USE_ENDPOINT must be http or https, not ${url.protocol}`) + if (url.protocol !== "https:" && !LOOPBACK.test(url.hostname)) + throw new Error( + `BCODE_FETCH_USE_ENDPOINT must use https outside loopback; refusing to send the api key in cleartext to ${url.origin}`, + ) + return configured +} + +// All of 127.0.0.0/8 is loopback rather than 127.0.0.1 alone, a trailing dot is +// the same name in its rooted form, and URL reports the IPv6 literal with its +// brackets, so "::1" would never match. Anchored and numeric so a DNS name like +// 127.example.com is not mistaken for the subnet. +const LOOPBACK = /^(localhost\.?|\[::1\]|127(\.\d{1,3}){3})$/ + export * as FetchUse from "./fetch-use" diff --git a/packages/bcode-browser/test/fetch-use.test.ts b/packages/bcode-browser/test/fetch-use.test.ts index 753118d57..3dc93c8ab 100644 --- a/packages/bcode-browser/test/fetch-use.test.ts +++ b/packages/bcode-browser/test/fetch-use.test.ts @@ -20,6 +20,111 @@ test("layer constructs and exposes `enabled` reflecting env", async () => { expect(enabled).toBe(haveKey) }) +test("BCODE_FETCH_USE_ENDPOINT redirects the request and forwards the target url", async () => { + // A real server, so this pins the wire behaviour a mediating proxy depends on: + // the override must be used AND the target url must arrive in the body, or the + // proxy has nothing to forward. + const seen: { url?: string; key?: string } = {} + const server = Bun.serve({ + port: 0, + fetch: async (req) => { + seen.url = ((await req.json()) as { url: string }).url + seen.key = req.headers.get("X-Browser-Use-API-Key") ?? undefined + return Response.json({ status_code: 200, body: "ok", headers: { "content-type": ["text/plain"] } }) + }, + }) + const realKey = process.env.BROWSER_USE_API_KEY + process.env.BCODE_FETCH_USE_ENDPOINT = `http://localhost:${server.port}/fetch` + process.env.BROWSER_USE_API_KEY = "sentinel-not-a-real-key" + try { + const result = await Effect.gen(function* () { + return yield* (yield* FetchUse.Service).fetch("https://example.com/page", { timeoutMs: 30_000 }) + }).pipe(Effect.provide(FetchUse.layer.pipe(Layer.provide(FetchHttpClient.layer))), Effect.runPromise) + + expect(seen.url).toBe("https://example.com/page") + expect(seen.key).toBe("sentinel-not-a-real-key") + expect(new TextDecoder().decode(result.body)).toBe("ok") + } finally { + server.stop(true) + delete process.env.BCODE_FETCH_USE_ENDPOINT + if (realKey === undefined) delete process.env.BROWSER_USE_API_KEY + else process.env.BROWSER_USE_API_KEY = realKey + } +}) + +// The endpoint carries the api key, so a bad value leaks a credential rather +// than merely failing. Loopback http is allowed because a mediating proxy on the +// same host is the normal local arrangement. +test.each([ + ["", "set but empty"], + [" ", "set but empty"], + ["not-a-url", "not a valid url"], + ["http://evil.example/fetch", "https outside loopback"], + // Would otherwise pass startup and fail at the first webfetch instead. + ["ftp://localhost/fetch", "must be http or https"], +])("rejects BCODE_FETCH_USE_ENDPOINT=%p", (value, reason) => { + process.env.BCODE_FETCH_USE_ENDPOINT = value + try { + expect(() => + Effect.runSync( + Effect.gen(function* () { + return (yield* FetchUse.Service).enabled + }).pipe(Effect.provide(FetchUse.layer.pipe(Layer.provide(FetchHttpClient.layer)))), + ), + ).toThrow(new RegExp(reason.replace(/ /g, "\\s"))) + } finally { + delete process.env.BCODE_FETCH_USE_ENDPOINT + } +}) + +test("rejection does not echo credentials carried in the endpoint value", () => { + // An endpoint can embed userinfo or a token in its query. Writing that into + // stderr on a typo is the same log leak this override exists to close, so the + // message may name the origin and nothing more. + process.env.BCODE_FETCH_USE_ENDPOINT = "http://user:hunter2@evil.example:8080/f?token=SECRET" + try { + let message = "" + try { + Effect.runSync( + Effect.gen(function* () { + return (yield* FetchUse.Service).enabled + }).pipe(Effect.provide(FetchUse.layer.pipe(Layer.provide(FetchHttpClient.layer)))), + ) + } catch (e) { + message = String(e) + } + expect(message).toContain("https outside loopback") + expect(message).toContain("evil.example:8080") + expect(message).not.toContain("hunter2") + expect(message).not.toContain("SECRET") + } finally { + delete process.env.BCODE_FETCH_USE_ENDPOINT + } +}) + +test.each([ + "https://proxy.example/fetch", + "http://127.0.0.1:7461/fetch", + "http://[::1]:7461/fetch", + // The whole 127/8 is loopback, and a rooted name is the same name. + "http://127.0.0.2:9/fetch", + "http://localhost./fetch", +])("accepts BCODE_FETCH_USE_ENDPOINT=%p", (value) => { + process.env.BCODE_FETCH_USE_ENDPOINT = value + try { + expect(() => + Effect.runSync( + Effect.gen(function* () { + return (yield* FetchUse.Service).enabled + }).pipe(Effect.provide(FetchUse.layer.pipe(Layer.provide(FetchHttpClient.layer)))), + ), + ).not.toThrow() + } finally { + delete process.env.BCODE_FETCH_USE_ENDPOINT + } + }, +) + test.skipIf(!haveKey)("live: fetches httpbin and returns body + content-type", async () => { const result = await Effect.gen(function* () { return yield* (yield* FetchUse.Service).fetch("https://httpbin.org/get", { timeoutMs: 30_000 })