From 9a240753099857e105e70bf67633aa034e2ca3b5 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 5 Aug 2026 12:48:10 +0200 Subject: [PATCH 1/2] do not archive ACLs that are equivalent to the mode bits A directory that only has a default ACL still counts as "extended" for acl_extended_file_nofollow/acl_extended_fd, so borg archived its access ACL, although that ACL just mirrors the traditional permission bits - the kernel does not keep a system.posix_acl_access xattr for it either. "borg mount" then offered such a bogus xattr. Same the other way round: a directory without a default ACL got an empty acl_default archived. Now only archive the access ACL if it is not equivalent to the mode bits (acl_equiv_mode) and the default ACL if it has any entries (acl_entries). The FUSE mounts additionally skip trivial/empty ACLs (acl_is_extended), so archives created by older borg versions do not expose these xattrs either. --- src/borg/fuse.py | 6 ++-- src/borg/hlfuse.py | 6 ++-- src/borg/platform/__init__.py | 1 + src/borg/platform/base.py | 26 ++++++++++++++ src/borg/platform/linux.pyx | 28 ++++++++++----- .../testsuite/archiver/mount_cmds_test.py | 21 +++++++++++ src/borg/testsuite/platform/linux_test.py | 35 +++++++++++++++++++ 7 files changed, 109 insertions(+), 14 deletions(-) diff --git a/src/borg/fuse.py b/src/borg/fuse.py index c61740d2f5..369d53b6d6 100644 --- a/src/borg/fuse.py +++ b/src/borg/fuse.py @@ -74,7 +74,7 @@ def async_wrapper(fn): from .storelocking import LockRefresher from .helpers.lrucache import LRUCache from .item import Item -from .platform import uid2user, gid2group, acl_text_to_xattr +from .platform import uid2user, gid2group, acl_text_to_xattr, acl_is_extended from .platformflags import is_darwin, is_linux from .repository import Repository @@ -683,7 +683,7 @@ def listxattr(self, inode, ctx=None): item = self.get_item(inode) names = list(item.get("xattrs", {}).keys()) # expose the archived POSIX ACLs, so e.g. getfacl or tools copying from the mount can read them. - names.extend(xattr_name for xattr_name, attr in ACL_XATTRS.items() if attr in item) + names.extend(xattr_name for xattr_name, attr in ACL_XATTRS.items() if acl_is_extended(item.get(attr))) return names @async_wrapper @@ -691,7 +691,7 @@ def getxattr(self, inode, name, ctx=None): item = self.get_item(inode) if name in ACL_XATTRS: acl = item.get(ACL_XATTRS[name]) - if acl is None: + if not acl_is_extended(acl): raise llfuse.FUSEError(ENOATTR) try: return acl_text_to_xattr(acl, numeric_ids=self.numeric_ids) diff --git a/src/borg/hlfuse.py b/src/borg/hlfuse.py index 201dccca9a..6413da3aa2 100644 --- a/src/borg/hlfuse.py +++ b/src/borg/hlfuse.py @@ -31,7 +31,7 @@ from .storelocking import LockRefresher from .helpers.lrucache import LRUCache from .item import Item -from .platform import uid2user, gid2group, acl_text_to_xattr +from .platform import uid2user, gid2group, acl_text_to_xattr, acl_is_extended from .platformflags import is_darwin, is_linux from .repository import Repository @@ -593,7 +593,7 @@ def listxattr(self, path): item = self.get_inode(node.ino) result = [k.decode("utf-8", "surrogateescape") for k in item.get("xattrs", {}).keys()] # expose the archived POSIX ACLs, so e.g. getfacl or tools copying from the mount can read them. - result.extend(xattr_name for xattr_name, attr in ACL_XATTRS.items() if attr in item) + result.extend(xattr_name for xattr_name, attr in ACL_XATTRS.items() if acl_is_extended(item.get(attr))) debug_log(f"listxattr -> {result}") return result @@ -606,7 +606,7 @@ def getxattr(self, path, name, position=0): name_str = name if isinstance(name, str) else name.decode("utf-8", "surrogateescape") if name_str in ACL_XATTRS: acl = item.get(ACL_XATTRS[name_str]) - if acl is None: + if not acl_is_extended(acl): debug_log("getxattr -> ENOATTR") raise hlfuse.FuseOSError(ENOATTR) try: diff --git a/src/borg/platform/__init__.py b/src/borg/platform/__init__.py index 168d4ce705..d428ab5450 100644 --- a/src/borg/platform/__init__.py +++ b/src/borg/platform/__init__.py @@ -12,6 +12,7 @@ from .base import SaveFile, sync_dir, fdatasync, safe_fadvise from .base import get_process_id, fqdn, hostname, hostid, swidth from .base import acl_text_to_xattr # overridden below for platforms supporting it +from .base import acl_is_extended # work around pyinstaller "forgetting" to include the xattr module from . import xattr # noqa: F401 diff --git a/src/borg/platform/base.py b/src/borg/platform/base.py index 21e8bf9081..da928f373d 100644 --- a/src/borg/platform/base.py +++ b/src/borg/platform/base.py @@ -95,6 +95,32 @@ def acl_text_to_xattr(acl, numeric_ids=False): raise NotImplementedError +def acl_is_extended(acl): + """ + Does *acl* (an ACL in the borg item text representation) define anything beyond the + traditional permission bits? + + A trivial ACL only has the 3 base entries (owner, owning group, other) and thus is fully + equivalent to the mode bits - the kernel does not keep an ACL xattr for such an ACL. + borg used to archive a trivial acl_access for directories that only had a default ACL + (and an empty acl_default for files/dirs without one), so such ACLs may be present in + older archives and must not be offered as ACL xattrs by the FUSE mount. + """ + if not acl: + return False + for entry in acl.split(b"\n"): + if not entry: + continue + fields = entry.split(b":") + if len(fields) < 3: # unexpected, let the ACL converter deal with it + return True + if fields[1]: # a named user/group entry + return True + if fields[0] not in (b"user", b"group", b"other"): # e.g. a mask entry + return True + return False + + try: from os import lchflags # type: ignore[attr-defined] diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index 7be20073fb..33a950635c 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -17,6 +17,7 @@ except ImportError: SYNC_FILE_RANGE_LOADED = False from libc cimport errno +from posix.types cimport mode_t @@ -48,10 +49,12 @@ cdef extern from "sys/acl.h": int acl_set_file(const char *path, int type, acl_t acl) int acl_set_fd(int fd, acl_t acl) acl_t acl_from_text(const char *buf) + int acl_equiv_mode(acl_t acl, mode_t *mode_p) cdef extern from "acl/libacl.h": int acl_extended_file_nofollow(const char *path) int acl_extended_fd(int fd) + int acl_entries(acl_t acl) char *acl_to_any_text(acl_t acl, const char *prefix, char separator, int options) int TEXT_NUMERIC_IDS @@ -298,10 +301,16 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): access_acl = acl_get_file(path, ACL_TYPE_ACCESS) if access_acl == NULL: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - access_text = acl_to_any_text(access_acl, NULL, '\n', TEXT_NUMERIC_IDS) - if access_text == NULL: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - item['acl_access'] = converter(access_text) + # a directory having only a default ACL also counts as "extended" (see above), but its + # access ACL then just mirrors the traditional permission bits and thus carries no extra + # information. the kernel does not keep a system.posix_acl_access xattr in that case + # either, so do not archive such an access ACL. acl_equiv_mode returns 0 if the ACL is + # equivalent to the mode bits, 1 if it is not and -1 on error (then we rather store it). + if acl_equiv_mode(access_acl, NULL) != 0: + access_text = acl_to_any_text(access_acl, NULL, '\n', TEXT_NUMERIC_IDS) + if access_text == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + item['acl_access'] = converter(access_text) finally: acl_free(access_text) acl_free(access_acl) @@ -311,10 +320,13 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): default_acl = acl_get_file(path, ACL_TYPE_DEFAULT) if default_acl == NULL: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - default_text = acl_to_any_text(default_acl, NULL, '\n', TEXT_NUMERIC_IDS) - if default_text == NULL: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - item['acl_default'] = converter(default_text) + # a directory without a default ACL gives an empty ACL here - nothing to archive + # (same as above: the kernel has no system.posix_acl_default xattr for it). + if acl_entries(default_acl) > 0: + default_text = acl_to_any_text(default_acl, NULL, '\n', TEXT_NUMERIC_IDS) + if default_text == NULL: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + item['acl_default'] = converter(default_text) finally: acl_free(default_text) acl_free(default_acl) diff --git a/src/borg/testsuite/archiver/mount_cmds_test.py b/src/borg/testsuite/archiver/mount_cmds_test.py index f86e50e76b..dd23e2fcb0 100644 --- a/src/borg/testsuite/archiver/mount_cmds_test.py +++ b/src/borg/testsuite/archiver/mount_cmds_test.py @@ -189,16 +189,28 @@ def test_fuse_acls(archivers, request): dir_path = os.path.join(archiver.input_path, "dir1") os.mkdir(dir_path) platform.acl_set(dir_path, {"acl_access": access_acl, "acl_default": default_acl}) + dir2_path = os.path.join(archiver.input_path, "dir2") + os.mkdir(dir2_path) + platform.acl_set(dir2_path, {"acl_default": default_acl}) # only a default ACL, no access ACL cmd(archiver, "create", "archive", "input") mountpoint = os.path.join(archiver.tmpdir, "mountpoint") with fuse_mount(archiver, mountpoint, "-a", "archive"): mounted_file = os.path.join(mountpoint, "archive", "input", "file1") mounted_dir = os.path.join(mountpoint, "archive", "input", "dir1") + mounted_dir2 = os.path.join(mountpoint, "archive", "input", "dir2") # the ACL xattrs must be listed: assert "system.posix_acl_access" in os.listxattr(mounted_file) assert "system.posix_acl_default" not in os.listxattr(mounted_file) assert "system.posix_acl_access" in os.listxattr(mounted_dir) assert "system.posix_acl_default" in os.listxattr(mounted_dir) + # a directory that only has a default ACL must not offer an access ACL xattr + # (its access ACL is just the mode bits, the source fs does not have that xattr either): + assert "system.posix_acl_access" not in os.listxattr(dir2_path) + assert "system.posix_acl_access" not in os.listxattr(mounted_dir2) + assert "system.posix_acl_default" in os.listxattr(mounted_dir2) + with pytest.raises(OSError) as exc_info: + os.getxattr(mounted_dir2, "system.posix_acl_access") + assert exc_info.value.errno in (errno.ENODATA, errno.ENOTSUP) # the binary xattr values must be identical to what the kernel provides for the source fs objects: try: mounted_file_acl = os.getxattr(mounted_file, "system.posix_acl_access") @@ -212,6 +224,9 @@ def test_fuse_acls(archivers, request): assert mounted_file_acl == os.getxattr(file_path, "system.posix_acl_access") for name in ("system.posix_acl_access", "system.posix_acl_default"): assert os.getxattr(mounted_dir, name) == os.getxattr(dir_path, name) + assert os.getxattr(mounted_dir2, "system.posix_acl_default") == os.getxattr( + dir2_path, "system.posix_acl_default" + ) # borg's own ACL code (going through libacl, like getfacl or tools copying # from the mount would) must see the same ACLs through the mount: item_src, item_mnt = {}, {} @@ -223,6 +238,12 @@ def test_fuse_acls(archivers, request): platform.acl_get(mounted_dir, item_mnt, os.stat(mounted_dir)) assert item_src["acl_access"] == item_mnt["acl_access"] assert item_src["acl_default"] == item_mnt["acl_default"] + item_src, item_mnt = {}, {} + platform.acl_get(dir2_path, item_src, os.stat(dir2_path)) + platform.acl_get(mounted_dir2, item_mnt, os.stat(mounted_dir2)) + assert "acl_access" not in item_src + assert "acl_access" not in item_mnt + assert item_src["acl_default"] == item_mnt["acl_default"] # also check with --numeric-ids: with fuse_mount(archiver, mountpoint, "-a", "archive", "--numeric-ids"): mounted_file = os.path.join(mountpoint, "archive", "input", "file1") diff --git a/src/borg/testsuite/platform/linux_test.py b/src/borg/testsuite/platform/linux_test.py index 6a4a3df3bd..895b777473 100644 --- a/src/borg/testsuite/platform/linux_test.py +++ b/src/borg/testsuite/platform/linux_test.py @@ -82,6 +82,41 @@ def test_default_acl(): assert get_acl(tmpdir)["acl_default"] == DEFAULT_ACL +@skipif_acls_not_working +def test_default_acl_only(): + # a directory can have a default ACL while its access ACL is just the traditional permission + # bits. such an access ACL must not be archived - it has no xattr on the source fs either. + with tempfile.TemporaryDirectory() as tmpdir: + set_acl(tmpdir, default=DEFAULT_ACL) + item = get_acl(tmpdir) + assert "acl_access" not in item + assert item["acl_default"] == DEFAULT_ACL + + +@skipif_acls_not_working +def test_access_acl_only_no_empty_default(): + # a directory without a default ACL must not get an empty acl_default archived. + with tempfile.TemporaryDirectory() as tmpdir: + set_acl(tmpdir, access=ACCESS_ACL) + item = get_acl(tmpdir) + assert item["acl_access"] == ACCESS_ACL + assert "acl_default" not in item + + +def test_acl_is_extended(): + from ...platform import acl_is_extended + + assert not acl_is_extended(None) + assert not acl_is_extended(b"") + # trivial ACLs (only the base entries) are equivalent to the mode bits: + assert not acl_is_extended(b"user::rw-\ngroup::r--\nother::r--") + assert not acl_is_extended(b"user::rw-\ngroup::r--\nother::r--\n") + # anything else is a real ACL: + assert acl_is_extended(b"user::rw-\ngroup::r--\nmask::rw-\nother::r--") + assert acl_is_extended(b"user::rw-\nuser:root:rw-:0\ngroup::r--\nother::r--") + assert acl_is_extended(b"user::rw-\ngroup::r--\ngroup:8888:rw-:8888\nother::r--") + + @skipif_acls_not_working @skipif_no_ubel_user def test_non_ascii_acl(): From 97e41d0da7d021d84f3b8353f85634758f1eba98 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 5 Aug 2026 13:01:24 +0200 Subject: [PATCH 2/2] freebsd: do not lose the default ACL of a directory acl_extended_link_np() only inspects the access (resp. NFSv4) ACL - see _acl_extended_file() in lib/libc/posix1e/acl_extended_file_np.c, which runs acl_is_trivial_np() on ACL_TYPE_ACCESS. So a directory that has only a default ACL does not count as "extended", borg returned early and never archived that default ACL at all - restoring it silently lost the inheritance policy. Now acl_extended_link_np() only gates the access / NFSv4 ACL, and for a directory on a POSIX.1e filesystem (checked via _PC_ACL_EXTENDED, just like acl_set does already) the default ACL is looked at in any case. Also, do not archive empty ACLs: for a directory without a default ACL the kernel returns success and an empty ACL (see ufs_getacl_posix1e()), so borg stored acl_default = b'' for those. --- src/borg/platform/freebsd.pyx | 33 ++++++++++++++++----- src/borg/testsuite/platform/freebsd_test.py | 22 ++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/borg/platform/freebsd.pyx b/src/borg/platform/freebsd.pyx index 83f2a4b285..fd45cd1062 100644 --- a/src/borg/platform/freebsd.pyx +++ b/src/borg/platform/freebsd.pyx @@ -137,7 +137,10 @@ cdef _get_acl(p, type, item, attribute, flags, fd=None): if text == NULL: acl_free(acl) raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p)) - item[attribute] = text + if text[0] != 0: + # an empty ACL (e.g. a directory that has no default ACL) means: there is none. + # do not archive it - it would only be restored as a no-op. + item[attribute] = text acl_free(text) acl_free(acl) @@ -154,21 +157,35 @@ def acl_get(path, item, st, numeric_ids=False, fd=None): flags |= ACL_TEXT_NUMERIC_IDS if numeric_ids else 0 if isinstance(path, str): path = os.fsencode(path) + is_dir = stat.S_ISDIR(st.st_mode) ret = acl_extended_link_np(path) if ret < 0: raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - if ret == 0: + # note: acl_extended_link_np only looks at the access (resp. NFSv4) ACL, so a directory + # that only has a default ACL is not "extended" for it - we must not stop looking then. + extended = ret == 1 + if not extended and not is_dir: # there is no ACL defining permissions other than those defined by the traditional file permission bits. return - ret = lpathconf(path, _PC_ACL_NFS4) - if ret < 0: - raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) - nfs4_acl = ret == 1 + if extended: + ret = lpathconf(path, _PC_ACL_NFS4) + if ret < 0: + raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path)) + nfs4_acl = ret == 1 + else: + # a directory with no extended access ACL: only POSIX.1e filesystems have default ACLs. + # if this filesystem does not do them (or we can not tell), there is nothing to archive. + nfs4_acl = False + if lpathconf(path, _PC_ACL_EXTENDED) != 1: + return if nfs4_acl: + # NFSv4 ACLs have no separate default ACL, inheritance is expressed by entry flags. _get_acl(path, ACL_TYPE_NFS4, item, 'acl_nfs4', flags, fd=fd) else: - _get_acl(path, ACL_TYPE_ACCESS, item, 'acl_access', flags, fd=fd) - if stat.S_ISDIR(st.st_mode): + if extended: + _get_acl(path, ACL_TYPE_ACCESS, item, 'acl_access', flags, fd=fd) + if is_dir: + # only directories can have a default ACL. _get_acl(path, ACL_TYPE_DEFAULT, item, 'acl_default', flags, fd=fd) diff --git a/src/borg/testsuite/platform/freebsd_test.py b/src/borg/testsuite/platform/freebsd_test.py index 11a059daef..f6c716be1e 100644 --- a/src/borg/testsuite/platform/freebsd_test.py +++ b/src/borg/testsuite/platform/freebsd_test.py @@ -93,4 +93,26 @@ def test_default_acl(): assert get_acl(tmpdir)["acl_default"] == DEFAULT_ACL +@skipif_acls_not_working +def test_default_acl_only(): + # a directory can have a default ACL while its access ACL is just the traditional permission + # bits. acl_extended_link_np() only considers the access ACL, so borg must not rely on it + # alone for directories - otherwise the default ACL would not be archived at all. + with tempfile.TemporaryDirectory() as tmpdir: + set_acl(tmpdir, default=DEFAULT_ACL) + item = get_acl(tmpdir) + assert "acl_access" not in item + assert item["acl_default"] == DEFAULT_ACL + + +@skipif_acls_not_working +def test_access_acl_only_no_empty_default(): + # a directory without a default ACL must not get an empty acl_default archived. + with tempfile.TemporaryDirectory() as tmpdir: + set_acl(tmpdir, access=ACCESS_ACL) + item = get_acl(tmpdir) + assert item["acl_access"] == ACCESS_ACL + assert "acl_default" not in item + + # nfs4 acls testing not implemented.