search

Kong / insomnia

The open-source, cross-platform API client for GraphQL, REST, WebSockets, SSE and gRPC. With Cloud, Local and Git storage.
https://insomnia.rest
Apache License 2.0
33.71k stars 1.88k forks source link

Plugin remote access from electron renderer to main process #4664

Open techchrism opened 2 years ago

techchrism commented 2 years ago

Expected Behavior

Access to the Electron remote module api from a plugin running in a renderer Electron process.

Actual Behavior

Remote module times out when trying to connect to the main process.

Reproduction Steps

  1. Create a plugin making use of the new Electron remote module api: https://github.com/electron/remote
  2. Try making use of the api (eg. opening a browser window)
  3. Observe error as the module is unable to communicate with the main process

Is there an existing issue for this?

Additional Information

As part of the Insomnia Electron upgrade to 17 #4529 , the Electron remote module was removed in favor of the new @electron/remote module: https://www.electronjs.org/docs/latest/breaking-changes#removed-remote-module

Switching to the new module requires explicitly initializing the module in the main process so renderer processes are able to make use of it. Additionally, it must be enabled for each WebContents object. For more information, read https://github.com/electron/remote From what I can tell, Insomnia currently does not do this which limits the capabilities of plugins looking to integrate with Electron apis.

A plugin I had written made use of the remote api to open browser windows with electron-specific event hooks (more than what could be done with the Insomnia plugin dialog api) and this seems to no longer be an option.

Insomnia Version

2022.2.1

What operating system are you using?

Windows

Operating System Version

Windows 10 Home 21H2

Installation method

download from insomnia.rest

Last Known Working Insomnia version

2022.1.1

ntenczar commented 2 years ago

Ah, I am currently having this problem as well. I did try using the new @electron/remote module but ran into the same problems you did.

filfreire commented 2 years ago

Hi folks, thank you for reporting this! Apologies for not getting back to you sooner, the team had been discussing about this very same issue on and off for the past few weeks.

The remote module is powerful but it came with many security issues which means we cannot re-enable it for the time being. For example, here's electron/remote's comment on this:

⚠️ Warning! This module has many subtle pitfalls. There is almost always a better way to accomplish your task than using this module. For example, ipcRenderer.invoke can serve many common use cases.

While the plugin API is still experimental we try to keep it backwards compatibility with any changes we make but sadly we cannot guarantee that electron specific APIs will be available.

We're planning to find an alternative for this issue and also improve our plugin API as soon as possible.

I encourage you and anyone else with giving us any feedback and ideas on how to better solve this. A PR on an API that could help expose the same functionality is also welcome 🙇

techchrism commented 2 years ago

Hello,

I appreciate the update. From what Insomnia plugins are currently capable of, I believe the issue is not one of security but rather, convenience.

Currently, nodeIntegration is enabled for the BrowserWindow in which plugins run. This gives all plugins access to write to the filesystem or run arbitrary code on the host computer. I hope this is an intentional decision and that bringing this to your attention is not seen as a "we need to fix this". The plugin I've made, insomnia-plugin-valorant, takes full advantage of the node fs api to read files from disk and node libraries to make fetch requests without concern for CORS. Removing nodeIntegration will cripple this plugin and likely many others. I don't have the statistics on hand for how many plugins use npm modules that ultimately depend on nodejs but I imagine I'm not the only one.

That being said, with nodeIntegration enabled, disabling the remote module provides no additional security. It is, of course, theoretically possible (although likely incredibly difficult) to patch the Insomnia process in-memory to enable the remote module. Alternatively, the app.asar file could be patched on disk and Insomnia forcefully restarted. There are no secrets that Insomnia keeps nor capabilities it withholds that are impossible to obtain using just nodeIntegration.

Having the remoteModule enabled gives my plugin the ability to open a BrowserWindow and listen for redirect events, something likely too niche for a general Insomnia api. I've considered many workarounds (including the automated app.asar patching idea) but the pre-electron-17 method is the cleanest, most convenient method for both me and the user.

Please reconsider enabling the remote module and I hope you don't disable nodeIntegration. The removal of nodeIntegration would severely limit the capabilities of plugins and potentially stifle the ecosystem. The quantity and capabilities of plugins are why I recommend Insomnia over alternatives when talking with others.

Thank you.

ntenczar commented 2 years ago

Having the remoteModule enabled gives my plugin the ability to open a BrowserWindow and listen for redirect events

this is precisely what my plugin does, as well. Thanks for the writeup @techchrism, I agree with all the points you made.

gamache commented 2 years ago

This issue is steering a purchasing decision for me right now. I would love to hear an update from the team on the intended direction of the product.

techchrism commented 2 years ago

@ntenczar I found a workaround for our applications - the webview tag is enabled which allows for navigation events. It bypasses x-frame-options and allows me to get the data I'm looking for from a redirection. My code can be found here: https://github.com/techchrism/insomnia-plugin-valorant/blob/2719e28808b587dbd142a3037049d6fa85371ec4/src/info-providers/RiotAuthProvider.js#L107 If you want any assistance, I'd be happy to further explain my code.

While my plugin is working again, I would still appreciate enabling the remote module so I can better control cookies. Currently I cannot view or forcefully invalidate third-party http-only cookies.

ntenczar commented 2 years ago

oh hell yeah I will take a look at that later today. Would be awesome to get our plugin working again!

dimitropoulos commented 2 years ago

We were looking at this on the insomnia stream just now.

Take a look at https://github.com/Kong/insomnia/blob/develop/packages/insomnia/src/preload.js#L12 for the other things you can do. Since the remote package is deprecated, we're going to hopefully be expanding the things you can do over the IPC bridge in the future (like you see in preload.js).

techchrism commented 2 years ago

Hey @dimitropoulos, thanks for the update.

I watched through the stream and I still disagree that adding the remote module presents a security issue. If you're looking to remove nodeIntegration, then I would agree that having remote enabled would be a poor idea but with nodeIntegration enabled, the remote module does not expose any security-related capabilities. The reason it's recommended to use a more direct IPC approach is to ensure arbitrary websites (from point number 4 at https://nornagon.medium.com/electrons-remote-module-considered-harmful-70d69500f31#91ec) that are designed to be in a sandbox cannot run code in the main process. Because plugins run in the renderer process and third-party websites run in webview tags, these security concerns do not apply. Again, disabling the remote module is only useful for security if nodeIntegration is disabled.

I'm not sure if you're saying that the remote module is deprecated in Insomnia specifically or in general. To be clear, the @electron/remote module is a non-deprecated replacement for the deprecated, built-in remote module.

I checked out how Hyper does their plugins and they have something that I think may help to resolve this. Their plugin system offers plugins the ability to run code both in the main process and in the renderer process. This would allow plugins to set up custom IPC hooks for the renderer side of the plugin. This would actually be more ideal than having the remote module enabled because it would give the same level of flexibility without compromising speed and efficiency (which, in this situation, are the true drawbacks of the remote module).

During my adventures of finding possible workarounds, I looked at the preload script to see if I could use the writeFile call to patch the app.asar while Insomnia was running (since the main process has a write lock on the file). If it was appropriate, I'd be happy to add and PR the cookie-related functionality I'm missing as IPC calls but I feel it's likely too niche.

I really like the Hyper approach of allowing code to be run in the main process and if this is a direction the team deems appropriate, I'd be happy to PR that behavior if I have the time.

filfreire commented 2 years ago

Hi @techchrism,

Regarding what you mention here:

I really like the Hyper approach of allowing code to be run in the main process and if this is a direction the team deems appropriate, I'd be happy to PR that behavior if I have the time.

Feel free to contribute a PR, we're curious to see what this would look like.