fix: validate transport type in custom MCP server config to prevent command injection

[jackieya]

fix: validate transport type in custom MCP server config

Summary

This PR adds a transport type whitelist check in the else branch of BaseMcpNode.execute() to prevent command injection via stdio transport in custom MCP server configurations.

Related: GHSA-38q2-4mm7-qf5h, GHSA-pw52-326g-r5xj

Problem

The fix in commit c8e8afa for GHSA-38q2-4mm7-qf5h only restricts the referencing code path. The else branch — which loads mcp_servers directly from user-supplied JSON — remains unrestricted. An attacker can set mcp_source to any non-referencing value (or omit it entirely, since the field is optional) to bypass the fix and inject a stdio transport configuration with arbitrary command and args, achieving Remote Code Execution.

Changes

File: apps/application/flow/step_node/mcp_node/impl/base_mcp_node.py

Added transport type validation after handle_variables() to ensure all server configs only use sse or streamable_http transport. Placing the check after variable substitution prevents bypass via variable references.

         else:
             servers = json.loads(mcp_servers)
             servers = self.handle_variables(servers)
+            for _name, _cfg in servers.items():
+                if isinstance(_cfg, dict) and _cfg.get('transport') not in ('sse', 'streamable_http'):
+                    raise ValueError(
+                        f"Unsupported transport type: {_cfg.get('transport')}. "
+                        "Only 'sse' and 'streamable_http' are allowed."
+                    )
             params = json.loads(json.dumps(tool_params))
             params = self.handle_variables(params)

Why after handle_variables()?

If the check is placed before handle_variables(), an attacker can use a variable reference (e.g., ["global", "evil_var"]) as the server config value. This bypasses the isinstance(cfg, dict) check, and then handle_variables() resolves it into a dict containing "transport": "stdio" — effectively bypassing the patch.

Testing

• With patch applied: PoC returns Unsupported transport type: stdio ✅
• Without patch: PoC returns ['root\n'] (RCE confirmed)
• Normal sse/streamable_http MCP workflows: unaffected ✅

fix: validate transport type in custom MCP server config to prevent command injection via stdio

[Copilot]

Pull request overview

This PR tightens security for the workflow MCP node by enforcing an allow-list of safe MCP transport types when MCP server configs are provided directly via user-supplied JSON (non-referencing path), preventing command injection via stdio transport configs.

Changes:

• Adds a transport allow-list validation (sse, streamable_http) after handle_variables() in the custom MCP config code path.
• Raises a clear error when an unsupported transport type is found in the resolved server config.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[liqiang-fit2cloud]

Please use ToolExecutor().validate_mcp_transport() instead like this.

[jackieya]

Please use ToolExecutor().validate_mcp_transport() instead like this.

Thanks for the suggestion! I've updated the PR to use ToolExecutor().validate_mcp_transport() across all three affected files. The commit has been squashed into a single clean commit.