From 355802fc0c93ccc247bc56738a6851fb062ad439 Mon Sep 17 00:00:00 2001 From: Neil Date: Mon, 19 Oct 2026 12:45:28 -0700 Subject: [PATCH] fix(ssh): orca open-url on a managed SSH host opens the page on the desktop through the browser bridge --- .../browser/browser-client-page-opening.ts | 13 ++- .../desktop-browser-bridge-dispatch.test.ts | 101 ++++++++++++++++++ .../desktop-browser-bridge-route.ts | 13 ++- .../ssh-orcad-browser-capabilities.spec.ts | 3 +- .../ssh-orcad-desktop-browser-bridge.spec.ts | 24 ++++- 5 files changed, 145 insertions(+), 9 deletions(-) diff --git a/src/main/runtime/browser/browser-client-page-opening.ts b/src/main/runtime/browser/browser-client-page-opening.ts index 8c0da2e32574..402004214e3e 100644 --- a/src/main/runtime/browser/browser-client-page-opening.ts +++ b/src/main/runtime/browser/browser-client-page-opening.ts @@ -125,10 +125,7 @@ export function openBrowserUrlOnClient( host: BrowserClientPageOpeningHost, params: { url: string; worktree: string } ): Promise<{ browserPageId: string }> { - const protocol = new URL(params.url).protocol - if (protocol !== 'http:' && protocol !== 'https:') { - throw new BrowserError('invalid_argument', 'Only http(s) URLs can be opened on the client.') - } + assertClientOpenableUrl(params.url) return createBrowserPageOnPairedHost( host, { url: params.url, worktree: params.worktree, activate: true, navigation: 'caller' }, @@ -136,6 +133,14 @@ export function openBrowserUrlOnClient( ) } +/** `orca open-url` opens only web pages, wherever the page ends up rendering. */ +export function assertClientOpenableUrl(url: string): void { + const protocol = new URL(url).protocol + if (protocol !== 'http:' && protocol !== 'https:') { + throw new BrowserError('invalid_argument', 'Only http(s) URLs can be opened on the client.') + } +} + /** A page its caller did not place goes to the one paired desktop hosting this runtime's pages. */ export async function createBrowserPageOnPairedHost( host: BrowserClientPageOpeningHost, diff --git a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts index 00d4ac308bbd..96bbdd3d0af2 100644 --- a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts +++ b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts @@ -12,6 +12,7 @@ import { RpcDispatcher } from '../rpc/dispatcher' import { DESKTOP_BROWSER_BRIDGE_RPC_METHODS } from '../rpc/methods/desktop-browser-bridge' import { BrowserTarget } from '../../../shared/rpc-contract/rpc-param-primitives' import { Goto } from '../../../shared/rpc-contract/browser-params' +import { BrowserOpenUrlParams } from '../../../shared/rpc-contract/browser-tab-create-params' import { getDesktopBrowserBridgeRegistry, setDesktopBrowserBridgeHostEnabled @@ -23,6 +24,7 @@ const ForwardedEvent = z.object({ requestId: z.string() }).passthrough() const localGoto = vi.fn(async () => ({ url: 'local', title: '' })) const localCookie = vi.fn(async () => ({ cookies: [] })) +const localOpenUrl = vi.fn(async () => ({ browserPageId: 'paired-host-page' })) const METHODS = [ ...DESKTOP_BROWSER_BRIDGE_RPC_METHODS, @@ -37,6 +39,12 @@ const METHODS = [ permission: 'workspace', params: BrowserTarget, handler: () => localCookie() + }), + defineMethod({ + name: 'browser.openUrl', + permission: 'workspace', + params: BrowserOpenUrlParams, + handler: () => localOpenUrl() }) ] @@ -108,6 +116,7 @@ afterEach(() => { setDesktopBrowserBridgeHostEnabled(false) localGoto.mockClear() localCookie.mockClear() + localOpenUrl.mockClear() }) describe('desktop browser bridge on orcad', () => { @@ -171,6 +180,98 @@ describe('desktop browser bridge on orcad', () => { expect(localGoto).not.toHaveBeenCalled() }) + it('sends open-url to the attached desktop as the tab create it already runs', async () => { + setDesktopBrowserBridgeHostEnabled(true) + const { runtime } = orcadRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) + const { events } = await attachDesktop(dispatcher) + const ready = ReadyEvent.parse(events[0]) + + const call = cliCall(dispatcher, 'browser.openUrl', { + url: 'https://example.com/', + worktree: 'path:/srv/app' + }) + await vi.waitFor(() => expect(events).toHaveLength(2)) + const forwarded = ForwardedEvent.parse(events[1]) + // An older bridged desktop has always accepted browser.tabCreate; it never knew browser.openUrl. + expect(forwarded).toEqual({ + type: 'request', + requestId: forwarded.requestId, + method: 'browser.tabCreate', + timeoutMs: expect.any(Number), + workspaceId: 'repo-1::/srv/app', + params: { url: 'https://example.com/', worktree: 'id:repo-1::/srv/app' } + }) + await dispatcher.dispatch( + { + id: 'answer', + authToken: 'bound', + method: 'desktopBrowserBridge.result', + params: { + attachmentId: ready.attachmentId, + requestId: forwarded.requestId, + ok: true, + result: { browserPageId: 'desktop-page' } + } + }, + DESKTOP + ) + + await expect(call).resolves.toMatchObject({ + ok: true, + result: { browserPageId: 'desktop-page' } + }) + expect(localOpenUrl).not.toHaveBeenCalled() + }) + + it('refuses a non-web open-url on the host before it reaches the desktop', async () => { + setDesktopBrowserBridgeHostEnabled(true) + const { runtime } = orcadRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) + const { events } = await attachDesktop(dispatcher) + + const response = await cliCall(dispatcher, 'browser.openUrl', { + url: 'file:///etc/hosts', + worktree: 'path:/srv/app' + }) + + expect(response).toMatchObject({ ok: false, error: { code: 'invalid_argument' } }) + expect(events).toHaveLength(1) + }) + + it('names the setting for open-url when no desktop hosts or shares a browser', async () => { + setDesktopBrowserBridgeHostEnabled(true) + const { runtime } = orcadRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) + + const response = await cliCall(dispatcher, 'browser.openUrl', { + url: 'https://example.com/', + worktree: 'path:/srv/app' + }) + + expect(response).toMatchObject({ + ok: false, + error: { code: 'desktop_browser_bridge_unavailable' } + }) + expect(response.ok ? '' : response.error.message).toContain( + DESKTOP_BROWSER_BRIDGE_SETTING_LABEL + ) + expect(localOpenUrl).not.toHaveBeenCalled() + }) + + it("leaves open-url to a desktop hosting this runtime's pages while none shares its browser", async () => { + setDesktopBrowserBridgeHostEnabled(true) + const { runtime } = orcadRuntime({ localBrowser: true }) + const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) + + await expect( + cliCall(dispatcher, 'browser.openUrl', { + url: 'https://example.com/', + worktree: 'path:/srv/app' + }) + ).resolves.toMatchObject({ ok: true, result: { browserPageId: 'paired-host-page' } }) + }) + it('never forwards a command outside the allowlist', async () => { setDesktopBrowserBridgeHostEnabled(true) const { runtime } = orcadRuntime() diff --git a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-route.ts b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-route.ts index ba04c852feab..38f53268af0b 100644 --- a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-route.ts +++ b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-route.ts @@ -11,6 +11,7 @@ import { isDesktopBrowserBridgeMethod } from '../../../shared/desktop-browser-bridge-protocol' import type { RpcCallerIdentity } from '../rpc/rpc-caller-identity' +import { assertClientOpenableUrl } from '../browser/browser-client-page-opening' import type { DesktopBrowserBridgeRegistry } from './desktop-browser-bridge-registry' export type DesktopBrowserBridgeRoute = { handled: false } | { handled: true; result: unknown } @@ -38,7 +39,10 @@ export async function routeDesktopBrowserBridgeRpc(options: { ) { return { handled: false } } - const forwarded = isDesktopBrowserBridgeMethod(method) + // Why tabCreate: open-url is `tab create --url` to the desktop, so it adds no new desktop surface + // and an older bridged desktop already runs it under the same scope, URL and focus rules. + const forwardedMethod = method === 'browser.openUrl' ? 'browser.tabCreate' : method + const forwarded = isDesktopBrowserBridgeMethod(forwardedMethod) if (!options.registry.hasAttachment()) { if (options.localBrowserAvailable || !forwarded) { return { handled: false } @@ -58,6 +62,9 @@ export async function routeDesktopBrowserBridgeRpc(options: { ) } const params: Record = isRecord(options.params) ? { ...options.params } : {} + if (method === 'browser.openUrl') { + assertClientOpenableUrl(String(params.url)) + } let workspaceId: string | undefined // Why resolve here: only orcad knows this host's selectors (path:, branch:, active). if (typeof params.worktree === 'string' && params.worktree.length > 0) { @@ -67,8 +74,8 @@ export async function routeDesktopBrowserBridgeRpc(options: { delete params.worktree } const result = await options.registry.forward( - { method, params, ...(workspaceId ? { workspaceId } : {}) }, - { timeoutMs: forwardTimeoutMs(method, params), signal: options.signal } + { method: forwardedMethod, params, ...(workspaceId ? { workspaceId } : {}) }, + { timeoutMs: forwardTimeoutMs(forwardedMethod, params), signal: options.signal } ) return { handled: true, result } } diff --git a/tests/e2e/ssh-orcad-browser-capabilities.spec.ts b/tests/e2e/ssh-orcad-browser-capabilities.spec.ts index 807c9fe8a5b5..9451e7d91586 100644 --- a/tests/e2e/ssh-orcad-browser-capabilities.spec.ts +++ b/tests/e2e/ssh-orcad-browser-capabilities.spec.ts @@ -54,7 +54,8 @@ test('a backendless managed host offers desktop-hosted pages and refuses its own const refused = await call('browser.tabList', {}) console.log('[managed-browser-command]', JSON.stringify(refused)) expect(refused).toMatchObject({ ok: false, error: { code: 'browser_unavailable' } }) - // A managed SSH desktop keeps its SSH-routed tabs, so it never takes this host's pages. + // A paired caller's open-url asks for a desktop hosting this host's pages, which a managed SSH + // desktop never is; the host's own CLI reaches it through the browser bridge instead. const openUrl = await call('browser.openUrl', { url: 'https://example.org/', worktree: 'active' diff --git a/tests/e2e/ssh-orcad-desktop-browser-bridge.spec.ts b/tests/e2e/ssh-orcad-desktop-browser-bridge.spec.ts index 1571ec02cb95..7670efc847b2 100644 --- a/tests/e2e/ssh-orcad-desktop-browser-bridge.spec.ts +++ b/tests/e2e/ssh-orcad-desktop-browser-bridge.spec.ts @@ -4,7 +4,8 @@ * * Off, the CLI is refused with an error that names the setting. On, `orca tab create` opens a * desktop tab in the host's workspace (its traffic leaves through the host, which alone resolves - * the fixture origin) and `orca goto` navigates it. Off again, the host is refused at once. + * the fixture origin), `orca goto` navigates it and `orca open-url` opens one the same way. Off + * again, the host is refused at once. */ import type { ElectronApplication, Page } from '@stablyai/playwright-test' import { z } from 'zod' @@ -185,6 +186,23 @@ test('a managed SSH host drives the desktop browser by default until its CLI con const listed = await hostCli(page, target, tabId, 'orca tab list') expect(listed.ok).toBe(true) expect(JSON.stringify(listed.result)).toContain(browserPageId) + // `orca open-url` reaches the desktop the same way, as a tab in the host's workspace. + const openedUrl = `${SSH_REMOTE_ONLY_ORIGIN}/echo/bridge-open-url` + const opened = await hostCliUntilOk(page, target, tabId, `orca open-url --url ${openedUrl}`) + const openedPageId = String(opened.result?.browserPageId ?? '') + expect(openedPageId).not.toBe('') + await expect + .poll(() => browserTabUrls(page, seeded.worktreeId), { timeout: 30_000 }) + .toContain(openedUrl) + await expect + .poll(() => readSshRemoteOnlyRequests(target).map((request) => request.path), { + timeout: 60_000, + message: 'the desktop tab open-url opened never loaded through the host' + }) + .toContain('/echo/bridge-open-url') + expect((await hostCli(page, target, tabId, `orca tab close --page ${openedPageId}`)).ok).toBe( + true + ) await page.screenshot({ path: testInfo.outputPath('desktop-browser-bridge-on.png') }) // A desktop file is never reachable, even through a tab the host may drive. const fileRead = await hostCli( @@ -210,6 +228,10 @@ test('a managed SSH host drives the desktop browser by default until its CLI con .toBe('desktop_browser_bridge_unavailable') const refused = await hostCli(page, target, tabId, 'orca tab list') expect(refused.error?.message).toContain(DESKTOP_BROWSER_BRIDGE_SETTING_LABEL) + // open-url names the same setting, not a desktop that is "offline". + const openRefused = await hostCli(page, target, tabId, `orca open-url --url ${openedUrl}`) + expect(openRefused.error?.code).toBe('desktop_browser_bridge_unavailable') + expect(openRefused.error?.message).toContain(DESKTOP_BROWSER_BRIDGE_SETTING_LABEL) } finally { try { if (app) {