-
Notifications
You must be signed in to change notification settings - Fork 90
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
Explain why xhost is needed and address security concerns #3
Comments
Hi @gotjoshua thanks for bringing that to our attention. I didn't write the Dockerfile, it was provided by a contributor. Honestly, it doesn't work on my system, so I can't really test it out for alternatives. What do you suggest that we add to the documentation? |
isn't this issue in the docs repo? i actually don't understand it enough to know what to write, thats why i asked... |
Oh geez, I'm sorry! Hahaha, my bad :-D Well, I don't anything about it. The whole Dockerfile was a PR, and I didn't even know what xhost was. I looked into it through your links, and didn't see much to concern me. So that's why I asked you. :-) Hopefully that Docker container won't be needed soon... |
refactor: Docker section and discourage usage of xhost (#3)
Hi @gotjoshua , @PabloCastellano has made improvements based on the feedback. Can you check if your concern is resolved? |
Looks good to me! |
Thanks for confirming. Closing the issue now |
The docker setup includes these two lines:
I've set up a fair number of docker apps, and haven't seen it before, so it raises a yellow flag.
seems harmless enough in this context but some more details about why its needed will be great.
The text was updated successfully, but these errors were encountered: