mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-07-08 11:56:59 +00:00
fix(agent): execute fenced tool calls with inline args and route bare email tool names (#3681)
* fix(agent): execute fenced tool calls with inline args and bare email tool names
Two bugs made local (Ollama) models unable to use email tools, leaving
raw fences like ```list_email_accounts {}``` in the chat:
1. _TOOL_BLOCK_RE required a newline right after the fence tag, so a
tool call with args on the same line ("```list_email_accounts {}")
never matched and was never executed. The fence now matches with
optional spaces/newline after the tag.
2. Even when parsed, bare email tool names had no dispatch branch in
tool_execution.py and fell through to "Unknown tool type". They now
route to the email MCP server as mcp__email__<name>, matching how
function_call_to_tool_block already maps them for native callers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(security): block all bare email tool names for non-admins; harden fence-tag regex
Review follow-up on #3681 (thanks @vgalin):
1. Routing bare email names made 10 of the 14 email tools executable by
non-admin owners — is_public_blocked_tool() runs on the bare name
before dispatch, and NON_ADMIN_BLOCKED_TOOLS only listed 4. Define the
full email tool set once (BUILTIN_EMAIL_TOOLS in tool_security.py) and
derive the blocklist, the fence tags (TOOL_TAGS), the bare-name
dispatch, and the native-call mapping from it so they can't drift.
This also fixes 4 tools (search_emails, draft_email, draft_email_reply,
ai_draft_email_reply) that were missing from the old tool_schemas copy
and therefore unreachable even for native function-calling models.
2. The relaxed fence regex from the previous commit could prefix-match
longer fence tags: ```python3 parsed as tool "python" with content
"3\nprint(...)" and executed as code. Add a (?![\w-]) boundary after
the tag.
Tests: test_public_agent_policy_blocks_sensitive_tools now covers all 14
bare email names + the mcp__email__ form; new tests/test_fenced_inline_args.py
pins inline-args parsing, the python3/hyphenated-tag non-matches, and
strip/parse display mirroring.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(security): gate bare and mcp-qualified email names together; stop executing Markdown info strings
Review follow-up on #3681 (thanks @RaresKeY):
1. P1: execute_tool_block() checked disabled_tools / the turn ToolPolicy
only against the incoming block name, then the bare-email branch
qualified it to mcp__email__<name> and called the MCP manager. Plan
mode and the MCP settings toggle write the QUALIFIED name into the
denylist, so a bare fence like ```list_emails``` sailed past a
mcp__email__list_emails entry. Both gates now match on both
spellings (bare <-> mcp__email__-qualified), in either direction.
2. P2: the relaxed fence regex accepted arbitrary same-line text after
a recognized tag, which made ordinary Markdown info strings
executable: ```python title="example.py" ran as a python tool call.
Same-line content now only counts as tool input when it starts with
{ or [ (JSON args); anything else leaves the fence as display text,
and strip_tool_blocks mirrors that (the fence stays visible).
Tests: disabled-tools alias regression (qualified entry blocks bare
name and vice versa, never reaching the MCP manager), ToolPolicy alias
regression, python/bash title="..." non-execution + display retention,
and inline JSON-array args still parsing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(security): reject brace-style fence metadata; cover the full email set in the friendly toggle
Review follow-up round 3 on #3681 (thanks @RaresKeY):
1. Brace-style fence metadata no longer executes. The previous narrowing
still treated any same-line {/[ after a recognized tag as tool input,
so ```bash {title="setup"} ran as a bash call. The fence header is now
captured separately and judged by one predicate shared between
parse_tool_blocks and strip_tool_blocks (_fenced_tool_call), so the
execute and display decisions can't disagree: same-line content only
counts as inline args when the tag is NOT a code tag (bash/python
never take same-line args — that text is Markdown fence attributes)
AND the inline text (plus any continuation lines) parses as standalone
JSON. ```bash {title="setup"}, ```python {"title":"example.py"} and
```list_emails {title="x"} all stay visible and inert.
2. The friendly `disable_tool email` toggle covered 3 of the 14 email
tools (mcp__email__{list_emails,read_email,send_email}); the other
bare aliases this PR routes stayed executable after an operator
disabled email. The alias now derives from BUILTIN_EMAIL_TOOLS in
BOTH spellings — bare (function-schema hiding, bare-fence dispatch)
and mcp__email__* (MCP schema hiding, qualified runtime blocks) —
so the toggle and the runtime gate can't drift apart.
Tests: brace/bracket metadata regressions for parse and strip symmetry
(code tags, invalid-JSON inline on a JSON tool, multi-line inline JSON
still parsing), and disable_tool/enable_tool email covering all 14 names
in both spellings.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(email): close remaining email-tool registry drift; classify every email tool for plan mode
Deep self-review follow-up on #3681. Three review rounds each found another
hand-maintained copy of the email tool list that had drifted; this commit
hunts down ALL remaining copies and pins them to BUILTIN_EMAIL_TOOLS.
The same 5 tools (search_emails, draft_email, draft_email_reply,
ai_draft_email_reply, download_attachment) were missing from every
advertising surface, so they were dispatchable but never offered:
- FUNCTION_TOOL_SCHEMAS: native function-calling models never saw them
(the round-1 fix covered dispatch only); schemas added, mirroring the
email server's inputSchema definitions.
- TOOL_SECTIONS: fenced-block models were never told about them; prompt
sections added.
- tool_index: absent from the RAG embedding registry (never retrievable),
the email keyword hints, and the scheduled assistant's always-available
set — the latter two now derive from BUILTIN_EMAIL_TOOLS.
- agent_loop._DOMAIN_TOOL_MAP["email"], tool_policy._COMMON_TOOL_NAMES,
the assistant tool-selector UI groups (assistant.js), and the default
Assistant crew seed (task_scheduler) now derive from / cover the set.
Plan mode now classifies every email tool explicitly:
- list_email_accounts and search_emails join PLAN_MODE_READONLY_TOOLS.
Without this, list_email_accounts sat in the plan-mode bare denylist
(schema-derived) while its qualified form passed the MCP read-only
filter — and the round-2 bare/qualified alias gate would have blocked
the qualified call too, regressing read-only email discovery in plan
mode.
- draft_email, draft_email_reply, ai_draft_email_reply, and
download_attachment join the fail-closed mutator backstop (drafts
create documents; download_attachment writes to disk).
Tests: tests/test_email_registry_sync.py pins every registry (including
the email server source and assistant.js) to BUILTIN_EMAIL_TOOLS and
asserts the plan-mode partition, so the next email tool can't drift; a
parse/strip mirror grid covers 192 fence shapes (tag x header x body)
asserting executed <=> stripped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor: move the email alias rule into tool_security; extract the assistant seed constant
Code-quality pass over the PR's own changes:
- The bare<->qualified email aliasing rule lived inline in the generic
dispatcher (_execute_tool_block_impl). It is policy knowledge, so it
moves next to BUILTIN_EMAIL_TOOLS as email_tool_policy_names(); the
dispatcher just consumes it, and the rule gets its own unit test
(including the mcp__email__<not-a-tool> and mcp__other__ non-alias
cases).
- The default Assistant's enabled_tools list was an inline literal
inside the CrewMember seed, and its registry-sync test asserted a
source-code substring. Extracted to DEFAULT_ASSISTANT_ENABLED_TOOLS
so the test imports and checks the actual value.
- _fenced_tool_call return type tightened to Optional[Tuple[str, str]].
No behavior change; suite green (3295 passed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* revert: move the email registry consolidation to a follow-up PR
Per review feedback on scope, this PR stays narrow: fenced inline-args
parsing, bare email tool routing, and the directly required safety
gates. This commit reverts the registry/advertising consolidation from
db29046 and 016ce47 (native schemas, prompt sections, RAG description
index + keyword hints, assistant always-available set, guide-only
known-names union, frontend tool-selector groups, default assistant
seed, and their sync tests) — all of that moves to a dedicated
follow-up PR together with the _EMAIL_TOOL_HINTS finding.
Kept here because the narrow scope needs them:
- email_tool_policy_names() in tool_security + its use in the
execute_tool_block gates and its unit test (refactor of this PR's own
round-2 alias fix),
- list_email_accounts in PLAN_MODE_READONLY_TOOLS (the alias gate works
both ways, and the schema-derived plan-mode bare denylist would
otherwise block the qualified read-only call too),
- the parse/strip mirror grid test (parser scope),
- the narrow registry sync tests (email server <-> BUILTIN_EMAIL_TOOLS
match, fence-tag coverage, non-admin blocklist coverage).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(email): execute empty email fences with empty args; reject non-object JSON args
Two gaps found by replaying captured local-model traffic against the
narrowed branch:
1. ```list_email_accounts``` with NO body — a shape gemma really emits
for no-arg tools — was silently dropped (parse skips empty content),
so the model concluded email was broken: the original #337 symptom
through a different door. Empty fences whose tag is a built-in email
tool now dispatch with {} args and the tool's own validation answers
(e.g. an empty send_email returns "to is required" instead of
silence). Empty bash/python/other fences keep skipping, and strip
stays mirrored (the fence was executed, so it is removed).
2. The fence parser accepts JSON arrays as inline args, but the email
dispatch parsed only objects — an array silently became {} args.
Non-object JSON now returns a correctable "arguments must be a JSON
object" error before reaching the MCP server (same class as #3966).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(security): classify all email tools for plan mode statically; reject invalid email JSON bodies
Review follow-up round 5 on #3681 (thanks @RaresKeY):
1. This PR makes every BUILTIN_EMAIL_TOOLS name fence-taggable, so each
one must be explicitly classified for plan mode — the draft tools and
download_attachment were in neither the read-only allowlist nor the
static denylist, leaving their bare-alias plan-mode safety dependent
on the MCP read-only inventory being present and current.
search_emails joins PLAN_MODE_READONLY_TOOLS (explicit, not
allowed-by-omission); draft_email, draft_email_reply,
ai_draft_email_reply, and download_attachment join the fail-closed
_PLAN_MODE_KNOWN_MUTATORS backstop. (Moved back from the #4053 split:
the partition is directly required for this PR to merge
independently.)
2. The classic tag/body fence form reaches execution unvalidated (only
INLINE args are JSON-checked by the parser), so a body like
{account: "work"} silently became {} args and read the DEFAULT
mailbox instead of the intended one. JSON-looking bodies that fail to
parse now return a correctable "not valid JSON" error before reaching
the MCP server.
Tests: a partition invariant (every email tool is explicitly read-only
or plan-mode-denied), a mutating-alias probe that uses only the static
denylist with a fake MCP manager (no inventory layer), and the
body-form invalid-JSON regression.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(tool-dispatch): decode inline JSON args for legacy MCP tools; reject all non-object email bodies
Review follow-up round 6 on #3681 (thanks @RaresKeY) — both pre-existing
on this branch, surfaced by the relaxed inline-args parser:
1. The relaxed parser accepts inline JSON for every non-code tag, but
the legacy line-based arg builders (web_search/web_fetch/read_file/
write_file/generate_image/manage_memory) wrapped the whole JSON
string as the query/url/path/prompt — so `web_search {"query": "x"}`
executed as a search for the literal string `{"query": "x"}`.
_build_mcp_args now uses a fenced JSON object directly when it carries
the tool's primary arg key (query/url/path/prompt/action). Keyed off
membership so it can't drift; an object without the primary key (e.g.
a freeform JSON query, or bare object content for write_file) falls
through to the line parser unchanged. Also fixes the same corruption
for the classic newline-JSON form.
2. The bare-email dispatch only rejected bodies starting with { or [, so
a non-empty non-JSON body like `account: work` still fell through to
{} args and silently read the DEFAULT mailbox. Now ANY non-empty body
must decode to a JSON object or it returns a correctable error; only a
truly empty body keeps the no-arg path (```list_email_accounts```).
Tests: inline-JSON arg decoding for the five legacy tools plus the
freeform and missing-primary-key fallbacks; the email body rejection
extended to cover the brace-looking and bare `key: value` shapes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(tool-dispatch): drop dead manage_memory JSON-decode entry; pin the live-path invariant
Self-audit catch on the round-6 fix. manage_memory was added to
_MCP_JSON_PRIMARY_KEYS, but _build_mcp_args is only reached via
_call_mcp_tool, which only runs for _MCP_TOOL_MAP tools — and
manage_memory isn't one (its tag routes through dispatch_ai_tool ->
do_manage_memory, which line-parses). So the round-6 decode for
manage_memory was dead code: the unit test exercising _build_mcp_args
passed while a real `manage_memory {"action": ...}` fence still parsed
the whole JSON blob as the action.
Remove the dead entry and add test_mcp_json_primary_keys_are_all_live,
which asserts every JSON-primary tool is in _MCP_TOOL_MAP so a dead
decode can't be added again. The same inline-JSON corruption for
manage_memory and the other tools that route through positional
dispatchers (create_session, ui_control, send_to_session, search_chats,
the document tools, etc.) is pre-existing (dev corrupts their newline
JSON form too) and tracked separately; the proper fix there is to route
fenced JSON through function_call_to_tool_block.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(tool-dispatch): decode inline JSON in WriteFileTool (its live path); round-6 fix was on the dead MCP path
Self-audit: round 6 claimed to fix inline JSON args for write_file via
_build_mcp_args, but there is no filesystem MCP server, so write_file
always runs through _direct_fallback -> WriteFileTool, never through
_build_mcp_args. WriteFileTool — unlike its siblings ReadFileTool /
WebSearchTool / WebFetchTool, which all decode JSON — took lines[0] as
the path, so `write_file {"path": "/tmp/x", "content": "y"}` wrote to a
file literally named with the JSON blob. The round-6 _build_mcp_args
entry decoded correctly but on a path that never executes (same class
as the manage_memory dead entry), and the round-6 unit test passed on
that dead path.
WriteFileTool now decodes a JSON object carrying "path" (matching
ReadFileTool directly above it), and the comment on _MCP_JSON_PRIMARY_KEYS
records that only generate_image has a live MCP server today — the other
entries are defense-in-depth for the MCP path; the live fix for each
server-less tool is in its handler.
Test: test_write_file_inline_json_args drives the LIVE path
(execute_tool_block with no MCP) and asserts the intended path is used —
verified to fail without the handler fix. web_search/web_fetch/read_file
were already correct (their handlers decode); write_file was the gap.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(strip-fence): derive the live-strip TOOL_TAGS from the real set
Semantic conflict from the dev merge that textual auto-merge didn't flag:
dev added test_live_strip_email_tool_fences.py whose _tool_tags() helper
source-scrapes only the TOOL_TAGS literal `{...}`, which worked on dev
because the email tool names were listed inline there. This branch makes
TOOL_TAGS the single source — `{...} | BUILTIN_EMAIL_TOOLS` — so the email
names are no longer in the literal and the scraper missed them, leaving the
email-fence strip assertions failing even though TOOL_TAGS does contain them
at runtime.
Import the real TOOL_TAGS instead of scraping source, so the test mirrors
exactly what GET /api/tools serves (sorted(TOOL_TAGS)) and the live
EXEC_FENCE_RE derives from — robust to however the set is composed. The
source-level frontend/route guards in the same file are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: botinate <285686135+botinate@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
botinate
@@ -14,6 +14,7 @@ Sub-modules:
|
||||
import logging
|
||||
from collections import namedtuple
|
||||
|
||||
from src.tool_security import BUILTIN_EMAIL_TOOLS
|
||||
from src.tool_utils import _truncate, get_mcp_manager, set_mcp_manager
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -86,9 +87,10 @@ TOOL_TAGS = {"bash", "python", "web_search", "web_fetch", "read_file", "write_fi
|
||||
"manage_endpoints", "manage_mcp", "manage_webhooks",
|
||||
"manage_tokens", "manage_documents", "manage_settings",
|
||||
"manage_notes", "manage_calendar",
|
||||
"resolve_contact", "manage_contact", "list_email_accounts", "send_email", "list_emails",
|
||||
"read_email", "reply_to_email", "bulk_email", "archive_email",
|
||||
"delete_email", "mark_email_read",
|
||||
"resolve_contact", "manage_contact",
|
||||
# Email tool names come from BUILTIN_EMAIL_TOOLS (unioned below)
|
||||
# so the fence regex, dispatch, and non-admin blocklist all cover
|
||||
# the same set.
|
||||
# Cookbook tools (LLM serving + downloads). Without these
|
||||
# entries, native function calls to e.g. list_served_models
|
||||
# are rejected as "Unknown function call" before reaching
|
||||
@@ -105,7 +107,7 @@ TOOL_TAGS = {"bash", "python", "web_search", "web_fetch", "read_file", "write_fi
|
||||
# Generic loopback to any UI-button endpoint (cookbook,
|
||||
# gallery, email folders, etc.) — agent uses this when
|
||||
# there's no named tool wrapper for the action.
|
||||
"app_api"}
|
||||
"app_api"} | BUILTIN_EMAIL_TOOLS
|
||||
|
||||
ToolBlock = namedtuple("ToolBlock", ["tool_type", "content"])
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ import logging
|
||||
from typing import Optional, Dict
|
||||
|
||||
from src.tool_utils import get_mcp_manager, _parse_tool_args
|
||||
from src.tool_security import BUILTIN_EMAIL_TOOLS
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -706,7 +707,14 @@ async def do_manage_settings(content: str, owner: Optional[str] = None) -> Dict:
|
||||
"tasks": ["manage_tasks"],
|
||||
"notes": ["manage_notes"],
|
||||
"calendar": ["manage_calendar"],
|
||||
"email": ["mcp__email__list_emails", "mcp__email__read_email", "mcp__email__send_email"],
|
||||
# The full built-in email tool set, in BOTH spellings: the
|
||||
# qualified mcp__email__* names drive MCP schema hiding, the
|
||||
# bare names drive function-schema hiding, and the runtime
|
||||
# gate accepts either — deriving from BUILTIN_EMAIL_TOOLS
|
||||
# keeps the toggle covering every tool the email server
|
||||
# exposes instead of a hand-picked subset.
|
||||
"email": sorted(BUILTIN_EMAIL_TOOLS)
|
||||
+ [f"mcp__email__{t}" for t in sorted(BUILTIN_EMAIL_TOOLS)],
|
||||
"research": ["web_search", "web_fetch"], # research is a per-request flag, not a tool (closest analog)
|
||||
}
|
||||
|
||||
|
||||
@@ -186,6 +186,21 @@ class WriteFileTool:
|
||||
lines = content.split("\n", 1)
|
||||
raw_path = lines[0].strip()
|
||||
body = lines[1] if len(lines) > 1 else ""
|
||||
# Decode JSON-object args (the fenced inline-args shape
|
||||
# ```write_file {"path": "...", "content": "..."}```), matching
|
||||
# ReadFileTool above. Without this the whole JSON string becomes the
|
||||
# path and the file is written under a garbage name. This is the live
|
||||
# path: there is no filesystem MCP server, so write_file always runs
|
||||
# here via _direct_fallback, not through _build_mcp_args.
|
||||
_stripped = content.strip()
|
||||
if _stripped.startswith("{"):
|
||||
try:
|
||||
_a = json.loads(_stripped)
|
||||
if isinstance(_a, dict) and "path" in _a:
|
||||
raw_path = str(_a.get("path", "")).strip()
|
||||
body = str(_a.get("content", ""))
|
||||
except (json.JSONDecodeError, TypeError, ValueError):
|
||||
pass
|
||||
try:
|
||||
path = _resolve_tool_path(raw_path)
|
||||
except ValueError as e:
|
||||
|
||||
+90
-3
@@ -21,7 +21,12 @@ from typing import Any, Awaitable, Callable, Dict, Optional, Tuple
|
||||
|
||||
|
||||
|
||||
from src.tool_security import is_public_blocked_tool, owner_is_admin_or_single_user
|
||||
from src.tool_security import (
|
||||
BUILTIN_EMAIL_TOOLS,
|
||||
email_tool_policy_names,
|
||||
is_public_blocked_tool,
|
||||
owner_is_admin_or_single_user,
|
||||
)
|
||||
from src.tool_policy import ToolPolicy
|
||||
from src.constants import MAX_OUTPUT_CHARS, MAX_READ_CHARS, MAX_DIFF_LINES, DATA_DIR
|
||||
from src.tool_utils import _truncate, get_mcp_manager
|
||||
@@ -390,8 +395,42 @@ _MCP_ARG_PARSERS: Dict[str, Callable[[str], Dict[str, str]]] = {
|
||||
}
|
||||
|
||||
|
||||
# Primary argument key(s) for the legacy line-parsed tools. When a fenced
|
||||
# block's content is a JSON object carrying one of these keys, it's structured
|
||||
# inline args (the relaxed parser's ```web_search {"query": "..."}``` shape) —
|
||||
# use the object directly instead of letting the line-based parsers wrap the
|
||||
# whole JSON string as the query/url/path/prompt. Keyed off membership only
|
||||
# (the primary key never changes), so this can't drift; an unrecognized object
|
||||
# safely falls through to the line-based parser, i.e. the previous behavior.
|
||||
#
|
||||
# IMPORTANT — this only covers the MCP path. _build_mcp_args is reached via
|
||||
# _call_mcp_tool only for _MCP_TOOL_MAP tools (so an entry outside that map is
|
||||
# dead, as manage_memory was). And of these, only generate_image has a live MCP
|
||||
# server today; web_search/web_fetch/read_file/write_file have none, so they run
|
||||
# via _direct_fallback -> TOOL_HANDLERS, whose handlers decode JSON themselves
|
||||
# (see ReadFileTool/WriteFileTool/WebSearchTool/WebFetchTool). The entries here
|
||||
# are kept as defense-in-depth for if/when those servers are added. The live
|
||||
# fix for each server-less tool lives in its handler. test_write_file_inline_
|
||||
# json_args and test_mcp_json_primary_keys_are_all_live pin both halves.
|
||||
_MCP_JSON_PRIMARY_KEYS: Dict[str, tuple] = {
|
||||
"web_search": ("query", "queries"),
|
||||
"web_fetch": ("url",),
|
||||
"read_file": ("path",),
|
||||
"write_file": ("path",),
|
||||
"generate_image": ("prompt",),
|
||||
}
|
||||
|
||||
|
||||
def _build_mcp_args(tool: str, content: str) -> Dict:
|
||||
"""Convert fenced-block text content to structured MCP arguments."""
|
||||
primaries = _MCP_JSON_PRIMARY_KEYS.get(tool)
|
||||
if primaries and content.strip().startswith("{"):
|
||||
try:
|
||||
decoded = json.loads(content.strip())
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
decoded = None
|
||||
if isinstance(decoded, dict) and any(k in decoded for k in primaries):
|
||||
return decoded
|
||||
parser = _MCP_ARG_PARSERS.get(tool)
|
||||
return parser(content) if parser else {}
|
||||
|
||||
@@ -596,6 +635,12 @@ async def _execute_tool_block_impl(
|
||||
tool = block.tool_type
|
||||
content = block.content
|
||||
|
||||
# The block/disable gates below must match every policy-equivalent
|
||||
# spelling of the tool name (bare email names alias their mcp__email__
|
||||
# form — see email_tool_policy_names), not just the spelling the model
|
||||
# happened to emit.
|
||||
policy_names = email_tool_policy_names(tool)
|
||||
|
||||
# Misformatted tool call detection: model put JSON inside ```python``` (or
|
||||
# similar) without naming the tool. Common with MiniMax-style outputs.
|
||||
# Return a helpful error so the model retries with the correct format.
|
||||
@@ -623,13 +668,13 @@ async def _execute_tool_block_impl(
|
||||
pass
|
||||
|
||||
# Reject tools that the user has disabled for this request
|
||||
if disabled_tools and tool in disabled_tools:
|
||||
if disabled_tools and not policy_names.isdisjoint(disabled_tools):
|
||||
desc = f"{tool}: BLOCKED"
|
||||
result = {"error": f"Tool '{tool}' is disabled by user.", "exit_code": 1}
|
||||
logger.info(f"Tool blocked by user: {tool}")
|
||||
return desc, result
|
||||
|
||||
if tool_policy and tool_policy.blocks(tool):
|
||||
if tool_policy and any(tool_policy.blocks(name) for name in policy_names):
|
||||
desc = f"{tool}: BLOCKED"
|
||||
result = {
|
||||
"error": f"Execution of tool '{tool}' is forbade by the active guide-only policy.",
|
||||
@@ -823,6 +868,48 @@ async def _execute_tool_block_impl(
|
||||
elif tool == "vault_unlock":
|
||||
desc = "vault_unlock"
|
||||
result = await do_vault_unlock(content, owner=owner)
|
||||
elif tool in BUILTIN_EMAIL_TOOLS:
|
||||
# Bare email tool name from fenced-block models (e.g. Ollama) — route to MCP email server.
|
||||
# Non-admin owners never reach here: BUILTIN_EMAIL_TOOLS ⊆ NON_ADMIN_BLOCKED_TOOLS,
|
||||
# so is_public_blocked_tool() above already rejected them.
|
||||
mcp = get_mcp_manager()
|
||||
qualified = f"mcp__email__{tool}"
|
||||
desc = f"email: {tool}"
|
||||
if mcp:
|
||||
_raw = content.strip()
|
||||
args = {}
|
||||
_args_error = None
|
||||
if _raw:
|
||||
# A non-empty body is always meant to be the call's arguments,
|
||||
# and every email tool takes a JSON object. Anything that
|
||||
# isn't one is a correctable error — NOT a silent empty-args
|
||||
# call, which would read the DEFAULT mailbox/folder instead of
|
||||
# the one the model meant (#3966 class). Only an EMPTY body
|
||||
# keeps the no-arg path (e.g. ```list_email_accounts```).
|
||||
try:
|
||||
parsed = json.loads(_raw)
|
||||
except (json.JSONDecodeError, TypeError) as _je:
|
||||
# Covers both `{account: "work"}` (looks like JSON, bad)
|
||||
# and `account: work` (not JSON at all).
|
||||
_args_error = (
|
||||
f"'{tool}' arguments are not valid JSON ({_je}). "
|
||||
'Send a JSON object, e.g. {"account": "work"} — '
|
||||
"keys and string values need double quotes."
|
||||
)
|
||||
else:
|
||||
if isinstance(parsed, dict):
|
||||
args = parsed
|
||||
else:
|
||||
_args_error = (
|
||||
f"'{tool}' arguments must be a JSON object, "
|
||||
'e.g. {"uid": "..."} — got a JSON array/value instead.'
|
||||
)
|
||||
if _args_error is not None:
|
||||
result = {"error": _args_error, "exit_code": 1}
|
||||
else:
|
||||
result = await mcp.call_tool(qualified, args)
|
||||
else:
|
||||
result = {"error": "MCP manager not available", "exit_code": 1}
|
||||
elif tool.startswith("mcp__"):
|
||||
# MCP tool dispatch
|
||||
mcp = get_mcp_manager()
|
||||
|
||||
+72
-6
@@ -10,9 +10,10 @@ import bisect
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
from typing import List, Optional
|
||||
from typing import List, Optional, Tuple
|
||||
|
||||
from src.agent_tools import ToolBlock, TOOL_TAGS
|
||||
from src.tool_security import BUILTIN_EMAIL_TOOLS
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -20,12 +21,63 @@ logger = logging.getLogger(__name__)
|
||||
# Regex patterns
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# Pattern 1: ```bash ... ``` fenced code blocks
|
||||
# Pattern 1: ```bash ... ``` fenced code blocks. The tag may be followed by a
|
||||
# newline (classic form) or by inline JSON args on the same line
|
||||
# (```list_email_accounts {}). The same-line part is captured separately
|
||||
# (group 2) and judged by _fenced_tool_call below — the regex alone only
|
||||
# requires it to start with { or [; anything else after the tag is a Markdown
|
||||
# info string (```python title="example.py") and the fence never matches.
|
||||
# (?![\w-]) keeps the alternation from prefix-matching longer fence tags:
|
||||
# without it, ```python3 would match as tool "python" with content "3\n..."
|
||||
# and execute as code.
|
||||
_TOOL_BLOCK_RE = re.compile(
|
||||
r"```(" + "|".join(TOOL_TAGS) + r")\s*\n([\s\S]*?)```",
|
||||
r"```(" + "|".join(TOOL_TAGS) + r")(?![\w-])"
|
||||
r"[ \t]*([{\[][^\n]*?)?[ \t]*(?=\r?\n|```)\r?\n?([\s\S]*?)```",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
# Tags whose fenced content is raw code, not JSON args. Same-line text after
|
||||
# these tags is Markdown fence metadata on a real language (```bash {title=
|
||||
# "setup"}), never inline tool args — only the classic tag-then-newline form
|
||||
# executes for them.
|
||||
_CODE_FENCE_TAGS = frozenset({"bash", "python"})
|
||||
|
||||
|
||||
def _fenced_tool_call(m) -> Optional[Tuple[str, str]]:
|
||||
"""Classify a Pattern-1 fence match: (tag, content) when it is an
|
||||
executable tool call, None when the fence must stay display text.
|
||||
|
||||
Shared by parse_tool_blocks and strip_tool_blocks so the execute and
|
||||
display decisions can never disagree: a fence that doesn't execute is
|
||||
never stripped, and vice versa.
|
||||
|
||||
Same-line text after the tag only counts as inline tool args when the
|
||||
tag's tool takes JSON args (not a code tag) AND the text is valid
|
||||
standalone JSON. ```bash {title="setup"} and ```python {"x": 1} are
|
||||
fence attributes on real languages, and {title="x"} on any tag is
|
||||
metadata, not arguments — all of those stay visible and inert.
|
||||
"""
|
||||
tag = m.group(1).lower()
|
||||
inline = (m.group(2) or "").strip()
|
||||
body = (m.group(3) or "").strip()
|
||||
if not inline:
|
||||
return tag, body
|
||||
if tag in _CODE_FENCE_TAGS:
|
||||
return None
|
||||
# Inline args may continue onto following lines (a JSON object opened on
|
||||
# the tag line); the combined text must parse as JSON or nothing runs.
|
||||
content = f"{inline}\n{body}" if body else inline
|
||||
try:
|
||||
json.loads(content)
|
||||
except (ValueError, TypeError):
|
||||
return None
|
||||
return tag, content
|
||||
|
||||
|
||||
def _strip_executed_fence(m) -> str:
|
||||
"""re.sub callback: remove only fences that parse as tool calls."""
|
||||
return "" if _fenced_tool_call(m) is not None else m.group(0)
|
||||
|
||||
# Pattern 2: [TOOL_CALL] ... [/TOOL_CALL] blocks (some models use this format)
|
||||
# Matches: {tool => "shell", args => {--command "ls -la"}} etc.
|
||||
_TOOL_CALL_RE = re.compile(
|
||||
@@ -923,9 +975,20 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
|
||||
# Pattern 1: fenced code blocks (skipped when `skip_fenced` — see docstring).
|
||||
if not skip_fenced:
|
||||
for m in _TOOL_BLOCK_RE.finditer(text):
|
||||
tag = m.group(1).lower()
|
||||
content = m.group(2).strip()
|
||||
call = _fenced_tool_call(m)
|
||||
if call is None:
|
||||
continue
|
||||
tag, content = call
|
||||
if not content:
|
||||
# An empty fence is still an unambiguous call for the email
|
||||
# tools — ```list_email_accounts``` with no body is a shape
|
||||
# local models really emit for no-arg tools. Dispatch with
|
||||
# empty args and let the tool's own validation answer;
|
||||
# silently dropping the call left models concluding email was
|
||||
# broken. Other tags (bash, python, ...) keep skipping: empty
|
||||
# content is nothing to run.
|
||||
if tag in BUILTIN_EMAIL_TOOLS:
|
||||
blocks.append(ToolBlock(tag, ""))
|
||||
continue
|
||||
# If a code block's content is an <invoke> XML call (some models wrap
|
||||
# tool calls in ```python or ```xml fences), parse the invoke instead.
|
||||
@@ -1037,7 +1100,10 @@ def strip_tool_blocks(text: str, skip_fenced: bool = False) -> str:
|
||||
# Normalize DSML first so its markup gets stripped by the <invoke>
|
||||
# / <tool_call> removers below instead of leaking to the user.
|
||||
text = _normalize_dsml(text)
|
||||
cleaned = text if skip_fenced else _TOOL_BLOCK_RE.sub('', text)
|
||||
# Keep the executed-vs-illustrative fence distinction (only strip fences
|
||||
# that actually dispatched; leave example fences from native models inert
|
||||
# but visible), then remove [TOOL_CALL]{...}[/TOOL_CALL] markup.
|
||||
cleaned = text if skip_fenced else _TOOL_BLOCK_RE.sub(_strip_executed_fence, text)
|
||||
# Forward-only removal mirrors parse_tool_blocks: _strip_delimited pairs each
|
||||
# opener with a later closer and stops when none is reachable, so untrusted
|
||||
# output can't drive the O(n^2) lazy-rescan (ReDoS); see _iter_delimited.
|
||||
|
||||
+5
-4
@@ -14,6 +14,7 @@ from typing import Optional
|
||||
|
||||
from src.agent_tools import ToolBlock, TOOL_TAGS
|
||||
from src.tool_parsing import _TOOL_NAME_MAP
|
||||
from src.tool_security import BUILTIN_EMAIL_TOOLS
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -1222,15 +1223,15 @@ def function_call_to_tool_block(name: str, arguments: str) -> Optional[ToolBlock
|
||||
return None
|
||||
|
||||
tool_type = _TOOL_NAME_MAP.get(name, name)
|
||||
_BUILTIN_EMAIL_TOOLS = {"list_email_accounts", "send_email", "list_emails", "read_email", "reply_to_email",
|
||||
"archive_email", "delete_email", "mark_email_read", "bulk_email", "download_attachment"}
|
||||
|
||||
# Some models emit valid JSON that isn't an object (e.g. a bare array
|
||||
# ["ls -la"], string, or number) as function arguments. Most local tools keep
|
||||
# the legacy empty-object coercion for stream robustness, but email MCP tools
|
||||
# must fail closed so a malformed call cannot read the default mailbox.
|
||||
# Uses the shared BUILTIN_EMAIL_TOOLS (single source of truth) so the
|
||||
# fail-closed set can't drift from the dispatch/blocklist sets.
|
||||
if not isinstance(args, dict):
|
||||
if tool_type.startswith("mcp__email__") or name in _BUILTIN_EMAIL_TOOLS:
|
||||
if tool_type.startswith("mcp__email__") or name in BUILTIN_EMAIL_TOOLS:
|
||||
logger.warning(f"Non-object email function call arguments for {name}: {args!r}; rejecting")
|
||||
return None
|
||||
logger.warning(f"Non-object function call arguments for {name}: {args!r}; treating as empty")
|
||||
@@ -1241,7 +1242,7 @@ def function_call_to_tool_block(name: str, arguments: str) -> Optional[ToolBlock
|
||||
content = json.dumps(args) if args else "{}"
|
||||
return ToolBlock(tool_type, content)
|
||||
# Email tools are implemented as MCP — route them to email
|
||||
if name in _BUILTIN_EMAIL_TOOLS:
|
||||
if name in BUILTIN_EMAIL_TOOLS:
|
||||
return ToolBlock(f"mcp__email__{name}", json.dumps(args) if args else "{}")
|
||||
if tool_type not in TOOL_TAGS:
|
||||
logger.warning(f"Unknown function call: {name}")
|
||||
|
||||
+70
-7
@@ -8,10 +8,36 @@ from typing import Optional, Set
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
# Every tool exposed by the built-in email MCP server
|
||||
# (mcp_servers/email_server.py). Single source of truth: the fence tags
|
||||
# (TOOL_TAGS), bare-name dispatch (tool_execution), native-call mapping
|
||||
# (tool_schemas), and the non-admin blocklist below all derive from this set,
|
||||
# so a tool added to the email server can't become reachable under its bare
|
||||
# name without also being blocked for non-admins.
|
||||
BUILTIN_EMAIL_TOOLS = frozenset({
|
||||
"list_email_accounts",
|
||||
"list_emails",
|
||||
"read_email",
|
||||
"search_emails",
|
||||
"send_email",
|
||||
"reply_to_email",
|
||||
"draft_email",
|
||||
"draft_email_reply",
|
||||
"ai_draft_email_reply",
|
||||
"archive_email",
|
||||
"delete_email",
|
||||
"mark_email_read",
|
||||
"bulk_email",
|
||||
"download_attachment",
|
||||
})
|
||||
|
||||
|
||||
# Tools regular/public users must not execute directly. These either expose
|
||||
# server/runtime access, sensitive user data, external messaging, persistent
|
||||
# state changes, or generic loopback/integration surfaces.
|
||||
NON_ADMIN_BLOCKED_TOOLS = {
|
||||
# state changes, or generic loopback/integration surfaces. All email tools are
|
||||
# included (SECURITY.md: email/MCP capabilities are privileged admin
|
||||
# functionality).
|
||||
NON_ADMIN_BLOCKED_TOOLS = BUILTIN_EMAIL_TOOLS | {
|
||||
"bash",
|
||||
"python",
|
||||
"manage_bg_jobs",
|
||||
@@ -34,10 +60,6 @@ NON_ADMIN_BLOCKED_TOOLS = {
|
||||
"manage_settings",
|
||||
"api_call",
|
||||
"app_api",
|
||||
"send_email",
|
||||
"reply_to_email",
|
||||
"list_emails",
|
||||
"read_email",
|
||||
"resolve_contact",
|
||||
"manage_contact",
|
||||
"manage_calendar",
|
||||
@@ -74,8 +96,20 @@ PLAN_MODE_READONLY_TOOLS = {
|
||||
"search_chats",
|
||||
"list_models",
|
||||
"list_sessions",
|
||||
# Read-only email tools. list_email_accounts must be here because the
|
||||
# bare/qualified alias gate in execute_tool_block works both ways: it has
|
||||
# a native function schema, so plan mode's schema-derived bare denylist
|
||||
# contains it — and without this allowlist entry that bare entry would
|
||||
# also block the qualified mcp__email__list_email_accounts call that the
|
||||
# MCP read-only filter deliberately allows.
|
||||
"list_email_accounts",
|
||||
"list_emails",
|
||||
"read_email",
|
||||
# Explicitly read-only rather than allowed-by-omission: this PR makes
|
||||
# every BUILTIN_EMAIL_TOOLS name fence-taggable, so each one must be
|
||||
# classified — see the plan-mode partition test in
|
||||
# tests/test_email_registry_sync.py.
|
||||
"search_emails",
|
||||
"list_served_models",
|
||||
"list_downloads",
|
||||
"list_cached_models",
|
||||
@@ -109,7 +143,14 @@ _PLAN_MODE_KNOWN_MUTATORS = {
|
||||
"manage_webhooks", "manage_tokens", "manage_settings", "manage_contact",
|
||||
"manage_calendar", "api_call", "app_api", "ui_control",
|
||||
"send_email", "reply_to_email", "bulk_email", "delete_email",
|
||||
"archive_email", "mark_email_read", "download_model", "serve_model",
|
||||
"archive_email", "mark_email_read",
|
||||
# The draft tools create documents and download_attachment writes to
|
||||
# disk — mutating. They have no native schemas (yet), so without these
|
||||
# static entries plan-mode safety for their bare fence tags would depend
|
||||
# entirely on the MCP read-only inventory being present and current.
|
||||
"draft_email", "draft_email_reply", "ai_draft_email_reply",
|
||||
"download_attachment",
|
||||
"download_model", "serve_model",
|
||||
"stop_served_model", "cancel_download", "adopt_served_model", "serve_preset",
|
||||
"generate_image", "edit_image", "trigger_research", "manage_research",
|
||||
# Shell is never read-only-safe; block it explicitly so it stays out of plan
|
||||
@@ -151,6 +192,28 @@ def plan_mode_disabled_tools() -> Set[str]:
|
||||
return (all_names | _PLAN_MODE_KNOWN_MUTATORS) - PLAN_MODE_READONLY_TOOLS
|
||||
|
||||
|
||||
def email_tool_policy_names(tool_name: str) -> frozenset:
|
||||
"""All policy-equivalent spellings of a tool name.
|
||||
|
||||
A bare built-in email tool name and its MCP-qualified mcp__email__<name>
|
||||
form dispatch to the same email server tool, but policy sources spell
|
||||
them either way — plan mode and the MCP settings toggle write qualified
|
||||
names into denylists, chat-level toggles write bare ones. Every gate must
|
||||
match against the full alias set, or a call in one spelling slips past a
|
||||
denylist entry written in the other. Non-email names alias only to
|
||||
themselves.
|
||||
"""
|
||||
if not isinstance(tool_name, str):
|
||||
return frozenset((tool_name,))
|
||||
if tool_name in BUILTIN_EMAIL_TOOLS:
|
||||
return frozenset((tool_name, f"mcp__email__{tool_name}"))
|
||||
if tool_name.startswith("mcp__email__"):
|
||||
bare = tool_name[len("mcp__email__"):]
|
||||
if bare in BUILTIN_EMAIL_TOOLS:
|
||||
return frozenset((tool_name, bare))
|
||||
return frozenset((tool_name,))
|
||||
|
||||
|
||||
def is_public_blocked_tool(tool_name: Optional[str]) -> bool:
|
||||
"""Return True when a non-admin/public user must not execute this tool.
|
||||
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
"""PR #3681 — the surfaces this PR derives from BUILTIN_EMAIL_TOOLS stay in sync.
|
||||
|
||||
The review rounds on #3681 each found a hand-maintained copy of the email tool
|
||||
list that had drifted. This PR's scope pins the SECURITY-RELEVANT surfaces to
|
||||
the single source of truth (the email MCP server itself, the fence tags, the
|
||||
non-admin blocklist, the bare<->qualified alias rule, and the plan-mode
|
||||
read-only fix the alias gate requires). The wider advertising/registry
|
||||
consolidation (schemas, prompt sections, RAG index, UI selector, assistant
|
||||
seed) lives in a follow-up PR with its own sync tests.
|
||||
"""
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import src.agent_tools # noqa: F401 — resolve the circular-import cluster first
|
||||
from src.tool_security import (
|
||||
BUILTIN_EMAIL_TOOLS,
|
||||
NON_ADMIN_BLOCKED_TOOLS,
|
||||
PLAN_MODE_READONLY_TOOLS,
|
||||
)
|
||||
|
||||
_REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
||||
|
||||
def test_email_server_tools_match_builtin_set():
|
||||
"""BUILTIN_EMAIL_TOOLS must equal exactly what the email server exposes."""
|
||||
source = (_REPO_ROOT / "mcp_servers" / "email_server.py").read_text()
|
||||
served = set(re.findall(r'Tool\(\s*name="(\w+)"', source))
|
||||
assert served == set(BUILTIN_EMAIL_TOOLS), (
|
||||
f"email_server tools != BUILTIN_EMAIL_TOOLS; "
|
||||
f"server-only: {sorted(served - BUILTIN_EMAIL_TOOLS)}, "
|
||||
f"set-only: {sorted(BUILTIN_EMAIL_TOOLS - served)}"
|
||||
)
|
||||
|
||||
|
||||
def test_fence_tags_cover_email_tools():
|
||||
from src.agent_tools import TOOL_TAGS
|
||||
|
||||
assert BUILTIN_EMAIL_TOOLS <= set(TOOL_TAGS)
|
||||
|
||||
|
||||
def test_non_admin_blocklist_covers_email_tools():
|
||||
assert BUILTIN_EMAIL_TOOLS <= NON_ADMIN_BLOCKED_TOOLS
|
||||
|
||||
|
||||
def test_plan_mode_classifies_every_email_tool():
|
||||
"""Every fence-taggable email tool must be EXPLICITLY classified for plan
|
||||
mode: read-only (allowlisted) or mutating (in the static denylist via the
|
||||
fail-closed backstop). Allowed-by-omission is not a classification — it
|
||||
silently flips when schemas/backstop change, and it leaves bare-alias
|
||||
safety depending on the MCP read-only inventory being present."""
|
||||
from src.tool_security import plan_mode_disabled_tools
|
||||
|
||||
denied = plan_mode_disabled_tools()
|
||||
readonly = {"list_email_accounts", "list_emails", "read_email", "search_emails"}
|
||||
for tool in sorted(BUILTIN_EMAIL_TOOLS):
|
||||
if tool in readonly:
|
||||
assert tool in PLAN_MODE_READONLY_TOOLS, f"{tool} must be explicit read-only"
|
||||
assert tool not in denied, f"read-only {tool} must not be denied in plan mode"
|
||||
else:
|
||||
assert tool in denied, f"mutating {tool} missing from the plan-mode denylist"
|
||||
|
||||
|
||||
def test_plan_mode_allows_qualified_readonly_email_discovery():
|
||||
"""list_email_accounts has a native schema, so plan mode's schema-derived
|
||||
bare denylist contains it; with the bidirectional alias gate, the bare
|
||||
entry would also block the qualified mcp__email__ call that the MCP
|
||||
read-only filter deliberately allows — unless it's in the read-only
|
||||
allowlist (which subtracts it from the denylist)."""
|
||||
assert "list_email_accounts" in PLAN_MODE_READONLY_TOOLS
|
||||
|
||||
|
||||
def test_email_policy_name_aliases():
|
||||
"""The alias rule every execution gate relies on."""
|
||||
from src.tool_security import email_tool_policy_names
|
||||
|
||||
assert email_tool_policy_names("list_emails") == {
|
||||
"list_emails", "mcp__email__list_emails",
|
||||
}
|
||||
assert email_tool_policy_names("mcp__email__delete_email") == {
|
||||
"delete_email", "mcp__email__delete_email",
|
||||
}
|
||||
# Non-email names alias only to themselves — including mcp__email__
|
||||
# spellings of tools the email server doesn't expose.
|
||||
assert email_tool_policy_names("bash") == {"bash"}
|
||||
assert email_tool_policy_names("mcp__email__not_a_tool") == {"mcp__email__not_a_tool"}
|
||||
assert email_tool_policy_names("mcp__other__list_emails") == {"mcp__other__list_emails"}
|
||||
@@ -0,0 +1,186 @@
|
||||
"""PR #3681 — fenced tool calls with inline args, and the fence-tag boundary.
|
||||
|
||||
Local fenced-block models (Ollama etc.) emit calls like ```list_email_accounts {}
|
||||
with the args on the same line as the tag; the parser must execute those. The
|
||||
relaxed tag pattern must NOT prefix-match longer fence tags: ```python3 is a
|
||||
language hint, not a "python" tool call with content "3\n...".
|
||||
"""
|
||||
import sys
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
for mod in ['src.agent_tools', 'src.tool_parsing', 'src.tool_schemas', 'src.tool_execution']:
|
||||
sys.modules.pop(mod, None)
|
||||
for mod in [
|
||||
'sqlalchemy', 'sqlalchemy.orm', 'sqlalchemy.ext', 'sqlalchemy.ext.declarative',
|
||||
'sqlalchemy.ext.hybrid', 'sqlalchemy.sql', 'sqlalchemy.sql.expression',
|
||||
'src.database', 'core.models', 'core.database', 'core.auth'
|
||||
]:
|
||||
if mod not in sys.modules:
|
||||
sys.modules[mod] = MagicMock()
|
||||
|
||||
import src.agent_tools # noqa: E402, F401
|
||||
from src.tool_parsing import parse_tool_blocks, strip_tool_blocks # noqa: E402
|
||||
|
||||
|
||||
def test_inline_args_on_tag_line_parse():
|
||||
# The original bug: ```list_email_accounts {} (args on the tag line)
|
||||
# never matched because the regex required a newline right after the tag.
|
||||
blocks = parse_tool_blocks('```list_email_accounts {}\n```')
|
||||
assert [(b.tool_type, b.content) for b in blocks] == [("list_email_accounts", "{}")]
|
||||
|
||||
|
||||
def test_inline_json_args_parse_for_email_tools():
|
||||
blocks = parse_tool_blocks('```list_emails {"max_results": 5}\n```')
|
||||
assert [(b.tool_type, b.content) for b in blocks] == [("list_emails", '{"max_results": 5}')]
|
||||
|
||||
|
||||
def test_next_line_content_still_parses():
|
||||
# No regression for the classic shape: tag, newline, content.
|
||||
blocks = parse_tool_blocks('```manage_memory\nadd\nsome text\n```')
|
||||
assert [(b.tool_type, b.content) for b in blocks] == [("manage_memory", "add\nsome text")]
|
||||
|
||||
|
||||
def test_plain_bash_fence_still_parses():
|
||||
blocks = parse_tool_blocks('```bash\necho hello\n```')
|
||||
assert [(b.tool_type, b.content) for b in blocks] == [("bash", "echo hello")]
|
||||
|
||||
|
||||
def test_python3_language_hint_is_not_a_python_tool_call():
|
||||
# ```python3 must not prefix-match the "python" fence tag — without the
|
||||
# (?![\w-]) boundary it parsed as tool "python" with content "3\nprint(...)"
|
||||
# and executed as code.
|
||||
blocks = parse_tool_blocks('```python3\nprint("hi")\n```')
|
||||
assert blocks == [], blocks
|
||||
|
||||
|
||||
def test_hyphenated_tag_is_not_a_tool_call():
|
||||
blocks = parse_tool_blocks('```bash-session\n$ ls\n```')
|
||||
assert blocks == [], blocks
|
||||
|
||||
|
||||
def test_markdown_info_string_is_not_executable_python():
|
||||
# ```python title="example.py" is Markdown fence metadata, not tool args.
|
||||
# Same-line content other than JSON args ({...}/[...]) must not execute —
|
||||
# otherwise a fence the model meant to display runs as code.
|
||||
blocks = parse_tool_blocks('```python title="example.py"\nprint("hi")\n```')
|
||||
assert blocks == [], blocks
|
||||
|
||||
|
||||
def test_markdown_info_string_is_not_executable_bash():
|
||||
blocks = parse_tool_blocks('```bash title="setup"\necho hi\n```')
|
||||
assert blocks == [], blocks
|
||||
|
||||
|
||||
def test_empty_email_fence_is_an_executable_call():
|
||||
# ```list_email_accounts``` with no body is a real shape local models emit
|
||||
# for no-arg tools — it must dispatch (with empty args), not vanish.
|
||||
blocks = parse_tool_blocks('```list_email_accounts\n```')
|
||||
assert [(b.tool_type, b.content) for b in blocks] == [("list_email_accounts", "")]
|
||||
|
||||
|
||||
def test_empty_non_email_fence_still_skipped():
|
||||
# Empty bash/python/other fences stay inert: empty content is nothing to run.
|
||||
for tag in ("bash", "python", "manage_memory"):
|
||||
assert parse_tool_blocks(f'```{tag}\n```') == []
|
||||
|
||||
|
||||
def test_empty_email_fence_is_stripped_from_display():
|
||||
# Executed (empty-args) email fences mirror like any executed fence.
|
||||
text = 'One sec.\n```list_email_accounts\n```\nDone.'
|
||||
assert strip_tool_blocks(text) == 'One sec.\n\nDone.'
|
||||
|
||||
|
||||
def test_inline_json_array_args_still_parse():
|
||||
# The narrowed same-line rule must keep accepting JSON args: { or [.
|
||||
blocks = parse_tool_blocks('```bulk_email {"action": "archive", "uids": [1, 2]}\n```')
|
||||
assert [(b.tool_type, b.content) for b in blocks] == [
|
||||
("bulk_email", '{"action": "archive", "uids": [1, 2]}')
|
||||
]
|
||||
|
||||
|
||||
def test_brace_metadata_on_bash_is_not_executable():
|
||||
# ```bash {title="setup"} is a Markdown fence attribute on a real
|
||||
# language. Code tags (bash/python) never take same-line args — even a
|
||||
# brace-shaped info string must stay display text.
|
||||
blocks = parse_tool_blocks('```bash {title="setup"}\necho hi\n```')
|
||||
assert blocks == [], blocks
|
||||
|
||||
|
||||
def test_valid_json_metadata_on_python_is_not_executable():
|
||||
# Same rule when the attribute happens to BE valid JSON: the tag decides.
|
||||
blocks = parse_tool_blocks('```python {"title": "example.py"}\nprint("hi")\n```')
|
||||
assert blocks == [], blocks
|
||||
|
||||
|
||||
def test_invalid_inline_json_on_email_tool_is_not_executable():
|
||||
# JSON-args tools only execute same-line content that parses as JSON —
|
||||
# {title="x"} is metadata/garbage, not arguments.
|
||||
blocks = parse_tool_blocks('```list_emails {title="x"}\n```')
|
||||
assert blocks == [], blocks
|
||||
|
||||
|
||||
def test_inline_json_continuing_on_next_lines_still_parses():
|
||||
# A JSON object opened on the tag line may close on a later line.
|
||||
blocks = parse_tool_blocks('```list_emails {"folder": "INBOX",\n"max_results": 5}\n```')
|
||||
assert [(b.tool_type, b.content) for b in blocks] == [
|
||||
("list_emails", '{"folder": "INBOX",\n"max_results": 5}')
|
||||
]
|
||||
|
||||
|
||||
def test_brace_metadata_fences_left_intact_in_display():
|
||||
# strip must mirror parse for every rejected fence shape.
|
||||
for text in (
|
||||
'Example:\n```bash {title="setup"}\necho hi\n```',
|
||||
'Example:\n```python {"title": "example.py"}\nprint("hi")\n```',
|
||||
'Example:\n```list_emails {title="x"}\n```',
|
||||
):
|
||||
assert strip_tool_blocks(text) == text
|
||||
|
||||
|
||||
def test_inline_args_fence_is_stripped_from_display():
|
||||
# strip must mirror parse: an executed inline-args fence must not leak
|
||||
# into the displayed text.
|
||||
text = 'Checking now.\n```list_email_accounts {}\n```\nDone.'
|
||||
assert strip_tool_blocks(text) == 'Checking now.\n\nDone.'
|
||||
|
||||
|
||||
def test_python3_fence_is_left_intact_in_display():
|
||||
# ...and a fence that did NOT parse as a tool call must stay visible.
|
||||
text = 'Example:\n```python3\nprint("hi")\n```'
|
||||
assert strip_tool_blocks(text) == text
|
||||
|
||||
|
||||
def test_markdown_info_string_fence_is_left_intact_in_display():
|
||||
# strip must mirror parse for info-string fences too: not executed,
|
||||
# so not stripped from the displayed text.
|
||||
text = 'Example:\n```python title="example.py"\nprint("hi")\n```'
|
||||
assert strip_tool_blocks(text) == text
|
||||
|
||||
|
||||
def test_parse_strip_mirror_across_fence_shape_grid():
|
||||
# Invariant for ANY single fence: either it executes AND is stripped, or
|
||||
# it doesn't execute AND stays fully visible. The one allowed exception is
|
||||
# an empty NON-EMAIL tool fence (no header, no body): never executed, but
|
||||
# stripped as noise — pre-PR behavior, kept deliberately. (Empty EMAIL
|
||||
# fences execute with empty args, so they fall under the first branch.)
|
||||
from src.agent_tools import TOOL_TAGS
|
||||
|
||||
tags = ["bash", "python", "list_emails", "bulk_email", "manage_memory",
|
||||
"python3", "bash-session", "notatool"]
|
||||
headers = ["", " ", ' title="x"', ' {title="x"}', ' {"a": 1}', " [1, 2]",
|
||||
" {bad json", ' {"a": 1} extra']
|
||||
bodies = ["", "content line\n", '{"k": "v"}\n']
|
||||
|
||||
for tag in tags:
|
||||
for header in headers:
|
||||
for body in bodies:
|
||||
text = f"before\n```{tag}{header}\n{body}```\nafter"
|
||||
blocks = parse_tool_blocks(text)
|
||||
stripped = strip_tool_blocks(text)
|
||||
case = (tag, header, body)
|
||||
if blocks:
|
||||
assert stripped == "before\n\nafter", case
|
||||
elif stripped != text:
|
||||
assert (
|
||||
tag in TOOL_TAGS and not header.strip() and not body.strip()
|
||||
), f"non-executed fence was stripped: {case}"
|
||||
@@ -24,7 +24,6 @@ import re
|
||||
from pathlib import Path
|
||||
|
||||
_SRC = Path("static/js/chatRenderer.js")
|
||||
_TOOLS_SRC = Path("src/agent_tools/__init__.py")
|
||||
_ROUTES_SRC = Path("routes/model_routes.py")
|
||||
|
||||
# Deliberately NOT stripped: legitimate code-example languages, not tool
|
||||
@@ -33,11 +32,13 @@ _NON_STRIPPED = {"bash", "python"}
|
||||
|
||||
|
||||
def _tool_tags() -> set[str]:
|
||||
"""Extract the backend TOOL_TAGS set from src/agent_tools/__init__.py (source-level)."""
|
||||
source = _TOOLS_SRC.read_text(encoding="utf-8")
|
||||
m = re.search(r"TOOL_TAGS\s*=\s*\{(?P<body>.*?)\}", source, re.DOTALL)
|
||||
assert m, "TOOL_TAGS literal not found in src/agent_tools/__init__.py"
|
||||
return set(re.findall(r'"([a-z_]+)"', m.group("body")))
|
||||
"""The backend TOOL_TAGS set — the same authoritative set GET /api/tools
|
||||
serves (sorted) and the live EXEC_FENCE_RE derives from. Imported rather
|
||||
than source-scraped so it reflects the real set however it is composed: the
|
||||
literal plus the ``| BUILTIN_EMAIL_TOOLS`` union (email tool names live in
|
||||
that single source, not inline in the literal)."""
|
||||
from src.agent_tools import TOOL_TAGS
|
||||
return set(TOOL_TAGS)
|
||||
|
||||
|
||||
def _exec_fence_regex() -> re.Pattern:
|
||||
|
||||
@@ -616,7 +616,16 @@ async def test_public_agent_policy_blocks_sensitive_tools(monkeypatch):
|
||||
|
||||
monkeypatch.setattr(auth_mod, "AuthManager", lambda: FakeAuth())
|
||||
|
||||
for tool_name in ("send_email", "read_file", "mcp__email__send_email"):
|
||||
# Every bare email tool name is spelled out (not imported from
|
||||
# BUILTIN_EMAIL_TOOLS) so accidentally dropping one from that set fails
|
||||
# here instead of silently shrinking the blocklist.
|
||||
bare_email_tools = (
|
||||
"list_email_accounts", "list_emails", "read_email", "search_emails",
|
||||
"send_email", "reply_to_email", "draft_email", "draft_email_reply",
|
||||
"ai_draft_email_reply", "archive_email", "delete_email",
|
||||
"mark_email_read", "bulk_email", "download_attachment",
|
||||
)
|
||||
for tool_name in bare_email_tools + ("read_file", "mcp__email__send_email"):
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type=tool_name, content="{}"),
|
||||
owner="regular-user",
|
||||
@@ -626,6 +635,311 @@ async def test_public_agent_policy_blocks_sensitive_tools(monkeypatch):
|
||||
assert "restricted to admin users" in result["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disabled_qualified_email_tool_blocks_bare_alias(monkeypatch):
|
||||
"""A bare email fence is an alias for its mcp__email__ form. Plan mode and
|
||||
the MCP settings toggle write the QUALIFIED name into disabled_tools, so
|
||||
the gate must block the bare spelling too — and never reach the MCP
|
||||
manager (PR #3681 review follow-up)."""
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import execute_tool_block
|
||||
|
||||
def fail_get_mcp_manager():
|
||||
raise AssertionError("blocked email tool must not reach the MCP manager")
|
||||
|
||||
monkeypatch.setattr(tool_execution, "get_mcp_manager", fail_get_mcp_manager)
|
||||
|
||||
for bare, disabled in (
|
||||
# qualified denylist entry blocks the bare alias…
|
||||
("list_emails", {"mcp__email__list_emails"}),
|
||||
("download_attachment", {"mcp__email__download_attachment"}),
|
||||
# …and a bare denylist entry blocks the qualified spelling.
|
||||
("mcp__email__delete_email", {"delete_email"}),
|
||||
):
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type=bare, content="{}"),
|
||||
owner="admin-user",
|
||||
disabled_tools=disabled,
|
||||
)
|
||||
assert desc == f"{bare}: BLOCKED"
|
||||
assert result["exit_code"] == 1
|
||||
assert "disabled by user" in result["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_tool_policy_qualified_email_block_covers_bare_alias(monkeypatch):
|
||||
"""Same aliasing rule for the turn ToolPolicy denylist."""
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import execute_tool_block
|
||||
from src.tool_policy import ToolPolicy
|
||||
|
||||
def fail_get_mcp_manager():
|
||||
raise AssertionError("blocked email tool must not reach the MCP manager")
|
||||
|
||||
monkeypatch.setattr(tool_execution, "get_mcp_manager", fail_get_mcp_manager)
|
||||
|
||||
policy = ToolPolicy(disabled_tools=frozenset({"mcp__email__send_email"}))
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type="send_email", content="{}"),
|
||||
owner="admin-user",
|
||||
tool_policy=policy,
|
||||
)
|
||||
assert desc == "send_email: BLOCKED"
|
||||
assert result["exit_code"] == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disable_tool_email_covers_full_builtin_set(monkeypatch):
|
||||
"""The friendly `disable_tool email` toggle must cover every built-in
|
||||
email tool, in BOTH spellings — bare names (function-schema hiding,
|
||||
bare-fence dispatch) and mcp__email__* (MCP schema hiding, runtime
|
||||
qualified blocks). Hand-picking a subset left tools like delete_email
|
||||
and download_attachment enabled (PR #3681 review follow-up)."""
|
||||
# Import first so the module loads against the real core package; only
|
||||
# the call-time SessionLocal import below sees the stub.
|
||||
from src.tool_implementations import do_manage_settings
|
||||
import src.settings as settings_mod
|
||||
|
||||
db_mod = types.ModuleType("core.database")
|
||||
|
||||
class _Db:
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
db_mod.SessionLocal = lambda: _Db()
|
||||
monkeypatch.setitem(sys.modules, "core.database", db_mod)
|
||||
|
||||
store = {}
|
||||
|
||||
def fake_load_settings():
|
||||
return dict(store)
|
||||
|
||||
def fake_save_settings(s):
|
||||
store.clear()
|
||||
store.update(s)
|
||||
|
||||
monkeypatch.setattr(settings_mod, "load_settings", fake_load_settings)
|
||||
monkeypatch.setattr(settings_mod, "save_settings", fake_save_settings)
|
||||
|
||||
result = await do_manage_settings(
|
||||
'{"action": "disable_tool", "tool": "email"}', owner="admin"
|
||||
)
|
||||
|
||||
assert result["exit_code"] == 0
|
||||
disabled = set(store["disabled_tools"])
|
||||
# Spelled out (not imported from BUILTIN_EMAIL_TOOLS) so dropping a name
|
||||
# from the constant fails here instead of silently shrinking the toggle.
|
||||
bare_email_tools = (
|
||||
"list_email_accounts", "list_emails", "read_email", "search_emails",
|
||||
"send_email", "reply_to_email", "draft_email", "draft_email_reply",
|
||||
"ai_draft_email_reply", "archive_email", "delete_email",
|
||||
"mark_email_read", "bulk_email", "download_attachment",
|
||||
)
|
||||
for tool_name in bare_email_tools:
|
||||
assert tool_name in disabled, tool_name
|
||||
assert f"mcp__email__{tool_name}" in disabled, tool_name
|
||||
|
||||
# enable_tool email must remove the full set again.
|
||||
result = await do_manage_settings(
|
||||
'{"action": "enable_tool", "tool": "email"}', owner="admin"
|
||||
)
|
||||
assert result["exit_code"] == 0
|
||||
assert store["disabled_tools"] == []
|
||||
|
||||
|
||||
def _install_admin_auth_stub(monkeypatch):
|
||||
auth_mod = _install_core_auth_stub(monkeypatch)
|
||||
|
||||
class FakeAdminAuth:
|
||||
is_configured = True
|
||||
|
||||
def is_admin(self, username):
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(auth_mod, "AuthManager", lambda: FakeAdminAuth())
|
||||
|
||||
|
||||
class _FakeMcpManager:
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
|
||||
async def call_tool(self, name, args):
|
||||
self.calls.append((name, args))
|
||||
return {"output": "ok", "exit_code": 0}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bare_email_dispatch_rejects_non_object_json_args(monkeypatch):
|
||||
"""The fence parser accepts JSON arrays as inline args, but email tools
|
||||
take objects — a correctable error must come back instead of a silent
|
||||
empty-args call (same class as #3966)."""
|
||||
_install_admin_auth_stub(monkeypatch)
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import execute_tool_block
|
||||
|
||||
mcp = _FakeMcpManager()
|
||||
monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp)
|
||||
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type="bulk_email", content='["10", "11"]'),
|
||||
owner="admin-user",
|
||||
)
|
||||
assert result["exit_code"] == 1
|
||||
assert "JSON object" in result["error"]
|
||||
assert mcp.calls == [], "non-object args must never reach the MCP server"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bare_email_dispatch_rejects_invalid_json_body(monkeypatch):
|
||||
"""The classic tag/body form reaches execution unvalidated (only INLINE
|
||||
args are JSON-checked by the parser). A non-JSON-object body must return a
|
||||
correctable parse error — silently becoming {} args would read the DEFAULT
|
||||
mailbox instead of the one the model meant. Covers both the brace-looking
|
||||
`{account: "work"}` and the bare `account: work` shapes."""
|
||||
_install_admin_auth_stub(monkeypatch)
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import execute_tool_block
|
||||
|
||||
for bad_body in ('{account: "work"}', "account: work"):
|
||||
mcp = _FakeMcpManager()
|
||||
monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp)
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type="list_emails", content=bad_body),
|
||||
owner="admin-user",
|
||||
)
|
||||
assert result["exit_code"] == 1, bad_body
|
||||
assert "not valid JSON" in result["error"], bad_body
|
||||
assert mcp.calls == [], f"malformed args must never reach MCP: {bad_body!r}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_legacy_mcp_tools_decode_inline_json_args(monkeypatch):
|
||||
"""The relaxed parser accepts inline JSON for non-code tags, but the legacy
|
||||
line-based arg builders (web_search/web_fetch/read_file/write_file/
|
||||
generate_image) would wrap the whole JSON string as the query/path/prompt.
|
||||
A JSON object carrying the tool's primary key must be used directly."""
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import _build_mcp_args
|
||||
|
||||
cases = {
|
||||
"web_search": ('{"query": "odysseus pr 3681"}', {"query": "odysseus pr 3681"}),
|
||||
"web_fetch": ('{"url": "https://example.com"}', {"url": "https://example.com"}),
|
||||
"read_file": ('{"path": "/tmp/x.txt"}', {"path": "/tmp/x.txt"}),
|
||||
"write_file": ('{"path": "/tmp/x", "content": "hi"}', {"path": "/tmp/x", "content": "hi"}),
|
||||
"generate_image": ('{"prompt": "a cat"}', {"prompt": "a cat"}),
|
||||
}
|
||||
for tool, (content, expected) in cases.items():
|
||||
assert _build_mcp_args(tool, content) == expected, tool
|
||||
|
||||
# Freeform (non-JSON) content keeps the line-based behavior.
|
||||
assert _build_mcp_args("web_search", "latest python release") == {"query": "latest python release"}
|
||||
# A JSON object WITHOUT the tool's primary key is not args — fall back
|
||||
# (write_file content the model happened to write as a bare object).
|
||||
assert _build_mcp_args("write_file", '{"config": "value"}') == {
|
||||
"path": '{"config": "value"}', "content": "",
|
||||
}
|
||||
|
||||
|
||||
def test_mcp_json_primary_keys_are_all_live():
|
||||
"""Every _MCP_JSON_PRIMARY_KEYS entry must be reachable: _build_mcp_args is
|
||||
only called from _call_mcp_tool, which only runs for _MCP_TOOL_MAP tools.
|
||||
An entry outside _MCP_TOOL_MAP is dead code whose inline-JSON decode never
|
||||
executes — manage_memory was exactly that (it routes through
|
||||
dispatch_ai_tool), and a unit test on _build_mcp_args passed on the dead
|
||||
path while the real call still corrupted. This pins it so it can't recur."""
|
||||
from src.tool_execution import _MCP_JSON_PRIMARY_KEYS, _MCP_TOOL_MAP
|
||||
|
||||
dead = set(_MCP_JSON_PRIMARY_KEYS) - set(_MCP_TOOL_MAP)
|
||||
assert not dead, f"dead JSON-primary entries (never reach _build_mcp_args): {sorted(dead)}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_write_file_inline_json_args(monkeypatch):
|
||||
"""write_file has no MCP server, so it runs via _direct_fallback ->
|
||||
WriteFileTool, NOT _build_mcp_args. Inline JSON must therefore be decoded
|
||||
by the handler itself: drive the LIVE path (execute_tool_block, no MCP) and
|
||||
assert the file is written to the intended path with the intended content,
|
||||
not a file literally named with the JSON blob. A _build_mcp_args unit test
|
||||
can't catch this — it's on the dead MCP path for write_file."""
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import execute_tool_block
|
||||
|
||||
monkeypatch.setattr(tool_execution, "_owner_is_admin", lambda owner: True)
|
||||
monkeypatch.setattr(tool_execution, "is_public_blocked_tool", lambda t: False)
|
||||
monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: None)
|
||||
|
||||
captured = {}
|
||||
import src.agent_tools.filesystem_tools as fst
|
||||
|
||||
def fake_resolve(p):
|
||||
captured["path"] = p
|
||||
raise ValueError("probe-stop-before-disk")
|
||||
|
||||
monkeypatch.setattr(tool_execution, "_resolve_tool_path", fake_resolve)
|
||||
|
||||
from src.tool_parsing import parse_tool_blocks
|
||||
blocks = parse_tool_blocks('```write_file {"path": "/tmp/wf.txt", "content": "hi"}\n```')
|
||||
for b in blocks:
|
||||
await execute_tool_block(b, owner="admin")
|
||||
|
||||
assert captured.get("path") == "/tmp/wf.txt", (
|
||||
f"write_file did not decode inline JSON args; got path {captured.get('path')!r}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_plan_mode_blocks_mutating_email_aliases_without_mcp_inventory(monkeypatch):
|
||||
"""Plan-mode safety for bare email aliases must hold from the STATIC
|
||||
partition alone — no MCP read-only inventory involved: mutators (the
|
||||
draft/download tools included) are blocked before dispatch, while the
|
||||
explicitly read-only search_emails goes through."""
|
||||
_install_admin_auth_stub(monkeypatch)
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import execute_tool_block
|
||||
from src.tool_security import plan_mode_disabled_tools
|
||||
|
||||
mcp = _FakeMcpManager()
|
||||
monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp)
|
||||
denied = plan_mode_disabled_tools()
|
||||
|
||||
for tool_name in ("draft_email", "draft_email_reply", "ai_draft_email_reply",
|
||||
"download_attachment", "send_email", "delete_email"):
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type=tool_name, content="{}"),
|
||||
owner="admin-user",
|
||||
disabled_tools=denied,
|
||||
)
|
||||
assert result["exit_code"] == 1, tool_name
|
||||
assert mcp.calls == [], f"{tool_name} reached the MCP server in plan mode"
|
||||
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type="search_emails", content='{"query": "x"}'),
|
||||
owner="admin-user",
|
||||
disabled_tools=denied,
|
||||
)
|
||||
assert result["exit_code"] == 0
|
||||
assert mcp.calls == [("mcp__email__search_emails", {"query": "x"})]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bare_email_dispatch_empty_content_calls_with_empty_args(monkeypatch):
|
||||
"""An empty fence (```list_email_accounts``` with no body) dispatches with
|
||||
{} args — the no-arg call shape local models really emit."""
|
||||
_install_admin_auth_stub(monkeypatch)
|
||||
import src.tool_execution as tool_execution
|
||||
from src.tool_execution import execute_tool_block
|
||||
|
||||
mcp = _FakeMcpManager()
|
||||
monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp)
|
||||
|
||||
desc, result = await execute_tool_block(
|
||||
SimpleNamespace(tool_type="list_email_accounts", content=""),
|
||||
owner="admin-user",
|
||||
)
|
||||
assert result["exit_code"] == 0
|
||||
assert mcp.calls == [("mcp__email__list_email_accounts", {})]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_email_mcp_non_object_args_fail_before_dispatch(monkeypatch):
|
||||
import src.tool_execution as tool_execution
|
||||
|
||||
Reference in New Issue
Block a user