Use safe file writing pattern (#421)

Write new file contents to a temporary file and then rename over the
target file to avoid data loss in the unlikely event there's an error
writing the new file contents or the process is interrupted.

Resolves #416.

packages/create/create.js

@@ -85,6 +85,12 @@ class LineSplitter extends stream.Transform {
 class InstallError extends Error { }
 
 
+async function safeOutputFile(file, data, options={}) {
+	let temporary = `${file}.tmp`;
+	await fs.outputFile(temporary, data, options);
+	await fs.rename(temporary, file);
+}
+
 async function execFile(cmd, args) {
 	logger.verbose(`executing ${cmd} ${args.join(" ")}`);
 	const asyncExec = util.promisify(child_process.execFile);
@@ -165,7 +171,7 @@ async function validateInstallDir() {
 
 async function installClusterio(mode, plugins) {
 	try {
-		await fs.writeFile("package.json", JSON.stringify({
+		await safeOutputFile("package.json", JSON.stringify({
 			name: "clusterio-install",
 			private: true,
 		}, null, 2), { flag: "wx" });
@@ -213,7 +219,7 @@ async function installClusterio(mode, plugins) {
 			}
 		}
 		try {
-			await fs.writeFile("plugin-list.json", JSON.stringify([...pluginList], null, 4));
+			await safeOutputFile("plugin-list.json", JSON.stringify([...pluginList], null, 4));
 		} catch (err) {
 			throw new InstallError(`Error writing plugin-list.json: ${err.message}`);
 		}
@@ -223,17 +229,17 @@ async function installClusterio(mode, plugins) {
 async function writeScripts(mode) {
 	if (["standalone", "master"].includes(mode)) {
 		if (process.platform === "win32") {
-			await fs.writeFile(
+			await safeOutputFile(
 				"run-master.cmd",
 				"@echo off\n.\\node_modules\\.bin\\clusteriomaster.cmd run\n"
 			);
 		} else {
-			await fs.writeFile(
+			await safeOutputFile(
 				"run-master.sh",
 				"#!/bin/sh\nexec ./node_modules/.bin/clusteriomaster run\n",
 				{ mode: 0o755 },
 			);
-			await fs.outputFile(
+			await safeOutputFile(
 				"systemd/clusteriomaster.service",
 				`[Unit]
 Description=Clusterio Master
@@ -254,17 +260,17 @@ WantedBy=multi-user.target
 
 	if (["standalone", "slave"].includes(mode)) {
 		if (process.platform === "win32") {
-			await fs.writeFile(
+			await safeOutputFile(
 				"run-slave.cmd",
 				"@echo off\n.\\node_modules\\.bin\\clusterioslave.cmd run\n"
 			);
 		} else {
-			await fs.writeFile(
+			await safeOutputFile(
 				"run-slave.sh",
 				"#!/bin/sh\nexec ./node_modules/.bin/clusterioslave run\n",
 				{ mode: 0o755 },
 			);
-			await fs.outputFile(
+			await safeOutputFile(
 				"systemd/clusterioslave.service",
 				`[Unit]
 Description=Clusterio Slave

packages/lib/database.js

@@ -8,6 +8,7 @@
 "use strict";
 const fs = require("fs-extra");
 
+const libFileOps = require("./file_ops");
 const { basicType } = require("./helpers");
 
 
@@ -78,7 +79,7 @@ async function loadJsonAsMap(filePath) {
  */
 async function saveMapAsJson(filePath, map) {
 	let obj = mapToObject(map);
-	await fs.outputFile(filePath, JSON.stringify(obj, null, 4));
+	await libFileOps.safeOutputFile(filePath, JSON.stringify(obj, null, 4));
 }
 
 /**
@@ -138,7 +139,7 @@ async function loadJsonArrayAsMap(filePath) {
  * @throws {Error} if an error occured writing to the file.
  */
 async function saveMapAsJsonArray(filePath, map) {
-	await fs.outputFile(filePath, JSON.stringify([...map.values()], null, 4));
+	await libFileOps.safeOutputFile(filePath, JSON.stringify([...map.values()], null, 4));
 }
 
 

packages/lib/factorio/patch.js

@@ -284,9 +284,11 @@ async function patch(savePath, modules) {
 	root.file("clusterio.json", JSON.stringify(patchInfo, null, 4));
 
 	// Write back the save
+	let tempSavePath = `${savePath}.tmp`;
 	let stream = zip.generateNodeStream({ compression: "DEFLATE" });
-	let pipe = stream.pipe(fs.createWriteStream(savePath));
+	let pipe = stream.pipe(fs.createWriteStream(tempSavePath));
 	await events.once(pipe, "finish");
+	await fs.rename(tempSavePath, savePath);
 }
 
 module.exports = {

packages/lib/factorio/server.js

@@ -6,10 +6,11 @@ const path = require("path");
 const events = require("events");
 const util = require("util");
 const crypto = require("crypto");
-
-const libIni = require("../ini");
 const rconClient = require("rcon-client");
+
 const libErrors = require("../errors");
+const libFileOps = require("../file_ops");
+const libIni = require("../ini");
 const { logger } = require("../logging");
 const { escapeRegExp } = require("../helpers");
 
@@ -1173,17 +1174,17 @@ class FactorioServer extends events.EventEmitter {
 				"write-data": this.writePath(),
 			},
 		});
-		await fs.outputFile(this.writePath("config.ini"), content);
+		await libFileOps.safeOutputFile(this.writePath("config.ini"), content);
 	}
 
 	async _writeMapSettings(mapGenSettings, mapSettings) {
 		if (mapGenSettings) {
-			await fs.outputFile(
+			await libFileOps.safeOutputFile(
 				this.writePath("map-gen-settings.json"), JSON.stringify(mapGenSettings, null, 4)
 			);
 		}
 		if (mapSettings) {
-			await fs.outputFile(
+			await libFileOps.safeOutputFile(
 				this.writePath("map-settings.json"), JSON.stringify(mapSettings, null, 4)
 			);
 		}

packages/lib/file_ops.js

@@ -69,7 +69,29 @@ async function findUnusedName(directory, name, extension = "") {
 	}
 }
 
+/**
+ * Safely write data to a file
+ *
+ * Same as fs-extra.outputFile except the data is written to a temporary
+ * file that's renamed over the target file.  The name of the temporary file
+ * is the same as the target file with the suffix `.tmp` added.
+ *
+ * If the operation fails it may leave behind the temporary file.  This
+ * should not be too much of an issue as the next time the same file is
+ * written the temporary will be overwritten and renamed to the target file.
+ *
+ * @param {string} file - Path to file to write.
+ * @param {string|Buffer} data - Content to write.
+ * @param {object|string} options - see fs.writeFile, `flag` must not be set.
+ */
+async function safeOutputFile(file, data, options={}) {
+	let temporary = `${file}.tmp`;
+	await fs.outputFile(temporary, data, options);
+	await fs.rename(temporary, file);
+}
+
 module.exports = {
 	getNewestFile,
 	findUnusedName,
+	safeOutputFile,
 };

packages/lib/shared_commands.js

@@ -7,6 +7,7 @@ const fs = require("fs-extra");
 const path = require("path");
 
 const libConfig = require("./config");
+const libFileOps = require("./file_ops");
 const { logger } = require("./logging");
 const libHelpers = require("./helpers");
 
@@ -70,7 +71,7 @@ async function handlePluginCommand(args, pluginList, pluginListPath) {
 		}
 
 		pluginList.set(pluginInfo.name, pluginPath);
-		await fs.outputFile(pluginListPath, JSON.stringify([...pluginList], null, 4));
+		await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, 4));
 		print(`Added ${pluginInfo.name}`);
 
 	} else if (command === "remove") {
@@ -80,7 +81,7 @@ async function handlePluginCommand(args, pluginList, pluginListPath) {
 			return;
 		}
 
-		await fs.outputFile(pluginListPath, JSON.stringify([...pluginList], null, 4));
+		await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, 4));
 		print(`Removed ${args.name}`);
 
 	} else if (command === "list") {
@@ -155,7 +156,7 @@ async function handleConfigCommand(args, instance, configPath) {
 
 		try {
 			instance.set(args.field, args.value);
-			await fs.outputFile(configPath, JSON.stringify(instance.serialize(), null, 4));
+			await libFileOps.safeOutputFile(configPath, JSON.stringify(instance.serialize(), null, 4));
 		} catch (err) {
 			if (err instanceof libConfig.InvalidField || err instanceof libConfig.InvalidValue) {
 				logger.error(err.message);

packages/master/master.js

@@ -26,6 +26,7 @@ const jwt = require("jsonwebtoken");
 
 // homebrew modules
 const libErrors = require("@clusterio/lib/errors");
+const libFileOps = require("@clusterio/lib/file_ops");
 const libPluginLoader = require("@clusterio/lib/plugin_loader");
 const libPrometheus = require("@clusterio/lib/prometheus");
 const libConfig = require("@clusterio/lib/config");
@@ -162,7 +163,7 @@ async function handleBootstrapCommand(args, masterConfig) {
 			console.log(content);
 		} else {
 			logger.info(`Writing ${args.output}`);
-			await fs.outputFile(args.output, content);
+			await libFileOps.safeOutputFile(args.output, content);
 		}
 	}
 }
@@ -294,7 +295,9 @@ async function initialize() {
 		let asyncRandomBytes = util.promisify(crypto.randomBytes);
 		let bytes = await asyncRandomBytes(256);
 		parameters.masterConfig.set("master.auth_secret", bytes.toString("base64"));
-		await fs.outputFile(parameters.masterConfigPath, JSON.stringify(parameters.masterConfig.serialize(), null, 4));
+		await libFileOps.safeOutputFile(
+			parameters.masterConfigPath, JSON.stringify(parameters.masterConfig.serialize(), null, 4)
+		);
 	}
 
 	if (command === "config") {

packages/master/src/Master.js

@@ -10,6 +10,7 @@ const path = require("path");
 
 const libConfig = require("@clusterio/lib/config");
 const libErrors = require("@clusterio/lib/errors");
+const libFileOps = require("@clusterio/lib/file_ops");
 const libLink = require("@clusterio/lib/link");
 const libPlugin = require("@clusterio/lib/plugin");
 const libPluginLoader = require("@clusterio/lib/plugin_loader");
@@ -241,7 +242,7 @@ class Master {
 		logger.info("Stopping master");
 
 		logger.info("Saving config");
-		await fs.outputFile(this.configPath, JSON.stringify(this.config.serialize(), null, 4));
+		await libFileOps.safeOutputFile(this.configPath, JSON.stringify(this.config.serialize(), null, 4));
 
 		if (this.devMiddleware) {
 			await new Promise((resolve, reject) => { this.devMiddleware.close(resolve); });
@@ -297,7 +298,7 @@ class Master {
 	}
 
 	static async saveSlaves(filePath, slaves) {
-		await fs.outputFile(filePath, JSON.stringify([...slaves.entries()], null, 4));
+		await libFileOps.safeOutputFile(filePath, JSON.stringify([...slaves.entries()], null, 4));
 	}
 
 	static async loadInstances(filePath) {
@@ -329,7 +330,7 @@ class Master {
 			serialized.push(instance.config.serialize());
 		}
 
-		await fs.outputFile(filePath, JSON.stringify(serialized, null, 4));
+		await libFileOps.safeOutputFile(filePath, JSON.stringify(serialized, null, 4));
 	}
 
 	static addAppRoutes(app, pluginInfos) {

packages/master/src/UserManager.js

@@ -1,6 +1,7 @@
 "use strict";
 const fs = require("fs-extra");
 
+const libFileOps = require("@clusterio/lib/file_ops");
 const libUsers = require("@clusterio/lib/users");
 
 /**
@@ -62,7 +63,7 @@ class UserManager {
 			users: serializedUsers,
 			roles: serializedRoles,
 		};
-		await fs.outputFile(filePath, JSON.stringify(serialized, null, 4));
+		await libFileOps.safeOutputFile(filePath, JSON.stringify(serialized, null, 4));
 	}
 
 	/**

packages/master/src/routes.js

@@ -2,13 +2,13 @@
 const Busboy = require("busboy");
 const crypto = require("crypto");
 const events = require("events");
-const fs = require("fs-extra");
 const JSZip = require("jszip");
 const jwt = require("jsonwebtoken");
 const path = require("path");
 const util = require("util");
 
 const libErrors = require("@clusterio/lib/errors");
+const libFileOps = require("@clusterio/lib/file_ops");
 const libHelpers = require("@clusterio/lib/helpers");
 const libLink = require("@clusterio/lib/link");
 const libPlugin = require("@clusterio/lib/plugin");
@@ -187,7 +187,7 @@ async function uploadExport(req, res) {
 		}
 
 		let name = path.posix.basename(filePath);
-		await fs.outputFile(path.join("static", "export", name), await file.async("nodebuffer"));
+		await libFileOps.safeOutputFile(path.join("static", "export", name), await file.async("nodebuffer"));
 	}
 
 	res.sendStatus(200);

packages/slave/slave.js

@@ -376,7 +376,7 @@ class Instance extends libLink.Link {
 	 */
 	async writeServerSettings() {
 		let serverSettings = await this.resolveServerSettings(this.config.get("factorio.settings"));
-		await fs.writeFile(
+		await libFileOps.safeOutputFile(
 			this.server.writePath("server-settings.json"),
 			JSON.stringify(serverSettings, null, 4)
 		);
@@ -410,7 +410,7 @@ class Instance extends libLink.Link {
 
 		if (this.config.get("factorio.sync_adminlist")) {
 			this.logger.verbose("Writing server-adminlist.json");
-			fs.outputFile(
+			libFileOps.safeOutputFile(
 				this.server.writePath("server-adminlist.json"),
 				JSON.stringify([...this._slave.adminlist], null, 4)
 			);
@@ -418,7 +418,7 @@ class Instance extends libLink.Link {
 
 		if (this.config.get("factorio.sync_banlist")) {
 			this.logger.verbose("Writing server-banlist.json");
-			fs.outputFile(
+			libFileOps.safeOutputFile(
 				this.server.writePath("server-banlist.json"),
 				JSON.stringify([...this._slave.banlist].map(
 					([username, reason]) => ({ username, reason })
@@ -428,7 +428,7 @@ class Instance extends libLink.Link {
 
 		if (this.config.get("factorio.sync_whitelist")) {
 			this.logger.verbose("Writing server-whitelist.json");
-			fs.outputFile(
+			libFileOps.safeOutputFile(
 				this.server.writePath("server-whitelist.json"),
 				JSON.stringify([...this._slave.whitelist], null, 4)
 			);
@@ -1299,7 +1299,7 @@ class Slave extends libLink.Link {
 			_warning: "Changes to this file will be overwritten by the master server's copy.",
 			...instanceInfo.config.serialize(),
 		};
-		await fs.outputFile(
+		await libFileOps.safeOutputFile(
 			path.join(instanceInfo.path, "instance.json"),
 			JSON.stringify(warnedOutput, null, 4)
 		);

plugins/inventory_sync/master.js

@@ -2,6 +2,7 @@
 const fs = require("fs-extra");
 const path = require("path");
 
+const libFileOps = require("@clusterio/lib/file_ops");
 const libPlugin = require("@clusterio/lib/plugin");
 const libErrors = require("@clusterio/lib/errors");
 
@@ -26,7 +27,7 @@ async function saveDatabase(masterConfig, playerDatastore, logger) {
 		let file = path.resolve(masterConfig.get("master.database_directory"), "inventories.json");
 		logger.verbose(`writing ${file}`);
 		let content = JSON.stringify(Array.from(playerDatastore));
-		await fs.outputFile(file, content);
+		await libFileOps.safeOutputFile(file, content);
 	}
 }
 

plugins/research_sync/master.js

@@ -2,6 +2,7 @@
 const fs = require("fs-extra");
 const path = require("path");
 
+const libFileOps = require("@clusterio/lib/file_ops");
 const libPlugin = require("@clusterio/lib/plugin");
 const RateLimiter = require("@clusterio/lib/RateLimiter");
 
@@ -25,7 +26,7 @@ async function loadTechnologies(masterConfig, logger) {
 async function saveTechnologies(masterConfig, technologies, logger) {
 	let filePath = path.join(masterConfig.get("master.database_directory"), "technologies.json");
 	logger.verbose(`writing ${filePath}`);
-	await fs.outputFile(filePath, JSON.stringify([...technologies.entries()], null, 4));
+	await libFileOps.safeOutputFile(filePath, JSON.stringify([...technologies.entries()], null, 4));
 }
 
 class MasterPlugin extends libPlugin.BaseMasterPlugin {

plugins/subspace_storage/master.js

@@ -3,6 +3,7 @@ const fs = require("fs-extra");
 const path = require("path");
 
 const libDatabase = require("@clusterio/lib/database");
+const libFileOps = require("@clusterio/lib/file_ops");
 const libPlugin = require("@clusterio/lib/plugin");
 const { Counter, Gauge } = require("@clusterio/lib/prometheus");
 const RateLimiter = require("@clusterio/lib/RateLimiter");
@@ -49,7 +50,7 @@ async function saveDatabase(masterConfig, items, logger) {
 		let file = path.resolve(masterConfig.get("master.database_directory"), "items.json");
 		logger.verbose(`writing ${file}`);
 		let content = JSON.stringify(items.serialize());
-		await fs.outputFile(file, content);
+		await libFileOps.safeOutputFile(file, content);
 	} else if (items) {
 		logger.error(`Item database too large, not saving (${items.size})`);
 	}

test/lib/file_ops.js

@@ -10,6 +10,8 @@ describe("lib/file_ops", function() {
 	async function setupTestingEnv() {
 		await fs.ensureDir(path.join(baseDir, "test", "folder"));
 		await fs.ensureDir(path.join(baseDir, "test", "another folder"));
+		await fs.remove(path.join(baseDir, "safe"));
+		await fs.ensureDir(path.join(baseDir, "safe"));
 
 		await fs.outputFile(path.join(baseDir, "test", "file.txt"), "contents");
 		await fs.outputFile(path.join(baseDir, "test", "another file.txt"), "more contents");
@@ -66,4 +68,20 @@ describe("lib/file_ops", function() {
 			}
 		});
 	});
+
+	describe("safeOutputFile()", function() {
+		it("should write new target file", async function() {
+			let target = path.join(baseDir, "safe", "simple.txt");
+			await libFileOps.safeOutputFile(target, "a text file", "utf8");
+			assert(!await fs.pathExists(`${target}.tmp`), "temporary was left behind");
+			assert.equal(await fs.readFile(target, "utf8"), "a text file");
+		});
+		it("should overwrite existing target file", async function() {
+			let target = path.join(baseDir, "safe", "exists.txt");
+			await fs.outputFile(target, "previous", "utf8");
+			await libFileOps.safeOutputFile(target, "current", "utf8");
+			assert(!await fs.pathExists(`${target}.tmp`), "temporary was left behind");
+			assert.equal(await fs.readFile(target, "utf8"), "current");
+		});
+	});
 });