Repository navigation
fix: correct directory ownership when creating cache dirs with sudo - #535
Merged
deepin-bot[bot] merged 1 commit intoSep 30, 2026
Merged
deepin-bot[bot] merged 1 commit into
deepin-bot[bot] merged 1 commit into
Conversation
Reviewer's GuideThe PR fixes sudo-created cache directories by adding a home-path-scoped UID/GID correction step after relevant mkpath calls, adds safe handling for extraction setup failures, and audits other directory creation sites to report failures through warnings. Sequence diagram for sudo cache directory ownership correctionsequenceDiagram
participant App as deepin-log-viewer
participant QDir
participant Utils
participant OS
App->>QDir: mkpath(cacheDirectory)
alt mkpath fails
QDir-->>App: false
App->>App: qCWarning
else directory created
QDir-->>App: true
App->>Utils: fixDirOwnership(cacheDirectory)
Utils->>OS: stat(homePath)
OS-->>Utils: realUID and realGID
alt getuid differs from realUID
Utils->>OS: chown(cacheDirectory, realUID, realGID)
end
end
Sequence diagram for extraction directory creation failuresequenceDiagram
participant Caller as getFileInfo or getOtherFileInfo
participant Access as UserLogAccess
participant QDir
participant Unzip
Caller->>Access: createExtractSubdir()
Access->>QDir: mkpath(extractSubdir)
alt mkpath fails
QDir-->>Access: false
Access-->>Caller: empty string
Caller->>Caller: skip unzip
Caller-->>Caller: retain original file path
else directory created
QDir-->>Access: true
Access-->>Caller: extractSubdir
Caller->>Unzip: unzipToTempFile(file, template)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="application/utils.cpp" line_range="375" />
<code_context>
+ }
+ uid_t realUID = st.st_uid;
+ if (getuid() != realUID) {
+ if (chown(dirPath.toUtf8().constData(), realUID, st.st_gid) != 0) {
+ qCWarning(logApp) << "fixDirOwnership: failed to chown" << dirPath << "to uid:" << realUID << "gid:" << st.st_gid << "error:" << strerror(errno);
+ }
</code_context>
<issue_to_address>
**🚨 issue (security):** `chown(dirPath, ...)` follows symlinks, so a user-controlled symlink anywhere below `Utils::homePath` can redirect the privileged sudo process to chown an arbitrary target outside the home directory. For example, a pre-created `~/.cache/deepin/deepin-log-viewer/user-log` symlink to `/etc` is accepted by the lexical prefix check and causes `/etc` itself to be reassigned to the logged-in user.
**Triggers:** When the cache or extraction path is a symlink created or replaced by the unprivileged logged-in user before the sudo process calls `fixDirOwnership()`.
**Suggested fix:** Reject symlinks and use no-follow, directory-relative ownership operations (such as `openat`/`fstatat` with `O_NOFOLLOW`) before changing ownership.
</issue_to_address>
wangrong1069
force-pushed
the
agent/pms-bug-bot/53bc45b51c92
branch
from
September 29, 2026 12:10
6306471 to
f38094e
Compare
When running `sudo deepin-log-viewer --reportcoredump`, directories created under ~/.cache/deepin/deepin-log-viewer/ are owned by root, causing subsequent normal launches to fail writing to them. Add Utils::fixDirOwnership() to chown newly created directories to the real logged-in user (detected via lstat(homePath)) when getuid() differs from the real user UID. Apply it to all mkpath call sites that create cache directories under the user's home: updateRepeatCoredumpExePaths, UserLogAccess constructor, and UserLogAccess::createExtractSubdir. Harden fixDirOwnership() against symlink attacks: walk the target path from root using openat(O_DIRECTORY | O_NOFOLLOW) at each level, then fchown() the resulting fd. This prevents a non-privileged user from pre-creating a symlink along the path to trick the privileged sudo process into chowning arbitrary targets outside the home directory. Add error handling for createExtractSubdir() returning empty string (mkpath failure) at all three call sites: skip unzip and fall back to returning original file paths directly, preventing silent file loss. Also add qCWarning logging for unchecked mkpath return values across the project. PMS: TASK-395141
wangrong1069
force-pushed
the
agent/pms-bug-bot/53bc45b51c92
branch
from
September 29, 2026 13:13
f38094e to
2d704d2
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰。fixDirOwnership 函数使用 openat 逐级遍历路径组件，文件描述符管理无泄漏：openat 失败时 close(fd) 后返回，成功时 close 旧 fd 并使用新 fd，循环结束后 fchown 再 close。createExtractSubdir 在 mkpath 失败时正确返回空字符串，getFileInfo/getOtherFileInfo 中通过 else 分支正确返回原始文件路径，与 PR 描述的'跳过 unzip 并直接返回原始文件路径'一致。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码结构清晰，注释完整。fixDirOwnership 函数有详细的安全相关注释，解释了 lstat 不跟随符号链接、openat(O_NOFOLLOW) 防止 symlink 攻击的设计意图。header 文件中有简洁的方法说明。无重复代码，fixDirOwnership 集中在 Utils 类中。无残留调试代码，qCWarning 日志为合理的错误处理日志。opslogexport.cpp 中使用 qWarning() 与该文件现有风格一致。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，资源使用合理。fixDirOwnership 仅在目录创建时调用，不在热路径上。openat 逐级遍历虽然需要多次系统调用，但路径深度有限（通常 5-6 级），且仅在 sudo 场景下触发。lstat 和 getuid 各调用一次，fchown 操作文件描述符而非路径，效率较高。文件描述符及时关闭，无资源泄漏。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 安全合规。fixDirOwnership 函数展现了良好的安全实践：1) 路径验证：仅处理 homePath 下的路径，防止误改系统目录；2) Symlink 攻击防护：使用 openat(O_DIRECTORY|O_NOFOLLOW) 逐级打开路径组件，任一组件为符号链接则拒绝继续；3) TOCTOU 防护：使用 fchown(fd) 而非 chown(path)，操作已打开的文件描述符而非路径；4) 权限安全：仅将属主改为真实登录用户 UID/GID，通过 lstat(homePath) 获取，不允许任意 UID 变更；5) 无命令注入风险：无 system()/exec() 调用；6) 无信息泄露：qCWarning 日志仅输出路径和错误信息，无敏感数据。 💡 改进建议代码示例// 本次提交代码质量优秀，无需修复示例。
// fixDirOwnership 已采用安全的 openat(O_NOFOLLOW) 模式实现，
// 是防范 symlink 攻击的最佳实践写法。本报告由 AI 代码审查工具自动生成 |
lzwind
approved these changes
Sep 30, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, wangrong1069 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
修复：sudo 执行时缓存目录属主为 root 的问题
问题
当
~/.cache/deepin/deepin-log-viewer/不存在且有未汇报的 coredump 时，执行sudo deepin-log-viewer --reportcoredump会创建该目录，但属主属组都是 root，导致后续正常启动的 deepin-log-viewer 无法写入。根因
sudo deepin-log-viewer --reportcoredump以 root 身份运行，QDir().mkpath()创建的目录属主为 root。Utils::homePath虽然能正确解析真实登录用户家目录，但未处理目录创建后的属主问题。修复内容
Utils新增fixDirOwnership()静态方法：通过stat(homePath)获取真实登录用户 UID/GID，当getuid() != 真实登录用户UID时 chown 新建目录。~/.cache/deepin/deepin-log-viewer/子目录的mkpath调用后添加fixDirOwnership()：Utils::updateRepeatCoredumpExePaths()UserLogAccess构造函数UserLogAccess::createExtractSubdir()createExtractSubdir()返回空字符串（mkpath 失败）的情况添加错误处理，跳过 unzip 并直接返回原始文件路径。qCWarning日志。PMS: TASK-395141
Summary by Sourcery
Correct ownership of user cache directories created with elevated privileges and improve failure handling for directory creation.
Bug Fixes:
Enhancements: