Skip to content

fix(desktop): shell-quote the target of open() and launch() - #2144

Open
PrabalParihar wants to merge 1 commit into
e2b-dev:mainfrom
PrabalParihar:fix/desktop-quote-open-launch
Open

PrabalParihar wants to merge 1 commit into
e2b-dev:mainfrom
PrabalParihar:fix/desktop-quote-open-launch

Conversation

@PrabalParihar

@PrabalParihar PrabalParihar commented Nov 14, 2026 •

Copy link
Copy Markdown

open() and launch() were putting the file or URL straight into the shell command, so a link with & in the query string got cut off and a path with spaces was split into two arguments. I quoted those values the same way write() already does, in both the JS and Python SDKs.

sandbox.open() and sandbox.launch() in the desktop SDKs (JS and Python) put the file, URL, application and URI straight into the shell command they run in the sandbox, without quoting. So a URL with & in its query string gets cut at the &, and a file path with spaces gets split into several arguments.

Problem

Sandbox commands run through /bin/bash -l -c <cmd>. open() builds xdg-open ${fileOrUrl} and launch() builds gtk-launch ${application} ${uri}, so bash parses the value itself:

  • open('https://example.com/search?q=e2b&page=2') runs xdg-open https://example.com/search?q=e2b in the background, then page=2 as a separate command. The browser opens the wrong page.
  • open('/home/user/Q3 report.pdf') passes /home/user/Q3 and report.pdf as two arguments.
  • A path with a ' or ( fails with a bash syntax error.
  • launch('google-chrome', 'https://example.com/?a=1&b=2') has the same & problem.

Computer-use agents often pass URLs with query strings to open(), so this comes up in normal use.

Change

  • JS: open() and launch() quote their arguments with the class's existing quoteString helper, which write() already uses.
  • Python: same change with shlex.quote (already imported as quote_string and used by write()).
  • launch() with no URI (or an empty one) still passes only the application, as before.
  • Changeset: patch for @e2b/desktop and @e2b/desktop-python.

Usage is the same; values that used to break now work:

await sandbox.open('https://example.com/search?q=e2b&page=2')
await sandbox.launch('google-chrome', 'https://example.com/?a=1&b=2')
sandbox.open("/home/user/Q3 report (Bob's copy).pdf")

Tests

The new tests replace commands.run with a recorder, then run the command the SDK built through bash -c, with xdg-open / gtk-launch stubbed by a shell function that prints each argument it receives. They check what the program would actually get, not the exact quoting.

  • packages/desktop-js/tests/openLaunch.test.ts (4 tests): URL with & and #, path with spaces/quotes/parentheses, launch with a URI, launch without a URI. On main, 3 fail (the URL arrives as https://example.com/search?q=e2b, the path gives a bash syntax error); with this change all 4 pass.
  • packages/desktop-python/tests/test_open_launch.py: the same 4 cases. 3 fail on main, all 4 pass here.
  • vitest run tests/openLaunch.test.ts tests/client.test.ts tests/readiness.test.ts: 18 passed. pnpm typecheck, pnpm lint (oxlint) and prettier --check src tests are clean.
  • uv run pytest tests/test_open_launch.py tests/test_client.py tests/test_readiness.py: 17 passed. ruff check, ruff format --check and ty check e2b_desktop are clean.
  • I didn't run the live-sandbox suites (apps, mouse, screen, test_controls) because they need an E2B_API_KEY.

Notes

This change was made with AI assistance; the tests listed above were run locally before opening this PR.


Devin Review

open() and launch() interpolated the file, URL, application and URI
straight into the shell command they run in the sandbox. A URL with `&`
in its query string was cut at the `&` (the rest ran as a separate
command), and a path with spaces or quotes was split or failed to parse.

Quote them with the quoting helper write() already uses (quoteString in
JS, shlex.quote in Python). launch() without a URI still passes only the
application.
@cla-bot

cla-bot Bot commented Nov 14, 2026

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement, and we don't have @PrabalParihar on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check'

@changeset-bot

changeset-bot Bot commented Nov 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 597da2d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@e2b/desktop Patch
@e2b/desktop-python Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-14T08:23:35.508831Z 597da2d PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TASTE.md review: complies — checked T-43 (shell-quote interpolated command args), T-1/T-2 (JS ↔ Python parity), T-3/T-69 (no signature or docstring changes introduced); 0 violations.

Written by Devin

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 597da2d8e7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// function that prints each argument it receives, one per line.
function argvSeenBy(program: string, command: string): string[] {
const stub = `${program}() { printf '%s\\n' "$@"; }; `
return execFileSync('bash', ['-c', stub + command], { encoding: 'utf8' })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep Bash subprocess tests out of the Cloudflare suite

The existing Cloudflare matrix leg in .github/workflows/desktop_js_tests.yml:82-91 runs the desktop suite inside workerd, and vitest.cloudflare.config.mts:75-76 includes this new file—the desktop Cloudflare config excludes only client.test.ts. Workerd cannot spawn Bash through node:child_process, and the Windows-only skip does not exclude that runtime, so these tests make the Cloudflare CI leg fail independently of the quoting behavior. Add this file to the desktop Cloudflare config's exclusions, or use Worker-compatible assertions.

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant