fix: install systemd drop-in under libdir - #741
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the CMake installation path for the project-provided systemd drop-in so it is installed alongside packaged units under the configured libdir rather than File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="deepin-devicemanager-server/deepin-devicecontrol/CMakeLists.txt" line_range="136" />
<code_context>
install(FILES deepin-devicecontrol.service DESTINATION lib/systemd/system/)
# 添加安全加固策略
-install(DIRECTORY deepin-service-group@.service.d DESTINATION /etc/systemd/system/)
+install(DIRECTORY deepin-service-group@.service.d DESTINATION ${CMAKE_INSTALL_LIBDIR}/systemd/system/)
</code_context>
<issue_to_address>
**🚨 issue (security):** On Debian multiarch builds, `${CMAKE_INSTALL_LIBDIR}` resolves to a path such as `lib/x86_64-linux-gnu`, so the drop-in is installed under `/usr/lib/x86_64-linux-gnu/systemd/system/`, which systemd does not search for unit drop-ins. The packaged `deepin-devicecontrol.service` remains under `/usr/lib/systemd/system/`, so this security-hardening drop-in is never applied.
**Triggers:** When the package is built on a multiarch system or with a non-default library directory.
**Suggested fix:** Install the drop-in beside the unit in `${CMAKE_INSTALL_PREFIX}/lib/systemd/system/`, or otherwise use the systemd vendor-unit directory rather than the architecture-specific library directory.
```suggestion
install(DIRECTORY deepin-service-group@.service.d DESTINATION lib/systemd/system/)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The deepin-service-group@.service.d drop-in is shipped by the project, so install it with packaged systemd units instead of /etc/systemd/system, which is reserved for local administrator overrides. Use lib/systemd/system to match deepin-devicecontrol.service and avoid architecture-specific CMAKE_INSTALL_LIBDIR values such as lib/x86_64-linux-gnu, which systemd does not search for unit drop-ins.
537d434 to
484b857
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: [] 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: [] 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: [] 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: [] 💡 改进建议代码示例// 暂无代码示例本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: felixonmars, lzwind 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 |
The service drop-in is shipped by the project, so install it next to packaged systemd units instead of under /etc/systemd/system, which is reserved for local administrator overrides.
Summary by Sourcery
Bug Fixes: