diff --git a/CHANGELOG.md b/CHANGELOG.md index 2872553c7..5232f63cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ Status of the `main` branch. Changes prior to the next official version change w - Fix: Host validation required a local host regardless of the listen address (regression introduced in v1.5.2), preventing remote connections +* Tools: + - Fix: `query_project` rejected read-only tools that the active context hides (e.g. + `read_file`, `find_file`, `list_dir`, `search_for_pattern` in CLI/IDE contexts such as + `claude-code`, `codex`, `copilot-cli`, `vscode`), preventing forwarding to other + registered projects. Globally disabled tools remain blocked. + # v1.5.3 (2026-05-26) Add meta-data for the GitHub MCP registry diff --git a/src/serena/config/serena_config.py b/src/serena/config/serena_config.py index ff63f5235..fdcc4275f 100644 --- a/src/serena/config/serena_config.py +++ b/src/serena/config/serena_config.py @@ -165,6 +165,16 @@ def is_fixed_tool_set(self) -> bool: raise ValueError("Cannot use both fixed_tools and excluded_tools/included_optional_tools at the same time.") return num_fixed > 0 + def disables_tool(self, tool_name: str) -> bool: + """Whether this definition disables the given tool by configuration policy. + + Fixed mode: anything outside the fixed set is disabled. + Incremental mode: only explicitly excluded tools are disabled. + """ + if self.is_fixed_tool_set(): + return tool_name not in self.fixed_tools + return tool_name in self.excluded_tools + @dataclass class NamedToolInclusionDefinition(ToolInclusionDefinition): diff --git a/src/serena/tools/query_project_tools.py b/src/serena/tools/query_project_tools.py index 2820ab29d..1fa131f67 100644 --- a/src/serena/tools/query_project_tools.py +++ b/src/serena/tools/query_project_tools.py @@ -53,14 +53,22 @@ def apply(self, project_name: str, tool_name: str, tool_params_json: str) -> str :param tool_params_json: the parameters to pass to the tool, encoded as a JSON string """ tool = self.agent.get_tool_by_name(tool_name) - assert tool.is_active(), f"Tool {tool_name} is not active." - assert tool.is_readonly(), f"Tool {tool_name} is not read-only and cannot be executed in another project." + # explicit ValueError (not assert): safety/policy gates must not be strippable via `python -O`. + if not tool.is_readonly(): + raise ValueError(f"Tool {tool_name} is not read-only and cannot be executed in another project.") + # the previous `is_active()` check conflated client-cosmetic context exclusions + # (e.g. search_for_pattern in the claude-code context, where the client has a native grep) + # with intentional global disabling. Forwarding a read-only tool to another project is + # valid even if the current context hides it; only the global config acts as kill-switch. + if self.agent.serena_config.disables_tool(tool_name): + raise ValueError(f"Tool {tool_name} is disabled in the global Serena configuration and cannot be executed.") if self._is_project_server_required(tool): client = ProjectServerClient() return client.query_project(project_name, tool_name, tool_params_json) else: registered_project = self.agent.serena_config.get_registered_project(project_name) - assert registered_project is not None, f"Project {project_name} is not registered and cannot be queried." + if registered_project is None: + raise ValueError(f"Project {project_name} is not registered and cannot be queried.") project = registered_project.get_project_instance(self.agent.serena_config) with tool.agent.active_project_context(project): return tool.apply(**json.loads(tool_params_json)) # type: ignore diff --git a/test/serena/config/test_serena_config.py b/test/serena/config/test_serena_config.py index 2b0a4d526..ef2f9337b 100644 --- a/test/serena/config/test_serena_config.py +++ b/test/serena/config/test_serena_config.py @@ -14,6 +14,7 @@ RegisteredProject, SerenaConfig, SerenaConfigError, + ToolInclusionDefinition, ) from serena.constants import PROJECT_TEMPLATE_FILE, SERENA_MANAGED_DIR_NAME from serena.project import MemoryManager, Project @@ -596,3 +597,21 @@ def test_list_memories(self): manager.save_memory("topic_b", "content b", is_tool_context=False) memories = manager.list_project_memories() assert sorted(memories.get_full_list()) == ["topic_a", "topic_b"] + + +class TestToolInclusionDefinitionDisablesTool: + """Unit tests for ToolInclusionDefinition.disables_tool, used by QueryProjectTool's forwarding gate.""" + + def test_default_disables_nothing(self): + definition = ToolInclusionDefinition() + assert not definition.disables_tool("any_tool") + + def test_incremental_mode_blocks_only_excluded(self): + definition = ToolInclusionDefinition(excluded_tools=["search_for_pattern"]) + assert definition.disables_tool("search_for_pattern") + assert not definition.disables_tool("find_symbol") + + def test_fixed_mode_blocks_anything_outside_the_fixed_set(self): + definition = ToolInclusionDefinition(fixed_tools=["find_symbol"]) + assert definition.disables_tool("search_for_pattern") + assert not definition.disables_tool("find_symbol") diff --git a/test/serena/test_query_project_tools.py b/test/serena/test_query_project_tools.py new file mode 100644 index 000000000..841c2b8f2 --- /dev/null +++ b/test/serena/test_query_project_tools.py @@ -0,0 +1,121 @@ +"""Tests for QueryProjectTool forwarding gates. + +These tests run fully in-process with no language server: ``search_for_pattern`` is +non-symbolic and read-only, so ``_is_project_server_required`` returns ``False`` and the +tool executes via ``active_project_context`` without starting a language server. +""" + +import json +import logging +from collections.abc import Callable, Iterator, Sequence + +import pytest + +from serena.agent import SerenaAgent +from serena.config.context_mode import SerenaAgentContext +from serena.config.serena_config import ProjectConfig, RegisteredProject, SerenaConfig +from serena.project import Project +from serena.tools.query_project_tools import QueryProjectTool +from solidlsp.ls_config import Language +from test.conftest import get_repo_path + +PROJECT_NAME = "test_repo_python" + + +def _make_agent( + context_excluded: Sequence[str] = (), + global_excluded: Sequence[str] = (), +) -> SerenaAgent: + config = SerenaConfig(gui_log_window=False, web_dashboard=False, log_level=logging.ERROR) + config.excluded_tools = list(global_excluded) + + repo_path = get_repo_path(Language.PYTHON) + project = Project( + project_root=str(repo_path), + project_config=ProjectConfig( + project_name=PROJECT_NAME, + languages=[Language.PYTHON], + ignored_paths=[], + excluded_tools=[], + read_only=False, + ignore_all_files_in_gitignore=True, + initial_prompt="", + encoding="utf-8", + ), + serena_config=config, + ) + config.projects = [RegisteredProject.from_project_instance(project)] + + context = SerenaAgentContext(name="test-query-project", prompt="", excluded_tools=list(context_excluded)) + # no project activated at startup -> no language server is started + return SerenaAgent(serena_config=config, context=context) + + +@pytest.fixture +def agent_factory() -> Iterator[Callable[..., SerenaAgent]]: + created: list[SerenaAgent] = [] + + def make(**kwargs: Sequence[str]) -> SerenaAgent: + agent = _make_agent(**kwargs) + created.append(agent) + return agent + + yield make + for agent in created: + agent.on_shutdown(timeout=5) + + +def _forward(agent: SerenaAgent, tool_name: str, params: dict | None = None) -> str: + tool = agent.get_tool(QueryProjectTool) + return tool.apply( + project_name=PROJECT_NAME, + tool_name=tool_name, + tool_params_json=json.dumps(params or {}), + ) + + +class TestQueryProjectForwardingGate: + def test_context_excluded_readonly_tool_is_forwardable(self, agent_factory) -> None: + """A read-only tool excluded only by the current context must still be forwardable.""" + agent = agent_factory(context_excluded=["search_for_pattern"]) + result = _forward( + agent, + "search_for_pattern", + {"substring_pattern": "def ", "restrict_search_to_code_files": False}, + ) + matches = json.loads(result) + assert matches, f"Expected non-empty search results, got: {result!r}" + + def test_non_readonly_tool_is_blocked(self, agent_factory) -> None: + """A non-read-only tool must never be forwarded. + + Pinned to ``ValueError`` (not ``AssertionError``): accepting ``AssertionError`` would + let a regression back to ``assert`` pass silently -- and ``assert`` can be stripped + by ``python -O``, which is precisely the failure mode this gate guards against. + """ + agent = agent_factory() + with pytest.raises(ValueError, match="read-only"): + _forward(agent, "create_text_file") + + def test_globally_excluded_tool_is_blocked(self, agent_factory) -> None: + """A tool disabled in the global Serena configuration must remain blocked. Pinned to + ``ValueError`` for the same reason as :meth:`test_non_readonly_tool_is_blocked`. + """ + agent = agent_factory(global_excluded=["search_for_pattern"]) + with pytest.raises(ValueError, match="global"): + _forward( + agent, + "search_for_pattern", + {"substring_pattern": "def ", "restrict_search_to_code_files": False}, + ) + + def test_unregistered_project_is_rejected(self, agent_factory) -> None: + """Forwarding to a project the caller has not registered must raise ``ValueError``.""" + agent = agent_factory() + tool = agent.get_tool(QueryProjectTool) + with pytest.raises(ValueError, match="not registered"): + tool.apply( + project_name="no_such_project", + tool_name="search_for_pattern", + tool_params_json=json.dumps({"substring_pattern": "def "}), + )