fix: Modify cache directory permissions in two steps (#3844)

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Co-authored-by: Klaim <Klaim@users.noreply.github.com>
This commit is contained in:
Julien Jerphanion 2025-03-07 14:31:48 +01:00 committed by GitHub
parent 1e66fabb8f
commit 26785a6b50
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 78 additions and 3 deletions

View File

@ -954,9 +954,34 @@ namespace mamba
{
const auto cache_dir = cache_path / "cache";
fs::create_directories(cache_dir);
const auto new_permissions = fs::perms::set_gid | fs::perms::owner_all | fs::perms::group_all
| fs::perms::others_read | fs::perms::others_exec;
fs::permissions(cache_dir.string().c_str(), new_permissions, fs::perm_options::replace);
// 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.
//
// We want to set the setgid bit on the cache directory to preserve the permissions as much
// as possible if we can; hence we proceed in two steps to set the permissions by
// 1. Setting the permissions without the setgid bit to the desired value without.
// 2. Trying to set the setgid bit on the directory and report success or failure in log
// without raising an error or propagating an error which was raised.
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)
{
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();
}
} // namespace mamba

View File

@ -8,9 +8,11 @@
#include <catch2/catch_all.hpp>
#include "mamba/core/subdirdata.hpp"
#include "mamba/core/util.hpp"
#include "mamba/core/util_scope.hpp"
#include "mamba/fs/filesystem.hpp"
#include "mamba/util/build.hpp"
namespace mamba
{
@ -349,6 +351,54 @@ namespace mamba
fs::remove_all(tmp_dir);
REQUIRE_FALSE(fs::exists(tmp_dir));
}
TEST_CASE("create_cache_dir")
{
// `create_cache_dir` create a `cache` subdirectory at a given path given as an
// argument.
const auto cache_path = fs::temp_directory_path() / "mamba-fs-cache-path";
const auto cache_dir = cache_path / "cache";
mamba::on_scope_exit _([&] { fs::remove_all(cache_path); });
// Get the information about whether the filesystem supports the `set_gid` bit.
bool supports_setgid_bit = false;
fs::create_directories(cache_path);
std::error_code ec;
fs::permissions(cache_path, fs::perms::set_gid, fs::perm_options::add, ec);
if (!ec)
{
supports_setgid_bit = (fs::status(cache_path).permissions() & fs::perms::set_gid)
== fs::perms::set_gid;
}
// Check that `cache_dir` does not exist before calling `create_cache_dir`
REQUIRE_FALSE(fs::exists(cache_dir));
create_cache_dir(cache_path);
REQUIRE(fs::exists(cache_dir));
REQUIRE(fs::is_directory(cache_dir));
// Check that the permissions of `cache_dir` are _at least_ `rwxr-xr-x` because
// `std::fs::temp_directory_path` might not have `rwxrwxr-x` permissions.
auto cache_dir_permissions = fs::status(cache_dir).permissions();
auto expected_min_owner_perm = fs::perms::owner_all;
auto expected_min_group_perm = fs::perms::group_read | fs::perms::group_exec;
auto expected_min_others_perm = fs::perms::others_read | fs::perms::others_exec;
REQUIRE((cache_dir_permissions & expected_min_owner_perm) == expected_min_owner_perm);
REQUIRE((cache_dir_permissions & expected_min_group_perm) == expected_min_group_perm);
REQUIRE((cache_dir_permissions & expected_min_others_perm) == expected_min_others_perm);
if (supports_setgid_bit)
{
REQUIRE((cache_dir_permissions & fs::perms::set_gid) == fs::perms::set_gid);
}
}
}
}