Skip to content

Commit f42c06b

Browse files
anticomputerCopilot
andcommitted
fix(anthropic_sdk): match blocked_tools against raw + namespaced names
Reviewer was correct that blocked_tools was effectively a no-op in the anthropic_sdk backend: taskflow YAML supplies raw tool names like 'read_file', but list_tools_unfiltered() returns namespace-prefixed names like '{hash}read_file'. The old 'tool.name not in blocked' check never matched, silently letting every blocked tool through. This is the security bug the reviewer flagged on PR #265. Fix: match the raw name against the un-prefixed portion of each tool's namespaced name, in addition to the literal name. The mcp_server_map keys stay namespaced because that's what Anthropic sends in tool_use. Regression tests: - raw 'read_file' filters out '{hash}read_file' (the bug case) - already-namespaced names still match (backwards compat) doc/GRAMMAR.md: also fix an inaccuracy the reviewer flagged in the same review pass -- the docs claimed copilot_sdk 'silently ignores' unsupported model_settings keys, but it actually raises BackendCapabilityError on 'temperature' and 'parallel_tool_calls' at validate() time. Updated wording to distinguish 'ignored' (anthropic_sdk) from 'rejected' (copilot_sdk) so users aren't surprised by a hard fail when they expected a silent drop. 281 tests pass, hatch fmt clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c97ab6a commit f42c06b

3 files changed

Lines changed: 129 additions & 14 deletions

File tree

doc/GRAMMAR.md

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -550,10 +550,4 @@ passed to the underlying model provider:
550550
| `endpoint` | API base URL for this model | The global `AI_API_ENDPOINT` env var |
551551
| `token` | Name of an environment variable containing the API key | Uses `AI_API_TOKEN` / `COPILOT_TOKEN` |
552552

553-
All other keys (e.g. `temperature`, `top_p`, `reasoning`) are forwarded to
554-
the selected SDK backend. Which keys are actually honored depends on the
555-
backend: `openai_agents` accepts the standard OpenAI parameter set;
556-
`anthropic_sdk` forwards a curated subset (currently `temperature`,
557-
`top_p`, `reasoning`, `max_tokens`, `stream_thinking`); `copilot_sdk`
558-
consumes only the keys its SDK exposes (e.g. `reasoning_effort`) and
559-
silently ignores the rest. Consult the backend-specific docs if in doubt.
553+
All other keys (e.g. `temperature`, `top_p`, `reasoning`) are forwarded to the selected SDK backend. Each backend decides what to do with each key: `openai_agents` accepts the standard OpenAI parameter set; `anthropic_sdk` forwards a curated subset (currently `temperature`, `top_p`, `reasoning`, `max_tokens`, `stream_thinking`, `prompt_caching`) and silently ignores keys outside that set; `copilot_sdk` consumes the keys its SDK exposes (e.g. `reasoning_effort`) and **rejects** unsupported keys at validate time with `BackendCapabilityError` (currently `temperature` and `parallel_tool_calls`) rather than silently dropping them. Consult the backend-specific docs if in doubt.

src/seclab_taskflow_agent/sdk/anthropic_sdk/backend.py

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,23 +131,35 @@ async def build(
131131
# list_tools(), which would require run_context/agent args to
132132
# invoke the openai-agents tool_filter -- args we don't have
133133
# outside the openai-agents run loop.
134+
#
135+
# blocked_tools in taskflow YAML are raw (un-namespaced) names,
136+
# consistent with how openai_agents and copilot_sdk consume them.
137+
# list_tools_unfiltered() returns namespace-prefixed names (the
138+
# MCP server wrapper applies the prefix). Match against both
139+
# forms so blocking works regardless of which name the taskflow
140+
# author used; key mcp_server_map by the namespaced name because
141+
# that's what Anthropic will send back in tool_use blocks.
134142
all_tools: list[dict[str, Any]] = []
135143
mcp_server_map: dict[str, Any] = {}
136144
blocked = set(spec.blocked_tools or [])
137145

146+
def _is_blocked(tool: Any, namespace: str) -> bool:
147+
name = tool.name
148+
if name in blocked:
149+
return True
150+
return name.startswith(namespace) and name[len(namespace):] in blocked
151+
138152
for mcp_spec in spec.mcp_servers:
139153
native_server = mcp_spec.params.get("_native")
140154
if native_server is None:
141155
continue
142156
try:
143157
mcp_tools = await native_server.list_tools_unfiltered()
144-
for tool in mcp_tools:
145-
if tool.name not in blocked:
146-
mcp_server_map[tool.name] = native_server
147-
anthropic_tools = _mcp_tools_to_anthropic(
148-
[t for t in mcp_tools if t.name not in blocked]
149-
)
150-
all_tools.extend(anthropic_tools)
158+
namespace = getattr(native_server, "namespace", "")
159+
kept = [t for t in mcp_tools if not _is_blocked(t, namespace)]
160+
for tool in kept:
161+
mcp_server_map[tool.name] = native_server
162+
all_tools.extend(_mcp_tools_to_anthropic(kept))
151163
except Exception:
152164
logger.exception("Failed to list tools from MCP server %s", mcp_spec.name)
153165

tests/test_sdk_anthropic_adapter.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,3 +381,112 @@ async def _run():
381381
assert captured.get("cache_control") == {"type": "ephemeral", "ttl": "1h"}, (
382382
f"expected cache_control with 1h ttl, got {captured.get('cache_control')!r}"
383383
)
384+
385+
386+
# -- blocked_tools filtering --
387+
388+
389+
def test_blocked_tools_matches_raw_name_against_namespaced_tool(monkeypatch):
390+
"""Regression: taskflow YAML blocked_tools uses raw (un-namespaced)
391+
names like 'read_file', but list_tools_unfiltered() returns
392+
namespace-prefixed names like '{hash}read_file'. The filter must
393+
match the raw name against the un-prefixed portion of the
394+
namespaced tool, otherwise blocking is silently bypassed.
395+
396+
See PR #265 review thread and openai_agents/copilot_sdk for
397+
how blocked_tools are consumed elsewhere (both use raw names).
398+
"""
399+
monkeypatch.setenv("AI_API_TOKEN", "test-token")
400+
import asyncio
401+
from unittest.mock import AsyncMock, MagicMock
402+
403+
from seclab_taskflow_agent.mcp_utils import MCPNamespaceWrap, compress_name
404+
from seclab_taskflow_agent.sdk.base import MCPServerSpec
405+
406+
class _FakeTool:
407+
def __init__(self, name):
408+
self.name = name
409+
self.description = ""
410+
self.inputSchema = {}
411+
412+
def copy(self):
413+
t = _FakeTool(self.name)
414+
return t
415+
416+
# Build a wrapper whose session.list_tools returns two raw tools.
417+
# list_tools_unfiltered() will return them with namespace prefix.
418+
obj = MagicMock()
419+
obj.name = "RepoContext"
420+
ns = compress_name("RepoContext")
421+
obj.session = MagicMock()
422+
obj.session.list_tools = AsyncMock(
423+
return_value=type("R", (), {"tools": [_FakeTool("read_file"), _FakeTool("safe_helper")]})()
424+
)
425+
wrap = MCPNamespaceWrap(confirms=[], obj=obj)
426+
427+
spec = AgentSpec(
428+
name="t",
429+
instructions="",
430+
model="claude-mythos-preview",
431+
mcp_servers=[MCPServerSpec(name="rc", kind="stdio", params={"_native": wrap})],
432+
blocked_tools=["read_file"], # raw name from YAML
433+
)
434+
backend = AnthropicSDKBackend()
435+
handle = asyncio.run(backend.build(spec))
436+
437+
# The blocked tool must be absent from both the tool list AND the
438+
# server map keys (which use the namespaced form).
439+
tool_names = [t["name"] for t in handle.tools]
440+
assert f"{ns}read_file" not in tool_names, (
441+
f"blocked raw name 'read_file' should have filtered out '{ns}read_file'; "
442+
f"got tools: {tool_names}"
443+
)
444+
assert f"{ns}safe_helper" in tool_names, (
445+
f"non-blocked tool 'safe_helper' should still be present; got: {tool_names}"
446+
)
447+
assert f"{ns}read_file" not in handle.mcp_server_map
448+
assert f"{ns}safe_helper" in handle.mcp_server_map
449+
450+
451+
def test_blocked_tools_also_matches_already_namespaced_name(monkeypatch):
452+
"""Backwards-compat: if a caller already passes the namespaced name
453+
in blocked_tools (e.g. they computed it externally), it should still
454+
match. The filter checks both forms."""
455+
monkeypatch.setenv("AI_API_TOKEN", "test-token")
456+
import asyncio
457+
from unittest.mock import AsyncMock, MagicMock
458+
459+
from seclab_taskflow_agent.mcp_utils import MCPNamespaceWrap, compress_name
460+
from seclab_taskflow_agent.sdk.base import MCPServerSpec
461+
462+
class _FakeTool:
463+
def __init__(self, name):
464+
self.name = name
465+
self.description = ""
466+
self.inputSchema = {}
467+
468+
def copy(self):
469+
return _FakeTool(self.name)
470+
471+
obj = MagicMock()
472+
obj.name = "RepoContext"
473+
ns = compress_name("RepoContext")
474+
obj.session = MagicMock()
475+
obj.session.list_tools = AsyncMock(
476+
return_value=type("R", (), {"tools": [_FakeTool("read_file")]})()
477+
)
478+
wrap = MCPNamespaceWrap(confirms=[], obj=obj)
479+
480+
spec = AgentSpec(
481+
name="t",
482+
instructions="",
483+
model="claude-mythos-preview",
484+
mcp_servers=[MCPServerSpec(name="rc", kind="stdio", params={"_native": wrap})],
485+
blocked_tools=[f"{ns}read_file"], # already namespaced
486+
)
487+
backend = AnthropicSDKBackend()
488+
handle = asyncio.run(backend.build(spec))
489+
490+
assert handle.tools == [], (
491+
f"blocked namespaced name should filter out the tool; got: {handle.tools}"
492+
)

0 commit comments

Comments
 (0)