From 769b4383596b38e70bb0048ce46786385951a9de Mon Sep 17 00:00:00 2001 From: utpal singh Date: Tue, 8 Sep 2026 23:21:12 +0530 Subject: [PATCH 1/3] fix: use public origin for approval URLs CLI browser approval links ignored EXECUTOR_WEB_BASE_URL and inherited the internal HTTP listener scheme, so TLS-proxied deployments got unreachable http:// URLs. --- .changeset/cli-approval-public-origin.md | 5 ++ apps/docs/local/cli.mdx | 12 ++++ apps/local/src/main.ts | 1 + apps/local/src/mcp-browser-resume.test.ts | 79 +++++++++++++++++++++-- apps/local/src/mcp.ts | 15 ++++- 5 files changed, 105 insertions(+), 7 deletions(-) create mode 100644 .changeset/cli-approval-public-origin.md diff --git a/.changeset/cli-approval-public-origin.md b/.changeset/cli-approval-public-origin.md new file mode 100644 index 0000000000..ebe473e9c0 --- /dev/null +++ b/.changeset/cli-approval-public-origin.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Pin CLI browser approval links to `EXECUTOR_WEB_BASE_URL` so a TLS reverse proxy no longer returns an unreachable `http://` URL. diff --git a/apps/docs/local/cli.mdx b/apps/docs/local/cli.mdx index f31a8b7855..68a380d826 100644 --- a/apps/docs/local/cli.mdx +++ b/apps/docs/local/cli.mdx @@ -43,6 +43,18 @@ executor web # open the web UI at http://127.0.0.1:4788 `executor install` registers Executor so it keeps running across restarts. For a throwaway foreground runtime instead, run `executor web --foreground`. +## Behind a TLS reverse proxy + +The daemon listens over HTTP on loopback. If a reverse proxy terminates TLS in +front of it, set `EXECUTOR_WEB_BASE_URL` to the public HTTPS origin so browser +approval links use that origin: + +```bash +EXECUTOR_WEB_BASE_URL=https://executor.example.test executor daemon run --foreground +``` + +Generated approval URLs take this value, not `X-Forwarded-Proto` or `Host`. + ## Connect an agent Add Executor to any MCP client (Claude Code, Cursor, OpenCode) with `npx add-mcp`. diff --git a/apps/local/src/main.ts b/apps/local/src/main.ts index 2ef674c572..12c314574c 100644 --- a/apps/local/src/main.ts +++ b/apps/local/src/main.ts @@ -125,6 +125,7 @@ export const createServerHandlers = async (token: string): Promise { if (resource.kind === "default") { return { diff --git a/apps/local/src/mcp-browser-resume.test.ts b/apps/local/src/mcp-browser-resume.test.ts index 32d5c03c9c..d3b2b58cc8 100644 --- a/apps/local/src/mcp-browser-resume.test.ts +++ b/apps/local/src/mcp-browser-resume.test.ts @@ -104,20 +104,35 @@ const makeExecutor = async (tmpDir: string): Promise => { }; }; -const makeMcpFetch = (executor: Executor) => { +const makeMcpFetch = ( + executor: Executor, + options: { + readonly webBaseUrl?: string; + readonly extraHeaders?: HeadersInit; + } = {}, +) => { const engine = createExecutionEngine({ executor, codeExecutor: makeQuickJsExecutor(), }); - const mcp = createMcpRequestHandler({ engine }); + const mcp = createMcpRequestHandler( + options.webBaseUrl === undefined + ? { engine } + : { defaultConfig: { engine }, webBaseUrl: options.webBaseUrl }, + ); const fetchImpl: typeof globalThis.fetch = Object.assign( (input: RequestInfo | URL, init?: RequestInit) => { const request = input instanceof Request ? input : new Request(input, init); - const url = new URL(request.url); - if (url.pathname.startsWith("/mcp")) return mcp.handleRequest(request); + const headers = new Headers(request.headers); + if (options.extraHeaders) { + new Headers(options.extraHeaders).forEach((value, key) => headers.set(key, value)); + } + const forwarded = new Request(request, { headers }); + const url = new URL(forwarded.url); + if (url.pathname.startsWith("/mcp")) return mcp.handleRequest(forwarded); if (url.pathname.startsWith("/api/mcp-sessions/")) { - return mcp.handleApprovalRequest(request); + return mcp.handleApprovalRequest(forwarded); } return Promise.resolve(new Response("Not found", { status: 404 })); }, @@ -192,6 +207,11 @@ describe("local MCP browser approval resume", () => { expect(first.isError).toBeFalsy(); const firstApproval = readApproval(first.structuredContent); + expect(firstApproval.url.origin).toBe(TEST_BASE_URL); + expect(firstApproval.url.pathname).toBe( + `/resume/${encodeURIComponent(firstApproval.executionId)}`, + ); + expect(firstApproval.url.searchParams.get("mcp_session_id")).not.toBeNull(); const second = await approveInBrowserThenResume(fetch, mcpClient, firstApproval); const secondApproval = readApproval(second.structuredContent); @@ -228,6 +248,55 @@ describe("local MCP browser approval resume", () => { rmSync(tmpDir, { recursive: true, force: true }); } }, 10_000); + + it("uses EXECUTOR_WEB_BASE_URL for approval links when the request is internal HTTP", async () => { + const tmpDir = mkdtempSync(join(tmpdir(), "executor-local-browser-resume-origin-")); + const executor = await makeExecutor(tmpDir); + const { fetch, dispose } = makeMcpFetch(executor, { + webBaseUrl: "https://executor.example.test:8443/prefix?from-base=1", + extraHeaders: { + "x-forwarded-proto": "https", + "x-forwarded-host": "poisoned.example", + }, + }); + const mcpClient = new Client( + { name: "browser-resume-origin-test-client", version: "1.0.0" }, + { capabilities: {} }, + ); + const transport = new StreamableHTTPClientTransport( + new URL("/mcp?elicitation_mode=browser", "http://127.0.0.1:4788"), + { fetch }, + ); + + await mcpClient.connect(transport); + + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: test owns MCP transports, web handler, and executor lifecycle + try { + const paused = await mcpClient.callTool({ + name: "execute", + arguments: { + code: `return await tools.api.singleApproval({});`, + }, + }); + + expect(paused.isError).toBeFalsy(); + const approval = readApproval(paused.structuredContent); + expect(approval.url.origin).toBe("https://executor.example.test:8443"); + expect(approval.url.pathname).toBe(`/resume/${encodeURIComponent(approval.executionId)}`); + expect(approval.url.pathname).not.toContain("/prefix"); + expect(approval.url.searchParams.get("from-base")).toBeNull(); + expect(approval.url.searchParams.get("mcp_session_id")).not.toBeNull(); + expect(approval.url.host).not.toBe("poisoned.example"); + expect(approval.url.protocol).not.toBe("http:"); + } finally { + await mcpClient.close(); + await Effect.runPromise(Effect.ignore(Effect.tryPromise(() => dispose()))); + await Effect.runPromise( + Effect.ignore(Effect.tryPromise(() => Effect.runPromise(executor.close()))), + ); + rmSync(tmpDir, { recursive: true, force: true }); + } + }, 10_000); }); const approveInBrowserThenResume = async ( diff --git a/apps/local/src/mcp.ts b/apps/local/src/mcp.ts index caa5548d4a..1e8bf98e47 100644 --- a/apps/local/src/mcp.ts +++ b/apps/local/src/mcp.ts @@ -15,7 +15,7 @@ import { type ExecutorMcpServerConfig, } from "@executor-js/host-mcp/tool-server"; import { - approvalUrlForRequest, + buildResumeApprovalUrl, decodeResumeResponse, formatResumeAcknowledgement, readArtifactsEnabled, @@ -56,6 +56,13 @@ export interface LocalMcpRequestHandlerConfig { readonly createConfigForResource?: ( resource: McpResource, ) => Promise | LocalMcpServerConfig; + /** + * Pinned public origin for browser-approval URLs. When set (for example + * `EXECUTOR_WEB_BASE_URL` behind a TLS proxy) it is preferred over the + * request URL, whose scheme is the internal HTTP listener. Omit it on + * loopback so the request origin stays the approval link. + */ + readonly webBaseUrl?: string; } // Local serves these error bodies in-process; like the self-host store they are @@ -235,7 +242,11 @@ export const createMcpRequestHandler = ( ? { mode: "browser" as const, approvalUrl: (executionId) => - approvalUrlForRequest(request, executionId, createdSessionId), + buildResumeApprovalUrl({ + origin: handlerConfig.webBaseUrl ?? request.url, + executionId, + sessionId: createdSessionId, + }), } : { mode: elicitationMode }, }), From 1fcae9587421533ea7b82c3e82a151b9bf6731ef Mon Sep 17 00:00:00 2001 From: utpal singh Date: Tue, 8 Sep 2026 23:29:58 +0530 Subject: [PATCH 2/3] fix: skip port-0 web base URL for approval links CLI --port 0 installs EXECUTOR_WEB_BASE_URL as http://127.0.0.1:0 before the OS assigns a listen port. Chrome rejects that origin as ERR_UNSAFE_PORT, so approval URLs fall back to the request origin in that case. --- apps/local/src/mcp-browser-resume.test.ts | 40 +++++++++++++++++++++++ apps/local/src/mcp.ts | 13 ++++++-- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/apps/local/src/mcp-browser-resume.test.ts b/apps/local/src/mcp-browser-resume.test.ts index d3b2b58cc8..e9b51acd80 100644 --- a/apps/local/src/mcp-browser-resume.test.ts +++ b/apps/local/src/mcp-browser-resume.test.ts @@ -297,6 +297,46 @@ describe("local MCP browser approval resume", () => { rmSync(tmpDir, { recursive: true, force: true }); } }, 10_000); + + it("falls back to the request origin when EXECUTOR_WEB_BASE_URL uses ephemeral port 0", async () => { + const tmpDir = mkdtempSync(join(tmpdir(), "executor-local-browser-resume-port0-")); + const executor = await makeExecutor(tmpDir); + const { fetch, dispose } = makeMcpFetch(executor, { + webBaseUrl: "http://127.0.0.1:0", + }); + const mcpClient = new Client( + { name: "browser-resume-port0-test-client", version: "1.0.0" }, + { capabilities: {} }, + ); + const transport = new StreamableHTTPClientTransport( + new URL("/mcp?elicitation_mode=browser", "http://127.0.0.1:4788"), + { fetch }, + ); + + await mcpClient.connect(transport); + + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: test owns MCP transports, web handler, and executor lifecycle + try { + const paused = await mcpClient.callTool({ + name: "execute", + arguments: { + code: `return await tools.api.singleApproval({});`, + }, + }); + + expect(paused.isError).toBeFalsy(); + const approval = readApproval(paused.structuredContent); + expect(approval.url.origin).toBe("http://127.0.0.1:4788"); + expect(approval.url.port).not.toBe("0"); + } finally { + await mcpClient.close(); + await Effect.runPromise(Effect.ignore(Effect.tryPromise(() => dispose()))); + await Effect.runPromise( + Effect.ignore(Effect.tryPromise(() => Effect.runPromise(executor.close()))), + ); + rmSync(tmpDir, { recursive: true, force: true }); + } + }, 10_000); }); const approveInBrowserThenResume = async ( diff --git a/apps/local/src/mcp.ts b/apps/local/src/mcp.ts index 1e8bf98e47..0ea30d6d84 100644 --- a/apps/local/src/mcp.ts +++ b/apps/local/src/mcp.ts @@ -60,7 +60,8 @@ export interface LocalMcpRequestHandlerConfig { * Pinned public origin for browser-approval URLs. When set (for example * `EXECUTOR_WEB_BASE_URL` behind a TLS proxy) it is preferred over the * request URL, whose scheme is the internal HTTP listener. Omit it on - * loopback so the request origin stays the approval link. + * loopback so the request origin stays the approval link. Port 0 (an + * ephemeral bind placeholder) is treated as unset. */ readonly webBaseUrl?: string; } @@ -129,6 +130,14 @@ const normalizeHandlerConfig = ( input: ExecutorMcpServerConfig | LocalMcpRequestHandlerConfig, ): LocalMcpRequestHandlerConfig => ("defaultConfig" in input ? input : { defaultConfig: input }); +// `--port 0` (e2e, some CLI boots) installs EXECUTOR_WEB_BASE_URL with port 0 +// before the OS assigns a listen port. That origin is not browser-reachable +// (Chrome ERR_UNSAFE_PORT), so approval URLs fall back to the request. +const resumeApprovalOrigin = (configured: string | undefined, requestUrl: string): string => { + if (configured === undefined || configured.length === 0) return requestUrl; + return new URL(configured).port === "0" ? requestUrl : configured; +}; + export const createMcpRequestHandler = ( input: ExecutorMcpServerConfig | LocalMcpRequestHandlerConfig, ): McpRequestHandler => { @@ -243,7 +252,7 @@ export const createMcpRequestHandler = ( mode: "browser" as const, approvalUrl: (executionId) => buildResumeApprovalUrl({ - origin: handlerConfig.webBaseUrl ?? request.url, + origin: resumeApprovalOrigin(handlerConfig.webBaseUrl, request.url), executionId, sessionId: createdSessionId, }), From dffbafd0b5fb383a79e11839969246c0960eb3d6 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:46:42 -0700 Subject: [PATCH 3/3] Exercise browser approvals through a TLS proxy --- .../mcp-browser-approve-public-origin.test.ts | 234 ++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 e2e/local/mcp-browser-approve-public-origin.test.ts diff --git a/e2e/local/mcp-browser-approve-public-origin.test.ts b/e2e/local/mcp-browser-approve-public-origin.test.ts new file mode 100644 index 0000000000..03d19bb500 --- /dev/null +++ b/e2e/local/mcp-browser-approve-public-origin.test.ts @@ -0,0 +1,234 @@ +// Local-only — the MCP BROWSER-APPROVAL flow, the gap a code review found: +// `resume.$executionId.tsx` POSTed to the bearer-gated `/api/mcp-sessions/*` +// with no Authorization header, so standalone-web approvals 401'd. The existing +// approval scenario (selfhost/mcp-approve.test.ts) approves PROGRAMMATICALLY via +// the MCP `resume` tool (auth on the API path), so it never drives the browser +// page and could not catch this. This drives the real page in a real browser. +// +// Flow: boot `executor web --foreground` → create a require_approval policy on a +// built-in tool → an MCP client (bearer) executes that tool with +// elicitation_mode=browser → the server returns a paused `approvalUrl` → open it +// in the browser (with the `?_token` bootstrap) → click Approve → the MCP +// `resume` call completes. Plus a negative: the approval endpoint 401s without +// the bearer. +import { execFileSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { createServer } from "node:https"; +import { request as httpRequest } from "node:http"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { HttpApiClient } from "effect/unstable/httpapi"; +import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import { composePluginApi } from "@executor-js/api/server"; + +import { scenario } from "../src/scenario"; +import { Browser, Cli, RunDir, Target } from "../src/services"; +import { withLocalServer } from "./local-server"; + +const coreApi = composePluginApi([] as const); + +// A built-in, read-only tool to gate (same target the selfhost approval test +// uses) — calling it under a require_approval policy forces the elicitation. +const APPROVAL_TARGET_TOOL = "executor.coreTools.policies.list"; +const EXECUTE_CODE = ` +const result = await tools.executor.coreTools.policies.list({}); +return JSON.stringify(result); +`; + +const tlsProxy = Effect.acquireRelease( + Effect.promise(async () => { + const dir = mkdtempSync(join(tmpdir(), "executor-tls-proxy-")); + execFileSync( + "openssl", + [ + "req", + "-x509", + "-newkey", + "rsa:2048", + "-nodes", + "-keyout", + join(dir, "key.pem"), + "-out", + join(dir, "cert.pem"), + "-days", + "1", + "-subj", + "/CN=localhost", + ], + { stdio: "ignore" }, + ); + let upstream: string | undefined; + const server = createServer( + { key: readFileSync(join(dir, "key.pem")), cert: readFileSync(join(dir, "cert.pem")) }, + (incoming, outgoing) => { + if (upstream === undefined) { + outgoing.writeHead(503).end(); + return; + } + const request = httpRequest( + new URL(incoming.url ?? "/", upstream), + { + method: incoming.method, + headers: { + ...incoming.headers, + "x-forwarded-proto": "http", + "x-forwarded-host": "untrusted.example", + }, + }, + (response) => { + outgoing.writeHead(response.statusCode ?? 502, response.headers); + response.pipe(outgoing); + }, + ); + request.on("error", () => { + outgoing.writeHead(502).end(); + }); + incoming.pipe(request); + }, + ); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (address === null || typeof address === "string") throw new Error("TLS proxy did not bind"); + return { + origin: `https://127.0.0.1:${address.port}`, + attach: (origin: string) => { + upstream = origin; + }, + close: () => { + server.closeAllConnections(); + server.close(); + rmSync(dir, { recursive: true, force: true }); + }, + }; + }), + (proxy) => Effect.sync(proxy.close), +); + +scenario( + "Local · TLS proxy approval uses the configured HTTPS origin and resumes in the browser", + { timeout: 180_000 }, + Effect.scoped( + Effect.gen(function* () { + const proxy = yield* tlsProxy; + const cli = yield* Cli; + const browser = yield* Browser; + const target = yield* Target; + const runDir = yield* RunDir; + const identity = yield* target.newIdentity(); + + yield* withLocalServer( + cli, + runDir, + (server) => + Effect.gen(function* () { + proxy.attach(server.origin); + // Bearer-authed typed API client (local has no session cookie — the + // credential is the printed token). Used only to plant the policy. + const api = yield* HttpApiClient.make(coreApi, { + baseUrl: new URL("/api", server.origin).toString(), + transformClient: HttpClient.mapRequest((request) => + HttpClientRequest.setHeader(request, "authorization", `Bearer ${server.token}`), + ), + }).pipe(Effect.provide(FetchHttpClient.layer)); + + yield* api.policies.create({ + payload: { owner: "org", pattern: APPROVAL_TARGET_TOOL, action: "require_approval" }, + }); + + yield* browser.session(identity, async ({ page, step }) => { + const security = await page.context().newCDPSession(page); + await security.send("Security.setIgnoreCertificateErrors", { ignore: true }); + // MCP client over the wire with the bearer (local /mcp is bearer-gated, + // not OAuth — so the raw SDK transport with an Authorization header, + // not mcporter's PKCE flow). elicitation_mode=browser makes the server + // mint an approval URL instead of a model-side pause. + const mcp = new Client( + { name: "e2e-local-approve", version: "1.0.0" }, + { capabilities: {} }, + ); + const transport = new StreamableHTTPClientTransport( + new URL(`${server.origin}/mcp?elicitation_mode=browser`), + { requestInit: { headers: { authorization: `Bearer ${server.token}` } } }, + ); + await mcp.connect(transport); + + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: the test owns the MCP transport lifecycle + try { + const executed = await mcp.callTool({ + name: "execute", + arguments: { code: EXECUTE_CODE }, + }); + const paused = executed.structuredContent as { + status: string; + executionId: string; + approvalUrl: string; + }; + expect(paused.status, "execute paused for browser approval").toBe( + "user_approval_required", + ); + expect(typeof paused.approvalUrl).toBe("string"); + expect(new URL(paused.approvalUrl).origin).toBe(proxy.origin); + + // The real human flow: approve in the browser FIRST. The page POSTs + // the decision to the bearer-gated /api/mcp-sessions/* endpoint (the + // path bug #2 left unauthenticated) — that just records the decision. + // Calling the MCP `resume` tool first would un-pause the engine and + // the page's getPaused would find nothing, so order matters. + await step("Open the approval URL and approve in the browser", async () => { + const approval = new URL(paused.approvalUrl); + approval.searchParams.set("_token", server.token); // bootstrap the bearer + await page.goto(approval.toString(), { waitUntil: "domcontentloaded" }); + await page.getByRole("button", { name: "Approve" }).waitFor({ timeout: 30_000 }); + // The page loaded the paused execution (bearer-authed) — not the + // "unavailable" error branch a getPaused 401/404 would render. + // (Playwright's toBeVisible matcher isn't in vitest's expect.) + expect( + await page.getByText("This paused execution is no longer available").count(), + "approval page loaded the paused execution, not the unavailable branch", + ).toBe(0); + await page.getByRole("button", { name: "Approve" }).click(); + // "Approve sent" only renders if the POST returned 200 — i.e. the + // bearer reached the gated endpoint. Pre-fix it 401'd and stuck. + await page.getByText("Approve sent").waitFor({ timeout: 15_000 }); + }); + + // The agent's `resume` now picks up the recorded approval and the + // engine finishes. + const resumed = await mcp.callTool({ + name: "resume", + arguments: { executionId: paused.executionId }, + }); + const resumedStructured = resumed.structuredContent as { status: string }; + expect( + resumedStructured.status, + "the MCP resume completed once the browser approved (bearer reached the gated endpoint)", + ).toBe("completed"); + + await step("The approval endpoint rejects a request with no bearer", async () => { + const unauthed = await fetch( + `${server.origin}/api/mcp-sessions/${encodeURIComponent( + paused.executionId, + )}/executions/${encodeURIComponent(paused.executionId)}/resume?approval_token=x`, + { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ action: "accept" }), + }, + ); + expect(unauthed.status, "no bearer → 401 at the shell gate").toBe(401); + }); + } finally { + await mcp.close(); + } + }); + }), + { env: { EXECUTOR_WEB_BASE_URL: proxy.origin } }, + ); + }), + ), +);