diff --git a/cmd/mcpproxy/cli_config_resolve_test.go b/cmd/mcpproxy/cli_config_resolve_test.go index b73c2807c..30cdac00f 100644 --- a/cmd/mcpproxy/cli_config_resolve_test.go +++ b/cmd/mcpproxy/cli_config_resolve_test.go @@ -199,3 +199,41 @@ func TestResolveCLIConfigPathPrecedence(t *testing.T) { t.Errorf("-d without a config file falls back to discovery, got %q", got) } } + +func TestLoadRegistryConfig_ExplicitConfigErrorPropagated(t *testing.T) { + resetLoaderGlobals(t) + configFile = filepath.Join(t.TempDir(), "nonexistent.json") + + _, err := loadRegistryConfig() + if err == nil { + t.Fatal("expected error when explicit global -c points to nonexistent file, got nil") + } + + configFile = "" + registryConfigPath = filepath.Join(t.TempDir(), "nonexistent-local.json") + _, err = loadRegistryConfig() + if err == nil { + t.Fatal("expected error when explicit --config points to nonexistent file, got nil") + } +} + +func TestLoadCLIConfig_DataDirAppliesTLSEnvOverrides(t *testing.T) { + resetLoaderGlobals(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Chdir(t.TempDir()) + t.Setenv("MCPPROXY_LISTEN", "127.0.0.1:23456") + t.Setenv("MCPPROXY_TLS_ENABLED", "false") + dataDir = filepath.Join(t.TempDir(), "new-datadir") + + cfg, err := loadCLIConfig("") + if err != nil { + t.Fatalf("loadCLIConfig failed: %v", err) + } + if cfg.Listen != "127.0.0.1:23456" { + t.Errorf("expected Listen to be 127.0.0.1:23456 from env override, got %q", cfg.Listen) + } + if cfg.TLS != nil && cfg.TLS.Enabled { + t.Errorf("expected TLS to be disabled from env override, got enabled") + } +} diff --git a/cmd/mcpproxy/doctor_redact_test.go b/cmd/mcpproxy/doctor_redact_test.go index 161bdfceb..acb0b1955 100644 --- a/cmd/mcpproxy/doctor_redact_test.go +++ b/cmd/mcpproxy/doctor_redact_test.go @@ -150,3 +150,27 @@ func TestStatusOutput_MasksKeyInEveryFormat(t *testing.T) { } } } + +func TestDoctorRedact_ApostropheURL(t *testing.T) { + input := "connect failed: dial https://h.example/mcp?label=Bob's&%74oken=secret-tok-123456 failed" + redacted := redactDoctorString(input) + if strings.Contains(redacted, "secret-tok-123456") { + t.Fatalf("credential after apostrophe leaked: %s", redacted) + } + if !strings.Contains(redacted, "Bob's") { + t.Fatalf("label before token should be preserved: %s", redacted) + } + if !strings.Contains(redacted, "REDACTED") { + t.Fatalf("expected REDACTED in output: %s", redacted) + } + + // Enclosed in quotes + inQuotes := "check 'https://h.example/mcp?label=Bob's&token=mysecret' for info" + redactedQuotes := redactDoctorString(inQuotes) + if strings.Contains(redactedQuotes, "mysecret") { + t.Fatalf("credential in quoted URL leaked: %s", redactedQuotes) + } + if !strings.HasSuffix(strings.TrimSpace(redactedQuotes), "' for info") { + t.Fatalf("trailing quote and prose should be preserved: %s", redactedQuotes) + } +} diff --git a/internal/httpapi/import_preview_fields_test.go b/internal/httpapi/import_preview_fields_test.go index d484b5cd6..266f7ad90 100644 --- a/internal/httpapi/import_preview_fields_test.go +++ b/internal/httpapi/import_preview_fields_test.go @@ -237,3 +237,27 @@ func TestImportPreview_RedactsCredentialShapedCommandFromSummary(t *testing.T) { func bytesContains(haystack []byte, needle string) bool { return bytes.Contains(haystack, []byte(needle)) } + +func TestImportPreview_EmptyObjectWithoutFormatHint(t *testing.T) { + logger := zap.NewNop().Sugar() + mock := &mockImportController{apiKey: "test-key"} + server := NewServer(mock, logger, nil) + + body, _ := json.Marshal(ImportRequest{Content: "{}"}) + req := httptest.NewRequest("POST", "/api/v1/servers/import/json?preview=true", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-API-Key", "test-key") + rr := httptest.NewRecorder() + server.router.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + var wrapped wrappedImportResponse + if err := json.Unmarshal(rr.Body.Bytes(), &wrapped); err != nil { + t.Fatalf("decode: %v", err) + } + if len(wrapped.Data.Imported) != 0 { + t.Fatalf("expected 0 imported servers, got %d", len(wrapped.Data.Imported)) + } +} diff --git a/internal/httpapi/server_config_patch_test.go b/internal/httpapi/server_config_patch_test.go index 7a48a1711..41b585058 100644 --- a/internal/httpapi/server_config_patch_test.go +++ b/internal/httpapi/server_config_patch_test.go @@ -353,3 +353,35 @@ func TestHandlePatchConfig_MergesOntoDesiredConfig(t *testing.T) { "a restart-pending routing mode the client never mentioned must survive the patch") assert.Equal(t, "secret-key", ctrl.captured.APIKey, "secrets still preserved verbatim") } + +func TestHandlePatchConfig_RequireMCPAuthChangeDetected(t *testing.T) { + live := &config.Config{ + APIKey: "secret-key", + RequireMCPAuth: false, + } + ctrl := &mockPatchConfigController{ + apiKey: "secret-key", + live: live, + detectChanges: true, + } + srv := NewServer(ctrl, zap.NewNop().Sugar(), nil) + + body, err := json.Marshal(map[string]interface{}{ + "require_mcp_auth": true, + }) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPatch, "/api/v1/config", bytes.NewReader(body)) + req.Header.Set("X-API-Key", "secret-key") + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + + res := decodePatchResult(t, w.Body.Bytes()) + assert.Equal(t, true, res["success"]) + assert.Equal(t, true, res["applied_immediately"]) + changed, ok := res["changed_fields"].([]interface{}) + require.True(t, ok) + assert.ElementsMatch(t, []interface{}{"require_mcp_auth"}, changed) +} diff --git a/internal/runtime/config_hotreload_test.go b/internal/runtime/config_hotreload_test.go index 3eedecd2b..62848881a 100644 --- a/internal/runtime/config_hotreload_test.go +++ b/internal/runtime/config_hotreload_test.go @@ -1201,3 +1201,86 @@ func TestDetectConfigChanges_AdvertiseUpstreamServers(t *testing.T) { assert.NotContains(t, DetectConfigChanges(mk(nil), mk(&on)).ChangedFields, "advertise_upstream_servers") } + +func TestDetectConfigChanges_Issue1466Residuals(t *testing.T) { + base := func() *config.Config { + return &config.Config{ + Listen: "127.0.0.1:8080", + DataDir: "/d", + TLS: &config.TLSConfig{}, + } + } + + t.Run("require_mcp_auth toggle detected hot-reloadable", func(t *testing.T) { + oldCfg := base() + oldCfg.RequireMCPAuth = false + newCfg := base() + newCfg.RequireMCPAuth = true + + result := DetectConfigChanges(oldCfg, newCfg) + require.True(t, result.Success) + assert.Equal(t, []string{"require_mcp_auth"}, result.ChangedFields) + assert.False(t, result.RequiresRestart) + assert.True(t, result.AppliedImmediately) + }) + + t.Run("quarantine_enabled toggle detected hot-reloadable", func(t *testing.T) { + oldCfg := base() + disabled := false + newCfg := base() + newCfg.QuarantineEnabled = &disabled + + result := DetectConfigChanges(oldCfg, newCfg) + require.True(t, result.Success) + assert.Equal(t, []string{"quarantine_enabled"}, result.ChangedFields) + assert.False(t, result.RequiresRestart) + }) + + t.Run("init_timeout change detected hot-reloadable", func(t *testing.T) { + oldCfg := base() + dur := config.Duration(45 * time.Second) + newCfg := base() + newCfg.InitTimeout = &dur + + result := DetectConfigChanges(oldCfg, newCfg) + require.True(t, result.Success) + assert.Equal(t, []string{"init_timeout"}, result.ChangedFields) + assert.False(t, result.RequiresRestart) + }) + + t.Run("max_result_size_chars change detected hot-reloadable", func(t *testing.T) { + oldCfg := base() + val := 250000 + newCfg := base() + newCfg.MaxResultSizeChars = &val + + result := DetectConfigChanges(oldCfg, newCfg) + require.True(t, result.Success) + assert.Equal(t, []string{"max_result_size_chars"}, result.ChangedFields) + assert.False(t, result.RequiresRestart) + }) + + t.Run("forward_proxy_env change detected hot-reloadable", func(t *testing.T) { + oldCfg := base() + oldCfg.ForwardProxyEnv = false + newCfg := base() + newCfg.ForwardProxyEnv = true + + result := DetectConfigChanges(oldCfg, newCfg) + require.True(t, result.Success) + assert.Equal(t, []string{"forward_proxy_env"}, result.ChangedFields) + assert.False(t, result.RequiresRestart) + }) + + t.Run("debug_search change detected restart-gated", func(t *testing.T) { + oldCfg := base() + oldCfg.DebugSearch = false + newCfg := base() + newCfg.DebugSearch = true + + result := DetectConfigChanges(oldCfg, newCfg) + require.True(t, result.Success) + assert.Equal(t, []string{"debug_search"}, result.ChangedFields) + assert.True(t, result.RequiresRestart) + }) +}