Upstream merge from vscode-server - #15335
Open
mbaynton wants to merge 3 commits into
Open
Conversation
Brings in: - rstudio/vscode-server#389: Isolate /proxy/ port access to the requesting session's own user Verifies the localhost port a /proxy/<port>/ request targets is owned by the session's uid before proxying HTTP or WebSocket traffic to it, closing a cross-user isolation gap on multi-user Workbench hosts (rstudio-pro#11470, vscode-server#388). Ownership is determined by reading /proc/net/tcp[6] for the listening socket's uid, without adding a native dependency. Enforcement fails open when unavailable (non-Linux, root sessions, unreadable /proc) and fails closed on a uid mismatch, returning 403 for HTTP and destroying the socket for WebSocket upgrades. A --www-proxy-localhost-verify-port-owner flag disables the check if needed. Tests are ported as Vitest (socketOwnership.vitest.ts, webClientServer.vitest.ts) rather than upstream's Mocha .test.ts, per Positron's convention for new Positron-owned server code.
|
All contributors have signed the CLA ✍️ ✅ |
|
E2E Tests 🚀 Why these tags?
More on automatic tags from changed files. |
Author
|
I have read the CLA Document and I hereby sign the CLA |
…erver.ts
webClientServer.ts computes APP_ROOT at module load via FileAccess.asFileUri(''),
which throws unless globalThis._VSCODE_FILE_ROOT is set. Bootstrap entry points set
this normally; a plain Vitest run doesn't go through one.
The "fails open" test read the real process uid via process.getuid(), but production code intentionally skips its warning log when uid === 0. CI containers commonly run as root, so the test failed there while passing locally. Stub process.getuid to a fixed non-zero uid instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upstream merge from vscode-server, bringing in rstudio/vscode-server#389.
Summary
Verifies the localhost port a
/proxy/<port>/request targets is owned by the requesting session's own user before proxying HTTP or WebSocket traffic to it. This closes a cross-user isolation gap on multi-user Workbench hosts (rstudio-pro#11470): without this check, one user's session could proxy into a port bound by another user's process on the same host.Ownership is determined by reading
/proc/net/tcp[6]for the listening socket's uid -- the same kernel data rserver'sNETLINK_SOCK_DIAG-based fix relies on, without adding a native dependency. Enforcement fails open when unavailable (non-Linux, root sessions, unreadable/proc) and fails closed on a uid mismatch, returning 403 for HTTP requests and destroying the socket for WebSocket upgrades. A new--www-proxy-localhost-verify-port-ownerflag disables the check if an environment needs to.New tests (
socketOwnership.vitest.ts,webClientServer.vitest.ts) are ported as Vitest rather than upstream's Mocha.test.ts, per Positron's convention for new Positron-owned server code.Other commits on
upstream/mainsince the last pull (#13497) were triaged and left out:rstudio.rstudio-workbenchversion bumps; Positron'sproduct.jsonalready pins an equal or newer version.github/,Jenkinsfile)Release Notes
New Features
Bug Fixes
/proxy/port access to the requesting session's own user on multi-user Workbench hostsValidation Steps
@:workbench @:web @:jupyter
python -m http.server 8123)./proxy/8123/through the Workbench-fronted URL -- expect a 403 "Access to the requested port is forbidden." response instead of a proxied response./proxy/8123/(their own port) -- expect normal proxying to succeed.--www-proxy-localhost-verify-port-owner=0disables the check (proxying succeeds regardless of port ownership).