Skip to content

feat(mcp-rules): add 4 new MCP security detection rules#456

Open
NY1024 wants to merge 2 commits into
Tencent:mainfrom
NY1024:add-mcp-security-rules
Open

feat(mcp-rules): add 4 new MCP security detection rules#456
NY1024 wants to merge 2 commits into
Tencent:mainfrom
NY1024:add-mcp-security-rules

Conversation

@NY1024

@NY1024 NY1024 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Add the following MCP server security detection rules to expand the MCP scanning coverage from 4 to 8 rules:

  • mcp_ssrf: Detect SSRF via unvalidated URL/hostname arguments reaching network clients without private-address-range checks
  • mcp_path_traversal: Detect path traversal via unsanitized file path arguments reaching filesystem APIs without canonicalization or directory-boundary checks
  • mcp_insecure_deserialization: Detect RCE via caller-supplied data deserialized with unsafe serializers (pickle, marshal, yaml.load, unserialize, ObjectInputStream)
  • mcp_hardcoded_secrets: Detect hardcoded API keys, tokens, private keys, and passwords embedded in MCP server source code

Each rule follows the existing format with detection criteria, strict judgment standards, and output requirements.

Add the following MCP server security detection rules to expand
the MCP scanning coverage from 4 to 8 rules:

- mcp_ssrf: Detect SSRF via unvalidated URL/hostname arguments
  reaching network clients without private-address-range checks
- mcp_path_traversal: Detect path traversal via unsanitized file
  path arguments reaching filesystem APIs without canonicalization
  or directory-boundary checks
- mcp_insecure_deserialization: Detect RCE via caller-supplied data
  deserialized with unsafe serializers (pickle, marshal, yaml.load,
  unserialize, ObjectInputStream)
- mcp_hardcoded_secrets: Detect hardcoded API keys, tokens, private
  keys, and passwords embedded in MCP server source code

Each rule follows the existing ATR (Agent Threat Rules) format with
detection criteria, strict judgment standards, and output requirements.
@boy-hack

boy-hack commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Code Review: feat(mcp-rules): add 4 new MCP security detection rules

Overall quality is good — well-structured prompt templates with clear vulnerability definitions and detection criteria.

✅ Strengths

  • All 4 rules follow the existing MCP rule schema (info.id, info.categories, prompt_template, response_format)
  • Detection criteria are specific and concrete (e.g., SSRF checks for private address range validation, path traversal checks for canonicalization)
  • mcp_hardcoded_secrets covers common credential patterns (AWS AKIA…, sk-…, ghp_…, PEM keys)
  • mcp_insecure_deserialization correctly targets pickle.loads, yaml.load(unsafe), eval() patterns

⚠️ Minor Issues

  1. mcp_path_traversal.yaml — the info.categories field uses ["code"] while the three other rules also use ["code"]. Consistent — no issue.
  2. mcp_insecure_deserialization.yaml — consider adding jsonpickle to the deserialization patterns, as it is commonly used in Python MCP servers and carries the same risk.
  3. Suggest adding info.author to all 4 files for attribution consistency with existing MCP rules like mcp_tool_poisoning.yaml.

🟢 Verdict

LGTM with minor suggestions. These 4 rules meaningfully expand MCP scanning coverage from 3 → 7 rules.
Waiting for @pythoncheng or @boy-hack to merge.

@NY1024

NY1024 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Addressed the minor suggestions:

  1. jsonpickle pattern added — included jsonpickle.decode(args.data) in the Python unsafe deserializers section of mcp_insecure_deserialization.yaml, noting that {"py/object": "..."} directives carry the same RCE risk as pickle. This covers a commonly used serializer in Python MCP servers.

  2. info.author field — all 4 files already include author: "ATR (Agent Threat Rules)", consistent with existing MCP rules like mcp_tool_poisoning.yaml.

Ready for merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants