Start config persistence groundwork with checked startup saves

.github/workflows/ci.yaml

@@ -218,6 +218,16 @@ jobs:
         shell: pwsh
         run: sccache --show-stats
 
+      - name: Test startup config saves
+        shell: pwsh
+        env:
+          PACKAGE_DIR: ${{ steps.xmake_cache_paths.outputs.package_dir }}
+        run: |
+          $header = Get-ChildItem -LiteralPath (Join-Path $env:PACKAGE_DIR 't/toml++') -Recurse -Filter toml.h |
+            Where-Object { $_.Directory.Name -eq 'toml++' } | Select-Object -First 1
+          if (-not $header) { throw 'Built toml++ package not found.' }
+          ./tests/run-config-save.ps1 -TomlInclude $header.Directory.Parent.FullName
+
       - name: Package
         shell: pwsh
         run: |
@@ -521,6 +531,16 @@ jobs:
         shell: bash
         run: sccache --show-stats
 
+      - name: Test startup config saves
+        shell: bash
+        env:
+          PACKAGE_DIR: ${{ steps.xmake_cache_paths.outputs.package_dir }}
+        run: |
+          set -euo pipefail
+          TOML_HEADER=$(find "$PACKAGE_DIR/t/toml++" -path '*/include/toml++/toml.h' -print -quit)
+          test -n "$TOML_HEADER"
+          bash tests/run-config-save.sh "$(dirname "$(dirname "$TOML_HEADER")")"
+
       - name: Report Swift module cache
         shell: bash
         run: |

docs/config-save.md

@@ -0,0 +1,44 @@
+# Startup config saves
+
+`Config::Save` writes complete TOML documents for two startup callers: the initial
+default config and the generated runtime snapshot. It keeps `File::MakePath`
+routing and the existing generated-file warning. Save errors are logged once by
+the caller; startup continues with the in-memory configuration.
+
+`SaveConfigDocument` serializes with toml++, parses the output before touching
+disk, exclusively creates a private sibling temporary file, checks writing and closing,
+and replaces the destination. No threads, frame callbacks, runtime controls or
+shutdown interception are installed. This is not the preserving TOML editor:
+whole-document saves do not merge concurrent setting changes or preserve comments.
+
+Staging is private before writing and remains private after close: Windows uses
+a protected owner/system DACL, and macOS uses mode `0600`. New documents keep
+these private permissions.
+
+Windows uses `ReplaceFileW` to preserve existing permissions and streams, with a
+temporary backup for its documented partial-failure cases. A missing destination
+falls back to a non-replacing move. The caller's startup existence check is not an
+exclusive create transaction. Ordinary failures clean up the temporary file; partial
+replacement failures retain recovery files and report their location. The backup
+name is the reported temporary path plus `.bak`. Recovery is not automatic.
+macOS uses rename after copying the existing permission bits. Extended metadata
+and hard-link identity are not preserved by that path. Existing and dangling final symlinks are
+resolved before staging; cyclic links fail without replacing the link. Replacement requires directory permissions in addition
+to any file access checks; it cannot exactly match an in-place overwrite.
+
+Successful close/replacement is not a guarantee against power loss. A forced exit
+can leave a temporary file. No automatic stale-file sweep is installed.
+
+Run the isolated Windows fixtures with `tests/run-config-save.ps1` after the
+normal AX build has installed toml++; `-TomlInclude` can select another include
+directory. Fixtures never access the installed game's files.
+
+On macOS, run `bash tests/run-config-save.sh TOML_INCLUDE_DIR`. Both native macOS
+CI jobs run these fixtures after the normal build, including permission-bit and
+symlink checks. The failure fixture injects short writes and failed closes at
+compile time and checks staging permissions before/during writes and after close;
+it does not install test controls in the mod.
+
+Native behavior references:
+- [Windows ReplaceFileW](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-replacefilew)
+- [POSIX rename](https://pubs.opengroup.org/onlinepubs/9799919799/functions/rename.html)

mods/src/config.cc

@@ -1,4 +1,5 @@
 #include "config.h"
+#include "config_save.h"
 #include "file.h"
 #include "patches/mapkey.h"
 #include "prime/KeyCode.h"
@@ -93,10 +94,10 @@ Config::Config()
 
 void Config::Save(const toml::table& config, const std::string_view filename, bool apply_warning)
 {
-  std::ofstream config_file;
+  std::ostringstream config_file;
 
   auto config_path = File::MakePath(filename, true);
-  config_file.open(config_path);
+  config_file.exceptions(std::ios::badbit | std::ios::failbit);
 
   if (apply_warning) {
     char defaultFile[255], configFile[255];
@@ -118,8 +119,7 @@ void Config::Save(const toml::table& config, const std::string_view filename, bo
     config_file << "#######################################################################\n\n";
   }
 
-  config_file << config;
-  config_file.close();
+  SaveConfigDocument(config, std::filesystem::path(config_path), config_file.str());
 }
 
 Config& Config::Get()
@@ -1403,7 +1403,11 @@ void Config::Load()
     message << "Creating " << File::Config() << " (default config file)";
     spdlog::warn(message.str());
 
-    Config::Save(parsed, File::Config(), false);
+    try {
+      Config::Save(parsed, File::Config(), false);
+    } catch (const std::exception& error) {
+      spdlog::error("Could not save default config: {}", error.what());
+    }
   }
 
   message.str("");
@@ -1418,7 +1422,11 @@ void Config::Load()
     std::filesystem::remove(FILE_DEF_PARSED);
   }
 
-  Config::Save(parsed, File::Vars());
+  try {
+    Config::Save(parsed, File::Vars());
+  } catch (const std::exception& error) {
+    spdlog::error("Could not save runtime config: {}", error.what());
+  }
 
   std::cout << "\n\n-----------------------------\n\n"
             << parsed << "\n\n-----------------------------\nVersion "

mods/src/config_save.cc

@@ -0,0 +1,141 @@
+#include "config_save.h"
+
+#include <atomic>
+#include <cerrno>
+#include <chrono>
+#include <cstdio>
+#include <sstream>
+#include <stdexcept>
+
+#if _WIN32
+#include <Windows.h>
+#include <sddl.h>
+#include <fcntl.h>
+#include <io.h>
+#pragma comment(lib, "advapi32.lib")
+#else
+#include <fcntl.h>
+#include <unistd.h>
+#endif
+
+// Compile-time substitutions are used only by the isolated failure fixture.
+#ifndef CONFIG_SAVE_WRITE
+#define CONFIG_SAVE_WRITE std::fwrite
+#endif
+#ifndef CONFIG_SAVE_CLOSE
+#define CONFIG_SAVE_CLOSE std::fclose
+#endif
+
+void SaveConfigDocument(const toml::table& config, const std::filesystem::path& path, std::string_view header)
+{
+  // Serialize and validate before opening any file. Values are encoded by toml++,
+  // never interpolated into TOML source. Validate the header too.
+  std::ostringstream output;
+  output.exceptions(std::ios::badbit | std::ios::failbit);
+  output << header << config;
+  const auto bytes = output.str();
+  (void)toml::parse(bytes);
+
+  // weakly_canonical alone leaves a dangling final symlink unresolved.
+  // Follow it before staging, preserving the former ofstream behavior.
+  auto destination = std::filesystem::weakly_canonical(path);
+  unsigned links = 0;
+  while (std::filesystem::is_symlink(std::filesystem::symlink_status(destination))) {
+    if (++links > 40)
+      throw std::filesystem::filesystem_error("config symlink cycle", path,
+                                              std::make_error_code(std::errc::too_many_symbolic_link_levels));
+    auto target = std::filesystem::read_symlink(destination);
+    destination = std::filesystem::weakly_canonical(target.is_absolute() ? target : destination.parent_path() / target);
+  }
+  static std::atomic<unsigned long long> sequence{0};
+  auto                                   temporary = destination;
+  temporary += ".tmp-" + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count()) + "-"
+               + std::to_string(sequence.fetch_add(1, std::memory_order_relaxed));
+
+  // Configs may contain tokens. Protect staging at creation, before any bytes,
+  // even if the destination is private beneath a more permissive directory.
+  std::FILE* file = nullptr;
+#if _WIN32
+  PSECURITY_DESCRIPTOR security = nullptr;
+  if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
+          L"D:P(A;;FA;;;OW)(A;;FA;;;SY)", SDDL_REVISION_1, &security, nullptr))
+    throw std::system_error(GetLastError(), std::system_category(), "could not protect temporary config file");
+  SECURITY_ATTRIBUTES attributes{sizeof(SECURITY_ATTRIBUTES), security, FALSE};
+  const auto handle = CreateFileW(temporary.c_str(), GENERIC_WRITE | READ_CONTROL, 0, &attributes,
+                                  CREATE_NEW, FILE_ATTRIBUTE_NORMAL, nullptr);
+  const auto create_error = GetLastError();
+  LocalFree(security);
+  if (handle == INVALID_HANDLE_VALUE)
+    throw std::system_error(create_error, std::system_category(), "could not create temporary config file");
+  const auto descriptor = _open_osfhandle(reinterpret_cast<intptr_t>(handle), _O_WRONLY | _O_BINARY);
+  if (descriptor == -1) {
+    CloseHandle(handle);
+  } else {
+    file = _fdopen(descriptor, "wb");
+    if (!file)
+      _close(descriptor);
+  }
+#else
+  const auto descriptor = ::open(temporary.c_str(), O_WRONLY | O_CREAT | O_EXCL, 0600);
+  if (descriptor == -1)
+    throw std::system_error(errno, std::generic_category(), "could not create temporary config file");
+  file = ::fdopen(descriptor, "wb");
+  if (!file)
+    ::close(descriptor);
+#endif
+  if (!file) {
+    const auto error = errno;
+    std::error_code ignored;
+    std::filesystem::remove(temporary, ignored);
+    throw std::system_error(error, std::generic_category(), "could not open temporary config stream");
+  }
+
+  bool replacing = false;
+  try {
+    if (CONFIG_SAVE_WRITE(bytes.data(), 1, bytes.size(), file) != bytes.size()) {
+      throw std::system_error(errno, std::generic_category(), "could not write temporary config file");
+    }
+    const auto closed = CONFIG_SAVE_CLOSE(file); // Includes flushing; failure prevents replacement.
+    file              = nullptr;
+    if (closed != 0) {
+      throw std::system_error(errno, std::generic_category(), "could not close temporary config file");
+    }
+#if _WIN32
+    // Let Windows retain the existing file's permissions and streams. A backup
+    // protects the old contents in ReplaceFile's documented partial-failure cases.
+    auto backup = temporary;
+    backup += ".bak";
+    if (!ReplaceFileW(destination.c_str(), temporary.c_str(), backup.c_str(), 0, nullptr, nullptr)) {
+      auto error = GetLastError();
+      if (error == ERROR_FILE_NOT_FOUND) {
+        // Missing-destination fallback: do not overwrite a file appearing before
+        // this move. The caller's earlier existence check is not a create-only transaction.
+        error = MoveFileExW(temporary.c_str(), destination.c_str(), 0) ? ERROR_SUCCESS : GetLastError();
+      }
+      if (error != ERROR_SUCCESS) {
+        replacing = error == ERROR_UNABLE_TO_MOVE_REPLACEMENT || error == ERROR_UNABLE_TO_MOVE_REPLACEMENT_2;
+        throw std::filesystem::filesystem_error(
+            replacing ? "config replacement failed; retain temporary/backup for recovery" : "config replacement failed",
+            temporary, destination, std::error_code(error, std::system_category()));
+      }
+    }
+    std::error_code ignored;
+    std::filesystem::remove(backup, ignored);
+#else
+    // Preserve ordinary permission bits when replacing an existing config.
+    if (std::filesystem::exists(destination)) {
+      std::filesystem::permissions(temporary, std::filesystem::status(destination).permissions());
+    }
+    std::filesystem::rename(temporary, destination);
+#endif
+  } catch (...) {
+    if (file) {
+      std::fclose(file);
+    }
+    std::error_code ignored;
+    if (!replacing) {
+      std::filesystem::remove(temporary, ignored);
+    }
+    throw;
+  }
+}

mods/src/config_save.h

@@ -0,0 +1,9 @@
+#pragma once
+
+#include <filesystem>
+#include <string_view>
+#include <toml++/toml.h>
+
+// Synchronous whole-document output for startup, not a runtime setting editor.
+// Throws on failure; the caller owns reporting. Does not guarantee power-loss durability.
+void SaveConfigDocument(const toml::table& config, const std::filesystem::path& path, std::string_view header = {});

tests/config_save_failure_test.cc

@@ -0,0 +1,113 @@
+#include <cerrno>
+#include <cstdio>
+#include <cassert>
+#include <filesystem>
+#if _WIN32
+#include <Windows.h>
+#include <Aclapi.h>
+#include <sddl.h>
+#include <io.h>
+#else
+#include <sys/stat.h>
+#endif
+
+static std::filesystem::path staging;
+static void CheckPrivateStaging(std::FILE* file)
+{
+#if _WIN32
+  PACL acl = nullptr;
+  PSECURITY_DESCRIPTOR security = nullptr;
+  const auto handle = file ? reinterpret_cast<HANDLE>(_get_osfhandle(_fileno(file))) : nullptr;
+  const auto error = file ? GetSecurityInfo(handle, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION,
+                                           nullptr, nullptr, &acl, nullptr, &security)
+                          : GetNamedSecurityInfoW(const_cast<wchar_t*>(staging.c_str()), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION,
+                                                  nullptr, nullptr, &acl, nullptr, &security);
+  assert(error == ERROR_SUCCESS && acl && acl->AceCount == 2);
+  SECURITY_DESCRIPTOR_CONTROL control;
+  DWORD revision;
+  assert(GetSecurityDescriptorControl(security, &control, &revision) && (control & SE_DACL_PROTECTED));
+  for (DWORD i = 0; i < acl->AceCount; ++i) {
+    void* entry = nullptr;
+    assert(GetAce(acl, i, &entry));
+    const auto* ace = static_cast<ACCESS_ALLOWED_ACE*>(entry);
+    assert(ace->Header.AceType == ACCESS_ALLOWED_ACE_TYPE && !(ace->Header.AceFlags & INHERITED_ACE));
+    wchar_t* sid = nullptr;
+    assert(ConvertSidToStringSidW(const_cast<DWORD*>(&ace->SidStart), &sid));
+    assert(std::wstring_view(sid) == L"S-1-3-4" || std::wstring_view(sid) == L"S-1-5-18");
+    LocalFree(sid);
+  }
+  LocalFree(security);
+#else
+  struct stat status;
+  assert((file ? ::fstat(fileno(file), &status) : ::stat(staging.c_str(), &status)) == 0);
+  assert((status.st_mode & 0777) == 0600);
+#endif
+}
+
+static bool failClose = false;
+
+static std::size_t ShortWrite(const void* data, std::size_t size, std::size_t count, std::FILE* file)
+{
+  for (const auto& entry : std::filesystem::directory_iterator(staging.parent_path()))
+    if (entry.path().filename().string().find(".tmp-") != std::string::npos)
+      staging = entry.path();
+  CheckPrivateStaging(file);
+  const auto written = std::fwrite(data, size, failClose ? count : count / 2, file);
+  CheckPrivateStaging(file);
+  if (failClose)
+    return written;
+  errno              = ENOSPC;
+  return written;
+}
+
+static int FailedClose(std::FILE* file)
+{
+  std::fclose(file);
+  CheckPrivateStaging(nullptr);
+  errno = ENOSPC;
+  return EOF;
+}
+
+// Exercise the production cleanup path without adding runtime injection controls.
+#define CONFIG_SAVE_WRITE ShortWrite
+#define CONFIG_SAVE_CLOSE FailedClose
+#include "../mods/src/config_save.cc"
+
+#include <cassert>
+#include <fstream>
+#include <iostream>
+
+int main(int argc, char** argv)
+{
+  assert(argc == 2);
+  const std::filesystem::path root(argv[1]);
+  std::filesystem::create_directories(root);
+  const auto        path     = root / "settings.toml";
+  staging = path;
+  const std::string original = "# keep this exactly\nenabled = false\n";
+  {
+    std::ofstream out(path, std::ios::binary);
+    out << original;
+  }
+#if !_WIN32
+  ::umask(0022);
+  std::filesystem::permissions(path, std::filesystem::perms::owner_read | std::filesystem::perms::owner_write);
+#endif
+  for (bool closeFailure : {false, true}) {
+    failClose   = closeFailure;
+    bool failed = false;
+    try {
+      SaveConfigDocument(toml::table{{"enabled", true}}, path);
+    } catch (const std::system_error&) {
+      failed = true;
+    }
+    assert(failed);
+    std::ifstream     input(path, std::ios::binary);
+    const std::string actual(std::istreambuf_iterator<char>{input}, {});
+    assert(actual == original);
+    for (const auto& entry : std::filesystem::directory_iterator(root)) {
+      assert(entry.path() == path);
+    }
+  }
+  std::cout << "Short-write and failed-close fixtures passed\n";
+}