Fix permissions for systemd-journal.plugin on offline installs#21953
Fix permissions for systemd-journal.plugin on offline installs#21953stelfrag merged 6 commits intonetdata:masterfrom
Conversation
github-actions
bot
added
the
area/packaging
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 3/5
- There is a concrete medium-risk issue in
packaging/makeself/install-or-update.sh:systemd-journal.pluginis being forced to setuid-root (4750) even whensetcapis available. - Because this affects privilege handling and is reported with high confidence, it raises regression/security risk enough to warrant a moderate merge-risk score.
- This likely has a straightforward fix (move
systemd-journal.plugininto the capability path), so risk should drop quickly once adjusted. - Pay close attention to
packaging/makeself/install-or-update.sh- avoid unnecessary setuid-root fallback when capabilities can be used.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packaging/makeself/install-or-update.sh">
<violation number="1" location="packaging/makeself/install-or-update.sh:227">
P2: This makes `systemd-journal.plugin` setuid-root even when `setcap` is available; add it to the capability branch instead of the unconditional 4750 fallback.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packaging/makeself/install-or-update.sh">
<violation number="1" location="packaging/makeself/install-or-update.sh:217">
P2: Reset the plugin mode to `0750` before `setcap`; otherwise successful upgrades can leave `systemd-journal.plugin` setuid-root even when file capabilities are available.</violation>
<violation number="2" location="packaging/makeself/install-or-update.sh:217">
P1: Guard the new `systemd-journal.plugin` permission changes with `[ -f ]`; disabled makeself builds will otherwise fail the offline installer.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fi | ||
| fi | ||
| if [ -f "usr/libexec/netdata/plugins.d/systemd-journal.plugin" ]; then | ||
| if ! run setcap "cap_dac_read_search=epi" "usr/libexec/netdata/plugins.d/systemd-journal.plugin"; then |
There was a problem hiding this comment.
eip is consistent with the packaged cap flags. Changed the order in the follow-up commit.
| fi | ||
| if [ -f "usr/libexec/netdata/plugins.d/systemd-journal.plugin" ]; then | ||
| if ! run setcap "cap_dac_read_search=epi" "usr/libexec/netdata/plugins.d/systemd-journal.plugin"; then | ||
| run chmod 4750 "usr/libexec/netdata/plugins.d/systemd-journal.plugin" |
There was a problem hiding this comment.
This is not needed. This is part of an install that overwrites the target files from the archive, therefor resetting any prior permissions.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run chmod 4750 "usr/libexec/netdata/plugins.d/otel-signal-viewer-plugin" | ||
| fi | ||
| fi | ||
| if [ -f "usr/libexec/netdata/plugins.d/systemd-journal.plugin" ]; then |
|
@stelfrag I can't seem to convince Copilot this last thing is needed without providing explicit instructions for this repo. |
Summary by cubic
Ensure
systemd-journal.plugingets correct ownership and permissions on offline installs. Triessetcapbefore SUID and skips changes when the plugin is absent to prevent journal read errors.packaging/makeself/install-or-update.sh, includesystemd-journal.pluginin ownership/permission handling; trysetcap cap_dac_read_search=eipwithchmod 4750fallback, all guarded by-fchecks.Written for commit 0b268bc. Summary will update on new commits.