Skip to content

Commit 9d6f7be

Browse files
committed
Mcp(fix[filters]): Reject unknown filter fields
`list_sessions`, `list_windows` and `list_panes` validated the operator half of a Django-style filter key and never the field half, so `filters={"nosuch_field__contains": "x"}` returned `[]`. A key with no `__` at all was not checked by anything — the loop only entered its branch when one was present, and bound the field to `_field`, the name that means "deliberately unused". libtmux's `QueryList` resolves a key by attribute traversal and treats a miss as "no match", so a misspelled field silently filtered every row out and the empty result was indistinguishable from a real one. That is the worst shape in this class: not an error the agent can react to, but a confident wrong answer shaped like a right one. Field names are now checked against the object being filtered, with near-misses suggested, mirroring the operator error that was already good. Validation covers only the leading segment, so nested traversal like `active_window__window_name__contains` keeps working — the check rejects what the type cannot have rather than whitelisting. The type is a parameter rather than read off the first item, so an empty list still validates. That is exactly when a typo most needs reporting.
1 parent 0950630 commit 9d6f7be

6 files changed

Lines changed: 104 additions & 8 deletions

File tree

CHANGES

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,28 @@ _Notes on upcoming releases will be added here_
88

99
### What's new
1010

11+
#### A typo in a filter field is an error, not an empty list
12+
13+
`list_sessions`, `list_windows` and `list_panes` validated the *operator*
14+
half of a Django-style filter key and never the *field* half, so
15+
`filters={"nosuch_field__contains": "x"}` returned `[]`. A key with no
16+
`__` at all, like `{"totally_bogus": "zzz"}`, was not checked by
17+
anything. libtmux's `QueryList` resolves a filter key by attribute
18+
traversal and treats a miss as "no match", so a misspelled field
19+
silently filtered every row out and the empty result was
20+
indistinguishable from a genuine one.
21+
22+
Field names are now checked against the object being filtered, with
23+
near-misses suggested:
24+
25+
```
26+
Unknown filter field 'session_nme' in 'session_nme__contains'.
27+
Did you mean: session_name, session_marked, session_id?
28+
```
29+
30+
Validation covers only the leading segment of a key, so nested
31+
traversal such as `active_window__window_name__contains` keeps working.
32+
1133
#### A pane that dies mid-wait reports the death, not a parse crash
1234

1335
Killing a pane while `wait_for_text` was waiting on it surfaced

src/libtmux_mcp/_utils.py

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from __future__ import annotations
88

99
import dataclasses
10+
import difflib
1011
import functools
1112
import json
1213
import logging
@@ -841,10 +842,29 @@ def _coerce_dict_arg(
841842
return value
842843

843844

845+
@functools.cache
846+
def _filterable_fields(obj_type: type) -> frozenset[str]:
847+
"""Attribute names a filter key may begin with.
848+
849+
``QueryList`` resolves a key by ``getattr`` traversal and treats a
850+
miss as "no match", so an unknown field silently filters every row
851+
out and an empty result is indistinguishable from a typo.
852+
853+
Deliberately permissive: it rejects names the type cannot have and
854+
accepts everything else, because ``__`` traversal into a nested
855+
object is legitimate and only the first segment is checkable here.
856+
"""
857+
names = {name for name in dir(obj_type) if not name.startswith("_")}
858+
if dataclasses.is_dataclass(obj_type):
859+
names |= {field.name for field in dataclasses.fields(obj_type)}
860+
return frozenset(names)
861+
862+
844863
def _apply_filters(
845864
items: t.Any,
846865
filters: dict[str, str] | str | None,
847866
serializer: t.Callable[..., M],
867+
obj_type: type,
848868
) -> list[M]:
849869
"""Apply QueryList filters and serialize results.
850870
@@ -858,6 +878,11 @@ def _apply_filters(
858878
If None or empty, all items are returned.
859879
serializer : callable
860880
Serializer function to convert each item to a model.
881+
obj_type : type
882+
libtmux class of the filtered items, used to validate filter
883+
field names. Taken as a parameter rather than read off the
884+
first item so an empty list still validates -- an empty result
885+
is exactly when a typo most needs reporting.
861886
862887
Returns
863888
-------
@@ -867,23 +892,39 @@ def _apply_filters(
867892
Raises
868893
------
869894
ExpectedToolError
870-
If a filter key uses an invalid lookup operator.
895+
If a filter key uses an invalid lookup operator or names a
896+
field the object cannot have.
871897
"""
872898
coerced = _coerce_dict_arg("filters", filters)
873899
if not coerced:
874900
return [serializer(item) for item in items]
875901
filters = coerced
876902

877903
valid_ops = sorted(LOOKUP_NAME_MAP.keys())
904+
allowed_fields = _filterable_fields(obj_type)
878905
for key in filters:
906+
field_path = key
879907
if "__" in key:
880-
_field, op = key.rsplit("__", 1)
908+
lhs, op = key.rsplit("__", 1)
881909
if op not in LOOKUP_NAME_MAP:
882910
msg = (
883911
f"Invalid filter operator '{op}' in '{key}'. "
884912
f"Valid operators: {', '.join(valid_ops)}"
885913
)
886914
raise ExpectedToolError(msg)
915+
field_path = lhs
916+
917+
# Only the leading segment is checkable; the rest may traverse
918+
# into a nested object.
919+
field = field_path.split("__", 1)[0]
920+
if field not in allowed_fields:
921+
msg = f"Unknown filter field '{field}' in '{key}'."
922+
close = difflib.get_close_matches(field, sorted(allowed_fields), n=3)
923+
if close:
924+
msg += f" Did you mean: {', '.join(close)}?"
925+
else:
926+
msg += " Call this tool without filters to see available fields."
927+
raise ExpectedToolError(msg)
887928

888929
filtered = items.filter(**filters)
889930
return [serializer(item) for item in filtered]

src/libtmux_mcp/tools/server_tools.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import typing as t
1111

1212
from fastmcp.exceptions import ToolError
13+
from libtmux.session import Session
1314

1415
from libtmux_mcp._history import _prepare_spawn_environment
1516
from libtmux_mcp._utils import (
@@ -63,7 +64,7 @@ def list_sessions(
6364
"""
6465
server = _get_server(socket_name=socket_name)
6566
sessions = server.sessions
66-
return _apply_filters(sessions, filters, _serialize_session)
67+
return _apply_filters(sessions, filters, _serialize_session, Session)
6768

6869

6970
@handle_tool_errors

src/libtmux_mcp/tools/session_tools.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import typing as t
66

77
from libtmux.constants import WindowDirection
8+
from libtmux.window import Window
89

910
from libtmux_mcp._history import _prepare_spawn_environment
1011
from libtmux_mcp._utils import (
@@ -72,7 +73,7 @@ def list_windows(
7273
windows = session.windows
7374
else:
7475
windows = server.windows
75-
return _apply_filters(windows, filters, _serialize_window)
76+
return _apply_filters(windows, filters, _serialize_window, Window)
7677

7778

7879
# get_session_info completes the core-tmux-hierarchy symmetry alongside

src/libtmux_mcp/tools/window_tools.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import typing as t
66

77
from libtmux.constants import PaneDirection
8+
from libtmux.pane import Pane
89

910
from libtmux_mcp._history import _prepare_spawn_environment
1011
from libtmux_mcp._utils import (
@@ -98,7 +99,7 @@ def list_panes(
9899
panes = session.panes
99100
else:
100101
panes = server.panes
101-
return _apply_filters(panes, filters, _serialize_pane)
102+
return _apply_filters(panes, filters, _serialize_pane, Pane)
102103

103104

104105
# get_window_info completes the core-tmux-hierarchy symmetry of get_*_info

tests/test_utils.py

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import pytest
99
from fastmcp.exceptions import ToolError
1010
from libtmux import exc
11+
from libtmux.session import Session
1112

1213
from libtmux_mcp._utils import (
1314
ANNOTATIONS_CREATE,
@@ -34,7 +35,6 @@
3435
if t.TYPE_CHECKING:
3536
from libtmux.pane import Pane
3637
from libtmux.server import Server
37-
from libtmux.session import Session
3838
from libtmux.window import Window
3939

4040

@@ -212,6 +212,36 @@ class ApplyFiltersFixture(t.NamedTuple):
212212
expect_error=True,
213213
error_match="Invalid filter operator",
214214
),
215+
# A typo'd FIELD used to return [] rather than erroring, so an empty
216+
# result was indistinguishable from "nothing matched".
217+
ApplyFiltersFixture(
218+
test_id="unknown_field_with_valid_operator_errors",
219+
filters={"nosuch_field__contains": "x"},
220+
expected_count=None,
221+
expect_error=True,
222+
error_match="Unknown filter field 'nosuch_field'",
223+
),
224+
ApplyFiltersFixture(
225+
test_id="unknown_field_without_operator_errors",
226+
filters={"totally_bogus": "zzz"},
227+
expected_count=None,
228+
expect_error=True,
229+
error_match="Unknown filter field 'totally_bogus'",
230+
),
231+
ApplyFiltersFixture(
232+
test_id="near_miss_field_suggests_alternatives",
233+
filters={"session_nme__contains": "x"},
234+
expected_count=None,
235+
expect_error=True,
236+
error_match="Did you mean: session_name",
237+
),
238+
ApplyFiltersFixture(
239+
test_id="nested_traversal_still_allowed",
240+
filters={"active_window__window_name__contains": ""},
241+
expected_count=None,
242+
expect_error=False,
243+
error_match=None,
244+
),
215245
ApplyFiltersFixture(
216246
test_id="contains_operator",
217247
filters={"session_name__contains": "<partial>"},
@@ -295,9 +325,9 @@ def test_apply_filters(
295325

296326
if expect_error:
297327
with pytest.raises(ToolError, match=error_match):
298-
_apply_filters(sessions, filters, _serialize_session)
328+
_apply_filters(sessions, filters, _serialize_session, Session)
299329
else:
300-
result = _apply_filters(sessions, filters, _serialize_session)
330+
result = _apply_filters(sessions, filters, _serialize_session, Session)
301331
assert isinstance(result, list)
302332
if expected_count is not None:
303333
assert len(result) == expected_count

0 commit comments

Comments
 (0)