fix(conversation-card): open VS Code URL with noopener and http(s) only

.pr/evidence/bundle-proof.md

@@ -0,0 +1,95 @@
+# Why the preview verifier sees pre-fix code (and why this PR's fix is correct)
+
+The preview verifier inspected the **`/canvas`** bundle and found pre-fix code
+(`window.open(t,"_blank")` with no `noopener`, no protocol allow-list). That
+bundle is the **Agent Canvas SPA**, built from the upstream OSS repo
+`OpenHands/OpenHands` at pinned tag **`v1.35.0`** (see
+`frontend/scripts/build-agent-canvas.sh`, `CANVAS_REF=v1.35.0`). It is a
+**separate frontend** from the one this PR edits.
+
+OSS PR [OpenHands/OpenHands#19229](https://github.com/OpenHands/OpenHands/pull/19229)
+is merged to OSS `main` (merge commit `0474345a8c8fcd1a9964e66bac6812cf7ac4ba5f`)
+but is **not in the `v1.35.0` tag** yet, so the served `/canvas` bundle correctly
+still has pre-fix code. The issue explicitly states this is out of scope:
+*"The OSS PR is not in a tagged release yet; this OHE change does not need the
+pin bump."* and P10 *"this is the legacy OHE conversation card. The bundle bump
+does not reach it."*
+
+## What this PR actually changes
+
+The **legacy OHE conversation card**, served from this repo's own `frontend/`
+build at `/` (the `./frontend/build` SPA mounted by `saas_server.py` /
+`openhands/app_server/app.py`). The changed files are:
+
+- `frontend/src/utils/vscode-url-helper.ts`
+- `frontend/src/components/features/conversation-panel/conversation-card/conversation-card.tsx`
+
+These are compiled into the OHE SPA chunk `conversation-*.js` (e.g.
+`conversation-Doant9s6.js`), NOT into the `/canvas` bundle.
+
+## Proof the fix is in the served OHE bundle
+
+Built the OHE frontend (`cd frontend && npm run build`) and grepped the output
+chunks in `frontend/build/assets/`:
+
+### 1. Protocol allow-list is present in the card chunk
+```
+$ grep -oE 'new Set\(\["http:","https:"\]\)' build/assets/conversation-Doant9s6.js
+new Set(["http:","https:"])
+```
+
+### 2. window.open uses noopener,noreferrer in the card chunk
+```
+$ grep -oE 'window\.open\([^)]*noopener,noreferrer[^)]*\)' build/assets/conversation-Doant9s6.js
+window.open(l,"_blank","noopener,noreferrer")
+```
+
+### 3. The OHE root-layout chunk does NOT contain the sync `e.vscode_url` handler
+The verifier reported `root-layout-*.js` with `if(e.vscode_url){let t=st(e.vscode_url);t&&window.open(t,"_blank")}`.
+That is the **Canvas** bundle's root-layout, not OHE's. The OHE root-layout
+chunk contains zero `vscode_url` references:
+```
+$ grep -c 'vscode_url' build/assets/root-layout-e7qLxVhU.js
+0
+```
+The OHE card fetches the URL asynchronously (`await ConversationService.getVSCodeUrl(...)`),
+which is why the served Canvas code (sync `e.vscode_url`) does not match this
+PR's source — they are two different components.
+
+## Why the verifier could not reach the OHE card in the preview
+
+The OHE SPA redirects `/` → `/canvas` (`frontend/src/routes/root-landing.tsx`,
+`window.location.replace(...)`). The preview login lands on `/` and is
+redirected to Canvas, so the verifier never opened the OHE sidebar. The OHE
+conversation panel (and thus `ConversationCard`) is reachable on OHE routes
+that render the sidebar (e.g. `/settings`, `/conversations/:id`), via the
+`ConversationPanelButton` in `frontend/src/components/features/sidebar/sidebar.tsx`.
+**Correction:** the sidebar panel renders `ConversationCard` without
+`showOptions`, so the card's "Download via VS Code" item is never shown there
+(`conversation-card-actions.tsx`: `conversationId && showOptions ? ... : undefined`).
+The UI-reachable path for this change is the VS Code tab on
+`/conversations/:id`, which runs the same `transformVSCodeUrl` via
+`useUnifiedVSCodeUrl`. See "How to verify on a preview" in the PR description.
+
+## Verification surface
+
+This is a security hardening of a URL helper + a single `window.open` call
+site, with no new UI. It is covered by unit tests, which pass:
+
+```
+cd frontend && npx vitest run __tests__/utils/vscode-url-helper.test.ts --no-coverage
+Test Files  1 passed (1)
+Tests       9 passed (9)
+```
+
+The tests assert every acceptance criterion in the issue:
+- `javascript:`, `data:`, `file:` and unparsable URLs → `null` (not opened).
+- `http(s)` URLs pass through; the `localhost → current hostname` rewrite still applies.
+- The card call site opens with `"noopener,noreferrer"` (proven in the built bundle above).
+
+## Conclusion
+
+The FAIL was caused by inspecting the `/canvas` (OSS v1.35.0) bundle instead of
+the OHE legacy frontend bundle this PR modifies. The fix is correctly compiled
+into the OHE bundle served at `/`. No code change is required to address the
+FAIL; this file documents the evidence.

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";
 
 // We'll use the actual i18next implementation but override the translation function
 
@@ -359,6 +360,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(
+      <ConversationCard
+        conversationId="conv-1"
+        onDelete={onDelete}
+        title="Conversation 1"
+        selectedRepository={null}
+        lastUpdatedAt="2021-10-01T12:00:00Z"
+        showOptions
+        contextMenuOpen
+        onContextMenuToggle={vi.fn()}
+      />,
+    );
+
+    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(
+      <ConversationCard
+        conversationId="conv-1"
+        onDelete={onDelete}
+        title="Conversation 1"
+        selectedRepository={null}
+        lastUpdatedAt="2021-10-01T12:00:00Z"
+        showOptions
+        contextMenuOpen
+        onContextMenuToggle={vi.fn()}
+      />,
+    );
+
+    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(
@@ -493,7 +557,7 @@ describe("ConversationCard", () => {
     (sandboxStatus, shouldShow) => {
       renderWithProviders(
         <ConversationCardActions
-          contextMenuOpen={true}
+          contextMenuOpen
           onContextMenuToggle={vi.fn()}
           onStop={vi.fn()}
           sandboxStatus={sandboxStatus}

frontend/__tests__/hooks/query/use-unified-vscode-url.test.tsx

@@ -0,0 +1,81 @@
+import { renderHook, waitFor } from "@testing-library/react";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import React from "react";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { useUnifiedVSCodeUrl } from "#/hooks/query/use-unified-vscode-url";
+import { useBatchAppConversations } from "#/hooks/query/use-batch-app-conversations";
+import { useBatchSandboxes } from "#/hooks/query/use-batch-sandboxes";
+import { I18nKey } from "#/i18n/declaration";
+
+vi.mock("#/hooks/use-conversation-id", () => ({
+  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 (
+      <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
+    );
+  }
+  return Wrapper;
+};
+
+function mockSandboxVSCodeUrl(url: string) {
+  vi.mocked(useBatchAppConversations).mockReturnValue({
+    data: [{ sandbox_id: "sb-1" }],
+    isLoading: false,
+  } as unknown as ReturnType<typeof useBatchAppConversations>);
+  vi.mocked(useBatchSandboxes).mockReturnValue({
+    data: [{ id: "sb-1", exposed_urls: [{ name: "VSCODE", url }] }],
+    isLoading: false,
+  } as unknown as ReturnType<typeof useBatchSandboxes>);
+}
+
+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,<script>alert(1)</script>",
+    "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,
+      }),
+    );
+  });
+});

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,<script>alert(1)</script>",
+    "file:///etc/passwd",
+  ])("should reject %s", (input) => {
+    expect(transformVSCodeUrl(input)).toBeNull();
   });
 });

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

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,

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;
 }