From c99117fa7cc5995423cce8fa2acd454d281fe00c Mon Sep 17 00:00:00 2001 From: Alexander Yue Date: Sun, 23 Aug 2026 22:21:40 -0700 Subject: [PATCH 1/3] feat(bcode-browser): allow overriding the fetch-use endpoint The default endpoint is a general-purpose URL fetcher, so BROWSER_USE_API_KEY is not bounded by whatever network allowlist the process runs under: anything holding the key can ask the fetcher to retrieve an arbitrary host, and a prompt-injected agent can put the key in that URL and read it back out of the attacker's logs. An egress allowlist does not help, because reaching the fetcher is exactly what it permits. BCODE_FETCH_USE_ENDPOINT lets the caller interpose. A sandboxed agent can be given a mediating proxy and a throwaway credential while the real key stays in the parent process, which is the arrangement the RL harness in benchmark-x-laminar needs. Unset, behaviour is unchanged. The test runs a real server on a loopback port and asserts both that the override is used and that the target url arrives in the body, since a proxy has nothing to forward otherwise. Confirmed it fails against the unmodified source rather than passing vacuously. --- packages/bcode-browser/src/fetch-use.ts | 10 ++++-- packages/bcode-browser/test/fetch-use.test.ts | 32 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/bcode-browser/src/fetch-use.ts b/packages/bcode-browser/src/fetch-use.ts index 83562b9828..0fd182f641 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,17 @@ export const layer = Layer.effect( Effect.gen(function* () { const http = yield* HttpClient.HttpClient const apiKey = process.env.BROWSER_USE_API_KEY ?? "" + // 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. + const endpoint = process.env.BCODE_FETCH_USE_ENDPOINT || DEFAULT_ENDPOINT 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 }), ) diff --git a/packages/bcode-browser/test/fetch-use.test.ts b/packages/bcode-browser/test/fetch-use.test.ts index 753118d579..2eb1367b83 100644 --- a/packages/bcode-browser/test/fetch-use.test.ts +++ b/packages/bcode-browser/test/fetch-use.test.ts @@ -20,6 +20,38 @@ 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 + } +}) + 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 }) From 4b1cee089e5f07361de25f60c2ac60e204727cbc Mon Sep 17 00:00:00 2001 From: Alexander Yue Date: Sun, 23 Aug 2026 23:11:13 -0700 Subject: [PATCH 2/3] fix(bcode-browser): validate the fetch-use endpoint override The variable names the host that receives X-Browser-Use-API-Key, so a bad value leaks a credential rather than merely failing, and each rejection here is an operator mistake catchable at startup. Set-but-empty was the worst of them: `||` sent it back to the default, which is the direct fetcher -- the exact path someone setting this variable is trying to leave. A typo'd or unexpanded value silently restored the behaviour the override exists to remove, which is the failure you least want to be quiet. `??` distinguishes unset (use the default, the ordinary case) from set and empty (a mistake). Cleartext is rejected outside loopback, since a mediating proxy on the same host is the normal local arrangement and both of our own callers satisfy this already: sandboxes get an https tunnel, local runs get 127.0.0.1. Validation is synchronous and throws rather than failing the Effect, because an error channel on this layer would propagate into ToolRegistry through registry.ts:430, and a startup misconfiguration is not a recoverable condition. Note for the review suggestion this came from: its loopback test used "::1", but URL reports the IPv6 literal with brackets, so an IPv6 endpoint would have been rejected. The test covers that case and fails against the unbracketed form. --- packages/bcode-browser/src/fetch-use.ts | 33 ++++++++++++--- packages/bcode-browser/test/fetch-use.test.ts | 41 +++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/bcode-browser/src/fetch-use.ts b/packages/bcode-browser/src/fetch-use.ts index 0fd182f641..24f4cc9aa7 100644 --- a/packages/bcode-browser/src/fetch-use.ts +++ b/packages/bcode-browser/src/fetch-use.ts @@ -32,12 +32,7 @@ export const layer = Layer.effect( Effect.gen(function* () { const http = yield* HttpClient.HttpClient const apiKey = process.env.BROWSER_USE_API_KEY ?? "" - // 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. - const endpoint = process.env.BCODE_FETCH_USE_ENDPOINT || DEFAULT_ENDPOINT + const endpoint = resolveEndpoint() return Service.of({ enabled: apiKey.length > 0, fetch: (url, { timeoutMs }) => @@ -62,4 +57,30 @@ 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") + if (!URL.canParse(configured)) throw new Error(`BCODE_FETCH_USE_ENDPOINT is not a valid url: ${configured}`) + const url = new URL(configured) + // Node reports the IPv6 literal with its brackets, so "::1" would never match. + const loopback = ["localhost", "127.0.0.1", "[::1]"].includes(url.hostname) + if (url.protocol !== "https:" && !loopback) + throw new Error( + `BCODE_FETCH_USE_ENDPOINT must use https outside loopback; refusing to send the api key in cleartext to ${configured}`, + ) + return configured +} + 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 2eb1367b83..956af7b5da 100644 --- a/packages/bcode-browser/test/fetch-use.test.ts +++ b/packages/bcode-browser/test/fetch-use.test.ts @@ -52,6 +52,47 @@ test("BCODE_FETCH_USE_ENDPOINT redirects the request and forwards the target url } }) +// 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"], +])("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.each(["https://proxy.example/fetch", "http://127.0.0.1:7461/fetch", "http://[::1]:7461/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 }) From d9d571ec9d9549f3ea5b01ef58992326e0326fc0 Mon Sep 17 00:00:00 2001 From: Alexander Yue Date: Sun, 23 Aug 2026 23:24:41 -0700 Subject: [PATCH 3/3] fix(bcode-browser): widen the loopback test and stop echoing the endpoint Three gaps in the validation added by the previous commit. Loopback was three literal hostnames, but all of 127.0.0.0/8 is loopback and a trailing dot is the same name in rooted form, so a proxy on 127.0.0.2 was refused with a message claiming it was not loopback. The pattern is anchored and numeric so a DNS name like 127.example.com is not mistaken for the subnet. The scheme check ran only against https, so ftp://localhost passed startup on the loopback exemption and would have failed at the first webfetch instead -- the deferred failure this validation exists to pull forward. http or https is now required before the exemption is considered. Both messages echoed the raw value, which can carry userinfo or a token in its query, so a typo wrote a credential into stderr: the same log leak the override exists to close. The cleartext message names url.origin, which drops userinfo, path and query, and the parse failure names only the variable, since an operator can read back their own environment. Tested against a value carrying both a password and a query token. Not adopted: the report also expected a proxy bound on 0.0.0.0 to be accepted. That is a bind address, not a connect target, so refusing it as non-loopback is correct. All four cases fail against the previous implementation and pass against this one. 13 pass / 1 skip; typecheck 17/17. --- packages/bcode-browser/src/fetch-use.ts | 22 ++++++++--- packages/bcode-browser/test/fetch-use.test.ts | 38 +++++++++++++++++-- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/packages/bcode-browser/src/fetch-use.ts b/packages/bcode-browser/src/fetch-use.ts index 24f4cc9aa7..73f6483985 100644 --- a/packages/bcode-browser/src/fetch-use.ts +++ b/packages/bcode-browser/src/fetch-use.ts @@ -72,15 +72,27 @@ function resolveEndpoint() { 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") - if (!URL.canParse(configured)) throw new Error(`BCODE_FETCH_USE_ENDPOINT is not a valid url: ${configured}`) + // 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) - // Node reports the IPv6 literal with its brackets, so "::1" would never match. - const loopback = ["localhost", "127.0.0.1", "[::1]"].includes(url.hostname) - if (url.protocol !== "https:" && !loopback) + // 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 ${configured}`, + `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 956af7b5da..3dc93c8ab4 100644 --- a/packages/bcode-browser/test/fetch-use.test.ts +++ b/packages/bcode-browser/test/fetch-use.test.ts @@ -60,6 +60,8 @@ test.each([ [" ", "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 { @@ -75,9 +77,39 @@ test.each([ } }) -test.each(["https://proxy.example/fetch", "http://127.0.0.1:7461/fetch", "http://[::1]:7461/fetch"])( - "accepts BCODE_FETCH_USE_ENDPOINT=%p", - (value) => { +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(() =>