chore: remove root user restriction in RepoCache rebuild

[dengbo11]

1. Removed the check that prevented root users from writing to states.json during cache rebuild
2. The previous workaround was needed when ll-cli could make states.json owned by root, causing permission issues
3. This restriction is no longer necessary as the underlying ownership issue has been addressed
4. Root users can now fully rebuild the repository cache without skipping the write operation

Log: Root users can now rebuild the repository cache and persist changes to states.json

Influence:

1. Test RepoCache rebuild functionality when running as root user
2. Verify states.json is correctly written and readable by both root and non-root users
3. Test cache rebuild in scenarios where ll-cli initializes the repository as root
4. Verify file ownership and permissions of states.json after root- initiated rebuilds
5. Test cache persistence across multiple rebuild operations as root
6. Verify no permission denied errors occur during cache writes as root

[deepin-ci-robot]

deepin pr auto review

这段代码的修改是移除了一段检查当前用户是否为 root 的逻辑。以下是对这段修改的详细审查意见，包括语法逻辑、代码质量、性能和安全性方面的分析：

1. 语法逻辑分析

• 修改内容：移除了 if (getuid() == 0) 的判断块及其内部的 return LINGLONG_OK;。
• 逻辑影响：
  • 修改前：如果当前用户是 root（UID 0），函数会直接返回成功，跳过后续的 writeToDisk() 操作。这是为了防止 root 用户创建的 states.json 文件导致普通用户后续无法写入。
  • 修改后：无论当前用户是否为 root，都会执行 writeToDisk()。
• 潜在问题：
  • 如果 root 用户执行了 rebuild，writeToDisk() 会以 root 权限创建或覆盖 states.json。这可能导致文件权限设置为 root:root，普通用户后续可能无法修改该文件，从而引发权限问题。

2. 代码质量分析

• 注释的移除：原代码中的 FIXME 注释被一并移除。虽然问题可能通过其他方式解决，但直接移除注释而不说明原因可能会降低代码的可维护性。
• 错误处理：修改后的代码依赖 writeToDisk() 的返回值来判断是否成功。如果 writeToDisk() 内部没有处理权限问题，可能会隐藏潜在的权限错误。

3. 性能分析

• 性能影响：移除 if 判断后，代码逻辑更直接，减少了分支预测的开销，但影响微乎其微。
• I/O 操作：writeToDisk() 是主要的 I/O 操作，修改前后均会执行（除非是 root 用户），因此性能差异不大。

4. 安全性分析

• 权限问题：
  • 修改前：通过跳过 writeToDisk() 避免了 root 用户创建的文件权限问题，但可能导致缓存数据不一致（因为 root 用户修改了缓存但未写入磁盘）。
  • 修改后：强制写入磁盘，但可能导致文件权限问题。如果 writeToDisk() 没有显式设置文件权限（如 0644），普通用户后续可能无法修改 states.json。
• 建议：
  • 在 writeToDisk() 中显式设置文件权限，确保普通用户可以读写。
  • 或者保留 if (getuid() == 0) 判断，但改为调整文件权限而非直接跳过写入。

5. 改进建议

1. 保留权限检查但优化逻辑：

   if (getuid() == 0) {
       LogW("Rebuild the cache by root, adjusting file permissions for states.json");
       // 显式设置文件权限，例如 0644
       chmod(this->statesFilePath.c_str(), 0644);
   }
   auto ret = writeToDisk();
   if (!ret) {
       return LINGLONG_ERR(ret);
   }

2. 在 writeToDisk() 中统一处理权限：

   • 确保 writeToDisk() 在创建或覆盖文件时设置正确的权限（如 0644），无论当前用户是否为 root。

3. 添加文档说明：

   • 如果移除 FIXME 注释是因为问题已解决，建议在代码或提交信息中说明解决方案，以提高可维护性。

总结

• 优点：移除 if 判断后代码逻辑更直接，避免了 root 用户可能导致的缓存不一致问题。
• 缺点：可能引入文件权限问题，尤其是当 root 用户执行 rebuild 时。
• 推荐：在 writeToDisk() 中显式设置文件权限，或保留权限检查但改为调整权限而非跳过写入。

[gemini-code-assist]

Code Review

This pull request removes a restriction that prevented the root user from updating the repository cache on disk. Reviewers identified a critical security vulnerability where root processes using fixed temporary filenames could be targeted by symlink attacks in user-writable directories. Additionally, concerns were raised regarding the noexcept specification of the rebuild function and the potential for state inconsistency if disk writes fail after the in-memory cache has been modified.

[gemini-code-assist]

Removing the root user check allows root processes to execute writeToDisk(). In the current implementation of writeToDisk(), a temporary file with a fixed name (temp-states.json) is created using std::ofstream. If this operation is performed in a directory writable by a non-privileged user (e.g., a user-local repository), it is vulnerable to a symlink attack. A malicious user could create a symlink at the temporary file's path to trick the root process into overwriting arbitrary system files. Consider using a unique temporary filename (e.g., via mkstemp) and opening it with O_EXCL, or ensuring that the repository directory permissions are strictly controlled to prevent non-root access.

[gemini-code-assist]

The rebuild function is marked as noexcept (at line 57), but it calls writeToDisk(), which is not noexcept and performs operations that can throw exceptions (e.g., std::filesystem path operations, std::string allocations, and nlohmann::json::dump()). If an exception is thrown, the program will call std::terminate().

Additionally, if writeToDisk() fails and returns an error, the in-memory state of this->cache has already been modified (cleared and repopulated), leading to an inconsistency with the persistent state on disk. Consider removing the noexcept specifier and implementing a transactional approach where the cache is rebuilt into a temporary object and only swapped into this->cache upon a successful write.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
libs/linglong/src/linglong/repo/repo_cache.cpp	23.97% <ø> (+0.24%)	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment