From 9c18c27749c147fe9c66e20f40a63f3adbd22f76 Mon Sep 17 00:00:00 2001
From: "Daniel A. Wozniak" <daniel.wozniak@broadcom.com>
Date: Sat, 27 Jun 2026 14:03:39 -0700
Subject: [PATCH] Fsync atomic_open temp file before rename

salt.utils.atomicfile._AtomicWFile.close() renamed the temp file over
the destination without first flushing user-space buffers and fsyncing
the file's data and metadata. POSIX does not require the kernel to
implicitly fsync on rename, and the fsync-on-rename heuristic of ext4
and friends is not universal. As a result a crash after the rename
could expose a written-but-unsynced file (truncated or partial),
defeating the durability contract callers expect from an "atomic
write" helper.

Flush + os.fsync() the underlying file descriptor before close() so
the new file's contents are durable on disk before the rename swaps
it into place.

Fixes #69583
---
 changelog/69583.fixed.md                    |  1 +
 salt/utils/atomicfile.py                    | 13 +++++++
 tests/pytests/unit/utils/test_atomicfile.py | 42 +++++++++++++++++++++
 3 files changed, 56 insertions(+)
 create mode 100644 changelog/69583.fixed.md

diff --git a/changelog/69583.fixed.md b/changelog/69583.fixed.md
new file mode 100644
index 000000000000..bcb5ea6939bb
--- /dev/null
+++ b/changelog/69583.fixed.md
@@ -0,0 +1 @@
+Fixed `salt.utils.atomicfile.atomic_open` to fsync the temp file before the atomic rename so a crash after the rename cannot expose a truncated or partial file.
diff --git a/salt/utils/atomicfile.py b/salt/utils/atomicfile.py
index a3bf23468042..bd8396e45410 100644
--- a/salt/utils/atomicfile.py
+++ b/salt/utils/atomicfile.py
@@ -128,6 +128,19 @@ def __enter__(self):
     def close(self):
         if self._fh.closed:
             return
+        # Flush user-space buffers and fsync the file's data + metadata
+        # before the atomic rename. Without this, a crash after the rename
+        # can expose a written-but-unsynced (truncated/partial) file:
+        # POSIX does not require an implicit fsync on rename, and the
+        # fsync-on-rename heuristic of ext4 et al. is not universal. The
+        # fileno() / fsync() pair is guarded because some file-like objects
+        # do not expose a real fd (e.g. an in-memory wrapper); in that case
+        # there is nothing useful to sync at the fd level.
+        try:
+            self._fh.flush()
+            os.fsync(self._fh.fileno())
+        except (AttributeError, OSError, ValueError):
+            pass
         self._fh.close()
         if salt.utils.win_dacl.HAS_WIN32:
             if os.path.isfile(self._filename):
diff --git a/tests/pytests/unit/utils/test_atomicfile.py b/tests/pytests/unit/utils/test_atomicfile.py
index 06dfd9a5b687..61b2ab174775 100644
--- a/tests/pytests/unit/utils/test_atomicfile.py
+++ b/tests/pytests/unit/utils/test_atomicfile.py
@@ -2,10 +2,14 @@
 Tests for atomicfile utility module.
 """
 
+import os
+
 import pytest
 
+import salt.utils.atomicfile
 import salt.utils.files
 from salt.utils.atomicfile import atomic_open
+from tests.support.mock import patch
 
 
 @pytest.mark.skip_on_windows(reason="Not a Windows test")
@@ -25,3 +29,41 @@ def test_atomicfile_respects_umask(tmp_path):
 
     assert new_file.read_text() == contents
     assert oct(new_file.stat().st_mode)[-3:] == "644"
+
+
+def test_atomicfile_fsyncs_before_rename(tmp_path):
+    """
+    The atomic_open close path must fsync the temp file before renaming it
+    over the destination, otherwise a crash after the rename can expose a
+    written-but-unsynced (truncated/partial) file. See #69583.
+    """
+    new_file = tmp_path / "foo"
+    fsynced_fds = []
+    call_order = []
+
+    real_fsync = os.fsync
+
+    def tracking_fsync(fd):
+        fsynced_fds.append(fd)
+        call_order.append("fsync")
+        return real_fsync(fd)
+
+    real_rename = salt.utils.atomicfile.atomic_rename
+
+    def tracking_rename(src, dst):
+        call_order.append("rename")
+        return real_rename(src, dst)
+
+    with patch.object(salt.utils.atomicfile.os, "fsync", tracking_fsync), patch.object(
+        salt.utils.atomicfile, "atomic_rename", tracking_rename
+    ):
+        with atomic_open(str(new_file), "w") as fh_:
+            tmp_fd = fh_.fileno()
+            fh_.write("bar")
+
+    assert tmp_fd in fsynced_fds, "expected os.fsync on the temp file fd"
+    assert "fsync" in call_order and "rename" in call_order
+    assert call_order.index("fsync") < call_order.index(
+        "rename"
+    ), "fsync must be called before the atomic rename"
+    assert new_file.read_text() == "bar"