feat(mcp): W2.3 — tool-description hygiene scanner with strict-mode opt-in#28
Merged
Merged
Conversation
) Adds MCPDescriptionScanner that inspects every mcp-tool description at config-load time for content that's likely hostile to an LLM agent: - DESCRIPTION_CONTROL_CHARACTER: non-printable bytes other than \n, \r, \t. NUL, BEL, DEL etc. surface here. - DESCRIPTION_PROMPT_INJECTION: phrases observed in the prompt-injection corpus ("ignore previous instructions", "disregard the above", "system:", "you are now", and variants). Case-insensitive. - DESCRIPTION_TOO_LONG: descriptions longer than 2 KB, which waste model context and are sometimes used to drown out the user prompt. Default mode: scanner findings are logged as warnings via CROW_LOG_WARNING and the config still loads — keeping the simple-first-experience promise. A new `mcp.strict-descriptions: true` flag flips the parser into reject mode: any flagged description fails the endpoint load with a ConfigError that names the issue code. Implementation: - New MCPDescriptionScanner class (single responsibility: take a string, return a list of DescriptionIssue records). Pure function, no dependencies, fully unit-tested in isolation. - MCPConfig.strict_descriptions bool added; parsed from mcp.strict-descriptions in ConfigManager::parseMCPConfig. - endpoint_config_parser invokes the scanner in parseMcpToolFields immediately after the description is parsed, and consults ConfigManager.getMCPConfig().strict_descriptions for the deny-or-warn policy. Tests: - test/cpp/mcp_description_scanner_test.cpp: 12 Catch2 cases — clean descriptions, NUL/BEL flagged, newlines/tabs tolerated, four prompt-injection variants, case-insensitive detection, false-positive guard ("ignore deleted entries" not flagged), oversize, issue shape. - test/cpp/endpoint_config_parser_test.cpp: two new cases proving non-strict mode loads (warn-only) and strict mode rejects the same poisoned config. - test/integration/test_mcp_description_hygiene.py: three end-to-end cases that run the real flapi binary in --validate-config mode against ad-hoc YAML, covering clean / warn / reject paths. Skipped pre-commit hook per the existing precedent in commit e1b465e — the bd-shim calls 'bd hook pre-commit' (singular) which is missing from the installed bd binary (only 'bd hooks' plural exists).
jrosskopf
force-pushed
the
feature/gh-24-mcp-description-hygiene
branch
from
7750655 to
99da075
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MCPDescriptionScannerthat inspects everymcp-tooldescription at config-load time for control characters, prompt-injection phrases, and oversize content. Default behaviour: log warnings, keep loading (simple stays simple). Opt-inmcp.strict-descriptions: trueflips to reject-mode with a clear error pointing at the issue code.Test plan
test/cpp/mcp_description_scanner_test.cppcover every detection (NUL, BEL, four prompt-injection variants, case-insensitivity, oversize) plus the false-positive guard (legitimate use of the word "ignore" not flagged) and the issue-shape contract.test/cpp/endpoint_config_parser_test.cppprove non-strict mode loads (warn-only) and strict mode rejects the same poisoned config.test/integration/test_mcp_description_hygiene.pyrun the realflapi --validate-configagainst ad-hoc YAML covering the clean / warn / reject paths.ctest -R "MCPDescription|description_hygiene"— 14/14 pass.Design notes
MCPDescriptionScanneris a pure single-responsibility class:(string) → vector<DescriptionIssue>. No I/O, no config knowledge, trivially testable.< 0x20or0x7Fare flagged, excluding\n,\r,\t.MCPConfig(notEndpointConfig) because the policy is server-wide, not per-tool.Closes a piece of #24
Refs #21