Skip to content

Conversation

@wangrong1069
Copy link
Contributor

No description provided.

Replaced direct `system()` calls with `QProcess`-based execution in
`application/utils.cpp` and `opsLogViewerService/opslogexport.cpp`.
Introduced `executCmd`, `parseCombinedArgString`, `appendToFile`,
`grepA`, `appendToFileWithGrepA`, `getPhysicalInterfaces`, and
`expandPathWithWildcardIterator` for safer and more flexible command
and file operations. This mitigates shell injection risks and improves
handling of command arguments and wildcard paths.

Task: https://pms.uniontech.com/task-view-381443.html
Refactor CMake files to:
- Remove redundant C++ standard version flags.
- Correctly apply security hardening linker flags by using CMAKE_EXE_LINKER_FLAGS and string(APPEND).
- Adjust the installation path for coredump reporter systemd user services.

Update Debian packaging rules:
- Migrate from debhelper to debhelper-compat (= 11) in debian/control.
- Remove the debian/compat file.
…ation

- Coredump Reporter Service:
    - Migrated coredump-reporter.service and coredump-reporter.timer installation from user-specific systemd/user to system-wide systemd/system.
    - Removed debian/deepin-log-viewer.postinst as the service is now managed globally.
    - Hardened coredump-reporter.service with User=root, ProtectSystem=strict, InaccessiblePaths for sensitive directories, and MemoryMax to improve security and resource management.

- D-Bus Service Authorization:
    - Replaced the generic isValidInvoker function with specific checkAuth calls using action IDs (s_Action_View, s_Action_Export) in LogViewerService.
    - Removed the isValidInvoker function and its declaration, streamlining the authorization logic.
Replaced the manual creation and deletion of the temporary directory
for operational logs with QTemporaryDir. QTemporaryDir provides a more
robust and safer mechanism for handling temporary files and directories.

Task: https://pms.uniontech.com/task-view-381443.html
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这次代码变更进行审查:

  1. 代码结构和组织改进:
  • 移除了 opsLogViewerService 子目录,将其功能合并到 logViewerService 中,减少了代码重复和维护成本
  • 删除了不必要的文件如 debian/compat、debian/deepin-log-viewer.postinst 等
  • 将 opslogexport 相关代码从 opsLogViewerService 移动到 logViewerService,使代码组织更合理
  1. 安全性改进:
  • 优化了编译选项,使用 string(APPEND CMAKE_EXE_LINKER_FLAGS) 替代直接设置 CMAKE_CXX_FLAGS
  • 在 coredump-reporter.service 中增加了安全限制,如 ProtectSystem=strict,InaccessiblePaths 等
  • 在 deepin-log-viewer-daemon.service 中增加了多项安全限制,如 NoNewPrivileges、ProtectKernelTunables 等
  1. 代码质量改进:
  • 移除了重复的安全编译参数设置
  • 删除了冗余的 LogViewerWatcher 类和相关代码
  • 优化了 DBus 接口的调用方式,使用 asyncCallWithArgumentList 替代原始的 QDBusMessage
  1. 性能优化:
  • 使用 QTemporaryDir 替代手动创建临时目录
  • 优化了文件操作,使用 executeCmd 替代 system() 调用
  • 改进了通配符路径处理,使用 expandPathWithWildcardIterator 函数
  1. 潜在问题和建议:

a) 在 opslogexport.cpp 中:

static int executCmd(const char *strCmd, const char *outputFile = nullptr, const QString &pattern = "", int afterLines = -1)

建议:

  • 添加错误处理和返回值检查
  • 考虑使用 QProcess 的错误信号处理机制
  • 添加超时机制防止命令执行卡死

b) 在 logviewerservice.cpp 中:

bool LogViewerService::checkAuthorization(const QString &actionId)

建议:

  • 添加更详细的错误日志记录
  • 考虑添加授权缓存机制
  • 增加对异常情况的处理

c) 在 utils.cpp 中:

static void appendToFile(const QString &filePath, const QByteArray &content)

建议:

  • 添加文件权限检查
  • 增加磁盘空间检查
  • 添加文件写入失败的错误处理
  1. 其他建议:
  • 考虑添加更多的单元测试覆盖新的代码变更
  • 建议在关键操作处添加更详细的日志记录
  • 可以考虑使用 C++17 或更高版本的标准特性来改进代码质量
  • 建议对文件操作添加更多的错误检查和异常处理

总体来说,这次代码变更在安全性、代码组织结构等方面都有明显改进,但在错误处理和日志记录方面还有优化空间。

@wangrong1069
Copy link
Contributor Author

/merge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Nov 27, 2025

This pr cannot be merged! (status: blocked)

@deepin-ci-robot
Copy link

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

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

@wangrong1069
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 67ee982 into linuxdeepin:master Nov 27, 2025
17 checks passed
@wangrong1069 wangrong1069 deleted the pr1118 branch November 27, 2025 10:46
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.

4 participants