fix(desktop): honor display and stream options on create() and stream.start()

.changeset/desktop-display-stream-options.md

@@ -0,0 +1,6 @@
+---
+'@e2b/desktop': patch
+'@e2b/desktop-python': patch
+---
+
+JS: honor the `display` option when `Sandbox.create()` is called with options only, and run noVNC on the `vncPort`/`port` passed to `stream.start()`. Python: generate a stream password when `stream.start()` is called again with auth still enabled, instead of storing the literal password `None`.

packages/desktop-js/src/sandbox.ts

@@ -185,7 +185,7 @@ export class Sandbox extends SandboxBase {
         : { template: this.defaultTemplate, sandboxOpts: templateOrOpts }
 
     // Add DISPLAY environment variable if not already set
-    const display = opts?.display || ':0'
+    const display = sandboxOpts?.display || ':0'
     const sandboxOptsWithDisplay = {
       ...sandboxOpts,
       envs: {
@@ -625,14 +625,10 @@ class VNCServer {
   private url: URL | null = null
   private novncHandle: CommandHandle | null = null
   private password: string | undefined
-  private readonly novncCommand: string
   private readonly desktop: Sandbox
 
   constructor(desktop: Sandbox) {
     this.desktop = desktop
-    this.novncCommand =
-      `cd /opt/noVNC/utils && ./novnc_proxy --vnc localhost:${this.vncPort} ` +
-      `--listen ${this.port} --web /opt/noVNC > /tmp/novnc.log 2>&1`
   }
 
   public getAuthKey(): string {
@@ -697,7 +693,10 @@ class VNCServer {
     const vncCommand = await this.getVNCCommand(opts.windowId)
     await this.desktop.commands.run(vncCommand)
 
-    this.novncHandle = await this.desktop.commands.run(this.novncCommand, {
+    const novncCommand =
+      `cd /opt/noVNC/utils && ./novnc_proxy --vnc localhost:${this.vncPort} ` +
+      `--listen ${this.port} --web /opt/noVNC > /tmp/novnc.log 2>&1`
+    this.novncHandle = await this.desktop.commands.run(novncCommand, {
       background: true,
       timeoutMs: 0,
     })

packages/desktop-js/tests/readiness.test.ts

@@ -91,3 +91,26 @@ test('waits between readiness probes that exit unsuccessfully', async () => {
   expect(timeout).toHaveBeenCalled()
   expect(run).toHaveBeenCalledTimes(2)
 })
+
+test.each([
+  ['options only', () => Sandbox.create({ display: ':1' })],
+  ['template and options', () => Sandbox.create('desktop', { display: ':1' })],
+])('starts the desktop on the requested display (%s)', async (_, create) => {
+  const sandbox = {
+    _start: vi.fn().mockResolvedValue(undefined),
+    kill: vi.fn().mockResolvedValue(undefined),
+  }
+  const baseCreate = vi
+    .spyOn(BaseSandbox, 'create')
+    .mockResolvedValue(sandbox as unknown as BaseSandbox)
+
+  await create()
+
+  expect(baseCreate).toHaveBeenCalledWith(
+    'desktop',
+    expect.objectContaining({
+      envs: expect.objectContaining({ DISPLAY: ':1' }),
+    })
+  )
+  expect(sandbox._start).toHaveBeenCalledWith(':1', expect.anything())
+})

packages/desktop-js/tests/stream.test.ts

@@ -0,0 +1,69 @@
+import { afterEach, expect, test, vi } from 'vitest'
+
+import { Sandbox } from '../src'
+
+afterEach(() => {
+  vi.restoreAllMocks()
+})
+
+async function startedDesktop() {
+  const run = vi.fn(async (cmd: string) => {
+    if (cmd === 'pgrep -x x11vnc') {
+      return { stdout: '' }
+    }
+    return { stdout: '', kill: vi.fn(), disconnect: vi.fn() }
+  })
+  const sandbox = Object.create(Sandbox.prototype) as Sandbox
+  // startXfce4() and _start() are not public; create() calls them.
+  const internals = sandbox as unknown as {
+    startXfce4(): Promise<void>
+    _start(display: string): Promise<void>
+  }
+  Object.defineProperty(sandbox, 'commands', { value: { run } })
+  vi.spyOn(sandbox, 'getHost').mockImplementation(
+    (port: number) => `${port}-sbx.e2b.app`
+  )
+  const waitAndVerify = vi
+    .spyOn(sandbox, 'waitAndVerify')
+    .mockResolvedValue(true)
+  vi.spyOn(internals, 'startXfce4').mockResolvedValue(undefined)
+
+  // Sets up sandbox.stream the same way create() does.
+  await internals._start(':0')
+  run.mockClear()
+  waitAndVerify.mockClear()
+
+  return { sandbox, run, waitAndVerify }
+}
+
+test('stream.start() runs noVNC on the requested ports', async () => {
+  const { sandbox, run, waitAndVerify } = await startedDesktop()
+
+  await sandbox.stream.start({ vncPort: 5901, port: 6081 })
+
+  const commands = run.mock.calls.map(([cmd]) => cmd)
+  const x11vnc = commands.find((cmd) => cmd.startsWith('x11vnc -bg'))
+  const novnc = commands.find((cmd) => cmd.includes('novnc_proxy'))
+
+  expect(x11vnc).toContain('-rfbport 5901')
+  expect(novnc).toContain('--vnc localhost:5901')
+  expect(novnc).toContain('--listen 6081')
+  expect(waitAndVerify).toHaveBeenCalledWith(
+    expect.stringContaining(':6081 '),
+    expect.any(Function)
+  )
+  expect(sandbox.stream.getUrl()).toContain('https://6081-sbx.e2b.app/')
+})
+
+test('stream.start() keeps the default ports when none are given', async () => {
+  const { sandbox, run } = await startedDesktop()
+
+  await sandbox.stream.start()
+
+  const novnc = run.mock.calls
+    .map(([cmd]) => cmd)
+    .find((cmd) => cmd.includes('novnc_proxy'))
+
+  expect(novnc).toContain('--vnc localhost:5900')
+  expect(novnc).toContain('--listen 6080')
+})

packages/desktop-python/e2b_desktop/main.py

@@ -171,7 +171,9 @@ def start(
         self._vnc_port = vnc_port or self._vnc_port
         self._port = port or self._port
         self._novnc_auth_enabled = require_auth or self._novnc_auth_enabled
-        self._novnc_password = self._generate_password() if require_auth else None
+        self._novnc_password = (
+            self._generate_password() if self._novnc_auth_enabled else None
+        )
 
         # Update URL with new port
         self._url = f"https://{self.__desktop.get_host(self._port)}/vnc.html"

packages/desktop-python/tests/test_stream.py

@@ -0,0 +1,54 @@
+from unittest.mock import Mock
+
+from e2b import CommandExitException
+
+from e2b_desktop import Sandbox
+from e2b_desktop.main import _VNCServer
+
+
+def stopped_desktop():
+    desktop = Mock(spec=Sandbox)
+    desktop._display = ":0"
+    desktop.get_host.side_effect = lambda port: f"{port}-sbx.e2b.app"
+    desktop._wait_and_verify.return_value = True
+
+    def run(cmd, **kwargs):
+        if cmd == "pgrep -x x11vnc":
+            raise CommandExitException(stderr="", stdout="", exit_code=1, error=None)
+        return Mock(stdout="")
+
+    desktop.commands.run.side_effect = run
+    return desktop
+
+
+def commands(desktop):
+    return [call.args[0] for call in desktop.commands.run.call_args_list]
+
+
+def test_restart_with_auth_still_enabled_uses_a_fresh_password():
+    desktop = stopped_desktop()
+    stream = _VNCServer(desktop)
+
+    stream.start(require_auth=True)
+    first_key = stream.get_auth_key()
+    stream.stop()
+    desktop.commands.run.reset_mock()
+
+    # require_auth stays enabled from the first start()
+    stream.start()
+
+    key = stream.get_auth_key()
+    assert key and key != first_key
+    assert f"x11vnc -storepasswd {key} ~/.vnc/passwd" in commands(desktop)
+    assert not any("-storepasswd None" in cmd for cmd in commands(desktop))
+    assert any("-usepw" in cmd for cmd in commands(desktop))
+
+
+def test_start_without_auth_has_no_password():
+    desktop = stopped_desktop()
+    stream = _VNCServer(desktop)
+
+    stream.start()
+
+    assert not any("-storepasswd" in cmd for cmd in commands(desktop))
+    assert any("-nopw" in cmd for cmd in commands(desktop))