From d1d709b4ad859213c742f7715f6e27ab7412d8af Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 19:55:49 +0200 Subject: [PATCH 1/4] feat(gui): block an address or spare a process straight from a connection row Two right-click actions on the Connections table. "Block this IP address" adds the row's address to the blocking field; "Leave this process alone" excludes that process from impairment by adding !name to the target. Both APPEND. They are used one row at a time, so replacing would discard the address blocked a moment ago and make the second click look broken. A repeat is dropped rather than doubled. With an empty target, !name is not a narrowing but a flip: "impair everything" becomes "impair everything except this", which is the case that entry exists for. The appending lives in matchers.add_term, not in the GUI, because splitting on commas is a question about the filter syntax (convention 10). It shipped the comma-escape bug for about ten minutes - re-joining without re-escaping emits a regex's literal comma as a separator - which is the same failure a property test once found in Matcher.describe. Pinned by a test now. Convention 15 holds and is finally guarded: the existing test was named "feeds the targeting FIELDS" and checked only the form, so a row action pushed straight into a running engine would have kept the suite green. The new guard asserts the engine is untouched before Apply and changed after it, and is mutation-checked. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 ++++ beantester/gui/app.py | 43 ++++++++++++++++++++++ beantester/gui/pages/conns.py | 15 ++++++++ beantester/matchers.py | 33 +++++++++++++++++ lang/en.json | 16 +++++--- lang/pl.json | 16 +++++--- tests/test_gui_state.py | 65 +++++++++++++++++++++++++++++++++ tests/test_matchers.py | 40 ++++++++++++++++++++ tests/test_mutation_registry.py | 10 +++++ 9 files changed, 234 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 773bcab..cdcad58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ## [Unreleased] +### Added + +- **Connections: two new right-click actions on a row.** "Block this IP address" adds it to the + blocking field, and "Leave this process alone" excludes that process from impairment. Both ADD + to what is already in the field, so you can build a list one row at a time, and a repeat is + ignored rather than duplicated. Like every other row action they fill the form only - press + "Apply changes" to put them into a running session. + ### Docs - Both READMEs now say in the licence section that **what you make with the tool is yours**. diff --git a/beantester/gui/app.py b/beantester/gui/app.py index 439e8c4..bec2e3b 100644 --- a/beantester/gui/app.py +++ b/beantester/gui/app.py @@ -33,6 +33,7 @@ from ..fields import SEED as F_SEED from ..fields import FIELD_DEFS, SECTIONS, UI_ONLY_KEYS, off_value from ..filters import cli_key_for, i18n_key_for, i18n_keys, windivert_for +from ..matchers import add_term from .. import crashlog from ..i18n import (FALLBACK_LANGUAGE, T, available_languages, current_language, set_language) @@ -1236,6 +1237,48 @@ def set_destination(self, ip, port): if self.running: self.log(T("log.apply_needed")) + def _append_to_field(self, key, term, log_key): + """Add one term to an expression field, keeping what is already there. + + The row actions below build a field up click by click - block this + address, then that one - so they append. Replacing would throw away the + addresses blocked a moment ago, which is the opposite of what the second + click means. ``matchers.add_term`` owns the syntax (convention 10): it + drops repeats, keeps the comma escape of a regex intact and never leaves + an empty term behind. + + Like every other row action, this only fills the form: convention 15, and + the running session hears about it through the same "apply needed" line. + """ + current = self.vars[key].get() + updated = add_term(current, term) + self.vars[key].set(updated) + self.form.set_values(self._settings_for_form()) + self.on_form_changed() + self.log(f"{T(log_key)}: {updated}") + if self.running: + self.log(T("log.apply_needed")) + + def block_ip_address(self, ip): + """Add an address to the blocking field (pipeline step 2c).""" + if not str(ip or "").strip(): + return + self._append_to_field("block_ip", str(ip).strip(), "log.block_ip_added") + + def leave_process_alone(self, name): + """Exclude a process from impairment by adding ``!name`` to the target. + + With a target already set this narrows it. With the target EMPTY it turns + "impair everything" into "impair everything except this one", because a + bare negative means exactly that in this expression language - which is + the case the menu entry is really for. + """ + name = str(name or "").strip() + if not name or name == "?": + self.log(T("log.no_process_for_row")) + return + self._append_to_field("target", f"!{name}", "log.process_excluded") + # -- scenario / config files ---------------------------------------------------- # def _update_scenario_label(self): if self.scenario_lbl is None: diff --git a/beantester/gui/pages/conns.py b/beantester/gui/pages/conns.py index a102220..a52190f 100644 --- a/beantester/gui/pages/conns.py +++ b/beantester/gui/pages/conns.py @@ -171,7 +171,10 @@ def _build_menu(self): self.menu.add_command(label=T("menu.copy_ip"), command=self._copy_ip) self.menu.add_separator() self.menu.add_command(label=T("menu.target_process"), command=self._target_process) + self.menu.add_command(label=T("menu.leave_process_alone"), + command=self._leave_process_alone) self.menu.add_command(label=T("menu.limit_dest"), command=self._limit_dest) + self.menu.add_command(label=T("menu.block_ip"), command=self._block_ip) self.menu.add_separator() self.menu.add_command(label=T("menu.reset_widths"), command=self.table.reset_widths) @@ -239,6 +242,12 @@ def _target_process(self): return self.app.set_target_expression(name) + def _leave_process_alone(self): + row = self._selected() + if not row: + return + self.app.leave_process_alone(str(row.get("proc") or "").strip()) + def _limit_dest(self): row = self._selected() if not row: @@ -246,6 +255,12 @@ def _limit_dest(self): self.app.set_destination(str(row.get("remote_ip") or ""), str(row.get("remote_port") or "")) + def _block_ip(self): + row = self._selected() + if not row: + return + self.app.block_ip_address(str(row.get("remote_ip") or "")) + # -- search -------------------------------------------------------------- # def _schedule_search(self): if self._search_job is not None: diff --git a/beantester/matchers.py b/beantester/matchers.py index c5e757f..d6bc730 100644 --- a/beantester/matchers.py +++ b/beantester/matchers.py @@ -264,6 +264,39 @@ def split_terms(text): return [p.strip() for p in parts if p.strip()] +def add_term(text, term): + """Return ``text`` with ``term`` appended, or unchanged if it is already there. + + The Connections context menu builds expressions one row at a time: block this + address, then that one, then leave this process alone. Appending is the only + behaviour that makes sense there - replacing would silently drop the two + addresses the user blocked a moment ago. + + It lives HERE rather than in the GUI because splitting on commas is a question + about the FILTER SYNTAX, and this module owns that (convention 10). Naive + string concatenation gets three things wrong that ``split_terms`` already + knows: an escaped ``\\,`` inside a regex is not a separator, terms carry + surrounding whitespace that must not become part of the value, and a trailing + comma from an earlier edit would produce an empty term. + + Duplicates are dropped rather than repeated. ``80,80`` means the same as + ``80``, so the only thing a repeat changes is that the field looks broken. + + 🔴 **The comma escape has to be put back on the way out**, exactly as in + ``Matcher.describe``. ``split_terms`` turns ``\\,`` into a literal comma inside + the term, so re-joining without escaping emits it as a SEPARATOR and silently + splits one regex into two nonsense terms. This function reintroduced that bug + when it was first written, on the same day its twin was cited as a solved one - + which is why the round trip is now a test, not a promise. + """ + term = str(term or "").strip() + if not term: + return str(text or "") + existing = split_terms(text) + terms = existing if term in existing else existing + [term] + return ",".join(t.replace(",", "\\,") for t in terms) + + # -- atom parsers --------------------------------------------------------------- # def _check_bounds(number, bounds, field, term): if bounds and not (bounds[0] <= number <= bounds[1]): diff --git a/lang/en.json b/lang/en.json index 38a5da1..c6bb671 100644 --- a/lang/en.json +++ b/lang/en.json @@ -109,8 +109,6 @@ "dialogs.start_failed": "Failed to start", "dialogs.values_numbers": "Values must be numbers.", "errors.bad_config_value": "Invalid value for '{field}' in the config file: {value}", - "errors.config_unknown_setting": "Unknown setting in the config file: {field}. Remove it or correct the spelling.", - "errors.config_unknown_setting_hint": "Unknown setting in the config file: {field}. Did you mean '{suggestion}'?", "errors.bad_filter_bounds": "Field '{field}': '{term}' is out of the allowed range ({min}-{max}).", "errors.bad_filter_compare": "Field '{field}': comparison '{term}' needs a number after the operator.", "errors.bad_filter_compare_name": "Field '{field}': comparison operators (>, <, >=, <=) work only with a PID (a number), not a process name - '{term}'.", @@ -121,6 +119,8 @@ "errors.bad_filter_regex": "Field '{field}': '{term}' is not a valid regular expression (a comma inside a pattern must be escaped: \\,).", "errors.bad_filter_term": "Empty entry in field '{field}': '{term}'.", "errors.bad_schedule_step": "bad schedule step: '{part}' (use dur:down:up)", + "errors.config_unknown_setting": "Unknown setting in the config file: {field}. Remove it or correct the spelling.", + "errors.config_unknown_setting_hint": "Unknown setting in the config file: {field}. Did you mean '{suggestion}'?", "errors.field_number": "The '{name}' field must be a number.", "errors.field_range": "Field '{name}' must be between {min} and {max}.", "errors.scenario_bad_json": "Not a valid JSON file: {error}", @@ -233,6 +233,7 @@ "log.applied_changes": "Applied changes", "log.apply_needed": "Click \"Apply changes\" to push this to the running session.", "log.apply_needs_start": "Apply changes works after start (START).", + "log.block_ip_added": "Blocking", "log.bug_marked": "BUG MARKED", "log.config_loaded_from": "Config loaded from", "log.config_saved_to": "Config saved to", @@ -263,6 +264,7 @@ "log.note_windows": "NOTE: capture works only on Windows (WinDivert).", "log.once": "once", "log.ports": "ports", + "log.process_excluded": "Target", "log.processes": "processes", "log.profile_deleted": "Profile deleted", "log.profile_saved": "Profile saved", @@ -284,6 +286,10 @@ "log.scenario_start": "Scenario: start", "log.schedule_skipped": "Schedule skipped", "log.send_failed": "WARNING: the tool could not put a packet back on the wire ({n} so far, most recently: {e}). Those packets are gone, and it was us that lost them, not the network. If it keeps happening, check that the connection is still up and that the WinDivert driver is still loaded.", + "log.shared_port_footer": "Every other port of the target is unaffected.", + "log.shared_port_hits": "Heads up: port {port} is open in several programs at once ({who}). The tool decides what to break from the port number, so on this one port their traffic gets broken too.", + "log.shared_port_misses": "Heads up: port {port} is open in several programs at once ({who}), and in the system's socket table it belongs to {winner}. The target's traffic on this one port will be skipped.", + "log.shared_port_same_app": "same program as your target", "log.start_filter": "Start. Filter", "log.start_first": "Start first (START) to create session data.", "log.starting": "Starting session…", @@ -295,15 +301,13 @@ "log.targeting": "Targeting", "log.targeting_error": "Targeting error", "log.targeting_none": "Targeting: NO matching process - traffic is NOT being impaired.", - "log.shared_port_hits": "Heads up: port {port} is open in several programs at once ({who}). The tool decides what to break from the port number, so on this one port their traffic gets broken too.", - "log.shared_port_misses": "Heads up: port {port} is open in several programs at once ({who}), and in the system's socket table it belongs to {winner}. The target's traffic on this one port will be skipped.", - "log.shared_port_same_app": "same program as your target", - "log.shared_port_footer": "Every other port of the target is unaffected.", "log.targeting_requires_psutil": "Targeting requires psutil: pip install psutil", "log.ui_error": "Internal error: {e} (the session keeps running, see the log).", "log.ui_state_problem": "Window-state file problem", + "menu.block_ip": "Block this IP address", "menu.copy_ip": "Copy IP address", "menu.copy_row": "Copy row (Ctrl+C)", + "menu.leave_process_alone": "Leave this process alone", "menu.limit_dest": "Limit to this IP:port", "menu.open_event_log": "Open event log in a window", "menu.reset_widths": "Reset column widths", diff --git a/lang/pl.json b/lang/pl.json index 5d13a2e..57e9d7a 100644 --- a/lang/pl.json +++ b/lang/pl.json @@ -109,8 +109,6 @@ "dialogs.start_failed": "Nie udało się uruchomić", "dialogs.values_numbers": "Wartości muszą być liczbami.", "errors.bad_config_value": "Nieprawidłowa wartość pola '{field}' w pliku konfiguracji: {value}", - "errors.config_unknown_setting": "Nieznane ustawienie w pliku konfiguracji: {field}. Usuń je albo popraw pisownię.", - "errors.config_unknown_setting_hint": "Nieznane ustawienie w pliku konfiguracji: {field}. Czy chodziło o '{suggestion}'?", "errors.bad_filter_bounds": "Pole '{field}': '{term}' jest poza dozwolonym zakresem ({min}-{max}).", "errors.bad_filter_compare": "Pole '{field}': porównanie '{term}' wymaga liczby po operatorze.", "errors.bad_filter_compare_name": "Pole '{field}': operatory porównania (>, <, >=, <=) działają tylko z PID-em (liczbą), nie z nazwą procesu - '{term}'.", @@ -121,6 +119,8 @@ "errors.bad_filter_regex": "Pole '{field}': '{term}' to niepoprawne wyrażenie regularne (przecinek we wzorcu trzeba poprzedzić ukośnikiem: \\,).", "errors.bad_filter_term": "Puste wyrażenie w polu '{field}': '{term}'.", "errors.bad_schedule_step": "zły krok harmonogramu: '{part}' (użyj dur:down:up)", + "errors.config_unknown_setting": "Nieznane ustawienie w pliku konfiguracji: {field}. Usuń je albo popraw pisownię.", + "errors.config_unknown_setting_hint": "Nieznane ustawienie w pliku konfiguracji: {field}. Czy chodziło o '{suggestion}'?", "errors.field_number": "Pole '{name}' musi być liczbą.", "errors.field_range": "Pole '{name}' musi mieścić się w zakresie od {min} do {max}.", "errors.scenario_bad_json": "To nie jest poprawny plik JSON: {error}", @@ -233,6 +233,7 @@ "log.applied_changes": "Zastosowano zmiany", "log.apply_needed": "Kliknij „Zastosuj zmiany”, aby przekazać to do działającej sesji.", "log.apply_needs_start": "Zastosuj zmiany działa po uruchomieniu (START).", + "log.block_ip_added": "Blokowanie", "log.bug_marked": "ZAZNACZONO BŁĄD", "log.config_loaded_from": "Wczytano konfigurację z", "log.config_saved_to": "Zapisano konfigurację do", @@ -263,6 +264,7 @@ "log.note_windows": "UWAGA: przechwytywanie działa tylko na Windows (WinDivert).", "log.once": "jednorazowo", "log.ports": "portów", + "log.process_excluded": "Cel", "log.processes": "procesów", "log.profile_deleted": "Usunięto profil", "log.profile_saved": "Zapisano profil", @@ -284,6 +286,10 @@ "log.scenario_start": "Scenariusz: start", "log.schedule_skipped": "Harmonogram pominięty", "log.send_failed": "UWAGA: narzędzie nie zdołało odesłać pakietu do sieci ({n} do tej pory, ostatnio: {e}). Te pakiety przepadły i zgubiliśmy je my, a nie sieć. Jeśli to się powtarza, sprawdź, czy połączenie nadal działa i czy sterownik WinDivert jest wciąż załadowany.", + "log.shared_port_footer": "Pozostałe porty celu są bez zmian.", + "log.shared_port_hits": "Uwaga: port {port} jest otwarty w kilku programach naraz ({who}). Narzędzie decyduje po numerze portu, więc na tym jednym porcie oberwie też ich ruch.", + "log.shared_port_misses": "Uwaga: port {port} jest otwarty w kilku programach naraz ({who}), a w tabeli gniazd systemu należy do {winner}. Ruch celu na tym jednym porcie zostanie pominięty.", + "log.shared_port_same_app": "ten sam program co cel", "log.start_filter": "Start. Filtr", "log.start_first": "Uruchom najpierw (START), aby powstały dane sesji.", "log.starting": "Uruchamiam sesję…", @@ -295,15 +301,13 @@ "log.targeting": "Celuję w", "log.targeting_error": "Błąd celowania", "log.targeting_none": "Celowanie: BRAK pasującego procesu - ruch NIE jest modyfikowany.", - "log.shared_port_hits": "Uwaga: port {port} jest otwarty w kilku programach naraz ({who}). Narzędzie decyduje po numerze portu, więc na tym jednym porcie oberwie też ich ruch.", - "log.shared_port_misses": "Uwaga: port {port} jest otwarty w kilku programach naraz ({who}), a w tabeli gniazd systemu należy do {winner}. Ruch celu na tym jednym porcie zostanie pominięty.", - "log.shared_port_same_app": "ten sam program co cel", - "log.shared_port_footer": "Pozostałe porty celu są bez zmian.", "log.targeting_requires_psutil": "Celowanie wymaga psutil: pip install psutil", "log.ui_error": "Błąd wewnętrzny: {e} (sesja działa dalej, sprawdź log).", "log.ui_state_problem": "Problem z plikiem stanu okna", + "menu.block_ip": "Blokuj ten adres IP", "menu.copy_ip": "Kopiuj adres IP", "menu.copy_row": "Kopiuj wiersz (Ctrl+C)", + "menu.leave_process_alone": "Nie psuj tego procesu", "menu.limit_dest": "Ogranicz do tego IP:port", "menu.open_event_log": "Otwórz dziennik zdarzeń w oknie", "menu.reset_widths": "Resetuj szerokości kolumn", diff --git a/tests/test_gui_state.py b/tests/test_gui_state.py index 2cdbf46..041fa96 100644 --- a/tests/test_gui_state.py +++ b/tests/test_gui_state.py @@ -158,6 +158,71 @@ def test_connection_row_feeds_the_targeting_fields(): """) +def test_a_row_action_fills_the_form_and_does_not_reach_a_running_engine(): + """Convention 15 for the Connections context menu: nothing applies itself. + + The test above proves the FIELDS get the values. It says nothing about the + engine, which is the half that matters: these actions are one click away from + the running session, and "helpfully" pushing them straight through is the + obvious future change that would break the rule while every test stayed green. + Same shape as the preset and profile pickers, which have had this guard for + longer, and same shape as the bug that made `narrow_filter` stay editable. + + So this asserts BOTH directions - untouched engine before Apply, changed + engine after it - because only the pair distinguishes "did not apply" from + "did not work at all". + """ + run_gui(""" + app.running = True + app.engine.start("both", divert=bnt.SyntheticDivert(gen_kbps=1)) + core = app.engine.core + assert not core.target_active and not core.dst_active, "engine starts unaimed" + + app.set_target_expression("chrome.exe") + app.set_destination("10.0.0.7", "443") + + assert not core.target_active, "targeting reached the engine without Apply" + assert not core.dst_active, "destination reached the engine without Apply" + s = app._settings_from_widgets() + assert s["target"] == "chrome.exe" and s["dst_ip"] == "10.0.0.7" + assert app._form_changed, "the Apply button must light up instead" + + app.apply_if_running(announce=False) + assert core.target_active and core.dst_active, "Apply did not push them" + app.engine.stop() + """) + + +def test_blocking_and_excluding_from_a_row_accumulate(): + """"Block this IP" and "Leave this process alone" ADD, they do not replace. + + Both are used one row at a time - block this address, then that one - so + replacing would silently discard the address blocked a moment ago, and the + second click would look broken. A repeat is dropped instead of doubled, + because `8.8.8.8,8.8.8.8` means the same thing and only reads as a bug. + + The exclusion is `!name` in the target field. With the target empty that is + not a narrowing but a flip: "impair everything" becomes "impair everything + except this", which is the case the menu entry exists for. + """ + run_gui(""" + app.block_ip_address("8.8.8.8") + app.block_ip_address("1.1.1.1") + app.block_ip_address("8.8.8.8") + assert app._settings_from_widgets()["block_ip"] == "8.8.8.8,1.1.1.1" + + app.leave_process_alone("chrome.exe") + app.leave_process_alone("msedge.exe") + assert app._settings_from_widgets()["target"] == "!chrome.exe,!msedge.exe" + + # a row with no known process must not write "!?" into the target + before = app._settings_from_widgets()["target"] + app.leave_process_alone("?") + app.leave_process_alone("") + assert app._settings_from_widgets()["target"] == before + """) + + def test_no_section_carries_an_enable_checkbox(): """The three "Enable" boxes are gone. diff --git a/tests/test_matchers.py b/tests/test_matchers.py index 50f28e0..29c9686 100644 --- a/tests/test_matchers.py +++ b/tests/test_matchers.py @@ -341,3 +341,43 @@ def test_describe_round_trips_an_escaped_comma(): original.matches(1, name) == reparsed.matches(1, name)) check("and it still matches what it should", original.matches(1, "aa")) check("and rejects what it should", not original.matches(1, "a")) + + +def test_add_term_appends_without_breaking_the_expression(): + """Row actions build an expression one click at a time, so appending must be safe. + + Replacing would silently drop the two addresses the user blocked a moment ago, + which is why the Connections menu appends. The cases below are the ones naive + concatenation gets wrong. + """ + from beantester.matchers import add_term + + check("appending to an empty field just sets it", add_term("", "8.8.8.8") == "8.8.8.8") + check("a second term is appended", add_term("8.8.8.8", "1.1.1.1") == "8.8.8.8,1.1.1.1") + check("a repeat is dropped, not doubled", add_term("8.8.8.8", "8.8.8.8") == "8.8.8.8") + check("surrounding spaces do not become part of a value", + add_term("8.8.8.8 , 1.1.1.1", "9.9.9.9") == "8.8.8.8,1.1.1.1,9.9.9.9") + check("a trailing comma does not produce an empty term", + add_term("8.8.8.8,", "1.1.1.1") == "8.8.8.8,1.1.1.1") + check("an empty term changes nothing", add_term("x", " ") == "x") + check("an exclusion is just another term", + add_term("chrome.exe", "!msedge.exe") == "chrome.exe,!msedge.exe") + + +def test_add_term_keeps_the_comma_escape_of_a_regex(): + """The bug this function shipped with for about ten minutes, now pinned. + + ``split_terms`` turns ``\,`` into a literal comma INSIDE the term. Re-joining + without escaping it emits that comma as a SEPARATOR, so one regex silently + becomes two nonsense terms - the same failure a property test once found in + ``Matcher.describe``. Appending to a field containing a regex is rare, and + "rare plus silent" is exactly what a regression test is for. + """ + from beantester.matchers import add_term, split_terms + + out = add_term(r"re:^a\,b$", "1.1.1.1") + check("the escape survives the round trip", r"\," in out, f"({out})") + check("the expression still has exactly two terms", len(split_terms(out)) == 2, + f"({split_terms(out)})") + check("the regex term is intact", split_terms(out)[0] == "re:^a,b$", + f"({split_terms(out)[0]!r})") diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index d9babad..f64450c 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -57,6 +57,16 @@ "new": " with crashlog.quiet(\"gui.app\"):\n pass", "test": "test_start_only_fields_are_locked_while_a_session_runs", }, + { + "label": "gui: a row action pushes straight through to the running engine", + "file": "beantester/gui/app.py", + "old": (" self.on_form_changed()\n" + " self.log(f\"{T('log.target_set')}: {expression}\")"), + "new": (" self.on_form_changed()\n" + " self.apply_if_running(announce=False)\n" + " self.log(f\"{T('log.target_set')}: {expression}\")"), + "test": "test_a_row_action_fills_the_form_and_does_not_reach_a_running_engine", + }, { "label": "guards: the repository collector returns nothing", "file": "tests/test_repo_conventions.py", From d7e6ead9ee230b661befc51d353662c81bd25b07 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 20:07:17 +0200 Subject: [PATCH 2/4] feat(gui): let the connections search name a column Plain text works exactly as before. A term may now name its column instead: port:443, ip:10.0.0.0/8, pid:>4000, scoped:yes, dropped:>0. Several terms narrow together, and a "?" next to the box opens the cheat sheet. The search used to match one substring against a blob of process, protocol, direction, addresses and ports - 6 of the table's 17 columns. A PID was on screen and could not be searched for, and there was no way to ask for "only the rows this session impaired" or "only the rows that dropped something", which are the questions a tester has when the table holds a hundred thousand flows. Values are parsed by matchers.py, so this is the same mini-language as the form fields rather than a second syntax to learn and maintain (convention 10). The query compiles ONCE into a list of predicates: parsing per row would put the expression parser on the path of every row on every keystroke. Half-typed queries match nothing instead of raising, and an unknown field falls back to plain text, because http://x is a URL someone pasted and not a field name. The first version silently returned zero rows for proto:tcp. A process matcher judges (pid, name), so passing a text column positionally handed it to pid, where a name cannot be evaluated - and an unevaluable term matches nothing without complaining. Now pinned by a test and a mutation entry. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 ++ beantester/gui/pages/conns.py | 14 ++++ beantester/views.py | 111 +++++++++++++++++++++++++++++++- lang/en.json | 5 +- lang/pl.json | 5 +- tests/test_mutation_registry.py | 7 ++ tests/test_views.py | 111 ++++++++++++++++++++++++++++++++ 7 files changed, 253 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdcad58..c9c30ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ### Added +- **Connections: the search box can search one column at a time.** Plain text works as before, + and now a term can name its column: `port:443`, `ip:10.0.0.0/8`, `pid:>4000`, `scoped:yes`, + `dropped:>0`. Values use the same notation as the Control page fields, and several terms narrow + together. A new "?" button lists the column names with examples. The search used to look at 6 of + the table's 17 columns, so a PID was on screen and could not be searched for. - **Connections: two new right-click actions on a row.** "Block this IP address" adds it to the blocking field, and "Leave this process alone" excludes that process from impairment. Both ADD to what is already in the field, so you can build a list one row at a time, and a repeat is diff --git a/beantester/gui/pages/conns.py b/beantester/gui/pages/conns.py index a52190f..21b2662 100644 --- a/beantester/gui/pages/conns.py +++ b/beantester/gui/pages/conns.py @@ -28,6 +28,7 @@ from ...i18n import T from ...views import (avg_packet_bytes, connection_proc, filter_sort_connections, traffic_totals) +from .. import dialogs from ..model_worker import AsyncModel from ..labels import wrapping_label from ..scaling import scaled @@ -121,6 +122,14 @@ def __init__(self, app, parent): entry.bind("", lambda e: self._schedule_search()) entry.bind("", lambda e: self._clear_search()) add_tooltip(entry, "tips.conn_search") + # The same "?" affordance the expression fields use (gui/form.py): the search + # box understands `port:443` and `ip:10.0.0.0/8`, and a cheat sheet you can + # read is the only way anyone finds that out. A tooltip cannot be it - it + # runs away from the pointer as soon as you click. + help_btn = ttk.Button(top, text=T("fields.match_help"), style="Help.TButton", + width=2, command=self._show_search_help) + help_btn.pack(side="left", padx=(0, scaled(8))) + add_tooltip(help_btn, "tips.conn_search_help") self.pause_var = tk.BooleanVar(value=False) pause = ttk.Checkbutton(top, text=T("buttons.freeze"), variable=self.pause_var, @@ -242,6 +251,11 @@ def _target_process(self): return self.app.set_target_expression(name) + def _show_search_help(self): + """The search cheat sheet, opened by the "?" next to the box.""" + dialogs.show_help(self.app.root, T("dialogs.conn_search_help_title"), + T("dialogs.conn_search_help")) + def _leave_process_alone(self): row = self._selected() if not row: diff --git a/beantester/views.py b/beantester/views.py index ba61dc2..36aaed6 100644 --- a/beantester/views.py +++ b/beantester/views.py @@ -6,8 +6,53 @@ """ import heapq +from .matchers import KIND_INT, KIND_IP, KIND_PROCESS, PORT_BOUNDS, parse_matcher + PARTIAL_SORT_RATIO = 10 # use a heap only when the limit is this much smaller +# -- field-qualified search ----------------------------------------------------- # +# +# The plain search matches one substring against a blob of process, protocol, +# direction, addresses and ports. That is 6 of the table's 17 columns: a PID is on +# screen and cannot be searched for, and neither can "only the impaired rows" or +# "only rows that dropped something" - which are the questions a tester actually +# has when the table holds a hundred thousand flows. +# +# So a term may name its column: `port:443`, `ip:10.0.0.0/8`, `pid:>4000`. The +# VALUE is parsed by `matchers.py`, i.e. the same mini-language as the form fields +# (comma lists, ranges, `!`, `>` `<`, wildcards, `re:`, CIDR). Nothing new to learn +# and, more to the point, nothing new to maintain: a second syntax for the same job +# would drift from the first one on its first edit (convention 10). +# +# Bare text keeps working exactly as before, so no existing habit breaks. +# +# `kind` is what the value is parsed as; `get` pulls the comparable value off a row. +# PROCESS is the text kind here: it is the one that understands wildcards, `re:` and +# `!` on names, which is what "search a text column" means. +SEARCH_FIELDS = { + "proc": (KIND_PROCESS, None), # special: matched with (pid, name) + "pid": (KIND_INT, lambda c, m: c.get("pid")), + "proto": (KIND_PROCESS, lambda c, m: c.get("proto")), + "dir": (KIND_PROCESS, lambda c, m: c.get("dir")), + "ip": (KIND_IP, lambda c, m: c.get("remote_ip")), + "port": (KIND_INT, lambda c, m: c.get("remote_port")), + "lport": (KIND_INT, lambda c, m: c.get("local_port")), + "packets": (KIND_INT, lambda c, m: c.get("packets")), + "dropped": (KIND_INT, lambda c, m: c.get("dropped")), + "down": (KIND_INT, lambda c, m: c.get("sent_in")), + "up": (KIND_INT, lambda c, m: c.get("sent_out")), + "bytes": (KIND_INT, lambda c, m: c.get("sent")), +} + +# The one genuine boolean on a row. It gets words rather than an expression because +# `scoped:yes` reads like a question and `scoped:1` reads like a bug report about the +# search box. "Has drops" needs no boolean of its own - that is `dropped:>0`. +BOOL_FIELDS = {"scoped"} +TRUE_WORDS = {"yes", "y", "true", "1", "tak"} +FALSE_WORDS = {"no", "n", "false", "0", "nie"} + +_BOUNDS = {"port": PORT_BOUNDS, "lport": PORT_BOUNDS} + class SearchIndex: """Cached lowercase search text, one entry per row. @@ -137,11 +182,71 @@ def _connection_blob(c, proc_map=None): f"{c.get('local_port') or ''}").lower() +def compile_query(query): + """Turn a search string into a list of predicates, ONCE per query. + + Compiling per row would put the expression parser on the path of every one of + a hundred thousand rows on every keystroke, which is the shape of the problem + ``SearchIndex`` exists to solve, reintroduced one layer up. So parse here and + return closures; the row loop then only calls them. + + A term is either ``field:value`` or bare text. Terms are ANDed, because that is + what narrowing means and what every search box a tester has used does. + + An unparsable value is NOT an error: the box is typed into character by + character, so `port:44` is a valid search on the way to `port:443` and half of + `ip:10.0.` must not throw or blank the table. Such a term simply matches + nothing until it becomes valid, and a term naming an unknown field falls back + to plain text - `http://x` is a URL someone pasted, not a field called `http`. + """ + tests = [] + for raw in str(query or "").split(): + field, sep, value = raw.partition(":") + field = field.lower() + if not sep or (field not in SEARCH_FIELDS and field not in BOOL_FIELDS): + text = raw.lower() + tests.append(lambda c, m, t=text: t in _connection_blob(c, m)) + continue + if field in BOOL_FIELDS: + want = value.strip().lower() + if want in TRUE_WORDS: + tests.append(lambda c, m, k=field: bool(c.get(k))) + elif want in FALSE_WORDS: + tests.append(lambda c, m, k=field: not c.get(k)) + else: # half-typed: match nothing yet + tests.append(lambda c, m: False) + continue + kind, getter = SEARCH_FIELDS[field] + try: + matcher = parse_matcher(value, kind, f"fields.{field}", + bounds=_BOUNDS.get(field)) + except ValueError: + tests.append(lambda c, m: False) + continue + if not matcher: # `port:` with nothing after it + continue + if field == "proc": + # The process kind judges (pid, name) together, exactly as the target + # field does - so `proc:1234` and `proc:chrome` both work here for the + # same reason they both work there. + tests.append(lambda c, m, x=matcher: x.matches(c.get("pid"), + connection_proc(c, m))) + elif kind == KIND_PROCESS: + # A text column is a process matcher with no pid: the value goes in the + # NAME position. Passing it positionally instead gives it to `pid`, + # where a name cannot be evaluated - and an unevaluable term quietly + # matches nothing, so `proto:tcp` found zero rows while looking correct. + tests.append(lambda c, m, x=matcher, g=getter: x.matches(None, g(c, m))) + else: + tests.append(lambda c, m, x=matcher, g=getter: x.matches(g(c, m))) + return tests + + def _filter_connections(conns, query, proc_map): - q = (query or "").strip().lower() - if not q: + tests = compile_query(query) + if not tests: return list(conns) - return [c for c in conns if q in _connection_blob(c, proc_map)] + return [c for c in conns if all(t(c, proc_map) for t in tests)] def traffic_totals(conns, query="", proc_map=None): diff --git a/lang/en.json b/lang/en.json index c6bb671..f91e723 100644 --- a/lang/en.json +++ b/lang/en.json @@ -84,6 +84,8 @@ "dialogs.buffer_help_title": "Buffer - how to choose", "dialogs.confirm_close_running": "The simulation is still running. Stop it and close the application?", "dialogs.confirm_close_title": "Session in progress", + "dialogs.conn_search_help": "Type any text to search the process, protocol, direction, addresses and ports of every row, exactly as before.\n\nTo search ONE column, put its name and a colon in front of the value:\n\n port:443 one port\n port:53,8080 either of two\n port:8000-8100 a range\n ip:10.0.0.0/8 a whole subnet\n ip:!192.168.* everything except\n proc:chrome a process by name, or by PID\n pid:>4000 a comparison\n scoped:yes only the rows this session impaired\n dropped:>0 only the rows that lost something\n\nColumn names: proc, pid, proto, dir, ip, port, lport, packets, dropped, down, up, bytes, scoped.\n\nWrite several and a row has to satisfy all of them:\n\n proc:chrome port:443 dropped:>0\n\nThe values use the same notation as the fields on the Control page, so a comma is \"either\", a dash is a range and \"!\" means \"not this\".\n\nA half-typed value simply finds nothing until it is complete. The count under the table always tells you how many rows matched.", + "dialogs.conn_search_help_title": "How to search", "dialogs.install_pydivert": "Install:\n\npip install pydivert", "dialogs.internal_error": "An unexpected error occurred in the interface:\n\n{e}\n\nThe app keeps running. If a session is active it is still running - press STOP to end it. Details are in the log.", "dialogs.internal_error_title": "Internal error", @@ -437,7 +439,8 @@ "tips.col_up": "Data that actually LEFT this machine on this connection, in kilobytes - what the application uploaded. Compare it with \"up seen\": the gap is what the tool destroyed.", "tips.col_up_seen": "Data the tool CAPTURED going out on this connection, before any impairment, in kilobytes - what the application tried to send. The difference from \"up\" is what never left.", "tips.confirm_close": "When on, closing the window during a running capture asks for confirmation first. Turn off to close without being asked.", - "tips.conn_search": "Filter connections by IP address, port or direction (e.g. '443', '10.0.0', 'out').", + "tips.conn_search": "Narrows the table. Plain text searches every column, or write port:443 or ip:10.0.0.0/8 to search one. Click ? for the full list.", + "tips.conn_search_help": "How to search this table", "tips.conn_table": "Who (process) talks to whom (IP:port). 'time[s]' is the connection duration, 'idle[s]' is seconds since the last packet. Click a header to sort. The list scrolls vertically and horizontally.", "tips.copy_cli": "Copies to the clipboard a ready CLI command that reproduces these conditions (a frozen .exe uses its own name).", "tips.corrupt": "Percent of packets with one flipped data bit - tests resilience to corrupted data.", diff --git a/lang/pl.json b/lang/pl.json index 57e9d7a..c0a317a 100644 --- a/lang/pl.json +++ b/lang/pl.json @@ -84,6 +84,8 @@ "dialogs.buffer_help_title": "Bufor - jak dobrać", "dialogs.confirm_close_running": "Symulacja nadal działa. Zatrzymać ją i zamknąć aplikację?", "dialogs.confirm_close_title": "Sesja w toku", + "dialogs.conn_search_help": "Wpisz dowolny tekst, a wyszukiwarka przejrzy proces, protokol, kierunek, adresy i porty kazdego wiersza - tak jak dotad.\n\nZeby szukac w JEDNEJ kolumnie, podaj jej nazwe i dwukropek przed wartoscia:\n\n port:443 jeden port\n port:53,8080 jeden albo drugi\n port:8000-8100 zakres\n ip:10.0.0.0/8 cala podsiec\n ip:!192.168.* wszystko oprocz\n proc:chrome proces po nazwie albo po PID\n pid:>4000 porownanie\n scoped:yes tylko wiersze psute w tej sesji\n dropped:>0 tylko te, ktore cos stracily\n\nNazwy kolumn: proc, pid, proto, dir, ip, port, lport, packets, dropped, down, up, bytes, scoped.\n\nMozesz wpisac kilka naraz - wiersz musi spelnic wszystkie:\n\n proc:chrome port:443 dropped:>0\n\nWartosci zapisuje sie tak samo jak w polach na stronie Sterowanie, wiec przecinek znaczy \"albo\", myslnik zakres, a \"!\" znaczy \"nie to\".\n\nNiedokonczona wartosc po prostu nic nie znajduje, dopoki jej nie dopiszesz. Licznik pod tabela zawsze mowi, ile wierszy pasuje.", + "dialogs.conn_search_help_title": "Jak szukac", "dialogs.install_pydivert": "Zainstaluj:\n\npip install pydivert", "dialogs.internal_error": "Wystąpił nieoczekiwany błąd interfejsu:\n\n{e}\n\nAplikacja działa dalej. Jeśli sesja jest aktywna, nadal trwa - naciśnij STOP, aby ją zakończyć. Szczegóły są w logu.", "dialogs.internal_error_title": "Błąd wewnętrzny", @@ -437,7 +439,8 @@ "tips.col_up": "Dane, które faktycznie WYSZŁY z tej maszyny na tym połączeniu, w kilobajtach - czyli ile aplikacja wysłała. Porównaj z „wys. widz.”: różnica to szkoda, którą zrobiło narzędzie.", "tips.col_up_seen": "Dane PRZECHWYCONE przez narzędzie na wyjściu tego połączenia, przed zakłóceniami, w kilobajtach - czyli ile aplikacja próbowała wysłać. Różnica względem „wysłane” to to, co nie wyszło.", "tips.confirm_close": "Gdy włączone, zamknięcie okna w trakcie przechwytywania najpierw pyta o potwierdzenie. Wyłącz, by zamykać bez pytania.", - "tips.conn_search": "Filtruj połączenia po adresie IP, porcie lub kierunku (np. '443', '10.0.0', 'out').", + "tips.conn_search": "Zawezenie tabeli. Zwykly tekst szuka po wszystkich kolumnach, a port:443 czy ip:10.0.0.0/8 w jednej. Pelna lista pod przyciskiem ?.", + "tips.conn_search_help": "Jak szukac w tej tabeli", "tips.conn_table": "Kto (proces) z kim (IP:port) gada. 'czas[s]' to czas trwania połączenia, 'nieakt.[s]' to ile sekund minęło od ostatniego pakietu. Klik w nagłówek sortuje. Lista przewija się pionowo i poziomo.", "tips.copy_cli": "Kopiuje do schowka gotową komendę CLI, która odtwarza te warunki (w zbudowanym .exe użyje jego nazwy).", "tips.corrupt": "Procent pakietów z przekłamanym jednym bitem danych - test odporności na uszkodzone dane.", diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index f64450c..0ddb3bd 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -67,6 +67,13 @@ " self.log(f\"{T('log.target_set')}: {expression}\")"), "test": "test_a_row_action_fills_the_form_and_does_not_reach_a_running_engine", }, + { + "label": "search: a text column is judged in the pid position again", + "file": "beantester/views.py", + "old": " tests.append(lambda c, m, x=matcher, g=getter: x.matches(None, g(c, m)))", + "new": " tests.append(lambda c, m, x=matcher, g=getter: x.matches(g(c, m)))", + "test": "test_a_text_column_is_matched_case_insensitively", + }, { "label": "guards: the repository collector returns nothing", "file": "tests/test_repo_conventions.py", diff --git a/tests/test_views.py b/tests/test_views.py index c0e4a0d..f9ecb15 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -289,3 +289,114 @@ def test_search_index_clear_empties_the_cache(): idx.blob({"id": 1, "t": "x"}) idx.clear() check("clear() empties the cache", idx._cache == {}) + + +# --- field-qualified search --------------------------------------------------- # +def _search_rows(): + return [ + {"proc": "chrome.exe", "pid": 100, "proto": "TCP", "dir": "out", + "remote_ip": "8.8.8.8", "remote_port": 53, "local_port": 50001, + "packets": 10, "dropped": 0, "scoped": True, + "sent": 900, "sent_in": 500, "sent_out": 400}, + {"proc": "chrome.exe", "pid": 100, "proto": "UDP", "dir": "out", + "remote_ip": "10.1.2.3", "remote_port": 443, "local_port": 50002, + "packets": 99, "dropped": 5, "scoped": False, + "sent": 10, "sent_in": 5, "sent_out": 5}, + {"proc": "msedge.exe", "pid": 900, "proto": "TCP", "dir": "in", + "remote_ip": "192.168.0.9", "remote_port": 8080, "local_port": 50003, + "packets": 1, "dropped": 0, "scoped": False, + "sent": 1, "sent_in": 1, "sent_out": 0}, + ] + + +def _found(query): + from beantester.views import _filter_connections + return [f"{r['proc']}:{r['remote_port']}" + for r in _filter_connections(_search_rows(), query, None)] + + +def test_plain_text_search_still_works_exactly_as_before(): + """The old behaviour is the default, so no existing habit breaks.""" + check("empty query returns everything", len(_found("")) == 3) + check("a bare word still matches the blob", _found("chrome") == + ["chrome.exe:53", "chrome.exe:443"], f"({_found('chrome')})") + check("a bare word that matches nothing returns nothing", _found("zzz") == []) + + +def test_a_term_can_name_its_column(): + """The point of the feature: 6 of 17 columns used to be searchable at all. + + A PID is on screen and could not be searched for, and neither could "only the + rows this session impaired" - which is the question a tester has when the + table holds a hundred thousand flows. + """ + check("port", _found("port:443") == ["chrome.exe:443"]) + check("pid, which plain text cannot reach", _found("pid:>500") == ["msedge.exe:8080"]) + check("scoped, which plain text cannot reach", _found("scoped:yes") == ["chrome.exe:53"]) + check("dropped with a comparison", _found("dropped:>0") == ["chrome.exe:443"]) + check("a text column", _found("dir:in") == ["msedge.exe:8080"]) + + +def test_the_values_use_the_expression_language_the_form_fields_use(): + """Comma lists, ranges, negation and CIDR - not a second syntax of our own.""" + check("a comma list", _found("port:53,8080") == ["chrome.exe:53", "msedge.exe:8080"]) + check("a range", _found("port:8000-8100") == ["msedge.exe:8080"]) + check("CIDR", _found("ip:10.0.0.0/8") == ["chrome.exe:443"]) + check("negation with a wildcard", + _found("ip:!192.168.*") == ["chrome.exe:53", "chrome.exe:443"]) + check("a process term takes a PID too, exactly like the target field", + _found("proc:100") == ["chrome.exe:53", "chrome.exe:443"]) + + +def test_a_text_column_is_matched_case_insensitively(): + """`proto:tcp` must find rows holding "TCP". + + This failed in the first implementation, and silently: a process matcher + judges (pid, name), so passing the value positionally handed it to `pid`, + where a name cannot be evaluated - and an unevaluable term matches nothing. + The search looked like it worked and found zero rows. + """ + check("lowercase query, uppercase data", + _found("proto:tcp") == ["chrome.exe:53", "msedge.exe:8080"]) + check("uppercase query, uppercase data", + _found("proto:TCP") == ["chrome.exe:53", "msedge.exe:8080"]) + check("the field name itself is case-insensitive", + _found("PROC:CHROME") == ["chrome.exe:53", "chrome.exe:443"]) + check("negation on a text column", + _found("proto:!udp") == ["chrome.exe:53", "msedge.exe:8080"]) + + +def test_several_terms_narrow_together(): + check("two columns", _found("proc:chrome port:443") == ["chrome.exe:443"]) + check("a column and plain text", _found("chrome port:53") == ["chrome.exe:53"]) + check("three terms", _found("scoped:no dropped:>0") == ["chrome.exe:443"]) + + +def test_a_half_typed_query_finds_nothing_instead_of_throwing(): + """The box is typed into character by character. + + `port:44` on the way to `port:443` and half of `ip:10.0.` must not raise and + must not blank the table with a traceback in the crash log. An unknown field + is not an error either: `http://x` is a URL someone pasted, not a field. + """ + check("an incomplete port matches nothing", _found("port:44") == []) + check("an incomplete address matches nothing", _found("ip:10.0.") == []) + check("an unparsable value matches nothing", _found("port:!!!") == []) + check("an unknown field falls back to plain text", _found("nosuchfield:x") == []) + check("a pasted URL does not explode", _found("http://8.8.8.8") == []) + # A lone colon is plain text, so it matches every row whose blob contains one - + # which is all of them, since the blob joins an address to its port. Keeping + # everything visible is the right answer while someone is halfway through + # typing `port:`: blanking the table on the way to a valid query reads as a + # bug. Asserted as it BEHAVES rather than as first expected. + check("a lone colon is plain text and keeps the table populated", + len(_found(":")) == 3, f"({_found(':')})") + + +def test_the_query_is_compiled_once_not_per_row(): + """Parsing per row would put the expression parser on the path of every one of + a hundred thousand rows on every keystroke.""" + from beantester.views import compile_query + tests = compile_query("proc:chrome port:443 dropped:>0") + check("one predicate per term", len(tests) == 3, f"({len(tests)})") + check("an empty query compiles to nothing to do", compile_query(" ") == []) From f5f08e4aa79723536f5cd879c0b9368e39c91218 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 20:12:35 +0200 Subject: [PATCH 3/4] refactor(gui): move the connection row actions onto the page The two new row actions and their shared helper were written as App methods and pushed gui/app.py from 1299 to 1319 logic lines, which turned the size ratchet red. The ratchet's answer is to put code where it belongs rather than to raise the number, and a Connections row action belongs to the Connections page. app.py is back to its previous size and conns.py, at less than half the ceiling, absorbs three short functions that already reach into app the way the rest of that page does. This also fixes a commit that should not have been made: the previous one landed with test_code_shape red, because the verification pipeline piped pytest into tail and therefore reported tail's exit code rather than pytest's. Co-Authored-By: Claude Opus 5 --- beantester/gui/app.py | 43 --------------------------- beantester/gui/pages/conns.py | 56 +++++++++++++++++++++++++++++++++-- tests/test_gui_state.py | 16 +++++----- 3 files changed, 63 insertions(+), 52 deletions(-) diff --git a/beantester/gui/app.py b/beantester/gui/app.py index bec2e3b..439e8c4 100644 --- a/beantester/gui/app.py +++ b/beantester/gui/app.py @@ -33,7 +33,6 @@ from ..fields import SEED as F_SEED from ..fields import FIELD_DEFS, SECTIONS, UI_ONLY_KEYS, off_value from ..filters import cli_key_for, i18n_key_for, i18n_keys, windivert_for -from ..matchers import add_term from .. import crashlog from ..i18n import (FALLBACK_LANGUAGE, T, available_languages, current_language, set_language) @@ -1237,48 +1236,6 @@ def set_destination(self, ip, port): if self.running: self.log(T("log.apply_needed")) - def _append_to_field(self, key, term, log_key): - """Add one term to an expression field, keeping what is already there. - - The row actions below build a field up click by click - block this - address, then that one - so they append. Replacing would throw away the - addresses blocked a moment ago, which is the opposite of what the second - click means. ``matchers.add_term`` owns the syntax (convention 10): it - drops repeats, keeps the comma escape of a regex intact and never leaves - an empty term behind. - - Like every other row action, this only fills the form: convention 15, and - the running session hears about it through the same "apply needed" line. - """ - current = self.vars[key].get() - updated = add_term(current, term) - self.vars[key].set(updated) - self.form.set_values(self._settings_for_form()) - self.on_form_changed() - self.log(f"{T(log_key)}: {updated}") - if self.running: - self.log(T("log.apply_needed")) - - def block_ip_address(self, ip): - """Add an address to the blocking field (pipeline step 2c).""" - if not str(ip or "").strip(): - return - self._append_to_field("block_ip", str(ip).strip(), "log.block_ip_added") - - def leave_process_alone(self, name): - """Exclude a process from impairment by adding ``!name`` to the target. - - With a target already set this narrows it. With the target EMPTY it turns - "impair everything" into "impair everything except this one", because a - bare negative means exactly that in this expression language - which is - the case the menu entry is really for. - """ - name = str(name or "").strip() - if not name or name == "?": - self.log(T("log.no_process_for_row")) - return - self._append_to_field("target", f"!{name}", "log.process_excluded") - # -- scenario / config files ---------------------------------------------------- # def _update_scenario_label(self): if self.scenario_lbl is None: diff --git a/beantester/gui/pages/conns.py b/beantester/gui/pages/conns.py index 21b2662..2c36d77 100644 --- a/beantester/gui/pages/conns.py +++ b/beantester/gui/pages/conns.py @@ -26,6 +26,7 @@ from tkinter import ttk from ...i18n import T +from ...matchers import add_term from ...views import (avg_packet_bytes, connection_proc, filter_sort_connections, traffic_totals) from .. import dialogs @@ -98,6 +99,57 @@ def port_cell(port): REBUILD_MS = 1000 + +def append_to_field(app, key, term, log_key): + """Add one term to an expression field, keeping what is already there. + + The row actions build a field up click by click - block this address, then + that one - so they append. Replacing would throw away what the previous click + put there, which is the opposite of what the second click means. + ``matchers.add_term`` owns the syntax (convention 10): it drops repeats, keeps + the comma escape of a regex intact and never leaves an empty term behind. + + Like every other row action this only fills the form (convention 15): the + running session hears about it through the same "apply needed" line, and + nothing reaches the engine until the user presses Apply. + + It lives on the PAGE rather than on ``App`` for a measured reason: ``app.py`` + was already the largest module in the package and sits on the size ratchet in + ``tests/test_code_shape.py``, which went red when these three helpers were + added there. The ratchet's answer is to put code where it belongs rather than + to raise the number, and a Connections row action belongs to the Connections + page. + """ + updated = add_term(app.vars[key].get(), term) + app.vars[key].set(updated) + app.form.set_values(app._settings_for_form()) + app.on_form_changed() + app.log(f"{T(log_key)}: {updated}") + if app.running: + app.log(T("log.apply_needed")) + + +def block_ip_address(app, ip): + """Add an address to the blocking field (decision pipeline step 2c).""" + if str(ip or "").strip(): + append_to_field(app, "block_ip", str(ip).strip(), "log.block_ip_added") + + +def leave_process_alone(app, name): + """Exclude a process from impairment by adding ``!name`` to the target. + + With a target already set this narrows it. With the target EMPTY it turns + "impair everything" into "impair everything except this one", because a bare + negative means exactly that in this expression language - and that is the case + the menu entry is really for. + """ + name = str(name or "").strip() + if not name or name == "?": + app.log(T("log.no_process_for_row")) + return + append_to_field(app, "target", f"!{name}", "log.process_excluded") + + class ConnsPage: ID = "connections" LABEL = "app.tabs.connections" @@ -260,7 +312,7 @@ def _leave_process_alone(self): row = self._selected() if not row: return - self.app.leave_process_alone(str(row.get("proc") or "").strip()) + leave_process_alone(self.app, str(row.get("proc") or "").strip()) def _limit_dest(self): row = self._selected() @@ -273,7 +325,7 @@ def _block_ip(self): row = self._selected() if not row: return - self.app.block_ip_address(str(row.get("remote_ip") or "")) + block_ip_address(self.app, str(row.get("remote_ip") or "")) # -- search -------------------------------------------------------------- # def _schedule_search(self): diff --git a/tests/test_gui_state.py b/tests/test_gui_state.py index 041fa96..f8b0dcd 100644 --- a/tests/test_gui_state.py +++ b/tests/test_gui_state.py @@ -206,19 +206,21 @@ def test_blocking_and_excluding_from_a_row_accumulate(): except this", which is the case the menu entry exists for. """ run_gui(""" - app.block_ip_address("8.8.8.8") - app.block_ip_address("1.1.1.1") - app.block_ip_address("8.8.8.8") + from beantester.gui.pages.conns import block_ip_address, leave_process_alone + + block_ip_address(app, "8.8.8.8") + block_ip_address(app, "1.1.1.1") + block_ip_address(app, "8.8.8.8") assert app._settings_from_widgets()["block_ip"] == "8.8.8.8,1.1.1.1" - app.leave_process_alone("chrome.exe") - app.leave_process_alone("msedge.exe") + leave_process_alone(app, "chrome.exe") + leave_process_alone(app, "msedge.exe") assert app._settings_from_widgets()["target"] == "!chrome.exe,!msedge.exe" # a row with no known process must not write "!?" into the target before = app._settings_from_widgets()["target"] - app.leave_process_alone("?") - app.leave_process_alone("") + leave_process_alone(app, "?") + leave_process_alone(app, "") assert app._settings_from_widgets()["target"] == before """) From d24cbc1769a22a17753f545189ddb57192182fee Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 2 Aug 2026 20:20:53 +0200 Subject: [PATCH 4/4] test(matchers): make the escape docstring raw so the suite stops warning The docstring pinning the comma-escape regression contained a literal backslash comma in a non-raw string, which Python 3.14 reports as an invalid escape sequence. The suite ran green with two SyntaxWarnings, and a warning nobody fixes is a warning nobody reads. Co-Authored-By: Claude Opus 5 --- tests/test_matchers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_matchers.py b/tests/test_matchers.py index 29c9686..0b8aadd 100644 --- a/tests/test_matchers.py +++ b/tests/test_matchers.py @@ -365,7 +365,7 @@ def test_add_term_appends_without_breaking_the_expression(): def test_add_term_keeps_the_comma_escape_of_a_regex(): - """The bug this function shipped with for about ten minutes, now pinned. + r"""The bug this function shipped with for about ten minutes, now pinned. ``split_terms`` turns ``\,`` into a literal comma INSIDE the term. Re-joining without escaping it emits that comma as a SEPARATOR, so one regex silently