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

Remove unstable native plugins #8490

Closed
bartlomieju opened this issue Nov 25, 2020 · 22 comments · Fixed by #10908
Closed

Remove unstable native plugins #8490

bartlomieju opened this issue Nov 25, 2020 · 22 comments · Fixed by #10908
Labels
cli related to cli/ dir deno_core Changes in "deno_core" crate are needed permissions related to --allow-* flags user feedback wanted feedback from the community is desired

Comments

@bartlomieju
Copy link
Member

Opening this issue to start discussion on potential removal of native plugins.

Native plugins are an unstable feature that hasn't seen much adoption; partially
due to lack of documentation and partially due to shortcomings of plugin interface.

Currently there's one major problem that prevents from using plugins in the same
way to use "ops" in Deno; namely code loaded from dynamic library cannot obtain
proper handle to current thread which renders Tokio unusable from the plugins (#7990).

Moreover the plugin system adds some complexity to Deno's op interface and brakes
the security promises of Deno (because we can't control what's going on in dynamically
loaded library).

Therefore I propose to completely remove native plugin interface in the next
minor release, revisit this topic after Deno 2.0, and in the meantime encourage
users to use WASM modules instead. I am aware that some of use cases for
plugins are not achievable using WASM (eg. using webview), but for that
purpose we're working on providing Rust "scaffold" for projects that want to
integrate deeply with Deno.

Related issues and PRs:

CC @ry @piscisaureus @eliassjogreen @manyuanrong @andreespirela

@bartlomieju
Copy link
Member Author

I will open a PR that removes all code related to plugins in the near future so we could evaluate its impact on the codebase.

@bartlomieju bartlomieju added cli related to cli/ dir deno_core Changes in "deno_core" crate are needed permissions related to --allow-* flags user feedback wanted feedback from the community is desired labels Nov 25, 2020
@bartlomieju bartlomieju changed the title Remove native plugins Remove unstable native plugins Nov 25, 2020
@bartlomieju
Copy link
Member Author

PR for reference #8493

@Soremwar
Copy link
Contributor

What does "Rust scaffold" means? Like a template for developing plugins or a library that can be used to integrate with Deno?

@eliassjogreen
Copy link
Contributor

Native plugins should fill the gap for modules that are not worth implementing into the deno core or are not general enough. While they haven't seen widespread adoption there is interest for such a feature and I think the reason is simply their immature state. I think replacing native plugins with ffi might be a better solution but I understand the issue remains the same.

The main (and my main) interest in deno plugins seems to come from the potential of using plugins for gui (webview/webgpu + windowing) and other os interactions (autopilot). The current pr for webgpu (#7977) could potentially fill this gap but would also require implementing a cross-platform windowing solution into deno core (unless the problem of windowing is passed to the user which in turn would require native plugins) which is far from ideal.

@andreespirela
Copy link

While I do understand every point made to why Deno should not have native plugins, it is worth saying that having plugins is more beneficial than we may think. Plugins offer a way to provide stability for applications with high requirements, one enterprise example would be: High-computing related tasks, these would be handled in the Rust side but call from a JS environment.

On this specific article: https://dev.to/andreespirela/how-mandarine-framework-could-surpass-nestjs-13c3 and this article: https://www.mandarinets.org/posts/making-mandarine-the-most-stable-framework-with-rust- I talk about leveraging JS with Rust and the benefits for it.

  • More stability since it allows you to bring stable functionalities from stable environments such as a database driver written in Rust into a no so-stable environment such as Deno Database drivers.
  • As mentioned by @eliassjogreen , Plugins allow you to bring modules that would not be so straightforward to write in JS, even when Deno would provide the necessary.

The way I see plugins is not really as an unstable piece of Deno, let me elaborate on this. It is very true that some parts of the plugin ecosystem are unstable and worth mentioning such as #7990 , although, plugins are different in its own nature, they provide a bridge between Rust (or c++) and JS which means, it is ok for them to have their own set of rules in order to work, for example: You cannot use Deno's main thread in your rust plugins, I think if there is documentation that states such issue as part of plugin's nature, and we keep treating Deno's plugins as something "Do at your own risk" , then we should not have an issue.

It is true having plugins break some concepts like the security Deno provides from inside, although, this kind of things should be on the user's side to decide: Not all users will use plugins nor Not all users will write modules in Rust, a good example of this would be a Deno Postgres driver & a Rust Postgres Driver used in Deno, where of course, the main recommendation is to use the Deno one.

I believe there is so much more to be discussed, but I am on the side that we should keep the plugins as they are right now, improve what we can improve, fix what we can fix. But removing the functionality will affect the ecosystem in terms of bringing modules that need some sort of system programming, and it will also affect the community in terms of leveraging enterprise needs with a typescript/JS ecosystem.

@eliassjogreen
Copy link
Contributor

I think maybe replacing the --allow-plugin flag with --allow-all would make it clearer that security cannot be guaranteed when using plugins.

@manyuanrong
Copy link
Contributor

There are indeed many problems with plug-in features at present, but these problems stem from actual use cases. Unless we are clear that we must not plug-in functionality (obviously not) instead of temporarily stranding, we should still keep it. More use cases and discoveries The problem prompts us to design better solutions, not to stop everyone from exploring. I think it is better to keep the status quo.

@nayeemrmn
Copy link
Collaborator

Is it clear that the existence of the current and flawed plugin interface hinders development and refactors on the rest of the codebase?

If so, I think that should be elaborated and clarified as the primary motivation of this issue since people are broadly defending the benefit of plugins or are otherwise confused as to why we would resort to removal. In particular, the security problem already exists with the subprocess interface and will likely exist for any good plugin interface.

If not, I agree with @manyuanrong.

@jiraguha
Copy link

I am not really in favor of removing it... why we do not just let it in the experimental phase in order to gain knowledge about how to secure it? Using an experimental flag like 'unsafe' is perfect for that!

@MierenManz
Copy link

I don't think that it should be removed. But leave it in with a warning that it probably will get a rewrite later on that might break the plugins developed for deno. But I think we should leave the feature implemented to see if people will develop plugins.
Maybe in half a year we can go back to this topic and see if people made plugins and if we want to remove plugin support or if we want to rewrite this feature

@andreespirela
Copy link

@MierenManz not even in the case that it’s not used it should be removed. The thing with plugins is that it isn’t a feature that many people will use as many people are not actively looking for rust solutions to be used in JS. The vast majority of people using NodeJS or Deno look for the creation of solution in JS/TS, Plugins are really something more advanced that we shouldn’t expect a great amount of people to work on them, but those who do should be enough.

The issue of removing plugins is really something beyond “how many people are using them right now”, personally I believe plugins should be kept by all means because of the very fact they are the only way to provide a bridge between a system language and JS, this is extremely important for enterprise needs and corporate adoption of Deno as calling Rust from the JS side allows a limitless amount of solutions just like high performance calculations for example (and that’s just a very vague example on how analytics company can use JS and Rust).

Levereging Rust and JS is possibly the most important “bridge” when it comes to enterprise adoption (opinion)

@MierenManz
Copy link

@andreespirela I agree with you on the part where we should keep it in, but only if it doesn't hurt performance and that we are sure alot of good development will happen in the plugin section of deno before Deno 2.0

Like Bartlomieju said:

I propose to completely remove native plugin interface in the next
minor release, revisit this topic after Deno 2.0, and in the meantime encourage
users to use WASM modules instead. I am aware that some of use cases for
plugins are not achievable using WASM

this would be a pretty good sollution I think. WASM can be compiled from alot of languages and even TS.
So it might be worth it to remove the rust plugins(for now) and maybe use WASM plugins and then revisit this topic later on

@manyuanrong
Copy link
Contributor

I agree and encourage the use of wasm in all scenarios that can be solved by wasm, or devote energy to the improvement of wasi. This can circumvent most of the current plugin problems. But for some scenes of native interaction, plugin mechanism is necessary, such as deno-ffi that I am currently improving, and deno-webview implemented by @eliassjogreen . deno-webview has a lot of attention, indicating that everyone has a lot of demand for this kind of scene.
So, my suggestion is to keep the current plugin feature

@ChayimFriedman2
Copy link
Contributor

I think that the plugin interface is not good, but the idea of plugins should exist. The lack of docs helps that.

We should think about a model that integrates better with Deno interface, especially around security. I don't have a concrete API in my head, but I imagine two things can be added to improve security with plugins:

  1. Allowing to load only specific plugins, by filename and maybe even cryptographically secure hash. This allows you to be sure that only trusted plugins will be loaded.
  2. Provide an API in the Rust side that allows a plugin to create custom permissions, and query existing permissions. So for example, a database driver allows you to specify --allow-dbs=my_db permission. This will require interpreting --allow- flags dynamically, and also change the (unstable) permissions API.

This way, plugins will integrate better with the security model of Deno leading to more adoption, IMO.

@tracker1
Copy link

I would go slightly further than @eliassjogreen suggests and make it --allow-unsafe

@eliassjogreen
Copy link
Contributor

@tracker1 Makes less sense to make a new --allow-unsafe permission for it instead of using --allow-all as someone could simply invoke a new process with whatever permissions wanted through the plugin making it less clear that the user is by extension allowing everything.

@tracker1
Copy link

@eliassjogreen I get it... just thinking that if "all" previously didn't allow plugins, that "unsafe" is a better meta, as you are allowing unsafe code, not just all security permissions.... --allow-unsafe === --allow-all | --allow-plugins.

@hayd
Copy link
Contributor

hayd commented Jan 14, 2021

There's no reason to tie allowing use of env/read/write etc from js (or read/write) when using a plugin. It's true that you are trusting the plugin (or maybe several specific plugins that could even be listed in the cli argument), and that plugins do sidestep the cli permissions, but using a plugin shouldn't mean you have to --allow-all for js.

@ghost
Copy link

ghost commented Jan 19, 2021

users to use WASM modules instead. I am aware that some of use cases for
plugins are not achievable using WASM

Wasm itself operates at the exact same privilege level as JavaScript; WebAssembly is weak. There is absolutely nothing that Wasm can do, that JS can't do slower.

Asm.js is a subset of JS, Wasm supersedes it. Native plugins and FFI calls are only necessary for what cannot be done in JS. To compare native code to Wasm is to compare apples to oranges, they are very different; any comparison between the two makes no sense in this context.

Security-wise, using solely JS, I could check Deno.build, then fetch an appropriate binary from the web, and execute it using Deno.run or something, how are plugins less safe than that?

Regardless, I support the issue that has been raised, to remove "unstable native plugins," is a good thing, as long as it paves the way to introducing "stable native plugins."

@jcc10
Copy link

jcc10 commented Feb 3, 2021

The only way that I could get behind something like this is if there was some way to get stuff like webview to work with whatever will replace it. I don't know how one would want to go about doing something like that though. If some sort of alternative could be provided via WASM/WASI, that would be fine, however I am unsure.

I would like to note that the Deno.run command (under --allow-run) combined with a websocket is a way around this without too much trouble. (Just tedium.) I think that plugins can be placed under the same permission since they are essentially the same thing just easier to work with.

It may be worth creating a example table of what different permissions can expose security wise. Possibly also a tool for that.

@d3x0r
Copy link

d3x0r commented Mar 29, 2021

WASM is not a solution to providing extensions that are platform specific.
GPU is getting fairly portable that wasm could nearly support it...

This is my project; and other than being used for work I do, and I'm not myself a corporation or corporate entitie, and most of this is solutions seeking problems; part of it overlaps internals, but has merits in being implemented as a homogenous external API.

https://github.com/d3x0r/sack.vfs

A feature a simple webserver script could do is just memory map a file, and pass that file to a socket handle without the buffer ever being copied to the JS heap (much less into a wasm heap).

There are a handful of OS specific extensions that Node (Deno?) can't really provide, as they are outside of the model it's based on... my system is purely interrupt driven; the platform abstraction I'm providing... which rather than being a OS itself just homogenouizes existing OS into a common experience for the developer.

It has a GUI too.. which is really just building with different npm scripts instead of a default; there's a separate branch for the GUI, but mostly it's just modifications to the docs to itself, and a change of the default build command.

I do rely on things like the OpenSSL library provided with Node - which is potentially a set of symbols you'll need to expose to plugins, but mostly it's bindings from C to V8, with a super thing C++ layer; although the C does just compile as C++....

I'm not a noob developer, but I really couldn't manage to figure how to get cargo linker to just build my C++ DLL and load it.

The entry point for plugins, for node, has been updated, because 'worker-threads' have their own isolates and contexts, so plugins need to be aware, so they can init for each context/isolate; I don't know if Deno even supports JS threads; because processes is THE model, right?

I'd be in favor of ripping it out, figuring out what it was providing, and reimplementing in a better way.

@samal-rasmussen
Copy link

I'm not super knowledgeable on this topic, but for anyone that finds this thread I believe it is relevant to know that there have been significant updates to the plugin api recently:

I was initially worried when I found this thread, but I looks like things are on the up and up for the plugins api. Though don't take my word for it. I would love it if someone on the Deno team could summarize the current state of the plugins api.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir deno_core Changes in "deno_core" crate are needed permissions related to --allow-* flags user feedback wanted feedback from the community is desired
Projects
None yet
Development

Successfully merging a pull request may close this issue.