create: do not archive an atime we caused ourselves, fixes #6194

src/borg/archive.py

@@ -287,6 +287,28 @@ def backup_io_iter(iterator):
         yield item
 
 
+class StatOrigAtime:
+    """
+    An os.stat_result look-alike: all attributes come from *st*, only the
+    access time comes from *st_orig*.
+
+    Rationale: borg needs to open() a fs item before it can fstat() it. If the
+    platform does not support O_NOATIME (or we are not allowed to use it), that
+    open() might have already updated the atime - we do not want to archive such
+    an atime, but the one the item had before borg touched it.
+    """
+
+    __slots__ = ("_st", "st_atime", "st_atime_ns")
+
+    def __init__(self, st, st_orig):
+        self._st = st
+        self.st_atime = st_orig.st_atime
+        self.st_atime_ns = st_orig.st_atime_ns
+
+    def __getattr__(self, name):
+        return getattr(self._st, name)
+
+
 def stat_update_check(st_old, st_curr):
     """
     this checks for some race conditions between the first filename-based stat()
@@ -309,6 +331,12 @@ def stat_update_check(st_old, st_curr):
     if st_old.st_ino != st_curr.st_ino:
         # in this case, the hard-links-related code in create_helper has the wrong inode - abort!
         raise BackupRaceConditionError("file inode changed (race condition), skipping file")
+    if st_old.st_atime_ns != st_curr.st_atime_ns:
+        # the atime was updated in between the 2 stat calls - most likely by us, because we
+        # had to open the item and O_NOATIME was not available / not usable, see #6194.
+        # in the (rare) case that somebody else accessed the item at just that moment, we
+        # lose that atime update, but that is much less of an issue than archiving our own.
+        return StatOrigAtime(st_curr, st_old)
     # looks ok, we are still dealing with the same thing - return current stat:
     return st_curr
 

src/borg/testsuite/archive_test.py

@@ -7,13 +7,14 @@
 
 import pytest
 
-from . import rejected_dotdot_paths
+from . import rejected_dotdot_paths, is_utime_fully_supported
 from ..cache import ChunkListEntry
 from ..constants import ROBJ_FILE_STREAM, zeros
 from ..crypto.key import ChecksumKey
 from ..archive import Archive, CacheChunkBuffer, DownloadPipeline, RobustUnpacker, valid_msgpacked_dict
 from ..archive import ITEM_KEYS, Statistics
-from ..archive import BackupOSError, backup_io, backup_io_iter, get_item_uid_gid
+from ..archive import BackupOSError, BackupRaceConditionError, backup_io, backup_io_iter, get_item_uid_gid
+from ..archive import stat_update_check
 from ..helpers import msgpack
 from ..repoobj import RepoObj
 from ..item import Item, ArchiveItem
@@ -467,6 +468,59 @@ def __next__(self):
         assert False, "StopIteration handled incorrectly"
 
 
+def _stat_with_atime(path, atime_ns, mtime_ns=234567890000000000):
+    os.utime(path, ns=(atime_ns, mtime_ns))
+    return os.stat(path)
+
+
+@pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot properly setup and execute test without utime")
+def test_stat_update_check_atime_updated(tmpdir):
+    path = str(tmpdir.join("file"))
+    with open(path, "wb") as f:
+        f.write(b"12345")
+    st_old = _stat_with_atime(path, 123456789000000000)
+    st_curr = _stat_with_atime(path, 987654321000000000)
+    st = stat_update_check(st_old, st_curr)
+    # the atime is the one from before we (usually: by opening the file) touched it, see #6194:
+    assert st.st_atime_ns == st_old.st_atime_ns
+    assert st.st_atime == st_old.st_atime
+    # everything else comes from the current stat:
+    assert st.st_mode == st_curr.st_mode
+    assert st.st_ino == st_curr.st_ino
+    assert st.st_size == st_curr.st_size
+    assert st.st_mtime_ns == st_curr.st_mtime_ns
+    # optional attributes must be present (or absent) just like on a real stat result:
+    assert hasattr(st, "st_birthtime_ns") == hasattr(st_curr, "st_birthtime_ns")
+    with pytest.raises(AttributeError):
+        st.st_does_not_exist
+
+
+@pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot properly setup and execute test without utime")
+def test_stat_update_check_atime_unchanged(tmpdir):
+    path = str(tmpdir.join("file"))
+    with open(path, "wb") as f:
+        f.write(b"12345")
+    st_old = _stat_with_atime(path, 123456789000000000)
+    st_curr = os.stat(path)
+    assert st_old.st_atime_ns == st_curr.st_atime_ns
+    # nothing to fix up, so we get the current stat result as is:
+    assert stat_update_check(st_old, st_curr) is st_curr
+
+
+def test_stat_update_check_race_conditions(tmpdir):
+    file_path = str(tmpdir.join("file"))
+    with open(file_path, "wb"):
+        pass
+    other_path = str(tmpdir.join("other_file"))
+    with open(other_path, "wb"):
+        pass
+    st_file, st_other, st_dir = os.stat(file_path), os.stat(other_path), os.stat(str(tmpdir))
+    with pytest.raises(BackupRaceConditionError):  # file type changed
+        stat_update_check(st_file, st_dir)
+    with pytest.raises(BackupRaceConditionError):  # inode changed
+        stat_update_check(st_file, st_other)
+
+
 def test_get_item_uid_gid():
     # test requires that:
     # - a user/group name for the current process' real uid/gid exists.

src/borg/testsuite/archiver/extract_cmd_test.py

@@ -9,6 +9,7 @@
 
 from ... import xattr
 from ... import platform
+from ... import archive as archive_module
 from ...archive import Archive
 from ...cache import Cache
 from ...chunkers import has_seek_hole
@@ -306,6 +307,47 @@ def has_noatime(some_file):
         assert same_ts_ns(sto.st_atime_ns, atime * 10**9)
 
 
+@pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot properly setup and execute test without utime")
+def test_atime_open_updates_atime(archiver):
+    # simulate a platform where already the open() updates the atime (no O_NOATIME support,
+    # e.g. cygwin) - borg must archive the atime the item had before borg opened it, see #6194.
+    atime, mtime = 123456780, 234567890
+
+    def updates_atime(some_file):
+        os.utime(some_file, (atime, mtime))
+        with open(some_file, "rb") as f:
+            f.read(1)
+        return os.stat(some_file).st_atime_ns != atime * 10**9
+
+    real_os_open = archive_module.os_open
+
+    def os_open_updating_atime(**kwargs):
+        # open without O_NOATIME (as on a platform not having it), so that reading
+        # updates the atime - and read right away, as if the open() had done that.
+        fd = real_os_open(**dict(kwargs, noatime=False))
+        if fd is not None:
+            try:
+                os.pread(fd, 1, 0)  # a read updates the atime (but not the ctime)
+            except OSError:
+                pass  # not seekable / not readable (fifo, directory, ...)
+        return fd
+
+    create_test_files(archiver.input_path)
+    if not updates_atime("input/file1"):
+        pytest.skip("filesystem does not update the atime when reading")
+    os.utime("input/file1", (atime, mtime))
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    with patch.object(archive_module, "os_open", os_open_updating_atime):
+        cmd(archiver, "create", "--atime", "test", "input")
+    # make sure the simulation worked: the open() did update the input file's atime.
+    assert os.stat("input/file1").st_atime_ns != atime * 10**9
+    with changedir("output"):
+        cmd(archiver, "extract", "test")
+    sto = os.stat("output/input/file1")
+    assert same_ts_ns(sto.st_mtime_ns, mtime * 10**9)
+    assert same_ts_ns(sto.st_atime_ns, atime * 10**9)
+
+
 @pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot setup and execute test without utime")
 @pytest.mark.skipif(not is_birthtime_fully_supported(), reason="cannot setup and execute test without birthtime")
 def test_birthtime(archivers, request):