-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
refuse to run as root user #1115
Changes from all commits
3ce8d95
1d01627
349a7f0
43ff953
a11f81a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -346,6 +346,11 @@ def start(self): | |
""" | ||
) | ||
|
||
flags['allow-root']=( | ||
{'NotebookApp' : {'allow_root' : True}}, | ||
"Allow the notebook to be run from root user." | ||
) | ||
|
||
# Add notebook manager flags | ||
flags.update(boolean_flag('script', 'FileContentsManager.save_script', | ||
'DEPRECATED, IGNORED', | ||
|
@@ -445,6 +450,10 @@ def _log_format_default(self): | |
help="Set the Access-Control-Allow-Credentials: true header" | ||
) | ||
|
||
allow_root = Bool(False, config=True, | ||
help="Whether to allow the user to run the notebook as root." | ||
) | ||
|
||
default_url = Unicode('/tree', config=True, | ||
help="The default URL to redirect to from `/`" | ||
) | ||
|
@@ -1100,6 +1109,13 @@ def start(self): | |
|
||
This method takes no arguments so all configuration and initialization | ||
must be done prior to calling this method.""" | ||
try: | ||
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: | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there some condition that we're expecting to catch here? Why would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Carreau What docs are those? I rebooted into Windows to check, and it ain't there:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @takluyver Whoops, I read the docs too fast
Should be excepting AttributeError, my bad! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem! |
||
|
||
super(NotebookApp, self).start() | ||
|
||
info = self.log.info | ||
|
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.
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
orAttributeError
(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.