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

Electron preload w/ Bytenode encoded renderer #134

Closed
petef19 opened this issue May 1, 2021 · 22 comments
Closed

Electron preload w/ Bytenode encoded renderer #134

petef19 opened this issue May 1, 2021 · 22 comments

Comments

@petef19
Copy link

petef19 commented May 1, 2021

Running into issues using a preload script in Electron with Bytenode encoded renderer files.

Contextisolation is enabled, hence require is not available in index.html in renderer, so the renderer jsc files can't be decoded there (which is what I did before using preload).

Moved this to my preload script:

contextBridge.exposeInMainWorld(
    'api',
    {
        view: () => {
            const bytenode = require('bytenode');
            return require('./path/to/my.jsc');
        }
    }
);

I can then trigger this via IPC from the renderer:

<script defer>
    window.api.view();
</script>

This does work (I see some of the app's console logs), but the problem is that the jsc file is now being executed in the context of the preload script and NOT in the browser window, hence all other calls to the preload's api object (from the app) do NOT work, which is what my app needs.

Any solution for this ?

I also tried using bytenode.runBytecodeFile() and bytenode.runBytecode(), could not get this to work.

Thanks.

@OsamaAbbas
Copy link
Collaborator

This is an interesting question.

Try this workaround:

contextBridge.exposeInMainWorld(
    'api',
    {
        view: (global) => {
            const bytenode = require('bytenode');
            return require('./path/to/my.jsc')(global); // you may have to change your my.js file accordingly
        }
    }
);

and

<script defer>
    window.api.view(window);
</script>

I'm not sure if this will work. Let me know the result after you test it.

@petef19
Copy link
Author

petef19 commented May 1, 2021

@OsamaAbbas

did not work, app started (as stated above) but no access to window object. I did get two additional log outputs now:

Uncaught Error: require(...) is not a function     at VM93 index.html:15
VM93 index.html:15 [Deprecation] 'window.webkitStorageInfo' is deprecated. Please use 'navigator.webkitTemporaryStorage' or 'navigator.webkitPersistentStorage' instead.

But maybe I did not use your example correctly.

what do you mean by ?

// you may have to change your my.js file accordingly

Which parts would I have to change ? I assume you refer to using the passed in window object.

My app is an Angular app, so I'm not sure where to receive the passed in window reference... ?

@OsamaAbbas
Copy link
Collaborator

Can you please make a minimal app that I can test?

@OsamaAbbas
Copy link
Collaborator

OsamaAbbas commented May 3, 2021

OK, I have setup a hello-world project and tried to reflect your use case.

What I have found is that your issue is not specific to .jsc files. Even with plain .js file, they will run in node context, not the window context.

The only workaround that I can think of is similar to what I have suggested in my previous comment, but with a fix (because we should have not altered the global variable). So here is my setup and workaround:

// main.js

// ... the usual main.js electron file, from Quick Start Guide

function createWindow() {
  const win = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      preload: path.join(__dirname, 'preload.js'),
      contextIsolation: true
    }
  })

  win.loadFile('index.html')
}

// ... the rest of main.js file
<! -- index.html -->
  <script>
    function sayMyContext() {
      console.log('index.html')
    }

  window.api.method1(window)
  window.api.method2()
  window.api.method3(window)
  </script>
// preload.js

const bytenode = require('bytenode')
const { contextBridge } = require('electron')

function sayMyContext() {
      console.log('preload.js')
    }

contextBridge.exposeInMainWorld('api', {
  method1: (ctx) => {
    return ctx.sayMyContext() // this should log 'index.html', if we pass `window` object to the function
  },
  method2: () => {
    return sayMyContext() // this should log 'preload.js'
  },
  method3: (ctx) => {
    return require('./protected.jsc').method(ctx)
  }
})
// protected.js
exports.method = (ctx) => {
  return ctx.sayMyContext() // this should log 'index.html'
}

and I have compiled this file using the new --electron flag.
bytenode --electron --compile protected.js

This solution works as expected. Basically you pass your context to your api functions and use it from there. However, I have no experience with Angular, so you have to figure out how to incorporate this workaround with your specific case.

@petef19
Copy link
Author

petef19 commented May 3, 2021

@OsamaAbbas had to read this a couple of times, great stuff. ;-)

Can't use this with Angular, because AFAIK there is no way to expose a method that would allow to pass in the ctx object when the app is bootstrapped. External data can only be obtained once the app is loaded (and we'd then be in the preload script context with no access to window object).

Thanks again.

@OsamaAbbas
Copy link
Collaborator

So I'm closing this issue now. Let me know if you have any further questions.

Also, if you have a bare minimum example of Angular + electron, please share it and I might find another way around. I tried to google [angular with electron], but all I can find are complex projects with unbelievable amount of moving parts, each one needs hours and hours to figure out how to deal with!

@petef19
Copy link
Author

petef19 commented May 3, 2021

yeah, the learning curve in the beginning is a bit steep re Electron + Ng project, but ultimately it's super simple:

Ng is a modular JS framework that allows to build enterprise level JS apps via TS. Ultimately, it will be compiled to a JS app w/ an index.html and that will be placed inside the /app folder in Electron.

So, when the renderer/main window is loaded, that index.html loads then the Ng app.

This JS app is bootstrapped by Angular, loading all node_modules/dependencies etc for the various components and classes, it's a full fletched, large scale JS framework. And there is no way to pass any external data to it when it loads. Any data, can only be loaded after the app has been initialized either via http calls or from local storage.

Electron's exposed preload api can be accessed through the window object, which is accessible in the Ng components (which are like classes).

But, in our scenario that window object is not available in the preload context...

I've looked into whether one can pass external data to Ng before app is loaded, it's not possible, other than storing the object in local storage and then accessing it later. But in our scenario, local storage would not be available in the preload context.

Thanks.

@OsamaAbbas
Copy link
Collaborator

Consider using NW.js instead of Electron. It's much much better and it has a dedicated tool (nwjc) which compiles the browser-javascript.

@petef19
Copy link
Author

petef19 commented May 4, 2021

Consider using NW.js instead of Electron. It's much much better and it has a dedicated tool (nwjc) which compiles the browser-javascript.

yeah, switching the main platform to NW.js is a major step. I looked at NW.js before deciding to use Electron and one major factor was support and long-term life expectancy of the technology. Electron wins hands down in both regards.

So, if we can't encode the renderer anymore w/ using strict security settings, then gotta push more sensitive logic into the main process, as we can still encode that.

@OsamaAbbas
Copy link
Collaborator

Good luck!

@OsamaAbbas
Copy link
Collaborator

OsamaAbbas commented May 4, 2021

@OsamaAbbas had to read this a couple of times, great stuff. ;-)

Can't use this with Angular, because AFAIK there is no way to expose a method that would allow to pass in the ctx object when the app is bootstrapped. External data can only be obtained once the app is loaded (and we'd then be in the preload script context with no access to window object).

Thanks again.

@petef19 After some digging here and there, the reason why my workaround would work is not «we'd then be in the preload script context with no access to window object» as you mentioned. We do have an access as I should in my comment (we were able to log values that exists in window object). However, that access is read-only with contextIsolation: true. So angular would not work because it needs to change a lot in window object and its members.

Also, as a bonus:
I found that you can run your app like this electron -r bytenode . and set your main entry to main.jsc directly. And main.jsc would be compiled using bytenode --electron --compile main.js.

Still thinking if we can protect preload.js or render process scripts in general.

@petef19
Copy link
Author

petef19 commented May 5, 2021

@OsamaAbbas

However, that access is read-only with contextIsolation: true. So angular would not work because it needs to change a lot in window object and its members.

not sure if I understand this correctly, but my Ng app (when NOT encoded via bytenode) does work with contextIsolation: true and it can access the window object just fine to access the preload defined api. Also, why would Ng change the window object ?

I found that you can run your app like this electron -r bytenode .

So this avoids having to use main.js with main.src.jsc ? if so, then this only applies to development as a packaged app could not be instructed to run this way, right ?

Iow: can we ditch main.js and just use main.jsc in a packaged app ?

Still thinking if we can protect preload.js or render process scripts in general.

As per my post in the other thread here, I opened up a ticket w/ Electron. If they integrate that we can define the preload script directly inside main.js then that would protect all preload code once we bytenode encode main.src.js

@OsamaAbbas
Copy link
Collaborator

not sure if I understand this correctly, but my Ng app (when NOT encoded via bytenode) does work with contextIsolation: true and it can access the window object just fine to access the preload defined api. Also, why would Ng change the window object ?

In this case, how do you include the angular javascript files? Using <script> tags, right? So it will run in its correct context with full access.

In case of bytenode, we MUST require those javascript files. Now, without contextIsolation, the preload.js has a full access to the window object. But with contextIsolation set to true, it will have a limited, read-only access.

As for why Angular changes the window object? Because it needs to define variables and objects ... etc. Keep in mind that window object IS the context. So when you define let variable = 'value', this is more or less equivalent to window.variable = 'value'. Again, if we required angular javascript files using require function from preload.js with contextIsolation: true, they will run in the wrong context, and any tricks we do (such as passing window itself as ctx parameter and using it there) will not work, because it will be a read-only object. I tried even to define a mechanism from within the webpage itself (like window.defineVariable = (name, value) => { window['name'] = value; }) and it did not work. The Context Isolation is strict here and rightfully so. (If we find a workaround this isolation, it should be reported as a security issue!).

So this avoids having to use main.js with main.src.jsc ? if so, then this only applies to development as a packaged app could not be instructed to run this way, right ?

I guess you will have to write your own launcher (in order to add -r bytenode to the executable). But I have little experience with Electron, so I'm not sure how it will work.

As per my post in the other thread here, I opened up a ticket w/ Electron. If they integrate that we can define the preload script directly inside main.js then that would protect all preload code once we bytenode encode main.src.js

Yes, I saw your ticket there. I hope they implement it as you described. However, if they implemented it in a manner that depends on converting the function into its .toString() representation, it won't work! Because bytenode breaks .toString() by design. See here to understand what I mean.

@petef19
Copy link
Author

petef19 commented May 5, 2021

re Ng, I think I misunderstood this comment:

After some digging here and there, the reason why my workaround would work

but later you then say

So angular would not work because it needs

So I incorrectly hoped (I guess) that there is a workaround for Ng ;-)

Because bytenode breaks .toString() by design. See here to understand what I mean.

do you mean it's breaking it in general or only when a function is converted via .toString() ?

B/c I use .toString() in some places in main.src.js which is bytenode protected later on, and it did not break the code in at least one place where I know .toString() is used right at the start of the app. in this code

Array(50).fill(0).map(() => Math.random().toString(36).charAt(2)).join('')

@OsamaAbbas
Copy link
Collaborator

do you mean it's breaking it in general or only when a function is converted via .toString() ?

It breaks .toString() in case of functions and functions only. To be exact, it breaks Function.prototype.toString() function.

@petef19
Copy link
Author

petef19 commented May 5, 2021

@OsamaAbbas

I edited my post there and quoted you, more precisely paraphrased one of your messages. lmk if this is not correct, see here: https://github.com/electron/electron/issues/28981#issuecomment-832395854

@petef19
Copy link
Author

petef19 commented May 5, 2021

@OsamaAbbas

btw, I happened to stumble across your NW.js vs. Electron comparison on hackermoon from 2018 (appreciate the write up and comparison). I know this is off topic here, but since this is closed, maybe I can ask a couple of q's: ;-)

(1) is there an updated version of this comparison ? is your opinion the same today ?
(2) have you ever written a blog post about migrating from Electron to NW.js or have you ever have to migrate one to the other ?

Looking into other options again re our latest discussions. Appreciate the feedback and advise shared.

@OsamaAbbas
Copy link
Collaborator

(1) is there an updated version of this comparison ? is your opinion the same today ?

I'm still leaning towards NW.js. If I have to choose between them I would go with NW.js for sure.

But before taking this decision, I would also study the security vulnerabilities (an example here) in Electron and figure out their counterparts in NW.js and how to deal with them.

(2) have you ever written a blog post about migrating from Electron to NW.js or have you ever have to migrate one to the other ?

I have not. I have no experience with Electron so far other than trying to solve bytenode's issues.

@Buminta
Copy link

Buminta commented Aug 4, 2021

This problem come back with version 13.1.8. Please fix it and keep for new version! @OsamaAbbas.
I'm using from 13.1.4 - 13.1.7 It's working!

@OsamaAbbas
Copy link
Collaborator

This problem come back with version 13.1.8. Please fix it and keep for new version! @OsamaAbbas.
I'm using from 13.1.4 - 13.1.7 It's working!

Can you please open a new issue with a clear example that works in 13.1.4 - 13.1.7 and reproduces your error with 13.1.8?

@yyccQQu
Copy link

yyccQQu commented Aug 25, 2021

comlplie terminal

"complie:main": "bytenode --electron --compile --no-module ./src/renderer/main:pre.js",

then you coule get your exception
main:pre.js-main:pre.jsc

var path = require('path');
var paths = path.join(__dirname, "../", "main:pre.jsc");
console.log(paths);
bytenode.runBytecodeFile(paths);

@petef19
Copy link
Author

petef19 commented May 13, 2022

we're having a general discussion regarding the Electron settings and their implications here

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

No branches or pull requests

4 participants