cleveragents/cleveragents-core

Fork 3

UAT: `SkillService._schema_to_skill_dict()` silently drops `tool_filter.exclude` blacklist — MCP server tool exclusions lost on skill registration #2917

New issue

Open

opened 2026-04-05 02:48:00 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 02:48:00 +00:00

Owner

Metadata

Branch: fix/skill-mcp-tool-filter-exclude-lost
Commit Message: fix(skills): preserve tool_filter.exclude blacklist in _schema_to_skill_dict() and SkillMcpSource domain model
Milestone: v3.7.0
Parent Epic: #393

Bug Report

Feature Area

Skills System — MCP server tool exposure (SkillService._schema_to_skill_dict() / SkillMcpSource domain model)

What Was Tested

The SkillService._schema_to_skill_dict() method in src/cleveragents/application/services/skill_service.py was analyzed for MCP server tool filter support, specifically the tool_filter.exclude blacklist field.

Expected Behavior (from spec)

The skill YAML schema (docs/schema/skill.schema.yaml) defines tool_filter for MCP servers with both include (whitelist) and exclude (blacklist) lists:

tool_filter:
  include:
    type: array
    description: "Whitelist of tool names to expose."
  exclude:
    type: array
    description: "Blacklist of tool names to hide."

The SkillMcpToolFilterSchema in src/cleveragents/skills/schema.py correctly captures both:

class SkillMcpToolFilterSchema(BaseModel):
    include: list[str] = Field(default_factory=list, ...)
    exclude: list[str] = Field(default_factory=list, ...)

When a skill is registered, both include and exclude filters should be preserved in the domain model so that the runtime can correctly restrict which MCP server tools are exposed to agents.

Actual Behavior (from code analysis)

In src/cleveragents/application/services/skill_service.py, _schema_to_skill_dict() only maps tool_filter.include:

mcp_dict: dict[str, Any] = {"server": mcp.name}
if mcp.tool_filter and mcp.tool_filter.include:
    mcp_dict["tools"] = list(mcp.tool_filter.include)
data["mcp_servers"].append(mcp_dict)

The tool_filter.exclude list is silently dropped — there is no mapping for it at all. Furthermore, the SkillMcpSource domain model in src/cleveragents/domain/models/core/skill.py has no exclude field, meaning the blacklist cannot be stored even if the service layer were fixed.

This means users cannot use the blacklist approach to hide specific tools from an MCP server. Only the whitelist approach (include) works, and even then only partially (see related issue #2893 which covers the broader MCP connection details loss).

Steps to Reproduce

Create a skill YAML with MCP tool exclusions:

name: local/safe-github
description: "GitHub integration without destructive operations"
mcp_servers:
  - name: github
    transport: stdio
    command: npx
    args: ["-y", "@modelcontextprotocol/server-github"]
    tool_filter:
      exclude:
        - delete_repository
        - delete_branch

Register: agents skill add --config safe-github.yaml
Inspect: agents skill show local/safe-github
Observe that the exclude filter is lost — all tools including destructive ones would be exposed

Code Locations

src/cleveragents/application/services/skill_service.py — _schema_to_skill_dict() method: only maps tool_filter.include, silently drops tool_filter.exclude
src/cleveragents/domain/models/core/skill.py — SkillMcpSource class: missing exclude field entirely
src/cleveragents/skills/schema.py — SkillMcpToolFilterSchema class: correctly captures exclude (source of truth)
docs/schema/skill.schema.yaml — schema definition showing exclude is a supported, spec-defined field

Severity

High — The exclude filter is a spec-defined safety feature for preventing exposure of dangerous MCP tools. Silent data loss means users cannot safely restrict MCP server tool access using the blacklist approach. This is a security-relevant gap: a user who configures tool_filter.exclude to hide destructive tools (e.g., delete_repository, delete_branch) will believe those tools are hidden, but they will actually be exposed to the agent at runtime.

Subtasks

Add exclude: list[str] field to SkillMcpSource domain model in src/cleveragents/domain/models/core/skill.py (with default_factory=list, matching the schema)
Update _schema_to_skill_dict() in src/cleveragents/application/services/skill_service.py to map tool_filter.exclude into the mcp_dict (e.g., as "exclude_tools" or "tool_exclude" key, consistent with the "tools" key used for include)
Verify round-trip fidelity: schema → dict → domain model → dict preserves both include and exclude lists
Update agents skill show output to display the exclude filter when present
Add Behave BDD unit test scenarios covering: tool_filter.exclude is preserved on registration, tool_filter.include and tool_filter.exclude can coexist, empty exclude list is handled gracefully, exclude list is correctly reconstructed from the domain model
Add/update Robot Framework integration test for agents skill add + agents skill show verifying tool_filter.exclude is stored and displayed
Run nox -e typecheck to verify all new fields are correctly typed (Pyright must pass with no suppressions)
Ensure all nox stages pass

Definition of Done

SkillMcpSource domain model contains an exclude field (type list[str], default empty list)
SkillService._schema_to_skill_dict() maps tool_filter.exclude from SkillMcpToolFilterSchema into the dict representation without dropping it
agents skill show displays the exclude tool filter list when it is non-empty
Behave BDD scenarios cover tool_filter.exclude preservation with ≥4 scenarios
Robot Framework integration test validates end-to-end skill registration and retrieval of tool_filter.exclude
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

## Metadata - **Branch**: `fix/skill-mcp-tool-filter-exclude-lost` - **Commit Message**: `fix(skills): preserve tool_filter.exclude blacklist in _schema_to_skill_dict() and SkillMcpSource domain model` - **Milestone**: v3.7.0 - **Parent Epic**: #393 ## Bug Report ### Feature Area Skills System — MCP server tool exposure (`SkillService._schema_to_skill_dict()` / `SkillMcpSource` domain model) ### What Was Tested The `SkillService._schema_to_skill_dict()` method in `src/cleveragents/application/services/skill_service.py` was analyzed for MCP server tool filter support, specifically the `tool_filter.exclude` blacklist field. ### Expected Behavior (from spec) The skill YAML schema (`docs/schema/skill.schema.yaml`) defines `tool_filter` for MCP servers with both `include` (whitelist) and `exclude` (blacklist) lists: ```yaml tool_filter: include: type: array description: "Whitelist of tool names to expose." exclude: type: array description: "Blacklist of tool names to hide." ``` The `SkillMcpToolFilterSchema` in `src/cleveragents/skills/schema.py` correctly captures both: ```python class SkillMcpToolFilterSchema(BaseModel): include: list[str] = Field(default_factory=list, ...) exclude: list[str] = Field(default_factory=list, ...) ``` When a skill is registered, both `include` and `exclude` filters should be preserved in the domain model so that the runtime can correctly restrict which MCP server tools are exposed to agents. ### Actual Behavior (from code analysis) In `src/cleveragents/application/services/skill_service.py`, `_schema_to_skill_dict()` only maps `tool_filter.include`: ```python mcp_dict: dict[str, Any] = {"server": mcp.name} if mcp.tool_filter and mcp.tool_filter.include: mcp_dict["tools"] = list(mcp.tool_filter.include) data["mcp_servers"].append(mcp_dict) ``` The `tool_filter.exclude` list is **silently dropped** — there is no mapping for it at all. Furthermore, the `SkillMcpSource` domain model in `src/cleveragents/domain/models/core/skill.py` has no `exclude` field, meaning the blacklist cannot be stored even if the service layer were fixed. This means users cannot use the blacklist approach to hide specific tools from an MCP server. Only the whitelist approach (`include`) works, and even then only partially (see related issue #2893 which covers the broader MCP connection details loss). ### Steps to Reproduce 1. Create a skill YAML with MCP tool exclusions: ```yaml name: local/safe-github description: "GitHub integration without destructive operations" mcp_servers: - name: github transport: stdio command: npx args: ["-y", "@modelcontextprotocol/server-github"] tool_filter: exclude: - delete_repository - delete_branch ``` 2. Register: `agents skill add --config safe-github.yaml` 3. Inspect: `agents skill show local/safe-github` 4. Observe that the `exclude` filter is lost — all tools including destructive ones would be exposed ### Code Locations - `src/cleveragents/application/services/skill_service.py` — `_schema_to_skill_dict()` method: only maps `tool_filter.include`, silently drops `tool_filter.exclude` - `src/cleveragents/domain/models/core/skill.py` — `SkillMcpSource` class: missing `exclude` field entirely - `src/cleveragents/skills/schema.py` — `SkillMcpToolFilterSchema` class: correctly captures `exclude` (source of truth) - `docs/schema/skill.schema.yaml` — schema definition showing `exclude` is a supported, spec-defined field ### Severity **High** — The `exclude` filter is a spec-defined safety feature for preventing exposure of dangerous MCP tools. Silent data loss means users cannot safely restrict MCP server tool access using the blacklist approach. This is a security-relevant gap: a user who configures `tool_filter.exclude` to hide destructive tools (e.g., `delete_repository`, `delete_branch`) will believe those tools are hidden, but they will actually be exposed to the agent at runtime. ## Subtasks - [ ] Add `exclude: list[str]` field to `SkillMcpSource` domain model in `src/cleveragents/domain/models/core/skill.py` (with `default_factory=list`, matching the schema) - [ ] Update `_schema_to_skill_dict()` in `src/cleveragents/application/services/skill_service.py` to map `tool_filter.exclude` into the `mcp_dict` (e.g., as `"exclude_tools"` or `"tool_exclude"` key, consistent with the `"tools"` key used for `include`) - [ ] Verify round-trip fidelity: schema → dict → domain model → dict preserves both `include` and `exclude` lists - [ ] Update `agents skill show` output to display the `exclude` filter when present - [ ] Add Behave BDD unit test scenarios covering: `tool_filter.exclude` is preserved on registration, `tool_filter.include` and `tool_filter.exclude` can coexist, empty `exclude` list is handled gracefully, `exclude` list is correctly reconstructed from the domain model - [ ] Add/update Robot Framework integration test for `agents skill add` + `agents skill show` verifying `tool_filter.exclude` is stored and displayed - [ ] Run `nox -e typecheck` to verify all new fields are correctly typed (Pyright must pass with no suppressions) - [ ] Ensure all nox stages pass ## Definition of Done - [ ] `SkillMcpSource` domain model contains an `exclude` field (type `list[str]`, default empty list) - [ ] `SkillService._schema_to_skill_dict()` maps `tool_filter.exclude` from `SkillMcpToolFilterSchema` into the dict representation without dropping it - [ ] `agents skill show` displays the `exclude` tool filter list when it is non-empty - [ ] Behave BDD scenarios cover `tool_filter.exclude` preservation with ≥4 scenarios - [ ] Robot Framework integration test validates end-to-end skill registration and retrieval of `tool_filter.exclude` - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester

freemo added this to the v3.7.0 milestone

2026-04-05 02:48:08 +00:00

freemo added a new dependency

2026-04-05 02:48:29 +00:00

#393 Epic: Skill & Tool Lifecycle

freemo commented

2026-04-05 02:52:31 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: High (confirmed)
MoSCoW: Should Have — tool_filter.exclude silently dropped

Valid UAT finding verified during batch triage.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: High (confirmed) - **MoSCoW**: Should Have — tool_filter.exclude silently dropped Valid UAT finding verified during batch triage. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-05 06:15:22 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo referenced this issue

2026-04-05 06:33:04 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo referenced this issue

2026-04-05 07:21:47 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094