Skip to content

fix(log): remove write to read-only system directory in initLog#727

Merged
re2zero merged 1 commit intolinuxdeepin:masterfrom
re2zero:fix/pms-347813-readonly-config
Apr 28, 2026
Merged

fix(log): remove write to read-only system directory in initLog#727
re2zero merged 1 commit intolinuxdeepin:masterfrom
re2zero:fix/pms-347813-readonly-config

Conversation

@re2zero
Copy link
Copy Markdown
Contributor

@re2zero re2zero commented Apr 28, 2026

Summary

  • Fix PMS Bug #347813: cooperation app triggers Panshi immutable system write rejection notification
  • Remove runtime write to /usr/share/<appName>/config.conf on Linux, use deb-installed read-only config instead
  • Add shared src/common/config.conf with default log level, installed via CMake to each app's share directory
  • Preserve Windows auto-creation logic (logDir is user-writable there)

Root Cause

Both commonutils.cpp implementations hardcoded /usr/share/<appName>/config.conf on Linux and attempted to write when the file didn't exist (setValue + sync). The file was never installed by any CMake rule or deb package, so it was always missing on a fresh install, triggering the write attempt on every startup. On immutable/read-only filesystems (磐石系统), this was blocked with a security notification.

Test Plan

  • Build and install deb package, verify config.conf exists in /usr/share/dde-cooperation/, /usr/share/dde-cooperation-daemon/, /usr/share/deepin-data-transfer/
  • Open cooperation, verify no Panshi write rejection notification
  • Verify log level is correctly set to default (2) from installed config
  • Modify installed config.conf to change log level, verify runtime update after 2 seconds
  • Verify Windows build still creates config.conf automatically in logDir

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @re2zero, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions
Copy link
Copy Markdown

  • 检测到debian目录文件有变更: debian/dde-cooperation.install,debian/deepin-data-transfer.install

@github-actions
Copy link
Copy Markdown

  • 检测到debian目录文件有变更: debian/dde-cooperation.install,debian/deepin-data-transfer.install

On Linux, config.conf is now installed by deb package as read-only.
Only create the file on Windows where logDir() is user-writable.

Linux端日志配置改为deb包预装只读文件,不再运行时写入系统目录。
Windows端保留原有的自动创建逻辑。

Log: 日志初始化不再写入系统只读目录
PMS: BUG-347813
Influence: 修复磐石系统拦截通知,启动时不再尝试写入/usr/share/目录
@re2zero re2zero force-pushed the fix/pms-347813-readonly-config branch from 7885e89 to 62da81a Compare April 28, 2026 06:10
@github-actions
Copy link
Copy Markdown

  • 检测到debian目录文件有变更: debian/dde-cooperation.install,debian/deepin-data-transfer.install

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码 diff 主要涉及为项目添加统一的日志级别配置文件 config.conf,并处理了 Linux 和 Windows 平台下的不同安装与加载逻辑。以下是针对语法逻辑、代码质量、性能和安全方面的详细审查意见:

1. 语法逻辑与代码质量

  • 跨平台路径拼接问题 (src/common/commonutils.cpp & src/compat/common/commonutils.cpp)

    • 问题:代码中使用 QString configFile = logConfPath + "config.conf"; 进行路径拼接。
    • 风险:虽然 logDir() 在大多数情况下应该会返回以 / 结尾的路径,但如果 logDir() 的实现发生变化(例如在 Windows 下返回不带斜杠的路径),或者未来重构导致其行为改变,这将导致路径错误(如 /var/logconfig.conf)。
    • 建议:使用 QDirQFileInfo 进行路径拼接,确保跨平台兼容性和健壮性。
    • 改进代码
      #include <QDir>
      // ...
      QString configFile = QDir(logConfPath).filePath("config.conf");
  • 文件操作与 QSettings 的混用

    • 问题:代码中先实例化了 QSettings settings(configFile, ...),然后才在 #ifndef linux 块中实例化 QFile file(configFile) 并检查是否存在。
    • 逻辑分析QSettings 在构造时如果文件不存在,通常不会报错,读取不存在的键会返回默认值。但在 Windows 逻辑中,意图是"如果文件不存在则创建并写入默认值"。
    • 建议:逻辑顺序可以微调。先检查文件是否存在(如果需要),再实例化 QSettings。不过目前的写法在功能上是可以工作的,只是略显混乱。更清晰的做法是将文件创建逻辑封装,或者确保 QSettings 的行为符合预期。目前的改动主要为了区分 Linux(只读,由包管理安装)和 Windows(可写,自动创建),逻辑是通顺的。
  • CMakeLists.txt 中的路径引用

    • 问题:在多个 CMakeLists.txt 中使用了 ${CMAKE_SOURCE_DIR}/src/common/config.conf
    • 建议:这是一个硬编码的相对路径。如果项目结构发生变化,这里需要修改。建议定义一个变量(如 CONFIG_FILE_PATH)来统一管理这个路径,或者确保 src/common 始终是源码树的一部分。目前作为小项目改动是可以接受的。

2. 代码性能

  • I/O 操作
    • 分析QSettings 在构造和 sync() 时会进行磁盘 I/O。在 initLog() 这种可能被频繁调用的初始化函数中,应尽量减少不必要的 I/O。
    • 现状:目前的逻辑仅在 Windows 下且文件不存在时执行一次写操作,sync() 也是必要的。Linux 下直接读取。性能影响可忽略不计。

3. 代码安全

  • 配置文件权限
    • Linux 安全性
      • 现状:在 Linux 下,配置文件被安装到 /usr/share/ 目录(例如 usr/share/dde-cooperation/config.conf),通常是只读的。
      • 风险:普通用户无法修改此文件来调整日志级别(例如调试问题)。如果需要临时修改日志级别,用户可能需要 root 权限或修改代码重新编译。
      • 建议:这是一个设计权衡。如果意图是"系统全局统一的日志级别",则当前做法是安全的。如果意图是允许用户自定义,通常应将配置文件放在用户目录(如 ~/.config/...)。鉴于注释明确写了"只读,由deb包安装",这符合预期设计,没有安全问题。
    • Windows 安全性
      • 现状:在 Windows 下,文件被创建在 logDir() 返回的路径中(推测是程序目录或用户目录)。
      • 风险:如果 logDir() 返回的是程序安装目录(如 C:\Program Files\...),普通用户可能没有权限写入,导致 settings.setValue() 失败,但程序可能不会报错,只是日志级别保持默认。
      • 建议:确保 Windows 下的 logDir() 返回的是用户可写的目录(如 AppData 或当前工作目录)。代码中已有注释 // On Windows, create config file if not exists (logDir is user-writable),说明开发者已意识到这一点。请确保 logDir() 的实现严格遵守这一承诺。

4. 其他建议

  • 版权年份 (src/common/config.conf)
    • 问题.reuse/dep5 中声明 Copyright: 2023 - 2026
    • 建议:通常版权年份不应超前于当前时间(除非有非常明确的长期规划)。建议使用 2023 - present 或具体到当前年份,避免法律上的潜在争议或混淆。

总结

这段代码的主要目的是为了规范化日志配置管理,区分 Linux 和 Windows 的处理方式。

  1. 逻辑:清晰,区分了包管理系统管理的配置(Linux)和运行时生成的配置(Windows)。
  2. 质量:路径拼接建议使用 QDir 以增强健壮性。
  3. 安全:Linux 下的只读策略符合系统软件规范,但需注意 Windows 下的目录写入权限。
  4. 格式:注意版权年份的声明规范。

总体来说,这是一次合理的重构,主要改进点在于使用 QDir 处理路径拼接。

@github-actions
Copy link
Copy Markdown

  • 检测到debian目录文件有变更: debian/dde-cooperation.install,debian/deepin-data-transfer.install

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: liujianqiang-niu, lzwind, pppanghu77, re2zero

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

@re2zero re2zero merged commit b149153 into linuxdeepin:master Apr 28, 2026
18 of 22 checks passed
@re2zero re2zero deleted the fix/pms-347813-readonly-config branch April 28, 2026 07:34
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.

5 participants