diff --git a/src/config/settings.cpp b/src/config/settings.cpp index 98505da..b1f92ed 100644 --- a/src/config/settings.cpp +++ b/src/config/settings.cpp @@ -11,8 +11,10 @@ #include #include #include +#include #include "../util/logger.h" +#include "../util/platform.h" #ifdef _WIN32 #include @@ -291,6 +293,17 @@ bool Settings::load(const std::string& path) return true; } catch (const std::exception& e) { DEBUG_LOGF("Failed to parse settings: %s\n", e.what()); + // The file exists but is unparseable (truncated/corrupt). Quarantine it so the + // next save() doesn't silently overwrite it with defaults — the user's data stays + // recoverable. Proceed with in-memory defaults. + file.close(); + std::error_code ec; + const std::string quarantine = + path + ".corrupt-" + std::to_string(static_cast(std::time(nullptr))); + fs::rename(path, quarantine, ec); + if (!ec) { + DEBUG_LOGF("Quarantined corrupt settings to %s\n", quarantine.c_str()); + } return false; } } @@ -411,17 +424,11 @@ bool Settings::save(const std::string& path) } try { - // Ensure directory exists - fs::path p(path); - fs::create_directories(p.parent_path()); - - std::ofstream file(path); - if (!file.is_open()) { - return false; - } - - file << j.dump(4); - return true; + // Atomic + durable: write to a temp file, fsync, then rename over the real file. + // A crash mid-write can no longer truncate settings.json (which would silently + // reset every preference on the next launch). Owner-only (0600) — it carries the + // lite-server list and address metadata. + return util::Platform::writeFileAtomically(path, j.dump(4), /*restrictPermissions=*/true); } catch (const std::exception& e) { DEBUG_LOGF("Failed to save settings: %s\n", e.what()); return false; diff --git a/src/data/address_book.cpp b/src/data/address_book.cpp index 05ff428..70eb8ef 100644 --- a/src/data/address_book.cpp +++ b/src/data/address_book.cpp @@ -9,6 +9,7 @@ #include #include "../util/logger.h" +#include "../util/platform.h" #ifdef _WIN32 #include @@ -113,20 +114,16 @@ bool AddressBook::save() j["entries"].push_back(e); } - // Ensure directory exists - fs::path p(file_path_); - fs::create_directories(p.parent_path()); - - std::ofstream file(file_path_); - if (!file.is_open()) { - DEBUG_LOGF("Could not open address book for writing: %s\n", file_path_.c_str()); + // Atomic + durable: temp file + fsync + rename, so a crash mid-write can't + // truncate addressbook.json (which is fully rewritten on every entry change). + // Owner-only (0600) — it holds the user's saved contacts. + if (!util::Platform::writeFileAtomically(file_path_, j.dump(2), /*restrictPermissions=*/true)) { + DEBUG_LOGF("Could not write address book: %s\n", file_path_.c_str()); return false; } - - file << j.dump(2); DEBUG_LOGF("Address book saved: %zu entries\n", entries_.size()); return true; - + } catch (const std::exception& e) { DEBUG_LOGF("Error saving address book: %s\n", e.what()); return false; diff --git a/src/util/platform.cpp b/src/util/platform.cpp index 62bdfef..672682f 100644 --- a/src/util/platform.cpp +++ b/src/util/platform.cpp @@ -30,6 +30,7 @@ #include #else #include +#include #include #include #include @@ -239,6 +240,82 @@ bool Platform::deleteFile(const std::string& path) #endif } +bool Platform::writeFileAtomically(const std::string& path, + const std::string& content, + bool restrictPermissions) +{ + namespace fs = std::filesystem; + std::error_code ec; + fs::path target(path); + fs::path dir = target.parent_path(); + if (!dir.empty()) { + fs::create_directories(dir, ec); // ignore ec; the open below will fail if truly unwritable + } + const std::string tmp = path + ".tmp"; + +#ifdef _WIN32 + { + std::ofstream out(tmp, std::ios::binary | std::ios::trunc); + if (!out.is_open()) return false; + out.write(content.data(), static_cast(content.size())); + out.flush(); + if (!out) { out.close(); DeleteFileA(tmp.c_str()); return false; } + } + // Flush the temp file's contents to stable storage before the rename. + HANDLE h = CreateFileA(tmp.c_str(), GENERIC_WRITE, 0, nullptr, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + if (h != INVALID_HANDLE_VALUE) { + FlushFileBuffers(h); + CloseHandle(h); + } + (void)restrictPermissions; // owner-only ACLs rely on the per-user profile dir on Windows + // MoveFileEx with REPLACE_EXISTING is atomic on NTFS; WRITE_THROUGH makes it durable. + if (MoveFileExA(tmp.c_str(), path.c_str(), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH) == 0) { + DeleteFileA(tmp.c_str()); + return false; + } + return true; +#else + const mode_t mode = restrictPermissions ? 0600 : 0644; + int fd = ::open(tmp.c_str(), O_WRONLY | O_CREAT | O_TRUNC, mode); + if (fd < 0) return false; + if (restrictPermissions) { + ::fchmod(fd, 0600); // enforce even if the temp file pre-existed with looser perms + } + bool ok = true; + size_t written = 0; + const char* data = content.data(); + while (written < content.size()) { + ssize_t n = ::write(fd, data + written, content.size() - written); + if (n < 0) { + if (errno == EINTR) continue; + ok = false; + break; + } + written += static_cast(n); + } + if (ok && ::fsync(fd) != 0) ok = false; + ::close(fd); + if (!ok) { + ::remove(tmp.c_str()); + return false; + } + if (::rename(tmp.c_str(), path.c_str()) != 0) { + ::remove(tmp.c_str()); + return false; + } + // fsync the directory so the rename (the metadata that makes the new file + // visible) survives a crash, not just the file contents. + int dfd = ::open(dir.empty() ? "." : dir.c_str(), O_RDONLY); + if (dfd >= 0) { + ::fsync(dfd); + ::close(dfd); + } + return true; +#endif +} + std::string Platform::getExecutableDirectory() { #ifdef _WIN32 diff --git a/src/util/platform.h b/src/util/platform.h index f39dc23..ba3f6c2 100644 --- a/src/util/platform.h +++ b/src/util/platform.h @@ -74,6 +74,26 @@ public: * @return true if successful or file didn't exist */ static bool deleteFile(const std::string& path); + + /** + * @brief Write a file atomically and durably. + * + * Writes @p content to `.tmp`, flushes it to stable storage + * (fsync / FlushFileBuffers), then atomically renames it over @p path + * (POSIX rename / Win32 MoveFileEx with REPLACE_EXISTING). A crash or power + * loss at any point leaves either the old file intact or the fully-written + * new one — never a truncated/corrupt file. The parent directory is created + * if missing, and on POSIX it is fsync'd so the rename itself is durable. + * + * @param path Destination file path. + * @param content Bytes to write. + * @param restrictPermissions When true, create the file owner-only (0600 on + * POSIX) before the rename so it is never briefly world-readable. + * @return true on success; on failure any pre-existing file is left untouched. + */ + static bool writeFileAtomically(const std::string& path, + const std::string& content, + bool restrictPermissions = false); /** * @brief Get the directory containing the executable