Merge pull request #10104 from ThomasWaldmann/flags-1345

file flags: fix generic set_flags, keep unprivileged flags on EPERM, warn on failure

docs/usage/notes.rst

@@ -81,6 +81,27 @@ On Linux, dealing with the flags needs some additional syscalls. Especially when
 dealing with lots of small files, this causes a noticeable overhead, so you can
 use this option also for speeding up operations.
 
+File flags (bsdflags) between platforms
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Borg archives file flags as BSD-style ``chflags(2)`` values, on all platforms.
+On Linux, the file attributes nodump, immutable and append-only (see
+``ioctl_iflags(2)`` / ``chattr(1)``) are translated to/from the corresponding
+BSD-style flags; other Linux attributes are not archived.
+
+The available flags differ between platforms, so when extracting, borg only
+sets the flags known to be settable from userspace on the destination platform —
+unsupported flags are dropped individually, the supported ones are still set.
+Flag bits that are not settable from userspace (like macOS's ``UF_COMPRESSED``
+or ``SF_DATALESS``, or unknown/future flags) are never modified: the extracted
+file keeps whatever the OS gives it.
+
+Some flags need special privileges to set (the super-user-only ``SF_*`` flags
+on BSD/macOS, immutable/append-only on Linux). If setting them is not
+permitted, borg still restores the remaining flags (like nodump) and skips the
+privileged ones. If the flags of a file cannot be set at all, borg issues a
+warning and sets the exit code to warning status.
+
 ``--umask``
 ~~~~~~~~~~~
 

src/borg/archive.py

@@ -1126,8 +1126,9 @@ Duration: {0.duration}
         if not is_win32 and not self.noflags and "bsdflags" in item:
             try:
                 set_flags(path, item.bsdflags, fd=fd)
-            except OSError:
-                pass
+            except OSError as e:
+                logger.warning("%s: when setting file flags: %s", remove_surrogates(str(path)), e)
+                set_ec(EXIT_WARNING)
 
     def set_meta(self, key, value):
         metadata = self._load_meta(self.id)

src/borg/platform/base.py

@@ -3,6 +3,7 @@ import functools
 import io
 import os
 import socket
+import stat
 import unicodedata
 import uuid
 from pathlib import Path
@@ -96,16 +97,52 @@ def acl_text_to_xattr(acl, numeric_ids=False):
     raise NotImplementedError
 
 
-try:
-    from os import lchflags  # type: ignore[attr-defined]
+# BSD-style file flags: only influence flags that are known to be settable from userspace
+# and preserve everything else (including unknown or future flags), see #9039.
+# The masks are built from flag names so that a constant missing on some platform or
+# Python version simply contributes 0. They are also used by the freebsd/darwin modules.
+OWNER_SETTABLE_FLAG_NAMES = ("UF_NODUMP", "UF_IMMUTABLE", "UF_APPEND", "UF_OPAQUE", "UF_NOUNLINK", "UF_HIDDEN")
+SUPERUSER_SETTABLE_FLAG_NAMES = ("SF_ARCHIVED", "SF_IMMUTABLE", "SF_APPEND", "SF_NOUNLINK")
 
-    def set_flags(path, bsd_flags, fd=None):
-        lchflags(path, bsd_flags)
+OWNER_SETTABLE_FLAGS_MASK = 0
+for _name in OWNER_SETTABLE_FLAG_NAMES:
+    OWNER_SETTABLE_FLAGS_MASK |= getattr(stat, _name, 0)
 
-except ImportError:
+SETTABLE_FLAGS_MASK = OWNER_SETTABLE_FLAGS_MASK
+for _name in SUPERUSER_SETTABLE_FLAG_NAMES:
+    SETTABLE_FLAGS_MASK |= getattr(stat, _name, 0)
 
-    def set_flags(path, bsd_flags, fd=None):
-        pass
+
+def set_flags(path, bsd_flags, fd=None):
+    """Set BSD-style file flags, preserving flags that are not settable from userspace."""
+    # Look up lchflags dynamically: it does not exist on all platforms (then this is a no-op).
+    lchflags = getattr(os, "lchflags", None)
+    if lchflags is None:
+        return
+    # Determine current flags.
+    try:
+        st = os.fstat(fd) if fd is not None else os.lstat(path)
+        current = st.st_flags
+    except (OSError, AttributeError):
+        # We can't determine the current flags, so better give up than corrupting anything.
+        return
+    # Python has no os.fchflags: an fd can only be used for fstat, the flags must be written via the path.
+    # The freebsd/darwin modules override this function to use fchflags(2) for that case.
+    mask = SETTABLE_FLAGS_MASK
+    while True:
+        try:
+            # Replace only the bits we want to influence, keep all others.
+            lchflags(path, (current & ~mask) | (bsd_flags & mask))
+            return
+        except OSError as e:
+            if e.errno == errno.EOPNOTSUPP:
+                return  # some filesystems do not support flags
+            if e.errno == errno.EPERM and mask != OWNER_SETTABLE_FLAGS_MASK:
+                # Not permitted to change super-user-only flags (e.g. not running as root):
+                # retry, influencing only the owner-settable flags.
+                mask = OWNER_SETTABLE_FLAGS_MASK
+                continue
+            raise
 
 
 def get_flags(path, st, fd=None):

src/borg/platform/darwin.pyx

@@ -213,31 +213,12 @@ cdef extern from "sys/stat.h":
     int fchflags(int fd, uint32_t flags)
 
 
-# Known-good settable flags from macOS chflags(2). We intentionally do NOT include
-# internal flags like UF_COMPRESSED and SF_DATALESS. Resolved once at import time.
-# getattr(..., 0) keeps this importable on non-Darwin platforms or Python versions
-# missing some constants.
-import stat as stat_mod
-
-SETTABLE_FLAG_NAMES = (
-    # Owner-settable (UF_*)
-    'UF_NODUMP',
-    'UF_IMMUTABLE',
-    'UF_APPEND',
-    'UF_OPAQUE',
-    'UF_NOUNLINK',
-    'UF_HIDDEN',
-    # Super-user-settable (SF_*)
-    'SF_ARCHIVED',
-    'SF_IMMUTABLE',
-    'SF_APPEND',
-    # SF_NOUNLINK exists on some BSDs; include defensively
-    'SF_NOUNLINK',
-)
-
-cdef uint32_t SETTABLE_FLAGS_MASK = 0
-for _name in SETTABLE_FLAG_NAMES:
-    SETTABLE_FLAGS_MASK |= <uint32_t> getattr(stat_mod, _name, 0)
+# Known-good settable flags from macOS chflags(2). We intentionally do NOT influence
+# internal flags like UF_COMPRESSED and SF_DATALESS.
+# The masks are defined in platform.base and shared by all platforms, see #9039.
+# Same logic as platform.base.set_flags, but uses fchflags(2) when an fd is given
+# (Python has no os.fchflags), so the flags are set on the open file, not via the path.
+from .base import OWNER_SETTABLE_FLAGS_MASK, SETTABLE_FLAGS_MASK
 
 
 def set_flags(path, bsd_flags, fd=None):
@@ -253,17 +234,27 @@ def set_flags(path, bsd_flags, fd=None):
         # We can't determine the current flags, so better give up than corrupting anything.
         return
 
-    new_flags = (current & ~SETTABLE_FLAGS_MASK) | (bsd_flags & SETTABLE_FLAGS_MASK)
-
-    # Apply flags.
-    cdef uint32_t c_flags = <uint32_t> new_flags
-    if fd is not None:
-        if fchflags(fd, c_flags) == -1:
-            raise OSError(errno.errno, os.strerror(errno.errno), path)
-    else:
-        path_bytes = os.fsencode(path)
-        if lchflags(path_bytes, c_flags) == -1:
-            raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path_bytes))
+    cdef uint32_t c_flags
+    mask = SETTABLE_FLAGS_MASK
+    path_bytes = os.fsencode(path)
+    while True:
+        # Replace only the bits we want to influence, keep all others.
+        c_flags = <uint32_t> ((current & ~mask) | (bsd_flags & mask))
+        if fd is not None:
+            result = fchflags(fd, c_flags)
+        else:
+            result = lchflags(path_bytes, c_flags)
+        if result != -1:
+            return
+        err = errno.errno
+        if err == errno.EOPNOTSUPP:
+            return  # some filesystems do not support flags
+        if err == errno.EPERM and mask != OWNER_SETTABLE_FLAGS_MASK:
+            # Not permitted to change super-user-only flags (e.g. not running as root):
+            # retry, influencing only the owner-settable flags.
+            mask = OWNER_SETTABLE_FLAGS_MASK
+            continue
+        raise OSError(err, os.strerror(err), path)
 
 
 import errno as errno_mod

src/borg/platform/freebsd.pyx

@@ -240,29 +240,10 @@ cdef extern from "sys/stat.h":
 # BSD file flags (FreeBSD)
 # ----------------------------
 # Only influence flags that are known to be settable and leave system-managed/read-only flags untouched.
-# We express the mask in terms of names to avoid hard failures if a constant does
-# not exist on a given FreeBSD version; missing names simply contribute 0.
-import stat as stat_mod
-
-SETTABLE_FLAG_NAMES = (
-    # Owner-settable (UF_*)
-    'UF_NODUMP',
-    'UF_IMMUTABLE',
-    'UF_APPEND',
-    'UF_OPAQUE',
-    'UF_NOUNLINK',
-    'UF_HIDDEN',
-    # Super-user-only (SF_*)
-    'SF_ARCHIVED',
-    'SF_IMMUTABLE',
-    'SF_APPEND',
-    'SF_NOUNLINK',
-)
-
-cdef unsigned long SETTABLE_FLAGS_MASK = 0
-for _name in SETTABLE_FLAG_NAMES:
-    # getattr(..., 0) keeps this importable when flags are missing on some FreeBSD versions
-    SETTABLE_FLAGS_MASK |= <unsigned long> getattr(stat_mod, _name, 0)
+# The masks are defined in platform.base and shared by all platforms, see #9039.
+# Same logic as platform.base.set_flags, but uses fchflags(2) when an fd is given
+# (Python has no os.fchflags), so the flags are set on the open file, not via the path.
+from .base import OWNER_SETTABLE_FLAGS_MASK, SETTABLE_FLAGS_MASK
 
 
 def set_flags(path, bsd_flags, fd=None):
@@ -281,19 +262,24 @@ def set_flags(path, bsd_flags, fd=None):
         # We can't determine the current flags, so better give up than corrupting anything.
         return
 
-    new_flags = (current & ~SETTABLE_FLAGS_MASK) | (bsd_flags & SETTABLE_FLAGS_MASK)
-
-    cdef unsigned long c_flags = <unsigned long> new_flags
-    if fd is not None:
-        if fchflags(fd, c_flags) == -1:
-            err = errno.errno
-            # Some filesystems may not support flags; ignore EOPNOTSUPP quietly.
-            if err != errno.EOPNOTSUPP:
-                # Keep error signature consistent with other platforms; st may not exist here.
-                raise OSError(err, os.strerror(err), path)
-    else:
-        path_bytes = os.fsencode(path)
-        if lchflags(path_bytes, c_flags) == -1:
-            err = errno.errno
-            if err != errno.EOPNOTSUPP:
-                raise OSError(err, os.strerror(err), os.fsdecode(path_bytes))
+    cdef unsigned long c_flags
+    mask = SETTABLE_FLAGS_MASK
+    path_bytes = os.fsencode(path)
+    while True:
+        # Replace only the bits we want to influence, keep all others.
+        c_flags = <unsigned long> ((current & ~mask) | (bsd_flags & mask))
+        if fd is not None:
+            result = fchflags(fd, c_flags)
+        else:
+            result = lchflags(path_bytes, c_flags)
+        if result != -1:
+            return
+        err = errno.errno
+        if err == errno.EOPNOTSUPP:
+            return  # some filesystems do not support flags
+        if err == errno.EPERM and mask != OWNER_SETTABLE_FLAGS_MASK:
+            # Not permitted to change super-user-only flags (e.g. not running as root):
+            # retry, influencing only the owner-settable flags.
+            mask = OWNER_SETTABLE_FLAGS_MASK
+            continue
+        raise OSError(err, os.strerror(err), path)

src/borg/platform/linux.pyx

@@ -64,7 +64,6 @@ cdef extern from "linux/fs.h":
     int FS_NODUMP_FL
     int FS_IMMUTABLE_FL
     int FS_APPEND_FL
-    int FS_COMPR_FL
 
 cdef extern from "sys/ioctl.h":
     int ioctl(int fildes, int request, ...)
@@ -137,6 +136,7 @@ def set_flags(path, bsd_flags, fd=None):
         if stat.S_ISBLK(st.st_mode) or stat.S_ISCHR(st.st_mode) or stat.S_ISLNK(st.st_mode):
             # see comment in get_flags()
             return
+    cdef int current
     cdef int flags
     cdef int mask = LINUX_MASK  # 1 at positions we want to influence
     cdef int new_flags = 0
@@ -149,23 +149,32 @@ def set_flags(path, bsd_flags, fd=None):
         fd = os.open(path, os.O_RDONLY|os.O_NONBLOCK|os.O_NOFOLLOW)
     try:
         # Get current flags.
-        if ioctl(fd, FS_IOC_GETFLAGS, &flags) == -1:
+        if ioctl(fd, FS_IOC_GETFLAGS, &current) == -1:
             # If this fails, give up because it is either not supported by the fs
             # or maybe not permitted? If we can't determine the current flags,
             # we better not risk corrupting them by setflags, see the comment below.
             return  # give up silently
 
-        # Replace only the bits we actually want to influence, keep others.
-        # We can't just set all flags to the archived value, because we might
-        # reset flags that are not controllable from userspace, see #9039.
-        flags = (flags & ~mask) | (new_flags & mask)
+        while True:
+            # Replace only the bits we actually want to influence, keep others.
+            # We can't just set all flags to the archived value, because we might
+            # reset flags that are not controllable from userspace, see #9039.
+            flags = (current & ~mask) | (new_flags & mask)
 
-        if ioctl(fd, FS_IOC_SETFLAGS, &flags) == -1:
+            if ioctl(fd, FS_IOC_SETFLAGS, &flags) != -1:
+                return
             error_number = errno.errno
             # Usually we would only catch EOPNOTSUPP here, but Linux Kernel 6.17
             # has a bug where it returns ENOTTY instead of EOPNOTSUPP.
-            if error_number not in (errno.EOPNOTSUPP, errno.ENOTTY):
-                raise OSError(error_number, strerror(error_number).decode(), path)
+            if error_number in (errno.EOPNOTSUPP, errno.ENOTTY):
+                return
+            if error_number == errno.EPERM and mask != FS_NODUMP_FL:
+                # Changing FS_IMMUTABLE_FL / FS_APPEND_FL needs CAP_LINUX_IMMUTABLE.
+                # Retry, influencing only FS_NODUMP_FL, so an unprivileged extract
+                # can still restore the nodump flag.
+                mask = FS_NODUMP_FL
+                continue
+            raise OSError(error_number, strerror(error_number).decode(), path)
     finally:
         if open_fd:
             os.close(fd)

src/borg/testsuite/archiver/extract_cmd_test.py

@@ -849,6 +849,25 @@ def test_extract_restores_append_flag(archivers, request):
         assert (flags & stat.UF_APPEND) == stat.UF_APPEND
 
 
+@pytest.mark.skipif(is_win32, reason="file flags are not restored on win32")
+def test_extract_flags_errors(archivers, request):
+    archiver = request.getfixturevalue(archivers)
+    if archiver.EXE:
+        pytest.skip("Skipping binary test due to patch objects")
+
+    def patched_set_flags(path, bsd_flags, fd=None):
+        raise OSError(errno.EPERM, "EPERM")
+
+    create_regular_file(archiver.input_path, "file")
+    cmd(archiver, "repo-create", "-e", "none-sha256")
+    cmd(archiver, "create", "test", "input")
+    with changedir("output"):
+        with patch.object(archive_module, "set_flags", patched_set_flags):
+            out = cmd(archiver, "extract", "test", exit_code=EXIT_WARNING)
+            assert "when setting file flags: " in out
+            assert "EPERM" in out
+
+
 def test_overwrite(archivers, request):
     archiver = request.getfixturevalue(archivers)
     if archiver.EXE:

src/borg/testsuite/platform/platform_test.py

@@ -1,12 +1,14 @@
 import errno
 import functools
 import os
+import stat
 
 import pytest
 
 from ...platformflags import is_darwin, is_freebsd, is_linux, is_win32
 from ...platform import acl_get, acl_set
 from ...platform import get_process_id, process_alive
+from ...platform import base
 from .. import unopened_tempfile
 from ..fslocking_test import free_pid  # NOQA
 
@@ -95,3 +97,81 @@ def test_process_id():
     assert len(hostname) > 0
     assert pid > 0
     assert get_process_id() == (hostname, pid, tid)
+
+
+# base.set_flags tests: fake lstat/lchflags, so they run on all platforms (incl. those without chflags).
+FLAGS_TESTFILE = "base-set-flags-testfile"
+
+
+def patch_flags(monkeypatch, current_flags, lchflags_func):
+    class FakeStat:
+        st_flags = current_flags
+
+    real_lstat = os.lstat
+
+    def fake_lstat(path, *args, **kwargs):
+        return FakeStat() if path == FLAGS_TESTFILE else real_lstat(path, *args, **kwargs)
+
+    monkeypatch.setattr(os, "lstat", fake_lstat)
+    monkeypatch.setattr(os, "lchflags", lchflags_func, raising=False)
+
+
+def test_base_set_flags_masks_and_preserves(monkeypatch):
+    # only settable flags may be influenced; all other bits must be preserved as-is, see #9039.
+    calls = []
+    patch_flags(monkeypatch, stat.SF_SNAPSHOT | stat.UF_COMPRESSED | stat.UF_NODUMP, lambda p, f: calls.append(f))
+    base.set_flags(FLAGS_TESTFILE, stat.UF_IMMUTABLE | stat.UF_COMPRESSED)
+    # UF_IMMUTABLE (settable) gets applied, UF_NODUMP (settable, not archived) gets cleared,
+    # UF_COMPRESSED / SF_SNAPSHOT (not settable) keep their on-disk state, the archived value is ignored.
+    assert calls == [stat.SF_SNAPSHOT | stat.UF_COMPRESSED | stat.UF_IMMUTABLE]
+
+
+def test_base_set_flags_eperm_retries_without_sf_flags(monkeypatch):
+    # when we lack permission for super-user-only flags, the owner-settable flags shall still be restored.
+    calls = []
+
+    def fake_lchflags(path, flags):
+        calls.append(flags)
+        if len(calls) == 1:
+            raise OSError(errno.EPERM, "Operation not permitted", path)
+
+    patch_flags(monkeypatch, stat.SF_SNAPSHOT, fake_lchflags)
+    base.set_flags(FLAGS_TESTFILE, stat.UF_NODUMP | stat.SF_ARCHIVED)
+    assert calls == [
+        stat.SF_SNAPSHOT | stat.UF_NODUMP | stat.SF_ARCHIVED,  # full attempt
+        stat.SF_SNAPSHOT | stat.UF_NODUMP,  # retry without super-user-only flags
+    ]
+
+
+def test_base_set_flags_eperm_raises_after_retry(monkeypatch):
+    calls = []
+
+    def fake_lchflags(path, flags):
+        calls.append(flags)
+        raise OSError(errno.EPERM, "Operation not permitted", path)
+
+    patch_flags(monkeypatch, 0, fake_lchflags)
+    with pytest.raises(OSError):
+        base.set_flags(FLAGS_TESTFILE, stat.UF_NODUMP)
+    assert len(calls) == 2  # full attempt + owner-settable-only retry
+
+
+def test_base_set_flags_unsupported_fs(monkeypatch):
+    def fake_lchflags(path, flags):
+        raise OSError(errno.EOPNOTSUPP, "Operation not supported", path)
+
+    patch_flags(monkeypatch, 0, fake_lchflags)
+    base.set_flags(FLAGS_TESTFILE, stat.UF_NODUMP)  # must not raise
+
+
+def test_base_set_flags_no_current_flags(monkeypatch):
+    # if the current flags can't be determined, do nothing (rather than risk corrupting them).
+    calls = []
+
+    def fake_lstat(path, *args, **kwargs):
+        raise OSError(errno.ENOENT, "No such file or directory", path)
+
+    monkeypatch.setattr(os, "lstat", fake_lstat)
+    monkeypatch.setattr(os, "lchflags", lambda p, f: calls.append(f), raising=False)
+    base.set_flags(FLAGS_TESTFILE, stat.UF_NODUMP)
+    assert calls == []