diff --git a/src/tool_execution.py b/src/tool_execution.py index 1680d18c4..c55c74dd1 100644 --- a/src/tool_execution.py +++ b/src/tool_execution.py @@ -907,6 +907,9 @@ async def _execute_tool_block_impl( if _args_error is not None: result = {"error": _args_error, "exit_code": 1} else: + if owner: + args = dict(args) + args[_EMAIL_MCP_OWNER_ARG] = owner result = await mcp.call_tool(qualified, args) else: result = {"error": "MCP manager not available", "exit_code": 1} diff --git a/static/js/chatRenderer.js b/static/js/chatRenderer.js index d3e840f85..ec6c4dcfb 100644 --- a/static/js/chatRenderer.js +++ b/static/js/chatRenderer.js @@ -425,6 +425,23 @@ const TOOL_CALL_RE = /\[TOOL_CALL\][\s\S]*?\[\/TOOL_CALL\]/gi; let EXEC_FENCE_RE = null; const EXEC_FENCE_NON_TOOL = new Set(['bash', 'python']); +function escapeRegex(source) { + return String(source).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +function stripExecutedFence(match, tag, inline, body) { + const inlineArgs = (inline || '').trim(); + if (!inlineArgs) return ''; + const bodyText = (body || '').trim(); + const content = bodyText ? `${inlineArgs}\n${bodyText}` : inlineArgs; + try { + JSON.parse(content); + } catch { + return match; + } + return ''; +} + async function loadExecFenceRegex() { try { const res = await fetch('/api/tools', { credentials: 'same-origin' }); @@ -434,7 +451,10 @@ async function loadExecFenceRegex() { .filter((id) => id && !EXEC_FENCE_NON_TOOL.has(id)); if (tags.length) { EXEC_FENCE_RE = new RegExp( - '```(?:' + tags.join('|') + ')\\s*\\n[\\s\\S]*?```', 'gi' + '```(' + tags.map(escapeRegex).join('|') + ')(?![\\w-])' + + '[ \\t]*([\\[{][^\\n]*?)?[ \\t]*(?=\\r?\\n|```)' + + '\\r?\\n?([\\s\\S]*?)```', + 'gi' ); } } catch (err) { @@ -889,7 +909,7 @@ export function roleTimestamp(when) { */ export function stripToolBlocks(text) { let cleaned = text.replace(TOOL_CALL_RE, ''); - if (EXEC_FENCE_RE) cleaned = cleaned.replace(EXEC_FENCE_RE, ''); + if (EXEC_FENCE_RE) cleaned = cleaned.replace(EXEC_FENCE_RE, stripExecutedFence); cleaned = cleaned.replace(DSML_TOOL_RE, ''); cleaned = cleaned.replace(DSML_STRAY_RE, ''); cleaned = cleaned.replace(XML_TOOL_CALL_RE, ''); diff --git a/tests/test_live_strip_email_tool_fences.py b/tests/test_live_strip_email_tool_fences.py index 1e066cb10..461b7f21b 100644 --- a/tests/test_live_strip_email_tool_fences.py +++ b/tests/test_live_strip_email_tool_fences.py @@ -20,6 +20,7 @@ the behavioral tests exercise an equivalent Python regex built straight from the backend ``TOOL_TAGS`` — the same source the live regex now derives from — and source-level guards assert the frontend keeps no hard-coded list. """ +import json import re from pathlib import Path @@ -46,18 +47,48 @@ def _exec_fence_regex() -> re.Pattern: derives from: the backend TOOL_TAGS (served via /api/tools) minus bash/python.""" tags = _tool_tags() - _NON_STRIPPED assert tags, "TOOL_TAGS is empty" - return re.compile(r"```(?:" + "|".join(sorted(tags)) + r")\s*\n[\s\S]*?```", re.IGNORECASE) + return re.compile( + r"```(" + "|".join(re.escape(tag) for tag in sorted(tags)) + r")(?![\w-])" + r"[ \t]*([{\[][^\n]*?)?[ \t]*(?=\r?\n|```)\r?\n?([\s\S]*?)```", + re.IGNORECASE, + ) + + +def _strip_live_exec_fences(text: str) -> str: + rx = _exec_fence_regex() + + def repl(match: re.Match) -> str: + inline = (match.group(2) or "").strip() + if not inline: + return "" + body = (match.group(3) or "").strip() + content = f"{inline}\n{body}" if body else inline + try: + json.loads(content) + except (TypeError, ValueError): + return match.group(0) + return "" + + return rx.sub(repl, text) def test_strips_executed_email_tool_fences(): - rx = _exec_fence_regex() # The exact shape the reporter observed lingering in the live bubble. text = 'Here are emails\n\n```list_emails\n{"max_results":10}\n```' - assert rx.sub("", text).strip() == "Here are emails" + assert _strip_live_exec_fences(text).strip() == "Here are emails" + + +def test_strips_executed_inline_email_tool_fences(): + text = 'Here are accounts\n\n```list_email_accounts {}\n```' + assert _strip_live_exec_fences(text).strip() == "Here are accounts" + + +def test_strips_multiline_inline_json_email_fences(): + text = 'Here are emails\n\n```list_emails {"folder": "INBOX",\n"max_results": 2}\n```' + assert _strip_live_exec_fences(text).strip() == "Here are emails" def test_strips_every_named_email_tool_fence(): - rx = _exec_fence_regex() email_tools = [ "list_email_accounts", "send_email", "list_emails", "read_email", "reply_to_email", "bulk_email", "archive_email", "delete_email", @@ -65,22 +96,28 @@ def test_strips_every_named_email_tool_fence(): ] for tool in email_tools: fence = f"```{tool}\n{{}}\n```" - assert rx.sub("", fence).strip() == "", f"{tool} fence not stripped" + assert _strip_live_exec_fences(fence).strip() == "", f"{tool} fence not stripped" def test_preserves_existing_web_search_stripping(): - rx = _exec_fence_regex() fence = '```web_search\n{"q":"x"}\n```' - assert rx.sub("", fence).strip() == "" + assert _strip_live_exec_fences(fence).strip() == "" def test_does_not_strip_bash_or_python_code_examples(): """bash/python fences are deliberately excluded — they are legitimate code examples a user may have asked the model to show, not tool invocations.""" - rx = _exec_fence_regex() for lang in sorted(_NON_STRIPPED): example = f"```{lang}\nls -la\n```" - assert rx.sub("", example) == example, f"{lang} example wrongly stripped" + assert _strip_live_exec_fences(example) == example, f"{lang} example wrongly stripped" + + +def test_does_not_strip_invalid_inline_json_metadata(): + for example in ( + '```list_email_accounts {title="setup"}\n```', + '```web_search {query="odysseus"}\n```', + ): + assert _strip_live_exec_fences(example) == example def test_frontend_keeps_no_hardcoded_tool_list(): @@ -99,6 +136,10 @@ def test_frontend_keeps_no_hardcoded_tool_list(): "chatRenderer.js must fetch the tool set from /api/tools to build " "EXEC_FENCE_RE." ) + assert "JSON.parse(content)" in source, ( + "chatRenderer.js must validate inline JSON before stripping same-line " + "tool fences so Markdown metadata stays visible." + ) # The bash/python carve-out must survive the move to the runtime list. m = re.search(r"EXEC_FENCE_NON_TOOL\s*=\s*new Set\(\[(?P.*?)\]\)", source, re.DOTALL) assert m, "bash/python carve-out (EXEC_FENCE_NON_TOOL) not found in chatRenderer.js" diff --git a/tests/test_review_regressions.py b/tests/test_review_regressions.py index 08333b2d1..2a1cd3450 100644 --- a/tests/test_review_regressions.py +++ b/tests/test_review_regressions.py @@ -918,7 +918,9 @@ async def test_plan_mode_blocks_mutating_email_aliases_without_mcp_inventory(mon disabled_tools=denied, ) assert result["exit_code"] == 0 - assert mcp.calls == [("mcp__email__search_emails", {"query": "x"})] + assert mcp.calls == [ + ("mcp__email__search_emails", {"query": "x", "_odysseus_owner": "admin-user"}), + ] @pytest.mark.asyncio @@ -937,7 +939,9 @@ async def test_bare_email_dispatch_empty_content_calls_with_empty_args(monkeypat owner="admin-user", ) assert result["exit_code"] == 0 - assert mcp.calls == [("mcp__email__list_email_accounts", {})] + assert mcp.calls == [ + ("mcp__email__list_email_accounts", {"_odysseus_owner": "admin-user"}), + ] @pytest.mark.asyncio @@ -997,6 +1001,27 @@ async def test_email_mcp_dispatch_includes_hidden_owner(monkeypatch): ] +@pytest.mark.asyncio +async def test_bare_email_mcp_dispatch_includes_hidden_owner(monkeypatch): + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + + fake = _FakeMcpManager() + monkeypatch.setattr(tool_execution, "_owner_is_admin", lambda owner: True) + monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: fake) + + desc, result = await execute_tool_block( + SimpleNamespace(tool_type="list_emails", content='{"folder":"INBOX"}'), + owner="alice", + ) + + assert desc == "email: list_emails" + assert result["exit_code"] == 0 + assert fake.calls == [ + ("mcp__email__list_emails", {"folder": "INBOX", "_odysseus_owner": "alice"}), + ] + + def test_public_agent_policy_hides_sensitive_tools(monkeypatch): auth_mod = _install_core_auth_stub(monkeypatch) from src.tool_security import blocked_tools_for_owner