Fence Computer viewer responses and scrub persisted tab payloads

Co-Authored-By: Paperclip <noreply@paperclip.ing>

ui/src/components/task-side-panel/TaskComputerPanel.test.tsx

@@ -0,0 +1,119 @@
+// @vitest-environment jsdom
+import { act } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { computersApi } from "@/api/computers";
+import { TaskComputerPanel } from "./TaskComputerPanel";
+
+vi.mock("@/api/computers", () => ({ computersApi: {
+  connect: vi.fn(), presence: vi.fn(), disconnect: vi.fn(async () => {}), preview: vi.fn(),
+} }));
+(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+const owner = { computerId: "computer", ownerId: "viewer", generation: 1 };
+const now = new Date("2026-10-10T12:00:00Z");
+const viewer = () => ({ viewerUrl: "https://viewer.example/#private-token", expiresAt: new Date(now.getTime() + 120_000).toISOString(), owner });
+function deferred<T>() {
+  let resolve!: (value: T) => void;
+  const promise = new Promise<T>(done => { resolve = done; });
+  return { promise, resolve };
+}
+let root: Root, host: HTMLDivElement, hidden = false, paneVisible = true, unmounted = false;
+async function render(props: { issueId?: string; environmentId?: string; active?: boolean } = {}) {
+  await act(async () => root.render(<TaskComputerPanel issueId={props.issueId ?? "task-a"} environmentId={props.environmentId ?? "environment-a"} active={props.active ?? true} />));
+}
+async function connect() {
+  const button = Array.from(host.querySelectorAll("button")).find(item => item.textContent === "Connect");
+  expect(button).toBeTruthy();
+  await act(async () => button!.click());
+}
+async function tick(ms: number) { await act(async () => { await vi.advanceTimersByTimeAsync(ms); }); }
+async function unmount() { await act(async () => root.unmount()); unmounted = true; }
+beforeEach(() => {
+  vi.useFakeTimers(); vi.setSystemTime(now); vi.clearAllMocks(); hidden = false; paneVisible = true; unmounted = false;
+  vi.mocked(computersApi.connect).mockResolvedValue(viewer());
+  vi.mocked(computersApi.presence).mockImplementation(async () => viewer());
+  vi.mocked(computersApi.disconnect).mockResolvedValue(undefined);
+  Object.defineProperty(document, "visibilityState", { configurable: true, get: () => hidden ? "hidden" : "visible" });
+  vi.spyOn(HTMLElement.prototype, "getClientRects").mockReturnValue([{}] as unknown as DOMRectList);
+  Object.defineProperty(Element.prototype, "checkVisibility", { configurable: true, value: () => paneVisible });
+  host = document.createElement("div"); document.body.append(host); root = createRoot(host);
+});
+afterEach(async () => { if (!unmounted) await unmount(); host.remove(); delete (Element.prototype as { checkVisibility?: unknown }).checkVisibility; vi.restoreAllMocks(); vi.useRealTimers(); });
+
+describe("Computer connection lifetime", () => {
+  it("connects only after the explicit action and keeps credentials in the iframe with no referrer", async () => {
+    await render(); await tick(60_000);
+    expect(computersApi.connect).not.toHaveBeenCalled();
+    expect(computersApi.presence).not.toHaveBeenCalled();
+    expect(host.querySelector("iframe")).toBeNull();
+    await connect();
+    expect(computersApi.connect).toHaveBeenCalledWith("task-a", "environment-a");
+    const iframe = host.querySelector("iframe")!;
+    expect(iframe.src).toBe(viewer().viewerUrl);
+    expect(iframe.getAttribute("referrerpolicy")).toBe("no-referrer");
+  });
+
+  it("does not renew while inactive or hidden and expires the stale credential without another connect", async () => {
+    await render(); await connect();
+    const renewals = vi.mocked(computersApi.presence).mock.calls.length;
+    await render({ active: false }); await tick(30_000);
+    expect(computersApi.presence).toHaveBeenCalledTimes(renewals);
+    hidden = true;
+    await render({ active: true }); await tick(30_000);
+    expect(computersApi.presence).toHaveBeenCalledTimes(renewals);
+    await tick(60_000);
+    expect(host.querySelector("iframe")).toBeNull();
+    expect(host.textContent).toContain("connection expired");
+    expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner);
+    expect(computersApi.connect).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not keep the computer alive while its selected pane is collapsed", async () => {
+    await render(); await connect();
+    const renewals = vi.mocked(computersApi.presence).mock.calls.length;
+    paneVisible = false;
+    await tick(30_000);
+    expect(computersApi.presence).toHaveBeenCalledTimes(renewals);
+    paneVisible = true;
+    await tick(30_000);
+    expect(computersApi.presence).toHaveBeenCalledTimes(renewals + 1);
+  });
+
+  it("revalidates when returning to a visible tab and clears an expired server hold", async () => {
+    await render(); await connect(); await render({ active: false });
+    vi.mocked(computersApi.presence).mockRejectedValueOnce(new Error("Connect again"));
+    await render({ active: true });
+    expect(host.querySelector("iframe")).toBeNull();
+    expect(host.textContent).toContain("Connect again");
+    expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner);
+  });
+
+  it.each(["issue", "environment"])("retires a late connect for an old %s without showing its desktop", async change => {
+    const pending = deferred<ReturnType<typeof viewer>>();
+    vi.mocked(computersApi.connect).mockReturnValueOnce(pending.promise);
+    await render(); await connect();
+    expect(host.textContent).toContain("Connecting");
+    await render(change === "issue" ? { issueId: "task-b" } : { environmentId: "environment-b" });
+    expect(host.textContent).not.toContain("Connecting");
+    await act(async () => pending.resolve(viewer()));
+    expect(host.querySelector("iframe")).toBeNull();
+    expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner);
+  });
+
+  it("retires connections resolved after unmount", async () => {
+    const pending = deferred<ReturnType<typeof viewer>>();
+    vi.mocked(computersApi.connect).mockReturnValueOnce(pending.promise);
+    await render(); await connect(); await unmount();
+    await act(async () => pending.resolve(viewer()));
+    expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner);
+  });
+
+  it("retires a connected owner on unmount even when its presence response is pending", async () => {
+    const pending = deferred<ReturnType<typeof viewer>>();
+    vi.mocked(computersApi.presence).mockReturnValueOnce(pending.promise);
+    await render(); await connect(); await unmount();
+    expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner);
+    await act(async () => pending.resolve(viewer()));
+    expect(host.querySelector("iframe")).toBeNull();
+  });
+});

ui/src/components/task-side-panel/TaskComputerPanel.tsx

@@ -5,16 +5,22 @@ import { computersApi } from "@/api/computers";
 import { Button } from "@/components/ui/button";
 import { Input } from "@/components/ui/input";
 
-export function TaskComputerPanel({ issueId, environmentId, active = true }: {
-  issueId: string;
-  environmentId: string;
-  active?: boolean;
-}) {
+type ComputerPanelProps = { issueId: string; environmentId: string; active?: boolean };
+
+export function TaskComputerPanel(props: ComputerPanelProps) {
+  // A connection belongs to one task/environment. A new scope gets new state,
+  // so a late response can only retire its old owner, never install its viewer.
+  return <ComputerConnection key={`${props.issueId}:${props.environmentId}`} {...props} />;
+}
+
+function ComputerConnection({ issueId, environmentId, active = true }: ComputerPanelProps) {
   const [viewer, setViewer] = useState<ComputerViewer | null>(null);
   const [connecting, setConnecting] = useState(false);
   const [error, setError] = useState<string | null>(null);
   const current = useRef<ComputerViewer | null>(null);
+  const frameRef = useRef<HTMLIFrameElement | null>(null);
   const mounted = useRef(true);
+  const connectPending = useRef(false);
   const [port, setPort] = useState("5173");
   const [previewPending, setPreviewPending] = useState(false);
 
@@ -26,11 +32,18 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: {
     setError(null);
     try {
       const result = await computersApi.preview(issueId, environmentId, Number(port));
-      previewWindow.location.replace(result.url);
+      if (mounted.current) previewWindow.location.replace(result.url);
+      else previewWindow.close();
     } catch (cause) {
       previewWindow.close();
-      setError(cause instanceof Error ? cause.message : "Could not open the preview.");
-    } finally { setPreviewPending(false); }
+      if (mounted.current) setError(cause instanceof Error ? cause.message : "Could not open the preview.");
+    } finally { if (mounted.current) setPreviewPending(false); }
+  }
+
+  function release(view: ComputerViewer) {
+    void computersApi.disconnect(issueId, environmentId, view.owner).catch(() => {
+      // The server's bounded presence deadline also releases lost viewers.
+    });
   }
 
   useEffect(() => {
@@ -39,20 +52,19 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: {
       mounted.current = false;
       const previous = current.current;
       current.current = null;
-      if (previous) void computersApi.disconnect(issueId, environmentId, previous.owner).catch(() => {
-        // The server's bounded presence deadline also releases lost viewers.
-      });
+      if (previous) release(previous);
     };
   }, [issueId, environmentId]);
 
   async function connect() {
-    if (connecting) return;
+    if (connectPending.current) return;
+    connectPending.current = true;
     setConnecting(true);
     setError(null);
     try {
       const result = await computersApi.connect(issueId, environmentId);
       if (!mounted.current) {
-        await computersApi.disconnect(issueId, environmentId, result.owner);
+        release(result);
         return;
       }
       current.current = result;
@@ -60,6 +72,7 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: {
     } catch (cause) {
       if (mounted.current) setError(cause instanceof Error ? cause.message : "Could not connect to the computer.");
     } finally {
+      connectPending.current = false;
       if (mounted.current) setConnecting(false);
     }
   }
@@ -69,24 +82,34 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: {
     let disposed = false;
     let pending = false;
     const renew = async () => {
-      if (pending || !active || document.visibilityState === "hidden") return;
+      const frame = frameRef.current;
+      if (pending || !active || document.visibilityState === "hidden" || !frame || !frame.getClientRects().length ||
+        (frame.checkVisibility && !frame.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }))) return;
       pending = true;
       try {
         const next = await computersApi.presence(issueId, environmentId, viewer.owner);
-        if (!disposed) {
-          current.current = next;
-          setViewer(next);
+        if (disposed || !mounted.current) {
+          // It may have renewed the old owner while this pane was closing.
+          if (current.current?.owner.ownerId !== next.owner.ownerId) release(next);
+          return;
         }
+        current.current = next;
+        setViewer(next);
       } catch (cause) {
         if (!disposed) {
+          const previous = current.current;
           current.current = null;
           setViewer(null);
+          if (previous) release(previous);
           setError(cause instanceof Error ? cause.message : "The connection ended. Connect to try again.");
         }
       } finally { pending = false; }
     };
     const timer = setInterval(() => void renew(), 30_000);
     document.addEventListener("visibilitychange", renew);
+    // A hidden pane may have outlived its server hold. Revalidate immediately
+    // when it becomes active, before waiting for the next periodic renewal.
+    void renew();
     return () => {
       disposed = true;
       clearInterval(timer);
@@ -94,7 +117,21 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: {
     };
   }, [issueId, environmentId, viewer?.owner.ownerId, active]);
 
+  useEffect(() => {
+    if (!viewer) return;
+    const expiresAt = Date.parse(viewer.expiresAt);
+    const timer = setTimeout(() => {
+      if (current.current !== viewer) return;
+      current.current = null;
+      setViewer(null);
+      setError("The connection expired. Connect to try again.");
+      release(viewer);
+    }, Math.max(0, Number.isFinite(expiresAt) ? expiresAt - Date.now() : 0));
+    return () => clearTimeout(timer);
+  }, [viewer]);
+
   const desktop = viewer ? <iframe
+    ref={frameRef}
     title="Agent computer"
     src={viewer.viewerUrl}
     className="h-full w-full border-0"

ui/src/lib/task-side-panel-state.test.ts

@@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it } from "vitest";
 import {
   readTaskSidePanelState,
   taskPanelDocumentTab,
+  taskPanelComputerTab,
   taskPanelFilesTab,
   taskPanelPropertiesTab,
   taskPanelSkillTab,
@@ -16,6 +17,31 @@ import {
 describe("task side-panel persistence", () => {
   beforeEach(() => window.localStorage.clear());
 
+  it("persists only computer identity and removes accidental credentials before writing storage", () => {
+    const environmentId = "12345678-1234-1234-1234-123456789012";
+    const tab = taskPanelComputerTab(environmentId);
+    const contaminated = { ...tab, viewerUrl: "https://viewer.example/#private-top-token",
+      payload: { ...tab.payload, viewerUrl: "https://viewer.example/#private-payload-token", owner: { ownerId: "private-owner" }, secretHeaders: { Cookie: "private-cookie" } } };
+    writeTaskSidePanelState("user-1", "company-1", "computer-task", {
+      state: { tabs: [contaminated], activeTabId: tab.id }, launcherOpen: false, userInteracted: true, autoPlanHandled: true, updatedAt: 1,
+    });
+    const stored = window.localStorage.getItem(window.localStorage.key(0)!)!;
+    expect(stored).not.toContain("private-");
+    expect(stored).not.toContain("viewerUrl");
+    expect(stored).not.toContain("secretHeaders");
+    expect(readTaskSidePanelState("user-1", "company-1", "computer-task", true)?.state.tabs[0]?.payload)
+      .toEqual({ kind: "computer", environmentId });
+
+    // A later write also scrubs unsupported fields from retained task entries.
+    const parsed = JSON.parse(stored);
+    parsed.tasks["computer-task"].state.tabs[0].payload.viewerUrl = "https://viewer.example/#private-old-token";
+    window.localStorage.setItem(window.localStorage.key(0)!, JSON.stringify(parsed));
+    writeTaskSidePanelState("user-1", "company-1", "other-task", {
+      state: { tabs: [taskPanelPropertiesTab()], activeTabId: "properties" }, launcherOpen: false, userInteracted: false, autoPlanHandled: false, updatedAt: 2,
+    });
+    expect(window.localStorage.getItem(window.localStorage.key(0)!)).not.toContain("private-old-token");
+  });
+
   it("round-trips an intentionally empty task state", () => {
     writeTaskSidePanelState("user-1", "company-1", "task-1", {
       state: { tabs: [], activeTabId: null },

ui/src/lib/task-side-panel-state.ts

@@ -209,7 +209,14 @@ export function writeTaskSidePanelState(
   if (typeof window === "undefined") return;
   try {
     const store = readStore(accountScope, companyId);
-    const tasks = { ...store.tasks, [taskId]: entry };
+    // Persist only the tab contract, including when a caller accidentally mixes
+    // ephemeral viewer credentials into a payload. Sanitize retained old tasks
+    // too, so a later write removes unsupported fields already in storage.
+    const tasks = Object.fromEntries(Object.entries({ ...store.tasks, [taskId]: entry })
+      .flatMap(([id, value]) => {
+        const parsed = parseEntry(value, true);
+        return parsed ? [[id, parsed]] : [];
+      }));
     const retained = Object.entries(tasks)
       .sort((left, right) => (right[1].updatedAt ?? 0) - (left[1].updatedAt ?? 0))
       .slice(0, MAX_TASK_STATES);