fix: detect config changes for require_mcp_auth and handle residual edge cases

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")
+	}
+}

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)
+	}
+}

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))
+	}
+}

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)
+}

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)
+	})
+}