Skip to content

fix: retry cache removal after modify directory permissions #1699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dengbo11 merged 1 commit into from
Jun 25, 2026
+131 −3
Merged

fix: retry cache removal after modify directory permissions #1699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion libs/linglong/src/linglong/package_manager/package_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1992,7 +1992,19 @@ utils::error::Result<void> PackageManager::removeCache(const package::Reference
std::error_code ec;
std::filesystem::remove_all(appCache, ec);
if (ec) {
return LINGLONG_ERR("failed to remove cache directory", ec);
LogD("failed to remove cache directory {}, retry after fixing permissions: {}",
appCache,
ec.message());

auto ret = utils::makeDirectoryTreeRemovable(appCache);
if (!ret) {
return LINGLONG_ERR("failed to make cache directory removable", ret);
}

std::filesystem::remove_all(appCache, ec);
if (ec) {
return LINGLONG_ERR("failed to remove cache directory", ec);
}
}

return LINGLONG_OK;
Expand Down
52 changes: 51 additions & 1 deletion libs/linglong/tests/ll-tests/src/linglong/utils/file_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// SPDX-FileCopyrightText: 2025-2026 UnionTech Software Technology Co., Ltd.
// SPDX-FileCopyrightText: 2025 - 2026 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: LGPL-3.0-or-later

Expand All @@ -10,9 +10,11 @@
#include <algorithm>
#include <filesystem>
#include <fstream>
#include <memory>

Check warning on line 13 in libs/linglong/tests/ll-tests/src/linglong/utils/file_test.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <memory> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <string>

Check warning on line 14 in libs/linglong/tests/ll-tests/src/linglong/utils/file_test.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <string> not found. Please note: Cppcheck does not need standard library headers to get proper results.

#include <unistd.h>

Check warning on line 16 in libs/linglong/tests/ll-tests/src/linglong/utils/file_test.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <unistd.h> not found. Please note: Cppcheck does not need standard library headers to get proper results.

namespace fs = std::filesystem;

class FileTest : public ::testing::Test
Expand Down Expand Up @@ -277,6 +279,54 @@
EXPECT_TRUE(fs::is_directory(multiple_dir));
}

TEST_F(FileTest, MakeDirectoryTreeRemovable)
{
const fs::perms ownerAll =
fs::perms::owner_read | fs::perms::owner_write | fs::perms::owner_exec;

fs::path root = dest_dir / "removable";
fs::path blockedDir = root / "blocked-dir";
fs::path nestedFile = blockedDir / "nested.txt";
fs::path blockedFile = root / "blocked-file.txt";
fs::path writeOnlyFile = root / "write-only.txt";

fs::create_directories(blockedDir);
std::ofstream(nestedFile) << "nested";
std::ofstream(blockedFile) << "blocked";
std::ofstream(writeOnlyFile) << "write";

fs::permissions(root, ownerAll, fs::perm_options::replace);
fs::permissions(blockedDir, fs::perms::none, fs::perm_options::replace);
fs::permissions(blockedFile, fs::perms::none, fs::perm_options::replace);
fs::permissions(writeOnlyFile, fs::perms::owner_write, fs::perm_options::replace);

if (geteuid() == 0) {
GTEST_SKIP() << "root can remove files regardless of the owner permission bits";
}

auto rootPermsBefore = fs::symlink_status(root).permissions();

std::error_code removeBeforeEc;
fs::remove_all(root, removeBeforeEc);
ASSERT_TRUE(removeBeforeEc)
<< "directory tree should not be removable before permissions are fixed";
ASSERT_TRUE(fs::exists(root));

auto result = linglong::utils::makeDirectoryTreeRemovable(root);
ASSERT_TRUE(result.has_value()) << result.error().message();

auto rootPermsAfter = fs::symlink_status(root).permissions();
EXPECT_EQ(rootPermsAfter & ownerAll, rootPermsBefore & ownerAll);

auto blockedDirPerms = fs::symlink_status(blockedDir).permissions();
EXPECT_EQ(blockedDirPerms & ownerAll, ownerAll);

std::error_code ec;
fs::remove_all(root, ec);
EXPECT_FALSE(ec) << ec.message();
EXPECT_FALSE(fs::exists(root));
}

TEST_F(FileTest, RelinkFile)
{
fs::path target = dest_dir / "target_file";
Expand Down
63 changes: 63 additions & 0 deletions libs/utils/src/linglong/utils/file.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@

namespace linglong::utils {

namespace {

linglong::utils::error::Result<void>
ensureOwnerPermissionsIfNeeded(const std::filesystem::path &path,
std::filesystem::perms currentPerms,
std::filesystem::perms requiredPerms) noexcept
{
LINGLONG_TRACE("ensure owner permissions if needed");

if ((currentPerms & requiredPerms) == requiredPerms) {
return LINGLONG_OK;
}

std::error_code ec;
std::filesystem::permissions(path, requiredPerms, std::filesystem::perm_options::add, ec);
if (ec) {
return LINGLONG_ERR(fmt::format("failed to change permissions: {}", path), ec);
}

return LINGLONG_OK;
}

} // namespace

linglong::utils::error::Result<std::string> readFile(const std::filesystem::path &filepath)
{
LINGLONG_TRACE(fmt::format("read file {}", filepath));
Expand Down Expand Up @@ -275,6 +299,45 @@ linglong::utils::error::Result<void> ensureDirectory(const std::filesystem::path
return LINGLONG_OK;
}

linglong::utils::error::Result<void>
makeDirectoryTreeRemovable(const std::filesystem::path &root) noexcept
{
LINGLONG_TRACE("make directory tree removable");

namespace fs = std::filesystem;

std::error_code ec;
fs::recursive_directory_iterator iter{ root,
fs::directory_options::skip_permission_denied,
ec };
if (ec) {
return LINGLONG_ERR(fmt::format("failed to iterate directory: {}", root), ec);
}
Comment on lines +309 to +315

@gemini-code-assist gemini-code-assist Bot Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the root directory itself has restricted permissions (e.g., 000 or lacks owner read/write/execute permissions), constructing the std::filesystem::recursive_directory_iterator will fail immediately with a permission denied error. This prevents the function from fixing the permissions of any subdirectories or the root itself.

To make this robust, we should first check if the root directory exists, and if so, ensure that the root directory itself has owner permissions (fs::perms::owner_all) before attempting to construct the iterator.

    std::error_code ec;
    if (!fs::exists(root, ec)) {
        if (ec) {
            return LINGLONG_ERR(fmt::format("failed to check existence of root: {}", root), ec);
        }
        return LINGLONG_OK;
    }

    auto rootStatus = fs::status(root, ec);
    if (ec) {
        return LINGLONG_ERR(fmt::format("failed to get status of root directory: {}", root), ec);
    }

    if (rootStatus.type() == fs::file_type::directory) {
        auto ret = ensureOwnerPermissionsIfNeeded(root, rootStatus.permissions(), fs::perms::owner_all);
        if (!ret) {
            return ret;
        }
    }

    fs::recursive_directory_iterator iter{ root,
                                           fs::directory_options::skip_permission_denied,
                                           ec };
    if (ec) {
        return LINGLONG_ERR(fmt::format("failed to iterate directory: {}", root), ec);
    }


const fs::recursive_directory_iterator end;
for (; iter != end; iter.increment(ec)) {
if (ec) {
return LINGLONG_ERR(fmt::format("failed to iterate directory: {}", root), ec);
}

auto status = iter->symlink_status(ec);
if (ec) {
return LINGLONG_ERR(fmt::format("failed to get entry status: {}", iter->path()), ec);
}

if (status.type() == fs::file_type::directory) {
auto ret = ensureOwnerPermissionsIfNeeded(iter->path(),
status.permissions(),
fs::perms::owner_all);
if (!ret) {
return ret;
}
}
}

return LINGLONG_OK;
}

linglong::utils::error::Result<void> relinkFileTo(const std::filesystem::path &link,
const std::filesystem::path &target) noexcept
{
Expand Down
5 changes: 4 additions & 1 deletion libs/utils/src/linglong/utils/file.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.
// SPDX-FileCopyrightText: 2024 - 2026 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: LGPL-3.0-or-later

Expand Down Expand Up @@ -38,6 +38,9 @@ getFiles(const std::filesystem::path &dir);

linglong::utils::error::Result<void> ensureDirectory(const std::filesystem::path &dir);

linglong::utils::error::Result<void>
makeDirectoryTreeRemovable(const std::filesystem::path &root) noexcept;

linglong::utils::error::Result<void> relinkFileTo(const std::filesystem::path &link,
const std::filesystem::path &target) noexcept;

Expand Down
Loading