-
Notifications
You must be signed in to change notification settings - Fork 5.9k
change path for /home/coder/project #475
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
change path for /home/coder/project #475
Conversation
I think it's fine as long as it would generate warning message Furthermore, the danger can not be resolved by simply changing the volume. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not what your PR is supposed to do. And the fact you citated this is for security reasons, which is not really the aim of your change list, this is what you can do in your end instead:
- Change PWD to your target folder's directory.
- Add Authentication
This is not bad behavior, its just your not configuring it correctly.
True it's not bad default behavior so much as a bad configuration decision. I just don't see why the example in the README should be an example of a poor decision when it would be simple to make it a little better. |
You don’t need to follow the README 100%. We have official docs for this. |
Recommending a closure since this isn't what this PR is supposed to do as described. |
Sorry if I'm misunderstanding, but it seems like if following the README leads to what you said was me not configuring something correctly, that should be changed. I cannot imagine when mounting home to /home/coder/project would be a good idea, and for a lot of users home will be PWD when they run this command. Also, if I wanted to mount home, I would much rather specify that explicitly (ie ~) so I could run the same command every time. I changed my initial description to make what I mean more clear. Maybe I'm missing something, but is there any situation under which you would actually want to mount whatever random directory you currently happen to be in? |
I see what you're saying. Not sure exactly sure what a solution would be. Defaulting to |
This doesn't make things any more secure because the container terminal is still exposed on your network. Closing as we can't really help here, the command explicitly uses localhost so if you change it to be public, you need to understand the ramifications and #441 will add a warning. |
I couldn't cd to any other directory so I assumed there wasn't some other way to do it - I guess there is? @nhooyr |
There is no legitimate way to escape a container but it still happens occasionally due to vulns. Without a tuned docker setup, it's not a good idea to rely on containers for security. E.g. see https://access.redhat.com/security/vulnerabilities/runcescape I think the convenience is worth the marginal loss in security by mounting in $PWD. |
Ok I would never want to mount PWD but I see what you're saying. Thanks for the explanation! |
The first time I ran coder I copied and pasted the command from the README, changed the IP, and immediately exposed my home directory to the network. Since it's the first example most people will run and it's encouraged to "try it out," I think mounting PWD to /home/coder/project is a bad recommendation. I was certainly surprised when I realized what I'd done, and I think it would make more sense for the README to make a better recommendation.