Skip to content

Conversation

@yixinshark
Copy link
Contributor

@yixinshark yixinshark commented Jan 14, 2026

Add busctl and gdbus as safe commands for notification actions

Log: update safeCommands whitelist for notification service

Summary by Sourcery

Enhancements:

  • Allow busctl and gdbus to be executed as safe commands by the notification service.

Add busctl and gdbus as safe commands for notification actions

Log: update safeCommands whitelist for notification service
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 14, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Extends the notification service configuration to treat busctl and gdbus as safe commands for notification-triggered actions by updating the JSON whitelist.

File-Level Changes

Change Details Files
Extend notification service safeCommands whitelist to include additional D-Bus-related utilities.
  • Add busctl to the list of whitelisted safe commands used by the notification service for executing actions.
  • Add gdbus to the list of whitelisted safe commands in the notification service configuration JSON.
  • Ensure the updated configuration remains valid JSON and consistent with existing safeCommands formatting and conventions.
panels/notification/server/configs/org.deepin.dde.shell.notification.json

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 left some high level feedback:

  • Consider scoping the busctl and gdbus entries as narrowly as possible (e.g., restricting arguments or target services) rather than whitelisting the bare commands to minimize potential security exposure.
  • Double-check that the updated safeCommands list remains consistent with other similar service configs in this repo so that notification actions behave uniformly across panels/services.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider scoping the `busctl` and `gdbus` entries as narrowly as possible (e.g., restricting arguments or target services) rather than whitelisting the bare commands to minimize potential security exposure.
- Double-check that the updated `safeCommands` list remains consistent with other similar service configs in this repo so that notification actions behave uniformly across panels/services.

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

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xionglinlin, yixinshark

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

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码是一个 JSON 配置文件的差异(diff),主要用于定义通知系统允许执行的"安全命令"白名单(safeCommands)。通过审查这段代码,我提出以下关于语法逻辑、代码质量、代码性能和代码安全的改进意见:

1. 语法逻辑

  • JSON 格式正确性:这段 diff 展示的 JSON 语法是完全正确的。它是一个有效的 JSON 数组,字符串都被双引号正确包裹,逗号分隔符使用得当。
  • 变更逻辑:变更逻辑清晰,即在现有的白名单数组中追加了三个新的命令字符串:"busctl""gdbus"

2. 代码质量

  • 可维护性
    • 建议:目前所有的命令都平铺在一个数组中。如果这个列表在未来继续增长,建议将其拆分为多行格式(每行一个命令),以提高可读性和便于后续合并冲突的解决。
    • 示例
      "value": [
        "xdg-open",
        "dbus-send",
        ...
        "busctl",
        "gdbus"
      ]
  • 注释说明:JSON 标准不支持注释。如果需要解释为什么添加 busctlgdbus,建议在提交信息(Commit Message)或相关文档中详细记录,说明引入这两个 D-Bus 工具的具体业务场景或依赖需求。

3. 代码性能

  • 查找效率safeCommands 通常用于在运行时检查某个命令是否被允许执行。
    • 现状:JSON 解析后通常为数组或列表结构。如果白名单很长(例如超过几十项),使用数组进行线性查找(O(n))会有轻微的性能开销。
    • 建议:在加载此配置的 C++ 或后端代码中,建议将此数组转换为哈希集合(如 QSetstd::unordered_set)。这样可以将查找复杂度从 O(n) 降低到 O(1),显著提升性能,尤其是在通知频繁触发的场景下。

4. 代码安全 —— 最重要

  • 命令白名单机制:这是一个关键的安全配置,用于防止任意命令执行(RCE)漏洞。只允许预定义的"安全"命令运行是一个很好的安全实践。
  • 新增命令的风险评估
    • busctlgdbus:这两个是强大的 D-Bus 交互工具(类似于已有的 dbus-sendqdbus)。将它们加入白名单意味着通知系统可以调用它们来与系统服务进行通信。
    • 潜在风险:虽然它们本身是系统工具,但如果通知消息的内容可以被部分控制(例如通过应用发送的通知参数),且后端代码没有严格校验传递给这些命令的参数,攻击者可能利用 busctlgdbus 调用敏感的系统 D-Bus 接口(例如修改设置、重启服务、读取敏感文件等)。
    • 改进建议
      1. 参数校验:确保后端代码在执行这些命令时,对传入的参数进行了严格的白名单校验或转义,防止命令注入。
      2. 最小权限原则:确认是否真的需要这两个工具。如果 dbus-send 已经满足需求,添加额外的工具会增加攻击面。如果必须添加,请确保调用它们的逻辑是受限的。
      3. 绝对路径:列表中混用了仅命令名(如 xdg-open)和绝对路径(如 /usr/lib/...)。
      • 为了更安全,建议全部使用绝对路径。如果只写 busctl,恶意用户可能通过修改 PATH 环境变量来劫持该命令,执行恶意的同名脚本。指定完整路径(如 /usr/bin/busctl)可以规避此类风险。

总结

这个变更是为了扩展通知系统的功能以支持更多的 D-Bus 工具。虽然语法和逻辑没有问题,但从安全角度出发,建议:

  1. 将所有命令改为绝对路径引用。
  2. 审查后端调用逻辑,确保参数传递安全,防止通过 D-Bus 工具进行提权或敏感操作。

@yixinshark
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Jan 14, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 526e65d into linuxdeepin:master Jan 14, 2026
8 of 11 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