Skip to content

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Dec 22, 2025

Added security hardening options to the application update notifier systemd service:

  1. ProtectSystem=strict - Prevents writing to system directories
  2. PrivateNetwork=yes - Isolates the service from network access
  3. RestrictAddressFamilies=AF_UNIX - Limits socket communication to local Unix sockets only
  4. NoNewPrivileges=yes - Prevents the service from gaining additional privileges

These security measures follow systemd best practices to minimize the service's attack surface and contain potential security breaches by restricting filesystem access, network capabilities, and privilege escalation.

Influence:

  1. Verify the update notifier service starts correctly with new restrictions
  2. Test that application update notifications still function properly
  3. Confirm the service cannot access network resources
  4. Validate that file system operations are properly restricted
  5. Check that privilege escalation attempts are blocked

chore: 增强 systemd 服务安全性

为应用程序更新通知器的 systemd 服务添加安全加固选项:

  1. ProtectSystem=strict - 防止写入系统目录
  2. PrivateNetwork=yes - 隔离服务与网络访问
  3. RestrictAddressFamilies=AF_UNIX - 限制套接字通信仅限本地 Unix 套接字
  4. NoNewPrivileges=yes - 防止服务获取额外权限

这些安全措施遵循 systemd 最佳实践,通过限制文件系统访问、网络能力和权限
提升来最小化服务的攻击面并遏制潜在的安全漏洞。

Influence:

  1. 验证更新通知器服务在新增限制下能正确启动
  2. 测试应用程序更新通知功能是否正常工作
  3. 确认服务无法访问网络资源
  4. 验证文件系统操作是否被正确限制
  5. 检查权限提升尝试是否被阻止

Summary by Sourcery

Enhancements:

  • Tighten filesystem, network, and privilege-related security options on the application update notifier systemd unit to follow systemd hardening best practices.

Added security hardening options to the application update notifier
systemd service:
1. ProtectSystem=strict - Prevents writing to system directories
2. PrivateNetwork=yes - Isolates the service from network access
3. RestrictAddressFamilies=AF_UNIX - Limits socket communication to
local Unix sockets only
4. NoNewPrivileges=yes - Prevents the service from gaining additional
privileges

These security measures follow systemd best practices to minimize the
service's attack surface and contain potential security breaches by
restricting filesystem access, network capabilities, and privilege
escalation.

Influence:
1. Verify the update notifier service starts correctly with new
restrictions
2. Test that application update notifications still function properly
3. Confirm the service cannot access network resources
4. Validate that file system operations are properly restricted
5. Check that privilege escalation attempts are blocked

chore: 增强 systemd 服务安全性

为应用程序更新通知器的 systemd 服务添加安全加固选项:
1. ProtectSystem=strict - 防止写入系统目录
2. PrivateNetwork=yes - 隔离服务与网络访问
3. RestrictAddressFamilies=AF_UNIX - 限制套接字通信仅限本地 Unix 套接字
4. NoNewPrivileges=yes - 防止服务获取额外权限

这些安全措施遵循 systemd 最佳实践,通过限制文件系统访问、网络能力和权限
提升来最小化服务的攻击面并遏制潜在的安全漏洞。

Influence:
1. 验证更新通知器服务在新增限制下能正确启动
2. 测试应用程序更新通知功能是否正常工作
3. 确认服务无法访问网络资源
4. 验证文件系统操作是否被正确限制
5. 检查权限提升尝试是否被阻止
@sourcery-ai
Copy link

sourcery-ai bot commented Dec 22, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Hardens the Application Update Notifier systemd service by adding several security-focused unit options that restrict filesystem access, networking, socket address families, and privilege escalation, following systemd hardening best practices.

File-Level Changes

Change Details Files
Tightened systemd service security configuration for the application update notifier.
  • Added ProtectSystem=strict to prevent the service from modifying system directories
  • Enabled PrivateNetwork=yes to isolate the service from host network access
  • Restricted allowed socket address families to AF_UNIX to limit communication to local Unix sockets
  • Set NoNewPrivileges=yes to block the service and its children from gaining additional privileges
apps/app-update-notifier/misc/systemd/system/org.desktopspec.ApplicationUpdateNotifier1.service.in

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:

  • Given ProtectSystem=strict and the likely need to write state somewhere, consider explicitly declaring StateDirectory=/CacheDirectory=/RuntimeDirectory= (or otherwise ensuring writable paths under /var are clearly defined) so future changes don’t accidentally break the service’s ability to persist data.
  • You might want to complement the existing hardening with other low-risk systemd protections that usually don’t interfere with D-Bus–only services, such as ProtectHome=yes, PrivateTmp=yes, and ProtectKernelTunables=yes, if they are compatible with how the notifier runs.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Given `ProtectSystem=strict` and the likely need to write state somewhere, consider explicitly declaring `StateDirectory=`/`CacheDirectory=`/`RuntimeDirectory=` (or otherwise ensuring writable paths under `/var` are clearly defined) so future changes don’t accidentally break the service’s ability to persist data.
- You might want to complement the existing hardening with other low-risk systemd protections that usually don’t interfere with D-Bus–only services, such as `ProtectHome=yes`, `PrivateTmp=yes`, and `ProtectKernelTunables=yes`, if they are compatible with how the notifier runs.

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: fly602, zccrs

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

@zccrs
Copy link
Member

zccrs commented Dec 24, 2025

为什么obs构建失败了

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个systemd服务文件的修改进行审查:

  1. 语法逻辑:
    修改内容完全符合systemd服务单元文件的语法规范,新增的安全配置项都是标准的systemd指令。

  2. 代码质量:

  • 新增的安全配置项组织合理,放在文件末尾并添加了注释说明,便于维护
  • 配置项的选择都很恰当,每个都有明确的安全目的
  1. 代码性能:
    这些安全限制对性能影响很小:
  • ProtectSystem=strict:只会在文件系统访问时增加一些检查开销
  • PrivateNetwork=yes:实际上可能略微提升性能,因为限制了网络访问
  • RestrictAddressFamilies=AF_UNIX:限制套接字类型,性能影响可忽略
  • NoNewPrivileges=yes:几乎没有性能开销
  1. 代码安全(重点):
    新增的安全配置提供了很好的安全加固:
  • ProtectSystem=strict:

    • 优点:将文件系统访问限制为只读,只允许写入/tmp、/var等少数目录,极大降低了文件系统被破坏的风险
    • 建议:如果程序需要写入特定目录,可能需要配合使用ReadWritePaths=
  • PrivateNetwork=yes:

    • 优点:禁用网络访问,防止服务被用作网络攻击的跳板
    • 建议:如果服务确实需要网络访问,应考虑使用更细粒度的控制,如IPAddressDeny/Allow
  • RestrictAddressFamilies=AF_UNIX:

    • 优点:只允许使用Unix域套接字通信,是最小权限原则的体现
    • 建议:这个配置很好,因为D-Bus服务确实只需要Unix域套接字
  • NoNewPrivileges=yes:

    • 优点:防止进程通过setuid等机制提升权限
    • 建议:这是很好的安全实践,应该保留

总体建议:

  1. 考虑添加更多的安全限制,如:

    • ProtectHome=yes(如果不需要访问用户主目录)
    • PrivateTmp=yes(使用私有/tmp目录)
    • ProtectKernelTunables=yes
    • ProtectKernelModules=yes
    • ProtectControlGroups=yes
  2. 如果服务需要特定的权限访问某些路径,应该明确声明,例如:

    • ReadWritePaths=/path/to/needed/dir
  3. 建议添加系统调用过滤,例如:

  4. 考虑添加资源限制,例如:

    • LimitNOFILE=65536
    • MemoryMax=512M

这些修改显著提升了服务的安全性,同时保持了良好的可维护性。建议继续采用这种安全加固的方式。

@fly602
Copy link
Contributor Author

fly602 commented Dec 25, 2025

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Dec 25, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit e2eff76 into linuxdeepin:master Dec 25, 2025
15 of 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