From 3495cad1335b59bf97bdaa635912063ee0aae61a Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 24 Mar 2026 15:01:33 -0400 Subject: [PATCH] fix: resolve localhost URLs in markdown with correct port and protocol (#23513) ## Summary Fixes several bugs in the markdown URL transform that replaces `localhost` URLs with workspace port-forward URLs in the AI agent chat. ## Bugs Fixed ### 1. URLs without an explicit port produce `NaN` in the subdomain When an LLM outputs a URL like `http://localhost/path` (no port), `parsed.port` is the empty string `""`. `parseInt("", 10)` returns `NaN`, producing a broken URL like: ``` http://NaN--agent--workspace--user.proxy.example.com/path ``` Now defaults to port 80 for HTTP and 443 for HTTPS via the new `resolveLocalhostPort()` helper. ### 2. Protocol always hardcoded to `"http"` The `urlTransform` in `AgentDetail.tsx` always passed `"http"` as the protocol argument, silently discarding the original URL's scheme. This meant `https://localhost:8443/...` would not get the `s` suffix in the subdomain. Now extracts the protocol from the parsed URL, matching the existing behavior of `openMaybePortForwardedURL`. ### 3. `urlTransform` not memoized The closure was re-created on every render. Wrapped in `useCallback` with the four primitive dependencies (`proxyHost`, `agentName`, `wsName`, `wsOwner`). ### 4. Duplicated `localHosts` definition The localhost detection set was defined separately in both `AgentDetail.tsx` and `portForward.ts`. Consolidated into a single shared export from `portForward.ts`. ## Changes - **`site/src/utils/portForward.ts`**: Export shared `localHosts` set and new `resolveLocalhostPort()` helper. Update `openMaybePortForwardedURL` to use both. - **`site/src/pages/AgentsPage/AgentDetail.tsx`**: Import shared `localHosts` and `resolveLocalhostPort`. Fix protocol extraction. Memoize `urlTransform`. - **`site/src/utils/portForward.jest.ts`**: Add tests for `resolveLocalhostPort` and `localHosts`. Renamed from `.test.ts` to `.jest.ts` to match project convention. --- site/src/pages/AgentsPage/AgentDetail.tsx | 23 +-- site/src/utils/portForward.jest.ts | 197 ++++++++++++++++++++++ site/src/utils/portForward.test.ts | 67 -------- site/src/utils/portForward.ts | 81 ++++++--- 4 files changed, 258 insertions(+), 110 deletions(-) create mode 100644 site/src/utils/portForward.jest.ts delete mode 100644 site/src/utils/portForward.test.ts diff --git a/site/src/pages/AgentsPage/AgentDetail.tsx b/site/src/pages/AgentsPage/AgentDetail.tsx index 53144c5450..7dc57c0e88 100644 --- a/site/src/pages/AgentsPage/AgentDetail.tsx +++ b/site/src/pages/AgentsPage/AgentDetail.tsx @@ -37,7 +37,7 @@ import { toast } from "sonner"; import type { UrlTransform } from "streamdown"; import { isMobileViewport } from "utils/mobile"; import { pageTitle } from "utils/page"; -import { portForwardURL } from "utils/portForward"; +import { rewriteLocalhostURL } from "utils/portForward"; import type { AgentsOutletContext } from "./AgentsPage"; import type { ChatMessageInputRef } from "./components/AgentChatInput"; import { useChatStore } from "./components/AgentDetail/ChatContext"; @@ -70,8 +70,6 @@ import { /** localStorage key controlling whether the right panel is visible. */ export const RIGHT_PANEL_OPEN_KEY = "agents.right-panel-open"; -const localHosts = new Set(["localhost", "127.0.0.1", "0.0.0.0"]); - const lastModelConfigIDStorageKey = "agents.last-model-config-id"; /** @internal Exported for testing. */ export const draftInputStorageKeyPrefix = "agents.draft-input."; @@ -401,24 +399,7 @@ const AgentDetail: FC = () => { if (!proxyHost || !agentName || !wsName || !wsOwner) { return url; } - try { - const parsed = new URL(url); - if (!localHosts.has(parsed.hostname)) { - return url; - } - return portForwardURL( - proxyHost, - Number.parseInt(parsed.port, 10), - agentName, - wsName, - wsOwner, - "http", - parsed.pathname, - parsed.search, - ); - } catch { - return url; - } + return rewriteLocalhostURL(url, proxyHost, agentName, wsName, wsOwner); }; const chatRecord = chatQuery.data; diff --git a/site/src/utils/portForward.jest.ts b/site/src/utils/portForward.jest.ts new file mode 100644 index 0000000000..a14debfdad --- /dev/null +++ b/site/src/utils/portForward.jest.ts @@ -0,0 +1,197 @@ +import { + portForwardURL, + resolveLocalhostPort, + rewriteLocalhostURL, +} from "./portForward"; + +describe("port forward URL", () => { + const proxyHostWildcard = "*.proxy-host.tld"; + const samplePort = 12345; + const sampleAgent = "my-agent"; + const sampleWorkspace = "my-workspace"; + const sampleUsername = "my-username"; + + it("https, host and port", () => { + const forwarded = portForwardURL( + proxyHostWildcard, + samplePort, + sampleAgent, + sampleWorkspace, + sampleUsername, + "https", + ); + expect(forwarded).toEqual( + "http://12345s--my-agent--my-workspace--my-username.proxy-host.tld/", + ); + }); + it("http, host, port and path", () => { + const forwarded = portForwardURL( + proxyHostWildcard, + samplePort, + sampleAgent, + sampleWorkspace, + sampleUsername, + "http", + "/path1/path2", + ); + expect(forwarded).toEqual( + "http://12345--my-agent--my-workspace--my-username.proxy-host.tld/path1/path2", + ); + }); + it("https, host, port, path and empty params", () => { + const forwarded = portForwardURL( + proxyHostWildcard, + samplePort, + sampleAgent, + sampleWorkspace, + sampleUsername, + "https", + "/path1/path2", + "?", + ); + expect(forwarded).toEqual( + "http://12345s--my-agent--my-workspace--my-username.proxy-host.tld/path1/path2?", + ); + }); + it("http, host, port, path and query params", () => { + const forwarded = portForwardURL( + proxyHostWildcard, + samplePort, + sampleAgent, + sampleWorkspace, + sampleUsername, + "http", + "/path1/path2", + "?key1=value1&key2=value2", + ); + expect(forwarded).toEqual( + "http://12345--my-agent--my-workspace--my-username.proxy-host.tld/path1/path2?key1=value1&key2=value2", + ); + }); +}); + +describe("resolveLocalhostPort", () => { + it("returns parsed port when port string is non-empty", () => { + expect(resolveLocalhostPort("3000", "http:")).toBe(3000); + }); + + it("defaults to 80 for http when port is empty", () => { + expect(resolveLocalhostPort("", "http:")).toBe(80); + }); + + it("defaults to 443 for https when port is empty", () => { + expect(resolveLocalhostPort("", "https:")).toBe(443); + }); +}); + +describe("rewriteLocalhostURL", () => { + const proxyHost = "*.proxy-host.tld"; + const agent = "my-agent"; + const workspace = "my-workspace"; + const username = "my-username"; + + it("rewrites http://localhost:3000/path", () => { + const result = rewriteLocalhostURL( + "http://localhost:3000/path", + proxyHost, + agent, + workspace, + username, + ); + expect(result).toEqual( + "http://3000--my-agent--my-workspace--my-username.proxy-host.tld/path", + ); + }); + + it("rewrites https://localhost:8443/path with s suffix", () => { + const result = rewriteLocalhostURL( + "https://localhost:8443/path", + proxyHost, + agent, + workspace, + username, + ); + expect(result).toEqual( + "http://8443s--my-agent--my-workspace--my-username.proxy-host.tld/path", + ); + }); + + it("defaults to port 80 when no port is specified", () => { + const result = rewriteLocalhostURL( + "http://localhost/path", + proxyHost, + agent, + workspace, + username, + ); + expect(result).toEqual( + "http://80--my-agent--my-workspace--my-username.proxy-host.tld/path", + ); + }); + + it("defaults to port 443 for https without explicit port", () => { + const result = rewriteLocalhostURL( + "https://localhost/path", + proxyHost, + agent, + workspace, + username, + ); + expect(result).toEqual( + "http://443s--my-agent--my-workspace--my-username.proxy-host.tld/path", + ); + }); + + it("rewrites 127.0.0.1", () => { + const result = rewriteLocalhostURL( + "http://127.0.0.1:5000/api", + proxyHost, + agent, + workspace, + username, + ); + expect(result).toEqual( + "http://5000--my-agent--my-workspace--my-username.proxy-host.tld/api", + ); + }); + + it("rewrites 0.0.0.0", () => { + const result = rewriteLocalhostURL( + "http://0.0.0.0:8080/", + proxyHost, + agent, + workspace, + username, + ); + expect(result).toEqual( + "http://8080--my-agent--my-workspace--my-username.proxy-host.tld/", + ); + }); + + it("preserves query params", () => { + const result = rewriteLocalhostURL( + "http://localhost:3000/path?key=value", + proxyHost, + agent, + workspace, + username, + ); + expect(result).toEqual( + "http://3000--my-agent--my-workspace--my-username.proxy-host.tld/path?key=value", + ); + }); + + it("returns non-localhost URLs unchanged", () => { + const url = "https://example.com/page"; + expect( + rewriteLocalhostURL(url, proxyHost, agent, workspace, username), + ).toBe(url); + }); + + it("returns invalid URLs unchanged", () => { + const url = "not-a-url"; + expect( + rewriteLocalhostURL(url, proxyHost, agent, workspace, username), + ).toBe(url); + }); +}); diff --git a/site/src/utils/portForward.test.ts b/site/src/utils/portForward.test.ts deleted file mode 100644 index 65c05da9ec..0000000000 --- a/site/src/utils/portForward.test.ts +++ /dev/null @@ -1,67 +0,0 @@ -import { portForwardURL } from "./portForward"; - -describe("port forward URL", () => { - const proxyHostWildcard = "*.proxy-host.tld"; - const samplePort = 12345; - const sampleAgent = "my-agent"; - const sampleWorkspace = "my-workspace"; - const sampleUsername = "my-username"; - - it("https, host and port", () => { - const forwarded = portForwardURL( - proxyHostWildcard, - samplePort, - sampleAgent, - sampleWorkspace, - sampleUsername, - "https", - ); - expect(forwarded).toEqual( - "http://12345s--my-agent--my-workspace--my-username.proxy-host.tld/", - ); - }); - it("http, host, port and path", () => { - const forwarded = portForwardURL( - proxyHostWildcard, - samplePort, - sampleAgent, - sampleWorkspace, - sampleUsername, - "http", - "/path1/path2", - ); - expect(forwarded).toEqual( - "http://12345--my-agent--my-workspace--my-username.proxy-host.tld/path1/path2", - ); - }); - it("https, host, port, path and empty params", () => { - const forwarded = portForwardURL( - proxyHostWildcard, - samplePort, - sampleAgent, - sampleWorkspace, - sampleUsername, - "https", - "/path1/path2", - "?", - ); - expect(forwarded).toEqual( - "http://12345s--my-agent--my-workspace--my-username.proxy-host.tld/path1/path2?", - ); - }); - it("http, host, port, path and query params", () => { - const forwarded = portForwardURL( - proxyHostWildcard, - samplePort, - sampleAgent, - sampleWorkspace, - sampleUsername, - "http", - "/path1/path2", - "?key1=value1&key2=value2", - ); - expect(forwarded).toEqual( - "http://12345--my-agent--my-workspace--my-username.proxy-host.tld/path1/path2?key1=value1&key2=value2", - ); - }); -}); diff --git a/site/src/utils/portForward.ts b/site/src/utils/portForward.ts index 78dae8c154..b682a67fbd 100644 --- a/site/src/utils/portForward.ts +++ b/site/src/utils/portForward.ts @@ -1,5 +1,27 @@ import type { WorkspaceAgentPortShareProtocol } from "api/typesGenerated"; +const localHosts = new Set(["localhost", "127.0.0.1", "0.0.0.0"]); + +/** + * Parse a port string from a URL, falling back to the protocol default + * (80 for http, 443 for https) when the port is empty (i.e. not + * specified). + * + * @param portStr - The port string from `URL.port` (empty when the URL + * uses the protocol's default port). + * @param protocol - The protocol string from `URL.protocol`, which + * always includes a trailing colon (e.g. `"https:"`). + */ +export const resolveLocalhostPort = ( + portStr: string, + protocol: string, +): number => { + if (portStr) { + return Number.parseInt(portStr, 10); + } + return protocol === "https:" ? 443 : 80; +}; + export const portForwardURL = ( host: string, port: number, @@ -26,6 +48,42 @@ export const portForwardURL = ( return url.toString(); }; +/** + * Rewrite a localhost URL to use the workspace port-forward subdomain. + * Returns the original URL unchanged when it is not a recognized + * localhost address or when parsing fails. + */ +export const rewriteLocalhostURL = ( + url: string, + proxyHost: string, + agentName: string, + workspaceName: string, + username: string, +): string => { + try { + const parsed = new URL(url); + if (!localHosts.has(parsed.hostname)) { + return url; + } + const protocol = parsed.protocol.replace( + ":", + "", + ) as WorkspaceAgentPortShareProtocol; + return portForwardURL( + proxyHost, + resolveLocalhostPort(parsed.port, parsed.protocol), + agentName, + workspaceName, + username, + protocol, + parsed.pathname, + parsed.search, + ); + } catch { + return url; + } +}; + // openMaybePortForwardedURL tries to open the provided URI through the // port-forwarded URL if it is localhost, otherwise opens it normally. export const openMaybePortForwardedURL = ( @@ -55,28 +113,7 @@ export const openMaybePortForwardedURL = ( return; } - try { - const url = new URL(uri); - const localHosts = ["0.0.0.0", "127.0.0.1", "localhost"]; - if (!localHosts.includes(url.hostname)) { - open(uri); - return; - } - open( - portForwardURL( - proxyHost, - Number.parseInt(url.port, 10), - agentName, - workspaceName, - username, - url.protocol.replace(":", "") as WorkspaceAgentPortShareProtocol, - url.pathname, - url.search, - ), - ); - } catch (_ex) { - open(uri); - } + open(rewriteLocalhostURL(uri, proxyHost, agentName, workspaceName, username)); }; export const saveWorkspaceListeningPortsProtocol = (