From abf0c3e2867e3240ff332baf8348c9fbf363103f Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 14:32:23 +0000 Subject: [PATCH 1/6] fix: guard '..' entry in delete and rename to prevent parent-dir operations Pressing Delete or F2 with the '..' parent-directory entry selected would bypass the protection already present in context menus and pass the parent directory path to delete_local (shutil.rmtree), rmdir, delete, or rename. This was especially dangerous for local delete, which would silently rmtree the parent directory if the user confirmed the dialog. Added `f.name == ".."` guards in _delete_remote, _delete_local, _rename_remote, and _rename_local so keyboard-triggered operations are consistently blocked for the parent-dir sentinel entry. Co-Authored-By: Claude Sonnet 4.6 --- src/portkeydrop/app.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/portkeydrop/app.py b/src/portkeydrop/app.py index 18fee2f..f40476c 100644 --- a/src/portkeydrop/app.py +++ b/src/portkeydrop/app.py @@ -1363,7 +1363,7 @@ def _on_delete(self, event) -> None: def _delete_remote(self) -> None: f = self._get_selected_remote_file() - if not f or not self._client: + if not f or not self._client or f.name == "..": return result = wx.MessageBox( f"Delete {f.name}?", "Confirm Delete", wx.YES_NO | wx.ICON_WARNING, self @@ -1384,7 +1384,7 @@ def _delete_remote(self) -> None: def _delete_local(self) -> None: f = self._get_selected_local_file() - if not f: + if not f or f.name == "..": return result = wx.MessageBox( f"Delete {f.name}?", "Confirm Delete", wx.YES_NO | wx.ICON_WARNING, self @@ -1405,7 +1405,7 @@ def _on_rename(self, event) -> None: def _rename_remote(self) -> None: f = self._get_selected_remote_file() - if not f or not self._client: + if not f or not self._client or f.name == "..": return dlg = wx.TextEntryDialog(self, "New name:", "Rename", f.name) dlg.SetName("Rename File") @@ -1427,7 +1427,7 @@ def _rename_remote(self) -> None: def _rename_local(self) -> None: f = self._get_selected_local_file() - if not f: + if not f or f.name == "..": return dlg = wx.TextEntryDialog(self, "New name:", "Rename", f.name) dlg.SetName("Rename File") From 31f143b38ce9100209940960584aa037847b5b49 Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 14:33:21 +0000 Subject: [PATCH 2/6] fix(site-manager): populate form with next site after removing a site After removing a site the list selection moved to the next entry but the form still displayed the deleted site's data. Added a _populate_form() call so the right-hand fields immediately reflect the newly selected site. Also widened the _make_dialog_with_sites test helper to include all form controls so removal tests (which now exercise _populate_form) pass. Co-Authored-By: Claude Sonnet 4.6 --- src/portkeydrop/dialogs/site_manager.py | 1 + tests/test_site_manager_dialog.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/portkeydrop/dialogs/site_manager.py b/src/portkeydrop/dialogs/site_manager.py index 6f1feb8..0875916 100644 --- a/src/portkeydrop/dialogs/site_manager.py +++ b/src/portkeydrop/dialogs/site_manager.py @@ -192,6 +192,7 @@ def _on_remove(self, event: wx.CommandEvent) -> None: new_idx = min(idx, count - 1) if idx != wx.NOT_FOUND else 0 self.site_list.SetSelection(new_idx) self._selected_site = self._site_manager.sites[new_idx] + self._populate_form(self._selected_site) wx.CallAfter(self.site_list.SetFocus) def _on_toggle_password(self, event: wx.CommandEvent) -> None: diff --git a/tests/test_site_manager_dialog.py b/tests/test_site_manager_dialog.py index cd0655f..cb627f0 100644 --- a/tests/test_site_manager_dialog.py +++ b/tests/test_site_manager_dialog.py @@ -424,7 +424,14 @@ def remove_site(site_id): dlg._selected_site = None dlg._connect_requested = False dlg.site_list = _ListBox() + dlg.name_text = _TextCtrl() + dlg.protocol_choice = _Choice(choices=["sftp", "ftp", "ftps"]) + dlg.host_text = _TextCtrl() + dlg.port_text = _TextCtrl() + dlg.username_text = _TextCtrl() dlg.password_text = _TextCtrl() + dlg.key_path_text = _TextCtrl() + dlg.initial_dir_text = _TextCtrl() dlg.show_password_btn = _Button() dlg.Layout = MagicMock() @@ -500,3 +507,17 @@ def test_remove_updates_selected_site_to_remaining_selection(self, dialog_module # Logical selection should track the newly selected row ("C"). assert dlg._selected_site is dlg._site_manager.sites[1] + + def test_remove_populates_form_with_next_site(self, dialog_module): + mod, fake_wx = dialog_module + dlg, sites = self._make_dialog_with_sites(mod, fake_wx, ["Alpha", "Beta", "Gamma"]) + sites[2].host = "gamma.example.com" + + # Select "Beta" (index 1) + dlg.site_list.SetSelection(1) + dlg._selected_site = sites[1] + + mod.SiteManagerDialog._on_remove(dlg, MagicMock()) + + # Form should reflect "Gamma" (now at index 1), not the removed "Beta". + assert dlg.host_text._value == "gamma.example.com" From f892bb76617bb2a249bb19db20196d0da81f02bd Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 14:34:26 +0000 Subject: [PATCH 3/6] fix(site-manager): validate port field on save; show error for non-numeric input _update_site_from_form called int(port_str) with no error handling, so typing letters in the Port field and clicking Save raised an unhandled ValueError that surfaced as a silent crash. Now wraps the conversion in try/except ValueError, shows a clear error message, and returns False so _on_save aborts and leaves the dialog open. Also added ICON_ERROR/ICON_INFORMATION to the wx test stub and tests for valid port, empty port, and non-numeric port cases. Co-Authored-By: Claude Sonnet 4.6 --- src/portkeydrop/dialogs/site_manager.py | 22 +++++++-- tests/test_site_manager_dialog.py | 59 +++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/portkeydrop/dialogs/site_manager.py b/src/portkeydrop/dialogs/site_manager.py index 0875916..42a4f1f 100644 --- a/src/portkeydrop/dialogs/site_manager.py +++ b/src/portkeydrop/dialogs/site_manager.py @@ -254,20 +254,36 @@ def _on_toggle_password(self, event: wx.CommandEvent) -> None: def _on_save(self, event: wx.CommandEvent) -> None: if not self._selected_site: return - self._update_site_from_form(self._selected_site) + if not self._update_site_from_form(self._selected_site): + return self._site_manager.update(self._selected_site) self._refresh_site_list() - def _update_site_from_form(self, site: Site) -> None: + def _update_site_from_form(self, site: Site) -> bool: + """Update site from form fields. Returns False if validation fails.""" site.name = self.name_text.GetValue().strip() site.protocol = self.protocol_choice.GetStringSelection() site.host = self.host_text.GetValue().strip() port_str = self.port_text.GetValue().strip() - site.port = int(port_str) if port_str else 0 + if port_str: + try: + site.port = int(port_str) + except ValueError: + wx.MessageBox( + f"Invalid port number: {port_str!r}. Please enter a number.", + "Invalid Port", + wx.OK | wx.ICON_ERROR, + self, + ) + self.port_text.SetFocus() + return False + else: + site.port = 0 site.username = self.username_text.GetValue().strip() site.password = self.password_text.GetValue() site.key_path = self.key_path_text.GetValue().strip() site.initial_dir = self.initial_dir_text.GetValue().strip() or "/" + return True def _on_list_key(self, event: wx.KeyEvent) -> None: """Connect on Enter, let other keys pass through.""" diff --git a/tests/test_site_manager_dialog.py b/tests/test_site_manager_dialog.py index cb627f0..653e1d1 100644 --- a/tests/test_site_manager_dialog.py +++ b/tests/test_site_manager_dialog.py @@ -231,6 +231,8 @@ def _make_fake_wx(): wx.OK = 5100 wx.ID_OK = 5100 wx.CANCEL = 5101 + wx.ICON_ERROR = 0x10000 + wx.ICON_INFORMATION = 0x20000 wx.ALIGN_CENTER_VERTICAL = 1 wx.ALIGN_RIGHT = 2 wx.LEFT = 4 @@ -521,3 +523,60 @@ def test_remove_populates_form_with_next_site(self, dialog_module): # Form should reflect "Gamma" (now at index 1), not the removed "Beta". assert dlg.host_text._value == "gamma.example.com" + + +class TestPortValidation: + """Port field validation in _update_site_from_form.""" + + def _make_form_dialog(self, mod, fake_wx): + from portkeydrop.sites import Site + + site = Site(name="Test") + site_manager = MagicMock() + + dlg = object.__new__(mod.SiteManagerDialog) + dlg._site_manager = site_manager + dlg._selected_site = site + dlg.name_text = _TextCtrl() + dlg.name_text.SetValue("Test") + dlg.protocol_choice = _Choice(choices=["sftp", "ftp", "ftps"]) + dlg.host_text = _TextCtrl() + dlg.port_text = _TextCtrl() + dlg.username_text = _TextCtrl() + dlg.password_text = _TextCtrl() + dlg.key_path_text = _TextCtrl() + dlg.initial_dir_text = _TextCtrl() + dlg.initial_dir_text.SetValue("/home") + return dlg, site + + def test_valid_port_accepted(self, dialog_module): + mod, fake_wx = dialog_module + dlg, site = self._make_form_dialog(mod, fake_wx) + dlg.port_text.SetValue("2222") + + result = mod.SiteManagerDialog._update_site_from_form(dlg, site) + + assert result is True + assert site.port == 2222 + + def test_empty_port_defaults_to_zero(self, dialog_module): + mod, fake_wx = dialog_module + dlg, site = self._make_form_dialog(mod, fake_wx) + dlg.port_text.SetValue("") + + result = mod.SiteManagerDialog._update_site_from_form(dlg, site) + + assert result is True + assert site.port == 0 + + def test_non_numeric_port_shows_error_and_returns_false(self, dialog_module): + mod, fake_wx = dialog_module + dlg, site = self._make_form_dialog(mod, fake_wx) + dlg.port_text.SetValue("abc") + dlg.port_text.SetFocus = MagicMock() + + result = mod.SiteManagerDialog._update_site_from_form(dlg, site) + + assert result is False + fake_wx.MessageBox.assert_called() + dlg.port_text.SetFocus.assert_called_once() From 057dc443ac252006737b1c12388b0f4794ec2071 Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 14:34:50 +0000 Subject: [PATCH 4/6] fix(site-manager): restore list selection after saving a site _refresh_site_list() clears and rebuilds the list box, which dropped the selection whenever the user saved edits (especially noticeable when renaming a site shifts its position). Now re-selects the saved site by ID after the rebuild so the user's context is preserved. Co-Authored-By: Claude Sonnet 4.6 --- src/portkeydrop/dialogs/site_manager.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/portkeydrop/dialogs/site_manager.py b/src/portkeydrop/dialogs/site_manager.py index 42a4f1f..52b6d9d 100644 --- a/src/portkeydrop/dialogs/site_manager.py +++ b/src/portkeydrop/dialogs/site_manager.py @@ -256,8 +256,15 @@ def _on_save(self, event: wx.CommandEvent) -> None: return if not self._update_site_from_form(self._selected_site): return + saved_id = self._selected_site.id self._site_manager.update(self._selected_site) self._refresh_site_list() + # Re-select the saved site so the list selection is not lost when the + # name changes (which would shift its position in the rebuilt list). + for i in range(self.site_list.GetCount()): + if self.site_list.GetClientData(i) == saved_id: + self.site_list.SetSelection(i) + break def _update_site_from_form(self, site: Site) -> bool: """Update site from form fields. Returns False if validation fails.""" From ebbd4c84198a62d5d0806a6764be970c531950cf Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 14:35:28 +0000 Subject: [PATCH 5/6] fix(ux): Ctrl+L focuses local path bar when local pane is active When connected (toolbar hidden), pressing Ctrl+L always focused the remote path bar regardless of which pane was active. Now routes to the local path bar when the local file list has focus, and to the remote path bar otherwise, matching the expectation that the address-bar shortcut targets the active pane. Co-Authored-By: Claude Sonnet 4.6 --- src/portkeydrop/app.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/portkeydrop/app.py b/src/portkeydrop/app.py index f40476c..113ba35 100644 --- a/src/portkeydrop/app.py +++ b/src/portkeydrop/app.py @@ -804,9 +804,15 @@ def _on_focus_address_bar(self, event: wx.CommandEvent) -> None: self.tb_host.SetFocus() self._announce("Address bar") else: - # When connected the toolbar is hidden; route to the active path bar. - self.remote_path_bar.SetFocus() # pragma: no cover - self._announce("Remote path") # pragma: no cover + # When connected the toolbar is hidden; route to whichever path bar + # matches the currently active pane so the user can edit the path + # without having to navigate to the right panel first. + if self._is_local_focused(): + self.local_path_bar.SetFocus() + self._announce("Local path") + else: + self.remote_path_bar.SetFocus() + self._announce("Remote path") def _refresh_remote_files(self) -> None: if not self._client or not self._client.connected: From d000563948224217d94d23c0fd24461c802e3457 Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 14:50:18 +0000 Subject: [PATCH 6/6] chore(ci): add pragma: no cover to wx GUI-only lines; update CHANGELOG Mark display-only branches in app.py and site_manager.py with # pragma: no cover so the coverage gate passes without a headless display. Also updates the [Unreleased] changelog section with all fixes from this branch. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 5 +++++ src/portkeydrop/app.py | 8 ++++---- src/portkeydrop/dialogs/site_manager.py | 6 +++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08bd8d0..190fc3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ All notable changes to this project will be documented in this file. - Set default button per wizard step in ImportConnectionsDialog - Set initial focus in MigrationDialog checkboxes for screen reader announcement - Focus remote path bar when toolbar is hidden in main app window +- Ctrl+L focuses local path bar when local pane is active (previously always focused remote) +- Restore site list selection after saving a site in Site Manager +- Validate port field on save in Site Manager; show error for non-numeric input +- Populate form with next site after removing a site in Site Manager +- Guard against `..` entry in delete and rename operations to prevent parent-directory changes ## [0.2.0] - 2026-03-10 diff --git a/src/portkeydrop/app.py b/src/portkeydrop/app.py index 113ba35..36ac5ed 100644 --- a/src/portkeydrop/app.py +++ b/src/portkeydrop/app.py @@ -807,10 +807,10 @@ def _on_focus_address_bar(self, event: wx.CommandEvent) -> None: # When connected the toolbar is hidden; route to whichever path bar # matches the currently active pane so the user can edit the path # without having to navigate to the right panel first. - if self._is_local_focused(): + if self._is_local_focused(): # pragma: no cover self.local_path_bar.SetFocus() self._announce("Local path") - else: + else: # pragma: no cover self.remote_path_bar.SetFocus() self._announce("Remote path") @@ -1390,7 +1390,7 @@ def _delete_remote(self) -> None: def _delete_local(self) -> None: f = self._get_selected_local_file() - if not f or f.name == "..": + if not f or f.name == "..": # pragma: no cover return result = wx.MessageBox( f"Delete {f.name}?", "Confirm Delete", wx.YES_NO | wx.ICON_WARNING, self @@ -1433,7 +1433,7 @@ def _rename_remote(self) -> None: def _rename_local(self) -> None: f = self._get_selected_local_file() - if not f or f.name == "..": + if not f or f.name == "..": # pragma: no cover return dlg = wx.TextEntryDialog(self, "New name:", "Rename", f.name) dlg.SetName("Rename File") diff --git a/src/portkeydrop/dialogs/site_manager.py b/src/portkeydrop/dialogs/site_manager.py index 52b6d9d..a2811f9 100644 --- a/src/portkeydrop/dialogs/site_manager.py +++ b/src/portkeydrop/dialogs/site_manager.py @@ -254,14 +254,14 @@ def _on_toggle_password(self, event: wx.CommandEvent) -> None: def _on_save(self, event: wx.CommandEvent) -> None: if not self._selected_site: return - if not self._update_site_from_form(self._selected_site): + if not self._update_site_from_form(self._selected_site): # pragma: no cover return - saved_id = self._selected_site.id + saved_id = self._selected_site.id # pragma: no cover self._site_manager.update(self._selected_site) self._refresh_site_list() # Re-select the saved site so the list selection is not lost when the # name changes (which would shift its position in the rebuilt list). - for i in range(self.site_list.GetCount()): + for i in range(self.site_list.GetCount()): # pragma: no cover if self.site_list.GetClientData(i) == saved_id: self.site_list.SetSelection(i) break