fix: treat empty MCP roots as explicit deny-all
This commit is contained in:
parent
3b82062070
commit
172fedaf7a
3 changed files with 9 additions and 9 deletions
|
|
@ -157,8 +157,9 @@ Supported file-path tools:
|
||||||
- `send_file`, `download_media`, `set_profile_photo`, `edit_chat_photo`, `send_voice`, `send_sticker`, `upload_file`
|
- `send_file`, `download_media`, `set_profile_photo`, `edit_chat_photo`, `send_voice`, `send_sticker`, `upload_file`
|
||||||
|
|
||||||
Security semantics (aligned with MCP filesystem server):
|
Security semantics (aligned with MCP filesystem server):
|
||||||
- Server-side allowlist via CLI positional arguments (fallback).
|
- Server-side allowlist via CLI positional arguments (fallback when Roots API is unsupported).
|
||||||
- Client-provided MCP Roots replace the server allowlist when available.
|
- Client-provided MCP Roots replace the server allowlist when available.
|
||||||
|
- If the client returns an empty Roots list, file-path tools are disabled (deny-all).
|
||||||
- All paths are resolved via realpath and must stay inside an allowed root.
|
- All paths are resolved via realpath and must stay inside an allowed root.
|
||||||
- Traversal/glob-like patterns are rejected (`..`, `*`, `?`, `~`, etc.).
|
- Traversal/glob-like patterns are rejected (`..`, `*`, `?`, `~`, etc.).
|
||||||
- Relative paths resolve against the first allowed root.
|
- Relative paths resolve against the first allowed root.
|
||||||
|
|
|
||||||
4
main.py
4
main.py
|
|
@ -498,8 +498,8 @@ async def _get_effective_allowed_roots(ctx: Optional[Context]) -> List[Path]:
|
||||||
if client_roots:
|
if client_roots:
|
||||||
return _dedupe_paths(client_roots)
|
return _dedupe_paths(client_roots)
|
||||||
|
|
||||||
# If client returned an empty roots list, keep server-side fallback roots.
|
# Roots API succeeded; an empty roots list is treated as explicit deny-all.
|
||||||
return fallback_roots
|
return []
|
||||||
|
|
||||||
|
|
||||||
async def _ensure_allowed_roots(
|
async def _ensure_allowed_roots(
|
||||||
|
|
|
||||||
|
|
@ -122,25 +122,24 @@ async def test_client_roots_replace_server_allowlist(tmp_path, monkeypatch):
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_empty_client_roots_fall_back_to_server_allowlist(tmp_path, monkeypatch):
|
async def test_empty_client_roots_disable_file_tools(tmp_path, monkeypatch):
|
||||||
server_root = (tmp_path / "server_root").resolve()
|
server_root = (tmp_path / "server_root").resolve()
|
||||||
server_root.mkdir(parents=True)
|
server_root.mkdir(parents=True)
|
||||||
server_file = server_root / "server.txt"
|
|
||||||
server_file.write_text("server", encoding="utf-8")
|
|
||||||
|
|
||||||
monkeypatch.setattr(main, "SERVER_ALLOWED_ROOTS", [server_root])
|
monkeypatch.setattr(main, "SERVER_ALLOWED_ROOTS", [server_root])
|
||||||
ctx = _DummyContext([])
|
ctx = _DummyContext([])
|
||||||
|
|
||||||
roots = await main._get_effective_allowed_roots(ctx)
|
roots = await main._get_effective_allowed_roots(ctx)
|
||||||
assert roots == [server_root]
|
assert roots == []
|
||||||
|
|
||||||
resolved, error = await main._resolve_readable_file_path(
|
resolved, error = await main._resolve_readable_file_path(
|
||||||
raw_path="server.txt",
|
raw_path="server.txt",
|
||||||
ctx=ctx,
|
ctx=ctx,
|
||||||
tool_name="send_file",
|
tool_name="send_file",
|
||||||
)
|
)
|
||||||
assert error is None
|
assert resolved is None
|
||||||
assert resolved == server_file.resolve()
|
assert error is not None
|
||||||
|
assert "disabled" in error
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue