Fix audit findings, expand tests, and add a CI quality gate

Addresses the deep codebase audit:

- Center the static wallpaper cover-fit crop so the software (wl_shm)
  path agrees with the GPU path and the greeter, instead of anchoring
  the crop top-left.
- Desktop-entry Exec tokenizer: respect single quotes and proper
  backslash escaping per the freedesktop spec.
- Greeter: dispatch CancelSession whenever the UI resets to the
  username stage so the error path can't leave greetd holding a stale
  PAM conversation; add a per-roundtrip timeout so a wedged greetd peer
  can't strand the "Working" spinner.
- start_locker: return an error instead of spawning a child that
  expect()-panics when WAYLAND_DISPLAY is unset.
- Bound in-flight PAM checks with a concurrency cap, since libpam
  cannot be cancelled and a stuck module would otherwise leak one
  uncancellable thread per retry.
- Expand unit/regression tests (157 total): tokenizer edge cases and a
  pseudo-fuzz, blit offset/clamp cases, horizontal+vertical cover
  centering, greetd roundtrip-timeout and connection-recovery.
- Reformat the workspace to rustfmt-clean and add a Forgejo CI gate
  (fmt --check, clippy -D warnings, all-target tests, locked release
  build) — previously the only workflow was an Arch package builder.

breadgreet/src/greetd/mod.rs

@@ -30,13 +30,22 @@ pub async fn run_actor<E>(cmd_rx: mpsc::UnboundedReceiver<Command>, emit: E)
 where
     E: FnMut(Event) + Send + 'static,
 {
-    run_actor_with(cmd_rx, emit, Client::connect).await;
+    run_actor_with(cmd_rx, emit, Client::connect, DEFAULT_ROUNDTRIP_TIMEOUT).await;
 }
 
+/// Upper bound for a single greetd roundtrip. Without it, a hung PAM module
+/// would leave the actor blocked on a `read_from` forever and the UI stuck on
+/// the "Working" spinner. When it fires the conversation is cancelled and an
+/// [`Event::Error`] is surfaced. Generous on purpose: a slow disk or a
+/// deliberate password iterate shouldn't false-positive, but a wedged peer
+/// must not wedge the greeter.
+const DEFAULT_ROUNDTRIP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
+
 async fn run_actor_with<E, C, Fut>(
     mut cmd_rx: mpsc::UnboundedReceiver<Command>,
     mut emit: E,
     mut connect: C,
+    roundtrip_timeout: std::time::Duration,
 ) where
     E: FnMut(Event),
     C: FnMut() -> Fut,
@@ -67,7 +76,26 @@ async fn run_actor_with<E, C, Fut>(
             }
         }
 
-        let result = exec_cmd(client.as_mut().expect("just connected"), cmd).await;
+        let result = match tokio::time::timeout(
+            roundtrip_timeout,
+            exec_cmd(client.as_mut().expect("just connected"), cmd),
+        )
+        .await
+        {
+            Ok(result) => result,
+            // Hunger guard: the peer accepted our request but never answered.
+            // Treat it as a wedged connection rather than going through the
+            // normal `Roundtrip(Err)` handler, whose `cancel_session().await`
+            // would itself block on a `read_from` against the same dead peer.
+            // Dropping the client makes the next command reconnect fresh.
+            Err(_elapsed) => {
+                client = None;
+                emit(Event::Error(format!(
+                    "greetd did not respond within {roundtrip_timeout:?}"
+                )));
+                continue;
+            }
+        };
         match result {
             CmdResult::Idle => {}
             CmdResult::Started => emit(Event::SessionStarted),
@@ -157,6 +185,7 @@ mod tests {
                         }
                     }
                 },
+                std::time::Duration::from_secs(30),
             )
             .await;
         });
@@ -177,15 +206,12 @@ mod tests {
             Response::Success.write_to(&mut stream).await.unwrap();
         });
 
-        cmd_tx
-            .send(Command::CreateSession("bob".into()))
-            .unwrap();
+        cmd_tx.send(Command::CreateSession("bob".into())).unwrap();
         let ev = ev_rx.recv().await.expect("actor should retry after bind");
         match ev {
             Event::Outcome(Outcome::Success) => {}
             other => panic!("expected Success, got {other:?}"),
         }
-
         drop(cmd_tx);
         server.await.unwrap();
         actor.await.unwrap();
@@ -195,4 +221,131 @@ mod tests {
         );
         std::fs::remove_file(&path).ok();
     }
+
+    #[tokio::test]
+    async fn wedged_roundtrip_times_out_instead_of_hanging() {
+        // A peer that accepts the request but never answers must not leave the
+        // actor blocked on `read_from` forever; the roundtrip timeout fires,
+        // the connection is dropped, and the UI is told so it can recover.
+        let (cmd_tx, cmd_rx) = mpsc::unbounded_channel();
+        let (ev_tx, mut ev_rx) = mpsc::unbounded_channel();
+
+        let path = sock("timeout");
+        std::fs::remove_file(&path).ok();
+        let listener = UnixListener::bind(&path).unwrap();
+
+        let server = tokio::spawn(async move {
+            // Accept and read the CreateSession request, then stay wedged: the
+            // socket stays open (holding `stream`) but no response is ever
+            // written, so the client hits its roundtrip timeout rather than
+            // an EOF.
+            let (mut stream, _) = listener.accept().await.unwrap();
+            let _ = Request::read_from(&mut stream).await;
+            std::future::pending::<()>().await;
+        });
+
+        let connect_path = path.clone();
+        let actor = tokio::spawn(async move {
+            run_actor_with(
+                cmd_rx,
+                move |ev| {
+                    let _ = ev_tx.send(ev);
+                },
+                move || {
+                    let connect_path = connect_path.clone();
+                    async move { Client::connect_to(&connect_path).await }
+                },
+                std::time::Duration::from_millis(200),
+            )
+            .await;
+        });
+
+        // Let the listener bind and the actor connect, then drive the roundtrip.
+        tokio::time::sleep(std::time::Duration::from_millis(30)).await;
+        cmd_tx.send(Command::CreateSession("bob".into())).unwrap();
+
+        let ev = tokio::time::timeout(std::time::Duration::from_secs(5), ev_rx.recv())
+            .await
+            .expect("actor must surface a timeout promptly, not hang")
+            .expect("actor must emit an event");
+        match ev {
+            Event::Error(msg) => assert!(
+                msg.contains("did not respond"),
+                "expected a roundtrip-timeout error, got {msg:?}"
+            ),
+            other => panic!("expected Error, got {other:?}"),
+        }
+
+        // The actor must not hang on a cancel read against the dead peer.
+        drop(cmd_tx);
+        actor.await.unwrap();
+        // Abort the never-completing wedged server and reap it.
+        server.abort();
+        let _ = server.await;
+        std::fs::remove_file(&path).ok();
+    }
+
+    #[tokio::test]
+    async fn roundtrip_connection_error_recovers_on_next_request() {
+        // A roundtrip that dies mid-conversation (EOF) is a connection error:
+        // the actor drops the client, surfaces the Error, and then reconnects
+        // on the next command so a transient blip can't wedge the greeter.
+        let (cmd_tx, cmd_rx) = mpsc::unbounded_channel();
+        let (ev_tx, mut ev_rx) = mpsc::unbounded_channel();
+
+        let path = sock("roundtrip-recover");
+        std::fs::remove_file(&path).ok();
+        let listener = UnixListener::bind(&path).unwrap();
+
+        let server = tokio::spawn(async move {
+            // First connection: read the request, then hang up -> the client
+            // sees EOF mid-roundtrip.
+            let (mut stream, _) = listener.accept().await.unwrap();
+            let _ = Request::read_from(&mut stream).await;
+            drop(stream);
+            // Second connection (after the actor reconnects): answer Success.
+            let (mut stream, _) = listener.accept().await.unwrap();
+            let _ = Request::read_from(&mut stream).await;
+            let _ = Response::Success.write_to(&mut stream).await;
+        });
+
+        let connect_path = path.clone();
+        let actor = tokio::spawn(async move {
+            run_actor_with(
+                cmd_rx,
+                move |ev| {
+                    let _ = ev_tx.send(ev);
+                },
+                move || {
+                    let connect_path = connect_path.clone();
+                    async move { Client::connect_to(&connect_path).await }
+                },
+                std::time::Duration::from_secs(30),
+            )
+            .await;
+        });
+
+        tokio::time::sleep(std::time::Duration::from_millis(30)).await;
+        cmd_tx.send(Command::CreateSession("bob".into())).unwrap();
+        let ev = ev_rx.recv().await.expect("first roundtrip error event");
+        match ev {
+            Event::Error(msg) => assert!(
+                msg.contains("greetd IPC error"),
+                "expected a connection (EOF) error, got {msg:?}"
+            ),
+            other => panic!("expected Error, got {other:?}"),
+        }
+
+        cmd_tx.send(Command::CreateSession("bob".into())).unwrap();
+        let ev = ev_rx.recv().await.expect("recovery success event");
+        match ev {
+            Event::Outcome(Outcome::Success) => {}
+            other => panic!("expected Success after reconnect, got {other:?}"),
+        }
+
+        drop(cmd_tx);
+        server.await.unwrap();
+        actor.await.unwrap();
+        std::fs::remove_file(&path).ok();
+    }
 }

breadgreet/src/main.rs

@@ -384,10 +384,9 @@ impl App {
             Stage::Prompt | Stage::Working => {
                 self.status_lbl.set_label("");
                 self.status_lbl.remove_css_class("error");
+                // `reset_to_username` dispatches the CancelSession itself, so
+                // we don't double-send it here.
                 self.reset_to_username();
-                if self.cmd_tx.send(greetd::Command::CancelSession).is_err() {
-                    self.show_error("Cannot reach greetd");
-                }
             }
         }
     }
@@ -416,6 +415,17 @@ impl App {
         self.stage = Stage::Username;
         self.username.clear();
         self.pam_status_held = false;
+        // Abort any greetd conversation still open server-side. Without this,
+        // the error/`show_error` reset path returns to the username entry but
+        // leaves greetd holding a half-done PAM conversation, so the next
+        // login attempt's CreateSession stacks on a stale session. On a
+        // broken channel we set the failure label directly rather than
+        // recursing into `show_error`, which would call back into
+        // `reset_to_username` forever.
+        if self.cmd_tx.send(greetd::Command::CancelSession).is_err() {
+            self.status_lbl.set_label("Cannot reach greetd");
+            self.status_lbl.add_css_class("error");
+        }
         if !self.sessions.is_empty() {
             self.entry.grab_focus();
         }

breadgreet/src/sessions.rs

@@ -103,23 +103,36 @@ fn split_exec(exec: &str) -> Vec<String> {
 fn tokenize_exec(exec: &str) -> Vec<String> {
     let mut args = Vec::new();
     let mut current = String::new();
-    let mut in_quote = false;
+    let mut in_quote = None; // Some('"') or Some('\'')
     let mut chars = exec.chars().peekable();
 
     while let Some(c) = chars.next() {
-        match c {
-            '"' => in_quote = !in_quote,
-            '\\' if in_quote => {
-                if let Some(n) = chars.next() {
-                    current.push(n);
+        match in_quote {
+            Some(q) => match c {
+                // Closing the active quote just toggles back to unquoted.
+                c if c == q => in_quote = None,
+                // Inside double quotes a backslash escapes the next char
+                // (freedesktop Exec). Inside single quotes it's literal.
+                '\\' if q == '"' => match chars.next() {
+                    Some(n) => current.push(n),
+                    None => current.push('\\'),
+                },
+                _ => current.push(c),
+            },
+            None => match c {
+                '"' | '\'' => in_quote = Some(c),
+                // Single quotes have no quoting/escaping inside them.
+                '\\' => match chars.next() {
+                    Some(n) => current.push(n),
+                    None => current.push('\\'),
+                },
+                c if c.is_whitespace() => {
+                    if !current.is_empty() {
+                        args.push(std::mem::take(&mut current));
+                    }
                 }
-            }
-            c if c.is_whitespace() && !in_quote => {
-                if !current.is_empty() {
-                    args.push(std::mem::take(&mut current));
-                }
-            }
-            _ => current.push(c),
+                _ => current.push(c),
+            },
         }
     }
     if !current.is_empty() {
@@ -173,6 +186,80 @@ mod tests {
         assert_eq!(split_exec(r#"echo "100%%""#), vec!["echo", "100%"]);
     }
 
+    #[test]
+    fn split_exec_handles_single_quotes_and_escapes() {
+        // Single-quoted arguments are one token (previously these split).
+        assert_eq!(split_exec(r#"cmd 'two words'"#), vec!["cmd", "two words"]);
+        // A backslash outside quotes escapes the next character, so an
+        // escaped space merges into the running token.
+        assert_eq!(split_exec(r"cmd a\ b"), vec!["cmd", "a b"]);
+        // `\"` inside double quotes is an escaped backslash then a close
+        // quote, i.e. a literal backslash inside a quoted argument.
+        assert_eq!(split_exec(r#"cmd "a\"b""#), vec!["cmd", "a\"b"]);
+    }
+
+    #[test]
+    fn tokenize_exec_quotes_spaces_and_escapes_edge_cases() {
+        // Quoting preserves embedded spaces; a backslash escapes a space.
+        assert_eq!(tokenize_exec("echo \"a b\""), vec!["echo", "a b"]);
+        assert_eq!(tokenize_exec("echo 'a b'"), vec!["echo", "a b"]);
+        assert_eq!(tokenize_exec("echo a\\ b"), vec!["echo", "a b"]);
+        // Inside double quotes a backslash escapes the next character,
+        // including the quote itself.
+        assert_eq!(tokenize_exec(r#"echo "a\"b""#), vec!["echo", "a\"b"]);
+        // An escaped backslash outside quotes yields one literal backslash.
+        assert_eq!(tokenize_exec(r"echo a\\"), vec!["echo", "a\\"]);
+        // Quoting can splice mid-word (the space is part of one argument).
+        assert_eq!(
+            tokenize_exec(r#"echo pre"mid dle"post"#),
+            vec!["echo", "premid dlepost"]
+        );
+        // Plain whitespace splits greedily, tabs/newlines included.
+        assert_eq!(tokenize_exec("  a   b\t c\n "), vec!["a", "b", "c"]);
+        // Field codes survive this layer; `split_exec` drops them later.
+        assert_eq!(
+            tokenize_exec("app %U --flag %f"),
+            vec!["app", "%U", "--flag", "%f"]
+        );
+        // An unclosed quote swallows the remainder as one token (no panic).
+        assert_eq!(tokenize_exec("app \"rest of"), vec!["app", "rest of"]);
+        // Empty / whitespace-only input yields no tokens.
+        assert!(tokenize_exec("").is_empty());
+        assert!(tokenize_exec("   \t  ").is_empty());
+    }
+
+    #[test]
+    fn tokenize_exec_fuzz_never_panics_and_emits_only_nonempty_tokens() {
+        // Pseudo-fuzz over quoting/escaping/whitespace/field-code characters:
+        // whatever the input, tokenizing must not panic and must never emit an
+        // empty token (the tokenizer only pushes non-empty buffers).
+        let alphabet: [char; 7] = ['a', 'b', ' ', '\'', '"', '\\', '%'];
+        let mut state: u64 = 0x2545_F491_4F6C_DD1D;
+        let mut rng = move || {
+            state = state
+                .wrapping_mul(6364136223846793005)
+                .wrapping_add(1442695040888963407);
+            state
+        };
+        for _ in 0..5_000 {
+            let len = (rng() % 32) as usize;
+            let input: String = (0..len)
+                .map(|_| alphabet[(rng() as usize) % alphabet.len()])
+                .collect();
+            let tokens = tokenize_exec(&input);
+            assert!(
+                tokens.iter().all(|t| !t.is_empty()),
+                "tokenize must not emit empty tokens for {input:?} -> {tokens:?}"
+            );
+            // Whitespace-only input must yield no tokens. (The converse is
+            // deliberately not asserted: an input of only a quote/backslash
+            // adds no word chars and so correctly yields nothing.)
+            if input.chars().all(char::is_whitespace) {
+                assert!(tokens.is_empty(), "{input:?} -> {tokens:?}");
+            }
+        }
+    }
+
     #[test]
     fn discover_returns_none_when_no_directories_exist() {
         assert!(discover(
@@ -231,21 +318,15 @@ mod tests {
         assert_eq!(listed[0].exec, vec!["/usr/local/bin/bos-session"]);
         assert_eq!(listed[0].kind, SessionKind::Wayland);
         assert_eq!(listed[2].kind, SessionKind::X11);
-        assert!(
-            listed[2]
-                .start_env()
-                .contains(&"XDG_SESSION_TYPE=x11".to_string())
-        );
-        assert!(
-            listed[0]
-                .start_env()
-                .contains(&"XDG_SESSION_TYPE=wayland".to_string())
-        );
-        assert!(
-            listed[0]
-                .start_env()
-                .contains(&"XDG_SESSION_DESKTOP=bos".to_string())
-        );
+        assert!(listed[2]
+            .start_env()
+            .contains(&"XDG_SESSION_TYPE=x11".to_string()));
+        assert!(listed[0]
+            .start_env()
+            .contains(&"XDG_SESSION_TYPE=wayland".to_string()));
+        assert!(listed[0]
+            .start_env()
+            .contains(&"XDG_SESSION_DESKTOP=bos".to_string()));
 
         std::fs::remove_dir_all(&wayland).ok();
         std::fs::remove_dir_all(&x11).ok();