Stop invalidating sudo timestamp on mode exit
This commit is contained in:
parent
026bbc4eff
commit
68a9696e7d
6 changed files with 55 additions and 43 deletions
|
|
@ -63,6 +63,8 @@ repository documents.
|
||||||
- The application starts in Read-only Mode.
|
- The application starts in Read-only Mode.
|
||||||
- Mutating actions require Privileged Mode, and a Pre-edit Backup must exist
|
- Mutating actions require Privileged Mode, and a Pre-edit Backup must exist
|
||||||
before writes are enabled.
|
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
|
- Default Entries are protected from mutation and must remain first when file
|
||||||
order changes.
|
order changes.
|
||||||
- All privileged writes go through `HostsManager`.
|
- All privileged writes go through `HostsManager`.
|
||||||
|
|
|
||||||
|
|
@ -83,8 +83,8 @@ sorting, DNS refresh, movement, undo, and redo.
|
||||||
unchanged.
|
unchanged.
|
||||||
- Pre-edit Backups are not listed or restored by the TUI and have no retention
|
- Pre-edit Backups are not listed or restored by the TUI and have no retention
|
||||||
management. Manual recovery is documented in the user guide.
|
management. Manual recovery is documented in the user guide.
|
||||||
- Leaving Privileged Mode currently invalidates the user's cached sudo
|
- Leaving Privileged Mode clears the application's session state but does not
|
||||||
timestamp. This is [tracked for removal](https://git.s1q.dev/phg/hosts/issues/4).
|
alter the user's cached sudo timestamp.
|
||||||
- Serialization preserves Host Entries and comments semantically, but
|
- Serialization preserves Host Entries and comments semantically, but
|
||||||
normalizes spacing, comment placement, and blank lines. It is not a
|
normalizes spacing, comment placement, and blank lines. It is not a
|
||||||
byte-for-byte round trip.
|
byte-for-byte round trip.
|
||||||
|
|
|
||||||
|
|
@ -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
|
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
|
which Pre-edit Backup belongs to the session. The backup file remains in the
|
||||||
temporary directory.
|
temporary directory. It does not alter your cached sudo timestamp, including
|
||||||
|
authorization used by other terminal sessions.
|
||||||
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).
|
|
||||||
|
|
||||||
## Configuration
|
## Configuration
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -27,7 +27,7 @@ class PermissionManager:
|
||||||
"""
|
"""
|
||||||
Manages sudo permissions for hosts file editing.
|
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.
|
needed for modifying the system hosts file.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
@ -101,15 +101,10 @@ class PermissionManager:
|
||||||
except Exception:
|
except Exception:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
def release_sudo(self) -> None:
|
def clear_permission_state(self) -> None:
|
||||||
"""Release sudo permissions."""
|
"""Clear application permission state without changing sudo's timestamp."""
|
||||||
try:
|
self.has_sudo = False
|
||||||
subprocess.run(["sudo", "-k"], capture_output=True, timeout=5)
|
self._sudo_validated = False
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
finally:
|
|
||||||
self.has_sudo = False
|
|
||||||
self._sudo_validated = False
|
|
||||||
|
|
||||||
|
|
||||||
class HostsManager:
|
class HostsManager:
|
||||||
|
|
@ -170,7 +165,7 @@ class HostsManager:
|
||||||
|
|
||||||
def exit_edit_mode(self) -> Tuple[bool, str]:
|
def exit_edit_mode(self) -> Tuple[bool, str]:
|
||||||
"""
|
"""
|
||||||
Exit edit mode and release permissions.
|
Exit edit mode and clear application permission state.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Tuple of (success, message)
|
Tuple of (success, message)
|
||||||
|
|
@ -179,7 +174,7 @@ class HostsManager:
|
||||||
return True, "Already in read-only mode"
|
return True, "Already in read-only mode"
|
||||||
|
|
||||||
try:
|
try:
|
||||||
self.permission_manager.release_sudo()
|
self.permission_manager.clear_permission_state()
|
||||||
self.edit_mode = False
|
self.edit_mode = False
|
||||||
self._backup_path = None
|
self._backup_path = None
|
||||||
self.undo_redo_history.clear_history() # Clear undo/redo history when exiting edit mode
|
self.undo_redo_history.clear_history() # Clear undo/redo history when exiting edit mode
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
"""Tests for application-level authorization flow."""
|
"""Tests for application-level authorization flow."""
|
||||||
|
|
||||||
from contextlib import contextmanager
|
from contextlib import contextmanager
|
||||||
from unittest.mock import Mock
|
from unittest.mock import Mock, patch
|
||||||
|
|
||||||
from textual.app import SuspendNotSupported
|
from textual.app import SuspendNotSupported
|
||||||
|
|
||||||
|
|
@ -125,3 +125,22 @@ class TestPrivilegedModeAuthorization:
|
||||||
app.update_status.assert_called_once_with(
|
app.update_status.assert_called_once_with(
|
||||||
"Interactive authorization requires terminal suspension; remaining in Read-only Mode."
|
"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")
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,7 @@ from pathlib import Path
|
||||||
from unittest.mock import Mock, patch
|
from unittest.mock import Mock, patch
|
||||||
from src.hosts.core.manager import PermissionManager, HostsManager
|
from src.hosts.core.manager import PermissionManager, HostsManager
|
||||||
from src.hosts.core.models import HostEntry, HostsFile
|
from src.hosts.core.models import HostEntry, HostsFile
|
||||||
|
from src.hosts.core.commands import ToggleEntryCommand
|
||||||
|
|
||||||
|
|
||||||
class TestPermissionManager:
|
class TestPermissionManager:
|
||||||
|
|
@ -183,32 +184,17 @@ class TestPermissionManager:
|
||||||
assert not result
|
assert not result
|
||||||
|
|
||||||
@patch("subprocess.run")
|
@patch("subprocess.run")
|
||||||
def test_release_sudo(self, mock_run):
|
def test_clear_permission_state_preserves_global_sudo_timestamp(self, mock_run):
|
||||||
"""Test releasing sudo permissions."""
|
"""Test exiting Privileged Mode preserves the global sudo timestamp."""
|
||||||
pm = PermissionManager()
|
pm = PermissionManager()
|
||||||
pm.has_sudo = True
|
pm.has_sudo = True
|
||||||
pm._sudo_validated = True
|
pm._sudo_validated = True
|
||||||
|
|
||||||
pm.release_sudo()
|
pm.clear_permission_state()
|
||||||
|
|
||||||
assert not pm.has_sudo
|
assert not pm.has_sudo
|
||||||
assert not pm._sudo_validated
|
assert not pm._sudo_validated
|
||||||
mock_run.assert_called_once_with(["sudo", "-k"], capture_output=True, timeout=5)
|
mock_run.assert_not_called()
|
||||||
|
|
||||||
@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
|
|
||||||
|
|
||||||
|
|
||||||
class TestHostsManager:
|
class TestHostsManager:
|
||||||
|
|
@ -306,14 +292,25 @@ class TestHostsManager:
|
||||||
assert not manager.edit_mode
|
assert not manager.edit_mode
|
||||||
|
|
||||||
def test_exit_edit_mode_success(self):
|
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:
|
with tempfile.NamedTemporaryFile() as temp_file:
|
||||||
manager = HostsManager(temp_file.name)
|
manager = HostsManager(temp_file.name)
|
||||||
manager.edit_mode = True
|
manager.edit_mode = True
|
||||||
manager._backup_path = Path("/tmp/backup")
|
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
|
# Mock permission manager
|
||||||
manager.permission_manager.release_sudo = Mock()
|
manager.permission_manager.clear_permission_state = Mock()
|
||||||
|
|
||||||
success, message = manager.exit_edit_mode()
|
success, message = manager.exit_edit_mode()
|
||||||
|
|
||||||
|
|
@ -321,7 +318,9 @@ class TestHostsManager:
|
||||||
assert "disabled" in message
|
assert "disabled" in message
|
||||||
assert not manager.edit_mode
|
assert not manager.edit_mode
|
||||||
assert manager._backup_path is None
|
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):
|
def test_exit_edit_mode_not_in_edit(self):
|
||||||
"""Test exiting edit mode when not in edit mode."""
|
"""Test exiting edit mode when not in edit mode."""
|
||||||
|
|
@ -341,7 +340,7 @@ class TestHostsManager:
|
||||||
manager.edit_mode = True
|
manager.edit_mode = True
|
||||||
|
|
||||||
# Mock permission manager to raise exception
|
# Mock permission manager to raise exception
|
||||||
manager.permission_manager.release_sudo = Mock(
|
manager.permission_manager.clear_permission_state = Mock(
|
||||||
side_effect=Exception("Test error")
|
side_effect=Exception("Test error")
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue