fix: add command whitelist validation for notification actions

[yixinshark]

Add safeCommands whitelist in dconfig with default safe commands
Validate commands against whitelist before execution

Log: add command whitelist validation for notification actions

Summary by Sourcery

Validate notification action commands against a configurable whitelist before executing them.

Bug Fixes:

• Prevent execution of unauthorized shell commands triggered by notification actions by enforcing a whitelist from configuration.

Enhancements:

• Introduce a configurable safeCommands whitelist in notification configuration to restrict which commands can be run by notification actions.

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds a DConfig-based whitelist of allowed shell commands for notification actions and validates notification action commands against this whitelist before executing them.

Sequence diagram for notification action command validation and execution

sequenceDiagram
    actor User
    participant App
    participant NotificationServer
    participant NotificationManager
    participant DConfig
    participant QProcess

    User->>App: TriggerNotificationAction
    App->>NotificationServer: ActionInvoked(entity, actionKey)
    NotificationServer->>NotificationManager: doActionInvoked(entity, actionKey)

    NotificationManager->>NotificationManager: ParseActionArguments
    NotificationManager->>NotificationManager: Extract cmd and args

    NotificationManager->>DConfig: create(org.deepin.dde.shell, org.deepin.dde.shell.notification)
    DConfig-->>NotificationManager: DConfigInstance
    NotificationManager->>DConfig: value(safeCommands)
    DConfig-->>NotificationManager: safeCommands

    NotificationManager->>NotificationManager: Check cmd in safeCommands
    alt CommandNotWhitelisted
        NotificationManager-->>NotificationServer: LogWarningAndReturn
    else CommandWhitelisted
        NotificationManager->>QProcess: setProgram(cmd)
        NotificationManager->>QProcess: setArguments(args)
        NotificationManager->>QProcess: start()
        QProcess-->>NotificationManager: ExecutionResult
        NotificationManager-->>NotificationServer: ActionHandled
    end

Loading

Class diagram for NotificationManager command execution with DConfig whitelist

classDiagram
    class NotificationManager {
        +void doActionInvoked(NotifyEntity entity, QString actionKey)
    }

    class DConfig {
        +static DConfig* create(QString domain, QString name)
        +QVariant value(QString key)
    }

    class QProcess {
        +void setProgram(QString program)
        +void setArguments(QStringList arguments)
        +void start()
    }

    class NotifyEntity {
        +QString appName
        +QString summary
        +QString body
        +QStringList actions
    }

    NotificationManager --> DConfig : uses
    NotificationManager --> QProcess : creates_and_uses
    NotificationManager --> NotifyEntity : parameter

Loading

Flow diagram for notification action command whitelist validation

flowchart TD
    A[Start doActionInvoked] --> B[Check entity and action type]
    B --> C[Extract args list]
    C --> D{args is empty}
    D -->|Yes| E[Return without executing]
    D -->|No| F[cmd = args.takeFirst]
    F --> G[Create DConfig with org.deepin.dde.shell and org.deepin.dde.shell.notification]
    G --> H[Read safeCommands from DConfig]
    H --> I{safeCommands contains cmd}
    I -->|No| J[Log warning command not allowed]
    J --> K[Return without executing]
    I -->|Yes| L[Initialize QProcess]
    L --> M[Set program to cmd]
    M --> N[Set arguments to remaining args]
    N --> O[Start QProcess]
    O --> P[Return after starting process]
    E --> P
    K --> P

Loading

File-Level Changes

Change	Details	Files
Add whitelist-based validation before executing notification action commands.	Create a DConfig instance for the notification domain before running a notification action command. Read the safeCommands string list from configuration and treat it as the whitelist of allowed commands. Abort command execution and log a warning if the requested command is not present in the whitelist. Proceed with QProcess execution only when the command is whitelisted.	panels/notification/server/notificationmanager.cpp
Define configuration for safe notification action commands.	Extend the notification DConfig schema to include a safeCommands entry used as the whitelist for allowed commands. Provide a default set (possibly empty) of safe commands that can be executed by notification actions.	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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• Creating a new DConfig instance on every action invocation may be unnecessary overhead; consider caching the config or the safeCommands list at the class level and refreshing it only when the config changes.
• If the safeCommands list is missing or empty, the current logic will block all commands; consider explicitly handling this case (e.g., with a clear fallback or error) so behavior is intentional and easier to diagnose.
• The warning log prints the entire safeCommands list on each blocked command, which may be noisy and expose internal configuration; consider logging only the rejected command and perhaps the list size or a config version instead.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Creating a new DConfig instance on every action invocation may be unnecessary overhead; consider caching the config or the `safeCommands` list at the class level and refreshing it only when the config changes.
- If the `safeCommands` list is missing or empty, the current logic will block all commands; consider explicitly handling this case (e.g., with a clear fallback or error) so behavior is intentional and easier to diagnose.
- The warning log prints the entire `safeCommands` list on each blocked command, which may be noisy and expose internal configuration; consider logging only the rejected command and perhaps the list size or a config version instead.

## Individual Comments

### Comment 1
<location> `panels/notification/server/notificationmanager.cpp:536-537` </location>
<code_context>
             if (!args.isEmpty()) {
                 QString cmd = args.takeFirst(); // 命令

+                QScopedPointer<DConfig> config(DConfig::create("org.deepin.dde.shell", "org.deepin.dde.shell.notification"));
+                QStringList safeCommands = config->value("safeCommands").toStringList();
+
+                if (!safeCommands.contains(cmd)) {
</code_context>

<issue_to_address>
**issue (bug_risk):** Guard against DConfig::create() returning nullptr before dereferencing `config`.

If `DConfig::create()` returns `nullptr`, `config->value(...)` will dereference a null pointer and crash. Please check `config` before use and either treat a null config as "no safe commands" (deny all) or fall back to a reasonable default.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Guard against DConfig::create() returning nullptr before dereferencing config.

If DConfig::create() returns nullptr, config->value(...) will dereference a null pointer and crash. Please check config before use and either treat a null config as "no safe commands" (deny all) or fall back to a reasonable default.

[yixinshark]

/forcemerge

[deepin-bot]

This pr force merged! (status: blocked)