[DRAFT] Don't mount host $HOME if custom home specified #230
Conversation
| @@ -379,8 +379,6 @@ generate_command() { | |||
| # Mount also the distrobox-init utility as the container entrypoint. | |||
| result_command="${result_command} | |||
| --env \"SHELL=${SHELL:-"/bin/bash"}\" | |||
| --env \"HOME=${container_user_home}\" | |||
| --volume \"${container_user_home}\":\"${container_user_home}\":rslave | |||
There was a problem hiding this comment.
Both of these can be done later, since there is a new conditional following. HOME will not be set and then set again (overridden) later.
| if [ -n "${workdir##*"${HOME}"*}" ]; then | ||
|
|
||
| # Find $HOME as it is defined in the container (could be a custom home) | ||
| local home=$(${container_manager} inspect --type container --format='{{.Config.Env}}' "${container_name}" \ |
There was a problem hiding this comment.
I believe it's necessary to look into the container to know the custom --home directory?
| if [ "$home" != "$HOME" ] ; then | ||
| workdir="$(echo "$home" | sed -e 's/"/\\\"/g')" | ||
| else | ||
| workdir="$(echo "${PWD:-${home:-"/"}}" | sed -e 's/"/\\\"/g')" |
There was a problem hiding this comment.
The determination of workdir still feels a bit suspect (both here and in the original, IMHO). Let's discuss it?
NOTE in these two lines I have used the previous expressions, but using the lower-case $home that represents either $HOME or the custom home inside the container.
|
Hey @gunnarx thanks for the discussion! So thinking of it, in #28 we were discussing the introduction of some pre-made profiles for sandboxing (with manual flags if you want to do things other ways) What about instead of this, simply implement a |
Yeah, although I feel it would be logical to not map host-home when a custom home is provided. No matter what the solution, it seems tricky to decide on an appropriate --workdir that never fails. |
|
Assuming this gets reworked, how do you feel about the lookup of the custom home in the container to use that as the workdir? |
Well it depends on how you view the use of pet containers, coming from toolbox (which is the pioneer really of this type of approach) you have some seamless transition between container and host Custom homes were born because a lot of tools use $HOME to litter with their dots (eg, npm, pip, various app etc) so to keep a clean home on the host, custom $HOME is a solution This new flag (
I was thinking the same as you, this would be a much cleaner and sure way to extrapolate the home One thing that worries me are performances, doing an inspect can take up to 200ms on a slower machine, we already to an inspect in line 345-350: container_status=unknown
container_PATH="${PATH}"
eval "$(${container_manager} inspect --type container "${container_name}" --format \
'container_status={{.State.Status}};
{{range .Config.Env}}{{if slice . 0 5 | eq "PATH="}}container_PATH={{slice . 5 | printf "%q"}}{{end}}{{end}}')"
container_exists="$?"At this point I'd add here in the eval of the inspect another variable to extrapolate the HOME, so that we can do all with only one inspect command, maintaining down the distrobox enter time |
That makes sense. Well I think we've exhausted the discussion around the code I provided, so I would close and perhaps return to it with another code approach later. Thanks. |
|
Cool will wait for it 😉 |
This is more of a discussion item still but I felt some code might be easier to discuss around. Let me know if you want a discussion Issue instead.
I prefer that $HOME isn't mounted if a custom home was specified.
Choosing the --workdir seems not perfect (in both main, and in this PR), but in my view it should at least not be host $HOME by default, if another HOME has been specified...
Ref:
...noted one of the problems that can still appear, that --workdir set to a non-existing location will cause an error.
Sandboxing apps and isolating them from personal files in $HOME is actually my main reason for trying out distrobox, but it's not the primary direction of the project.
I feel would be good to protect the host's
$HOMEdir from applications in the container if a custom home was selected. Not mounting it at/home/<user>is a good start, but of course not ultimately secure since it is still at/run/host/home/<user>as things stand now.Ultimately, this is only a step towards Sandbox implementation which is an open issue, but I wonder if you also think changing to this behavior now is logical.
This behavior is at least what I expected, and the modification is what I use now. There's more tweaking possible probably so consider this a draft PR for discussion?
Ref: