Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use unique socket per user for managing editor sessions #6278

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

sleexyz
Copy link
Contributor

@sleexyz sleexyz commented Jun 20, 2023

Fixes #6275

@sleexyz sleexyz requested a review from a team as a code owner June 20, 2023 20:46
@codecov
Copy link

codecov bot commented Jun 20, 2023

Codecov Report

Merging #6278 (351d9b4) into main (fdeaba9) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6278      +/-   ##
==========================================
+ Coverage   73.56%   73.59%   +0.02%     
==========================================
  Files          31       31              
  Lines        1861     1863       +2     
  Branches      399      399              
==========================================
+ Hits         1369     1371       +2     
  Misses        415      415              
  Partials       77       77              
Impacted Files Coverage Δ
src/node/main.ts 51.04% <100.00%> (+0.51%) ⬆️
src/node/vscodeSocket.ts 87.50% <100.00%> (+0.12%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d3c9ed...351d9b4. Read the comment docs.

@code-asher code-asher merged commit b5a9ef8 into coder:main Jun 21, 2023
10 checks passed
@sleexyz sleexyz deleted the unique-socket branch June 21, 2023 19:43
@code-asher
Copy link
Member

code-asher commented Jun 21, 2023

I just realized we forgot to update the patch. Maybe we can add the path the same way we pass down some other custom things like disable-update-check.

@code-asher
Copy link
Member

Or maybe just an environment variable to make things easy.

@code-asher
Copy link
Member

I am throwing up a PR soon with that plus a CLI flag to override.

@mirekphd
Copy link

mirekphd commented Jun 26, 2023

Commenting @bilogic: Yes, to avoid misconfigurations such as that reported in #6275 I'd not try to write to /tmp anymore, as this may be a shared (multi-tenant) folder. It's also important not to write in root-owned locations (as that would introduce running as root requirement or add yet another root-owned folder to "weaken" in hardened setups such as OKD/OCP).

I think a good candidate folder for unique user socket data seems to be ~/.local/share/.

Being inside each user's home folder, it will be isolated between multiple users, and will be also protected from data loss during container restart (because home folders are mapped to persistent storage).

Jupyter Notebook / Lab and various apps write here already their temporary (session) data:

(base) coder@5800d4264eeb:~$ ls -lant /home/jovyan/.local/share/
total 28
drwx------ 4 1177 0 4096 May 24 07:51 jupyter
drwx------ 2 1177 0 4096 May 20 21:33 mc
drwxr-xr-x 2 1177 0 4096 Mar  8 11:59 applications
drwx------ 2 1177 0 4096 Oct 21  2022 nano
[..]

(base) coder@5800d4264eeb:~$ ls -lant /home/jovyan/.local/share/jupyter/runtime/
total 124
-rw-r--r-- 1 1177 0  670 Jun 25 12:40 nbserver-7-open.html
-rw-r--r-- 1 1177 0  258 Jun 25 12:40 nbserver-7.json
drwx-----T 2 1177 0 4096 May 24 20:51 .
-rw------- 1 1177 0  263 May 24 20:51 kernel-bfd0bc64-b49c-43ac-b61b-afd83982b1f5.json
drwx------ 4 1177 0 4096 May 24 07:51 ..
-rw------- 1 1177 0  263 May 21 08:00 kernel-8a9eaff3-0495-4436-941a-ae70cdd45633.json
-rw------- 1 1177 0  263 May 20 11:08 kernel-1a10d2e2-7bab-459b-9a6e-54ef4f9552a3.json
-rw------- 1 1177 0  263 May 20 10:34 kernel-012dcb64-a5b0-4319-b571-ab7392d9b09f.json
-rw------- 1 1177 0  263 May 20 10:34 kernel-96b35e09-7e0b-4f71-ab4b-da8532027482.json
-rw

Environment variables are a good idea too (very container-friendly), and they are automatically less persistent (they won't persist against designer's will after system / container crashes like files do). I'm not sure about their isolation between multiple users on the same host though.

@code-asher
Copy link
Member

code-asher commented Jun 26, 2023 via email

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.

[Bug]: Can't start 2 instances of code-server 4.14.0 for separate users
3 participants