Skip to content

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Jan 31, 2026

安全加固保护了home目录,然而grub需要读取home下的.Xauthority文件才能和x11通讯. 导致检查支持的分辨率的时候被x服务拒绝

Log:
Influence: 安全加固

Summary by Sourcery

Bug Fixes:

  • Fix X11-based resolution detection in grub2 when the home directory is protected and .Xauthority access is restricted.

安全加固保护了home目录,然而grub需要读取home下的.Xauthority文件才能和x11通讯. 导致检查支持的分辨率的时候被x服务拒绝

Log:
Influence: 安全加固
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 31, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the deepin-grub2 systemd service so grub’s resolution-detection helper can access the user’s X11 authority data despite home directory hardening, fixing X11-based resolution probing failures under security hardening.

File-Level Changes

Change Details Files
Update deepin-grub2 systemd service definition to allow X11-based resolution detection to work when home directories are hardened.
  • Modify the service configuration so the grub/X11 helper can read the .Xauthority file required for X11 communication under security-hardened home directories
  • Ensure the service still runs under the intended security constraints while permitting the minimal access required for X11-based resolution probing
misc/systemd/services/system/deepin-grub2.service

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

Copy link

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

Hey - I've reviewed your changes and they look great!


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

deepin pr auto review

这段代码修改涉及的是 Linux 系统中 systemd 服务单元文件的配置,具体是针对 deepin-grub2.service 服务的安全策略调整。

以下是对该 diff 的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的分析:

1. 语法逻辑

  • 现状:修改将 ProtectHome=yes 改为了 ProtectHome=read-only
  • 分析:在 systemd.service 手册中,ProtectHome= 参数接受布尔值或特殊字符串 read-only
    • yes(或 true):表示 /home 目录对服务完全不可见(挂载空目录)。
    • read-only:表示 /home 目录以只读方式挂载给服务。
    • no:表示允许完全读写访问。
  • 结论:语法完全正确,符合 systemd 配置规范。

2. 代码质量

  • 注释说明:代码中添加了 # 允许只读访问 /home 以读取 .Xauthority 文件
    • 优点:这是一个非常好的实践。它清晰地解释了“为什么要放宽安全策略”。如果没有这个注释,审查者可能会认为这是一个安全漏洞或错误配置。明确指出是为了读取 .Xauthority(通常用于 X11 认证),说明了业务逻辑的必要性。
  • 结论:代码质量高,注释清晰,意图明确。

3. 代码性能

  • 分析ProtectHome 的变更主要影响的是命名空间的挂载方式和文件系统的访问权限。
    • yes(完全隐藏)变为 read-only(只读挂载),在服务启动时,挂载逻辑会稍有不同(不再是挂载一个空的 tmpfs,而是 bind mount 并标记为 ro),但这对于系统启动性能的影响微乎其微,完全可以忽略不计。
  • 结论:对性能无显著影响。

4. 代码安全 —— 重点关注

这是本次修改的核心部分,涉及安全边界的变更。

  • 安全风险分析

    • 原配置 (ProtectHome=yes):这是最严格的模式。服务无法访问 /home 下的任何文件。如果该服务被攻破,攻击者无法窃取用户家目录下的敏感文件(如私钥、浏览器数据、密码本等)。
    • 新配置 (ProtectHome=read-only):放宽了限制。服务现在可以读取 /home 下的所有文件。
    • 具体风险点:为了读取 .Xauthority 而开放整个 /home 目录的“读取权限”,存在信息泄露风险。如果 deepin-grub2.service 中存在漏洞被利用,攻击者虽然不能写入文件,但可以遍历并读取用户目录下的所有数据(例如 ~/.ssh/id_rsa~/.bash_history 等)。
  • 改进建议

    • 最小权限原则:虽然 read-onlyno(读写)安全,但比 yes(不可见)要弱。如果确实需要访问 .Xauthority,建议确认是否可以通过其他方式避免。
    • 替代方案(如果适用)
      1. 检查依赖:确认该服务是否真的需要直接与 X 交互?如果是后台生成 GRUB 配置,通常不需要 X 认证。
      2. 文件迁移:如果必须使用该文件,能否将必要的认证信息复制到一个服务本来就有权访问的非家目录位置(如 /run/var/lib),由其他受信任的组件负责维护?
      3. 用户隔离:确保该服务以特定的非特权用户(如 root 或专门的系统用户)运行,避免权限混乱。
    • 如果必须保留此修改
      • 请务必确保 deepin-grub2.service 内部处理文件路径的代码是健壮的,防止路径遍历攻击导致意外读取(虽然只读限制了写入,但读取本身也是隐私泄露)。
      • 确保服务本身没有远程网络接口,或者网络接口足够安全,防止远程代码执行(RCE)后直接沦为信息窃取工具。

总结

该修改在语法和逻辑上是正确的,注释也增加了代码的可维护性。但在安全性上,这是一个“降级”操作(放宽了沙盒限制)。

最终建议
如果业务逻辑上确实必须读取 .Xauthority,且无法通过架构调整避免,那么这个修改是可接受的,但需要意识到随之而来的信息泄露风险。建议在代码审查时,重点确认 deepin-grub2 进程的安全性,确保它不易被攻破。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

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

@fly602 fly602 merged commit 0742229 into linuxdeepin:master Jan 31, 2026
17 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