Skip to content

fix: keep ops log zip archive inside temp dir to avoid /var/log exposure - #484

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
GongHeng2017:202607060924-re-eagle-fix
Jul 6, 2026
Merged

fix: keep ops log zip archive inside temp dir to avoid /var/log exposure#484
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
GongHeng2017:202607060924-re-eagle-fix

Conversation

@GongHeng2017

@GongHeng2017 GongHeng2017 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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:

  • Ensure the ops log zip archive and collected logs are created only inside the temporary ops directory, preventing transient exposure under /var/log.

Enhancements:

  • Simplify exportOpsLog compression by using a dedicated log-ops subdirectory and direct zip creation without pre-creating a temporary file.

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 日志导出流程,提升安全性,不影响其他功能
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
  • 检测到敏感词export变动
详情
    {
    "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;"
            ]
        }
    }
}

@github-actions
github-actions Bot requested a review from liujianqiang-niu July 6, 2026 02:42
@sourcery-ai

sourcery-ai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

The 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 handling

sequenceDiagram
    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
Loading

File-Level Changes

Change Details Files
Refactor exportOpsLog to collect logs into a dedicated subdirectory within the temporary opsDir and write the zip archive there instead of under /var/log.
  • Add QDir include to support directory creation.
  • Create log-ops subdirectory inside opsDir via QDir::mkpath and handle failure by logging and cleaning up the temporary directory.
  • Instantiate OpsLogExport with the new log-ops subdirectory path instead of the root opsDir.
  • Replace QTemporaryFile-based zip path under /var/log with a fixed log-ops.zip path inside opsDir.
  • Simplify zip invocation: set working directory to log-ops, zip its contents with relative path '.', and remove the previous workaround that deleted a pre-created empty zip file.
logViewerService/logviewerservice.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 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.
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.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码成功修复了临时文件路径暴露和zip命令报错的历史缺陷,逻辑严密且无安全漏洞
修复方案精准消除了权限泄露风险,代码结构清晰规范,符合优秀标准仅作微调扣5分

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

修复后的代码通过在临时目录下创建log-ops子目录进行日志收集,并将压缩包输出至同一临时目录,彻底避免了原逻辑中创建0字节空文件导致zip报错退出的问题。QDir().mkpath与QProcess的调用顺序及参数传递均符合Qt规范,异常分支处理完整。
建议:可考虑对OpsLogExport::run()执行后的收集结果进行非空校验,防止在收集失败时依然执行压缩空目录的无用操作

  • 2.代码质量(良好)✓

注释详尽且准确解释了修复动机与实现细节,变量命名如logCollectDir、tmpZipPath语义清晰,准确反映了其业务用途。使用QStringLiteral替代字符串字面量提升了内存效率。
建议:无重大问题,保持现有规范即可

  • 3.代码性能(无性能问题)✓

使用QProcess::waitForFinished(-1)进行同步等待,在D-Bus系统服务的方法调用场景下是合理的,避免了复杂的异步状态机开销。压缩操作本身属于I/O密集型,无算法层面的性能损耗。
建议:若未来日志体量极大,可考虑在QProcess启动前校验待压缩目录的文件总大小

  • 4.代码安全(安全,存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 2 个,持平 0 个
本次修复彻底消除了原代码在/var/log下创建临时文件导致的权限泄露风险,同时QProcess使用QStringList传递参数,有效杜绝了命令注入风险。logCollectDir与tmpZipPath均由安全的QTemporaryDir路径与硬编码常量拼接而成,不存在路径遍历可能。

  • 建议:继续保持使用参数列表启动外部进程的安全编码习惯

■ 【改进建议代码示例】

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 创建新压缩包,无需先创建空文件再删除。

@deepin-ci-robot

Copy link
Copy Markdown

[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.

Details Needs approval from an approver in each of these files:

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

@GongHeng2017

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This pr force merged! (status: unstable)

@deepin-bot
deepin-bot Bot merged commit 52b7751 into linuxdeepin:release/eagle Jul 6, 2026
21 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants