diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 67d5a1dcb..cf12219ef 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -136,13 +136,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) - throw SysError("could not set permissions on '%s' to 755", perUserDir); + chmodIfNeeded(perUserDir, 0755, S_IRWXU | S_IRWXG | S_IRWXO); } } diff --git a/src/libutil-tests/file-system.cc b/src/libutil-tests/file-system.cc index 2c10d4869..3a288c110 100644 --- a/src/libutil-tests/file-system.cc +++ b/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{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); +} + } diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 6fe93b63a..13f21689d 100644 --- a/src/libutil/file-system.cc +++ b/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 diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index a94603eaf..08ddbccb9 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -366,4 +366,20 @@ typedef std::function 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); + }