config: require token for public HTTP

Amolith created 1 month ago

Change summary

internal/appconfig/config.go      | 21 ++++++++
internal/appconfig/config_test.go | 80 +++++++++++++++++++++++++++++++++
2 files changed, 100 insertions(+), 1 deletion(-)

Detailed changes

internal/appconfig/config.go

@@ -13,6 +13,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/BurntSushi/toml"
 
@@ -103,6 +104,9 @@ func Load(ctx context.Context, path string, resolver *configvalue.Resolver) (Con
 	if err != nil {
 		return Config{}, err
 	}
+	if err := validateHTTPListenerSafety(httpAddr, httpToken); err != nil {
+		return Config{}, err
+	}
 
 	return Config{
 		Username:  username,
@@ -225,8 +229,23 @@ func httpToken(ctx context.Context, resolver *configvalue.Resolver, config *stri
 	return value, nil
 }
 
+func validateHTTPListenerSafety(addr, token string) error {
+	host, _, err := net.SplitHostPort(addr)
+	if err != nil {
+		return fmt.Errorf("invalid MCP HTTP listen address: %w", err)
+	}
+	if isLoopbackHost(host) {
+		return nil
+	}
+	if token != "" {
+		return nil
+	}
+
+	return fmt.Errorf("MCP HTTP token is required for non-loopback HTTP listen addresses")
+}
+
 func isLoopbackHost(host string) bool {
-	if host == "localhost" {
+	if strings.EqualFold(host, "localhost") {
 		return true
 	}
 

internal/appconfig/config_test.go

@@ -205,6 +205,86 @@ http_token = ""
 	}
 }
 
+// server.SECURITY.1
+func TestLoadACIDServerSecurity1AllowsLoopbackHTTPAddrWithoutToken(t *testing.T) {
+	tests := []struct {
+		name string
+		addr string
+	}{
+		{name: "ipv4", addr: "127.0.0.1:8123"},
+		{name: "localhost", addr: "localhost:8123"},
+		{name: "ipv6", addr: "[::1]:8123"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			configPath := filepath.Join(t.TempDir(), "config.toml")
+			writeConfig(t, configPath, `[user]
+name = "toml-user"
+password = "toml-password"
+
+[mcp]
+http_addr = "`+tt.addr+`"
+`)
+
+			got, err := Load(context.Background(), configPath, configvalue.NewResolver())
+			if err != nil {
+				t.Fatalf("Load() error = %v", err)
+			}
+			if got.HTTPAddr != tt.addr {
+				t.Fatalf("HTTPAddr = %q, want %q", got.HTTPAddr, tt.addr)
+			}
+			if got.HTTPToken != "" {
+				t.Fatalf("HTTPToken = %q, want empty", got.HTTPToken)
+			}
+		})
+	}
+}
+
+// server.SECURITY.1
+func TestLoadACIDServerSecurity1RejectsNonLoopbackHTTPAddrWithoutToken(t *testing.T) {
+	configPath := filepath.Join(t.TempDir(), "config.toml")
+	writeConfig(t, configPath, `[user]
+name = "toml-user"
+password = "toml-password"
+
+[mcp]
+http_addr = "0.0.0.0:8123"
+`)
+
+	_, err := Load(context.Background(), configPath, configvalue.NewResolver())
+	if err == nil {
+		t.Fatal("Load() error = nil, want non-loopback HTTP token error")
+	}
+	if !strings.Contains(err.Error(), "MCP HTTP token is required") {
+		t.Fatalf("Load() error = %v, want HTTP token required error", err)
+	}
+}
+
+// server.SECURITY.1
+func TestLoadACIDServerSecurity1AllowsNonLoopbackHTTPAddrWithToken(t *testing.T) {
+	configPath := filepath.Join(t.TempDir(), "config.toml")
+	writeConfig(t, configPath, `[user]
+name = "toml-user"
+password = "toml-password"
+
+[mcp]
+http_addr = "0.0.0.0:8123"
+http_token = "secret-token"
+`)
+
+	got, err := Load(context.Background(), configPath, configvalue.NewResolver())
+	if err != nil {
+		t.Fatalf("Load() error = %v", err)
+	}
+	if got.HTTPAddr != "0.0.0.0:8123" {
+		t.Fatalf("HTTPAddr = %q, want non-loopback address", got.HTTPAddr)
+	}
+	if got.HTTPToken != "secret-token" {
+		t.Fatal("HTTPToken was not resolved")
+	}
+}
+
 func TestLoadRejectsMissingExplicitConfigFile(t *testing.T) {
 	_, err := Load(context.Background(), filepath.Join(t.TempDir(), "missing.toml"), configvalue.NewResolver())
 	if err == nil {