fix(tools): reject directory targets in open_preview (#95853)
Existing directories were reported as success:true while the preview pane opened nothing. Fail closed with an explicit error and do not emit preview.open. HTTP(S) URLs and regular files are unchanged.
This commit is contained in:
@@ -23,3 +23,60 @@ def test_emitter_failure_is_reported():
|
||||
|
||||
desktop_ui.set_emitter(_boom)
|
||||
assert "no window" in json.loads(op.open_preview_tool("https://x.example"))["error"]
|
||||
|
||||
|
||||
def _capture_emits():
|
||||
emitted: list = []
|
||||
|
||||
def _emit(_sid, event, payload):
|
||||
emitted.append((event, payload))
|
||||
|
||||
desktop_ui.set_emitter(_emit)
|
||||
return emitted
|
||||
|
||||
|
||||
def test_existing_directory_is_an_error_not_success(tmp_path):
|
||||
"""#95853: a directory must not report success while opening nothing."""
|
||||
emitted = _capture_emits()
|
||||
folder = tmp_path / "Active"
|
||||
folder.mkdir()
|
||||
|
||||
result = json.loads(op.open_preview_tool(str(folder)))
|
||||
|
||||
assert "error" in result
|
||||
assert "director" in result["error"].lower()
|
||||
assert result.get("success") is not True
|
||||
assert emitted == []
|
||||
|
||||
|
||||
def test_existing_file_still_opens(tmp_path):
|
||||
emitted = _capture_emits()
|
||||
path = tmp_path / "notes.md"
|
||||
path.write_text("hi", encoding="utf-8")
|
||||
|
||||
result = json.loads(op.open_preview_tool(str(path)))
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["url"] == str(path)
|
||||
assert emitted == [("preview.open", {"url": str(path), "label": ""})]
|
||||
|
||||
|
||||
def test_https_url_is_not_treated_as_a_directory():
|
||||
emitted = _capture_emits()
|
||||
result = json.loads(op.open_preview_tool("https://example.com/docs"))
|
||||
|
||||
assert result["success"] is True
|
||||
assert emitted[0][0] == "preview.open"
|
||||
|
||||
|
||||
def test_file_uri_directory_is_an_error(tmp_path):
|
||||
emitted = _capture_emits()
|
||||
folder = tmp_path / "docs"
|
||||
folder.mkdir()
|
||||
uri = folder.resolve().as_uri()
|
||||
|
||||
result = json.loads(op.open_preview_tool(uri))
|
||||
|
||||
assert "error" in result
|
||||
assert "director" in result["error"].lower()
|
||||
assert emitted == []
|
||||
|
||||
@@ -6,7 +6,10 @@ the normalizer + open action. Emits ``preview.open`` via ``desktop_ui``: the ren
|
||||
the pane for the window that asked and never steals focus for a background session.
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
from pathlib import Path
|
||||
from urllib.parse import unquote, urlparse
|
||||
|
||||
from tools import desktop_ui
|
||||
from tools.registry import tool_error
|
||||
@@ -28,6 +31,39 @@ def _normalize_target(raw: str) -> str:
|
||||
return v
|
||||
|
||||
|
||||
def _local_fs_path(target: str) -> Path | None:
|
||||
"""Return a filesystem path for local targets; None for http(s) URLs."""
|
||||
raw = (target or "").strip()
|
||||
if not raw:
|
||||
return None
|
||||
if "://" in raw:
|
||||
parsed = urlparse(raw)
|
||||
if parsed.scheme.lower() != "file":
|
||||
return None
|
||||
path = unquote(parsed.path or "")
|
||||
if parsed.netloc and parsed.netloc not in {"", "localhost"}:
|
||||
path = f"//{parsed.netloc}{path}"
|
||||
elif (
|
||||
os.name == "nt"
|
||||
and len(path) >= 3
|
||||
and path[0] == "/"
|
||||
and path[2] == ":"
|
||||
):
|
||||
path = path[1:]
|
||||
return Path(path) if path else None
|
||||
return Path(raw).expanduser()
|
||||
|
||||
|
||||
def _is_existing_directory(target: str) -> bool:
|
||||
path = _local_fs_path(target)
|
||||
if path is None:
|
||||
return False
|
||||
try:
|
||||
return path.is_dir()
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
def open_preview_tool(url: str, label: str = "") -> str:
|
||||
"""Ask the desktop GUI to show ``url`` in the preview pane beside the chat."""
|
||||
target = _normalize_target(url or "")
|
||||
@@ -35,6 +71,12 @@ def open_preview_tool(url: str, label: str = "") -> str:
|
||||
return tool_error(
|
||||
"url is required — a web URL (https://…), a localhost dev server, or a "
|
||||
"file path to show in the preview pane.")
|
||||
if _is_existing_directory(target):
|
||||
return tool_error(
|
||||
"directories are not previewable — pass a file path or a URL. "
|
||||
f"{target} is a directory."
|
||||
)
|
||||
|
||||
label = (label or "").strip()
|
||||
return desktop_ui.emit_or_error(
|
||||
"preview.open",
|
||||
|
||||
Reference in New Issue
Block a user