diff options
| -rw-r--r-- | docs/TEXTUAL_NOTES.md | 30 | ||||
| -rw-r--r-- | idatui/app.py | 121 | ||||
| -rw-r--r-- | idatui/project.py | 23 | ||||
| -rw-r--r-- | idatui/worker.py | 9 | ||||
| -rw-r--r-- | tests/test_scenarios.py | 25 |
5 files changed, 196 insertions, 12 deletions
diff --git a/docs/TEXTUAL_NOTES.md b/docs/TEXTUAL_NOTES.md index 4bf7fcb..059b47c 100644 --- a/docs/TEXTUAL_NOTES.md +++ b/docs/TEXTUAL_NOTES.md @@ -80,3 +80,33 @@ Hard-won Textual behaviour and the patterns this app relies on. Pairs with failures. Re-run on a warm session before trusting a regression. - Edits mutate the `.i64` — revert renames (via API) for idempotency; use a PID-unique temp name to dodge collisions from a prior crashed run. + + +## A priority app binding fires even under a modal + +`Binding("tab,shift+tab", "toggle_view", priority=True)` on the App runs before +the focus chain, so Tab never reached ANY dialog — nothing in a modal could be +tabbed to, in this app, ever. It looked like a bug in one dialog (the load +options address field was unreachable) and was actually app-wide. + +The fix is in the action, not the binding: if a modal is up, hand the key back. + +```python +def action_toggle_view(self): + if self.screen is not self.screen_stack[0]: + self.screen.focus_next() + return +``` + +Related: DOM order is rarely the tab order you want. In a dialog with a filter +box, an option list and a second field, `focus_next()` stops at the list — which +is arrow-driven and has nothing to type — so text meant for the second field +lands in the filter. Override `focus_next`/`focus_previous` on the screen to +cycle the fields people actually type into. + +## A modal can outgrow the terminal and clip its own controls + +`#pal-list { max-height: 24 }` plus a 21-row list pushed the address field and +help line off the bottom of the screen. Nothing errors; the field is simply not +there, which reads to the user as "Tab does nothing". Cap the scrollable part +per-dialog so the controls below it are always visible. diff --git a/idatui/app.py b/idatui/app.py index f861f42..87d8f70 100644 --- a/idatui/app.py +++ b/idatui/app.py @@ -2566,6 +2566,24 @@ class LoadOptionsScreen(ModalScreen): self._apply("") self.query_one("#pal-input", Input).focus() + def focus_next(self, selector="*"): # type: ignore[override] + """Tab moves between the two things you TYPE into. + + DOM order would stop at the option list on the way, which is + arrow-driven and has nothing to type — and the address you meant to + enter goes into whichever box happened to have focus. Typing an address + into the processor filter is then taken as a processor name, IDA rejects + it, and the open fails; that is a bad enough outcome to be worth + overriding Tab for. + """ + inp = self.query_one("#pal-input", Input) + base = self.query_one("#load-base", Input) + (inp if self.focused is base else base).focus() + return self.focused + + def focus_previous(self, selector="*"): # type: ignore[override] + return self.focus_next() + def on_input_changed(self, event: Input.Changed) -> None: event.stop() if event.input.id == "pal-input": @@ -3238,6 +3256,10 @@ class IdaTui(App): #pal-input { border: none; height: 1; margin: 0 1; background: $panel; color: $text; } #pal-list { height: auto; max-height: 24; } #load-note { height: 2; padding: 1 1 0 1; color: $text-muted; } + /* Cap the processor list so the ADDRESS FIELD is always on screen: with the + palette default (24) the box outgrew the terminal and the field you need + was clipped off the bottom, which read as "Tab does nothing". */ + LoadOptionsScreen #pal-list { max-height: 12; } #load-base { border: none; height: 1; margin: 1 1 0 1; background: $panel; color: $text; } #load-help { height: 1; padding: 0 1; color: $text-muted; } StructEditor { align: center middle; } @@ -3298,6 +3320,8 @@ class IdaTui(App): self._pending_restore = None # entry to reopen after a switch self._goto_after_switch = None # cross-binary search hit to land on self._hops: list[str] = [] # binaries a navigation crossed FROM + self._load_for_label = None # project binary the dialog is for + self._pending_switch = None # switch waiting on that answer self._nav_seq = 0 # bumped per navigation; drops stale ones # None = teardown wasn't an explicit quit (crash/kill): save defensively. # False = the user chose discard, or we already saved on the way out. @@ -3412,8 +3436,13 @@ class IdaTui(App): # A file no loader recognises has to be described before it can be # opened, so ask BEFORE the worker starts — once IDA has made a database # the answer is baked in and changing it means deleting the .i64. - if self._should_ask_load_options(): - self._ask_load_options() + if self._project is not None: + ref = self._pending_load_ref() + if ref is not None: + self._ask_load_options(ref.source, label=ref.label) + return + elif self._should_ask_load_options(): + self._ask_load_options(self._open_path) return # Show a loading overlay immediately so a slow open/analysis (big binary) # isn't just dead air behind empty panes; dismissed once we land. @@ -3429,8 +3458,8 @@ class IdaTui(App): re-passing switches fails the open); or the file is a format IDA recognises, which is nearly always. """ - if self._project is not None or not self._open_path: - return False # project mode carries per-binary options already + if not self._open_path: + return False if self._load_args: return False from .formats import needs_load_options @@ -3439,21 +3468,66 @@ class IdaTui(App): return False return needs_load_options(self._open_path) - def _ask_load_options(self) -> None: + def _retry_load_options(self) -> None: + """Re-ask after IDA refused what we told it.""" + path, label = self._open_path, None + if self._project is not None and self._binary is not None: + ref = self._project.by_label(self._binary) + if ref is not None: + path, label = ref.source, ref.label + # Clear the rejected answer or _pending_load_ref would see the + # binary as already described and never ask again. + self._project.set_load(label, processor="", base=0) + self._project._entries[self._project._refs.index(ref)].pop( + "processor", None) + self._project.save() + self._load_args = "" + if path: + self._status("those load options were rejected \u2014 try again") + self._ask_load_options(path, label=label) + + def _pending_load_ref(self, label: str | None = None): # type: ignore[no-untyped-def] + """The project binary about to be opened, if it needs describing. + + Checked against the SOURCE: staging may not have happened yet, and the + question is about the bytes, not where they were copied to. + """ + if self._project is None: + return None + label = label or self._binary or self._project.refs[0].label + ref = self._project.by_label(label) + if ref is None or ref.load_args: + return None + if os.path.exists(ref.db) or os.path.exists( + os.path.splitext(ref.staged)[0] + ".i64"): + return None # already analysed: the .i64 records how + from .formats import needs_load_options + return ref if needs_load_options(ref.source) else None + + def _ask_load_options(self, path: str, label: str | None = None) -> None: try: - size = os.path.getsize(self._open_path) + size = os.path.getsize(path) except OSError: size = 0 - self.push_screen(LoadOptionsScreen(self._open_path, size), - self._on_load_options) + self._load_for_label = label + self.push_screen(LoadOptionsScreen(path, size), self._on_load_options) def _on_load_options(self, choice) -> None: # type: ignore[no-untyped-def] from .formats import load_args choice = choice or {} - if choice.get("processor"): - self._load_args = load_args(choice["processor"], choice.get("base", 0)) - self._status(f"loading as {choice['processor']} " - f"@ {choice.get('base', 0):#x}") + label, self._load_for_label = self._load_for_label, None + proc, base = choice.get("processor", ""), int(choice.get("base", 0) or 0) + if proc: + if label is not None and self._project is not None: + # Persist it: the answer belongs to the binary, not to this run. + self._project.set_load(label, proc, base) + else: + self._load_args = load_args(proc, base) + self._status(f"loading as {proc} @ {base:#x}") + if self._pending_switch is not None: + label2, self._pending_switch = self._pending_switch, None + self._switch_binary(label2) + return self._loading_screen = LoadingScreen(self._loading_title()) self.push_screen(self._loading_screen) self._connect() @@ -3600,6 +3674,12 @@ class IdaTui(App): except Exception as e: # noqa: BLE001 self.app.call_from_thread(self._status, f"connect failed: {e}") self.app.call_from_thread(self._dismiss_loading) + # A load we described ourselves and IDA refused: offer the dialog + # again rather than leaving an empty app with an error in the status + # bar. Getting the processor wrong is an ordinary mistake and should + # cost one more keypress, not a restart. + if "load options" in str(e): + self.app.call_from_thread(self._retry_load_options) return self.client = client self.program = program @@ -3993,6 +4073,13 @@ class IdaTui(App): self._switch_binary(label) def _switch_binary(self, label: str) -> None: + # A blob nobody has described yet has to be described before its worker + # opens it — same as at boot, just reached by switching instead. + ref = self._pending_load_ref(label) + if ref is not None and self._load_for_label is None: + self._pending_switch = label + self._ask_load_options(ref.source, label=label) + return # Snapshot what we're leaving so coming back restores the view, then let # the pool hand us a worker (spawning + evicting as the budget dictates). if self._binary is not None: @@ -4134,6 +4221,16 @@ class IdaTui(App): def action_toggle_view(self) -> None: """Tab: switch the code pane between disassembly and pseudocode (or leave the hex view back to the preferred code view).""" + # Tab is a PRIORITY app binding, so it fires even while a modal is up and + # nothing inside a dialog could ever be tabbed to. Hand it back to the + # dialog: this is the only reason the load dialog's address field was + # unreachable, and it was broken the same way in every other modal. + if self.screen is not self.screen_stack[0]: + try: + self.screen.focus_next() + except Exception: # noqa: BLE001 -- screen with nothing focusable + pass + return if self._cur is None: return if self._split: diff --git a/idatui/project.py b/idatui/project.py index 8f2674d..e2fc542 100644 --- a/idatui/project.py +++ b/idatui/project.py @@ -239,6 +239,29 @@ class Project: def refs(self) -> tuple[BinaryRef, ...]: return self._refs + def set_load(self, label: str, processor: str = "", base: int = 0, + ida_args: str = "") -> BinaryRef | None: + """Record how ``label`` should be loaded, and persist it. + + Answered once: the dialog that asks writes the answer here, so reopening + the project doesn't ask again — and neither does adding the same blob to + another project, since it travels with the entry. + """ + ref = self.by_label(label) + if ref is None: + return None + i = self._refs.index(ref) + e = self._entries[i] + if processor: + e["processor"] = processor + if base: + e["base"] = int(base) + if ida_args: + e["ida_args"] = ida_args + self._refs = self._build_refs() + self.save() + return self._refs[i] + def by_label(self, label: str) -> BinaryRef | None: return next((r for r in self._refs if r.label == label), None) diff --git a/idatui/worker.py b/idatui/worker.py index 26f0816..a4e3509 100644 --- a/idatui/worker.py +++ b/idatui/worker.py @@ -111,6 +111,15 @@ def _open_and_register(binpath: str, load_args: str = ""): args = None if idapro.open_database(binpath, run_auto_analysis=True, args=args): # nonzero == failure + if args: + # With load switches in play they are the likeliest culprit by far: + # IDA refuses an unknown -p name with no diagnostic of its own, so + # saying "the database is locked" here sends people hunting a + # problem they don't have. + raise RuntimeError( + f"failed to open {binpath} with load options {args!r}: IDA " + f"rejected them \u2014 an unknown processor name is the usual " + f"cause (see tools/verify_procs.py for the valid ones)") raise RuntimeError( f"failed to open {binpath}: the .i64 is likely held by a running " f"ida-mcp worker (try: pkill -f idalib) or wedged from a crash " diff --git a/tests/test_scenarios.py b/tests/test_scenarios.py index ee2f77b..6ccbf0d 100644 --- a/tests/test_scenarios.py +++ b/tests/test_scenarios.py @@ -369,6 +369,31 @@ async def s_load_options(c: Ctx): c.check("dialog output converts a base to paragraphs", load_args("arm", 0x8000000) == "-parm -b800000") + # Tab is a PRIORITY app binding (disasm<->pseudocode), so it fired even with + # a modal up and nothing in a dialog could be tabbed to. That is why the load + # dialog's address field was unreachable — and it was broken in every other + # modal too. + from idatui.app import LoadOptionsScreen + app.push_screen(LoadOptionsScreen("/tmp/probe.bin", 1234)) + await c.wait(lambda: isinstance(app.screen, LoadOptionsScreen), 10) + sc = app.screen + first = app.focused + await c.press("tab") + await c.pause(0.2) + c.check("Tab moves focus inside a modal instead of toggling the view", + app.focused is not first and isinstance(app.screen, LoadOptionsScreen), + f"focus={getattr(app.focused, 'id', None)}") + c.check("Tab in the load dialog lands on the address field", + getattr(app.focused, "id", None) == "load-base", + f"focus={getattr(app.focused, 'id', None)}") + await c.press("tab") + await c.pause(0.2) + c.check("Tab again returns to the processor filter", + getattr(app.focused, "id", None) == "pal-input", + f"focus={getattr(app.focused, 'id', None)}") + await c.press("escape") + await c.wait(lambda: not isinstance(app.screen, LoadOptionsScreen), 10) + @scenario("command_palette") async def s_command_palette(c: Ctx): |