Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions config/scripts/ci-e2e-job-selection.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ it.each([
expect(selectPrE2eSpecs([path])).toContain('tests/e2e/ssh-orcad-terminal-browser-link.spec.ts')
})

it.each([
'src/shared/desktop-browser-bridge-protocol.ts',
'src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-route.ts',
'src/main/browser/desktop-browser-bridge/desktop-browser-bridge-executor.ts',
'src/main/ipc/desktop-browser-bridge-wiring.ts',
'src/main/runtime/rpc/methods/desktop-browser-bridge.ts'
])('routes the managed desktop browser bridge oracle from %s', (path) => {
expect(selectPrE2eSpecs([path])).toContain('tests/e2e/ssh-orcad-desktop-browser-bridge.spec.ts')
})

const workflow = parse(readFileSync('.github/workflows/e2e.yml', 'utf8'))
const prWorkflow = parse(readFileSync('.github/workflows/pr.yml', 'utf8'))
const classify = (specs) => classifyE2eJobs(JSON.stringify(specs))
Expand Down
2 changes: 1 addition & 1 deletion config/scripts/locale-ko-key-overrides.json
Original file line number Diff line number Diff line change
Expand Up @@ -6264,7 +6264,7 @@
"ko": "이 호스트의 orca CLI가 Orca를 제어하도록 허용"
},
"auto.components.settings.SshTargetForm.allowRemoteCliControlHelp": {
"ko": "꺼져 있으면 이 호스트에서 실행한 orca 명령은 이 호스트의 터미널에만 적용됩니다. 켜면 다른 호스트와 이 컴퓨터의 오케스트레이션, 파일 및 연동 기능에도 접근할 수 있습니다."
"ko": "켜면 이 호스트에서 실행한 orca 명령이 이 호스트의 작업 공간에서만, 연결되어 있는 동안에만 이 데스크톱의 Orca 브라우저 탭을 열고 조작할 수 있습니다. 파일, 클립보드, 쿠키는 읽을 수 없습니다. 끄면 이 호스트 자체의 Orca 서버에만 접근합니다."
},
"auto.components.settings.AdvancedNetworkSettingsSection.3e431564b5": {
"ko": "localhost, 127.0.0.1, *.internal"
Expand Down
9 changes: 9 additions & 0 deletions config/scripts/pr-e2e-source-routing.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,15 @@ export const PR_E2E_SOURCE_ROUTES = [
file
)
},
{
id: 'ssh.orcad-desktop-browser-bridge',
specs: ['tests/e2e/ssh-orcad-desktop-browser-bridge.spec.ts'],
matches: (file) =>
isProductSource(file) &&
/^src\/(?:shared\/desktop-browser-bridge-protocol\.ts$|main\/(?:runtime\/desktop-browser-bridge\/|browser\/desktop-browser-bridge\/|ipc\/desktop-browser-bridge-|runtime\/rpc\/methods\/desktop-browser-bridge\.ts$))/.test(
file
)
},
{
id: 'ssh.orcad-browser-drop-owner',
specs: ['tests/e2e/ssh-orcad-browser-drop-owner.spec.ts'],
Expand Down
17 changes: 12 additions & 5 deletions src/main/browser/agent-browser-bridge-capture-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { captureFullPageScreenshot } from './cdp-screenshot'
import { acquireElectronDebugger } from './electron-debugger-lease'
import { AgentBrowserBridgeUtilityCommands } from './agent-browser-bridge-utility-commands'
import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep'
import { captureRemoteCliBrowserWorkBound } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope'

export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBridgeUtilityCommands {
async screenshot(
Expand Down Expand Up @@ -60,6 +61,8 @@ export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBrid
worktreeId?: string,
browserPageId?: string
): Promise<BrowserEvalResult> {
// Why captured here: the queue may run this later, outside the bridged command's context.
const bound = captureRemoteCliBrowserWorkBound()
return this.enqueueTargetedCommand(
worktreeId,
browserPageId,
Expand All @@ -68,11 +71,15 @@ export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBrid
let releaseDebugger = (): void => {}
try {
releaseDebugger = acquireElectronDebugger(wc).release
const { result, exceptionDetails } = (await wc.debugger.sendCommand('Runtime.evaluate', {
expression,
returnByValue: true,
awaitPromise: true
})) as {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Runtime.evaluate with returnByValue always answers this CDP shape; unchanged from before the bound.
const { result, exceptionDetails } = (await bound(
wc.debugger.sendCommand('Runtime.evaluate', {
expression,
returnByValue: true,
awaitPromise: true
}),
() => void wc.debugger.sendCommand('Runtime.terminateExecution').catch(() => {})
)) as {
result: { value?: unknown; description?: string }
exceptionDetails?: { text: string; exception?: { description?: string } }
}
Expand Down
16 changes: 15 additions & 1 deletion src/main/browser/agent-browser-bridge-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
EnqueueTargetedCommandOptions,
ResolvedBrowserCommandTarget
} from './agent-browser-bridge-types'
import { captureRemoteCliBrowserRevocation } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope'

export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown {
// Why: route tab switch through the command queue so it can't race in-flight commands targeting the old tab.
Expand Down Expand Up @@ -59,6 +60,8 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
options: EnqueueTargetedCommandOptions = {}
): Promise<T> {
this.assertCommandAdmission()
const assertNotRevoked = captureRemoteCliBrowserRevocation()
assertNotRevoked()
const { browserPageId: pageId } = this.resolveCommandTarget(
worktreeId,
browserPageId,
Expand All @@ -73,7 +76,18 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
this.commandQueues.set(sessionName, queue)
}
queue.push({
execute: () => this.executeQueuedCommand(worktreeId, pageId, execute, options),
execute: () => {
assertNotRevoked()
return this.executeQueuedCommand(
worktreeId,
pageId,
(session, target) => {
assertNotRevoked(target.webContentsId)
return execute(session, target)
},
options
)
},
resolve: resolve as (value: unknown) => void,
reject
})
Expand Down
5 changes: 5 additions & 0 deletions src/main/browser/browser-manager-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ export abstract class BrowserManagerQueries extends BrowserManagerRegistration {
return this.tabIdByWebContentsId.get(webContentsId) ?? null
}

/** The page a guest belongs to, following popups back to the page that opened them. */
getOwningTabIdForGuestWebContentsId(guestWebContentsId: number): string | null {
return this.resolveBrowserTabIdForGuestWebContentsId(guestWebContentsId)
}

getWorktreeIdForTab(browserTabId: string): string | undefined {
return this.worktreeIdByTabId.get(browserTabId)
}
Expand Down
46 changes: 45 additions & 1 deletion src/main/browser/browser-session-partition-policies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ const mocks = vi.hoisted(() => ({
handleGuestWillDownload: vi.fn(),
noticeDocPreviewDownloadBlocked: vi.fn(),
clearBrowserWebAuthnAccessHandlers: vi.fn(),
installBrowserWebAuthnAccessHandlers: vi.fn()
installBrowserWebAuthnAccessHandlers: vi.fn(),
isRemoteCliDrivenGuest: vi.fn((_webContentsId?: number) => false)
}))

type WillDownloadListener = (
Expand Down Expand Up @@ -66,6 +67,9 @@ vi.mock('electron', () => ({
}
}
}))
vi.mock('./desktop-browser-bridge/remote-cli-driven-workspaces', () => ({
isRemoteCliDrivenGuest: mocks.isRemoteCliDrivenGuest
}))
vi.mock('./browser-manager', () => ({
browserManager: {
handleGuestWillDownload: mocks.handleGuestWillDownload,
Expand Down Expand Up @@ -195,6 +199,46 @@ describe('partition permission policy', () => {
expect(mocks.clearBrowserWebAuthnAccessHandlers).not.toHaveBeenCalled()
})

it('denies clipboard reads only to pages an SSH host has driven (#29145)', async () => {
mocks.isRemoteCliDrivenGuest.mockImplementation((id?: number) => id === 7)
const install = await loadInstaller()
install(profileFor('persist:browsing-2'))
const sess = sessionsByPartition.get('persist:browsing-2')
if (!sess) {
throw new Error('Expected the browsing session')
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the installer registered exactly this handler on the fake session.
const requestHandler = sess.setPermissionRequestHandler.mock.calls[0]?.[0] as (
webContents: { id: number; getURL: () => string },
permission: string,
callback: (allowed: boolean) => void
) => void
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the installer registered exactly this handler on the fake session.
const checkHandler = sess.setPermissionCheckHandler.mock.calls[0]?.[0] as (
webContents: { id: number },
permission: string
) => boolean
const guest = (id: number) => ({ id, getURL: () => 'https://a.test/' })

const decisions: boolean[] = []
requestHandler(guest(7), 'clipboard-read', (allowed) => decisions.push(allowed))
requestHandler(guest(8), 'clipboard-read', (allowed) => decisions.push(allowed))
requestHandler(guest(7), 'clipboard-sanitized-write', (allowed) => decisions.push(allowed))

expect(decisions).toEqual([false, true, true])
expect(checkHandler({ id: 7 }, 'clipboard-read')).toBe(false)
expect(checkHandler({ id: 8 }, 'clipboard-read')).toBe(true)
})

it('cancels a download from a page an SSH host drives (#29145)', async () => {
mocks.isRemoteCliDrivenGuest.mockImplementation((id?: number) => id === 42)
const install = await loadInstaller()
install(profileFor('persist:browsing-3'))

expect(fireWillDownload('persist:browsing-3')).toEqual({ cancelled: true })
expect(mocks.handleGuestWillDownload).not.toHaveBeenCalled()
})

it('denies every request and check on a strict partition without WebAuthn handlers', async () => {
const install = await loadInstaller()
install(profileFor('orca-doc-preview'), { permissions: 'deny' })
Expand Down
23 changes: 20 additions & 3 deletions src/main/browser/browser-session-partition-policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
installBrowserWebAuthnAccessHandlers
} from './browser-webauthn-access'
import { noticeDocPreviewDownloadBlocked } from './doc-preview-download-block-notice'
import { isRemoteCliDrivenGuest } from './desktop-browser-bridge/remote-cli-driven-workspaces'

// Why: one shared installer keeps every partition's deny-by-default permission/download policies from drifting apart.
const configuredPartitions = new Set<string>()
Expand Down Expand Up @@ -48,15 +49,26 @@ function configureBrowserSessionUserAgentPolicy(sess: Session, installExceptions
)
}

// Why: an SSH host scripting a page must not read the desktop clipboard through it.
function deniesRemoteCliDrivenClipboardRead(permission: string, webContentsId?: number): boolean {
return permission === 'clipboard-read' && isRemoteCliDrivenGuest(webContentsId)
}

/** Drop only the installer memo; retired-session guards remain fail-closed. */
export function forgetBrowserSessionPartitionConfiguration(partition: string): void {
configuredPartitions.delete(partition)
}
const handleWillDownload = (
_event: Electron.Event,
event: Electron.Event,
item: Electron.DownloadItem,
webContents: Electron.WebContents
): void => {
// Why: a host-driven page could otherwise write a host-chosen file into this desktop's Downloads.
if (isRemoteCliDrivenGuest(webContents.id)) {
event.preventDefault()
console.info('[desktop-browser-bridge] refused a download from a page an SSH host drives')
return
}
browserManager.handleGuestWillDownload({ guestWebContentsId: webContents.id, item })
}

Expand Down Expand Up @@ -159,7 +171,9 @@ export async function installBrowserSessionPartitionPolicies(
)
return
}
const allowed = isAutoGrantedBrowserSessionPermission(permission)
const allowed =
isAutoGrantedBrowserSessionPermission(permission) &&
!deniesRemoteCliDrivenClipboardRead(permission, webContents?.id)
if (!allowed) {
const rawUrl = resolvePermissionNoticeUrl(webContents, details)
browserManager.notifyPermissionDenied({
Expand All @@ -170,7 +184,10 @@ export async function installBrowserSessionPartitionPolicies(
}
callback(allowed)
})
sess.setPermissionCheckHandler((_webContents, permission, _origin, details) => {
sess.setPermissionCheckHandler((webContents, permission, _origin, details) => {
if (deniesRemoteCliDrivenClipboardRead(permission, webContents?.id)) {
return false
}
if (permission === 'media') {
return hasSystemMediaAccess(details?.mediaType)
}
Expand Down
Loading
Loading