Vscode: node.js-free, sandboxed renderer

Created on 6 Mar 2020  路  10Comments  路  Source: microsoft/vscode

(from @bpasero) We want to explore a model where the workbench in Electron does not access node.js APIs. In that model the Electron renderer process runs in sandboxed mode, much like any browser tab would do. Privileged code has to either move into a backend process and be accessible via IPC communication or into a preload script that exposes certain methods to the page.

November/December

  • [ ] :runner: Remove dependency on node for startup path https://github.com/microsoft/vscode/issues/98682 @bpasero @deepak1556 @alexdima

    • [x] move off file:// protocol for serving static assets @deepak1556

    • [x] adopt V8 cached data option for browser windows @deepak1556

    • [ ] investigate to load webpacked node modules via script tags too (ASAR support?)

    • [ ] UNC support (related: electron/electron#25618)

    • [ ] V8 cached options support (needs Electron fix)

    • [ ] Startup Perf validation

    • [ ] Selfhosting

  • [ ] move or webpack node.js module dependencies of renderer

    • [ ] vscode-sqlite3 native non-context-aware - @bpasero

    • [x] native-keymap native context-aware - @alexdima _approach would be to move the code to the main process_

      Future Milestones


Goal: foundations for sandbox without enabling sandbox

  • [ ] move or webpack node.js module dependencies of renderer

    • [ ] spdlog native non-context-aware - Sandeep

    • [ ] chokidar native context-aware - @bpasero

    • [ ] vscode-nsfw native non-context-aware - @bpasero

    • [ ] node-pty native non-context-aware - Daniel

    • [ ] windows-process-tree native non-context-aware - Daniel (should move where terminal moves)

    • [ ] handshake native unsure context??? (--> depends on where from remote connection is made)

    • [ ] v8-inspect-profiler - Jo

    • [ ] graceful-fs - @bpasero

    • [x] vscode-proxy-agent (used only from extension host) - Christoph

  • [ ] move electron-browser modules into electron-sandbox by using preload APIs

    • [ ] rename electron-sandbox back to electron-browser once complete

    • [ ] enable strict layer rules for electron-browser (i.e. no node.js)

  • [ ] implement a mini-workbench for good splash screen support
  • [ ] Investigate localization support @dbaeumer

Backlog

Goal: enable sandbox

  • enable contextIsolation and use contextBridge for workbench preload script
  • Figure out how to support ASAR in built product? (ASAR support is hooked into Chrome network stack by Electron)
  • Use direct window to window communication APIs (e.g. could send telemetry from issue reporter through this channel as a test)
  • connect the renderer to a "server" process

    • leverage the shared process and new message channel API for this?

  • Explore to start the extension host process early from the "server" process and not wait for the renderer to ask for the connection
  • Should all windows disable devtools (devTools: false)?
  • Go through TODO@sandbox in code that still need cleanup

Backlog

Sandbox

Requirements

  • We run in the most secure environment (sandbox, context isolation, async IPC only, strict CSP)

Backlog

  • Bootstrap an environment for testing a minimal sandboxed workbench
  • Enable a remote connection to localhost for management and extension host
  • Establish IPC channels for cross-process communication

    • Main process (desktop integration)

    • Shared process

  • Reduce node/electron dependencies

    • Services



      • Logging (including output channels)


      • Hashing


      • Configuration (including resolver)


      • Storage


      • Environment (process.env)


      • Remote (connect to external remotes, tunnels)


      • Dialogs (including file pickers)


      • Integrity


      • Search (rely on extension host exclusively?)


      • Text Files (save as admin)


      • Context Menu


      • Extensions Management


      • Telemetry


      • Accessibility


      • Backups


      • Product URLs


      • Workspaces


      • User Data Sync


      • Authentication


      • Request (http/https fetch, proxy support)


      • Host


      • Lifecycle


      • Clipboard


      • Update


      • Menu


      • Keytar



    • Contributions



      • Workpsace Tags


      • Backup


      • Debugging (including extensions)


      • Webview


      • Extension Profiler


      • Running Extensions


      • Terminal (internal, external)


      • Remote Indicator


      • Selection Clipboard


      • Startup Performance


      • CLI integration (code command)


      • Issue reporting


      • Tasks


      • User data sync


      • Configuration export


      • Telemetry opt-out



Performance

Requirements

  • Do not regress in performance compared to the current solution

Backlog

  • Enable instant cached data production for fast startup timing as today (upstream?)

    • How do others (Slack, Teams) serve local content?

  • 馃弮 Investigate how to support direct renderer <-> background-process communication (upstream)

    • Leverage Chrome utility process with node.js environment and message channel API?

    • Needs support for child processes which Chrome currently does not support

  • Support the layout splash for fast perceived loading (web + desktop)
  • Understand the impact of having to use Uint8Array over Buffer
  • Measure throughput of file service via IPC compared to native solution

    • Implement a custom websocket factory for the management connection?

  • Verify we are benefiting from warmed-up Chrome proceses after cold startup for new windows
  • macOS: Investigate 8kb limit of chunks in streams (https://github.com/microsoft/vscode/issues/40681)

:runner: <webview>

Requirements

  • Process isolation
  • Usability (keybindings, mouse interaction)
  • Robust (can move in DOM while presering state)
  • Performance
  • Works offline (no need for a webview server)

Backlog

  • <webview> is unsupported in sandboxed renderers with contextIsolation:true

Keyboard Support

Requirements

  • native-keymap is being removed from the renderer
  • We support variants of keyboard layouts and provide same configuration flexibility as today.

Backlog

  • Investigate to switch to the web-based solution
  • Alternatively move the keyboard layout resolution to a privileged process

Localization Support

Requirements

  • Works in desktop using Node free renderer
  • Continue to support language packs via extensions

Backlog

  • Implement localization support for web [https://github.com/microsoft/vscode/issues/82595]
  • Investigate same solution for desktop

Diagnostics and Testing

Backlog

  • Support crash reporter for sandboxed renderers (using preload script)
  • Support integration and smoke tests (driver)
plan-item sandbox

Most helpful comment

A minimal workbench in sandbox and contextIsolation mode can now be run via scripts/code.sh --__sandbox. It provides a in-memory file system and stubbed services for many things that are not yet adopted:

image

All 10 comments

An "implict" node-dependency that's not listed here is code loading. Today, the loader uses the fs and vm-modules which allows for v8 cached data usage.

Bootstrap sandboxed workbench code pointers:

  • enable sandbox & contextIsolation in window.ts
  • load something like a workbench-sandboxed.html from doGetUrl
  • this will likely fail early on because of our AMD loader requiring node.js for cached data etc. and some other things we depend on from window.js so we need something like workbench-sandboxed.js and possibly workbench-preload.js that work properly in this mode
  • instead of vs/workbench/workbench.desktop.main load a vs/workbench/workbench.desktop-sandboxed.main here with those things commented out that are not yet adopted
  • instead of vs/workbench/electron-browser/desktop.main, require a vs/workbench/browser/desktop-sandboxed.main here that brings up the workbench without depending on node.js

@bpasero we shouldn't mix localization support in the Web with support for Node free renderer. We need a very different story for the Web whereas for a node free renderer we can still install the LP on the local disk. We can simply not access them from the renderer. So I will change the item (I have discussed it that way with @egamma as well)

A minimal workbench in sandbox and contextIsolation mode can now be run via scripts/code.sh --__sandbox. It provides a in-memory file system and stubbed services for many things that are not yet adopted:

image

Some notes about vscode-ripgrep from Rob:

  • It is not a native node module,
  • Its just a container for ripgrep binary
  • The module is not used from the render process but rather from the search process where it spawns the ripgrep binary.
  • From the remote scenario it is launched from the extension host process.

@bpasero given this we can move out the module from the transition list

Notes from discussion with Daniel:

node-pty

  • Plan is to move to a pure nodejs process host that would allow the capability to use worker threads, as currently there is a potential deadlock issue https://github.com/microsoft/node-pty/pull/415
  • Electron needs to provide this option, currently there is render <-> render api but we need nodejs host <-> renderer
  • We don't want to host this module in main or extension host

xterm, xterm-addon-*

  • These are just frontend to the terminal
  • There are no node components here and they already run on the web

windows-process-tree

  • This is used by the terminal to get the process names
  • It is ideal for this module to live wherever node-pty lives

A few more notes on node-pty:

  • This cannot live only under the extension host as that will impact terminal launch time.
  • Perhaps we need a general pure node process (off main or shared) which hosts native modules like node-pty, file watchers, etc.
  • Issue to move node-pty out onto another process https://github.com/microsoft/vscode/issues/74620 - this idea is also related to potentially keeping terminal processes around after VS Code is closed, if we went this route the general node process above wouldn't work.

@deepak1556

@bpasero given this we can move out the module from the transition list

Sort of, are we saying that search can move 100% into the extension host? If so, we should do this on master already and thus the ripgrep dependency would move out of the renderer into the extension host.

Perhaps we need a general pure node process (off main or shared) which hosts native modules like node-pty, file watchers, etc.

@Tyriar Would the shared process not be sufficient for this already? Does it have to be a child-process of the shared process?

I agree that e.g. the file watcher must be a child-process simply because it uses a lot of CPU cycles potentially, but not sure about others.

@bpasero it would be for most things. In order to fix this bug on Windows where closing a terminal can deadlock the UI thread https://github.com/microsoft/node-pty/pull/415, we need node-pty to support workers. Doing it with a web worker is an option, but node-pty is a node library and supporting web workers is a hack that I'd rather not do.

That was just an idea though, it probably makes more sense to have a node process just for managing terminal processes to consolidate logic between main (local terminals) and ext host processes (remote terminals).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

trstringer picture trstringer  路  3Comments

biij5698 picture biij5698  路  3Comments

shanalikhan picture shanalikhan  路  3Comments

NikosEfthias picture NikosEfthias  路  3Comments

lukehoban picture lukehoban  路  3Comments