From e268794564bf6d52b0cfa3271d314ae64f404cc0 Mon Sep 17 00:00:00 2001 From: phg Date: Fri, 4 Sep 2026 20:19:17 +0200 Subject: [PATCH] Fix #13: Expose Pre-edit Backup restoration Preserve full diff opcodes before grouping, route modal navigation to a two-axis viewport-sized scroller, and calculate side-by-side layout using terminal cell widths so tabbed Hosts File lines keep a stable divider. --- README.md | 6 +- docs/adr/0001-privileged-mode-gates-writes.md | 7 +- docs/user-guide.md | 35 +- src/hosts/core/manager.py | 50 ++- src/hosts/core/restore_preview.py | 178 +++++++++ src/hosts/tui/app.py | 62 +++ src/hosts/tui/backup_restore_modal.py | 358 ++++++++++++++++++ src/hosts/tui/help_modal.py | 2 +- src/hosts/tui/keybindings.py | 1 + tests/test_app.py | 223 ++++++++++- tests/test_backup_restore_modal.py | 70 ++++ tests/test_manager.py | 57 ++- tests/test_restore_preview.py | 73 ++++ 13 files changed, 1103 insertions(+), 19 deletions(-) create mode 100644 src/hosts/core/restore_preview.py create mode 100644 src/hosts/tui/backup_restore_modal.py create mode 100644 tests/test_backup_restore_modal.py create mode 100644 tests/test_restore_preview.py diff --git a/README.md b/README.md index db9a7a4..0dba2f3 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ on macOS and Linux. - Creates DNS Entries and manually refreshes their resolved IP addresses. - Gates every `/etc/hosts` mutation behind an explicit Privileged Mode. - Creates a timestamped Pre-edit Backup before Privileged Mode begins. +- Restores the current session's Pre-edit Backup after explicit confirmation. - Supports undo and redo during the current Privileged Mode session. The application starts in Read-only Mode. Default Entries cannot be edited, @@ -63,6 +64,7 @@ including persistence behavior and manual recovery. | --- | --- | | `Up` / `Down` | Select a Host Entry | | `Ctrl+E` | Enter or leave Privileged Mode | +| `b` | Review and restore the session Pre-edit Backup | | `n` | Add a Host Entry | | `e` | Open the selected Host Entry in the Entry Editor | | `d` | Delete the selected Host Entry | @@ -79,8 +81,8 @@ sorting, DNS refresh, movement, undo, and redo. ## Safety and known limitations - Reloading discards unsaved in-memory state. -- Pre-edit Backups are not listed or restored by the TUI and have no retention - management. Manual recovery is documented in the user guide. +- The TUI can restore only the Pre-edit Backup associated with the current + Privileged Mode session; it does not list old backups or manage retention. - 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 diff --git a/docs/adr/0001-privileged-mode-gates-writes.md b/docs/adr/0001-privileged-mode-gates-writes.md index acaede8..5b0f4d0 100644 --- a/docs/adr/0001-privileged-mode-gates-writes.md +++ b/docs/adr/0001-privileged-mode-gates-writes.md @@ -4,4 +4,9 @@ The application starts read-only and permits changes to `/etc/hosts` only after sudo access has been validated and a timestamped backup has been created. All privileged writes go through `HostsManager` during that mode. This makes the authorization boundary visible to the user and guarantees a pre-edit safety -snapshot; the temporary backup is not a user-facing recovery system. +snapshot. While Privileged Mode remains active, the TUI exposes that session's +exact Pre-edit Backup path and can restore it after explicit confirmation. The +restore confirmation identifies both the backup source and Hosts File target; +it presents an exact-file Git-style diff and rechecks both reviewed files before +restoring. On success the application reloads the Hosts File and clears +undo/redo history. diff --git a/docs/user-guide.md b/docs/user-guide.md index 45edd28..8352228 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -55,9 +55,8 @@ enabling mutations it verifies write access and creates a Pre-edit Backup. If authorization, permission validation, or backup creation fails, the application remains in Read-only Mode. -The footer shows `Edit` while Privileged Mode is active. This label refers to -Privileged Mode; it is separate from the Entry Editor used to change one Host -Entry. +The footer shows `PRIVILEGED` while Privileged Mode is active. This label is +separate from the Entry Editor used to change one Host Entry. ## Change a Host Entry @@ -127,11 +126,29 @@ If an immediate save fails, the application restores the complete pre-action in-memory state, visible selection, and undo/redo history. A later mutation therefore cannot accidentally persist the failed change. -### Locate a Pre-edit Backup +### Restore the session Pre-edit Backup Entering Privileged Mode creates one timestamped Pre-edit Backup below the -operating system's temporary directory. The application does not currently show -this path. Ask Python for the directory used on your system: +operating system's temporary directory. While that Privileged Mode session is +active, press `b` to open the restore review. It displays the exact backup +source path and `/etc/hosts` target path, then shows a Git-style diff of the +current Hosts File (before) and Pre-edit Backup (after restoration). At 120 or +more columns the diff is side by side; at smaller supported sizes it is a +unified diff. Use `[` and `]` to move between changed hunks, `f` to toggle the +full diff, and `r` to refresh it. + +Choose **Restore Hosts File** only when those paths are the ones you intend. +The application copies the displayed backup over the Hosts File, reloads the +on-disk contents into the interface, and clears undo/redo history. If the copy +fails, the current Hosts File and interface remain unchanged and the status +message names the failure. The application checks both displayed files again +when you confirm; if either changed, it refreshes the diff and requires a new +confirmation. Cancel leaves both files unchanged. + +The TUI does not list old backups, apply retention rules, or retain the session +association after leaving Privileged Mode. For recovery outside that session, +locate and restore a backup manually. Ask Python for the temporary directory on +your system: ```bash python3 -c 'import tempfile; print(tempfile.gettempdir() + "/hosts-manager-backups")' @@ -148,9 +165,10 @@ The application does not remove old Pre-edit Backups, apply retention rules, or record which file belongs to a later session. Do not choose a backup solely because it is the newest; inspect its timestamp and contents first. -### Restore manually +### Restore manually outside the session -Restoration is not available through the TUI. To restore manually: +To restore a backup that is no longer associated with an active Privileged Mode +session: 1. Leave the application or return it to Read-only Mode. 2. Locate and inspect the intended Pre-edit Backup. @@ -243,6 +261,7 @@ continually retrying it. | `Shift+R` | Refresh all DNS Entries | | `Ctrl+Z` / `Ctrl+Y` | Undo or redo and save the result | | `Ctrl+S` | Save the current in-memory state | +| `b` | Review session backup changes and confirm restoration | Within the Entry Editor, use `Tab` and `Shift+Tab` to move between fields and `Escape` to leave the form. If values changed, the application asks whether to diff --git a/src/hosts/core/manager.py b/src/hosts/core/manager.py index f244599..25a820a 100644 --- a/src/hosts/core/manager.py +++ b/src/hosts/core/manager.py @@ -8,12 +8,14 @@ and safe file modifications with backup and validation. import os import subprocess import tempfile +from uuid import uuid4 from copy import deepcopy from dataclasses import dataclass from pathlib import Path from typing import Optional, Tuple from .models import HostEntry, HostsFile from .parser import HostsParser +from .restore_preview import RestorePreviewError, fingerprint_file from .commands import ( UndoRedoHistory, ToggleEntryCommand, @@ -132,6 +134,11 @@ class HostsManager: self._backup_path: Optional[Path] = None self.undo_redo_history = UndoRedoHistory() + @property + def backup_path(self) -> Optional[Path]: + """Return the Pre-edit Backup associated with this Privileged Mode session.""" + return self._backup_path + def enter_edit_mode(self) -> Tuple[bool, str]: """ Enter edit mode with proper permission management. @@ -668,7 +675,11 @@ class HostsManager: except Exception as e: return False, f"Error saving hosts file: {e}" - def restore_backup(self) -> Tuple[bool, str]: + def restore_backup( + self, + expected_backup_digest: str | None = None, + expected_hosts_file_digest: str | None = None, + ) -> Tuple[bool, str]: """ Restore the hosts file from backup. @@ -681,19 +692,48 @@ class HostsManager: if not self._backup_path or not self._backup_path.exists(): return False, "No backup available" + try: + if ( + expected_backup_digest is not None + and fingerprint_file(self._backup_path, "Pre-edit Backup") + != expected_backup_digest + ): + return False, "Pre-edit Backup changed since review" + if ( + expected_hosts_file_digest is not None + and fingerprint_file(self.parser.file_path, "Hosts File") + != expected_hosts_file_digest + ): + return False, "Hosts File changed since review" + except RestorePreviewError as error: + return False, str(error) + + restore_path = self.parser.file_path.with_name( + f".{self.parser.file_path.name}.restore-{uuid4().hex}" + ) try: result = subprocess.run( - ["sudo", "cp", str(self._backup_path), str(self.parser.file_path)], + ["sudo", "cp", str(self._backup_path), str(restore_path)], capture_output=True, text=True, timeout=10, ) - if result.returncode == 0: - return True, "Backup restored successfully" - else: + if result.returncode != 0: return False, f"Failed to restore backup: {result.stderr}" + result = subprocess.run( + ["sudo", "mv", str(restore_path), str(self.parser.file_path)], + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode != 0: + return False, f"Failed to replace hosts file: {result.stderr}" + + self.undo_redo_history.clear_history() + return True, "Backup restored successfully" + except Exception as e: return False, f"Error restoring backup: {e}" diff --git a/src/hosts/core/restore_preview.py b/src/hosts/core/restore_preview.py new file mode 100644 index 0000000..33d3c99 --- /dev/null +++ b/src/hosts/core/restore_preview.py @@ -0,0 +1,178 @@ +"""Exact-file diffing and freshness checks for Pre-edit Backup restoration.""" + +from dataclasses import dataclass +from difflib import SequenceMatcher +from hashlib import sha256 +from itertools import zip_longest +from pathlib import Path +from collections.abc import Sequence + + +class RestorePreviewError(Exception): + """A file cannot safely be shown in a restore preview.""" + + +@dataclass(frozen=True) +class RestoreDiffRow: + """One aligned current-to-restored line pair.""" + + current_line_number: int | None + restored_line_number: int | None + current_text: str | None + restored_text: str | None + + +@dataclass(frozen=True) +class RestoreDiffHunk: + """A contiguous group of aligned diff rows.""" + + current_start: int + current_count: int + restored_start: int + restored_count: int + rows: tuple[RestoreDiffRow, ...] + + +@dataclass(frozen=True) +class RestorePreview: + """The reviewed source, target, and deterministic line-level diff.""" + + backup_path: Path + hosts_file_path: Path + backup_digest: str + hosts_file_digest: str + current_has_final_newline: bool + restored_has_final_newline: bool + changed_hunks: tuple[RestoreDiffHunk, ...] + full_hunks: tuple[RestoreDiffHunk, ...] + + @property + def has_changes(self) -> bool: + """Whether copying the Pre-edit Backup would alter the Hosts File.""" + return self.backup_digest != self.hosts_file_digest + + def matches_files(self) -> bool: + """Return whether both files still match the reviewed contents.""" + try: + return ( + fingerprint_file(self.backup_path, "Pre-edit Backup") + == self.backup_digest + and fingerprint_file(self.hosts_file_path, "Hosts File") + == self.hosts_file_digest + ) + except RestorePreviewError: + return False + + +def fingerprint_file(path: Path, label: str) -> str: + """Read and fingerprint a file, raising an actionable preview error.""" + try: + return sha256(path.read_bytes()).hexdigest() + except OSError as error: + raise RestorePreviewError(f"Cannot read {label} at {path}: {error}") from error + + +def create_restore_preview(backup_path: Path, hosts_file_path: Path) -> RestorePreview: + """Read both exact files and prepare changed and full line-level hunks.""" + backup_bytes = _read_bytes(backup_path, "Pre-edit Backup") + hosts_file_bytes = _read_bytes(hosts_file_path, "Hosts File") + backup_text = _decode(backup_bytes, backup_path, "Pre-edit Backup") + hosts_file_text = _decode(hosts_file_bytes, hosts_file_path, "Hosts File") + restored_lines = backup_text.splitlines() + current_lines = hosts_file_text.splitlines() + matcher = SequenceMatcher(None, current_lines, restored_lines, autojunk=False) + # get_grouped_opcodes() trims the cached opcode list in place. Preserve an + # immutable copy first so Full diff always retains the complete files. + opcodes = tuple(matcher.get_opcodes()) + + return RestorePreview( + backup_path=backup_path, + hosts_file_path=hosts_file_path, + backup_digest=sha256(backup_bytes).hexdigest(), + hosts_file_digest=sha256(hosts_file_bytes).hexdigest(), + current_has_final_newline=hosts_file_bytes.endswith(b"\n"), + restored_has_final_newline=backup_bytes.endswith(b"\n"), + changed_hunks=tuple( + _make_hunk(group, current_lines, restored_lines) + for group in matcher.get_grouped_opcodes(3) + ), + full_hunks=(_make_hunk(opcodes, current_lines, restored_lines),), + ) + + +def _read_bytes(path: Path, label: str) -> bytes: + try: + return path.read_bytes() + except OSError as error: + raise RestorePreviewError(f"Cannot read {label} at {path}: {error}") from error + + +def _decode(contents: bytes, path: Path, label: str) -> str: + try: + return contents.decode("utf-8") + except UnicodeDecodeError as error: + raise RestorePreviewError( + f"Cannot preview {label} at {path}: it is not valid UTF-8 text" + ) from error + + +def _make_hunk( + opcodes: Sequence[tuple[str, int, int, int, int]], + current_lines: list[str], + restored_lines: list[str], +) -> RestoreDiffHunk: + current_start = opcodes[0][1] + 1 if opcodes else 1 + restored_start = opcodes[0][3] + 1 if opcodes else 1 + current_end = opcodes[-1][2] if opcodes else 0 + restored_end = opcodes[-1][4] if opcodes else 0 + rows: list[RestoreDiffRow] = [] + for ( + tag, + current_start_index, + current_end_index, + restored_start_index, + restored_end_index, + ) in opcodes: + current_range = range(current_start_index, current_end_index) + restored_range = range(restored_start_index, restored_end_index) + if tag == "equal": + rows.extend( + RestoreDiffRow( + current_index + 1, + restored_index + 1, + current_lines[current_index], + restored_lines[restored_index], + ) + for current_index, restored_index in zip(current_range, restored_range) + ) + elif tag == "delete": + rows.extend( + RestoreDiffRow(index + 1, None, current_lines[index], None) + for index in current_range + ) + elif tag == "insert": + rows.extend( + RestoreDiffRow(None, index + 1, None, restored_lines[index]) + for index in restored_range + ) + else: + rows.extend( + RestoreDiffRow( + current_index + 1 if current_index is not None else None, + restored_index + 1 if restored_index is not None else None, + current_lines[current_index] if current_index is not None else None, + restored_lines[restored_index] + if restored_index is not None + else None, + ) + for current_index, restored_index in zip_longest( + current_range, restored_range + ) + ) + return RestoreDiffHunk( + current_start=current_start, + current_count=current_end - current_start + 1, + restored_start=restored_start, + restored_count=restored_end - restored_start + 1, + rows=tuple(rows), + ) diff --git a/src/hosts/tui/app.py b/src/hosts/tui/app.py index f25de59..237fb2e 100644 --- a/src/hosts/tui/app.py +++ b/src/hosts/tui/app.py @@ -5,6 +5,7 @@ This module contains the main application class that orchestrates all the handlers and provides the primary user interface. """ +import asyncio from dataclasses import dataclass from textual.app import App, ComposeResult, SuspendNotSupported @@ -25,10 +26,12 @@ from ..core.parser import HostsParser from ..core.models import HostEntry, HostsFile from ..core.config import Config from ..core.manager import HostsManager, MutationState +from ..core.restore_preview import RestorePreviewError, create_restore_preview from ..core.dns import DNSService from ..core.filters import EntryFilter, FilterOptions from .config_modal import ConfigModal from .add_entry_modal import AddEntryModal +from .backup_restore_modal import BackupRestoreModal from .delete_confirmation_modal import DeleteConfirmationModal from .filter_modal import FilterModal from .help_modal import HelpModal @@ -718,6 +721,65 @@ class HostsManagerApp(App): else: self.update_status(f"Error entering edit mode: {message}") + def action_restore_backup(self) -> None: + """Prepare a reviewed restore flow for the Pre-edit Backup session.""" + if not self._allow_mutation_action(): + return + if self.entry_edit_mode: + self.update_status( + "Finish or cancel the Entry Editor before restoring the Pre-edit Backup." + ) + return + backup_path = self.manager.backup_path + if not self.edit_mode or backup_path is None: + self.update_status( + "No session Pre-edit Backup is available. Enter Privileged Mode first." + ) + return + + hosts_file_path = self.manager.parser.file_path + + async def prepare_restore_preview() -> None: + try: + preview = await asyncio.to_thread( + create_restore_preview, backup_path, hosts_file_path + ) + except RestorePreviewError as error: + self.update_status(f"Cannot preview Pre-edit Backup: {error}") + return + + async def refresh_preview(): + return await asyncio.to_thread( + create_restore_preview, backup_path, hosts_file_path + ) + + modal = BackupRestoreModal(preview, refresh_preview) + + def handle_restore_confirmation(confirmed: bool | None) -> None: + if not confirmed: + return + + success, message = self.manager.restore_backup( + modal.preview.backup_digest, modal.preview.hosts_file_digest + ) + if success: + self.load_hosts_file() + self.update_status( + f"Pre-edit Backup restored from {backup_path} to {hosts_file_path}" + ) + else: + self.update_status( + "Failed to restore Pre-edit Backup from " + f"{backup_path} to {hosts_file_path}: {message}. " + "The Hosts File and interface are unchanged. " + "Check write access and try again." + ) + + self.push_screen(modal, handle_restore_confirmation) + + self.run_worker(prepare_restore_preview(), exclusive=False) + self.update_status("Preparing Pre-edit Backup diff…") + def _enter_edit_mode_interactively(self) -> None: """Run one foreground sudo/PAM conversation outside the TUI.""" interrupted = False diff --git a/src/hosts/tui/backup_restore_modal.py b/src/hosts/tui/backup_restore_modal.py new file mode 100644 index 0000000..ac812f9 --- /dev/null +++ b/src/hosts/tui/backup_restore_modal.py @@ -0,0 +1,358 @@ +"""Git-style review and confirmation for Pre-edit Backup restoration.""" + +from collections.abc import Awaitable, Callable +from difflib import SequenceMatcher + +from rich.cells import cell_len +from rich.text import Text +from textual.app import ComposeResult +from textual.binding import Binding +from textual.containers import Horizontal, ScrollableContainer, Vertical +from textual.screen import ModalScreen +from textual.widgets import Button, Static + +from ..core.restore_preview import RestoreDiffRow, RestorePreview, RestorePreviewError + + +class BackupRestoreModal(ModalScreen[bool]): + """Show exactly what restoring the session backup would change.""" + + BINDINGS = [ + Binding("escape", "cancel", "Cancel"), + Binding("r", "refresh", "Refresh preview"), + Binding("f", "toggle_full", "Toggle full diff"), + Binding("[", "previous_hunk", "Previous hunk"), + Binding("]", "next_hunk", "Next hunk"), + Binding("up", "scroll_up", show=False, priority=True), + Binding("down", "scroll_down", show=False, priority=True), + Binding("left", "scroll_left", show=False, priority=True), + Binding("right", "scroll_right", show=False, priority=True), + Binding("pageup", "page_up", show=False, priority=True), + Binding("pagedown", "page_down", show=False, priority=True), + Binding("home", "scroll_home", show=False, priority=True), + Binding("end", "scroll_end", show=False, priority=True), + ] + + CSS = """ + BackupRestoreModal { align: center middle; } + #backup-restore-container { + width: 1fr; height: 90%; max-height: 90%; margin: 0 2; + background: $surface; border: thick $error; padding: 1 2; + } + .backup-restore-title { text-align: center; text-style: bold; color: $error; } + .backup-restore-copy { margin-top: 1; color: $text-muted; } + .backup-restore-path { color: $primary; text-style: bold; } + #backup-restore-status { height: 1; margin-top: 1; color: $warning; } + #backup-diff-scroll { height: 1fr; margin-top: 1; border: round $primary; } + #backup-diff { width: auto; height: auto; } + #restore-button { margin-left: 1; } + """ + + def __init__( + self, + preview: RestorePreview, + refresh_preview: Callable[[], Awaitable[RestorePreview]], + ): + super().__init__() + self.preview = preview + self._refresh_preview = refresh_preview + self._show_full = False + self._hunk_index = 0 + self._hunk_offsets: list[int] = [] + + def compose(self) -> ComposeResult: + with Vertical(id="backup-restore-container"): + yield Static("Restore Pre-edit Backup", classes="backup-restore-title") + yield Static( + "Review the exact current Hosts File before replacing it with the session backup.", + classes="backup-restore-copy", + ) + yield Static(id="backup-current-path", classes="backup-restore-path") + yield Static(id="backup-restored-path", classes="backup-restore-path") + yield Static(id="backup-restore-status") + with ScrollableContainer(id="backup-diff-scroll"): + yield Static(id="backup-diff", expand=False, shrink=False) + with Horizontal(classes="button-row"): + yield Button("Cancel", id="cancel-button", variant="default") + yield Button("Restore Hosts File", id="restore-button", variant="error") + + def on_mount(self) -> None: + self.query_one("#cancel-button", Button).focus() + self._render_preview() + + def on_button_pressed(self, event: Button.Pressed) -> None: + if event.button.id == "restore-button": + self.action_confirm() + elif event.button.id == "cancel-button": + self.action_cancel() + + def action_refresh(self) -> None: + self.run_worker(self._refresh(), exclusive=True) + + def action_toggle_full(self) -> None: + self._show_full = not self._show_full + self._hunk_index = 0 + self._render_preview() + + def action_next_hunk(self) -> None: + self._jump_hunk(1) + + def action_previous_hunk(self) -> None: + self._jump_hunk(-1) + + def action_scroll_up(self) -> None: + self._scroller().scroll_up(animate=False) + + def action_scroll_down(self) -> None: + self._scroller().scroll_down(animate=False) + + def action_scroll_left(self) -> None: + self._scroller().scroll_left(animate=False) + + def action_scroll_right(self) -> None: + self._scroller().scroll_right(animate=False) + + def action_page_up(self) -> None: + scroller = self._scroller() + scroller.scroll_to(y=scroller.scroll_y - scroller.size.height, animate=False) + + def action_page_down(self) -> None: + scroller = self._scroller() + scroller.scroll_to(y=scroller.scroll_y + scroller.size.height, animate=False) + + def action_scroll_home(self) -> None: + self._scroller().scroll_to(x=0, y=0, animate=False) + + def action_scroll_end(self) -> None: + scroller = self._scroller() + scroller.scroll_to(y=scroller.max_scroll_y, animate=False) + + def action_confirm(self) -> None: + if self.preview.has_changes: + self.run_worker(self._confirm(), exclusive=True) + + def action_cancel(self) -> None: + self.dismiss(False) + + async def _refresh(self) -> None: + self._set_status("Refreshing exact-file diff…") + try: + self.preview = await self._refresh_preview() + except RestorePreviewError as error: + self._set_status(str(error)) + self.query_one("#restore-button", Button).disabled = True + return + self._hunk_index = 0 + self._render_preview() + self._set_status("Preview refreshed. Review it before restoring.") + + async def _confirm(self) -> None: + self._set_status("Checking the reviewed files…") + try: + refreshed = await self._refresh_preview() + except RestorePreviewError as error: + self._set_status(str(error)) + self.query_one("#restore-button", Button).disabled = True + return + if (refreshed.backup_digest, refreshed.hosts_file_digest) != ( + self.preview.backup_digest, + self.preview.hosts_file_digest, + ): + self.preview = refreshed + self._hunk_index = 0 + self._render_preview() + self._set_status( + "A reviewed file changed. The diff was refreshed; confirm Restore again." + ) + return + self.dismiss(True) + + def _render_preview(self) -> None: + self.query_one("#backup-current-path", Static).update( + f"Current Hosts File (before): {self.preview.hosts_file_path}" + ) + self.query_one("#backup-restored-path", Static).update( + f"Pre-edit Backup (after restore): {self.preview.backup_path}" + ) + restore_button = self.query_one("#restore-button", Button) + restore_button.disabled = not self.preview.has_changes + if not self.preview.has_changes: + self._set_status("Restoration would make no changes.") + elif self._show_full: + self._set_status("Full diff · f changed hunks · r refresh") + else: + self._set_status( + "Changed hunks · [ previous · ] next · f full diff · r refresh" + ) + self.query_one("#backup-diff", Static).update(self._render_diff()) + + def _render_diff(self) -> Text: + hunks = ( + self.preview.full_hunks if self._show_full else self.preview.changed_hunks + ) + if not hunks: + if self.preview.has_changes: + text = Text(no_wrap=True, overflow="ignore") + self._append_final_newline_markers(text) + return text + return Text( + "No changes between the current Hosts File and Pre-edit Backup." + ) + text = Text(no_wrap=True, overflow="ignore") + self._hunk_offsets = [] + offset = 0 + for index, hunk in enumerate(hunks): + self._hunk_offsets.append(offset) + if self.size.width >= 120: + text.append( + f"@@ current -{hunk.current_start},{hunk.current_count} " + f"restored +{hunk.restored_start},{hunk.restored_count} @@\n", + style="bold", + ) + offset += 1 + for row in hunk.rows: + self._append_side_by_side_row(text, row) + text.append("\n") + offset += 1 + else: + text.append( + f"@@ -{hunk.current_start},{hunk.current_count} " + f"+{hunk.restored_start},{hunk.restored_count} @@\n", + style="bold", + ) + offset += 1 + for row in hunk.rows: + self._append_unified_row(text, row) + text.append("\n") + offset += 2 if self._is_replacement(row) else 1 + if index < len(hunks) - 1: + text.append("\n") + offset += 1 + self._append_final_newline_markers(text) + return text + + def _append_side_by_side_row(self, text: Text, row: RestoreDiffRow) -> None: + width = self._side_width() + self._append_side_cell( + text, + row.current_line_number, + row.current_text, + row.restored_text, + "-", + width, + ) + text.append(" │ ", style="dim") + self._append_side_cell( + text, + row.restored_line_number, + row.restored_text, + row.current_text, + "+", + width, + ) + + def _append_side_cell( + self, + text: Text, + number: int | None, + value: str | None, + paired: str | None, + marker: str, + width: int, + ) -> None: + changed = value is not None and value != paired + prefix = f"{marker if changed else ' '} " + ( + f"{number:>4} " if number is not None else " " + ) + text.append(prefix, style="bold" if changed else "") + rendered = self._display_line(value) + paired_rendered = self._display_line(paired) if paired is not None else None + self._append_intraline_text(text, rendered, paired_rendered, changed) + text.append(" " * max(0, width - cell_len(prefix) - cell_len(rendered))) + + def _append_unified_row(self, text: Text, row: RestoreDiffRow) -> None: + if self._is_replacement(row): + text.append(f"- {row.current_line_number:>4} ", style="bold") + self._append_intraline_text( + text, + self._display_line(row.current_text), + self._display_line(row.restored_text), + True, + ) + text.append("\n+ " + f"{row.restored_line_number:>4} ", style="bold") + self._append_intraline_text( + text, + self._display_line(row.restored_text), + self._display_line(row.current_text), + True, + ) + elif row.current_text is not None: + marker = " " if row.restored_text == row.current_text else "-" + text.append( + f"{marker} {row.current_line_number:>4} " + f"{self._display_line(row.current_text)}", + style="bold" if marker == "-" else "", + ) + else: + text.append( + f"+ {row.restored_line_number:>4} " + f"{self._display_line(row.restored_text)}", + style="bold", + ) + + def _append_intraline_text( + self, text: Text, value: str, paired: str | None, changed: bool + ) -> None: + if not changed or paired is None or value == paired: + text.append(value) + return + for tag, start, end, _, _ in SequenceMatcher( + None, value, paired, autojunk=False + ).get_opcodes(): + text.append(value[start:end], style="bold" if tag != "equal" else "") + + def _append_final_newline_markers(self, text: Text) -> None: + if not self.preview.current_has_final_newline: + text.append("\\ No newline at end of current Hosts File\n", style="bold") + if not self.preview.restored_has_final_newline: + text.append( + "\\ No newline at end of restored Pre-edit Backup", style="bold" + ) + + def _side_width(self) -> int: + longest = max( + ( + cell_len(self._display_line(value)) + for hunk in self.preview.full_hunks + for row in hunk.rows + for value in (row.current_text, row.restored_text) + ), + default=0, + ) + return max(50, longest + 7) + + @staticmethod + def _display_line(value: str | None) -> str: + """Expand tabs before adding diff gutters so columns remain deterministic.""" + return (value or "").expandtabs(8) + + def _is_replacement(self, row: RestoreDiffRow) -> bool: + return ( + row.current_text is not None + and row.restored_text is not None + and row.current_text != row.restored_text + ) + + def _jump_hunk(self, direction: int) -> None: + if self._show_full or not self._hunk_offsets: + return + self._hunk_index = (self._hunk_index + direction) % len(self._hunk_offsets) + self._scroller().scroll_to( + y=self._hunk_offsets[self._hunk_index], animate=False + ) + + def _scroller(self) -> ScrollableContainer: + return self.query_one("#backup-diff-scroll", ScrollableContainer) + + def _set_status(self, message: str) -> None: + self.query_one("#backup-restore-status", Static).update(Text(message)) diff --git a/src/hosts/tui/help_modal.py b/src/hosts/tui/help_modal.py index 72dd7ab..13794f4 100644 --- a/src/hosts/tui/help_modal.py +++ b/src/hosts/tui/help_modal.py @@ -50,7 +50,7 @@ class HelpModal(ModalScreen[None]): ) yield Static("Privileged Mode", classes="help-section") yield Static( - "Ctrl+E Enter or leave Privileged Mode n New e Edit d Delete", + "Ctrl+E Enter or leave Privileged Mode b Review Pre-edit Backup", classes="help-copy", ) yield Button("Close", id="help-close", variant="primary") diff --git a/src/hosts/tui/keybindings.py b/src/hosts/tui/keybindings.py index 9d7dc51..4281a68 100644 --- a/src/hosts/tui/keybindings.py +++ b/src/hosts/tui/keybindings.py @@ -48,6 +48,7 @@ HOSTS_MANAGER_BINDINGS = [ Binding("i", "sort_by_ip", "Sort by IP address", show=False), Binding("h", "sort_by_hostname", "Sort by hostname", show=False), Binding("ctrl+s", "save_file", "Save hosts file", show=False), + Binding("b", "restore_backup", "Restore Pre-edit Backup", show=False), Binding("shift+up", "move_entry_up", "Move entry up", show=False), Binding("shift+down", "move_entry_down", "Move entry down", show=False), Binding("ctrl+z", "undo", "Undo", show=False, id="left:undo"), diff --git a/tests/test_app.py b/tests/test_app.py index 43494e4..3830615 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,19 +1,22 @@ """Tests for application-level authorization flow.""" from contextlib import contextmanager +import asyncio from io import StringIO import os import signal from unittest.mock import Mock, patch import pytest +from rich.cells import cell_len from textual.app import SuspendNotSupported -from textual.widgets import Input, RadioButton, Static +from textual.widgets import Button, Input, RadioButton, Static from src.hosts.core.filters import FilterOptions from src.hosts.core.models import HostEntry, HostsFile from src.hosts.tui.app import HostsManagerApp from src.hosts.tui.custom_footer import CustomFooter +from src.hosts.tui.backup_restore_modal import BackupRestoreModal from src.hosts.tui.filter_modal import FilterModal from src.hosts.tui.help_modal import HelpModal from src.hosts.tui.privilege_prompt import ( @@ -246,6 +249,147 @@ async def test_small_viewport_blocks_hidden_mutation_shortcuts(): app.manager.enter_edit_mode.assert_not_called() +@pytest.mark.asyncio +async def test_restore_opens_a_side_by_side_preview_with_exact_paths(tmp_path): + """The recovery workflow makes its before-and-after files reviewable.""" + app = app_with_filterable_entries() + backup_path = tmp_path / "hosts.backup" + hosts_file_path = tmp_path / "hosts" + backup_path.write_text("127.0.0.1 restored.test\n") + hosts_file_path.write_text("127.0.0.1 current.test\n") + app.edit_mode = True + app.manager = Mock() + app.manager.backup_path = backup_path + app.manager.parser.file_path = hosts_file_path + + async with app.run_test(size=(120, 40)) as pilot: + app.action_restore_backup() + await pilot.pause() + await pilot.pause() + + assert isinstance(app.screen, BackupRestoreModal) + assert str(backup_path) in str( + app.screen.query_one("#backup-restored-path", Static).render() + ) + assert str(hosts_file_path) in str( + app.screen.query_one("#backup-current-path", Static).render() + ) + diff = str(app.screen.query_one("#backup-diff", Static).render()) + assert "current.test" in diff + assert "restored.test" in diff + + +@pytest.mark.asyncio +async def test_restore_side_by_side_divider_uses_terminal_cell_widths(tmp_path): + """Tabs in Hosts File lines cannot move the right-hand file between rows.""" + app = app_with_filterable_entries() + backup_path = tmp_path / "hosts.backup" + hosts_file_path = tmp_path / "hosts" + hosts_file_path.write_text( + "127.0.0.1\tlocalhost\n" + "255.255.255.255\tbroadcasthost\n" + "::1\tlocalhost\n" + "192.0.2.10\tcurrent.test\n" + ) + backup_path.write_text( + "127.0.0.1\tlocalhost\n" + "255.255.255.255\tbroadcasthost\n" + "::1\tlocalhost\n" + "192.0.2.10\trestored.test\n" + ) + app.edit_mode = True + app.manager = Mock() + app.manager.backup_path = backup_path + app.manager.parser.file_path = hosts_file_path + + async with app.run_test(size=(160, 40)) as pilot: + app.action_restore_backup() + await pilot.pause() + await pilot.pause() + + modal = app.screen + assert isinstance(modal, BackupRestoreModal) + lines = [ + line for line in modal._render_diff().plain.splitlines() if " │ " in line + ] + divider_columns = { + cell_len(line[: line.index("│")].expandtabs(8)) for line in lines + } + + assert "\t" not in "\n".join(lines) + assert len(divider_columns) == 1 + + +@pytest.mark.asyncio +async def test_restore_preview_disables_a_no_change_operation(tmp_path): + """A no-op restore cannot perform an unnecessary privileged write.""" + app = app_with_filterable_entries() + backup_path = tmp_path / "hosts.backup" + hosts_file_path = tmp_path / "hosts" + contents = "127.0.0.1 localhost\n" + backup_path.write_text(contents) + hosts_file_path.write_text(contents) + app.edit_mode = True + app.manager = Mock() + app.manager.backup_path = backup_path + app.manager.parser.file_path = hosts_file_path + + async with app.run_test(size=(120, 40)) as pilot: + app.action_restore_backup() + await pilot.pause() + await pilot.pause() + + modal = app.screen + assert isinstance(modal, BackupRestoreModal) + assert modal.query_one("#restore-button", Button).disabled + assert "would make no changes" in str( + modal.query_one("#backup-restore-status", Static).render() + ) + + +@pytest.mark.asyncio +async def test_restore_preview_fills_width_and_routes_navigation_from_cancel(tmp_path): + """The safe initial focus does not prevent full mode or two-axis scrolling.""" + app = app_with_filterable_entries() + backup_path = tmp_path / "hosts.backup" + hosts_file_path = tmp_path / "hosts" + current_lines = [ + f"192.0.2.{index} host-{index}.test " + "x" * 120 for index in range(1, 81) + ] + restored_lines = current_lines.copy() + restored_lines[39] = "192.0.2.40 restored.test " + "y" * 120 + hosts_file_path.write_text("\n".join(current_lines) + "\n") + backup_path.write_text("\n".join(restored_lines) + "\n") + app.edit_mode = True + app.manager = Mock() + app.manager.backup_path = backup_path + app.manager.parser.file_path = hosts_file_path + + async with app.run_test(size=(160, 40)) as pilot: + app.action_restore_backup() + await pilot.pause() + await pilot.pause() + + modal = app.screen + assert isinstance(modal, BackupRestoreModal) + container = modal.query_one("#backup-restore-container") + assert container.outer_size.width == 156 + + changed_diff = str(modal.query_one("#backup-diff", Static).render()) + await pilot.press("f") + await pilot.pause() + full_diff = str(modal.query_one("#backup-diff", Static).render()) + assert full_diff != changed_diff + assert "host-1.test" in full_diff + + scroller = modal.query_one("#backup-diff-scroll") + assert app.focused is modal.query_one("#cancel-button") + await pilot.press("down", "right") + await pilot.pause() + assert scroller.scroll_y > 0 + assert scroller.scroll_x > 0 + + class TestPrivilegedModeAuthorization: """Test the user-visible privileged-mode authorization flow.""" @@ -474,3 +618,80 @@ class TestPrivilegedModeAuthorization: assert not app.manager.permission_manager._sudo_validated mock_run.assert_not_called() app.update_status.assert_called_once_with("Edit mode disabled") + + +class TestPreEditBackupRestoration: + """Test the user-reachable Pre-edit Backup recovery flow.""" + + def test_restore_action_confirms_paths_then_reloads_after_success(self, tmp_path): + app = HostsManagerApp() + app.edit_mode = True + app.manager = Mock() + backup_path = tmp_path / "hosts.backup.123" + hosts_file_path = tmp_path / "hosts" + backup_path.write_text("127.0.0.1 restored.test\n") + hosts_file_path.write_text("127.0.0.1 current.test\n") + app.manager.backup_path = backup_path + app.manager.parser.file_path = hosts_file_path + app.manager.restore_backup.return_value = (True, "Backup restored successfully") + app.push_screen = Mock() + app.load_hosts_file = Mock() + app.update_status = Mock() + + app.run_worker = Mock(side_effect=lambda coroutine, **_: asyncio.run(coroutine)) + app.action_restore_backup() + + modal, callback = app.push_screen.call_args.args + assert modal.preview.backup_path == backup_path + assert modal.preview.hosts_file_path == hosts_file_path + + callback(True) + + app.manager.restore_backup.assert_called_once_with( + modal.preview.backup_digest, modal.preview.hosts_file_digest + ) + app.load_hosts_file.assert_called_once_with() + assert app.update_status.call_args.args == ( + f"Pre-edit Backup restored from {backup_path} to {hosts_file_path}", + ) + + def test_failed_restore_keeps_interface_state_and_reports_paths(self, tmp_path): + app = HostsManagerApp() + app.edit_mode = True + app.manager = Mock() + backup_path = tmp_path / "hosts.backup.123" + hosts_file_path = tmp_path / "hosts" + backup_path.write_text("127.0.0.1 restored.test\n") + hosts_file_path.write_text("127.0.0.1 current.test\n") + app.manager.backup_path = backup_path + app.manager.parser.file_path = hosts_file_path + app.manager.restore_backup.return_value = (False, "Permission denied") + app.push_screen = Mock() + app.load_hosts_file = Mock() + app.update_status = Mock() + + app.run_worker = Mock(side_effect=lambda coroutine, **_: asyncio.run(coroutine)) + app.action_restore_backup() + _, callback = app.push_screen.call_args.args + callback(True) + + app.load_hosts_file.assert_not_called() + assert app.update_status.call_args.args == ( + f"Failed to restore Pre-edit Backup from {backup_path} to {hosts_file_path}: Permission denied. " + "The Hosts File and interface are unchanged. Check write access and try again.", + ) + + def test_restore_is_unavailable_while_the_entry_editor_has_unsaved_state(self): + app = HostsManagerApp() + app.entry_edit_mode = True + app.manager = Mock() + app.push_screen = Mock() + app.update_status = Mock() + + app.action_restore_backup() + + app.push_screen.assert_not_called() + app.manager.restore_backup.assert_not_called() + app.update_status.assert_called_once_with( + "Finish or cancel the Entry Editor before restoring the Pre-edit Backup." + ) diff --git a/tests/test_backup_restore_modal.py b/tests/test_backup_restore_modal.py new file mode 100644 index 0000000..a6e7fe2 --- /dev/null +++ b/tests/test_backup_restore_modal.py @@ -0,0 +1,70 @@ +"""Tests for the Pre-edit Backup restoration confirmation.""" + +import asyncio +from unittest.mock import Mock + +from textual.widgets import Button + +from hosts.tui.backup_restore_modal import BackupRestoreModal +from hosts.core.restore_preview import create_restore_preview + + +async def unchanged_preview(preview): + """Return the already reviewed files for modal unit tests.""" + return preview + + +class TestBackupRestoreModal: + """The modal identifies both paths and requires an explicit choice.""" + + def test_modal_exposes_backup_and_hosts_file_paths(self, tmp_path): + backup = tmp_path / "hosts.backup" + hosts_file = tmp_path / "hosts" + backup.write_text("127.0.0.1 localhost\n") + hosts_file.write_text("192.0.2.1 current.test\n") + preview = create_restore_preview(backup, hosts_file) + modal = BackupRestoreModal(preview, lambda: unchanged_preview(preview)) + + assert modal.preview.backup_path == backup + assert modal.preview.hosts_file_path == hosts_file + + def test_confirm_dismisses_true_after_freshness_check(self, tmp_path): + backup = tmp_path / "backup" + hosts_file = tmp_path / "hosts" + backup.write_text("127.0.0.1 restored.test\n") + hosts_file.write_text("127.0.0.1 current.test\n") + preview = create_restore_preview(backup, hosts_file) + modal = BackupRestoreModal(preview, lambda: unchanged_preview(preview)) + modal.dismiss = Mock() + modal._set_status = Mock() + + asyncio.run(modal._confirm()) + + modal.dismiss.assert_called_once_with(True) + + def test_cancel_is_focused_and_dismisses_false(self): + preview = Mock() + modal = BackupRestoreModal(preview, Mock()) + modal.dismiss = Mock() + cancel = Mock() + modal.query_one = Mock(return_value=cancel) + modal._render_preview = Mock() + + modal.on_mount() + modal.action_cancel() + + modal.query_one.assert_called_once_with("#cancel-button", Button) + cancel.focus.assert_called_once_with() + modal.dismiss.assert_called_once_with(False) + + def test_final_newline_difference_is_visible_without_changed_lines(self, tmp_path): + backup = tmp_path / "backup" + hosts_file = tmp_path / "hosts" + backup.write_bytes(b"127.0.0.1 localhost\n") + hosts_file.write_bytes(b"127.0.0.1 localhost") + preview = create_restore_preview(backup, hosts_file) + modal = BackupRestoreModal(preview, lambda: unchanged_preview(preview)) + + rendered = modal._render_diff() + + assert "No newline at end of current Hosts File" in str(rendered) diff --git a/tests/test_manager.py b/tests/test_manager.py index 9a1c91c..34f38c7 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -13,6 +13,7 @@ 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 +from src.hosts.core.restore_preview import fingerprint_file class TestPermissionManager: @@ -592,15 +593,69 @@ class TestHostsManager: manager._backup_path = Path(backup_file.name) try: + manager.undo_redo_history.clear_history = Mock() success, message = manager.restore_backup() assert success assert "restored successfully" in message - mock_run.assert_called_once() + assert mock_run.call_count == 2 + assert mock_run.call_args_list[0].args[0][:2] == ["sudo", "cp"] + assert mock_run.call_args_list[0].args[0][-1] != temp_file.name + assert mock_run.call_args_list[1].args[0][:2] == ["sudo", "mv"] + assert mock_run.call_args_list[1].args[0][-1] == temp_file.name + manager.undo_redo_history.clear_history.assert_called_once_with() finally: # Clean up manager._backup_path.unlink() + @patch("subprocess.run") + def test_restore_backup_copy_failure_leaves_hosts_file_target_untouched( + self, mock_run + ): + """A failed staged copy never opens the Hosts File target for writing.""" + mock_run.return_value = Mock(returncode=1, stderr="Disk full") + + with tempfile.NamedTemporaryFile() as temp_file: + manager = HostsManager(temp_file.name) + manager.edit_mode = True + with tempfile.NamedTemporaryFile(delete=False) as backup_file: + manager._backup_path = Path(backup_file.name) + + try: + success, message = manager.restore_backup() + + assert not success + assert "Disk full" in message + mock_run.assert_called_once() + assert mock_run.call_args.args[0][-1] != temp_file.name + finally: + manager._backup_path.unlink() + + @patch("subprocess.run") + def test_restore_backup_refuses_files_that_changed_since_preview(self, mock_run): + """The privileged write uses the exact source and target the user reviewed.""" + with tempfile.NamedTemporaryFile() as temp_file: + manager = HostsManager(temp_file.name) + manager.edit_mode = True + with tempfile.NamedTemporaryFile(delete=False) as backup_file: + backup_file.write(b"127.0.0.1 restored.test\n") + manager._backup_path = Path(backup_file.name) + + try: + backup_digest = fingerprint_file( + manager._backup_path, "Pre-edit Backup" + ) + hosts_digest = fingerprint_file(manager.parser.file_path, "Hosts File") + Path(temp_file.name).write_text("192.0.2.2 changed.test\n") + + success, message = manager.restore_backup(backup_digest, hosts_digest) + + assert not success + assert "changed since review" in message + mock_run.assert_not_called() + finally: + manager._backup_path.unlink() + def test_restore_backup_not_in_edit_mode(self): """Test restoring backup when not in edit mode.""" with tempfile.NamedTemporaryFile() as temp_file: diff --git a/tests/test_restore_preview.py b/tests/test_restore_preview.py new file mode 100644 index 0000000..27131ca --- /dev/null +++ b/tests/test_restore_preview.py @@ -0,0 +1,73 @@ +"""Tests for the exact-file preview shown before Pre-edit Backup restoration.""" + +import pytest + +from hosts.core.restore_preview import RestorePreviewError, create_restore_preview + + +def test_preview_pairs_current_and_restored_lines_in_changed_hunks(tmp_path): + """The current Hosts File is the left/before side of the preview.""" + backup = tmp_path / "hosts.backup" + hosts_file = tmp_path / "hosts" + backup.write_text("127.0.0.1 localhost\n192.0.2.1 restored.test\n") + hosts_file.write_text("127.0.0.1 localhost\n192.0.2.2 current.test\n") + + preview = create_restore_preview(backup, hosts_file) + + assert preview.has_changes + row = preview.changed_hunks[0].rows[1] + assert row.current_line_number == 2 + assert row.restored_line_number == 2 + assert row.current_text == "192.0.2.2 current.test" + assert row.restored_text == "192.0.2.1 restored.test" + + +def test_preview_distinguishes_missing_final_newlines(tmp_path): + backup = tmp_path / "hosts.backup" + hosts_file = tmp_path / "hosts" + backup.write_bytes(b"127.0.0.1 localhost\n") + hosts_file.write_bytes(b"127.0.0.1 localhost") + + preview = create_restore_preview(backup, hosts_file) + + assert preview.has_changes + assert preview.restored_has_final_newline + assert not preview.current_has_final_newline + + +def test_preview_rejects_files_that_cannot_be_decoded_as_hosts_text(tmp_path): + backup = tmp_path / "hosts.backup" + hosts_file = tmp_path / "hosts" + backup.write_bytes(b"\xff") + hosts_file.write_text("127.0.0.1 localhost\n") + + with pytest.raises(RestorePreviewError, match="Pre-edit Backup"): + create_restore_preview(backup, hosts_file) + + +def test_preview_detects_when_either_reviewed_file_changes(tmp_path): + backup = tmp_path / "hosts.backup" + hosts_file = tmp_path / "hosts" + backup.write_text("127.0.0.1 localhost\n") + hosts_file.write_text("192.0.2.2 current.test\n") + preview = create_restore_preview(backup, hosts_file) + + hosts_file.write_text("192.0.2.3 changed.test\n") + + assert not preview.matches_files() + + +def test_full_preview_keeps_lines_outside_changed_hunk_context(tmp_path): + """Full mode retains the complete files after changed hunks are calculated.""" + backup = tmp_path / "hosts.backup" + hosts_file = tmp_path / "hosts" + current_lines = [f"192.0.2.{index} host-{index}.test" for index in range(1, 41)] + restored_lines = current_lines.copy() + restored_lines[19] = "192.0.2.20 restored.test" + hosts_file.write_text("\n".join(current_lines) + "\n") + backup.write_text("\n".join(restored_lines) + "\n") + + preview = create_restore_preview(backup, hosts_file) + + assert len(preview.changed_hunks[0].rows) == 7 + assert len(preview.full_hunks[0].rows) == 40