voila icon indicating copy to clipboard operation
voila copied to clipboard

using voila specific kernel manager

Open maartenbreddels opened this issue 6 years ago • 10 comments

Now voila and the server extension both use a voila_kernel_manager instead of kernel_manager. This means that voila used as server extension also does not allow arbitrary code execution.

maartenbreddels avatar Dec 07 '18 15:12 maartenbreddels

Also allow the custom_message for voila standalone and server extension.

maartenbreddels avatar Dec 07 '18 15:12 maartenbreddels

Thinking about this a bit more, having a separate kernel manager for voila could also be very useful to:

  • show the "voila only" kernels (https://github.com/voila-dashboards/voila/pull/611), as clients could send a GET /voila/api/kernels when voila is used as a server extension, and will only get the voila kernels without the regular kernel sessions started via the notebook frontend
  • configure the idle culler to only cull the voila kernels when voila is used as a server extension (this was suggested by @maartenbreddels).

jtpio avatar May 28 '20 15:05 jtpio

I can rebase this PR onto the latest master tomorrow.

(cc @maartenbreddels let me know if you have any objections)

jtpio avatar May 28 '20 16:05 jtpio

Yes, please go ahead, I think we should have a different manager, with different defaults, different permissions, and different authentication.

maartenbreddels avatar May 28 '20 20:05 maartenbreddels

Something that crossed my mind: Would it be a different pool of kernels? Like would a kernel spawned by notebook or lab be visible to voila? I'm thinking about a future use case where you have a running notebook, and want to share that existing kernel via voila.

maartenbreddels avatar May 29 '20 08:05 maartenbreddels

I'm thinking about a future use case where you have a running notebook, and want to share that existing kernel via voila.

It looks like this should be possible at the moment with voila running as a server extension sharing the same kernel manager as the notebook server, by passing an existing kernel_id to the start_kernel?

https://github.com/voila-dashboards/voila/blob/b75e05f4041aaaf91b3ee9e3c08848136ebec5a3/voila/handler.py#L120

Although there will probably be some issues with the widgets (multiple clients for the same kernel)

Would it be a different pool of kernels? Like would a kernel spawned by notebook or lab be visible to voila?

If we isolate the voila kernel manager, then this becomes a bit trickier. Unless the voila kernel manager is able to proxy some of the messages to the other kernel manager (not sure yet if that's even possible right now).

jtpio avatar May 29 '20 09:05 jtpio

Although there will probably be some issues with the widgets (multiple clients for the same kernel)

Currently yes, but there are ways around that, and for sure it's going to be supported in the future :)

maartenbreddels avatar May 29 '20 09:05 maartenbreddels

I don't think that this resolves the arbitrary code execution issue though, because the raw jupyter server always exposes the base kernel API endpoint.

SylvainCorlay avatar May 29 '20 10:05 SylvainCorlay

A couple of things to cover before this change can be considered as "ready":

  • [ ] Fix failing test in tests/server/cwd_subdir_test.py (for reference: https://github.com/voila-dashboards/voila/runs/720801386?check_suite_focus=true)
  • [ ] Shutdown all voila kernels on Ctrl-C / SIGTERM when voila is used as server extension (they are not shut down automatically anymore now that there is a separate kernel manager instance)
  • [ ] Add test for the kernel namespacing when voila is used as a server extension (/api/kernels and /voila/api/kernels should return different results)

jtpio avatar Jun 02 '20 09:06 jtpio

Shutdown all voila kernels on Ctrl-C / SIGTERM when voila is used as server extension (they are not shut down automatically anymore now that there is a separate kernel manager instance)

The ExtensionApp.stop() method might be relevant here (although it seems to be specific to standalone mode?):

https://github.com/jupyter/jupyter_server/blob/f1df254e17854aecb0ef025a5891c6910c62fd70/jupyter_server/extension/application.py#L387-L391

jtpio avatar Jun 02 '20 12:06 jtpio

Closing as the proposed changes are not applicable anymore to the next release of Voila

trungleduc avatar Aug 01 '23 09:08 trungleduc