fix(updater): Pass around a file descriptor to/and make things safer

backend/lib/entities/core/updater/ValetudoUpdaterApplyPendingState.js

@@ -9,6 +9,7 @@ class ValetudoUpdaterApplyPendingState extends ValetudoUpdaterState {
      * @param {string} options.version The version (e.g. 2021.10.0)
      * @param {Date}   options.releaseTimestamp The release date as found in the manifest
      * @param {string} options.downloadPath The path the new binary was downloaded to
+     * @param {number} options.downloadPathFd file descriptor
      *
      * @class
      */
@@ -18,6 +19,7 @@ class ValetudoUpdaterApplyPendingState extends ValetudoUpdaterState {
         this.version = options.version;
         this.releaseTimestamp = options.releaseTimestamp;
         this.downloadPath = options.downloadPath;
+        this.downloadPathFd = options.downloadPathFd;
     }
 }
 

backend/lib/updater/Updater.js

@@ -110,7 +110,6 @@ class Updater {
         if (!(this.state instanceof States.ValetudoUpdaterApprovalPendingState)) {
             throw new Error("Downloads can only be started when there's pending approval");
         }
-        const downloadPath = this.state.downloadPath;
         this.state.busy = true;
 
         const step = new Steps.ValetudoUpdaterDownloadStep({
@@ -138,8 +137,22 @@ class Updater {
         step.execute().then((state) => {
             this.state = state;
 
+            const downloadPath = state.downloadPath;
+            const downloadPathFd = state.downloadPathFd;
+
             this.cleanupHandler = () => {
-                fs.unlinkSync(downloadPath);
+                try {
+                    fs.closeSync(downloadPathFd);
+                } catch (e) {
+                    /* intentional */
+                }
+
+                try {
+                    fs.unlinkSync(downloadPath);
+                } catch (e) {
+                    /* intentional */
+                }
+
 
                 this.state = new States.ValetudoUpdaterIdleState({
                     currentVersion: this.updateProvider.getCurrentVersion()
@@ -172,7 +185,8 @@ class Updater {
         this.state.busy = true;
 
         const step = new Steps.ValetudoUpdaterApplyStep({
-            downloadPath: this.state.downloadPath
+            downloadPath: this.state.downloadPath,
+            downloadPathFd: this.state.downloadPathFd
         });
 
         step.execute().catch(err => { //no .then() required as the system will reboot

backend/lib/updater/lib/steps/ValetudoUpdaterApplyStep.js

@@ -1,27 +1,64 @@
 const fs = require("fs");
+const Logger = require("../../../Logger");
+const path = require("path");
 const ValetudoUpdaterStep = require("./ValetudoUpdaterStep");
+const {pipeline} = require("stream/promises");
 const {spawnSync} = require("child_process");
 
 class ValetudoUpdaterApplyStep extends ValetudoUpdaterStep {
     /**
      * @param {object} options
      * @param {string} options.downloadPath
+     * @param {number} options.downloadPathFd
      */
     constructor(options) {
         super();
 
         this.downloadPath = options.downloadPath;
+        this.downloadPathFd = options.downloadPathFd;
     }
 
     // @ts-ignore - Because spawnSync("reboot") ends our process, we don't have to return anything
     async execute() {
-        fs.unlinkSync(process.argv0);
-        fs.copyFileSync(this.downloadPath, process.argv0);
-        fs.chmodSync(process.argv0, fs.constants.S_IXUSR | fs.constants.S_IXGRP | fs.constants.S_IXOTH);
-        fs.unlinkSync(this.downloadPath);
+        const destinationDir = path.dirname(process.argv0);
+        const destinationFilename = path.basename(process.argv0);
+        const tmpDestination = path.join(destinationDir, `${destinationFilename}.upd`);
 
-        spawnSync("sync");
-        spawnSync("reboot");
+        try {
+            const downloadedUpdateReadStream = fs.createReadStream(null, {
+                fd: this.downloadPathFd,
+                start: 0,
+                autoClose: false
+            });
+            const tmpWriteStream = fs.createWriteStream(tmpDestination, {
+                flush: true,
+                mode: fs.constants.S_IXUSR | fs.constants.S_IXGRP | fs.constants.S_IXOTH
+            });
+
+            await pipeline(downloadedUpdateReadStream, tmpWriteStream);
+
+            fs.closeSync(this.downloadPathFd);
+
+            try {
+                fs.unlinkSync(this.downloadPath);
+            } catch (e) {
+                // intentionally ignored as this is what happens when the file got deleted mid-update
+                // we still have the fd and the downloadLocation is usually a ramdisk anyway, so we do not really care
+            }
+
+            fs.renameSync(tmpDestination, process.argv0);
+
+            spawnSync("sync");
+            spawnSync("reboot");
+        } catch (err) {
+            try {
+                fs.unlinkSync(tmpDestination);
+            } catch (e) {
+                Logger.warn(`Error while deleting tmp file ${tmpDestination}`, e);
+            }
+
+            throw err;
+        }
     }
 }
 

backend/lib/updater/lib/steps/ValetudoUpdaterDownloadStep.js

@@ -31,8 +31,12 @@ class ValetudoUpdaterDownloadStep extends ValetudoUpdaterStep {
         this.onProgressUpdate = () => {};
     }
 
-
+    /**
+     * @returns {Promise<import("../../../entities/core/updater").ValetudoUpdaterApplyPendingState>}
+     */
     async execute() {
+        let fd;
+
         try {
             const downloadResponse = await get(this.downloadUrl, {responseType: "stream"});
             const expectedDownloadSize = parseInt(downloadResponse.headers?.["content-length"]);
@@ -50,14 +54,30 @@ class ValetudoUpdaterDownloadStep extends ValetudoUpdaterStep {
                 }
             });
 
+            fd = fs.openSync(this.downloadPath, "w+");
+
             await pipeline(
                 downloadResponse.data,
                 progressTracker,
-                fs.createWriteStream(this.downloadPath)
+                fs.createWriteStream(null, {fd: fd, flush: true, autoClose: false})
             );
         } catch (e) {
             Logger.error("Error while downloading release binary", e);
 
+            if (fd !== undefined) {
+                try {
+                    fs.closeSync(fd);
+                } catch (e) {
+                    /* intentional */
+                }
+
+                try {
+                    fs.unlinkSync(this.downloadPath);
+                } catch (e) {
+                    /* intentional */
+                }
+            }
+
             throw new ValetudoUpdaterError(
                 ValetudoUpdaterError.ERROR_TYPE.DOWNLOAD_FAILED,
                 "Error while downloading release binary"
@@ -69,7 +89,7 @@ class ValetudoUpdaterDownloadStep extends ValetudoUpdaterStep {
         try {
             checksum = await new Promise((resolve, reject) => {
                 const hash = crypto.createHash("sha256");
-                const readStream = fs.createReadStream(this.downloadPath);
+                const readStream = fs.createReadStream(null, {fd: fd, start: 0, autoClose: false});
 
                 readStream.on("error", err => {
                     reject(err);
@@ -86,6 +106,20 @@ class ValetudoUpdaterDownloadStep extends ValetudoUpdaterStep {
         } catch (e) {
             Logger.error("Error while calculating downloaded binary checksum", e);
 
+            if (fd !== undefined) {
+                try {
+                    fs.closeSync(fd);
+                } catch (e) {
+                    /* intentional */
+                }
+
+                try {
+                    fs.unlinkSync(this.downloadPath);
+                } catch (e) {
+                    /* intentional */
+                }
+            }
+
             throw new ValetudoUpdaterError(
                 ValetudoUpdaterError.ERROR_TYPE.UNKNOWN,
                 "Error while calculating downloaded binary checksum"
@@ -93,6 +127,20 @@ class ValetudoUpdaterDownloadStep extends ValetudoUpdaterStep {
         }
 
         if (checksum !== this.expectedHash) {
+            if (fd !== undefined) {
+                try {
+                    fs.closeSync(fd);
+                } catch (e) {
+                    /* intentional */
+                }
+
+                try {
+                    fs.unlinkSync(this.downloadPath);
+                } catch (e) {
+                    /* intentional */
+                }
+            }
+
             throw new ValetudoUpdaterError(
                 ValetudoUpdaterError.ERROR_TYPE.INVALID_CHECKSUM,
                 `Expected Checksum: ${this.expectedHash}. Actual: ${checksum}`
@@ -101,7 +149,8 @@ class ValetudoUpdaterDownloadStep extends ValetudoUpdaterStep {
             return new States.ValetudoUpdaterApplyPendingState({
                 version: this.version,
                 releaseTimestamp: this.releaseTimestamp,
-                downloadPath: this.downloadPath
+                downloadPath: this.downloadPath,
+                downloadPathFd: fd
             });
         }
     }