Skip to content
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.

Get Rollup and Node builtin modules to work #884

Closed
geigerzaehler opened this issue Sep 9, 2020 · 6 comments
Closed

Get Rollup and Node builtin modules to work #884

geigerzaehler opened this issue Sep 9, 2020 · 6 comments

Comments

@geigerzaehler
Copy link
Contributor

At the moment, it is not possible to bundle packages with rollup that depend on Node builtin modules like event or util. If we start using packages that depend on this (e.g. web3) we’ll need to resolve this issue.

I’ve investigated rollup-plugin-node-polyfills to make builtins available in the bundle. However, I quickly ran into issue where the provided builtins were not compatible with the node builtins and packages depending on them would break. Moreover the plugin is not actively maintained and was last released a year ago.

There exists a fork rollup-plugin-node-polyfills2 on NPM but this package does have an associated repository and is not used very often so I’d avoid trusting it.

The solution that I’m suggesting is curating our own list of builtin module replacements that work building on browserify.

import * as browserifyNodeBuiltins from "browserify/lib/builtins";
const nodeBuiltinsPlugin = {
  name: "node-builtins",
  resolveId(importee) {
    if (importee == "util") {
      return { id: require.resolve("util/util.js") };
    }
    const builtinPath = browserifyNodeBuiltins[importee];
    if (builtinPath) {
      return { id: builtinPath };
    }
  },
};

import inject from "@rollup/plugin-inject";
const injecNodeGlobals = inject({
  modules: {
    process: "_process",
    Buffer: ["buffer", "Buffer"],
  },
});
@xla xla added this to the Housekeeping milestone Sep 9, 2020
@rudolfs
Copy link
Member

rudolfs commented Sep 28, 2020

Do I understand correctly that this would provide some of parts of the node API from the main process to libraries used in renderer?

So far we've been trying to not expose any node API to the renderer directly, because the renderer has to deal with untrusted content:

  • when rendering project README markdown;
  • opening external links from that rendered markdown.

To mitigate the risks, we currently try to isolate main and renderer by using a preload.js, as outlined here. As there is no reasonable way to disable navigating to external resources in electron, we capture will-navigate events and try to sanitise them as best as we can: https://github.com/radicle-dev/radicle-upstream/blob/master/native/main.js#L87.

How would the proposed changes would influence security here?
Would be great to have some extra eyes on this @NunoAlexandre, @xla, @kim, @cloudhead.

See more on locking down electron here.

@rudolfs
Copy link
Member

rudolfs commented Sep 28, 2020

Refs: #952.

@geigerzaehler
Copy link
Contributor Author

Do I understand correctly that this would provide some of parts of the node API from the main process to libraries used in renderer?

Security will not be impacted by adding replacements for builtin Node modules to the bundle. Adding replacements for Node builtins to the bundle will not expose any main process APIs to the renderer. For example, even if an fs module is added to the bundle it cannot provide access to the machines filesystem. (The separation is enforced by electron.)

@rudolfs
Copy link
Member

rudolfs commented Sep 29, 2020

@geigerzaehler thanks for the clarification!

@juliendonck
Copy link
Member

@geigerzaehler still relevant?

@geigerzaehler
Copy link
Contributor Author

This is still relevant and needs to be addressed by the ethereum integration work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants