Simplify electron autoupdater by dajimenezriv-internxt · Pull Request #1361 · internxt/drive-desktop

dajimenezriv-internxt · 2026-04-23T15:24:14Z

What

Previously we were using the package electron-updater with uses the autoUpdate feature of electron. However, during an issue that sometimes we were having with the latest.yml and the hash I started investigating how the autoupdate was working by letting it work in my laptop. However, it was producing strange behaviours like this image when updating:

I was reading about how it worked, but the customization was very limited and it was working as a black box. I started lookign for alternatives and found a repo that was doing something similar without the hash verification. So, since the implementation it's "simple" and we just need to support windows, I decided to implement it manually so we can customize it as we want and decide when to install, when to ask the user, when to force the update and so.

I've added in this comment 2 videos showing how the update will be made and the dialog that we show the user. https://inxt.atlassian.net/browse/PB-6340?focusedCommentId=71094

dajimenezriv-internxt · 2026-04-24T14:12:18Z


 (Get-Content $yamlPath) `
-| ForEach-Object { $_ -replace '^(\s*sha512:\s*).+', "`$1$base64" } `
+| ForEach-Object { $_ -replace '^(\s*sha512:\s*).+', ('${1}' + $base64) } `


The build release script was sometimes generating wrongly the latest.yml because of the hash.

dajimenezriv-internxt · 2026-04-24T14:22:37Z

+    logger.debug({ msg: 'New release available', latest, filePath });
+
+    const installing = await checkExistingFile({ latest, filePath });
+    if (installing) return true;


We want to check if the release was already downloaded to install it so we don't need to download it again, we just install it and open the app again.

dajimenezriv-internxt · 2026-04-24T16:20:40Z


-  afterEach(() => {
-    vi.useRealTimers();
-  });


These are already removed with clearMocks: true in the vitest config.

Hmm, I think clearMocks: true calls vi.clearAllMocks() before each test. But vi.clearAllMocks() is not calling vi.useRealTimers, so timers won't be reset cause they have a separate API

sg-gs · 2026-04-27T07:24:27Z

I decided to implement it manually

Can we rollback to electron-autoupdater if any issue arises or needed in the future? Which things are lost (the won ones are clear) by choosing this path? And how reliable is it in case of a download corruption?

sg-gs · 2026-04-27T07:39:40Z

Adding @TamaraFinogina here to ensure there are no security regression issues here regarding authenticity, filesystem usage and security guarantees in general @dajimenezriv-internxt (against the current electron-autoupdater version)

dajimenezriv-internxt · 2026-04-27T08:15:47Z

@sg-gs Regarding the first comment. In case the download is corrupted we are checking against the hash, so it will be retried the download in the next startup. What we lose in case something doesn't work is that the user will need to install the release manually or create another fast release going back to electron-updater (the implementation was very simple). Apart from that, we lose that we are not using the default electron update package, however, as pointed in the description it was working a bit strange (that happened in my laptop) and since we only use windows and github releases it wasn't difficult to implement it manually and we can add as much customization as we want. I think that we don't lose anything, I've been checking but I haven't found any better thing in electron-updater. The only thing is that it was checking against the blockmap and not downloading the whole file (however it was failing everytime and downloading the entire file in the end).

TamaraFinogina · 2026-04-27T08:48:56Z

+    const res = await fetch('https://api.github.com/repos/internxt/drive-desktop/releases/latest');
+    const data = await res.json();
+    const release = data as { tag_name: string };
+    const latest = release.tag_name.replace(/^v/, '');


Shall we add a check that the tag doesn't have '../' or some weird characters? I think we have a fixed name for all tags, but even just checking for that tag only has letters/numbers should do

if (!/^[0-9A-Za-z._-]+$/.test(latest)) { throw new Error("Wrong tag format"); }

Actually, it's always with numbers v.X.Y.Z, so we can remove letters from the check

dajimenezriv-internxt · 2026-04-27T08:50:28Z

About a comment made by @TamaraFinogina, she asked me why don't use update-electron-app. Is another library I considered but in the end I discarded it because it didn't include hash validation. However, I was wrong, that library uses internally autoUpdater from electron that already includes hash validation. So, I tried the library:

    updateElectronApp({
      updateSource: {
        type: UpdateSourceType.ElectronPublicUpdateService,
        repo: 'internxt/drive-desktop',
      },
      updateInterval: '5m',
      logger: {
        error(message) {
          logger.debug({ msg: message });
        },
        warn(message) {
          logger.debug({ msg: message });
        },
        info(message) {
          logger.debug({ msg: message });
        },
        log(message) {
          logger.debug({ msg: message });
        },
      },
    });

but it gave me some issues:

We need to try the app packaged to test anything (since they do a check of app.isPackaged inside the library). This happen with electron-updater and this library, which is another reason why I discarded electron-updater in the first place. Every test needed the build of the app (it takes around 250s).
We have been building the app using the target nsis but update-electron-app requires squirrel:

[2026-04-27 10:40:23.845] { header: '  - b -     ', msg: 'feedURL' }
[2026-04-27 10:40:23.846] { header: '  - b -     ', msg: 'requestHeaders' }
[2026-04-27 10:40:23.848] { header: '  - b -     ', msg: 'updater error' }
[2026-04-27 10:40:24.415] {
  header: '  - b -     ',
  msg: Error: Can not find Squirrel
      at AutoUpdater.checkForUpdates (node:electron/js2c/browser_init:2:9080)
      at initUpdater (C:\Users\dajim\AppData\Local\Programs\internxt-drive\resources\app.asar\dist\main\webpack:\internxt-drive\node_modules\update-electron-app\dist\index.js:125:28)
      at EventEmitter.<anonymous> (C:\Users\dajim\AppData\Local\Programs\internxt-drive\resources\app.asar\dist\main\webpack:\internxt-drive\node_modules\update-electron-app\dist\index.js:53:42)
      at EventEmitter.emit (node:events:520:35)
}

Btw, squirrel is deprecated in electron-builder.
The manual implementation is still simple, we can make it very customizable (for example, to force the install, if the user doesn't want to install manually when the update check, the install is the first thing the app does when opening again) and we can test with it in development.

TamaraFinogina · 2026-04-27T08:54:54Z

-      !(startsWith(github.head_ref, 'merge-') && endsWith(github.head_ref, '-release') && github.base_ref == 'main')
+    # if: |
+    #   !(startsWith(github.head_ref, 'merge-') && endsWith(github.head_ref, '-release') && github.base_ref == 'main')
+    if: false


Is the PR size checker turned off for all?

Yes, because there are many tests and the lines were 550, to not split a test into another PR. It will be reverted in the next PR.

TamaraFinogina · 2026-04-27T09:09:00Z


-  afterEach(() => {
-    vi.useRealTimers();
-  });


Hmm, I think clearMocks: true calls vi.clearAllMocks() before each test. But vi.clearAllMocks() is not calling vi.useRealTimers, so timers won't be reset cause they have a separate API

TamaraFinogina · 2026-04-27T09:14:44Z

-    vi.useRealTimers();
-  });
-
  it('should not schedule backups if there is no last backup', () => {


same, I don't think timers are affected by clearMocks

TamaraFinogina · 2026-04-27T09:28:32Z

+
+  try {
+    logger.debug({ msg: 'Release already downloaded' });
+    await verifyHash({ filePath, latest });


No version downgrade prevention?

It's checked here: https://github.com/internxt/drive-desktop/pull/1361/changes#diff-7413da9f63c5e485ce84aadfe7ab7bd545dc82013be911cd74f936a01b34025aR43

TamaraFinogina · 2026-04-27T09:29:31Z

+    calls(verifyHashMock).toHaveLength(0);
+  });
+
+  it('should install release if file exists and hash is valid', async () => {


Can we add a test for trying to install an inferior version to what the user already has? Say the user has 11.0.0 and we try to install 10.0.0

It cannot reach this code, as explained in the above comment.

TamaraFinogina · 2026-04-27T09:32:26Z

+  try {
+    const res = await fetch('https://api.github.com/repos/internxt/drive-desktop/releases/latest');
+    const data = await res.json();
+    const { tag_name } = ReleaseSchema.parse(data);


Maybe use the name, so we don't have to remove v? Cause I think name and tag_name are the same except for v

TamaraFinogina · 2026-04-27T09:44:43Z

+
+    if (!isNewer(INTERNXT_VERSION, latest)) {
+      logger.debug({ msg: 'App is up to date', latest });
+      setTimeout(checkForUpdates, 60 * 60 * 1000);


existsSync (called inside checkForUpdates) will pose the app. Are we sure we want to do the periodic checks?

Link: https://www.geeksforgeeks.org/node-js/node-js-fs-existssync-method/

In other words, I don't know how the file upload/download in progress will react to this interruption. @dajimenezriv-internxt will it be ok?

Technically it's ok, also because it's something that we just do every hour, however I've been investigating more and probably we should access instead. I thought that it wasn't an async version because the existsSync was so fast than the overhead of making it async was not worth. It happens that with better_sqlite3 for example. I'm going to change it here and I will do the same change in the loop that iterates the whole file explorer in the next PR since there it will be more useful.

TamaraFinogina · 2026-04-27T09:57:44Z

+import { installRelease } from './show-dialog';
+import { verifyHash } from './verify-hash';
+
+export async function checkExistingFile({ filePath, latest }: { filePath: string; latest: string }) {


technically, it's check and install

TamaraFinogina · 2026-04-27T10:00:48Z

+    icon: nativeImage.createFromPath(iconPath),
+    title: 'Update Available',
+    message: `Version ${latest} is available`,
+    detail: 'Download and install the update now?',


At this point, it's already downloaded

TamaraFinogina · 2026-04-27T10:19:56Z

+
+  logger.debug({ msg: 'Verifying release hash', actual });
+
+  const url = `https://github.com/internxt/drive-desktop/releases/download/v${latest}/latest.yml`;


I think if we use api.github.com, we can get the latest without giving the release number

https://api.github.com/repos/internxt/drive-desktop/releases/latest

True, it also applies to the .exe, I'm changing it.

TamaraFinogina · 2026-04-27T10:41:18Z

+
+  logger.debug({ msg: 'Verifying release hash', actual });
+
+  const url = `https://github.com/internxt/drive-desktop/releases/download/v${latest}/latest.yml`;


https://github.com/internxt/drive-desktop/releases/latest/download/latest.yml

Wait, I think this one will work without a version number.

TamaraFinogina · 2026-04-27T10:46:56Z

  });

-  afterEach(() => {
-    vi.useRealTimers();


same about timers, don't think clearMocks affects vi.useFakeTimers();

TamaraFinogina · 2026-04-27T10:47:26Z

  });

-  afterAll(() => {
-    vi.useRealTimers();


TamaraFinogina · 2026-04-27T10:47:36Z

  });

-  afterEach(() => {
-    vi.clearAllTimers();


TamaraFinogina · 2026-04-27T10:49:48Z

        'sonarjs/no-alphabetical-sort': 'off',
        'sonarjs/no-empty-test-file': 'off',
        'sonarjs/os-command': 'off',
+        'sonarjs/publicly-writable-directories': 'off',


@dajimenezriv-internxt It's not fixing the Sonar warnings; it just disables them. Sonar keeps complaining but silently :)

It's because we are using the test path /tmp inside the tests, however the rule is only disabled inside the tests. The warnings inside the production code are not disabled.

https://github.com/internxt/drive-desktop/pull/1361/changes/BASE..e8464948dd462cae74b1bf650b8da0dd25dcbb08#diff-e2954b558f2aa82baff0e30964490d12942e0e251c1aa56c3294de6ec67b7cf5L21

TamaraFinogina · 2026-04-27T13:09:30Z

+
+    const time = await measurePerfomance(async () => {
+      const url = `https://github.com/internxt/drive-desktop/releases/download/v${latest}/${fileName}`;
+      const res = await fetch(url);


@dajimenezriv-internxt Can you try this?

const res = await fetch(`https://api.github.com/repos/${owner}/${repo}/releases/latest`, { headers: { 'Accept': 'application/vnd.github+json' } }); if (!res.ok) throw new Error(`HTTP ${res.status}`); const release = await res.json(); if (release.draft || release.prerelease) throw new Error('Latest release is not a stable release'); const asset = release.assets.find((a: { name: string }) => /^Internxt-Setup-[\d.]+\.exe$/.test(a.name) ); if (!asset) throw new Error(`No setup .exe found in release ${release.tag_name}`); const response = await fetch(asset.browser_download_url);

What's the benefit? Because the filename is constructed from the latest.yml? The same as the browser_download_url. So technically if an attacker achieves to inject a different filename he should be able to inject the asset.browser_download_url` too, right?

What we can do is to name all .exe as Internxt-Setup.exe and we have a fixed name and it doesn't change per release. However, I think that most open source projects include the version in the name as a standard.

TamaraFinogina · 2026-04-27T14:29:29Z

        'sonarjs/no-alphabetical-sort': 'off',
        'sonarjs/no-empty-test-file': 'off',
        'sonarjs/os-command': 'off',
+        'sonarjs/publicly-writable-directories': 'off',


@dajimenezriv-internxt We should return it, it's a useful check.

We can always pass the latest as input and just do the path at the moment.

const filePath = join(tmpdir(), `Internxt-Setup-${latest}.exe`);

The eslint rule doesn't apply there, it's just for the test files (https://github.com/internxt/drive-desktop/pull/1361/changes#diff-e2954b558f2aa82baff0e30964490d12942e0e251c1aa56c3294de6ec67b7cf5L21) because I'm writing /tmp inside a test: https://github.com/internxt/drive-desktop/pull/1361/changes#diff-ab5bba20517d999790e6182db1ccbb8a9d39f9dad2951ddcb9b2df62087e7457R13.

Never mind, you are right, it's for tests only

TamaraFinogina · 2026-04-27T14:53:18Z

+
+    logger.debug({ msg: 'New release available', latest, filePath });
+
+    const installing = await checkAndInstall({ filePath });


Ideally, I would bring hash and installation on top, something like:

const alreadyDownloaded = await checkIfAlreadyDownloaded({ filePath }); if(!alreadyDownloaded) downloadRelease({fileName}); await verifyHash({ filePath }); const installNow = await showDialog({ latest }); if(installNow) installRelease({ filePath });

Now installation happens in two different points: with user confirmation (immediately after download) and without (when already downloaded). I guess it's on purpose.

Anyway, if we ever need to debug a client issue with updates, we will need to know in which path it happened, so the support will have to go back and forth asking for details (did the user press later or install, etc).

We already know the path it took because of the logs. I'm going to add a log for the dialog response.

With the code you have provided we are not forcing the installation in the startup, you always show the dialog. I want to force the installation when the user opens the app.

If the user selects 'later', in an hour there will be another check, and it will be installed, right?

Nop, the timeout only runs if there wasn't any update, so we check again every hour. If the user clicks on Later we don't check again. See that the setTimeout only runs when the version is up to date. It will be installed after the user closes the app and opens it again (when turning off and on the laptop for example).

const alreadyDownloaded = await checkIfAlreadyDownloaded({ filePath }); if (!alreadyDownloaded) downloadRelease({ fileName }); await verifyHash({ filePath }); const installNow = alreadyDownloaded ? true: await showDialog({ latest }); if (installNow) installRelease({ filePath });

Btw, this code will do what you need, but it's ok

It won't, because downloadRelease needs to be awaited and that will block the main thread, that's why we need to write twice the verifyHash and so. We want to check if the release is downloaded and install before doing anything. Otherwise, we want to download and so in background.

ok, then it's fine

sonarqubecloud · 2026-04-27T15:08:38Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dajimenezriv-internxt · 2026-04-27T16:35:33Z

If you can do a final review with the changes @sg-gs

Simplify electron autoupdater

fb20ea9

dajimenezriv-internxt self-assigned this Apr 23, 2026

github-advanced-security AI found potential problems Apr 23, 2026

View reviewed changes

Comment thread src/apps/main/electron/autoupdater/show-dialog.ts Fixed

Comment thread src/apps/main/electron/autoupdater/verify-hash.ts Fixed

dajimenezriv-internxt added 9 commits April 24, 2026 10:07

Commit

00d2e0a

Commit

480af77

Commit

40392dd

Commit

25e43e4

Update download-release.ts

7e129c2

Commit

818afd7

Commit

23b0e49

Commit

bffe603

Commit

39dd64e

dajimenezriv-internxt commented Apr 24, 2026

View reviewed changes

dajimenezriv-internxt requested a review from sg-gs April 24, 2026 14:19

Commit

6b6f372

dajimenezriv-internxt commented Apr 24, 2026

View reviewed changes

sg-gs requested a review from TamaraFinogina April 27, 2026 07:37

TamaraFinogina reviewed Apr 27, 2026

View reviewed changes

dajimenezriv-internxt added 3 commits April 27, 2026 11:01

Commit

0730c92

Update verify-hash.ts

9319504

Update verify-hash.test.ts

e846494

TamaraFinogina reviewed Apr 27, 2026

View reviewed changes

dajimenezriv-internxt added 2 commits April 27, 2026 13:05

Commit

5c96446

Update check-for-updates.test.ts

efc351e

TamaraFinogina reviewed Apr 27, 2026

View reviewed changes

dajimenezriv-internxt added 4 commits April 27, 2026 15:11

Commit

300dda3

Update check-and-install.test.ts

7158c99

Update check-for-updates.test.ts

28a145b

Update access.test.ts

3c8e632

dajimenezriv-internxt requested a review from TamaraFinogina April 27, 2026 14:24

TamaraFinogina reviewed Apr 27, 2026

View reviewed changes

Update show-dialog.ts

a9133a9

dajimenezriv-internxt requested a review from TamaraFinogina April 27, 2026 15:41

TamaraFinogina approved these changes Apr 27, 2026

View reviewed changes


		logger.debug({ msg: 'Verifying release hash', actual });

		const url = `https://github.com/internxt/drive-desktop/releases/download/v${latest}/latest.yml`;


		logger.debug({ msg: 'New release available', latest, filePath });

		const installing = await checkAndInstall({ filePath });

Conversation

dajimenezriv-internxt commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sg-gs commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sg-gs commented Apr 27, 2026

Uh oh!

dajimenezriv-internxt commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dajimenezriv-internxt commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TamaraFinogina Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dajimenezriv-internxt commented Apr 23, 2026 •

edited

Loading

sg-gs commented Apr 27, 2026 •

edited

Loading

dajimenezriv-internxt commented Apr 27, 2026 •

edited

Loading

dajimenezriv-internxt commented Apr 27, 2026 •

edited

Loading

TamaraFinogina Apr 27, 2026 •

edited

Loading

dajimenezriv-internxt Apr 27, 2026 •

edited

Loading

TamaraFinogina Apr 27, 2026 •

edited

Loading

TamaraFinogina Apr 27, 2026 •

edited

Loading