diff --git a/.changeset/desktop-display-stream-options.md b/.changeset/desktop-display-stream-options.md new file mode 100644 index 0000000000..65137581bb --- /dev/null +++ b/.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`. diff --git a/packages/desktop-js/src/sandbox.ts b/packages/desktop-js/src/sandbox.ts index 4ee6fbea24..3213238b3a 100644 --- a/packages/desktop-js/src/sandbox.ts +++ b/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, }) diff --git a/packages/desktop-js/tests/readiness.test.ts b/packages/desktop-js/tests/readiness.test.ts index 1962b8dc5b..55ef156296 100644 --- a/packages/desktop-js/tests/readiness.test.ts +++ b/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()) +}) diff --git a/packages/desktop-js/tests/stream.test.ts b/packages/desktop-js/tests/stream.test.ts new file mode 100644 index 0000000000..7f32b00902 --- /dev/null +++ b/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 + _start(display: string): Promise + } + 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') +}) diff --git a/packages/desktop-python/e2b_desktop/main.py b/packages/desktop-python/e2b_desktop/main.py index 3ab7415bf4..08d097d147 100644 --- a/packages/desktop-python/e2b_desktop/main.py +++ b/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" diff --git a/packages/desktop-python/tests/test_stream.py b/packages/desktop-python/tests/test_stream.py new file mode 100644 index 0000000000..1a59f951d5 --- /dev/null +++ b/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))