diff --git a/.pr/evidence/01_home.png b/.pr/evidence/01_home.png new file mode 100644 index 0000000000..11400dec08 Binary files /dev/null and b/.pr/evidence/01_home.png differ diff --git a/.pr/evidence/02_after_reload.png b/.pr/evidence/02_after_reload.png new file mode 100644 index 0000000000..2b7da8ccc5 Binary files /dev/null and b/.pr/evidence/02_after_reload.png differ diff --git a/.pr/evidence/03_conversation.png b/.pr/evidence/03_conversation.png new file mode 100644 index 0000000000..afb36bb689 Binary files /dev/null and b/.pr/evidence/03_conversation.png differ diff --git a/.pr/evidence/04_negative_rejection.png b/.pr/evidence/04_negative_rejection.png new file mode 100644 index 0000000000..2e867eac28 Binary files /dev/null and b/.pr/evidence/04_negative_rejection.png differ diff --git a/.pr/evidence/05_reply.png b/.pr/evidence/05_reply.png new file mode 100644 index 0000000000..a9a796100d Binary files /dev/null and b/.pr/evidence/05_reply.png differ diff --git a/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx b/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx index 850263e25e..0acab01a61 100644 --- a/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx +++ b/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx @@ -16,6 +16,7 @@ import { ConversationCard } from "#/components/features/conversation-panel/conve import { clickOnEditButton } from "./utils"; import { ConversationCardActions } from "#/components/features/conversation-panel/conversation-card/conversation-card-actions"; import { V1SandboxStatus } from "#/api/sandbox-service/sandbox-service.types"; +import ConversationService from "#/api/conversation-service/conversation-service.api"; vi.mock("react-i18next", async () => { const actual = await vi.importActual("react-i18next"); @@ -352,6 +353,69 @@ describe("ConversationCard", () => { expect(onClick).not.toHaveBeenCalled(); }); + it("opens the VS Code URL without an opener reference", async () => { + const user = userEvent.setup(); + const vscodeUrl = "https://vscode.example.dev/?tkn=abc&folder=workspace"; + vi.spyOn(ConversationService, "getVSCodeUrl").mockResolvedValue({ + vscode_url: vscodeUrl, + }); + renderWithProviders( + , + ); + + await user.click( + within(screen.getByTestId("context-menu")).getByTestId( + "download-vscode-button", + ), + ); + + await vi.waitFor(() => + expect(window.open).toHaveBeenCalledWith( + vscodeUrl, + "_blank", + "noopener,noreferrer", + ), + ); + }); + + it("does not open a non-http(s) VS Code URL", async () => { + const user = userEvent.setup(); + const getVSCodeUrl = vi + .spyOn(ConversationService, "getVSCodeUrl") + // eslint-disable-next-line no-script-url + .mockResolvedValue({ vscode_url: "javascript:alert(1)" }); + renderWithProviders( + , + ); + + await user.click( + within(screen.getByTestId("context-menu")).getByTestId( + "download-vscode-button", + ), + ); + + await vi.waitFor(() => expect(getVSCodeUrl).toHaveBeenCalledWith("conv-1")); + expect(window.open).not.toHaveBeenCalled(); + }); + it("should not display the edit or delete options if the handler is not provided", async () => { const onContextMenuToggle = vi.fn(); const { rerender } = renderWithProviders( @@ -486,7 +550,7 @@ describe("ConversationCard", () => { (sandboxStatus, shouldShow) => { renderWithProviders( ({ + useConversationId: () => ({ conversationId: "conv-1" }), +})); +vi.mock("#/hooks/use-runtime-is-ready", () => ({ + useRuntimeIsReady: () => true, +})); +vi.mock("#/hooks/query/use-batch-app-conversations"); +vi.mock("#/hooks/query/use-batch-sandboxes"); + +const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + function Wrapper({ children }: { children: React.ReactNode }) { + return ( + {children} + ); + } + return Wrapper; +}; + +function mockSandboxVSCodeUrl(url: string) { + vi.mocked(useBatchAppConversations).mockReturnValue({ + data: [{ sandbox_id: "sb-1" }], + isLoading: false, + } as unknown as ReturnType); + vi.mocked(useBatchSandboxes).mockReturnValue({ + data: [{ id: "sb-1", exposed_urls: [{ name: "VSCODE", url }] }], + isLoading: false, + } as unknown as ReturnType); +} + +describe("useUnifiedVSCodeUrl", () => { + afterEach(() => { + vi.resetAllMocks(); + }); + + it("returns an http(s) sandbox VS Code URL", async () => { + mockSandboxVSCodeUrl("https://vscode.example.dev/?tkn=abc"); + + const { result } = renderHook(() => useUnifiedVSCodeUrl(), { + wrapper: createWrapper(), + }); + + await waitFor(() => + expect(result.current.data).toEqual({ + url: "https://vscode.example.dev/?tkn=abc", + error: null, + }), + ); + }); + + it.each([ + // eslint-disable-next-line no-script-url + "javascript:alert(1)", + "data:text/html,", + "not-a-valid-url", + ])("reports %s as not available", async (url) => { + mockSandboxVSCodeUrl(url); + + const { result } = renderHook(() => useUnifiedVSCodeUrl(), { + wrapper: createWrapper(), + }); + + await waitFor(() => + expect(result.current.data).toEqual({ + url: null, + error: I18nKey.VSCODE$URL_NOT_AVAILABLE, + }), + ); + }); +}); diff --git a/frontend/__tests__/routes/accept-tos.test.tsx b/frontend/__tests__/routes/accept-tos.test.tsx index 27b5fb130f..1329660270 100644 --- a/frontend/__tests__/routes/accept-tos.test.tsx +++ b/frontend/__tests__/routes/accept-tos.test.tsx @@ -83,7 +83,12 @@ describe("AcceptTOS", () => { beforeEach(() => { sessionStorage.clear(); useSelectedOrganizationStore.setState({ organizationId: "test-org-id" }); - vi.stubGlobal("location", { href: "" }); + vi.stubGlobal("location", { + href: "", + origin: "http://localhost", + pathname: "/accept-tos", + search: "", + }); }); afterEach(() => { diff --git a/frontend/__tests__/utils/vscode-url-helper.test.ts b/frontend/__tests__/utils/vscode-url-helper.test.ts index a55b03bbbf..f181564321 100644 --- a/frontend/__tests__/utils/vscode-url-helper.test.ts +++ b/frontend/__tests__/utils/vscode-url-helper.test.ts @@ -56,6 +56,16 @@ describe("transformVSCodeUrl", () => { it("should handle invalid URLs gracefully", () => { const input = "not-a-valid-url"; - expect(transformVSCodeUrl(input)).toBe(input); + expect(transformVSCodeUrl(input)).toBeNull(); + }); + + it.each([ + "not-a-valid-url", + // eslint-disable-next-line no-script-url + "javascript:alert(1)", + "data:text/html,", + "file:///etc/passwd", + ])("should reject %s", (input) => { + expect(transformVSCodeUrl(input)).toBeNull(); }); }); diff --git a/frontend/src/components/features/conversation-panel/conversation-card/conversation-card.tsx b/frontend/src/components/features/conversation-panel/conversation-card/conversation-card.tsx index ee4474cc59..3dcb3fba59 100644 --- a/frontend/src/components/features/conversation-panel/conversation-card/conversation-card.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-card/conversation-card.tsx @@ -90,7 +90,7 @@ export function ConversationCard({ if (data.vscode_url) { const transformedUrl = transformVSCodeUrl(data.vscode_url); if (transformedUrl) { - window.open(transformedUrl, "_blank"); + window.open(transformedUrl, "_blank", "noopener,noreferrer"); } } // VS Code URL not available diff --git a/frontend/src/hooks/query/use-unified-vscode-url.ts b/frontend/src/hooks/query/use-unified-vscode-url.ts index ddc9be3783..00123caffe 100644 --- a/frontend/src/hooks/query/use-unified-vscode-url.ts +++ b/frontend/src/hooks/query/use-unified-vscode-url.ts @@ -59,10 +59,15 @@ export const useUnifiedVSCodeUrl = () => { }; } - return { - url: transformVSCodeUrl(vscodeUrl.url), - error: null, - }; + const url = transformVSCodeUrl(vscodeUrl.url); + if (!url) { + return { + url: null, + error: I18nKey.VSCODE$URL_NOT_AVAILABLE, + }; + } + + return { url, error: null }; }, enabled: runtimeIsReady && !!conversationId && !!sandboxesQuery.data, refetchOnMount: true, diff --git a/frontend/src/utils/vscode-url-helper.ts b/frontend/src/utils/vscode-url-helper.ts index 21f32a66a8..b2c75d4121 100644 --- a/frontend/src/utils/vscode-url-helper.ts +++ b/frontend/src/utils/vscode-url-helper.ts @@ -1,31 +1,37 @@ +const ALLOWED_PROTOCOLS = new Set(["http:", "https:"]); + /** * Helper function to transform VS Code URLs * * This function checks if a VS Code URL points to localhost and replaces it with - * the current window's hostname if they don't match. + * the current window's hostname if they don't match. URLs that do not parse or + * are not http(s) are rejected, since the value comes from the backend and is + * opened in a new tab. * * @param vsCodeUrl The original VS Code URL from the backend - * @returns The transformed URL with the correct hostname + * @returns The transformed URL with the correct hostname, or null if rejected */ export function transformVSCodeUrl(vsCodeUrl: string | null): string | null { if (!vsCodeUrl) return null; + let url: URL; try { - const url = new URL(vsCodeUrl); + url = new URL(vsCodeUrl); + } catch { + return null; + } - // Check if the URL points to localhost - if ( - url.hostname === "localhost" && - window.location.hostname !== "localhost" - ) { - // Replace localhost with the current hostname - url.hostname = window.location.hostname; - return url.toString(); - } + if (!ALLOWED_PROTOCOLS.has(url.protocol)) return null; - return vsCodeUrl; - } catch { - // Silently handle the error and return the original URL - return vsCodeUrl; + // Check if the URL points to localhost + if ( + url.hostname === "localhost" && + window.location.hostname !== "localhost" + ) { + // Replace localhost with the current hostname + url.hostname = window.location.hostname; + return url.toString(); } + + return vsCodeUrl; }