Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions chart/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ spec:
- "--token-auth-only"
- "true"
{{- end }}
{{- if .Values.security.allowedHosts }}
- "--allowed-host"
- {{ join "," .Values.security.allowedHosts | quote }}
{{- end }}
{{- if .Values.security.trustedOrigins }}
- "--trusted-origin"
- {{ join "," .Values.security.trustedOrigins | quote }}
{{- end }}
env:
{{- if .Values.azure.subscriptionId }}
- name: AZURE_SUBSCRIPTION_ID
Expand Down
24 changes: 24 additions & 0 deletions chart/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,30 @@ oauth:
# Requires azure.clientSecret (or azure.existingSecret with a client-secret key) to be set.
oboEnabled: false

# HTTP transport security: DNS-rebinding / cross-origin protection.
# Applies to streamable-http and sse transports only; stdio is unaffected.
#
# The chart deployment hard-codes --host 0.0.0.0 so the listener is always
# routable inside the cluster. When app.transport is streamable-http or sse
# and oauth.enabled is false, the server REFUSES to start unless
# security.allowedHosts is set. Pick one of:
# (a) enable OAuth (oauth.enabled: true), or
# (b) declare an explicit trusted-host allowlist below.
# A default install with neither set will CrashLoopBackOff on purpose.
security:
# HTTP Host header values to accept on /mcp, /sse and /message.
# Entries may include a port (e.g. "aks-mcp.example.com:8000") to match
# only that port, or omit it to match any port for that hostname.
# Use the literal "*" only when an upstream reverse proxy validates Host
# on the operator's behalf.
# Example: ["aks-mcp.internal.example.com"]
allowedHosts: []
# HTTP Origin header values to accept on browser-style cross-origin requests
# to /mcp, /sse and /message. Empty Origin (non-browser clients) always allowed.
# Use the literal "*" only to disable Origin enforcement entirely.
# Example: ["https://chat.internal.example.com"]
trustedOrigins: []

# Application configuration
config:
# Enabled components (empty means all components enabled)
Expand Down
13 changes: 11 additions & 2 deletions cmd/aks-mcp/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,21 @@ func TestContainerNetworkTransport(t *testing.T) {
t.Skip("Docker image 'aks-mcp:test' not available, skipping network transport test")
}

// Start container with default CMD (streamable-http transport)
// Start container with streamable-http transport. After the DNS-rebinding
// fail-closed validator landed, the default CMD (--host 0.0.0.0 with no
// auth and no --allowed-host) is intentionally rejected at startup, so
// this test supplies --allowed-host=localhost to satisfy the validator
// while still exercising the HTTP listener on localhost.
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

// Start container in background
cmd := exec.CommandContext(ctx, "docker", "run", "--rm", "-p", "8000:8000", "aks-mcp:test")
cmd := exec.CommandContext(ctx, "docker", "run", "--rm", "-p", "8000:8000",
"aks-mcp:test",
"--transport", "streamable-http",
"--host", "0.0.0.0",
"--allowed-host", "localhost",
)

// Start the container
err := cmd.Start()
Expand Down
11 changes: 9 additions & 2 deletions internal/components/azapi/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,22 @@ func AzApiHandler(azClient azcli.Client, cfg *config.ConfigData) func(ctx contex
return mcp.NewToolResultError(errMsg), nil
}

if result.Error != "" {
errMsg := fmt.Sprintf("Azure CLI command failed: %s", result.Error)
if result.ExitCode != 0 {
errMsg := fmt.Sprintf("Azure CLI command failed with exit code %d", result.ExitCode)
if result.Error != "" {
errMsg = fmt.Sprintf("%s: %s", errMsg, result.Error)
}
logger.Errorf("AzApiHandler: %s", errMsg)
if cfg.TelemetryService != nil {
cfg.TelemetryService.TrackToolInvocation(ctx, req.Params.Name, sanitizeCliCommand(cliCommand), false)
}
return mcp.NewToolResultError(errMsg), nil
}

if result.Error != "" {
logger.Warnf("AzApiHandler: Azure CLI emitted stderr with successful exit code: %s", result.Error)
}

logger.Debugf("AzApiHandler: Command completed successfully")

if cfg.TelemetryService != nil {
Expand Down
75 changes: 65 additions & 10 deletions internal/components/azapi/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ func (m *mockAzClient) ExecuteCommand(ctx context.Context, command string) (*azc
return m.executeFunc(ctx, command)
}
return &azcli.Result{
Output: json.RawMessage("mock output"),
Error: "",
Output: json.RawMessage("mock output"),
ExitCode: 0,
Error: "",
}, nil
}

Expand All @@ -51,8 +52,9 @@ func TestAzApiHandler_Success(t *testing.T) {
t.Errorf("expected command 'az group list', got '%s'", command)
}
return &azcli.Result{
Output: json.RawMessage(`[{"name":"rg1","location":"eastus"}]`),
Error: "",
Output: json.RawMessage(`[{"name":"rg1","location":"eastus"}]`),
ExitCode: 0,
Error: "",
}, nil
},
}
Expand Down Expand Up @@ -185,8 +187,9 @@ func TestAzApiHandler_CommandError(t *testing.T) {
mockClient := &mockAzClient{
executeFunc: func(ctx context.Context, command string) (*azcli.Result, error) {
return &azcli.Result{
Output: json.RawMessage(""),
Error: "command error: resource not found",
Output: json.RawMessage(""),
ExitCode: 2,
Error: "command error: resource not found",
}, nil
},
}
Expand All @@ -213,6 +216,56 @@ func TestAzApiHandler_CommandError(t *testing.T) {
if !result.IsError {
t.Fatal("expected error result")
}

textContent, ok := result.Content[0].(mcp.TextContent)
if !ok {
t.Fatalf("expected TextContent, got %T", result.Content[0])
}

if !strings.Contains(textContent.Text, "exit code 2") {
t.Errorf("expected exit code in error message, got %s", textContent.Text)
}
}

func TestAzApiHandler_WarningOnlyResult(t *testing.T) {
mockClient := &mockAzClient{
executeFunc: func(ctx context.Context, command string) (*azcli.Result, error) {
return &azcli.Result{
Output: json.RawMessage(`["aks-a","aks-b"]`),
ExitCode: 0,
Error: "WARNING: The behavior of this command has been altered by the following extension: aks-preview",
}, nil
},
}

cfg := newTestConfig(30)
handler := AzApiHandler(mockClient, cfg)

req := mcp.CallToolRequest{
Params: mcp.CallToolParams{
Name: "call_az",
Arguments: map[string]interface{}{
"cli_command": "az aks list --query [].name -o tsv",
},
},
}

result, err := handler(context.Background(), req)
if err != nil {
t.Fatalf("expected no error, got %v", err)
}
if result.IsError {
t.Fatal("expected success result")
}

textContent, ok := result.Content[0].(mcp.TextContent)
if !ok {
t.Fatalf("expected TextContent, got %T", result.Content[0])
}

if textContent.Text != `["aks-a","aks-b"]` {
t.Errorf("unexpected output: %s", textContent.Text)
}
}

func TestAzApiHandler_CustomTimeout(t *testing.T) {
Expand All @@ -230,8 +283,9 @@ func TestAzApiHandler_CustomTimeout(t *testing.T) {
}
}
return &azcli.Result{
Output: json.RawMessage("success"),
Error: "",
Output: json.RawMessage("success"),
ExitCode: 0,
Error: "",
}, nil
},
}
Expand Down Expand Up @@ -267,8 +321,9 @@ func TestAzApiHandler_NilTelemetryService(t *testing.T) {
mockClient := &mockAzClient{
executeFunc: func(ctx context.Context, command string) (*azcli.Result, error) {
return &azcli.Result{
Output: json.RawMessage(`"ok"`),
Error: "",
Output: json.RawMessage(`"ok"`),
ExitCode: 0,
Error: "",
}, nil
},
}
Expand Down
91 changes: 91 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package config
import (
"context"
"fmt"
"net"
"os"
"regexp"
"strings"
Expand Down Expand Up @@ -83,6 +84,20 @@ type ConfigData struct {
// DefaultAKSResourceID is the default AKS cluster resource ID used when aks_resource_id is not provided by the caller.
// Set via --default-aks-resource-id flag or AZURE_AKS_RESOURCE_ID environment variable.
DefaultAKSResourceID string

// AllowedHosts is the set of HTTP Host header values (with or without port)
// that the streamable-http / sse transports will accept. Empty means
// loopback-only (the safe default). The literal "*" disables Host
// enforcement entirely — only use behind a reverse proxy that validates
// Host on the operator's behalf.
AllowedHosts []string

// AllowedOrigins is the set of HTTP Origin header values that the
// streamable-http / sse transports will accept on browser-style cross
// origin requests. Empty Origin headers (non-browser callers) are always
// allowed. Empty list rejects every non-empty Origin. The literal "*"
// disables Origin enforcement entirely.
AllowedOrigins []string
}

// NewConfig creates and returns a new configuration instance
Expand All @@ -100,6 +115,8 @@ func NewConfig() *ConfigData {
LogLevel: "info",
UseLegacyTools: os.Getenv("USE_LEGACY_TOOLS") == "true",
TokenAuthOnly: false,
AllowedHosts: []string{},
AllowedOrigins: []string{},
}
}

Expand Down Expand Up @@ -143,6 +160,13 @@ func (cfg *ConfigData) ParseFlags() {
enabledComponents := flag.String("enabled-components", "",
"Comma-separated list of enabled components (empty means all components enabled). Available: az_cli,monitor,fleet,network,compute,detectors,advisor,inspektorgadget,kubectl,helm,cilium,hubble")

// HTTP transport security: DNS-rebinding / cross-origin protections
// (apply to streamable-http and sse transports only).
allowedHosts := flag.String("allowed-host", "",
"Comma-separated list of HTTP Host header values to accept on /mcp, /sse and /message. Empty means loopback only (localhost, 127.0.0.1, [::1]). Use '*' as an escape valve when behind a trusted reverse proxy. Required for non-loopback bindings when OAuth is disabled.")
trustedOrigins := flag.String("trusted-origin", "",
"Comma-separated list of HTTP Origin header values to accept on browser-style cross-origin requests to /mcp, /sse and /message (e.g. https://chat.example.com). Empty Origin headers from non-browser clients are always allowed. Use '*' to disable Origin enforcement entirely.")

// Kubernetes namespaces configuration
flag.StringVar(&cfg.AllowNamespaces, "allow-namespaces", "",
"Comma-separated list of allowed Kubernetes namespaces (empty means all namespaces)")
Expand Down Expand Up @@ -214,6 +238,29 @@ func (cfg *ConfigData) ParseFlags() {
}
}
}

// Parse HTTP transport allowlists.
cfg.AllowedHosts = splitAndTrim(*allowedHosts)
cfg.AllowedOrigins = splitAndTrim(*trustedOrigins)
}

// splitAndTrim splits raw on commas, trims whitespace, and drops empty entries.
// Returns nil for empty input so a zero allowlist remains a zero allowlist.
func splitAndTrim(raw string) []string {
if raw == "" {
return nil
}
parts := strings.Split(raw, ",")
out := make([]string, 0, len(parts))
for _, p := range parts {
if v := strings.TrimSpace(p); v != "" {
out = append(out, v)
}
}
if len(out) == 0 {
return nil
}
return out
}

// parseOAuthConfig parses OAuth-related command line arguments
Expand Down Expand Up @@ -357,9 +404,53 @@ func (cfg *ConfigData) ValidateConfig() error {
return fmt.Errorf("token-only authentication mode (--token-auth-only) requires unified tools and is not compatible with legacy tools (USE_LEGACY_TOOLS=true)")
}

// Refuse to start a publicly reachable streamable-http / sse listener
// with no authentication and no explicit trusted-host allowlist. This
// is the DNS-rebinding posture: a browser-origin attacker can otherwise
// reach /mcp through the victim's local network. The operator must pick
// one of three safe combinations:
// (a) bind to loopback (default --host 127.0.0.1)
// (b) enable OAuth (--oauth-enabled)
// (c) declare an explicit trusted-host allowlist (--allowed-host)
if isHTTPTransport(cfg.Transport) &&
!isLoopbackBindHost(cfg.Host) &&
!cfg.OAuthConfig.Enabled &&
len(cfg.AllowedHosts) == 0 {
return fmt.Errorf("transport %q bound to non-loopback host %q without OAuth and without --allowed-host: refusing to start to avoid DNS-rebinding exposure; choose one of: (a) bind --host 127.0.0.1, (b) --oauth-enabled, (c) --allowed-host=<your.hostname>",
cfg.Transport, cfg.Host)
}

return nil
}

// isHTTPTransport reports whether transport opens an HTTP listener that
// browsers can target. stdio is excluded because it has no HTTP attack surface.
func isHTTPTransport(transport string) bool {
switch transport {
case "streamable-http", "sse":
return true
}
return false
}

// isLoopbackBindHost reports whether host (as configured via --host) only
// binds to a loopback interface and therefore cannot be reached by a remote
// browser-origin attacker. Used for startup-time safety validation.
// An empty host is treated as loopback because callers that never invoke
// ParseFlags (notably unit tests) leave Host at its zero value, and the
// production default applied by ParseFlags is 127.0.0.1 anyway.
// Any address in 127.0.0.0/8 or the IPv6 ::1 loopback is accepted, matching
// the kernel's notion of "loopback" and the runtime middleware's check.
func isLoopbackBindHost(host string) bool {
if host == "" || host == "localhost" {
return true
}
if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() {
return true
}
return false
}

// InitializeTelemetry initializes the telemetry service
func (cfg *ConfigData) InitializeTelemetry(ctx context.Context, serviceName, serviceVersion string) {
// Create telemetry configuration
Expand Down
Loading
Loading