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

Passing --enable-sandbox argument? #2562

Closed
kewde opened this issue Feb 7, 2018 · 21 comments
Closed

Passing --enable-sandbox argument? #2562

kewde opened this issue Feb 7, 2018 · 21 comments
Labels

Comments

@kewde
Copy link

kewde commented Feb 7, 2018

Is there a way to pass the --enable-sandbox argument to electron?

I know of the --enable-mixed-sandbox hack, but is there are better way?

@develar develar closed this as completed May 3, 2018
@develar
Copy link
Member

develar commented May 3, 2018

See https://github.com/electron/electron/blob/master/docs/api/app.md

@tanx
Copy link

tanx commented Jul 16, 2018

See https://github.com/electron/electron/blob/master/docs/api/app.md

Could you point to the part where --enable-sandbox can be set?

@develar
Copy link
Member

develar commented Jul 16, 2018

@tanx
Copy link

tanx commented Jul 16, 2018

https://github.com/electron/electron/blob/master/docs/api/sandbox-option.md

It is important to note that this option alone won't enable the OS-enforced sandbox. To enable this feature, the --enable-sandbox command-line argument must be passed to electron, which will force sandbox: true for all BrowserWindow instances.

Note that it is not enough to call app.commandLine.appendSwitch('--enable-sandbox'), as electron/node startup code runs after it is possible to make changes to chromium sandbox settings. The switch must be passed to electron on the command-line:

electron --enable-sandbox app.js

So it seems like OS sandboxing will not be enforced without the command-line arg.

@develar
Copy link
Member

develar commented Jul 16, 2018

@tanx I am not expert here. Other options are not option for you? "To create a sandboxed window, pass sandbox: true to webPreferences:"

@tanx
Copy link

tanx commented Jul 16, 2018

@tanx I am not expert here. Other options are not option for you? "To create a sandboxed window, pass sandbox: true to webPreferences:"

See: lightninglabs/lightning-app#75 (comment)

@tanx
Copy link

tanx commented Jul 16, 2018

I guess the more general question would be... is it possible to set cli args at all in electron-builder like in electron-packager? See: lightninglabs/lightning-app#75 (comment)

@develar
Copy link
Member

develar commented Jul 16, 2018

@tanx It is not possible technically without introducing special wrapper. lightninglabs/lightning-app#75 (comment) Please point me to docs.

@tanx If you will not find docs how it is possible using electron-packager (I guess it is not possible) and will still want to pass --enable-sandbox, we can discuss more specify about adding such support for targets (each target requires own solution, generic solution not possible).

@tanx
Copy link

tanx commented Jul 16, 2018

@tanx It is not possible technically without introducing special wrapper. lightninglabs/lightning-app#75 (comment) Please point me to docs.

@tanx If you will not find docs how it is possible using electron-packager (I guess it is not possible) and will still want to pass --enable-sandbox, we can discuss more specify about adding such support for targets (each target requires own solution, generic solution not possible).

Ok. Thanks for explaining. I was assuming that e.g. for Mac OS the *.app binary executes ./path/to/electron with certain cli args when starting the *.app and that this could somehow be configured?

@develar
Copy link
Member

develar commented Jul 16, 2018

@tanx Exactly. Short googling doesn't give me official answer from docs, but I found ProgramArguments

<key>ProgramArguments</key>
<array>
<string>--enable-sandbox</string>
</array>

You can try to configure your macOS build:

"build": {
  "mac": {
    "extendInfo": {
      "ProgramArguments": ["--enable-sandbox"]
    }
  }
}

@develar
Copy link
Member

develar commented Jul 16, 2018

To make clear — I am open to add this functionality for some targets, but I need to be sure that you are understand what you are asking for. So, if you clearly stated that the only way to enable sandbox it is passing CLI args and you need this functionality, I will add such option for some targets. For example, it is possible for all macOS targets, for Snap and AppImage targets for Linux. And not possible for any Windows targets without additional investigation and work.

@tanx
Copy link

tanx commented Jul 16, 2018

You can try to configure your macOS build:

Awesome. The proposed solution looks promising. Didn't know you could pass platform specific configuration like that. I'll test it out and report back.

To make clear — I am open to add this functionality for some targets, but I need to be sure that you are understand what you are asking for.

Basically just a flag that guarantees OS-level sandboxing will be enforced for all rendering processes/windows.

@develar
Copy link
Member

develar commented Jul 16, 2018

https://www.electron.build/configuration/mac/

extendInfo any - The extra entries for Info.plist.

What is not clear for me:

  • why official Apple docs didn't describe a way to pass args
  • how does it work if end user also passes args.

@kewde
Copy link
Author

kewde commented Jul 17, 2018

It's been a while since I looked into this, I recall testing some changes to electron-forge (electron/forge#411) which was used by electron-packager and I assumed that it might be a possible route there.

Looking back at my notes of that time, I figured that it would be easier to compile my own custom electron binaries with a patch to enable the sandbox by default. electron-builder supports a configuration option electronDist, allowing you to pass a custom binary. That way you only have to worry about compiling electron for all platforms instead of delving in the builders source codes etc.

@develar
Copy link
Member

develar commented Jul 17, 2018

I recall testing some changes to electron-forge

It is only for start command, not for packed application.

it would be easier to compile my own custom electron binaries with a patch to enable the sandbox by default

Please specify what targets do you use and want.

@develar develar reopened this Jul 17, 2018
@tanx
Copy link

tanx commented Jul 17, 2018

Please specify what targets do you use and want.

FWIW these are the targets we're currently building:
https://github.com/lightninglabs/lightning-app/blob/e35541b438ce420cae260a9d507d8b4fa4382c00/package.json#L82-L104

@kewde
Copy link
Author

kewde commented Jul 17, 2018

We use mac, window (ia32 & x64) & linux (x64), I can provide early testing for those by the way.
Here's a thinned down configuration:

    "mac": {
      "target": [
        "dmg",
        "zip"
      ]
    },
    "nsis": {
      ...
    },
    "win": {
      "artifactName": "${name}-${version}-${os}-${arch}.${ext}",
      "target": [
        {
          "target": "nsis",
          "arch": [
            "x64",
            "ia32"
          ]
        },
        {
          "target": "zip",
          "arch": [
            "x64",
            "ia32"
          ]
        }
      ]
    },
    "linux": {
      "maintainer": "particl contributors <hello@particl.io>",
      "artifactName": "${name}-${version}-${os}-${arch}.${ext}",
      "target": [
        {
          "target": "deb",
          "arch": [
            "x64"
          ]
        },
        {
          "target": "tar.gz",
          "arch": [
            "x64"
          ]
        }
      ],
      "desktop": {
        "Exec": "/opt/particl/particl %U",
      },
    }
}

The desktop configuration does allow for some trickery, there is the option to add --enable-sandbox to the Exec commmand but it isn't the most reliable way.

@develar
Copy link
Member

develar commented Jul 17, 2018

there is the option to add --enable-sandbox to the Exec commmand but it isn't the most reliable way

Because user can start app directly, right?

@kewde
Copy link
Author

kewde commented Jul 17, 2018

Being able to pass parameters to the underlying electron binary is a neat feature.
It does seem a slippery slope with all the different targets, I can't imagine it is possible on zip, tar.gz, etc..

A sandbox option that would download prebuilt sandboxed binaries seems like a less flexible option but a lot more reliable and perhaps easier to implement.

@vladimiry
Copy link
Contributor

vladimiry commented Apr 30, 2019

Electron v5 comes with OS-level sandbox enabled by default. And so there is a need to disable it in some cases by passing --no-sandbox argument.

Here is another case I needed to pass an argument to the app, which is not the same as doing that through app.commandLine.appendSwitch.

I'd suggest renaming to issue to something like Allow embedding custom CLI arguments as passing --enable-sandbox argument is not the only case.

I just realized there might be a conflict of embedded and directly passed arguments. A directly passed argument should probably have a priority. I think this task might be outside of electron-userland's scope as it looks like there will be a need to introduce some kind of loader that will load needed arguments from the config file and the call the app with those arguments. So users could edit the arguments config file.

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

No branches or pull requests

4 participants