diff --git a/CLAUDE.md b/CLAUDE.md index eecca95..1bf0f5a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,3 +44,8 @@ uv run moongreet - `i18n.py` — Locale-Erkennung (LANG / /etc/locale.conf) und String-Tabellen (DE/EN) - `greeter.py` — GTK4 UI (Overlay-Layout), Faillock-Warnung nach 2 Fehlversuchen - `main.py` — Entry Point, GTK App, Layer Shell Setup + +## Design Decisions + +- **Synchrones I/O im GTK-Konstruktor**: `load_config`, `load_strings`, `get_users` und `get_sessions` laufen synchron in `GreeterWindow.__init__`. Async Loading mit Placeholder-UI wäre möglich, erhöht aber die Komplexität erheblich. Der Greeter startet 1x pro Boot auf lokaler Hardware — die Daten sind klein (passwd, locale.conf, wenige .desktop-Files), die Latenz im Normalfall vernachlässigbar. +- **Synchrones Avatar-Decoding**: `GdkPixbuf.Pixbuf.new_from_file_at_scale` läuft synchron auf dem Main Thread. Bei großen Bildern als `.face`-Datei kann die UI kurz stocken. Der Avatar-Cache (`_avatar_cache`) federt das nach dem ersten Laden ab. Async Decoding per Worker-Thread + `GLib.idle_add` wäre die Alternative, rechtfertigt aber den Aufwand nicht für einen Single-User-Greeter. diff --git a/config/moongreet.toml b/config/moongreet.toml index 1bc0f18..1069fae 100644 --- a/config/moongreet.toml +++ b/config/moongreet.toml @@ -4,7 +4,3 @@ [appearance] # Absolute path to wallpaper image background = "/usr/share/backgrounds/wallpaper.jpg" - -[behavior] -# show_user_list = true -# default_session = "Hyprland" diff --git a/src/moongreet/config.py b/src/moongreet/config.py index 35876d1..f0c6b82 100644 --- a/src/moongreet/config.py +++ b/src/moongreet/config.py @@ -33,8 +33,11 @@ def load_config(config_path: Path | None = None) -> Config: if not config_path.exists(): return Config() - with open(config_path, "rb") as f: - data = tomllib.load(f) + try: + with open(config_path, "rb") as f: + data = tomllib.load(f) + except (tomllib.TOMLDecodeError, OSError): + return Config() config = Config() appearance = data.get("appearance", {}) diff --git a/src/moongreet/greeter.py b/src/moongreet/greeter.py index bf40621..8178a59 100644 --- a/src/moongreet/greeter.py +++ b/src/moongreet/greeter.py @@ -1,8 +1,11 @@ # ABOUTME: Main greeter window — builds the GTK4 UI layout for the Moongreet greeter. # ABOUTME: Handles user selection, session choice, password entry, and power actions. +import logging import os +import re import shlex +import shutil import socket import stat import subprocess @@ -22,8 +25,12 @@ from moongreet.users import User, get_users, get_avatar_path, get_user_gtk_theme from moongreet.sessions import Session, get_sessions from moongreet.power import reboot, shutdown +logger = logging.getLogger(__name__) + LAST_USER_PATH = Path("/var/cache/moongreet/last-user") FAILLOCK_MAX_ATTEMPTS = 3 +VALID_USERNAME = re.compile(r"^[a-zA-Z0-9_.-]+$") +MAX_USERNAME_LENGTH = 256 PACKAGE_DATA = files("moongreet") / "data" DEFAULT_AVATAR_PATH = PACKAGE_DATA / "default-avatar.svg" DEFAULT_WALLPAPER_PATH = PACKAGE_DATA / "wallpaper.jpg" @@ -56,13 +63,33 @@ class GreeterWindow(Gtk.ApplicationWindow): self._sessions = get_sessions() self._selected_user: User | None = None self._greetd_sock: socket.socket | None = None + self._greetd_sock_lock = threading.Lock() self._default_avatar_pixbuf: GdkPixbuf.Pixbuf | None = None self._avatar_cache: dict[str, GdkPixbuf.Pixbuf] = {} self._failed_attempts: dict[str, int] = {} + self._wallpaper_ctx = None self._build_ui() - self._select_initial_user() self._setup_keyboard_navigation() + # Defer initial user selection until the window is realized, + # so get_color() returns the actual theme foreground for SVG tinting + self.connect("realize", self._on_realize) + + def _on_realize(self, widget: Gtk.Widget) -> None: + """Called when the window is realized — select initial user. + + Deferred from __init__ so get_color() returns actual theme values + for SVG tinting. Uses idle_add so the first frame renders before + avatar loading blocks the main loop. + """ + GLib.idle_add(self._select_initial_user) + + def do_unrealize(self) -> None: + """Clean up resources when the window is unrealized.""" + if self._wallpaper_ctx is not None: + self._wallpaper_ctx.__exit__(None, None, None) + self._wallpaper_ctx = None + super().do_unrealize() def _build_ui(self) -> None: """Build the complete greeter UI layout.""" @@ -375,12 +402,13 @@ class GreeterWindow(Gtk.ApplicationWindow): def _close_greetd_sock(self) -> None: """Close the greetd socket and reset the reference.""" - if self._greetd_sock: - try: - self._greetd_sock.close() - except OSError: - pass - self._greetd_sock = None + with self._greetd_sock_lock: + if self._greetd_sock: + try: + self._greetd_sock.close() + except OSError: + pass + self._greetd_sock = None def _set_login_sensitive(self, sensitive: bool) -> None: """Enable or disable login controls during authentication.""" @@ -412,7 +440,8 @@ class GreeterWindow(Gtk.ApplicationWindow): sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) sock.settimeout(10.0) sock.connect(sock_path) - self._greetd_sock = sock + with self._greetd_sock_lock: + self._greetd_sock = sock # Step 1: Create session response = create_session(sock, user.username) @@ -440,7 +469,7 @@ class GreeterWindow(Gtk.ApplicationWindow): # Step 3: Start session if response.get("type") == "success": cmd = shlex.split(session.exec_cmd) - if not cmd: + if not cmd or not shutil.which(cmd[0]): cancel_session(sock) GLib.idle_add(self._on_login_error, None, self._strings.invalid_session_command) return @@ -457,11 +486,13 @@ class GreeterWindow(Gtk.ApplicationWindow): self._close_greetd_sock() except ConnectionError as e: + logger.error("greetd connection error: %s", e) self._close_greetd_sock() - GLib.idle_add(self._on_login_error, None, self._strings.connection_error.format(error=e)) + GLib.idle_add(self._on_login_error, None, self._strings.connection_error) except (OSError, ValueError) as e: + logger.error("greetd socket error: %s", e) self._close_greetd_sock() - GLib.idle_add(self._on_login_error, None, self._strings.socket_error.format(error=e)) + GLib.idle_add(self._on_login_error, None, self._strings.socket_error) def _on_login_error(self, response: dict | None, message: str) -> None: """Handle login error on the GTK main thread.""" @@ -483,12 +514,13 @@ class GreeterWindow(Gtk.ApplicationWindow): def _cancel_pending_session(self) -> None: """Cancel any in-progress greetd session.""" - if self._greetd_sock: - try: - cancel_session(self._greetd_sock) - except (ConnectionError, OSError): - pass - self._close_greetd_sock() + with self._greetd_sock_lock: + if self._greetd_sock: + try: + cancel_session(self._greetd_sock) + except (ConnectionError, OSError): + pass + self._close_greetd_sock() def _get_selected_session(self) -> Session | None: """Get the currently selected session from the dropdown.""" @@ -535,9 +567,12 @@ class GreeterWindow(Gtk.ApplicationWindow): """Load the last logged-in username from cache.""" if LAST_USER_PATH.exists(): try: - return LAST_USER_PATH.read_text().strip() + username = LAST_USER_PATH.read_text().strip() except OSError: return None + if len(username) > MAX_USERNAME_LENGTH or not VALID_USERNAME.match(username): + return None + return username return None @staticmethod diff --git a/src/moongreet/i18n.py b/src/moongreet/i18n.py index 85943fa..f445a92 100644 --- a/src/moongreet/i18n.py +++ b/src/moongreet/i18n.py @@ -31,9 +31,11 @@ class Strings: reboot_failed: str shutdown_failed: str - # Templates (use .format()) + # Error messages (continued) connection_error: str socket_error: str + + # Templates (use .format()) faillock_attempts_remaining: str faillock_locked: str @@ -54,8 +56,8 @@ _STRINGS_DE = Strings( session_start_failed="Session konnte nicht gestartet werden", reboot_failed="Neustart fehlgeschlagen", shutdown_failed="Herunterfahren fehlgeschlagen", - connection_error="Verbindungsfehler: {error}", - socket_error="Socket-Fehler: {error}", + connection_error="Verbindungsfehler", + socket_error="Socket-Fehler", faillock_attempts_remaining="Noch {n} Versuch(e) vor Kontosperrung!", faillock_locked="Konto ist möglicherweise gesperrt", ) @@ -76,8 +78,8 @@ _STRINGS_EN = Strings( session_start_failed="Failed to start session", reboot_failed="Reboot failed", shutdown_failed="Shutdown failed", - connection_error="Connection error: {error}", - socket_error="Socket error: {error}", + connection_error="Connection error", + socket_error="Socket error", faillock_attempts_remaining="{n} attempt(s) remaining before lockout!", faillock_locked="Account may be locked", ) @@ -102,7 +104,9 @@ def detect_locale(locale_conf_path: Path = DEFAULT_LOCALE_CONF) -> str: return "en" # Extract language prefix: "de_DE.UTF-8" → "de" - lang = lang.split("_")[0].split(".")[0] + lang = lang.split("_")[0].split(".")[0].lower() + if not lang.isalpha(): + return "en" return lang diff --git a/src/moongreet/power.py b/src/moongreet/power.py index 84f672e..6d6b3c2 100644 --- a/src/moongreet/power.py +++ b/src/moongreet/power.py @@ -4,11 +4,14 @@ import subprocess +POWER_TIMEOUT = 30 + + def reboot() -> None: """Reboot the system via loginctl.""" - subprocess.run(["loginctl", "reboot"], check=True) + subprocess.run(["loginctl", "reboot"], check=True, timeout=POWER_TIMEOUT) def shutdown() -> None: """Shut down the system via loginctl.""" - subprocess.run(["loginctl", "poweroff"], check=True) + subprocess.run(["loginctl", "poweroff"], check=True, timeout=POWER_TIMEOUT) diff --git a/src/moongreet/sessions.py b/src/moongreet/sessions.py index a5af249..fca1dad 100644 --- a/src/moongreet/sessions.py +++ b/src/moongreet/sessions.py @@ -2,6 +2,7 @@ # ABOUTME: Parses .desktop files from standard session directories. import configparser +from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path @@ -37,8 +38,8 @@ def _parse_desktop_file(path: Path, session_type: str) -> Session | None: def get_sessions( - wayland_dirs: tuple[Path, ...] = DEFAULT_WAYLAND_DIRS, - xsession_dirs: tuple[Path, ...] = DEFAULT_XSESSION_DIRS, + wayland_dirs: Sequence[Path] = DEFAULT_WAYLAND_DIRS, + xsession_dirs: Sequence[Path] = DEFAULT_XSESSION_DIRS, ) -> list[Session]: """Discover available sessions from .desktop files.""" sessions: list[Session] = [] diff --git a/tests/test_config.py b/tests/test_config.py index d9b92ae..53c673e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -35,6 +35,14 @@ class TestLoadConfig: assert config.background is None + def test_returns_defaults_for_corrupt_toml(self, tmp_path: Path) -> None: + toml_file = tmp_path / "moongreet.toml" + toml_file.write_text("this is not valid [[[ toml !!!") + + config = load_config(toml_file) + + assert config.background is None + def test_resolves_relative_path_against_config_dir(self, tmp_path: Path) -> None: toml_file = tmp_path / "moongreet.toml" toml_file.write_text( diff --git a/tests/test_i18n.py b/tests/test_i18n.py index 39afe14..34c0d4e 100644 --- a/tests/test_i18n.py +++ b/tests/test_i18n.py @@ -63,6 +63,13 @@ class TestDetectLocale: assert result == "en" + def test_rejects_non_alpha_lang(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("LANG", "../../etc") + + result = detect_locale() + + assert result == "en" + class TestLoadStrings: """Tests for loading the correct string table.""" @@ -111,8 +118,9 @@ class TestLoadStrings: result = strings.faillock_attempts_remaining.format(n=1) assert "1" in result - def test_connection_error_template(self) -> None: + def test_connection_error_is_generic(self) -> None: strings = load_strings("en") - result = strings.connection_error.format(error="timeout") - assert "timeout" in result + # Error messages should not contain format placeholders (no info leakage) + assert "{" not in strings.connection_error + assert "{" not in strings.socket_error diff --git a/tests/test_integration.py b/tests/test_integration.py index 75fec69..a824e22 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -40,16 +40,27 @@ class MockGreetd: self._thread = threading.Thread(target=self._serve, daemon=True) self._thread.start() + @staticmethod + def _recvall(conn: socket.socket, n: int) -> bytes: + """Receive exactly n bytes from a socket, handling fragmented reads.""" + buf = bytearray() + while len(buf) < n: + chunk = conn.recv(n - len(buf)) + if not chunk: + break + buf.extend(chunk) + return bytes(buf) + def _serve(self) -> None: conn, _ = self._server.accept() try: for response in self._responses: # Receive a message - header = conn.recv(4) + header = self._recvall(conn, 4) if len(header) < 4: break length = struct.unpack("!I", header)[0] - payload = conn.recv(length) + payload = self._recvall(conn, length) msg = json.loads(payload.decode("utf-8")) self._received.append(msg) @@ -182,6 +193,10 @@ class TestLoginFlow: class TestFaillockWarning: """Tests for the faillock warning message logic.""" + def test_no_warning_on_zero_attempts(self) -> None: + strings = load_strings("de") + assert faillock_warning(0, strings) is None + def test_no_warning_on_first_attempt(self) -> None: strings = load_strings("de") assert faillock_warning(1, strings) is None @@ -231,3 +246,21 @@ class TestLastUser: from moongreet.greeter import GreeterWindow result = GreeterWindow._load_last_user() assert result is None + + def test_load_last_user_rejects_oversized_username(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + cache_path = tmp_path / "last-user" + cache_path.write_text("a" * 300) + monkeypatch.setattr("moongreet.greeter.LAST_USER_PATH", cache_path) + + from moongreet.greeter import GreeterWindow + result = GreeterWindow._load_last_user() + assert result is None + + def test_load_last_user_rejects_invalid_characters(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + cache_path = tmp_path / "last-user" + cache_path.write_text("../../etc/passwd") + monkeypatch.setattr("moongreet.greeter.LAST_USER_PATH", cache_path) + + from moongreet.greeter import GreeterWindow + result = GreeterWindow._load_last_user() + assert result is None diff --git a/tests/test_power.py b/tests/test_power.py index 2adae3d..0bb0b3b 100644 --- a/tests/test_power.py +++ b/tests/test_power.py @@ -6,7 +6,7 @@ from unittest.mock import patch, call import pytest -from moongreet.power import reboot, shutdown +from moongreet.power import reboot, shutdown, POWER_TIMEOUT class TestReboot: @@ -17,7 +17,7 @@ class TestReboot: reboot() mock_run.assert_called_once_with( - ["loginctl", "reboot"], check=True + ["loginctl", "reboot"], check=True, timeout=POWER_TIMEOUT ) @patch("moongreet.power.subprocess.run") @@ -36,7 +36,7 @@ class TestShutdown: shutdown() mock_run.assert_called_once_with( - ["loginctl", "poweroff"], check=True + ["loginctl", "poweroff"], check=True, timeout=POWER_TIMEOUT ) @patch("moongreet.power.subprocess.run")