search

sagemathinc / cocalc-docker

DEPRECATED (was -- Docker setup for running CoCalc as downloadable software on your own computer)
https://cocalc.com
Other
398 stars 103 forks source link

Cocalc runs chown -R on project homes as a part of initialization #126

Closed michalsta closed 1 year ago

michalsta commented 2 years ago

Which means that anyone who decides to make data on host system available by binding host's root into the Docker image inside the project's $HOME is going to end up with hosed host system, with key binaries (sudo, su, etc...) chown'd to some random UID. This is rather unintuitive and very dangerous behaviour, and should at least be documented (with big warnings in the README file), or, preferably, changed. Perhaps chown on project home that's careful not to cross volume boundaries, a'la find's -xdev option?

williamstein commented 2 years ago

I think it would be fine to just delete this behavior. It was mainly there for legacy reasons due to migrating from one uid schema to another, etc.. At this point, we haven't changed the user --> uid mapping in years and don't plan too.

Thus I think the best fix is to delete the line(s) of code that does this chown.

williamstein commented 2 years ago

Note to myself about how to do this. Basically rewrite the relevant 3 lines of code here:

https://github.com/sagemathinc/cocalc/blob/master/src/packages/hub/project-control/multi-user.ts#L97

Check if the HOME directory already exists. If so, don't make it and don't chown. If it does NOT exist, then make it and chown.

michalsta commented 2 years ago

Yeah I guess that would work too ;)

williamstein commented 1 year ago

I rewrote all the relevant code, so it's now here: https://github.com/sagemathinc/cocalc/blob/master/src/packages/server/projects/control/multi-user.ts

It doesn't look like it does any recursive chown anymore. It only chowns the HOME directory, but not all files inside of it.

I also just tested this directly and it doesn't recursively chown anymore.