Fix ui-post-install.sh to use the full username #4809
Conversation
|
|
WalkthroughThe post-install script Changes
Sequence Diagram(s)sequenceDiagram
participant RPM as RPM %post
participant Script as ui-post-install.sh
participant Proc as /proc
participant Sys as System
RPM->>Script: execute post-install script
Script->>Script: set -e, set -u
Script->>Script: pgrep netbird-ui => PID?
alt PID found
Script->>Proc: read /proc/[PID]/loginuid => UID
Script->>Script: id -nu UID => username
Script->>Sys: pkill netbird-ui
Script->>Sys: su - username -c "nohup /opt/netbird-ui/netbird-ui &"
else no PID
Script-->>RPM: skip restart (no running process)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
release_files/ui-post-install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
release_files/ui-post-install.sh
[warning] 5-5: In POSIX sh, [[ ]] is undefined.
(SC3010)
🔇 Additional comments (1)
release_files/ui-post-install.sh (1)
11-11: Verifysu -l -syntax correctness on Fedora 43.The
suinvocation uses the syntaxsu -l - "${username}", which is non-standard. Standard forms are:
su - "${username}"(login shell, implied by-)su -l "${username}"(explicit-lflag)The sequence
-l -has the second-in an unusual position after the first flag. This may fail or behave unexpectedly on strictsuimplementations. The intent is unclear: is the second-a typo, or is it meant to request a login shell while preserving a different behavior?Please verify that
su -l - "${username}"works correctly on Fedora 43 and other target systems. If the syntax is non-standard, consider simplifying tosu - "${username}"orsu -l "${username}".Additionally, test the script end-to-end with a long username to confirm it resolves and restarts correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
release_files/ui-post-install.sh(1 hunks)
🔇 Additional comments (1)
release_files/ui-post-install.sh (1)
3-8: POSIX shell compatibility issue resolved. The previous SC3010 warning about Bash-specific[[ ]]syntax has been fixed. Line 8 now correctly uses the POSIX-compliant[ -n "${pid}" ]test, which will work across dash, ash, and other POSIX shells.
|
|
Darwin tests failed at:
Since my change only can affect Debian and Fedora/RHEL packages, not Darwin/macOS, I assume this is an unrelated issue? |
2 participants
Describe your changes
Fixes #4808 by extracting the full username by:
pgrep/proc/${PID}/loginuididAlso replaces "complex" pipe from
pstosedwith a (hopefully) "simpler" (as in requiring less knowledge about the arguments ofpsand regexps) invocation ofcatandid.Issue ticket number and link
#4808
Stack
Checklist
Documentation
Select exactly one:
This fixes a post-install script used in Linux packages, which should be invisible to regular users.
Docs PR URL (required if "docs added" is checked)
N/A
Closes: #4808
Summary by CodeRabbit