From 3f07c65510d5f907c75803e7d58884bdafb78132 Mon Sep 17 00:00:00 2001
From: Alois Wohlschlager <alois1@gmx-topmail.de>
Date: Tue, 10 Sep 2024 18:05:32 +0200
Subject: [PATCH] local-store: make extended attribute handling more robust

* Move the extended attribute deletion after the hardlink sanity check. We
  shouldn't be removing extended attributes on random files.
* Make the entity owner-writable before attempting to remove extended
  attributes, since this operation usually requires write access on the file,
  and we shouldn't fail xattr deletion on a file that has been made unwritable
  by the builder or a previous canonicalisation pass.

Fixes: https://git.lix.systems/lix-project/lix/issues/507
Change-Id: I7e6ccb71649185764cd5210f4a4794ee174afea6
---
 src/libstore/local-store.cc | 44 +++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 49991e38a..2da8dd2a6 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -664,27 +664,6 @@ static void canonicalisePathMetaData_(
     if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode)))
         throw Error("file '%1%' has an unsupported type", path);
 
-#if __linux__
-    /* Remove extended attributes / ACLs. */
-    ssize_t eaSize = llistxattr(path.c_str(), nullptr, 0);
-
-    if (eaSize < 0) {
-        if (errno != ENOTSUP && errno != ENODATA)
-            throw SysError("querying extended attributes of '%s'", path);
-    } else if (eaSize > 0) {
-        std::vector<char> eaBuf(eaSize);
-
-        if ((eaSize = llistxattr(path.c_str(), eaBuf.data(), eaBuf.size())) < 0)
-            throw SysError("querying extended attributes of '%s'", path);
-
-        for (auto & eaName: tokenizeString<Strings>(std::string(eaBuf.data(), eaSize), std::string("\000", 1))) {
-            if (settings.ignoredAcls.get().count(eaName)) continue;
-            if (lremovexattr(path.c_str(), eaName.c_str()) == -1)
-                throw SysError("removing extended attribute '%s' from '%s'", eaName, path);
-        }
-     }
-#endif
-
     /* Fail if the file is not owned by the build user.  This prevents
        us from messing up the ownership/permissions of files
        hard-linked into the output (e.g. "ln /etc/shadow $out/foo").
@@ -699,6 +678,29 @@ static void canonicalisePathMetaData_(
         return;
     }
 
+#if __linux__
+    /* Remove extended attributes / ACLs. */
+    ssize_t eaSize = llistxattr(path.c_str(), nullptr, 0);
+
+    if (eaSize < 0) {
+        if (errno != ENOTSUP && errno != ENODATA)
+            throw SysError("querying extended attributes of '%s'", path);
+    } else if (eaSize > 0) {
+        std::vector<char> eaBuf(eaSize);
+
+        if ((eaSize = llistxattr(path.c_str(), eaBuf.data(), eaBuf.size())) < 0)
+            throw SysError("querying extended attributes of '%s'", path);
+
+        if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode))
+            chmod(path.c_str(), st.st_mode | S_IWUSR);
+        for (auto & eaName: tokenizeString<Strings>(std::string(eaBuf.data(), eaSize), std::string("\000", 1))) {
+            if (settings.ignoredAcls.get().count(eaName)) continue;
+            if (lremovexattr(path.c_str(), eaName.c_str()) == -1)
+                throw SysError("removing extended attribute '%s' from '%s'", eaName, path);
+        }
+     }
+#endif
+
     inodesSeen.insert(Inode(st.st_dev, st.st_ino));
 
     canonicaliseTimestampAndPermissions(path, st);