fix: keep ops log zip archive inside temp dir to avoid /var/log exposure - #484
Conversation
Previously exportOpsLog created the zip archive as a separate QTemporaryFile directly under /var/log, briefly exposing system logs to other users before cleanup. Now logs are collected into a log-ops subdirectory inside the random temp dir (opsDir) and the zip is written there too, so neither the archive nor its content ever appear under /var/log. 此前 exportOpsLog 将压缩包作为独立的 QTemporaryFile 直接创建在 /var/log 下,在清理前会短暂暴露系统日志给其他用户。现在日志收集到 随机临时目录(opsDir)内的 log-ops 子目录中,压缩包也写入该目录, 压缩包与日志内容均不再暴露在 /var/log 下。 Log: exportOpsLog 压缩包改至临时目录内,避免 /var/log 暴露 Influence: 仅影响 exportOpsLog 日志导出流程,提升安全性,不影响其他功能
|
Warning
详情 {
"export": {
"logViewerService/logviewerservice.cpp": {
"a": [
" qCWarning(logService) << \"exportOpsLog: failed to create temp zip file:\" << tmpZipFile.errorString();"
],
"b": [
" qCWarning(logService) << \"exportOpsLog: failed to create log collect dir:\" << logCollectDir;"
]
}
}
} |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR changes exportOpsLog so that both the collected ops logs and the generated zip archive are kept entirely inside a per-call temporary directory instead of briefly creating the zip under /var/log, tightening security and simplifying the zip creation flow. Sequence diagram for updated exportOpsLog temporary directory handlingsequenceDiagram
participant LogViewerService
participant QTemporaryDir
participant QDir
participant OpsLogExport
participant QProcess
LogViewerService->>QTemporaryDir: QTemporaryDir tmpOpsDir
LogViewerService->>QTemporaryDir: tmpOpsDir.path()
QTemporaryDir-->>LogViewerService: opsDir
LogViewerService->>QDir: mkpath(logCollectDir)
alt mkpath fails
LogViewerService->>LogViewerService: removeOpsTempDirByPathInternal(opsDir)
LogViewerService-->>LogViewerService: return false
else mkpath succeeds
LogViewerService->>OpsLogExport: OpsLogExport(logCollectDir.toStdString())
LogViewerService->>OpsLogExport: run()
LogViewerService->>QProcess: setWorkingDirectory(logCollectDir)
LogViewerService->>QProcess: start("zip", ["-r", tmpZipPath, "."])
LogViewerService->>QProcess: waitForFinished(-1)
alt zipProc.exitCode() != 0
LogViewerService-->>LogViewerService: return false
else zip succeeds
LogViewerService-->>LogViewerService: write zip to fd and cleanup
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider checking the return value or providing error reporting for
OpsLogExport::run()so that failures in log collection are surfaced before attempting compression. - Using
QDir opsDirDir(opsDir); const QString logCollectDir = opsDirDir.filePath("log-ops");(and similarly forlog-ops.zip) would avoid manual string concatenation and make the directory handling more robust. - It may be safer to use a bounded timeout in
waitForFinished()or at least log when the call times out to prevent a potential indefinite hang ifzipmisbehaves.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider checking the return value or providing error reporting for `OpsLogExport::run()` so that failures in log collection are surfaced before attempting compression.
- Using `QDir opsDirDir(opsDir); const QString logCollectDir = opsDirDir.filePath("log-ops");` (and similarly for `log-ops.zip`) would avoid manual string concatenation and make the directory handling more robust.
- It may be safer to use a bounded timeout in `waitForFinished()` or at least log when the call times out to prevent a potential indefinite hang if `zip` misbehaves.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review★ 总体评分:95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 diff --git a/logViewerService/logviewerservice.cpp b/logViewerService/logviewerservice.cpp
index c9efa7e8..12345678 100644
--- a/logViewerService/logviewerservice.cpp
+++ b/logViewerService/logviewerservice.cpp
@@ -1087,6 +1087,12 @@ bool LogViewerService::exportOpsLog(const QDBusUnixFileDescriptor &fd)
OpsLogExport ops(logCollectDir.toStdString());
ops.run();
+ // 增加对收集结果的校验,避免在收集失败时压缩空目录
+ if (QDir(logCollectDir).isEmpty()) {
+ qCWarning(logService) << "exportOpsLog: no logs collected in:" << logCollectDir;
+ removeOpsTempDirByPathInternal(opsDir);
+ return false;
+ }
+
// 将收集到的 log-ops 子目录内容整体压缩。压缩包放在 opsDir 内(log-ops.zip),
// 与被压缩内容同处一个随机目录,不暴露在 /var/log 下,避免被其它用户短暂读取。
// 直接以最终路径调用 zip 创建新压缩包,无需先创建空文件再删除。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs 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 |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
52b7751
into
linuxdeepin:release/eagle
Previously exportOpsLog created the zip archive as a separate QTemporaryFile directly under /var/log, briefly exposing system logs to other users before cleanup. Now logs are collected into a log-ops subdirectory inside the random temp dir (opsDir) and the zip is written there too, so neither the archive nor its content ever appear under /var/log.
此前 exportOpsLog 将压缩包作为独立的 QTemporaryFile 直接创建在
/var/log 下,在清理前会短暂暴露系统日志给其他用户。现在日志收集到
随机临时目录(opsDir)内的 log-ops 子目录中,压缩包也写入该目录,
压缩包与日志内容均不再暴露在 /var/log 下。
Log: exportOpsLog 压缩包改至临时目录内,避免 /var/log 暴露
Influence: 仅影响 exportOpsLog 日志导出流程,提升安全性,不影响其他功能
Summary by Sourcery
Keep exportOpsLog archives confined to their QTemporaryDir instead of /var/log to avoid exposing logs to other users.
Bug Fixes:
Enhancements: