Merge pull request #12586 from xokdvium/refactor/chmod-if-needed

{libutil,libstore}: Factor out chmodIfNeeded

src/libstore/local-store.cc

@@ -135,13 +135,10 @@ LocalStore::LocalStore(
     for (auto & perUserDir : {profilesDir + "/per-user", gcRootsDir + "/per-user"}) {
         createDirs(perUserDir);
         if (!readOnly) {
-            auto st = lstat(perUserDir);
-
             // Skip chmod call if the directory already has the correct permissions (0755).
             // This is to avoid failing when the executing user lacks permissions to change the directory's permissions
             // even if it would be no-op.
-            if ((st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) != 0755 && chmod(perUserDir.c_str(), 0755) == -1)
+            chmodIfNeeded(perUserDir, 0755, S_IRWXU | S_IRWXG | S_IRWXO);
-                throw SysError("could not set permissions on '%s' to 755", perUserDir);
         }
     }
 

src/libutil-tests/file-system.cc

@@ -275,4 +275,30 @@ TEST(makeParentCanonical, root)
 {
     ASSERT_EQ(makeParentCanonical("/"), "/");
 }
+
+/* ----------------------------------------------------------------------------
+ * chmodIfNeeded
+ * --------------------------------------------------------------------------*/
+
+TEST(chmodIfNeeded, works)
+{
+    auto [autoClose_, tmpFile] = nix::createTempFile();
+    auto deleteTmpFile = AutoDelete(tmpFile);
+
+    auto modes = std::vector<mode_t>{0755, 0644, 0422, 0600, 0777};
+    for (mode_t oldMode : modes) {
+        for (mode_t newMode : modes) {
+            chmod(tmpFile.c_str(), oldMode);
+            bool permissionsChanged = false;
+            ASSERT_NO_THROW(permissionsChanged = chmodIfNeeded(tmpFile, newMode));
+            ASSERT_EQ(permissionsChanged, oldMode != newMode);
+        }
+    }
+}
+
+TEST(chmodIfNeeded, nonexistent)
+{
+    ASSERT_THROW(chmodIfNeeded("/schnitzel/darmstadt/pommes", 0755), SysError);
+}
+
 }

src/libutil/file-system.cc

@@ -776,4 +776,18 @@ std::filesystem::path makeParentCanonical(const std::filesystem::path & rawPath)
     }
 }
 
+bool chmodIfNeeded(const fs::path & path, mode_t mode, mode_t mask)
+{
+    auto pathString = path.string();
+    auto prevMode = lstat(pathString).st_mode;
+
+    if (((prevMode ^ mode) & mask) == 0)
+        return false;
+
+    if (chmod(pathString.c_str(), mode) != 0)
+        throw SysError("could not set permissions on '%s' to %o", pathString, mode);
+
+    return true;
+}
+
 } // namespace nix

src/libutil/file-system.hh

@@ -2,7 +2,7 @@
 /**
  * @file
  *
- * Utiltities for working with the file sytem and file paths.
+ * Utilities for working with the file system and file paths.
  */
 
 #include "types.hh"
@@ -366,4 +366,20 @@ typedef std::function<bool(const Path & path)> PathFilter;
 
 extern PathFilter defaultPathFilter;
 
+/**
+ * Change permissions of a file only if necessary.
+ *
+ * @details
+ * Skip chmod call if the directory already has the requested permissions.
+ * This is to avoid failing when the executing user lacks permissions to change the
+ * directory's permissions even if it would be no-op.
+ *
+ * @param path Path to the file to change the permissions for.
+ * @param mode New file mode.
+ * @param mask Used for checking if the file already has requested permissions.
+ *
+ * @return true if permissions changed, false otherwise.
+ */
+bool chmodIfNeeded(const std::filesystem::path & path, mode_t mode, mode_t mask = S_IRWXU | S_IRWXG | S_IRWXO);
+
 }