From 68a9696e7ddeb4f00ad82d96c1fd88e50e5b5794 Mon Sep 17 00:00:00 2001 From: phg Date: Thu, 3 Sep 2026 19:28:01 +0200 Subject: [PATCH] Stop invalidating sudo timestamp on mode exit --- CONTRIBUTING.md | 2 ++ README.md | 4 ++-- docs/user-guide.md | 7 ++---- src/hosts/core/manager.py | 19 ++++++----------- tests/test_app.py | 21 +++++++++++++++++- tests/test_manager.py | 45 +++++++++++++++++++-------------------- 6 files changed, 55 insertions(+), 43 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b8f9183..ef67bbf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -63,6 +63,8 @@ repository documents. - The application starts in Read-only Mode. - Mutating actions require Privileged Mode, and a Pre-edit Backup must exist before writes are enabled. +- Leaving Privileged Mode clears only application permission and undo/redo + session state; it must not alter sudo's cached timestamp. - Default Entries are protected from mutation and must remain first when file order changes. - All privileged writes go through `HostsManager`. diff --git a/README.md b/README.md index dde5bd0..50c4444 100644 --- a/README.md +++ b/README.md @@ -83,8 +83,8 @@ sorting, DNS refresh, movement, undo, and redo. unchanged. - Pre-edit Backups are not listed or restored by the TUI and have no retention management. Manual recovery is documented in the user guide. -- Leaving Privileged Mode currently invalidates the user's cached sudo - timestamp. This is [tracked for removal](https://git.s1q.dev/phg/hosts/issues/4). +- Leaving Privileged Mode clears the application's session state but does not + alter the user's cached sudo timestamp. - Serialization preserves Host Entries and comments semantically, but normalizes spacing, comment placement, and blank lines. It is not a byte-for-byte round trip. diff --git a/docs/user-guide.md b/docs/user-guide.md index fa17ced..1e33f6b 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -176,11 +176,8 @@ selection of the newest backup without inspecting it. Press `Ctrl+E` again. The application clears its undo/redo history and forgets which Pre-edit Backup belongs to the session. The backup file remains in the -temporary directory. - -Leaving Privileged Mode currently runs `sudo -k`, which invalidates your cached -sudo timestamp for other terminal sessions as well. This behavior is -[tracked for removal](https://git.s1q.dev/phg/hosts/issues/4). +temporary directory. It does not alter your cached sudo timestamp, including +authorization used by other terminal sessions. ## Configuration diff --git a/src/hosts/core/manager.py b/src/hosts/core/manager.py index db7383d..1057604 100644 --- a/src/hosts/core/manager.py +++ b/src/hosts/core/manager.py @@ -27,7 +27,7 @@ class PermissionManager: """ Manages sudo permissions for hosts file editing. - Handles requesting, validating, and releasing elevated permissions + Handles requesting, validating, and tracking the elevated permissions needed for modifying the system hosts file. """ @@ -101,15 +101,10 @@ class PermissionManager: except Exception: return False - def release_sudo(self) -> None: - """Release sudo permissions.""" - try: - subprocess.run(["sudo", "-k"], capture_output=True, timeout=5) - except Exception: - pass - finally: - self.has_sudo = False - self._sudo_validated = False + def clear_permission_state(self) -> None: + """Clear application permission state without changing sudo's timestamp.""" + self.has_sudo = False + self._sudo_validated = False class HostsManager: @@ -170,7 +165,7 @@ class HostsManager: def exit_edit_mode(self) -> Tuple[bool, str]: """ - Exit edit mode and release permissions. + Exit edit mode and clear application permission state. Returns: Tuple of (success, message) @@ -179,7 +174,7 @@ class HostsManager: return True, "Already in read-only mode" try: - self.permission_manager.release_sudo() + self.permission_manager.clear_permission_state() self.edit_mode = False self._backup_path = None self.undo_redo_history.clear_history() # Clear undo/redo history when exiting edit mode diff --git a/tests/test_app.py b/tests/test_app.py index 57710fb..3e74142 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,7 +1,7 @@ """Tests for application-level authorization flow.""" from contextlib import contextmanager -from unittest.mock import Mock +from unittest.mock import Mock, patch from textual.app import SuspendNotSupported @@ -125,3 +125,22 @@ class TestPrivilegedModeAuthorization: app.update_status.assert_called_once_with( "Interactive authorization requires terminal suspension; remaining in Read-only Mode." ) + + @patch("src.hosts.core.manager.subprocess.run") + def test_leaving_privileged_mode_preserves_the_sudo_timestamp(self, mock_run): + """Test Ctrl+E clears app state without invalidating sudo globally.""" + app = HostsManagerApp() + app.edit_mode = True + app.manager.edit_mode = True + app.manager.permission_manager.has_sudo = True + app.manager.permission_manager._sudo_validated = True + app.update_status = Mock() + + app.action_toggle_edit_mode() + + assert not app.edit_mode + assert not app.manager.edit_mode + assert not app.manager.permission_manager.has_sudo + assert not app.manager.permission_manager._sudo_validated + mock_run.assert_not_called() + app.update_status.assert_called_once_with("Edit mode disabled") diff --git a/tests/test_manager.py b/tests/test_manager.py index 5eefa79..2f35464 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -12,6 +12,7 @@ from pathlib import Path from unittest.mock import Mock, patch from src.hosts.core.manager import PermissionManager, HostsManager from src.hosts.core.models import HostEntry, HostsFile +from src.hosts.core.commands import ToggleEntryCommand class TestPermissionManager: @@ -183,32 +184,17 @@ class TestPermissionManager: assert not result @patch("subprocess.run") - def test_release_sudo(self, mock_run): - """Test releasing sudo permissions.""" + def test_clear_permission_state_preserves_global_sudo_timestamp(self, mock_run): + """Test exiting Privileged Mode preserves the global sudo timestamp.""" pm = PermissionManager() pm.has_sudo = True pm._sudo_validated = True - pm.release_sudo() + pm.clear_permission_state() assert not pm.has_sudo assert not pm._sudo_validated - mock_run.assert_called_once_with(["sudo", "-k"], capture_output=True, timeout=5) - - @patch("subprocess.run") - def test_release_sudo_exception(self, mock_run): - """Test releasing sudo with exception.""" - mock_run.side_effect = Exception("Test error") - - pm = PermissionManager() - pm.has_sudo = True - pm._sudo_validated = True - - pm.release_sudo() - - # Should still reset state even if command fails - assert not pm.has_sudo - assert not pm._sudo_validated + mock_run.assert_not_called() class TestHostsManager: @@ -306,14 +292,25 @@ class TestHostsManager: assert not manager.edit_mode def test_exit_edit_mode_success(self): - """Test exiting edit mode successfully.""" + """Test exit clears the Privileged Mode session state.""" with tempfile.NamedTemporaryFile() as temp_file: manager = HostsManager(temp_file.name) manager.edit_mode = True manager._backup_path = Path("/tmp/backup") + hosts_file = HostsFile( + entries=[ + HostEntry("192.0.2.1", ["example.test"]), + HostEntry("192.0.2.2", ["example-two.test"]), + ] + ) + manager.undo_redo_history.execute_command(ToggleEntryCommand(0), hosts_file) + manager.undo_redo_history.execute_command(ToggleEntryCommand(1), hosts_file) + manager.undo_redo_history.undo(hosts_file) + assert manager.can_undo() + assert manager.can_redo() # Mock permission manager - manager.permission_manager.release_sudo = Mock() + manager.permission_manager.clear_permission_state = Mock() success, message = manager.exit_edit_mode() @@ -321,7 +318,9 @@ class TestHostsManager: assert "disabled" in message assert not manager.edit_mode assert manager._backup_path is None - manager.permission_manager.release_sudo.assert_called_once() + assert not manager.can_undo() + assert not manager.can_redo() + manager.permission_manager.clear_permission_state.assert_called_once() def test_exit_edit_mode_not_in_edit(self): """Test exiting edit mode when not in edit mode.""" @@ -341,7 +340,7 @@ class TestHostsManager: manager.edit_mode = True # Mock permission manager to raise exception - manager.permission_manager.release_sudo = Mock( + manager.permission_manager.clear_permission_state = Mock( side_effect=Exception("Test error") )