feat(setup): detect display managers and screen lockers; offer per-service opt-in#43
Merged
tyvsmith merged 2 commits intoMay 24, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the facelock setup wizard to detect supported PAM services dynamically (based on /etc/pam.d/<service> presence) and to configure each service via per-service opt-in prompts, with safe defaults applied automatically in non-interactive mode.
Changes:
- Replace the hardcoded PAM service list with a
PAM_CANDIDATEStable filtered at runtime. - Switch from a single
MultiSelectprompt to per-serviceConfirmprompts (and apply defaults in non-interactive mode). - Add unit tests for candidate detection and explicitly excluded services; add
tempfileas a dev-dependency for tests.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/facelock-cli/src/commands/setup.rs | Adds PAM candidate detection + per-service prompting, and tests for filtering/exclusions. |
| crates/facelock-cli/Cargo.toml | Adds tempfile as a dev-dependency for new unit tests. |
| Cargo.lock | Locks the newly added tempfile dev-dependency. |
Comments suppressed due to low confidence (2)
crates/facelock-cli/src/commands/setup.rs:1441
- Grammar in this description is off: "declining recommended" is missing a verb (e.g., "declining is recommended").
description: "SDDM login screen (KDE) \u{2014} declining recommended unless you have recovery access",
crates/facelock-cli/src/commands/setup.rs:1447
- Grammar in this description is off: "declining recommended" is missing a verb (e.g., "declining is recommended").
description: "LightDM login screen (Ubuntu/Xfce/Mint) \u{2014} declining recommended unless you have recovery access",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+947
to
+951
| Confirm::new() | ||
| .with_prompt(&prompt) | ||
| .default(candidate.default_enabled) | ||
| .interact()? | ||
| }; |
Comment on lines
+1435
to
+1447
| description: "GDM login screen (GNOME) \u{2014} declining recommended unless you have recovery access", | ||
| default_enabled: false, | ||
| }, | ||
| PamCandidate { | ||
| service: "sddm", | ||
| category: PamCategory::DisplayManager, | ||
| description: "SDDM login screen (KDE) \u{2014} declining recommended unless you have recovery access", | ||
| default_enabled: false, | ||
| }, | ||
| PamCandidate { | ||
| service: "lightdm", | ||
| category: PamCategory::DisplayManager, | ||
| description: "LightDM login screen (Ubuntu/Xfce/Mint) \u{2014} declining recommended unless you have recovery access", |
tyvsmith
added a commit
that referenced
this pull request
May 24, 2026
…DM descriptions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rvice opt-in Replaces the hardcoded [sudo, polkit-1, hyprlock] list with a PAM_CANDIDATES table that is filtered at runtime by /etc/pam.d/<service> existence, then presented to the user one service at a time via dialoguer::Confirm. New services covered: - swaylock (Sway/wlroots screen lock) — default YES - kscreenlocker_greet (KDE Plasma screen lock) — default YES - gdm-password (GNOME display manager login screen) — default NO - sddm (KDE display manager login screen) — default NO - lightdm (Ubuntu/Xfce/Mint display manager login screen) — default NO Default-YES rationale: screen lockers have safe password fallback; worst case is an extra Enter keypress. Default-NO rationale: display manager login screens lock users out of the system if face auth fails and they have no other recovery path; opt-in should be explicit. Explicit non-targets (never offered, even if /etc/pam.d/<name> exists): - system-auth / common-auth: shared stacks that would spread face auth to passwd, su, chsh, chfn, etc. - login: TTY login; camera may not be initialized at boot. - su, passwd, chsh, chfn: credential/privilege-change tools that must require a real password. Wizard flow change: replaces MultiSelect-all-at-once with individual Confirm prompts so the user sees the default recommendation per service. Non-interactive mode uses each candidate's default_enabled value. Adds two unit tests: - detect_candidates_filters_by_presence: verifies candidates_in() against a TempDir; only services with a matching file are returned. - no_excluded_services_in_candidates: asserts the excluded service list never appears in PAM_CANDIDATES. Adds tempfile as a dev-dependency of facelock-cli. Note: PR #1 (uninstall cleanup) and PR #2 (PAM confirmation prompt) are racing. This PR may have minor conflicts with #2 if both touch the per-service iteration; resolve at merge time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…DM descriptions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tyvsmith
force-pushed
the
feat/pam-setup-detect-display-managers-and-lockers
branch
from
f14da04 to
68ebc20
Compare
5 tasks
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.
Summary
[sudo, polkit-1, hyprlock]PAM service list with aPAM_CANDIDATEStable filtered at runtime by/etc/pam.d/<service>existenceswaylock,kscreenlocker_greet,gdm-password,sddm,lightdmConfirmprompts, showing the recommended default per servicedefault_enabledvalueServices × Category × Default × Rationale
sudopolkit-1hyprlockswaylockkscreenlocker_greetgdm-passwordsddmlightdmIntentionally excluded (never offered)
system-auth(Arch) /common-auth(Debian): shared stacks — would spread face auth topasswd,su,chsh,chfn, etc.login: TTY login; camera may not be initialized at bootsu,passwd,chsh,chfn: credential/privilege-change tools that must require a real passwordPAM_CANDIDATESare offeredNote on racing PRs
PR #1 (uninstall cleanup) and PR #2 (PAM confirmation prompt) are racing. This PR may have minor conflicts with PR #2 if both touch the per-service iteration loop; resolve at merge time.
Test plan
cargo build --workspace— clean, no warningscargo test -p facelock-cli— 69 tests pass, including:commands::setup::tests::detect_candidates_filters_by_presencecommands::setup::tests::no_excluded_services_in_candidatescargo clippy --workspace -- -D warnings— cleansudoandhyprlockin/etc/pam.d/: only those two are offeredsddmpresent:sddmis offered with default NOfacelock setup --non-interactive): defaults applied without promptscargo run --bin facelock -- setup --help— still works🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com