-
Notifications
You must be signed in to change notification settings - Fork 55
fix: update safeCommands whitelist for notification service #1395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add busctl and gdbus as safe commands for notification actions Log: update safeCommands whitelist for notification service
Reviewer's guide (collapsed on small PRs)Reviewer's GuideExtends the notification service configuration to treat File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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
busctlandgdbusentries 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
safeCommandslist 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.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto review这段代码是一个 JSON 配置文件的差异(diff),主要用于定义通知系统允许执行的"安全命令"白名单( 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全 —— 最重要
总结这个变更是为了扩展通知系统的功能以支持更多的 D-Bus 工具。虽然语法和逻辑没有问题,但从安全角度出发,建议:
|
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Add busctl and gdbus as safe commands for notification actions
Log: update safeCommands whitelist for notification service
Summary by Sourcery
Enhancements: