fix(output,tui): neutralize control sequences and markup in server text (#206)

CHANGELOG.md

@@ -16,10 +16,18 @@ All notable changes to netbox-super-cli are tracked here. Format follows [Keep a
 - **Long error messages wrap inside the terminal error panel** instead of being
   cut off. For example, the conflict error's closing
   `re-run with --acknowledge-conflicts` hint used to disappear.
-- **Server text in terminal error panels can't send terminal control
-  sequences.** The error message, endpoint, operation id and conflict values
-  were printed raw, so an ESC sequence in an object name could retitle or clear
-  the terminal. Control and bidi characters now show as visible `\x..` escapes.
+- **Server data can't send terminal control sequences or Rich/Textual markup**
+  ([#206]). Rich's markup escaping doesn't strip ESC, so an object name containing
+  an escape sequence could retitle or clear your terminal or spoof text. Control
+  and bidi characters now show as visible `\x..`/`\u....` escapes in:
+  - table output, including column headers, which weren't escaped at all;
+  - the `--explain` panel;
+  - error panels;
+  - the TUI: list and detail cells, global search results, record picker, FK
+    buttons, column chooser, error notifications and diff views.
+
+  In the TUI, values were also parsed as markup, so a value like `[red]x[/]`
+  was styled and lost its brackets. They now render literally.
 - **Plugin endpoints get one command group per resource** ([#202]). NetBox tags
   every plugin endpoint `plugins`, so all of a plugin's endpoints were merged
   into one `nsc plugins <plugin>` resource whose verbs overwrote each other. For
@@ -221,6 +229,7 @@ notices, and a fix for `nsc login` reporting the wrong user.
 [#192]: https://github.com/thomaschristory/netbox-super-cli/issues/192
 [#196]: https://github.com/thomaschristory/netbox-super-cli/issues/196
 [#202]: https://github.com/thomaschristory/netbox-super-cli/issues/202
+[#206]: https://github.com/thomaschristory/netbox-super-cli/issues/206
 [#171]: https://github.com/thomaschristory/netbox-super-cli/issues/171
 [#179]: https://github.com/thomaschristory/netbox-super-cli/issues/179
 [#173]: https://github.com/thomaschristory/netbox-super-cli/issues/173

nsc/output/errors.py

@@ -7,21 +7,19 @@
 from __future__ import annotations
 
 import json
-import unicodedata
 from enum import Enum, StrEnum
 from typing import Any, Literal, TextIO
 
 from pydantic import BaseModel, ConfigDict, Field
 from rich import box
 from rich.console import Group, RenderableType
-from rich.markup import escape
 from rich.panel import Panel
 from rich.table import Table
-from rich.text import Text
 
 from nsc.config.models import OutputFormat
 from nsc.model.command_model import HttpMethod
 from nsc.output._console import make_console
+from nsc.output.sanitize import safe_markup, safe_text
 
 
 class ErrorType(StrEnum):
@@ -109,50 +107,35 @@ def render_to_rich_stderr(env: ErrorEnvelope, *, stream: TextIO, color: bool = F
     # which cut off e.g. the "re-run with --acknowledge-conflicts" hint.
     console = make_console(stream, color=color)
     # env.error/endpoint/operation_id/audit_log_path/details carry server- and
-    # user-sourced text: `_safe` neutralizes Rich markup and terminal control
+    # user-sourced text: `safe_markup` neutralizes Rich markup and terminal control
     # sequences (an object name must not retitle or clear the terminal).
     body_lines = [
-        f"[bold red]{env.type.value}[/]: {_safe(env.error)}",
+        f"[bold red]{env.type.value}[/]: {safe_markup(env.error)}",
     ]
     if env.endpoint:
-        body_lines.append(f"endpoint: {_safe(env.endpoint)}")
+        body_lines.append(f"endpoint: {safe_markup(env.endpoint)}")
     if env.method is not None:
         body_lines.append(f"method:   {env.method.value}")
     if env.status_code is not None:
         body_lines.append(f"status:   {env.status_code}")
     if env.operation_id:
-        body_lines.append(f"op:       {_safe(env.operation_id)}")
+        body_lines.append(f"op:       {safe_markup(env.operation_id)}")
     if env.attempt_n is not None:
         body_lines.append(f"attempt:  {env.attempt_n}")
     if env.audit_log_path:
-        body_lines.append(f"audit:    {_safe(env.audit_log_path)}")
+        body_lines.append(f"audit:    {safe_markup(env.audit_log_path)}")
     details = dict(env.details)
     conflicts = _conflict_table(details.get("conflicts"))
     if conflicts is not None:
         del details["conflicts"]
     if details:
-        body_lines.append(f"details:  {_safe(str(details))}")
+        body_lines.append(f"details:  {safe_markup(str(details))}")
     body: RenderableType = "\n".join(body_lines)
     if conflicts is not None:
         body = Group(body, conflicts)
     console.print(Panel(body, title="nsc error", border_style="red"))
 
 
-def _printable(text: str) -> str:
-    """Show control and format characters (ESC, BEL, bidi overrides, ...) as
-    visible escapes so server text cannot drive the terminal."""
-    return "".join(
-        ch
-        if ch in "\n\t" or not unicodedata.category(ch).startswith("C")
-        else ch.encode("unicode_escape").decode("ascii")
-        for ch in text
-    )
-
-
-def _safe(text: str) -> str:
-    return escape(_printable(text))
-
-
 _CONFLICT_SIDES = ("original", "branch", "main")
 
 
@@ -173,9 +156,8 @@ def _conflict_table(raw: Any) -> Table | None:
         names = [str(f) for f in fields] if isinstance(fields, list) and fields else [""]
         for i, name in enumerate(names):
             values = [_conflict_value(sides.get(side), name) for side in _CONFLICT_SIDES]
-            # Text cells are never parsed as markup; `_printable` handles control codes.
             cells = (obj if i == 0 else "", name, *values)
-            table.add_row(*(Text(_printable(c)) for c in cells))
+            table.add_row(*(safe_text(c) for c in cells))
     return table
 
 

nsc/output/explain.py

@@ -11,7 +11,6 @@
 from typing import Any, Literal, TextIO
 
 from pydantic import BaseModel, ConfigDict, Field
-from rich.markup import escape
 from rich.panel import Panel
 
 from nsc.cli.writes.apply import ResolvedRequest
@@ -20,6 +19,7 @@
 from nsc.cli.writes.preflight import PreflightResult
 from nsc.model.command_model import Operation
 from nsc.output._console import make_console
+from nsc.output.sanitize import safe_markup
 
 DECISION_CAP = 200
 
@@ -155,33 +155,33 @@ def render_to_json(trace: ExplainTrace) -> str:
 def render_to_rich_stdout(trace: ExplainTrace, *, stream: TextIO, color: bool = False) -> None:
     console = make_console(stream, color=color, soft_wrap=True)
     # Reasoning strings and decision values carry user/schema/server text that
-    # may contain [..]; escape it so it is not parsed as Rich markup.
+    # may contain [..] or control characters; `safe_markup` neutralizes both.
     body = [
-        f"[bold cyan]operation:[/] {escape(trace.operation_id)}",
+        f"[bold cyan]operation:[/] {safe_markup(trace.operation_id)}",
     ]
     if trace.operation_summary:
-        body.append(f"summary:   {escape(trace.operation_summary)}")
-    body.append(f"method:    {escape(trace.method_reasoning)}")
-    body.append(f"url:       {escape(trace.url_reasoning)}")
+        body.append(f"summary:   {safe_markup(trace.operation_summary)}")
+    body.append(f"method:    {safe_markup(trace.method_reasoning)}")
+    body.append(f"url:       {safe_markup(trace.url_reasoning)}")
     if trace.bulk_reasoning:
-        body.append(f"bulk:      {escape(trace.bulk_reasoning)}")
+        body.append(f"bulk:      {safe_markup(trace.bulk_reasoning)}")
     for r in trace.requests:
-        body.append(f"  → {r.method.value} {escape(r.url)}")
+        body.append(f"  → {r.method.value} {safe_markup(r.url)}")
         if r.body is not None:
-            body.append(f"    body: {escape(str(r.body))}")
+            body.append(f"    body: {safe_markup(str(r.body))}")
     if trace.preflight is not None and not trace.preflight.ok:
         body.append("preflight: [red]FAIL[/]")
         for issue in trace.preflight.issues:
             loc = f"records[{issue.record_index}].{issue.field_path}"
-            body.append(escape(f"  [{issue.kind}] {loc}: {issue.message}"))
+            body.append(safe_markup(f"  [{issue.kind}] {loc}: {issue.message}"))
     elif trace.preflight is not None:
         body.append("preflight: [green]OK[/]")
     if trace.decisions:
         body.append("decisions:")
         for d in trace.decisions:
             extra = f" — {d.note}" if d.note else ""
             body.append(
-                escape(
+                safe_markup(
                     f"  {d.field_path}  [{d.source}]  {d.raw_value!r} → {d.resolved_value!r}{extra}"
                 )
             )

nsc/output/sanitize.py

@@ -0,0 +1,35 @@
+"""Make server-sourced text safe to put on a terminal.
+
+Rich's `escape()` stops `[..]` markup, and Rich/Textual strip only BEL, BS, VT,
+FF and CR. ESC, other C0/C1 controls and bidi overrides still reach the
+terminal, so a NetBox object name could retitle or clear it, or spoof text
+(issue #206).
+"""
+
+from __future__ import annotations
+
+import unicodedata
+
+from rich.markup import escape
+from rich.text import Text
+
+
+def printable(text: str) -> str:
+    """Show control and format characters as visible `\\x..`/`\\u....` escapes."""
+    return "".join(
+        ch
+        if ch in "\n\t" or not unicodedata.category(ch).startswith("C")
+        else ch.encode("unicode_escape").decode("ascii")
+        for ch in text
+    )
+
+
+def safe_markup(text: str) -> str:
+    """`text` for interpolation into a Rich markup string."""
+    return escape(printable(text))
+
+
+def safe_text(text: str, style: str = "") -> Text:
+    """A Rich `Text` (never parsed as markup) for widgets that would otherwise
+    parse a plain `str` as markup, e.g. Textual's `DataTable` and `Label`."""
+    return Text(printable(text), style=style)

nsc/output/table.py

@@ -5,12 +5,12 @@
 import sys
 from typing import Any, TextIO
 
-from rich.markup import escape
 from rich.table import Table
 
 from nsc.output._console import make_console
 from nsc.output.colors import ColoredValue
 from nsc.output.flatten import flatten
+from nsc.output.sanitize import safe_markup
 
 _STATUS_COLORS: dict[str, str] = {
     "active": "green",
@@ -49,7 +49,7 @@ def render(
 
     table = Table(show_header=True, header_style="bold")
     for col in fieldnames:
-        table.add_column(header_labels.get(col, col) if header_labels else col)
+        table.add_column(safe_markup(header_labels.get(col, col) if header_labels else col))
     for r in flat_records:
         table.add_row(*[_format_cell(r.get(col, ""), color=color) for col in fieldnames])
 
@@ -65,7 +65,7 @@ def _gather_fieldnames(records: list[dict[str, Any]]) -> list[str]:
 
 
 def _colored_markup(value: ColoredValue) -> str:
-    inner = escape(value.text)
+    inner = safe_markup(value.text)
     if value.color is None:
         return inner
     return f"[#{value.color}]{inner}[/]"
@@ -75,9 +75,9 @@ def _format_colored(value: ColoredValue | list[ColoredValue], *, color: bool) ->
     # Object colors only ever reach here when color is on (gated in render), but
     # escape unconditionally so a bare ColoredValue can't inject markup.
     if isinstance(value, ColoredValue):
-        return _colored_markup(value) if color else escape(value.text)
+        return _colored_markup(value) if color else safe_markup(value.text)
     if not color:
-        return escape(", ".join(v.text for v in value))
+        return safe_markup(", ".join(v.text for v in value))
     return ", ".join(_colored_markup(v) for v in value)
 
 
@@ -99,14 +99,14 @@ def _format_cell(value: Any, *, color: bool = False) -> str:
         text = str(value)
 
     # Rich parses markup in table cells regardless of the no-color setting, so
-    # arbitrary cell values must be escaped on every return path.
+    # arbitrary cell values must go through `safe_markup` on every return path.
     if not color:
-        return escape(text)
+        return safe_markup(text)
 
     if not text:
         return "[dim]-[/]"
 
     style = _STATUS_COLORS.get(text.lower())
     if style:
-        return f"[{style}]{escape(text)}[/]"
-    return escape(text)
+        return f"[{style}]{safe_markup(text)}[/]"
+    return safe_markup(text)

nsc/tui/errors.py

@@ -10,9 +10,16 @@
 import json
 
 from nsc.http.errors import NetBoxAPIError, NetBoxClientError
+from nsc.output.sanitize import safe_markup
 
 
 def api_error_message(exc: Exception) -> str:
+    """The message as Textual markup: `notify` parses markup, and the text is
+    server data, so it is passed through `safe_markup` (#206)."""
+    return safe_markup(_message(exc))
+
+
+def _message(exc: Exception) -> str:
     if isinstance(exc, NetBoxAPIError):
         detail = _format_body(exc.body_snippet)
         return f"API {exc.status_code} — {detail}" if detail else f"API {exc.status_code}"

nsc/tui/screens/bulk_edit_form.py

@@ -30,6 +30,7 @@
 from textual.widgets.selection_list import Selection
 
 from nsc.model.command_model import CommandModel, Operation
+from nsc.output.sanitize import safe_text
 from nsc.savedfilters.custom_fields import CustomFieldDef
 from nsc.savedfilters.tags import TagDef
 from nsc.tui._bindings import textual_bindings
@@ -199,7 +200,7 @@ def _compose_fk(self, name: str) -> ComposeResult:
                 yield Label(target.hint, classes="bulk-fk-hint")
             return
         yield Button(
-            f"{name}: {self._fk_seed_label(name)}",
+            safe_text(f"{name}: {self._fk_seed_label(name)}"),
             id=f"fk-{encode_field_id(name)}",
             classes="bulk-fk",
         )
@@ -352,9 +353,9 @@ def _stage(result: tuple[int, str] | None) -> None:
             if result is not None:
                 self._values[name] = result[0]
                 self._fk_labels[name] = result[1]
-                self.query_one(
-                    f"#fk-{encode_field_id(name)}", Button
-                ).label = f"{name}: {result[1]}"
+                self.query_one(f"#fk-{encode_field_id(name)}", Button).label = safe_text(
+                    f"{name}: {result[1]}"
+                )
 
         self.app.push_screen(RecordPicker(self._client, target.list_op, target.current_id), _stage)
 

nsc/tui/screens/columns.py

@@ -10,6 +10,7 @@
 from textual.screen import ModalScreen
 from textual.widgets import Label, ListItem, ListView
 
+from nsc.output.sanitize import safe_text
 from nsc.tui.columns import ColumnSelection
 
 _HINT = "space toggle · shift+↑/↓ reorder · ↑/↓ move · Enter apply · Esc cancel"
@@ -49,7 +50,7 @@ def _rebuild(self, index: int) -> None:
         listing.clear()
         for name in self.selection.items:
             mark = "x" if self.selection.is_visible(name) else " "
-            listing.append(ListItem(Label(f"[{mark}] {self._labels.get(name, name)}")))
+            listing.append(ListItem(Label(safe_text(f"[{mark}] {self._labels.get(name, name)}"))))
         if self.selection.items:
             listing.index = max(0, min(index, len(self.selection.items) - 1))
 

nsc/tui/screens/detail.py

@@ -22,6 +22,7 @@
 from nsc.http.errors import NetBoxAPIError, NetBoxClientError
 from nsc.model.command_model import CommandModel, Resource
 from nsc.output.flatten import flatten
+from nsc.output.sanitize import safe_text
 from nsc.tui._bindings import textual_bindings
 from nsc.tui.errors import api_error_message
 from nsc.tui.fk import is_fk_value, resolve_fk_target
@@ -121,7 +122,9 @@ def _refresh_rows(self) -> None:
         for row in self._rows:
             dirty = row.name in patch
             label = f"* {row.name}" if dirty else row.name
-            table.add_row(label, self._value_display(row, flat))
+            value = self._value_display(row, flat)
+            # DataTable parses a plain `str` as markup; keys and values are server data.
+            table.add_row(safe_text(label), value if isinstance(value, Text) else safe_text(value))
         if table.row_count:
             table.move_cursor(row=min(cursor, table.row_count - 1))
 

nsc/tui/screens/edit_form.py

@@ -18,6 +18,7 @@
 
 from nsc.http.errors import NetBoxAPIError, NetBoxClientError
 from nsc.model.command_model import CommandModel, Operation
+from nsc.output.sanitize import safe_text
 from nsc.savedfilters.custom_fields import CustomFieldDef
 from nsc.savedfilters.tags import TagDef
 from nsc.tui._bindings import textual_bindings
@@ -193,7 +194,7 @@ def _compose_fk(self, name: str, value: Any) -> ComposeResult:
             if isinstance(nested, dict)
             else ("" if value is None else str(value))
         )
-        yield Button(f"{name}: {current}", id=f"fk-{name}", classes="edit-fk")
+        yield Button(safe_text(f"{name}: {current}"), id=f"fk-{name}", classes="edit-fk")
 
     @staticmethod
     def _field_name(ident: str | None) -> str | None:
@@ -287,7 +288,7 @@ def _stage(result: tuple[int, str] | None) -> None:
             if result is not None:
                 self.staged[name] = result[0]
                 self._fk_labels[name] = result[1]
-                self.query_one(f"#fk-{name}", Button).label = f"{name}: {result[1]}"
+                self.query_one(f"#fk-{name}", Button).label = safe_text(f"{name}: {result[1]}")
 
         self.app.push_screen(RecordPicker(self._client, target.list_op, target.current_id), _stage)