search

ScottLogic / kdb-boothroyd

a prototype re-implementation of KDB Studio using JavaScript and Electron
Apache License 2.0
5 stars 2 forks source link

Disable auto-update #68

Open gyorokpeter opened 2 years ago

gyorokpeter commented 2 years ago

Currently the exe opens these connections: lb-140-82-121-3-fra.github.com https [2606:50c0:8000:0:0:0:0:154] https

Also when running the dev server as explained in the readme, "dev:react" listens on port 4000.

In a corporate environment these will be heavily scrutinized as being potential attack surfaces and may hinder onboarding.

OiNutter commented 2 years ago

I think the first connection is from the auto update functionality checking the github releases listing for any new versions. If that's likely to be a problem we'll need to look into ways of optionally turning that off.

I'm not sure what that second request is. The dev server one shouldn't be an issue though as I would expect users to be running the production version in those sorts of environments.

gyorokpeter commented 2 years ago

Changed title. I just noticed that auto update is active even if I run the exe from the "win-unpacked" directory and not the installed path. This is a blocker for enterprise use and a mere annoyance for home use. I think auto update should be disabled by default and it should need to be enabled manually by setting a build-time flag. A github action could build a downloadable installer that has it enabled.

OiNutter commented 2 years ago

Doing this via a build flag is not the way we want to go with this. The intention is users shouldn't have to build the app for themselves, they should use one of our compiled releases. I'm looking in to a couple of ways of disabling auto update if you don't want that functionality. One is building an msi installer for windows which according to the docs for the electron-updater package shouldn't include auto update, the other is a toggle in settings where the user can choose to turn this setting off.

gyorokpeter commented 2 years ago

As I mentioned, it would be fine to provide a separate installer that includes auto-update. That way we can simply opt to not use it. We will be building from source anyway. But when compiling from source, whoever does that would probably not want auto-update because if they want to update then they will pull the updated source from github. Also in an enterprise environment, any kind of unjustified TCP connection, especially to external servers, is a red flag because it can be an attack vector.

So to keep things simple, the source-based build should not have auto-update. Best would be to not even be able to enable it in the settings by the end user, even if it defaults to off, because some curious user will try changing every option to see what it does. And the build should default to not include the code, otherwise we will have to make customizations to disable it during onboarding.

ColinEberhardt commented 2 years ago

Hi @gyorokpeter - auto-update is a pretty standard feature for any desktop application, including developer productivity tools (e.g. VS Code). For that reason, our preference is that the KDB Boothroyd releases have this feature turned on as default.

As you will be building from source anyway, we'd be happy to make it easy to turn this off, so that you can simply add some configuration option to your build pipeline.

mcleo-d commented 2 years ago

As discussed in KDB meeting https://github.com/finos/kdb/issues/59 there should be a local config to disable auto-update for banking teams where this could be an infrastructure concern.

OiNutter commented 2 years ago

My initial investigation into the msi installer does show that it doesn't do auto-update, but I haven't had chance to test that it isn't still sending the requests out. I have also added auto-update as a configurable option in my work on adding a settings.json file.

OiNutter commented 2 years ago

Ok, I've run some basic checks with Fiddler. This isn't my area of expertise so I'm happy for someone to test and confirm this but when I run the app as installed by the msi installer, I see no network traffic, if I install through one of the nsis installers, I see the request to github releases. So it seems the msi installer will probably work well for large enterprise situations where we want auto update disabled by default.

I'll add that as a build option and next time we add code that will trigger a release there will be an msi installer available. In the meantime you can run:

npm run dist -- -w msi

To build the msi installer locally. I'm not sure if we need to create mac and linux installers that disable this by default as well?

OiNutter commented 2 years ago

MSI installer is now available for the latest release. Let me know if that works as one solution. Still working on the settings file stuff when I get chance.

gyorokpeter commented 2 years ago

We won't be using installers therefore I'm only interested in how to disable it at build time. The instructuions should be straighforward and shouldn't require research (e.g. include it in the "how to build" secion of the readme).

I have filled in the onboarding questionnaire and one of the questions is whether the software opens up external connections. I provisionally answered "no", but that means it must indeed not make any. If it turns out that it does, there could be severe consequences.