Skip to content

Revert "feat: integrate security loader into session startup" - #227

Open
fly602 wants to merge 1 commit into
linuxdeepin:masterfrom
fly602:master
Open

Revert "feat: integrate security loader into session startup"#227
fly602 wants to merge 1 commit into
linuxdeepin:masterfrom
fly602:master

Conversation

@fly602

@fly602 fly602 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

This reverts commit 8b39a25.

回退 飞行模式安全启动的注册

原因:配合dde-daemon回退安全启动校验的方案。因为该接口的安全校验影响范围较广,涉及到控制中心、快捷键、任务栏插件等。由于项目架构问题,在这些项目上加安全启动很复杂,而且会严重影响这些项目的架构。

后续计划

需重新评估安全方案的整体设计,制定更契合 V25 架构的分阶段实施方案。

Summary by Sourcery

Revert security loader integration from session startup and restore the previous dde-session service behavior.

Enhancements:

  • Remove security loader integration from dde-session startup and restore direct systemd readiness notification.

Build:

  • Remove security loader wrapper installation and testing configuration from the build.

Chores:

  • Delete the security loader helper implementation and associated wrapper files.

Summary by Sourcery

Revert security-loader integration from dde-session startup and restore the previous session service behavior.

Enhancements:

  • Restore dde-session's direct systemd readiness notification and remove security-loader authorization from session startup.

Build:

  • Remove security-loader wrapper installation and testing configuration from the build.

Chores:

  • Delete the security-loader helper and associated wrapper files.

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

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

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602

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

@sourcery-ai

sourcery-ai Bot commented Aug 18, 2026

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

Reviewer's Guide

Reverts the previously added security loader integration and its test harness from dde-session, simplifying startup while still notifying systemd when the session is ready.

Sequence diagram for updated dde-session startup and systemd readiness notification

sequenceDiagram
    actor systemd
    participant dde_session

    systemd->>dde_session: start dde-session-manager service
    dde_session->>dde_session: main parse systemd-service option
    dde_session->>dde_session: create Session and IOWaitWatcher
    dde_session->>systemd: sd_notify 0 "READY=1"
    systemd->>systemd: mark dde-session-manager as ready
Loading

File-Level Changes

Change Details Files
Remove security loader integration from dde-session startup path.
  • Drop inclusion of securityloaderhelper and related header from main.cpp
  • Remove command line options for security loader file descriptors
  • Delete security loader handshake logic using SecurityLoaderInfo and authorization checks
src/dde-session/main.cpp
src/dde-session/securityloaderhelper.cpp
src/dde-session/securityloaderhelper.h
Simplify CMake and install-time wiring related to the security loader and tests.
  • Remove securityloaderhelper sources from dde-session target
  • Remove installation of dde-session into libexecdir and the dde-session-loader-wrapper script
  • Delete securityloaderhelper_test target and associated CTest integration
src/dde-session/CMakeLists.txt
CMakeLists.txt
misc/dde-session-loader-wrapper.in
src/dde-session/securityloaderhelper_test.cpp
Ensure systemd service is notified when dde-session is ready without going through the security loader wrapper.
  • Add sd-daemon include to main.cpp
  • Call sd_notify with READY=1 once the session setup and IO watchers are initialized
src/dde-session/main.cpp
systemd/dde-session-manager.service.in
debian/control

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

…式安全启动的注册

原因:配合 dde-daemon 回退安全启动校验的方案。因为该接口的安全校验
影响范围较广,涉及控制中心、快捷键、任务栏插件等。由于项目架构问题,
在这些项目上加安全启动很复杂,且会严重影响这些项目的架构。

后续计划

需重新评估安全方案的整体设计,制定更契合 V25 架构的分阶段实施方案。
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码移除了复杂的鉴权包装逻辑,重构为标准的 systemd notify 模式,架构显著优化
逻辑完全正确且无安全漏洞,符合最高评分标准

■ 【详细分析】

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

src/dde-session/main.cpp 中移除了对已删除头文件 securityloaderhelper.h 的引用,新增的 sd_notify(0, "READY=1") 调用准确放置在 QCoreApplication::exec() 事件循环启动之前,符合 systemd notify 协议的时序要求,无编译或运行时逻辑错误
潜在问题:无
建议:无

  • 2.代码质量(优秀)✓

删除了约 500 行复杂的 Shell 包装脚本和 C++ 管道通信代码,消除了 forking 模式下的 GuessMainPID 不可靠问题,同时移除了基于 gdbus 轮询的 ExecStartPost,改用原生通知机制,代码结构大幅精简且符合现代 Linux 服务开发规范
潜在问题:无
建议:无

  • 3.代码性能(高效)✓

彻底消除了 gdbus wait 带来的最长 30 秒轮询阻塞开销,以及原 securityloaderhelper.cpp 中基于 poll 的 5 秒超时管道读写开销,服务启动延迟降至最低
潜在问题:无
建议:无

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
移除了自定义的文件描述符传递与管道鉴权机制,有效缩减了文件描述符劫持和自定义协议解析带来的攻击面,权限控制回归至 dde-daemon 自身的标准鉴权链路

  • 建议:无

■ 【改进建议代码示例】

// 可选的防御性编程改进:检查 sd_notify 返回值并记录日志,便于排查 systemd 状态同步问题
#include <systemd/sd-daemon.h>
#include <cstring>

// ... (省略中间代码)
    int ret = sd_notify(0, "READY=1");
    if (ret < 0) {
        qWarning() << "Failed to notify systemd:" << strerror(-ret);
    }
// ...

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.

2 participants