diff --git a/libmamba/include/mamba/core/util.hpp b/libmamba/include/mamba/core/util.hpp index 6bc424ac4c..eb7a9c1f8c 100644 --- a/libmamba/include/mamba/core/util.hpp +++ b/libmamba/include/mamba/core/util.hpp @@ -62,10 +62,12 @@ namespace mamba inline void make_executable(const fs::u8path& p) { - fs::permissions( - p, - fs::perms::owner_all | fs::perms::group_all | fs::perms::others_read | fs::perms::others_exec - ); + const auto permissions = fs::perms::owner_all | fs::perms::group_all + | fs::perms::others_read | fs::perms::others_exec; + if (!fs::has_permissions(p, permissions)) + { + fs::permissions(p, permissions); + } } // @return `true` if `TemporaryFile` will not delete files once destroy. diff --git a/libmamba/include/mamba/fs/filesystem.hpp b/libmamba/include/mamba/fs/filesystem.hpp index c1fbbd0292..7396409167 100644 --- a/libmamba/include/mamba/fs/filesystem.hpp +++ b/libmamba/include/mamba/fs/filesystem.hpp @@ -1139,6 +1139,14 @@ namespace mamba::fs return std::filesystem::last_write_time(path, new_time, std::forward(args)...); } + /* Check if we have modification rights on a path. + * This function is not a wrapping on a of std::filesystem function, but it + * uses std::filesystem::status(). + * Exceptions may be thrown by std::filesystem::status() if the path is not + * valid. + */ + bool has_permissions(const u8path&, const fs::perms&); + // void permissions(const path& p, perms prms, perm_options opts = perm_options::replace); // void permissions(const path& p, perms prms, error_code& ec) noexcept; // void permissions(const path& p, perms prms, perm_options opts, error_code& ec); diff --git a/libmamba/src/core/subdir_index.cpp b/libmamba/src/core/subdir_index.cpp index 70d80a3a2d..a4f89f9a9d 100644 --- a/libmamba/src/core/subdir_index.cpp +++ b/libmamba/src/core/subdir_index.cpp @@ -1122,7 +1122,10 @@ namespace mamba auto create_cache_dir(const fs::u8path& cache_path) -> std::string { const auto cache_dir = cache_path / "cache"; - fs::create_directories(cache_dir); + if (!std::filesystem::is_directory(cache_dir)) + { + fs::create_directories(cache_dir); + } // Some filesystems don't support special permissions such as setgid on directories (e.g. // NFS). and fail if we try to set the setgid bit on the cache directory. @@ -1135,20 +1138,27 @@ namespace mamba const auto permissions = fs::perms::owner_all | fs::perms::group_all | fs::perms::others_read | fs::perms::others_exec; - fs::permissions(cache_dir, permissions, fs::perm_options::replace); - LOG_TRACE << "Set permissions on cache directory " << cache_dir << " to 'rwxrwxr-x'"; - - std::error_code ec; - fs::permissions(cache_dir, fs::perms::set_gid, fs::perm_options::add, ec); - - if (!ec) + if (!fs::has_permissions(cache_dir, permissions)) { - LOG_TRACE << "Set setgid bit on cache directory " << cache_dir; + fs::permissions(cache_dir, permissions, fs::perm_options::replace); + LOG_TRACE << "Set permissions on cache directory " << cache_dir << " to 'rwxrwxr-x'"; } - else + + const auto setgid_perms = fs::perms::set_gid; + if (!fs::has_permissions(cache_dir, setgid_perms)) { - LOG_TRACE << "Could not set setgid bit on cache directory " << cache_dir - << "\nReason:" << ec.message() << "; ignoring and continuing"; + std::error_code ec; + fs::permissions(cache_dir, setgid_perms, fs::perm_options::add, ec); + + if (!ec) + { + LOG_TRACE << "Set setgid bit on cache directory " << cache_dir; + } + else + { + LOG_TRACE << "Could not set setgid bit on cache directory " << cache_dir + << "\nReason:" << ec.message() << "; ignoring and continuing"; + } } return cache_dir.string(); diff --git a/libmamba/src/fs/filesystem.cpp b/libmamba/src/fs/filesystem.cpp index 40491bdb68..151354b02d 100644 --- a/libmamba/src/fs/filesystem.cpp +++ b/libmamba/src/fs/filesystem.cpp @@ -12,7 +12,9 @@ #ifndef _WIN32 #include +#include #include +#include // We can use the presence of UTIME_OMIT to detect platforms that provide // utimensat. #if defined(UTIME_OMIT) @@ -79,4 +81,13 @@ namespace mamba::fs std::filesystem::last_write_time(path, new_time, ec); #endif } + + bool has_permissions(const u8path& path, const fs::perms& perm) + { + // Get path permissions + auto p = std::filesystem::status(path).permissions(); + + // Path perms must include wanted perms + return perm == (p & perm); + } } diff --git a/libmamba/tests/src/core/test_filesystem.cpp b/libmamba/tests/src/core/test_filesystem.cpp index 9ba979b0e0..a84632bf56 100644 --- a/libmamba/tests/src/core/test_filesystem.cpp +++ b/libmamba/tests/src/core/test_filesystem.cpp @@ -452,4 +452,117 @@ namespace mamba } + namespace + { + TEST_CASE("has_permissions()") + { + // Create temp folder + const auto tmp_dir = fs::temp_directory_path() / "mamba-fs-has_permissions"; + mamba::on_scope_exit _([&] { fs::remove_all(tmp_dir); }); + fs::create_directories(tmp_dir); + + // Create file + const auto some_file = tmp_dir / "some_file"; + { + std::ofstream ofs{ some_file.std_path(), + std::ofstream::binary | std::ofstream::trunc }; + ofs << "ABC" << std::endl; + } + + // Set permissions + const auto perms = fs::perms::owner_read | fs::perms::owner_write | fs::perms::group_read; + fs::permissions(some_file, perms, fs::perm_options::replace); + + // Check permissions + REQUIRE(fs::has_permissions(some_file, perms)); + REQUIRE(fs::has_permissions(some_file, fs::perms::owner_read)); + REQUIRE(fs::has_permissions(some_file, fs::perms::owner_write)); + REQUIRE(fs::has_permissions(some_file, fs::perms::group_read)); + REQUIRE(fs::has_permissions(some_file, fs::perms::owner_read | fs::perms::owner_write)); + REQUIRE(fs::has_permissions(some_file, fs::perms::owner_read | fs::perms::group_read)); + REQUIRE(fs::has_permissions(some_file, fs::perms::owner_write | fs::perms::group_read)); + REQUIRE_FALSE(fs::has_permissions(some_file, fs::perms::owner_exec)); + REQUIRE_FALSE(fs::has_permissions(some_file, fs::perms::group_write)); + REQUIRE_FALSE(fs::has_permissions(some_file, fs::perms::group_exec)); + REQUIRE_FALSE(fs::has_permissions(some_file, fs::perms::others_read)); + REQUIRE_FALSE(fs::has_permissions(some_file, fs::perms::others_write)); + REQUIRE_FALSE(fs::has_permissions(some_file, fs::perms::others_exec)); + REQUIRE_FALSE( + fs::has_permissions(some_file, fs::perms::owner_read | fs::perms::owner_exec) + ); + } + + // 2025-11-27 make_executable() bug + TEST_CASE( + "Bug: failure when calling make_executable()" + " on already executable file inside non-writable folder." + ) + { + // Create temp folder + const auto tmp_dir = fs::temp_directory_path() + / "mamba-fs-make_executable-2025-11-27-bug"; + mamba::on_scope_exit _([&] { fs::remove_all(tmp_dir); }); + const auto folder = tmp_dir / "some_folder"; + fs::create_directories(folder); + + // Create file + const auto some_file = tmp_dir / "some_file"; + { + std::ofstream ofs{ some_file.std_path(), + std::ofstream::binary | std::ofstream::trunc }; + ofs << "ABC" << std::endl; + } + + // Make executable + make_executable(some_file); + + // Remove write permissions on parent folder + const auto perms = fs::perms::owner_read | fs::perms::owner_exec | fs::perms::group_read + | fs::perms::group_exec | fs::perms::others_read + | fs::perms::others_exec; + fs::permissions(folder, perms, fs::perm_options::replace); + + // Make executable (again!) + // This should not fail + make_executable(some_file); + + // Reset writing permission + const auto write_perms = fs::perms::owner_all; + fs::permissions(folder, write_perms, fs::perm_options::replace); + } + + // 2025-11-27 create_cache_dir() bug + TEST_CASE( + "Bug: failure when calling create_cache_dir()" + " on already existing cache directory inside" + " non-writable folder." + ) + { + // Create temp folder + const auto tmp_dir = fs::temp_directory_path() + / "mamba-fs-create_cache_dir-2025-11-27-bug"; + mamba::on_scope_exit _([&] { fs::remove_all(tmp_dir); }); + const auto folder = tmp_dir / "some_folder"; + fs::create_directories(folder); + + // Create cache folder + auto cache_dir = folder / "cache_dir"; + create_cache_dir(cache_dir); + + // Remove write permissions on parent folder + const auto perms = fs::perms::owner_read | fs::perms::owner_exec | fs::perms::group_read + | fs::perms::group_exec | fs::perms::others_read + | fs::perms::others_exec; + fs::permissions(folder, perms, fs::perm_options::replace); + + // Create cache folder (again!) + // This should not fail + create_cache_dir(cache_dir); + + // Reset writing permission + const auto write_perms = fs::perms::owner_all; + fs::permissions(folder, write_perms, fs::perm_options::replace); + } + } + }