Skip to content

Commit 420dbe8

Browse files
committed
docs: Enhance MCP documentation and validation for network permissions
Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>
1 parent 09a1c09 commit 420dbe8

4 files changed

Lines changed: 106 additions & 5 deletions

File tree

docs/mcps.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,22 @@ tools:
212212

213213
Enforcement in compiled workflows:
214214

215-
- A Squid proxy is generated and pinned to a dedicated Docker network for each proxy‑enabled MCP server.
215+
- A [Squid proxy](https://www.squid-cache.org/) is generated and pinned to a dedicated Docker network for each proxy‑enabled MCP server.
216216
- The MCP container is configured with `HTTP_PROXY`/`HTTPS_PROXY` to point at Squid; iptables rules only allow egress to the proxy.
217217
- The proxy is seeded with an `allowed_domains.txt` built from your `allowed` list; requests to other domains are blocked.
218218

219219
Notes:
220220

221-
- Applies to stdio MCP servers that specify a `container`. Non‑container stdio and remote `type: http` servers do not use this control (at the moment)
221+
- **Only applies to stdio MCP servers with `container`** - Non‑container stdio and `type: http` servers will cause compilation errors
222222
- Use bare domains without scheme; list each domain you intend to permit.
223223

224+
### Validation Rules
225+
226+
The compiler enforces these network permission rules:
227+
228+
- ❌ **HTTP servers**: `network egress permissions do not apply to remote 'type: http' servers`
229+
- ❌ **Non-container stdio**: `network egress permissions only apply to stdio MCP servers that specify a 'container'`
230+
- ✅ **Container stdio**: Network permissions work correctly
224231

225232
## Debugging and Troubleshooting
226233

docs/security-notes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ Enforcement details:
190190

191191
- Compiler generates a per‑tool Squid proxy and Docker network; MCP egress is forced through the proxy via iptables.
192192
- Only listed domains are reachable; all others are denied at the network layer.
193-
- Applies to `mcp.container` stdio servers. Non‑container stdio and `type: http` servers are not governed by this control.
193+
- Applies to `mcp.container` stdio servers. Non‑container stdio and `type: http` servers are not supported and will cause compilation errors.
194194

195195
Operational guidance:
196196

pkg/workflow/mcp-config.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func ValidateMCPConfigs(tools map[string]any) error {
371371
}
372372

373373
// Validate MCP configuration requirements (before transformation)
374-
if err := validateMCPRequirements(toolName, mcpConfig); err != nil {
374+
if err := validateMCPRequirements(toolName, mcpConfig, config); err != nil {
375375
return err
376376
}
377377
}
@@ -471,7 +471,7 @@ func hasNetworkPermissions(toolConfig map[string]any) (bool, []string) {
471471
}
472472

473473
// validateMCPRequirements validates the specific requirements for MCP configuration
474-
func validateMCPRequirements(toolName string, mcpConfig map[string]any) error {
474+
func validateMCPRequirements(toolName string, mcpConfig map[string]any, toolConfig map[string]any) error {
475475
// Validate 'type' property
476476
mcpType, hasType := mcpConfig["type"]
477477
if err := validateStringProperty(toolName, "type", mcpType, hasType); err != nil {
@@ -489,6 +489,25 @@ func validateMCPRequirements(toolName string, mcpConfig map[string]any) error {
489489
return fmt.Errorf("tool '%s' mcp configuration 'type' value must be one of: stdio, http", toolName)
490490
}
491491

492+
// Validate network permissions usage first
493+
hasNetPerms, _ := hasNetworkPermissions(toolConfig)
494+
if !hasNetPerms {
495+
// Also check if permissions are nested in the mcp config itself
496+
hasNetPerms, _ = hasNetworkPermissions(map[string]any{"mcp": mcpConfig})
497+
}
498+
if hasNetPerms {
499+
switch typeStr {
500+
case "http":
501+
return fmt.Errorf("tool '%s' has network permissions configured, but network egress permissions do not apply to remote 'type: http' servers", toolName)
502+
case "stdio":
503+
// Network permissions only apply to stdio servers with container
504+
_, hasContainer := mcpConfig["container"]
505+
if !hasContainer {
506+
return fmt.Errorf("tool '%s' has network permissions configured, but network egress permissions only apply to stdio MCP servers that specify a 'container'", toolName)
507+
}
508+
}
509+
}
510+
492511
// Validate type-specific requirements
493512
switch typeStr {
494513
case "http":

pkg/workflow/mcp_json_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,81 @@ func TestValidateMCPConfigs(t *testing.T) {
320320
wantErr: true,
321321
errMsg: "missing property 'url'",
322322
},
323+
{
324+
name: "network permissions with HTTP type should fail",
325+
tools: map[string]any{
326+
"httpWithNetPerms": map[string]any{
327+
"mcp": map[string]any{
328+
"type": "http",
329+
"url": "https://example.com",
330+
},
331+
"permissions": map[string]any{
332+
"network": map[string]any{
333+
"allowed": []any{"example.com"},
334+
},
335+
},
336+
"allowed": []any{"tool1"},
337+
},
338+
},
339+
wantErr: true,
340+
errMsg: "network egress permissions do not apply to remote 'type: http' servers",
341+
},
342+
{
343+
name: "network permissions with stdio non-container should fail",
344+
tools: map[string]any{
345+
"stdioNonContainerWithNetPerms": map[string]any{
346+
"mcp": map[string]any{
347+
"type": "stdio",
348+
"command": "python",
349+
},
350+
"permissions": map[string]any{
351+
"network": map[string]any{
352+
"allowed": []any{"example.com"},
353+
},
354+
},
355+
"allowed": []any{"tool1"},
356+
},
357+
},
358+
wantErr: true,
359+
errMsg: "network egress permissions only apply to stdio MCP servers that specify a 'container'",
360+
},
361+
{
362+
name: "network permissions with stdio container should pass",
363+
tools: map[string]any{
364+
"stdioContainerWithNetPerms": map[string]any{
365+
"mcp": map[string]any{
366+
"type": "stdio",
367+
"container": "mcp/fetch",
368+
},
369+
"permissions": map[string]any{
370+
"network": map[string]any{
371+
"allowed": []any{"example.com"},
372+
},
373+
},
374+
"allowed": []any{"tool1"},
375+
},
376+
},
377+
wantErr: false,
378+
},
379+
{
380+
name: "network permissions in mcp section with HTTP type should fail",
381+
tools: map[string]any{
382+
"httpWithMcpNetPerms": map[string]any{
383+
"mcp": map[string]any{
384+
"type": "http",
385+
"url": "https://example.com",
386+
"permissions": map[string]any{
387+
"network": map[string]any{
388+
"allowed": []any{"example.com"},
389+
},
390+
},
391+
},
392+
"allowed": []any{"tool1"},
393+
},
394+
},
395+
wantErr: true,
396+
errMsg: "network egress permissions do not apply to remote 'type: http' servers",
397+
},
323398
}
324399

325400
for _, tt := range tests {

0 commit comments

Comments
 (0)