feat(delegate): profile toolset isolation + MCP bypass for no_mcp orchestrators#35526
feat(delegate): profile toolset isolation + MCP bypass for no_mcp orchestrators#35526davidgut1982 wants to merge 7 commits into
Conversation
|
Competing with #32727 — both implement profile-based MCP bypass for delegate_task and close #32668. #32727 also touches gateway/run.py, hermes_cli/main.py, and mcp_tool.py (broader scope). This PR adds batch injection blocking, aliasing prevention, and inherit_mcp guard (narrower but deeper). Maintainer should pick one. |
|
Thanks @alt-glitch — agreed. I'll consolidate onto #35526 and fold in #32727's lazy MCP discovery (mcp_tool.py |
… toolsets bypass no_mcp parent intersection (NousResearch#32668/NousResearch#32727) Under a no_mcp orchestrator (platform_toolsets: [no_mcp, delegation, knowledge]), expanded_parent has zero MCP entries. The previous call site hardcoded profile_name=None into _build_child_agent, so the MCP-bypass branch (lines 972-976 of delegate_tool.py) never activated and fat sub-agents received an empty toolset. Fix: - Add _load_agent_profiles() helper to read agent_profiles from the top-level config (not the delegation sub-key that _load_config() returns). - Add profile: Optional[str] param to delegate_task(); when set, resolve the named profile's toolsets from agent_profiles and store as the effective toolsets for the child. - Change the _build_child_agent call site from profile_name=None to profile_name=resolved_profile_name so the MCP-bypass branch activates for named profiles. - Add "profile" to DELEGATE_TASK_SCHEMA so the model reliably emits the field; without a formal schema entry the LLM strips it at the provider API boundary. - Update registry lambda and _dispatch_delegate_task in run_agent.py to forward profile= through both invocation paths. Security: the bypass is scoped to MCP toolsets only, and only when a named profile explicitly declares them in config. Non-MCP toolsets still go through the parent intersection. Unknown profile names fall back gracefully (warning + no bypass). Tests added (test_delegate_toolset_scope.py): - TestDelegateTaskProfileWiring: verifies delegate_task() calls _build_child_agent with profile_name='documents' and the profile's resolved toolsets. Key regression guard: FAILS against pre-fix code (profile_name=None hardcoded) and PASSES after the fix. - TestDelegateTaskSchemaProfile: asserts 'profile' is a declared string property in DELEGATE_TASK_SCHEMA and is not in required[]. - TestProfileMcpBypassEndToEnd: direct _build_child_agent tests covering the post-fix bypass (profile_name set → mcp-nextcloud-files retained) and the security baseline (profile_name=None → MCP stripped). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…toolset injection (security)
Two privilege-escalation vectors closed:
1. Batch-mode injection (primary fix): the batch loop previously used
`t.get("toolsets") or toolsets` for each task, so a model could name
a valid profile (activating the mcp-* bypass in _build_child_agent)
while supplying per-task toolsets that the profile never declared.
Fix: when resolved_profile_name is set, all tasks in the batch receive
profile_resolved_toolsets exclusively — per-task model-supplied toolsets
are ignored.
2. Empty-profile bypass (secondary fix): a profile with no/empty toolsets
still set resolved_profile_name, activating the mcp-* bypass with
whatever caller-supplied toolsets were present. Fix: resolved_profile_name
is only set when the profile declares a non-empty toolsets list; otherwise
a warning is logged and the normal intersection path is used.
Tests added (tests/tools/test_delegate_toolset_scope.py):
- test_batch_injection_blocked_model_cannot_inject_evil_mcp_toolset: FAILS
pre-fix (mcp-injected-evil present), PASSES post-fix
- test_single_task_profile_toolsets_unchanged: regression guard, PASSES both
- test_empty_profile_toolsets_bypass_not_activated: FAILS pre-fix
(profile_name set), PASSES post-fix
Total: 16 → 19 tests, all green. tool_search: 63 pass, no collateral.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l security hardening) Assign list(profile_toolsets) instead of the bare reference at line ~2082 so in-place mutations of the working `toolsets` variable cannot corrupt agent_profiles config for the process lifetime. Also take an independent list(toolsets) copy for profile_resolved_toolsets at line ~2110 (defence- in-depth against future code inserted between the two assignments). Adds TestProfileToolsetsAliasing::test_profile_toolsets_copy_prevents_config_corruption: resolves a profile, mutates the child toolsets list returned to _build_child_agent, and asserts the original profiles["documents"]["toolsets"] config list is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rent MCP bleed (security hardening) When a named agent_profile is used, the profile's declared toolsets are authoritative. Previously _preserve_parent_mcp_toolsets ran unconditionally when inherit_mcp_toolsets=True, silently appending every parent MCP toolset absent from the child's list — including ones the profile never declared. Guard the bleed path with `and not profile_name` so inheritance is skipped whenever delegation resolves through a profile. Non-profile (ad-hoc) delegation continues to inherit parent MCP toolsets unchanged. The variable `profile_name` is already in scope at the call site (function parameter, also used at line 972 for the existing MCP bypass guard). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…G reaches the log
_read_logging_config now returns the loggers dict (4th element).
setup_logging iterates logging.loggers.<name> entries after attaching
handlers and calls logging.getLogger(name).setLevel() for each valid
entry. Both bare-string ("DEBUG") and dict ({level: "DEBUG"}) shapes
are accepted; invalid levels are silently skipped.
When a per-logger override is finer-grained than the configured
top-level (e.g. DEBUG while agent.log runs at INFO), the root logger
and the agent.log handler are also lowered to the minimum per-logger
level so that propagated records actually reach the file.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c8d1948 to
74ceaa5
Compare
When the active platform includes the no_mcp sentinel in its toolsets, skip eager MCP server discovery at gateway/CLI startup. Discovery is deferred until the first delegate_task() call that targets an MCP toolset, using a thread-safe one-shot Event/Lock pattern. This eliminates unnecessary MCP connection overhead for api_server platform (orchestrator) while preserving full MCP access for child agents via the Phase 1 profile_name bypass. cli/cron/telegram platforms are unaffected: their toolsets lack no_mcp, so the gate evaluates False and eager discovery runs exactly as before. Changes: - tools/mcp_tool.py: add mark_eager_discovery_skipped() + ensure_mcp_discovered() (one-shot, thread-safe, failure-tolerant lazy discovery trigger) - gateway/run.py: add _active_platform_uses_no_mcp() helper; gate the eager discover_mcp_tools() call in start_gateway() on the platform no_mcp check - hermes_cli/main.py: gate the inline CLI-startup discover_mcp_tools() call in _prepare_agent_startup() with the same no_mcp check (covers `gateway run`) - tools/delegate_tool.py: call ensure_mcp_discovered() before building any child agent that requests MCP toolsets - tests/tools/test_mcp_lazy_discovery.py: 12 tests covering the skip flag, no-op/once/idempotent/thread-safe/failure paths, and platform resolution Part of fix/profile-mcp-toolset-bypass branch (stacked on Phase 1). Resolves: NousResearch#32668 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pre-declare _cfg as Dict[str, Any] = {} so it is always bound and
Pyright knows the type is dict throughout the config bridge block.
Use an intermediate _expanded variable after _expand_env_vars() and
guard with isinstance(_, dict) so the re-assignment stays within the
declared dict type — _expand_env_vars has no return annotation and
Pyright infers a broad str | list | dict union.
Simplify the IPv4 network_cfg line: now that _cfg is always bound and
typed as dict, the old ('_cfg' in dir() else {}) guard is unnecessary.
Fixes Pyright errors at lines 863, 901, 917, 926, 930, 936, 978 that
were introduced by the Phase 2 lazy MCP discovery work.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Lazy MCP discovery from #32727 folded in (this PR is now the complete superset) Folded the lazy MCP discovery work from fix/profile-mcp-toolset-bypass (#32727) into this branch via cherry-pick of:
What was added to this branch:
Test results (run locally on this branch):
This PR (#35526) is now the complete implementation closing #32668. #32727 (fix/profile-mcp-toolset-bypass) will be closed as superseded once this PR merges — it is intentionally left open pending that merge. |
|
Closing this PR — the multi-agent gateway stack (feat/mga-1 through feat/mga-6, merged via #37495–#37502) introduced a competing implementation of delegation profile/toolset isolation using The mga stack's implementation covers the core isolation requirement. Rather than maintain two diverging approaches, this PR is being retired. If the specific security hardening cases here (batch injection blocking, parent MCP bleed guard, config aliasing deep-copy) are not covered by the mga implementation, they should be re-submitted as targeted fixes on top of the mga stack rather than as a competing full rewrite. |
What
Implements per-profile toolset isolation for delegated sub-agents. Profile-declared toolsets are now the single source of truth; batch-mode model-supplied toolsets are rejected when a profile is set, parent MCP context does not leak into children via
inherit_mcp_toolsets, and toolset lists are deep-copied to prevent config aliasing. Four surgical fixes with dedicated tests: profile name wiring, batch injection blocking, config aliasing fix, and parent MCP bleed guard. Also fixes per-logger levels from config so DEBUG logging reaches output.Files modified:
tools/delegate_tool.py(profile wiring, injection block, copy fix, inherit guard),tests/tools/test_delegate_toolset_scope.py(new),config/agent_cfg.py(logger level application).Why
The orchestrator pattern (no_mcp parent routing work to MCP-capable child agents) requires toolset boundaries. Before this PR, three gaps allowed boundary violations: batch-mode injection, parent MCP bleed, and config aliasing. This hardens the security model so delegation is safe at scale.
Tests
4 test classes pass covering all security hardening scenarios; 9 adversarial E2E scenarios confirmed no privilege escalation.
Platforms tested
Linux (CT/LXC environment, Python 3.13)