fix: 禁用 stdio 传输以防止命令注入风险

This commit is contained in:
wizardchen
2026-01-15 14:49:19 +08:00
parent 48a0f9b508
commit 57d6fea8bc
4 changed files with 29 additions and 368 deletions
@@ -29,100 +29,19 @@
<t-radio-group v-model="formData.transport_type">
<t-radio value="sse">{{ t('mcpServiceDialog.transport.sse') }}</t-radio>
<t-radio value="http-streamable">{{ t('mcpServiceDialog.transport.httpStreamable') }}</t-radio>
<t-radio value="stdio">{{ t('mcpServiceDialog.transport.stdio') }}</t-radio>
<!-- Stdio transport is disabled for security reasons -->
</t-radio-group>
</t-form-item>
<!-- URL for SSE/HTTP Streamable -->
<t-form-item
v-if="formData.transport_type !== 'stdio'"
:label="t('mcpServiceDialog.serviceUrl')"
name="url"
>
<t-input v-model="formData.url" :placeholder="t('mcpServiceDialog.serviceUrlPlaceholder')" />
</t-form-item>
<!-- Stdio Config -->
<template v-if="formData.transport_type === 'stdio'">
<t-form-item :label="t('mcpServiceDialog.command')" name="stdio_config.command">
<t-radio-group v-model="formData.stdio_config.command">
<t-radio value="uvx">uvx</t-radio>
<t-radio value="npx">npx</t-radio>
</t-radio-group>
</t-form-item>
<t-form-item :label="t('mcpServiceDialog.args')" name="stdio_config.args">
<div class="args-input-container">
<div
v-for="(arg, index) in formData.stdio_config.args"
:key="index"
class="arg-item"
>
<t-input
v-model="formData.stdio_config.args[index]"
:placeholder="t('mcpServiceDialog.argPlaceholder', { index: index + 1 })"
class="arg-input"
/>
<t-button
variant="text"
theme="danger"
@click="removeArg(index)"
:disabled="formData.stdio_config.args.length === 1"
>
<template #icon><t-icon name="delete" /></template>
</t-button>
</div>
<t-button
variant="outline"
size="small"
@click="addArg"
class="add-arg-btn"
>
<template #icon><t-icon name="add" /></template>
{{ t('mcpServiceDialog.addArg') }}
</t-button>
</div>
</t-form-item>
<t-form-item :label="t('mcpServiceDialog.envVars')">
<div class="env-vars-container">
<div
v-for="(value, key, index) in formData.env_vars"
:key="index"
class="env-var-item"
>
<t-input
v-model="envVarKeys[index]"
:placeholder="t('mcpServiceDialog.envKeyPlaceholder')"
class="env-key-input"
@blur="updateEnvVarKey(index, envVarKeys[index])"
/>
<t-input
v-model="formData.env_vars[key]"
:placeholder="t('mcpServiceDialog.envValuePlaceholder')"
type="password"
class="env-value-input"
/>
<t-button
variant="text"
theme="danger"
@click="removeEnvVar(key)"
>
<template #icon><t-icon name="delete" /></template>
</t-button>
</div>
<t-button
variant="outline"
size="small"
@click="addEnvVar"
class="add-env-var-btn"
>
<template #icon><t-icon name="add" /></template>
{{ t('mcpServiceDialog.addEnvVar') }}
</t-button>
</div>
</t-form-item>
</template>
<!-- Stdio Config removed for security reasons -->
<t-form-item :label="t('mcpServiceDialog.enableService')" name="enabled">
<t-switch v-model="formData.enabled" />
@@ -212,13 +131,8 @@ const formData = ref({
name: '',
description: '',
enabled: true,
transport_type: 'sse' as 'sse' | 'http-streamable' | 'stdio',
transport_type: 'sse' as 'sse' | 'http-streamable',
url: '',
stdio_config: {
command: 'uvx' as 'uvx' | 'npx',
args: ['']
},
env_vars: {} as Record<string, string>,
auth_config: {
api_key: '',
token: ''
@@ -230,52 +144,22 @@ const formData = ref({
}
})
// Track env var keys separately for easier editing
const envVarKeys = ref<string[]>([])
const rules: Record<string, FormRule[]> = {
name: [{ required: true, message: t('mcpServiceDialog.rules.nameRequired') as string, type: 'error' }],
transport_type: [{ required: true, message: t('mcpServiceDialog.rules.transportRequired') as string, type: 'error' }],
url: [
{
validator: (val: string) => {
if (formData.value.transport_type !== 'stdio') {
if (!val || val.trim() === '') {
return { result: false, message: t('mcpServiceDialog.rules.urlRequired') as string, type: 'error' }
}
// Basic URL validation
try {
new URL(val)
return { result: true, message: '', type: 'success' }
} catch {
return { result: false, message: t('mcpServiceDialog.rules.urlInvalid') as string, type: 'error' }
}
if (!val || val.trim() === '') {
return { result: false, message: t('mcpServiceDialog.rules.urlRequired') as string, type: 'error' }
}
return { result: true, message: '', type: 'success' }
}
}
],
'stdio_config.command': [
{
validator: (val: string) => {
if (formData.value.transport_type === 'stdio') {
if (!val || (val !== 'uvx' && val !== 'npx')) {
return { result: false, message: t('mcpServiceDialog.rules.commandRequired') as string, type: 'error' }
}
// Basic URL validation
try {
new URL(val)
return { result: true, message: '', type: 'success' }
} catch {
return { result: false, message: t('mcpServiceDialog.rules.urlInvalid') as string, type: 'error' }
}
return { result: true, message: '', type: 'success' }
}
}
],
'stdio_config.args': [
{
validator: (val: string[]) => {
if (formData.value.transport_type === 'stdio') {
if (!val || val.length === 0 || val.every(arg => !arg || arg.trim() === '')) {
return { result: false, message: t('mcpServiceDialog.rules.argsRequired') as string, type: 'error' }
}
}
return { result: true, message: '', type: 'success' }
}
}
]
@@ -294,11 +178,6 @@ const resetForm = () => {
enabled: true,
transport_type: 'sse',
url: '',
stdio_config: {
command: 'uvx',
args: ['']
},
env_vars: {},
auth_config: {
api_key: '',
token: ''
@@ -309,90 +188,22 @@ const resetForm = () => {
retry_delay: 1
}
}
envVarKeys.value = []
formRef.value?.clearValidate()
}
// Watch transport_type to reset related fields
watch(
() => formData.value.transport_type,
(newType) => {
if (newType === 'stdio') {
formData.value.url = ''
if (!formData.value.stdio_config || formData.value.stdio_config.args.length === 0) {
formData.value.stdio_config = {
command: 'uvx',
args: ['']
}
}
} else {
formData.value.stdio_config = {
command: 'uvx',
args: ['']
}
formData.value.env_vars = {}
envVarKeys.value = []
}
formRef.value?.clearValidate()
}
)
// Args management
const addArg = () => {
formData.value.stdio_config.args.push('')
}
const removeArg = (index: number) => {
if (formData.value.stdio_config.args.length > 1) {
formData.value.stdio_config.args.splice(index, 1)
}
}
// Env vars management
const addEnvVar = () => {
const key = `VAR_${Date.now()}`
formData.value.env_vars[key] = ''
envVarKeys.value.push(key)
}
const removeEnvVar = (key: string) => {
delete formData.value.env_vars[key]
const index = envVarKeys.value.indexOf(key)
if (index > -1) {
envVarKeys.value.splice(index, 1)
}
}
const updateEnvVarKey = (index: number, newKey: string) => {
const oldKey = envVarKeys.value[index]
if (oldKey && oldKey !== newKey && formData.value.env_vars[oldKey] !== undefined) {
const value = formData.value.env_vars[oldKey]
delete formData.value.env_vars[oldKey]
if (newKey && newKey.trim() !== '') {
formData.value.env_vars[newKey] = value
envVarKeys.value[index] = newKey
} else {
envVarKeys.value.splice(index, 1)
}
}
}
// Watch service prop to initialize form
watch(
() => props.service,
(service) => {
if (service) {
// Note: stdio transport_type will fall back to 'sse' as stdio is disabled
const transportType = service.transport_type === 'stdio' ? 'sse' : (service.transport_type || 'sse')
formData.value = {
name: service.name || '',
description: service.description || '',
enabled: service.enabled ?? true,
transport_type: service.transport_type || 'sse',
transport_type: transportType as 'sse' | 'http-streamable',
url: service.url || '',
stdio_config: service.stdio_config || {
command: 'uvx',
args: ['']
},
env_vars: service.env_vars || {},
auth_config: {
api_key: service.auth_config?.api_key || '',
token: service.auth_config?.token || ''
@@ -403,8 +214,6 @@ watch(
retry_delay: service.advanced_config?.retry_delay || 1
}
}
// Initialize env var keys
envVarKeys.value = Object.keys(formData.value.env_vars)
} else {
resetForm()
}
@@ -428,32 +237,8 @@ const handleSubmit = async () => {
api_key: formData.value.auth_config.api_key || undefined,
token: formData.value.auth_config.token || undefined
},
advanced_config: formData.value.advanced_config
}
// Add URL or stdio_config based on transport type
if (formData.value.transport_type === 'stdio') {
// Filter out empty args
const args = formData.value.stdio_config.args.filter(arg => arg && arg.trim() !== '')
data.stdio_config = {
command: formData.value.stdio_config.command,
args
}
// Build env vars using envVarKeys to get the correct key names
// This fixes the issue where user-entered keys weren't being saved
const envVars: Record<string, string> = {}
const formEnvVarsEntries = Object.entries(formData.value.env_vars)
for (let i = 0; i < formEnvVarsEntries.length; i++) {
const [, value] = formEnvVarsEntries[i]
// Use the key from envVarKeys (which reflects user input) instead of formData key
const actualKey = envVarKeys.value[i]
if (actualKey && actualKey.trim() !== '' && value && value.trim() !== '') {
envVars[actualKey.trim()] = value.trim()
}
}
data.env_vars = Object.keys(envVars).length > 0 ? envVars : undefined
} else {
data.url = formData.value.url || undefined
advanced_config: formData.value.advanced_config,
url: formData.value.url || undefined
}
if (props.mode === 'add') {
@@ -482,48 +267,6 @@ const handleClose = () => {
</script>
<style scoped lang="less">
.args-input-container {
display: flex;
flex-direction: column;
gap: 8px;
.arg-item {
display: flex;
gap: 8px;
align-items: center;
.arg-input {
flex: 1;
}
}
.add-arg-btn {
align-self: flex-start;
}
}
.env-vars-container {
display: flex;
flex-direction: column;
gap: 8px;
.env-var-item {
display: flex;
gap: 8px;
align-items: center;
.env-key-input {
width: 150px;
}
.env-value-input {
flex: 1;
}
}
.add-env-var-btn {
align-self: flex-start;
}
}
/* Stdio-related styles removed as stdio transport is disabled for security reasons */
</style>
+6 -33
View File
@@ -31,20 +31,9 @@ func NewMCPServiceService(
// CreateMCPService creates a new MCP service
func (s *mcpServiceService) CreateMCPService(ctx context.Context, service *types.MCPService) error {
// Security validation for stdio transport type
// Stdio transport is disabled for security reasons
if service.TransportType == types.MCPTransportStdio {
if service.StdioConfig == nil {
return fmt.Errorf("stdio_config is required for stdio transport")
}
// Validate stdio configuration to prevent command injection (CWE-78)
if err := secutils.ValidateStdioConfig(
service.StdioConfig.Command,
service.StdioConfig.Args,
service.EnvVars,
); err != nil {
logger.GetLogger(ctx).Warnf("MCP service creation blocked due to security validation: %v", err)
return fmt.Errorf("security validation failed: %w", err)
}
return fmt.Errorf("stdio transport is disabled for security reasons; please use SSE or HTTP Streamable transport instead")
}
// Set default advanced config if not provided
@@ -129,31 +118,15 @@ func (s *mcpServiceService) UpdateMCPService(ctx context.Context, service *types
return fmt.Errorf("MCP service not found")
}
// Security validation for stdio transport type when updating stdio config
// Determine the final transport type and stdio config after merge
// Determine the final transport type after merge
finalTransportType := existing.TransportType
if service.TransportType != "" {
finalTransportType = service.TransportType
}
finalStdioConfig := existing.StdioConfig
if service.StdioConfig != nil {
finalStdioConfig = service.StdioConfig
}
finalEnvVars := existing.EnvVars
if service.EnvVars != nil {
finalEnvVars = service.EnvVars
}
// Validate if the final configuration uses stdio transport
if finalTransportType == types.MCPTransportStdio && finalStdioConfig != nil {
if err := secutils.ValidateStdioConfig(
finalStdioConfig.Command,
finalStdioConfig.Args,
finalEnvVars,
); err != nil {
logger.GetLogger(ctx).Warnf("MCP service update blocked due to security validation: %v", err)
return fmt.Errorf("security validation failed: %w", err)
}
// Stdio transport is disabled for security reasons
if finalTransportType == types.MCPTransportStdio {
return fmt.Errorf("stdio transport is disabled for security reasons; please use SSE or HTTP Streamable transport instead")
}
// Store old enabled state BEFORE any updates
+2 -31
View File
@@ -9,7 +9,6 @@ import (
"github.com/Tencent/WeKnora/internal/logger"
"github.com/Tencent/WeKnora/internal/types"
secutils "github.com/Tencent/WeKnora/internal/utils"
"github.com/mark3labs/mcp-go/client"
"github.com/mark3labs/mcp-go/client/transport"
"github.com/mark3labs/mcp-go/mcp"
@@ -119,36 +118,8 @@ func NewMCPClient(config *ClientConfig) (MCPClient, error) {
return nil, fmt.Errorf("failed to create HTTP streamable client: %w", err)
}
case types.MCPTransportStdio:
if config.Service.StdioConfig == nil {
return nil, fmt.Errorf("stdio_config is required for stdio transport")
}
// Security validation: validate command, args, and env vars before execution
// This prevents command injection attacks (CWE-78)
if err := secutils.ValidateStdioConfig(
config.Service.StdioConfig.Command,
config.Service.StdioConfig.Args,
config.Service.EnvVars,
); err != nil {
return nil, fmt.Errorf("stdio configuration validation failed: %w", err)
}
// Convert env vars map to []string format (KEY=value)
envVars := make([]string, 0, len(config.Service.EnvVars))
for key, value := range config.Service.EnvVars {
envVars = append(envVars, fmt.Sprintf("%s=%s", key, value))
}
// Create stdio client with options
// NewStdioMCPClientWithOptions(command string, env []string, args []string, opts ...transport.StdioOption)
mcpClient, err = client.NewStdioMCPClientWithOptions(
config.Service.StdioConfig.Command,
envVars,
config.Service.StdioConfig.Args,
)
if err != nil {
return nil, fmt.Errorf("failed to create stdio client: %w", err)
}
// Stdio transport is disabled for security reasons (potential command injection vulnerabilities)
return nil, fmt.Errorf("stdio transport is disabled for security reasons; please use SSE or HTTP Streamable transport instead")
default:
return nil, ErrUnsupportedTransport
}
+4 -30
View File
@@ -35,17 +35,17 @@ func NewMCPManager() *MCPManager {
}
// GetOrCreateClient gets an existing client or creates a new one
// For stdio transport, always creates a new client (not cached)
// For SSE/HTTP Streamable, caches and reuses existing connections
// Caches and reuses existing connections for SSE/HTTP Streamable
// Note: Stdio transport is disabled for security reasons
func (m *MCPManager) GetOrCreateClient(service *types.MCPService) (MCPClient, error) {
// Check if service is enabled
if !service.Enabled {
return nil, fmt.Errorf("MCP service %s is not enabled", service.Name)
}
// For stdio transport, always create a new client (don't cache)
// Stdio transport is disabled for security reasons
if service.TransportType == types.MCPTransportStdio {
return m.createStdioClient(service)
return nil, fmt.Errorf("stdio transport is disabled for security reasons; please use SSE or HTTP Streamable transport instead")
}
// For SSE/HTTP Streamable, check if client already exists and reuse
@@ -95,32 +95,6 @@ func (m *MCPManager) GetOrCreateClient(service *types.MCPService) (MCPClient, er
return client, nil
}
// createStdioClient creates a new stdio client (not cached)
func (m *MCPManager) createStdioClient(service *types.MCPService) (MCPClient, error) {
// Create new client
config := &ClientConfig{
Service: service,
}
client, err := NewMCPClient(config)
if err != nil {
return nil, fmt.Errorf("failed to create stdio MCP client: %w", err)
}
// For stdio, Connect() starts the subprocess
// Use manager's context for the connection lifecycle
if err := client.Connect(m.ctx); err != nil {
return nil, fmt.Errorf("failed to connect to stdio MCP service: %w", err)
}
if err := m.initializeClient(service, client, "failed to initialize stdio MCP client"); err != nil {
return nil, err
}
logger.GetLogger(m.ctx).Infof("MCP stdio client created and initialized for service: %s", service.Name)
return client, nil
}
// initializeClient handles the shared initialization flow with timeout enforcement.
func (m *MCPManager) initializeClient(service *types.MCPService, client MCPClient, errPrefix string) error {
initTimeout := 30 * time.Second