Skip to content

fix: use public origin for browser approval URLs - #1967

Open
utpalsinghdev wants to merge 2 commits into
UsefulSoftwareCo:mainfrom
utpalsinghdev:utpalsinghdev/approval-url
Open

fix: use public origin for browser approval URLs#1967
utpalsinghdev wants to merge 2 commits into
UsefulSoftwareCo:mainfrom
utpalsinghdev:utpalsinghdev/approval-url

Conversation

@utpalsinghdev

Copy link
Copy Markdown

Summary

The CLI daemon listens over HTTP on loopback. Behind a TLS reverse proxy, browser approval responses built approvalUrl from request.url, so the scheme stayed http:// even when EXECUTOR_WEB_BASE_URL was set to an HTTPS origin.

createServerHandlers() already had that configured origin (and used it for artifact URLs). createMcpRequestHandler() did not. Self-host already prefers webBaseUrl in makeInMemoryMcpSessionStore. This change threads an explicit EXECUTOR_WEB_BASE_URL into the local handler the same way. When the env var is unset, the request origin is still used.

Generated URLs follow packages/core/sdk/src/public-origin.ts: the operator-set origin, not X-Forwarded-Proto or Host.

Linked issue

Closes #1954

Reproduction

  1. Run executor daemon run --foreground --port 4788 (HTTP on loopback).
  2. Put a TLS-terminating reverse proxy in front at https://executor.example.test.
  3. POST https://executor.example.test/mcp?elicitation_mode=browser, initialize, then execute a tool covered by require_approval.
  4. Before this change, structuredContent.approvalUrl is http://…/resume/<execution-id>?mcp_session_id=… and does not open on an HTTPS-only listener.
  5. With EXECUTOR_WEB_BASE_URL=https://executor.example.test, the approval URL origin is that HTTPS value. Resume path and mcp_session_id stay the same.

The focused test sends the MCP request as http://127.0.0.1:4788/mcp?elicitation_mode=browser and asserts the returned origin is the configured HTTPS public origin (including a non-default port). It also checks that path/query on EXECUTOR_WEB_BASE_URL are not inherited and that X-Forwarded-Proto / X-Forwarded-Host do not override it.

Verification

  • bunx --bun vitest run src/mcp-browser-resume.test.ts in apps/local: 2 passed. Before the production change the new case failed with origin http://127.0.0.1:4788; after it, https://executor.example.test:8443.
  • bunx --bun vitest run in apps/local: 17 files, 103 tests passed.
  • bun run format:check: passed.
  • bun run lint: passed.
  • bun run typecheck: 45/45 packages passed.
  • bun run test: failed on unrelated packages while turbo ran the workspace in parallel (@executor-js/host-cloudflare Cloudflare Access e2e timed out at 5s). A local v1 to v2 SIGKILL migration test also failed under that load and passed when @executor-js/local was run alone. There is no e2e scenario for a TLS-proxied CLI daemon; the local MCP integration test is the public-behavior coverage.

Docs

apps/docs/local/cli.mdx now documents EXECUTOR_WEB_BASE_URL for a TLS reverse proxy in front of the CLI daemon.

CLI browser approval links ignored EXECUTOR_WEB_BASE_URL and inherited the
internal HTTP listener scheme, so TLS-proxied deployments got unreachable
http:// URLs.
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.
@utpalsinghdev
utpalsinghdev force-pushed the utpalsinghdev/approval-url branch from 57a31e5 to 1fcae95 Compare September 8, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] CLI daemon composes approvalUrl from its own listener origin, so behind TLS the link is http:// and unreachable

1 participant