From 5cc663c8306e517bf021c7fe731ed4971f464111 Mon Sep 17 00:00:00 2001
From: Don-Vito <khvitaly@gmail.com>
Date: Thu, 12 Aug 2021 19:47:16 +0300
Subject: [PATCH] Fix WriteUTF8FileAtomic to preserve symlinks (#10908)

WriteUTF8FileAtomic  overrides the content of the file "atomically"
by creating a temp file and then renaming it to the original path.
The problem arises when the original path is symbolic link,
as the link itself gets overridden by a file (rather than the link target).
This PR introduces a special handling of the symlinks:
if the path as a symlink we resolve the path and use:
1. target's directory to create a temp-file in
2. target itself to be replaced with the tempfile.

Symlink resolution is problematic when the target path does not exist,
as there is no good utility that resolves such link (canonical() fails).
In this corner case we skip the "atomic" approach of renaming the file
and write the link target directly.

Closes #10787
---
 .github/actions/spelling/expect/expect.txt    |  1 +
 .../TerminalSettingsModel/FileUtils.cpp       | 24 +++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt
index 68e8addb4e7..d0472889b6a 100644
--- a/.github/actions/spelling/expect/expect.txt
+++ b/.github/actions/spelling/expect/expect.txt
@@ -2241,6 +2241,7 @@ SWMR
 SWP
 swprintf
 SYMED
+symlink
 SYNCPAINT
 sys
 syscalls
diff --git a/src/cascadia/TerminalSettingsModel/FileUtils.cpp b/src/cascadia/TerminalSettingsModel/FileUtils.cpp
index b952f6a3d3c..81fbb6cee67 100644
--- a/src/cascadia/TerminalSettingsModel/FileUtils.cpp
+++ b/src/cascadia/TerminalSettingsModel/FileUtils.cpp
@@ -123,7 +123,27 @@ namespace Microsoft::Terminal::Settings::Model
 
     void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view content)
     {
-        auto tmpPath = path;
+        // GH#10787: rename() will replace symbolic links themselves and not the path they point at.
+        // It's thus important that we first resolve them before generating temporary path.
+        std::error_code ec;
+        const auto resolvedPath = std::filesystem::is_symlink(path) ? std::filesystem::canonical(path, ec) : path;
+        if (ec)
+        {
+            if (ec.value() != ERROR_FILE_NOT_FOUND)
+            {
+                THROW_WIN32_MSG(ec.value(), "failed to compute canonical path");
+            }
+
+            // The original file is a symbolic link, but the target doesn't exist.
+            // Consider two fall-backs:
+            //   * resolve the link manually, which might be less accurate and more prone to race conditions
+            //   * write to the file directly, which lets the system resolve the symbolic link but leaves the write non-atomic
+            // The latter is chosen, as this is an edge case and our 'atomic' writes are only best-effort.
+            WriteUTF8File(path, content);
+            return;
+        }
+
+        auto tmpPath = resolvedPath;
         tmpPath += L".tmp";
 
         // Writing to a file isn't atomic, but...
@@ -132,6 +152,6 @@ namespace Microsoft::Terminal::Settings::Model
         // renaming one is (supposed to be) atomic.
         // Wait... "supposed to be"!? Well it's technically not always atomic,
         // but it's pretty darn close to it, so... better than nothing.
-        std::filesystem::rename(tmpPath, path);
+        std::filesystem::rename(tmpPath, resolvedPath);
     }
 }