From d5a143ee367329d9de8bbe9f6421615c82144788 Mon Sep 17 00:00:00 2001 From: phg Date: Fri, 4 Sep 2026 20:02:46 +0200 Subject: [PATCH] feat: Add sudo authentication screen and update authorization flow - Introduced a new module for terminal presentation during sudo authentication. - Updated the application to display a temporary authentication notice. - Enhanced tests to cover the new sudo authentication flow and its edge cases. --- README.md | 7 +- docs/user-guide.md | 9 +-- src/hosts/tui/app.py | 12 ++-- src/hosts/tui/privilege_prompt.py | 80 +++++++++++++++++++++ tests/test_app.py | 113 ++++++++++++++++++++++++++++-- 5 files changed, 206 insertions(+), 15 deletions(-) create mode 100644 src/hosts/tui/privilege_prompt.py diff --git a/README.md b/README.md index 68a7e35..db9a7a4 100644 --- a/README.md +++ b/README.md @@ -49,9 +49,10 @@ uvx git+https://git.s1q.dev/phg/hosts.git > limitations before granting sudo access. Press `Ctrl+E` to enter Privileged Mode. The application uses an existing sudo -authorization or temporarily returns control of the terminal to `sudo` and its -configured PAM authentication method. It then creates a Pre-edit Backup before -enabling changes. `hosts` never collects or handles authentication credentials. +authorization or displays a temporary authentication notice before returning +control of the terminal to `sudo` and its configured PAM authentication method. +It then creates a Pre-edit Backup before enabling changes. `hosts` never +collects or handles authentication credentials. Start with the [user guide](docs/user-guide.md) for the complete workflow, including persistence behavior and manual recovery. diff --git a/docs/user-guide.md b/docs/user-guide.md index 32ebf13..45edd28 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -186,10 +186,11 @@ reachable through the TUI. Editing them manually is not a supported workflow. ### Privileged Mode needs authentication -This is expected when no cached sudo authorization is available. `hosts` never -collects credentials; `sudo` and PAM present any configured authentication -method, such as Touch ID or a password prompt. Canceling authentication leaves -the application in Read-only Mode. +This is expected when no cached sudo authorization is available. `hosts` first +shows a temporary authentication notice, then `sudo` and PAM present any +configured authentication method, such as Touch ID or a password prompt. +`hosts` never collects credentials. Canceling authentication leaves the +application in Read-only Mode. ### Privileged Mode cannot be enabled diff --git a/src/hosts/tui/app.py b/src/hosts/tui/app.py index 7656d60..f25de59 100644 --- a/src/hosts/tui/app.py +++ b/src/hosts/tui/app.py @@ -39,6 +39,7 @@ from .table_handler import TableHandler from .details_handler import DetailsHandler from .edit_handler import EditHandler from .navigation_handler import NavigationHandler +from .privilege_prompt import sudo_authentication_screen @dataclass @@ -722,15 +723,18 @@ class HostsManagerApp(App): interrupted = False try: with self.suspend(): - try: - success, message = self.manager.authorize_interactively() - except KeyboardInterrupt: - interrupted = True + with sudo_authentication_screen(): + try: + success, message = self.manager.authorize_interactively() + except KeyboardInterrupt: + interrupted = True except SuspendNotSupported: self.update_status( "Interactive authorization requires terminal suspension; remaining in Read-only Mode." ) return + except KeyboardInterrupt: + interrupted = True if interrupted: self.update_status( diff --git a/src/hosts/tui/privilege_prompt.py b/src/hosts/tui/privilege_prompt.py new file mode 100644 index 0000000..8d13237 --- /dev/null +++ b/src/hosts/tui/privilege_prompt.py @@ -0,0 +1,80 @@ +"""Terminal presentation for foreground sudo authentication.""" + +from collections.abc import Iterator +from contextlib import contextmanager +import os +from shutil import get_terminal_size +import signal +import sys +from typing import TextIO + + +_ALTERNATE_SCREEN_ENTER = "\x1b[?1049h" +_ALTERNATE_SCREEN_EXIT = "\x1b[?1049l" +_AMBER_BOLD = "\x1b[1;38;5;214m" +_RESET = "\x1b[0m" +_FRAMED_NOTICE_MIN_WIDTH = 52 + + +def render_sudo_authentication_notice(*, width: int, color: bool) -> str: + """Return the sudo handoff notice suited to the available terminal width.""" + instructions = ( + "\n" + "Follow the sudo authentication prompt below.\n" + "hosts does not collect or store your credentials.\n" + "Press Ctrl+C to cancel and remain in Read-only Mode.\n" + ) + + if width < _FRAMED_NOTICE_MIN_WIDTH: + heading = "PRIVILEGED MODE REQUESTED\nsudo authentication required\n" + else: + heading = ( + "+--------------------------------------------------+\n" + "| PRIVILEGED MODE REQUESTED |\n" + "| sudo authentication required |\n" + "+--------------------------------------------------+\n" + ) + + if color: + heading = f"{_AMBER_BOLD}{heading}{_RESET}" + return f"{heading}{instructions}" + + +def _should_use_color(stream: TextIO) -> bool: + """Respect terminal and NO_COLOR conventions for the handoff notice.""" + return stream.isatty() and "NO_COLOR" not in os.environ + + +@contextmanager +def sudo_authentication_screen( + *, + stream: TextIO | None = None, + width: int | None = None, +) -> Iterator[None]: + """Isolate a foreground sudo/PAM conversation from the shell buffer. + + Terminals that support the alternate-screen sequence hide the original + shell viewport and scrollback for the duration of the conversation. If a + terminal ignores that sequence, the notice is simply written to the + existing terminal as the agreed graceful fallback. + """ + output = stream if stream is not None else sys.stdout + columns = width if width is not None else get_terminal_size().columns + previous_sigint_handler = signal.getsignal(signal.SIGINT) + signal.signal(signal.SIGINT, signal.default_int_handler) + try: + output.write(_ALTERNATE_SCREEN_ENTER) + output.write( + render_sudo_authentication_notice( + width=columns, + color=_should_use_color(output), + ) + ) + output.flush() + try: + yield + finally: + output.write(f"{_RESET}{_ALTERNATE_SCREEN_EXIT}") + output.flush() + finally: + signal.signal(signal.SIGINT, previous_sigint_handler) diff --git a/tests/test_app.py b/tests/test_app.py index d717165..43494e4 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,6 +1,9 @@ """Tests for application-level authorization flow.""" from contextlib import contextmanager +from io import StringIO +import os +import signal from unittest.mock import Mock, patch import pytest @@ -13,6 +16,10 @@ from src.hosts.tui.app import HostsManagerApp from src.hosts.tui.custom_footer import CustomFooter from src.hosts.tui.filter_modal import FilterModal from src.hosts.tui.help_modal import HelpModal +from src.hosts.tui.privilege_prompt import ( + render_sudo_authentication_notice, + sudo_authentication_screen, +) @contextmanager @@ -242,6 +249,53 @@ async def test_small_viewport_blocks_hidden_mutation_shortcuts(): class TestPrivilegedModeAuthorization: """Test the user-visible privileged-mode authorization flow.""" + def test_sudo_notice_has_a_framed_and_a_narrow_terminal_variant(self): + """The notice keeps its safety language when there is no room for a frame.""" + framed = render_sudo_authentication_notice(width=80, color=False) + narrow = render_sudo_authentication_notice(width=40, color=False) + + for notice in (framed, narrow): + assert "PRIVILEGED MODE REQUESTED" in notice + assert "sudo authentication required" in notice + assert "does not collect or store your credentials" in notice + assert "Ctrl+C to cancel and remain in Read-only Mode" in notice + + assert "+--------------------------------------------------+" in framed + assert "+--------------------------------------------------+" not in narrow + + def test_sudo_notice_uses_the_alternate_screen_for_the_entire_conversation(self): + """The shell viewport is restored after sudo/PAM is finished.""" + output = StringIO() + + with sudo_authentication_screen(stream=output, width=80): + output.write("sudo output\n") + + rendered = output.getvalue() + assert rendered.startswith("\x1b[?1049h") + assert "PRIVILEGED MODE REQUESTED" in rendered + assert "sudo output" in rendered + assert rendered.endswith("\x1b[0m\x1b[?1049l") + + def test_sudo_screen_turns_sigint_into_a_catchable_keyboard_interrupt(self): + """Asyncio's SIGINT handler must not cancel the Textual main task.""" + output = StringIO() + previous_handler = signal.getsignal(signal.SIGINT) + runner_handler_called = False + + def runner_handler(_signum, _frame): + nonlocal runner_handler_called + runner_handler_called = True + + signal.signal(signal.SIGINT, runner_handler) + try: + with pytest.raises(KeyboardInterrupt): + with sudo_authentication_screen(stream=output, width=80): + os.kill(os.getpid(), signal.SIGINT) + finally: + signal.signal(signal.SIGINT, previous_handler) + + assert not runner_handler_called + def test_interactive_authorization_suspends_and_restores_the_tui(self): """Test foreground PAM authorization runs while Textual is suspended.""" events = [] @@ -252,13 +306,26 @@ class TestPrivilegedModeAuthorization: yield events.append("restored") - def authorize_interactively(): + @contextmanager + def authentication_screen(): assert events == ["suspended"] + events.append("notice shown") + yield + events.append("notice dismissed") + + def authorize_interactively(): + assert events == ["suspended", "notice shown"] events.append("authorized") return True, "Sudo access granted" def finish_entering_edit_mode(): - assert events == ["suspended", "authorized", "restored"] + assert events == [ + "suspended", + "notice shown", + "authorized", + "notice dismissed", + "restored", + ] events.append("edit mode enabled") return True, "Edit mode enabled" @@ -273,14 +340,24 @@ class TestPrivilegedModeAuthorization: app.suspend = Mock(return_value=suspended_tui()) app.update_status = Mock() - app.action_toggle_edit_mode() + with patch( + "src.hosts.tui.app.sudo_authentication_screen", authentication_screen + ): + app.action_toggle_edit_mode() assert app.edit_mode app.suspend.assert_called_once_with() app.manager.enter_edit_mode.assert_called_once_with() app.manager.authorize_interactively.assert_called_once_with() app.manager.finish_entering_edit_mode.assert_called_once_with() - assert events == ["suspended", "authorized", "restored", "edit mode enabled"] + assert events == [ + "suspended", + "notice shown", + "authorized", + "notice dismissed", + "restored", + "edit mode enabled", + ] app.update_status.assert_called_once_with("Edit mode enabled") def test_rejected_authorization_keeps_the_app_read_only(self): @@ -333,6 +410,34 @@ class TestPrivilegedModeAuthorization: "Authorization was not granted; remaining in Read-only Mode." ) + def test_interrupt_while_dismissing_the_sudo_notice_keeps_the_app_running(self): + """An interrupt during terminal cleanup must not escape and quit the app.""" + app = HostsManagerApp() + app.manager = Mock() + app.manager.enter_edit_mode.return_value = ( + False, + "Interactive authorization required", + ) + app.manager.authorize_interactively.return_value = ( + False, + "Authorization was not granted", + ) + app.suspend = Mock(return_value=suspended_tui()) + app.update_status = Mock() + + @contextmanager + def interrupted_notice(): + yield + raise KeyboardInterrupt + + with patch("src.hosts.tui.app.sudo_authentication_screen", interrupted_notice): + app.action_toggle_edit_mode() + + assert not app.edit_mode + app.update_status.assert_called_once_with( + "Authorization was not granted; remaining in Read-only Mode." + ) + def test_unsupported_suspension_keeps_the_app_read_only(self): """Test the user receives an actionable suspension failure message.""" app = HostsManagerApp()