refuse to run as root user #1115
Conversation
| @@ -445,6 +450,10 @@ def _log_format_default(self): | |||
| help="Set the Access-Control-Allow-Credentials: true header" | |||
| ) | |||
|
|
|||
| allow_root = Bool(False, config=False, | |||
There was a problem hiding this comment.
You just have config=False here, which make it not configurable. Just switch to config=True and it works.
|
Do you want to add a note id |
Yeah, we can try to make a test. Not sure it is worth though. |
|
@Carreau No need for tests on this. I was just explaining what Travis was :) |
|
Hey this is cool to do. Does Windows need to be cased off? |
|
@Carreau |
|
Hi @Secant !
|
| self.log.critical("Running as root is forbidden. Use --allow-root to bypass.") | ||
| self.exit(1) | ||
| except OSError as e: | ||
| pass |
There was a problem hiding this comment.
Is there some condition that we're expecting to catch here? Why would geteuid() fail? If we're catching this, I think we should at least log the error so that there's an indication that something has gone wrong.
There was a problem hiding this comment.
No geteuid raise OSError on windows (according to the docs). So the try/catch is the dealing with windows case.
There was a problem hiding this comment.
@Carreau What docs are those? I rebooted into Windows to check, and it ain't there:
>>> os.geteuid()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: module 'os' has no attribute 'geteuid'
There was a problem hiding this comment.
@takluyver Whoops, I read the docs too fast
Note All functions in this module raise OSError in the case of invalid or inaccessible file names and paths, or other arguments that have the correct type, but are not accepted by the operating system.
Should be excepting AttributeError, my bad!
| is_root = os.geteuid() == 0 | ||
| except AttributeError as e: | ||
| import ctypes | ||
| is_root = ctypes.windll.shell32.IsUserAnAdmin() == 1 |
There was a problem hiding this comment.
I wouldn't do this - admin users in Windows aren't really equivalent to root on Unix, and I don't think it makes sense to block them from running the notebook. Admin users on Windows are more like the sudoers group on Linux.
| if os.geteuid() == 0: | ||
| self.log.critical("Running as root is not recommended. Use --allow-root to bypass.") | ||
| self.exit(1) | ||
| except AttributeError as e: |
There was a problem hiding this comment.
I think it's probably OK here, but it's a good idea to put as little code as possible into a try block when you're catching NameError or AttributeError (any error, really, but especially these), because they can hide mistakes.
E.g. imagine if someone changed the logging level, but accidentally spelled it errorr. That's an attribute error - and the code will silently catch it and carry on as if nothing had happened, disabling the check completely.
|
Thanks, @Secant, welcome to the project! |
|
Thanks for taking care of this ! Sorry I was mostly offline this Week-end. |
Hi! I'm a undergraduate at UC Berkeley working with Matthias (@Carreau). This is to resolve #1074. Currently, running the notebook as root will cause the program to terminate, but the --allow-root flag isn't working/being recognized and I'm not sure why.