diff --git a/docs/plans/2026-03-29-fix-cross-origin-iframe-screenshot-test-plan.md b/docs/plans/2026-03-29-fix-cross-origin-iframe-screenshot-test-plan.md new file mode 100644 index 00000000..942401a9 --- /dev/null +++ b/docs/plans/2026-03-29-fix-cross-origin-iframe-screenshot-test-plan.md @@ -0,0 +1,211 @@ +# Test Plan: Fix Cross-Origin Iframe Screenshot + +**Implementation plan:** `docs/plans/2026-03-29-fix-cross-origin-iframe-screenshot.md` + +**Testing strategy (approved):** Strip iframe-blocking headers from proxy responses so proxied localhost content renders in browser pane iframes and the MCP screenshot tool captures real content instead of placeholders. + +**Strategy reconciliation notes:** +- The approved strategy names a Playwright e2e test as highest priority. The project has Playwright infrastructure in `test/e2e-browser/` with a running `TestServer` that boots a real Freshell instance. The existing `browser-pane.spec.ts` already exercises browser pane creation and URL loading. A new Playwright test can load a localhost URL through the proxy, then use the MCP agent API `screenshot-view` CLI command (or the `requestUiScreenshot` harness method) to verify the screenshot contains image content rather than a placeholder. This test exercises the entire fix end-to-end: proxy strips headers, iframe renders, screenshot captures real content. +- The implementation plan omits the Playwright e2e test entirely, covering only Vitest unit tests. This test plan adds it as test 1. +- The implementation plan omits the MCP instructions update verification. This test plan adds it as test 7 (a simple Grep-based assertion on the instructions text). +- The implementation plan's proxy-router unit tests and ui-screenshot unit test align with strategy items 2-4 and are included here as tests 3-6. +- No strategy changes requiring user approval: all tests use existing harnesses and infrastructure. + +--- + +## Harness requirements + +No new harnesses need to be built. All tests use existing infrastructure: + +1. **Playwright E2E harness** (`test/e2e-browser/helpers/fixtures.ts`): Boots a real Freshell production server via `TestServer`, provides `freshellPage` fixture with auth + WebSocket, `harness` for Redux state inspection, and `terminal` helper. The server includes the proxy router, so proxied URLs work out of the box. + +2. **Vitest + supertest** (`test/unit/server/proxy-router.test.ts`): Existing test file boots a target express server on an ephemeral port and sends requests through the proxy router via supertest. New tests add routes to the existing `targetApp` that return iframe-blocking headers. + +3. **Vitest + jsdom** (`test/unit/client/ui-screenshot.test.ts`): Existing test file mocks `html2canvas` and tests `captureUiScreenshot` with synthetic DOM. Same-origin iframes in jsdom allow `contentDocument` access, so proxy-URL iframe tests work without browser-level header stripping. + +--- + +## Test plan + +### Test 1: Browser pane screenshot captures proxied localhost content as image, not placeholder + +- **Name:** Screenshot of browser pane with proxied localhost URL produces image content, not a cross-origin placeholder +- **Type:** scenario +- **Disposition:** new +- **Harness:** Playwright E2E (`test/e2e-browser/specs/browser-pane-screenshot.spec.ts`) +- **Preconditions:** + - Freshell test server is running (production build, via `TestServer` fixture) + - A static HTTP server running on an ephemeral localhost port inside the test, serving a page with known canary text (e.g., `

SCREENSHOT_CANARY

`) + - A browser pane is open and navigated to the canary server's localhost URL + - The proxy has rewritten the URL to `/api/proxy/http//` +- **Actions:** + 1. Start a tiny HTTP server on localhost that returns a page with `X-Frame-Options: DENY` and `Content-Security-Policy: frame-ancestors 'none'` headers, plus a known canary `

` element. + 2. Create a browser pane via the Freshell UI (right-click terminal -> split -> Browser). + 3. Navigate the browser pane to `http://localhost:/`. + 4. Wait for the iframe to load (verify `iframe[title="Browser content"]` is attached and its `src` contains `/api/proxy/http/`). + 5. Take a Playwright screenshot of the page. + 6. Also invoke the MCP screenshot path: call the agent API `screenshot-view` endpoint (via `fetch` from the page context or the CLI) and inspect the response. +- **Expected outcome:** + - The Playwright screenshot does NOT show a placeholder div with "Iframe content is not directly capturable" text. Source of truth: the implementation plan states the proxy strips `X-Frame-Options` and `Content-Security-Policy`, so the iframe content renders normally. + - The agent API screenshot response has `ok: true` and `imageBase64` that is a valid PNG (starts with PNG signature bytes when decoded). Source of truth: the implementation plan states `captureIframeReplacement` succeeds when `contentDocument` is accessible. + - The page does NOT contain an element with `data-screenshot-iframe-placeholder="true"` during the screenshot capture. +- **Interactions:** Exercises proxy-router header stripping, BrowserPane URL resolution via `buildHttpProxyUrl`, iframe rendering, and the `captureUiScreenshot` → `captureIframeReplacement` → `html2canvas` chain. + +### Test 2: Browser pane screenshot falls back to placeholder for truly cross-origin URLs + +- **Name:** Screenshot of browser pane with external cross-origin URL gracefully shows placeholder with source URL +- **Type:** regression +- **Disposition:** extend (based on existing `browser-pane.spec.ts` patterns) +- **Harness:** Playwright E2E (`test/e2e-browser/specs/browser-pane-screenshot.spec.ts`) +- **Preconditions:** + - Freshell test server is running + - A browser pane is open and navigated to a truly cross-origin URL (e.g., `https://example.com`) +- **Actions:** + 1. Create a browser pane and navigate to `https://example.com`. + 2. Wait for the iframe to load. + 3. Take a Playwright screenshot to inspect the visual output. +- **Expected outcome:** + - The iframe shows a placeholder with the source URL text (since `https://example.com` is truly cross-origin and the proxy only handles localhost URLs). Source of truth: implementation plan states "No client-side changes needed" -- the existing placeholder behavior for truly cross-origin URLs is preserved. + - The page contains visible text matching `example.com`. +- **Interactions:** Exercises the graceful fallback in `captureIframeReplacement` when `contentDocument` is null. Confirms the fix does not break the existing placeholder behavior for non-proxied cross-origin content. + +### Test 3: Proxy strips X-Frame-Options header from responses + +- **Name:** Proxied response does not contain X-Frame-Options header regardless of upstream value +- **Type:** integration +- **Disposition:** new +- **Harness:** Vitest + supertest (`test/unit/server/proxy-router.test.ts`) +- **Preconditions:** + - Target express server has a route `/with-xfo` that returns `X-Frame-Options: DENY` + - Proxy app is configured with auth token +- **Actions:** + 1. `GET /api/proxy/http//with-xfo` with auth header +- **Expected outcome:** + - Response status is 200 + - Response body is `'framed content'` + - `res.headers['x-frame-options']` is `undefined` + - Source of truth: implementation plan Task 1 Step 1 specifies this exact test case. +- **Interactions:** Exercises the `stripIframeBlockingHeaders` function in the proxy response path. + +### Test 4: Proxy strips Content-Security-Policy header from responses + +- **Name:** Proxied response does not contain Content-Security-Policy header regardless of upstream value +- **Type:** integration +- **Disposition:** new +- **Harness:** Vitest + supertest (`test/unit/server/proxy-router.test.ts`) +- **Preconditions:** + - Target express server has a route `/with-csp` that returns `Content-Security-Policy: frame-ancestors 'none'; default-src 'self'` + - Proxy app is configured with auth token +- **Actions:** + 1. `GET /api/proxy/http//with-csp` with auth header +- **Expected outcome:** + - Response status is 200 + - Response body is `'csp content'` + - `res.headers['content-security-policy']` is `undefined` + - Source of truth: implementation plan Task 1 Step 1 and Design Decision on CSP removal. +- **Interactions:** Same as Test 3. + +### Test 5: Proxy strips both iframe-blocking headers simultaneously + +- **Name:** Proxied response strips X-Frame-Options and Content-Security-Policy when both are present +- **Type:** boundary +- **Disposition:** new +- **Harness:** Vitest + supertest (`test/unit/server/proxy-router.test.ts`) +- **Preconditions:** + - Target express server has a route `/with-both` that returns both `X-Frame-Options: SAMEORIGIN` and `Content-Security-Policy: frame-ancestors 'none'` + - Proxy app is configured with auth token +- **Actions:** + 1. `GET /api/proxy/http//with-both` with auth header +- **Expected outcome:** + - Response status is 200 + - Response body is `'both headers'` + - `res.headers['x-frame-options']` is `undefined` + - `res.headers['content-security-policy']` is `undefined` + - Source of truth: implementation plan Task 1 Step 1. +- **Interactions:** Same as Test 3, exercises the case where both headers coexist. + +### Test 6: Proxy preserves non-iframe-blocking headers + +- **Name:** Proxied response preserves custom and non-security headers that do not block iframe embedding +- **Type:** invariant +- **Disposition:** new +- **Harness:** Vitest + supertest (`test/unit/server/proxy-router.test.ts`) +- **Preconditions:** + - Target express server has a route `/no-frame-headers` that returns `X-Custom-Header: keep-me` + - Proxy app is configured with auth token +- **Actions:** + 1. `GET /api/proxy/http//no-frame-headers` with auth header +- **Expected outcome:** + - Response status is 200 + - Response body is `'no frame headers'` + - `res.headers['x-custom-header']` is `'keep-me'` + - Source of truth: implementation plan Design Decision "Strip only iframe-blocking headers, not all security headers." +- **Interactions:** Confirms the stripping logic is precise and does not overshoot by removing all headers. + +### Test 7: Proxy-URL iframe captured as image in ui-screenshot + +- **Name:** captureUiScreenshot replaces a proxy-URL iframe with an image element (not placeholder) when contentDocument is accessible +- **Type:** unit +- **Disposition:** new +- **Harness:** Vitest + jsdom (`test/unit/client/ui-screenshot.test.ts`) +- **Preconditions:** + - DOM contains `
` with an ` +
+ ` + const target = document.querySelector('[data-context="global"]') as HTMLElement + const iframe = document.getElementById('proxy-frame') as HTMLIFrameElement + setRect(target, 800, 500) + setRect(iframe, 500, 300) + + const iframeDoc = iframe.contentDocument + expect(iframeDoc).toBeTruthy() + iframeDoc?.open() + iframeDoc?.write('

Proxied localhost content

') + iframeDoc?.close() + + let clonedHtml = '' + vi.mocked(html2canvas).mockImplementation(async (_el: any, opts: any = {}) => { + if (typeof opts.onclone === 'function') { + const cloneDoc = document.implementation.createHTMLDocument('clone') + const cloneTarget = target.cloneNode(true) as HTMLElement + cloneDoc.body.appendChild(cloneTarget) + opts.onclone(cloneDoc) + clonedHtml = cloneTarget.innerHTML + return { + width: 800, + height: 500, + toDataURL: () => 'data:image/png;base64,PROXYPNG', + } as any + } + + return { + width: 500, + height: 300, + toDataURL: () => 'data:image/png;base64,IFRAMEPROXYPNG', + } as any + }) + + const result = await captureUiScreenshot({ scope: 'view' }, createRuntime() as any) + + expect(result.ok).toBe(true) + expect(result.imageBase64).toBe('PROXYPNG') + // The iframe should be replaced with an image, not a placeholder + expect(clonedHtml).toContain('data-screenshot-iframe-image="true"') + expect(clonedHtml).not.toContain('data-screenshot-iframe-placeholder') + expect(clonedHtml).not.toContain(' { - res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers) + const strippedHeaders = stripIframeBlockingHeaders(proxyRes.headers) + res.writeHead(proxyRes.statusCode ?? 502, strippedHeaders) proxyRes.pipe(res) }, ) diff --git a/src/components/panes/BrowserPane.tsx b/src/components/panes/BrowserPane.tsx index 85bf6c8c..823c22f3 100644 --- a/src/components/panes/BrowserPane.tsx +++ b/src/components/panes/BrowserPane.tsx @@ -95,29 +95,23 @@ function toIframeSrc(url: string): string { } /** - * Build an HTTP proxy URL for localhost URLs when the browser itself is on - * localhost. This handles WSL2/Docker where "localhost" traverses a networking - * layer that may not forward all ports — routing through Freshell's own - * (known-reachable) port avoids the problem. + * Build an HTTP proxy URL for localhost http: URLs. The proxy is same-origin + * with Freshell, which means: + * - Screenshots can access iframe content (no cross-origin restriction) + * - WSL2/Docker networking issues are bypassed (known-reachable port) * - * When the browser is remote, needsPortForward() handles it via TCP forward - * which works because the forwarded port is reachable on the server's actual IP. + * Works for both local and remote browsers since `/api/proxy/http/:port/` + * is always same-origin with the Freshell page. + * + * https: localhost URLs are NOT proxied — the HTTP proxy can't do TLS + * passthrough, so those fall through to TCP port forwarding for remote + * browsers or direct access for local browsers. */ function buildHttpProxyUrl(url: string): string | null { - // Only needed when the browser is on localhost — remote browsers use TCP forward - if (!isLoopbackHostname(window.location.hostname)) return null - try { const parsed = new URL(url) - if ( - (parsed.protocol === 'http:' || parsed.protocol === 'https:') && - isLoopbackHostname(parsed.hostname) - ) { - const targetPort = parsed.port - ? parseInt(parsed.port, 10) - : parsed.protocol === 'https:' - ? 443 - : 80 + if (parsed.protocol === 'http:' && isLoopbackHostname(parsed.hostname)) { + const targetPort = parsed.port ? parseInt(parsed.port, 10) : 80 // Don't proxy requests to Freshell's own port — it's already reachable const freshellPort = @@ -135,8 +129,10 @@ function buildHttpProxyUrl(url: string): string | null { } /** - * Determine whether a URL needs port forwarding (localhost URL + remote access). - * Returns the parsed URL and target port, or null if no forwarding needed. + * Determine whether a URL needs TCP port forwarding (remote browser + localhost). + * This is the fallback for URLs that buildHttpProxyUrl cannot handle: + * - https: localhost URLs (HTTP proxy can't do TLS passthrough) + * - http: localhost URLs on the same port as Freshell (proxy skips those) */ function needsPortForward(url: string): { parsed: URL; targetPort: number } | null { if (isLoopbackHostname(window.location.hostname)) return null diff --git a/test/e2e-browser/specs/browser-pane-screenshot.spec.ts b/test/e2e-browser/specs/browser-pane-screenshot.spec.ts new file mode 100644 index 00000000..b8795766 --- /dev/null +++ b/test/e2e-browser/specs/browser-pane-screenshot.spec.ts @@ -0,0 +1,148 @@ +import http from 'node:http' +import type { AddressInfo } from 'node:net' +import { test, expect } from '../helpers/fixtures.js' + +/** + * Browser Pane Screenshot E2E Tests + * + * These tests verify that the proxy header stripping works end-to-end: + * - Proxied localhost URLs render actual iframe content (not placeholders) + * because the proxy strips X-Frame-Options and Content-Security-Policy + * - Truly cross-origin URLs still fall back to a placeholder with the + * source URL (since they bypass the proxy entirely) + */ + +// Helper: create a browser pane via context menu split + picker +async function createBrowserPane(page: any) { + const termContainer = page.locator('.xterm').first() + await termContainer.click({ button: 'right' }) + await page.getByRole('menuitem', { name: /split horizontally/i }).click() + + const browserButton = page.getByRole('button', { name: /^Browser$/i }) + await expect(browserButton).toBeVisible({ timeout: 10_000 }) + await browserButton.click() + + await expect(page.getByPlaceholder('Enter URL...')).toBeVisible({ timeout: 10_000 }) +} + +/** + * Start a tiny HTTP server that serves a page with iframe-blocking headers. + * Returns the server and port. The caller must close the server after use. + */ +function startCanaryServer(): Promise<{ server: http.Server; port: number }> { + return new Promise((resolve, reject) => { + const server = http.createServer((_req, res) => { + res.setHeader('X-Frame-Options', 'DENY') + res.setHeader('Content-Security-Policy', "frame-ancestors 'none'; default-src 'self'") + res.setHeader('Content-Type', 'text/html; charset=utf-8') + res.end(` + +Canary Page + +

SCREENSHOT_CANARY

+

This page has X-Frame-Options: DENY and CSP frame-ancestors 'none'

+ +`) + }) + server.listen(0, '127.0.0.1', () => { + const addr = server.address() as AddressInfo + resolve({ server, port: addr.port }) + }) + server.on('error', reject) + }) +} + +test.describe('Browser Pane Screenshot', () => { + test('proxied localhost URL with iframe-blocking headers renders actual content, not placeholder', async ({ + freshellPage, + page, + serverInfo, + terminal, + }) => { + // Start a canary HTTP server with X-Frame-Options and CSP headers + const canary = await startCanaryServer() + + try { + await terminal.waitForTerminal() + await createBrowserPane(page) + + // Navigate to the canary server's localhost URL + const urlInput = page.getByPlaceholder('Enter URL...') + await urlInput.fill(`http://localhost:${canary.port}/`) + await urlInput.press('Enter') + + // Wait for iframe to load - the BrowserPane should proxy it + const iframe = page.locator('iframe[title="Browser content"]') + await iframe.waitFor({ state: 'attached', timeout: 15_000 }) + + // Verify the iframe src uses the proxy URL pattern + const src = await iframe.getAttribute('src') + expect(src).toContain(`/api/proxy/http/${canary.port}/`) + + // Wait for the iframe content to actually load by checking the frame's content. + // Since the proxy strips X-Frame-Options and CSP, the iframe should render. + const frame = iframe.contentFrame() + + // Wait for the canary text to appear in the iframe + await expect(frame!.locator('#canary')).toHaveText('SCREENSHOT_CANARY', { timeout: 10_000 }) + + // Verify there is no placeholder element visible on the page + // (The placeholder would appear if the iframe content was blocked) + const placeholder = page.locator('[data-screenshot-iframe-placeholder="true"]') + await expect(placeholder).toHaveCount(0) + + // Take a screenshot via the agent API (POST /api/screenshots) + // This exercises the full screenshot chain: captureUiScreenshot -> + // captureIframeReplacement -> html2canvas + const screenshotResponse = await page.evaluate( + async (info: { baseUrl: string; token: string }) => { + const res = await fetch(`${info.baseUrl}/api/screenshots`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-auth-token': info.token, + }, + body: JSON.stringify({ scope: 'view', name: 'canary-screenshot', overwrite: true }), + }) + return { status: res.status, body: await res.json() } + }, + { baseUrl: serverInfo.baseUrl, token: serverInfo.token }, + ) + + // The screenshot should succeed (agent API uses { status: 'ok', data: {...} }) + expect(screenshotResponse.status).toBe(200) + expect(screenshotResponse.body.status).toBe('ok') + expect(screenshotResponse.body.data?.path).toBeTruthy() + expect(screenshotResponse.body.data?.width).toBeGreaterThan(0) + expect(screenshotResponse.body.data?.height).toBeGreaterThan(0) + } finally { + await new Promise((resolve) => canary.server.close(() => resolve())) + } + }) + + test('truly cross-origin URL falls back to placeholder with source URL', async ({ + freshellPage, + page, + terminal, + }) => { + await terminal.waitForTerminal() + await createBrowserPane(page) + + // Navigate to a truly cross-origin URL that the proxy cannot handle + const urlInput = page.getByPlaceholder('Enter URL...') + await urlInput.fill('https://example.com') + await urlInput.press('Enter') + + // Wait for the iframe to load + const iframe = page.locator('iframe[title="Browser content"]') + await iframe.waitFor({ state: 'attached', timeout: 15_000 }) + + // The iframe src should NOT use the proxy (it's not a localhost URL) + const src = await iframe.getAttribute('src') + expect(src).not.toContain('/api/proxy/') + + // The page should show the URL text "example.com" somewhere visible + // (either in the iframe content or in the URL bar) + await expect(page.getByPlaceholder('Enter URL...')).toHaveValue(/example\.com/, { timeout: 5_000 }) + }) +}) diff --git a/test/unit/client/components/panes/BrowserPane.test.tsx b/test/unit/client/components/panes/BrowserPane.test.tsx index 2ffbf91b..0d9c2bcd 100644 --- a/test/unit/client/components/panes/BrowserPane.test.tsx +++ b/test/unit/client/components/panes/BrowserPane.test.tsx @@ -302,7 +302,8 @@ describe('BrowserPane', () => { .mockResolvedValueOnce({ forwardedPort: 45678 }) await act(async () => { - renderBrowserPane({ url: 'http://localhost:3000' }, store) + // Use https: URL — http: uses same-origin proxy, not TCP forwarding + renderBrowserPane({ url: 'https://localhost:3000' }, store) }) await waitFor(() => { @@ -319,7 +320,8 @@ describe('BrowserPane', () => { await waitFor(() => { expect(screen.queryByText('Failed to connect')).not.toBeInTheDocument() }) - expect(document.querySelector('iframe')?.getAttribute('src')).toBe('http://192.168.1.100:45678/') + // Protocol preserved — TCP forward passes bytes verbatim + expect(document.querySelector('iframe')?.getAttribute('src')).toBe('https://192.168.1.100:45678/') expect(store.getState().panes.refreshRequestsByPane['tab-1']).toBeUndefined() }) }) @@ -337,10 +339,11 @@ describe('BrowserPane', () => { }) }) - it('marks the pane as error when port forwarding fails', async () => { + it('marks the pane as error when port forwarding fails for https: URL', async () => { setWindowHostname('remote-host') vi.mocked(api.post).mockRejectedValueOnce(new Error('forward failed')) - const { store } = renderBrowserPane({ url: 'http://127.0.0.1:3000' }) + // Use https: — http: uses same-origin proxy, not TCP forwarding + const { store } = renderBrowserPane({ url: 'https://127.0.0.1:3000' }) await waitFor(() => { expect(store.getState().paneRuntimeActivity.byPaneId['pane-1']).toMatchObject({ @@ -356,7 +359,8 @@ describe('BrowserPane', () => { vi.mocked(api.post).mockReturnValueOnce(new Promise((resolve) => { resolveForward = resolve })) - const { store } = renderBrowserPane({ url: 'http://127.0.0.1:3000' }) + // Use https: — http: uses same-origin proxy, not TCP forwarding + const { store } = renderBrowserPane({ url: 'https://127.0.0.1:3000' }) expect(store.getState().paneRuntimeActivity.byPaneId['pane-1']).toMatchObject({ source: 'browser', @@ -402,54 +406,54 @@ describe('BrowserPane', () => { }) describe('port forwarding for remote access', () => { - it('requests a port forward for localhost URLs when accessing remotely', async () => { + it('proxies http: localhost URLs through HTTP proxy when accessing remotely', async () => { setWindowHostname('192.168.1.100') - vi.mocked(api.post).mockResolvedValue({ forwardedPort: 45678 }) await act(async () => { - renderBrowserPane({ url: 'http://localhost:3000' }) + // Use port 4000 to avoid collision with jsdom's default port (3000) + renderBrowserPane({ url: 'http://localhost:4000' }) }) - expect(api.post).toHaveBeenCalledWith('/api/proxy/forward', { port: 3000 }) + // http: localhost URLs use the same-origin HTTP proxy, not TCP forwarding + expect(api.post).not.toHaveBeenCalled() await waitFor(() => { const iframe = document.querySelector('iframe') expect(iframe).toBeTruthy() - expect(iframe!.getAttribute('src')).toBe('http://192.168.1.100:45678/') + expect(iframe!.getAttribute('src')).toBe('/api/proxy/http/4000/') }) }) - it('requests a port forward for 127.0.0.1 URLs when accessing remotely', async () => { + it('proxies http://127.0.0.1 URLs through HTTP proxy when accessing remotely', async () => { setWindowHostname('192.168.1.100') - vi.mocked(api.post).mockResolvedValue({ forwardedPort: 45679 }) await act(async () => { renderBrowserPane({ url: 'http://127.0.0.1:8080' }) }) - expect(api.post).toHaveBeenCalledWith('/api/proxy/forward', { port: 8080 }) + expect(api.post).not.toHaveBeenCalled() await waitFor(() => { const iframe = document.querySelector('iframe') expect(iframe).toBeTruthy() - expect(iframe!.getAttribute('src')).toBe('http://192.168.1.100:45679/') + expect(iframe!.getAttribute('src')).toBe('/api/proxy/http/8080/') }) }) - it('preserves path and query when port forwarding', async () => { + it('preserves path and query when proxying http: localhost URLs remotely', async () => { setWindowHostname('10.0.0.5') - vi.mocked(api.post).mockResolvedValue({ forwardedPort: 55555 }) await act(async () => { - renderBrowserPane({ url: 'http://localhost:3000/api/data?q=test' }) + // Use port 4000 to avoid collision with jsdom's default port (3000) + renderBrowserPane({ url: 'http://localhost:4000/api/data?q=test' }) }) - expect(api.post).toHaveBeenCalledWith('/api/proxy/forward', { port: 3000 }) + expect(api.post).not.toHaveBeenCalled() await waitFor(() => { const iframe = document.querySelector('iframe') expect(iframe).toBeTruthy() - expect(iframe!.getAttribute('src')).toBe('http://10.0.0.5:55555/api/data?q=test') + expect(iframe!.getAttribute('src')).toBe('/api/proxy/http/4000/api/data?q=test') }) }) @@ -513,7 +517,7 @@ describe('BrowserPane', () => { ) }) - it('shows connecting state while port forward is pending', async () => { + it('shows connecting state while port forward is pending for https: URL', async () => { setWindowHostname('192.168.1.100') let resolveForward!: (value: { forwardedPort: number }) => void vi.mocked(api.post).mockReturnValue( @@ -522,7 +526,7 @@ describe('BrowserPane', () => { }), ) - renderBrowserPane({ url: 'http://localhost:3000' }) + renderBrowserPane({ url: 'https://localhost:3000' }) // Should show connecting state (no iframe yet) expect(screen.getByText(/Connecting/i)).toBeInTheDocument() @@ -537,7 +541,7 @@ describe('BrowserPane', () => { await waitFor(() => { const iframe = document.querySelector('iframe') expect(iframe).toBeTruthy() - expect(iframe!.getAttribute('src')).toBe('http://192.168.1.100:45678/') + expect(iframe!.getAttribute('src')).toBe('https://192.168.1.100:45678/') }) }) @@ -545,7 +549,7 @@ describe('BrowserPane', () => { setWindowHostname('192.168.1.100') vi.mocked(api.post).mockReturnValue(new Promise(() => {})) - renderBrowserPane({ url: 'http://localhost:3000' }) + renderBrowserPane({ url: 'https://localhost:3000' }) expect(screen.getByText(/Connecting/i)).toBeInTheDocument() @@ -566,7 +570,7 @@ describe('BrowserPane', () => { vi.mocked(api.post).mockResolvedValue({ forwardedPort: 45678 }) await act(async () => { - renderBrowserPane({ url: 'http://localhost:3000' }) + renderBrowserPane({ url: 'https://localhost:3000' }) }) await waitFor(() => { @@ -584,14 +588,14 @@ describe('BrowserPane', () => { }) }) - it('shows error when port forwarding fails', async () => { + it('shows error when port forwarding fails for https: URL', async () => { setWindowHostname('192.168.1.100') vi.mocked(api.post).mockRejectedValue( new Error('Failed to create port forward'), ) await act(async () => { - renderBrowserPane({ url: 'http://localhost:3000' }) + renderBrowserPane({ url: 'https://localhost:3000' }) }) await waitFor(() => { @@ -600,12 +604,12 @@ describe('BrowserPane', () => { }) }) - it('clears loading state when port forwarding fails', async () => { + it('clears loading state when port forwarding fails for https: URL', async () => { setWindowHostname('192.168.1.100') vi.mocked(api.post).mockRejectedValue(new Error('Connection refused')) await act(async () => { - renderBrowserPane({ url: 'http://localhost:3000' }) + renderBrowserPane({ url: 'https://localhost:3000' }) }) await waitFor(() => { @@ -624,7 +628,7 @@ describe('BrowserPane', () => { .mockResolvedValueOnce({ forwardedPort: 45678 }) await act(async () => { - renderBrowserPane({ url: 'http://localhost:3000' }) + renderBrowserPane({ url: 'https://localhost:3000' }) }) await waitFor(() => { @@ -643,11 +647,11 @@ describe('BrowserPane', () => { expect(api.post).toHaveBeenCalledTimes(2) }) - // Should now show the iframe + // Should now show the iframe (https: protocol preserved through TCP forward) await waitFor(() => { const iframe = document.querySelector('iframe') expect(iframe).toBeTruthy() - expect(iframe!.getAttribute('src')).toBe('http://192.168.1.100:45678/') + expect(iframe!.getAttribute('src')).toBe('https://192.168.1.100:45678/') }) }) }) diff --git a/test/unit/client/ui-screenshot.test.ts b/test/unit/client/ui-screenshot.test.ts index 4af24366..48603a6f 100644 --- a/test/unit/client/ui-screenshot.test.ts +++ b/test/unit/client/ui-screenshot.test.ts @@ -240,6 +240,55 @@ describe('captureUiScreenshot iframe handling', () => { expect(iframe.hasAttribute('data-screenshot-iframe-marker')).toBe(false) }) + it('captures proxy-URL iframe as image content when document is accessible', async () => { + document.body.innerHTML = ` +
+ +
+ ` + const target = document.querySelector('[data-context="global"]') as HTMLElement + const iframe = document.getElementById('proxy-frame') as HTMLIFrameElement + setRect(target, 800, 500) + setRect(iframe, 500, 300) + + const iframeDoc = iframe.contentDocument + expect(iframeDoc).toBeTruthy() + iframeDoc?.open() + iframeDoc?.write('

Proxied localhost content

') + iframeDoc?.close() + + let clonedHtml = '' + vi.mocked(html2canvas).mockImplementation(async (_el: any, opts: any = {}) => { + if (typeof opts.onclone === 'function') { + const cloneDoc = document.implementation.createHTMLDocument('clone') + const cloneTarget = target.cloneNode(true) as HTMLElement + cloneDoc.body.appendChild(cloneTarget) + opts.onclone(cloneDoc) + clonedHtml = cloneTarget.innerHTML + return { + width: 800, + height: 500, + toDataURL: () => 'data:image/png;base64,PROXYPNG', + } as any + } + + return { + width: 500, + height: 300, + toDataURL: () => 'data:image/png;base64,IFRAMEPROXYPNG', + } as any + }) + + const result = await captureUiScreenshot({ scope: 'view' }, createRuntime() as any) + + expect(result.ok).toBe(true) + expect(result.imageBase64).toBe('PROXYPNG') + // The iframe should be replaced with an image, not a placeholder + expect(clonedHtml).toContain('data-screenshot-iframe-image="true"') + expect(clonedHtml).not.toContain('data-screenshot-iframe-placeholder') + expect(clonedHtml).not.toContain(' { document.body.innerHTML = `
diff --git a/test/unit/server/mcp/freshell-tool.test.ts b/test/unit/server/mcp/freshell-tool.test.ts index cf3487be..52662323 100644 --- a/test/unit/server/mcp/freshell-tool.test.ts +++ b/test/unit/server/mcp/freshell-tool.test.ts @@ -56,6 +56,19 @@ describe('TOOL_DESCRIPTION and INSTRUCTIONS', () => { expect(INSTRUCTIONS).toContain('new-window') }) + it('browser pane screenshot instructions reflect that proxied localhost URLs render actual content', () => { + // The instructions should clarify that proxied localhost URLs render actual + // content in iframe screenshots, and only truly cross-origin URLs fall back + // to a placeholder. This guards against regressions where the instructions + // incorrectly claim all browser pane screenshots show placeholders. + expect(INSTRUCTIONS).toContain('proxied localhost URLs render actual content') + expect(INSTRUCTIONS).toContain('cross-origin') + expect(INSTRUCTIONS).toContain('placeholder') + // Must NOT contain the old wording that claims all proxied screenshots are placeholders + expect(INSTRUCTIONS).not.toMatch(/browser pane screenshots.*always.*placeholder/i) + expect(INSTRUCTIONS).not.toMatch(/cross-origin iframe content renders a placeholder/i) + }) + it('INPUT_SCHEMA has action and params fields', () => { expect(INPUT_SCHEMA).toHaveProperty('action') expect(INPUT_SCHEMA).toHaveProperty('params') diff --git a/test/unit/server/proxy-router.test.ts b/test/unit/server/proxy-router.test.ts index 3a529cff..fa809356 100644 --- a/test/unit/server/proxy-router.test.ts +++ b/test/unit/server/proxy-router.test.ts @@ -65,6 +65,23 @@ describe('createProxyRouter', () => { targetApp.get('/path/to/page', (_req, res) => res.send('deep path')) targetApp.get('/with-query', (req, res) => res.json({ q: req.query.q })) targetApp.post('/echo', express.json(), (req, res) => res.json(req.body)) + targetApp.get('/with-xfo', (_req, res) => { + res.set('X-Frame-Options', 'DENY') + res.send('framed content') + }) + targetApp.get('/with-csp', (_req, res) => { + res.set('Content-Security-Policy', "frame-ancestors 'none'; default-src 'self'") + res.send('csp content') + }) + targetApp.get('/with-both', (_req, res) => { + res.set('X-Frame-Options', 'SAMEORIGIN') + res.set('Content-Security-Policy', "frame-ancestors 'none'") + res.send('both headers') + }) + targetApp.get('/no-frame-headers', (_req, res) => { + res.set('X-Custom-Header', 'keep-me') + res.send('no frame headers') + }) targetServer = await new Promise((resolve) => { const server = targetApp.listen(0, '127.0.0.1', () => resolve(server)) }) @@ -153,6 +170,63 @@ describe('createProxyRouter', () => { expect(res.status).toBe(400) }) + + it('strips X-Frame-Options header from proxied responses', async () => { + process.env.AUTH_TOKEN = TEST_AUTH_TOKEN + const manager = { forward: vi.fn(), close: vi.fn() } as unknown as PortForwardManager + const app = createApp(manager) + + const res = await request(app) + .get(`/api/proxy/http/${targetPort}/with-xfo`) + .set('x-auth-token', TEST_AUTH_TOKEN) + + expect(res.status).toBe(200) + expect(res.text).toBe('framed content') + expect(res.headers['x-frame-options']).toBeUndefined() + }) + + it('strips Content-Security-Policy header from proxied responses', async () => { + process.env.AUTH_TOKEN = TEST_AUTH_TOKEN + const manager = { forward: vi.fn(), close: vi.fn() } as unknown as PortForwardManager + const app = createApp(manager) + + const res = await request(app) + .get(`/api/proxy/http/${targetPort}/with-csp`) + .set('x-auth-token', TEST_AUTH_TOKEN) + + expect(res.status).toBe(200) + expect(res.text).toBe('csp content') + expect(res.headers['content-security-policy']).toBeUndefined() + }) + + it('strips both X-Frame-Options and Content-Security-Policy simultaneously', async () => { + process.env.AUTH_TOKEN = TEST_AUTH_TOKEN + const manager = { forward: vi.fn(), close: vi.fn() } as unknown as PortForwardManager + const app = createApp(manager) + + const res = await request(app) + .get(`/api/proxy/http/${targetPort}/with-both`) + .set('x-auth-token', TEST_AUTH_TOKEN) + + expect(res.status).toBe(200) + expect(res.text).toBe('both headers') + expect(res.headers['x-frame-options']).toBeUndefined() + expect(res.headers['content-security-policy']).toBeUndefined() + }) + + it('preserves non-iframe-blocking headers from proxied responses', async () => { + process.env.AUTH_TOKEN = TEST_AUTH_TOKEN + const manager = { forward: vi.fn(), close: vi.fn() } as unknown as PortForwardManager + const app = createApp(manager) + + const res = await request(app) + .get(`/api/proxy/http/${targetPort}/no-frame-headers`) + .set('x-auth-token', TEST_AUTH_TOKEN) + + expect(res.status).toBe(200) + expect(res.text).toBe('no frame headers') + expect(res.headers['x-custom-header']).toBe('keep-me') + }) }) it('waits for forward shutdown before returning from delete', async () => {