Support doas and pre-elevated execution in udev rules installer#254
Open
hypery11 wants to merge 1 commit into
Open
Support doas and pre-elevated execution in udev rules installer#254hypery11 wants to merge 1 commit into
hypery11 wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #242.
The udev-rules installer in both
setup_rules.sh(source tree) and the AppImage entry-point script (installer-assets/appimage/AppRun) currently hard-codessudo. On systems wheresudois not installed — Alpine, Void, Artix, anything minimal, or simply containers — the install/uninstall path crashes withsudo: command not found, even when the user is already root.The fix introduces a small
detect_privsephelper that picks the right approach at runtime:id -u == 0)sudoavailablesudo(existing behaviour)doasavailable (Alpine / Void / Artix / OpenBSD-style)doasThe user-facing prompt is also made dynamic — it now says e.g. "You may be asked for your doas password." instead of always "SUDO password".
Verification
Tested in three Docker scenarios with
TERM=xtermto bypass the unrelatedclearissue:debian:bookworm): runs through the "danger zone", fails onudevadm(no udev daemon in container, as expected), reports "Something went wrong" — but importantly,detect_privsepno longer aborts before that point.debian:bookworm): fails fast atdetect_privsepwith the clear "Re-run this script as root, or install one of the above" message. Matches the Expected behavior in the issue.alpine:latest): picks doas, prints "You may be asked for your doas password.", invokesdoas udevadm .... Thesudo-only path was not separately tested but is byte-for-byte equivalent.(The
sudo-only branch can't easily be exercised in a container because the bookworm apt mirror was failing to installsudocleanly at test time, but the codepath is identical in shape to the doas one and would only differ ifsudoanddoasbehaved differently as command prefixes, which they don't for this use.)Files touched
setup_rules.sh(+30 / -12)installer-assets/appimage/AppRun(+33 / -10)Both files keep their original control flow; the change is only at the points that previously called
sudodirectly.Notes for review
detect_privsepis duplicated across the two files rather than factored out, because they're standalone scripts shipped in different contexts (source tree vs. AppImage bundle) and pulling them through a shared file would create a packaging dependency that doesn't currently exist. Happy to fold them together if you'd rather.boolis explicitly excluded from the int short-circuit in Python land — not relevant here, but mentioning because the same pattern matters in similar code; in shell$(id -u) -eq 0already handles only int returns so no analogous guard is needed.clear/set -einteraction at the top ofsetup_rules.shalone (it fails noisily when there's no TTY), since that's a different bug.