Run fleet desktop with user SELinux context when applicable - #30882
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes update how the fleet desktop application is launched on Linux. Instead of using Changes
Sequence Diagram(s)sequenceDiagram
participant Launcher as Fleet Desktop Launcher
participant User as Target User
participant SELinux as SELinux Context
participant OS as Linux OS
Launcher->>User: Get user info and environment
Launcher->>SELinux: Get SELinux context (if enabled)
alt SELinux context present
Launcher->>OS: Launch process with runcon + runuser (correct context and user)
else SELinux context absent
Launcher->>OS: Launch process with runuser (correct user)
end
OS-->>Launcher: Process started under correct user and context
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #30882 +/- ##
==========================================
- Coverage 64.01% 64.01% -0.01%
==========================================
Files 1905 1896 -9
Lines 187625 187405 -220
Branches 5369 5287 -82
==========================================
- Hits 120108 119966 -142
+ Misses 58073 57993 -80
- Partials 9444 9446 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@lucasmrod this is ready for code review, I still need to test it on a few OSs to check I didn't break anything. |
lucasmrod
left a comment
There was a problem hiding this comment.
LGTM! Left some nit comments
|
@lucasmrod addressed your comments, thanks! |
For #29793
Details
This PR changes the way that Orbit launches processes such as the desktop app on Linux, in order to ensure that on SELinux-enabled systems the correct user context is set when running the command.
Previously,
sudo -uwas used to launch commands on Linux. This PR switches to userunuserinstead, which is recommended in situations where the root user wants to execute a command as a user with reduced privileges (see the blog post by one of the creators of runuser). This avoids certain errors that can come from interacting with PAM modules as the system user.Additionally, if we detect that SELinux is set up on a system, we now use
runconto force the command to run using the logged-in user's SELinux context. It's possible that on some systems they may have configuration wheresudowill switch to the user's SELinux context automatically, but this is not guaranteed. Usingrunuser+runconis our best bet for ensuring that the desktop app (and anything that it spawns) runs under the correct context.This PR also does some refactoring so that the three
runmethods for Linux (run,runWithOutputandrunWithStdin) all use the same base code to create the command with the correct args and env vars, and differ only in how they handle the i/o.Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
runtime.GOOS).Manual QA must be performed in the three main OSs, macOS, Windows and Linux.(n/a, code is linux only)Auto-update manual QA, from released version of component to new version (see tools/tuf/test).n/aTesting
runWithOutputstill works (tested withgo run ./tools/dialog/main.go --dialog=zenity)(this isn't currently used by Linux)runWithStdinstill worksSummary by CodeRabbit
Bug Fixes
Refactor