Skip to content
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

feat: add option to replace traceback on errors #326

Merged

Conversation

maartenbreddels
Copy link
Contributor

Showing a traceback can be a security risk in a context such as Voila.
(for instance showing paths, usernames or values in the exception message)

Similar to what we do at notebook execution time at:
voila-dashboards/voila#758

here we also disable the transfer of tracebacks at runtime.

cc @SylvainCorlay @mariobuikhuizen

Showing a traceback can be a security risk in a context such as Voila.
(for instance showing paths, usernames or values in the exception message)

Similar to what we do at notebook execution time at:
voila-dashboards/voila#758

here we also disable the transfer of tracebacks at runtime.
Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me - thank you @maartenbreddels.

Copy link
Member

@Zsailer Zsailer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. 👍 Thanks, @maartenbreddels.

@Zsailer
Copy link
Member

Zsailer commented Nov 4, 2020

@maartenbreddels, would you be able to write a test for this? If not, we should open an issue to remind ourselves to write a test at a later time.

@maartenbreddels
Copy link
Contributor Author

If you give me some pointers, I could take a look, maybe you know of a few test that would look similar.

@kevin-bates
Copy link
Member

None of the current kernel tests send messages - they only test lifecycle management operations. It would be good to add such a test or two that exercises the channels handler and this change. I've gone ahead and created #330.

@maartenbreddels
Copy link
Contributor Author

Ok, great, then I prefer we merge this as is.

@kevin-bates kevin-bates merged commit b76acb8 into jupyter-server:master Nov 5, 2020
hMED22 pushed a commit to hMED22/jupyter_server that referenced this pull request Jan 23, 2023
…ce_traceback

feat: add option to replace traceback on errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants