fix(httpapi,runtime): API correctness follow-ups (404 logs, empty scope param, import preview, telemetry lock, changed-fields audit)

internal/contracts/types.go

@@ -2,11 +2,16 @@
 package contracts
 
 import (
+	"errors"
 	"time"
 
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
 )
 
+// ErrServerNotFound is returned (wrapped) by controllers when a named upstream
+// server does not exist, so the REST layer can answer 404 via errors.Is.
+var ErrServerNotFound = errors.New("server not found")
+
 // APIResponse is the standard wrapper for all API responses
 type APIResponse struct {
 	Success   bool        `json:"success"`

internal/httpapi/config_funnel.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"net/http"
+	"strings"
 
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
 	internalRuntime "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime"
@@ -45,6 +46,32 @@ func refuseLockedTelemetryChange(stored, edited *config.Config) error {
 	}
 }
 
+// refuseAmbiguousLockedTelemetryKeys refuses, while the env telemetry lock is
+// active, a raw document that spells the telemetry section more than once
+// under case variants ("telemetry" and "Telemetry"). encoding/json decodes keys
+// case-insensitively, so which spelling wins would depend on map iteration /
+// marshal order; a locked setting must not hinge on that.
+func refuseAmbiguousLockedTelemetryKeys(document map[string]interface{}) error {
+	disabled, reason := telemetry.IsDisabledByEnv()
+	if !disabled {
+		return nil
+	}
+	n := 0
+	for k := range document {
+		if strings.EqualFold(k, "telemetry") {
+			n++
+		}
+	}
+	if n < 2 {
+		return nil
+	}
+	return &configMutationRefusal{
+		status: http.StatusUnprocessableEntity,
+		msg: "telemetry is locked: the document repeats the telemetry section under different key spellings, which is ambiguous while telemetry is disabled by " +
+			string(reason) + " in the environment.",
+	}
+}
+
 // configFunnelController is the optional controller capability behind every
 // config-writing REST path (Spec 108-f F2): the read of the desired config, the
 // caller's mutation, the FR-008a guard and the write all run under one lock,

internal/httpapi/config_funnel_test.go

@@ -134,3 +134,25 @@ func TestPatchConfig_FunnelValidationErrorIs400WithField(t *testing.T) {
 	assert.Equal(t, "anonymous_profile", body["field"])
 	assert.Equal(t, `unknown profile "ghost"`, body["error"])
 }
+
+// #1466: with the env telemetry lock active, a case-variant duplicate of the
+// locked key cannot smuggle a change past the lock — the comparison is on the
+// typed value the document resolves to.
+func TestApplyConfig_TelemetryLockRefusesCaseVariantDuplicate(t *testing.T) {
+	t.Setenv("DO_NOT_TRACK", "1")
+	off := false
+	ctrl := &funnelController{desired: &config.Config{Listen: "127.0.0.1:8080", Telemetry: &config.TelemetryConfig{Enabled: &off}}}
+	srv := NewServer(ctrl, zap.NewNop().Sugar(), nil)
+
+	docs := []string{
+		`{"telemetry":{"enabled":false},"Telemetry":{"ENABLED":true}}`,
+		`{"Telemetry":{"ENABLED":true},"telemetry":{"anonymous_id":"x"}}`,
+		`{"telemetry":{"anonymous_id":"x"},"Telemetry":{"ENABLED":true}}`,
+	}
+	for _, doc := range docs {
+		w := funnelRequest(t, srv, http.MethodPost, "/api/v1/config/apply", doc)
+		assert.Equal(t, http.StatusUnprocessableEntity, w.Code, "doc %s: %s", doc, w.Body.String())
+	}
+	require.NotNil(t, ctrl.desired.Telemetry.Enabled)
+	assert.False(t, *ctrl.desired.Telemetry.Enabled, "the locked value was not changed")
+}

internal/httpapi/scope_filters.go

@@ -72,7 +72,7 @@ func scopeFilterListContains(list []string, name string) bool {
 func rejectUnsupportedScopeFilters(w http.ResponseWriter, r *http.Request, honoured ...string) bool {
 	q := r.URL.Query()
 	for _, name := range []string{"profile", "client", "token", "agent"} {
-		if q.Get(name) == "" {
+		if !q.Has(name) {
 			continue
 		}
 

internal/httpapi/scope_filters_gate_test.go

@@ -150,3 +150,18 @@ func TestStatus_FeaturesScopeFiltersEqualsList(t *testing.T) {
 	require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body))
 	assert.Equal(t, scopeFilterSupportedFilters, body.Data.Features.ScopeFilters)
 }
+
+// #1394: a present-but-empty scope parameter (?token=) on a handler that does
+// not honour it is rejected like a valued one, not silently served unfiltered.
+func TestRejectUnsupportedScopeFilters_EmptyValueStillRejected(t *testing.T) {
+	for _, name := range []string{"profile", "client", "token", "agent"} {
+		req := httptest.NewRequest(http.MethodGet, "/api/v1/activity/summary?"+name+"=", nil)
+		rec := httptest.NewRecorder()
+		assert.Falsef(t, rejectUnsupportedScopeFilters(rec, req), "?%s= must be rejected", name)
+		assert.Equal(t, http.StatusBadRequest, rec.Code)
+		assert.Contains(t, rec.Body.String(), "unsupported_scope_filter")
+	}
+	// Honoured parameters stay accepted when empty.
+	req := httptest.NewRequest(http.MethodGet, "/api/v1/activity?token=", nil)
+	assert.True(t, rejectUnsupportedScopeFilters(httptest.NewRecorder(), req, "token"))
+}

internal/httpapi/server.go

@@ -4159,6 +4159,10 @@ func (s *Server) handleGetServerLogs(w http.ResponseWriter, r *http.Request) {
 
 	logEntries, err := s.controller.GetServerLogs(serverID, tail)
 	if err != nil {
+		if errors.Is(err, contracts.ErrServerNotFound) {
+			s.writeError(w, r, http.StatusNotFound, fmt.Sprintf("Server not found: %s", serverID))
+			return
+		}
 		s.logger.Errorw("Failed to get server logs", "server", serverID, "error", err)
 		s.writeError(w, r, http.StatusInternalServerError, fmt.Sprintf("Failed to get logs: %v", err))
 		return
@@ -5709,6 +5713,9 @@ func (s *Server) handleApplyConfig(w http.ResponseWriter, r *http.Request) {
 	// door's masks over the operator's credentials. The unmask runs INSIDE the
 	// config funnel, against the desired config it read under the lock.
 	result, ok := s.mutateConfig(w, r, "Failed to apply configuration", func(stored *config.Config) error {
+		if err := refuseAmbiguousLockedTelemetryKeys(document); err != nil {
+			return err
+		}
 		resolved, err := oauth.UnmaskLiveConfigDocument(document, stored)
 		if err != nil {
 			s.logger.Warnw("Refused a configuration write carrying an unbindable mask", "error", err)
@@ -5846,6 +5853,9 @@ func (s *Server) handlePatchConfig(w http.ResponseWriter, r *http.Request) {
 	// Direct and then changed any other setting lost the routing switch with no
 	// warning, on disk, with a success toast.
 	result, ok := s.mutateConfig(w, r, "Failed to apply configuration patch", func(cfg *config.Config) error {
+		if err := refuseAmbiguousLockedTelemetryKeys(patchMap); err != nil {
+			return err
+		}
 		merged, refusal := s.mergeConfigPatch(cfg, patchMap)
 		if refusal != nil {
 			return refusal

internal/httpapi/server_logs_notfound_test.go

@@ -0,0 +1,46 @@
+package httpapi
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"go.uber.org/zap"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
+)
+
+type logsErrController struct {
+	*MockServerController
+	err error
+}
+
+func (c *logsErrController) GetServerLogs(_ string, _ int) ([]contracts.LogEntry, error) {
+	return nil, c.err
+}
+
+// #1466: an unknown server is a 404 (as the swagger comment promises), any
+// other failure stays a 500.
+func TestGetServerLogs_NotFoundIs404(t *testing.T) {
+	cases := []struct {
+		name string
+		err  error
+		want int
+	}{
+		{"not found", fmt.Errorf("%w: ghost", contracts.ErrServerNotFound), http.StatusNotFound},
+		{"other error", fmt.Errorf("failed to read log"), http.StatusInternalServerError},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctrl := &logsErrController{MockServerController: &MockServerController{}, err: tc.err}
+			srv := NewServer(ctrl, zap.NewNop().Sugar(), nil)
+			req := httptest.NewRequest("GET", "/api/v1/servers/ghost/logs", http.NoBody)
+			req.Header.Set("X-API-Key", mockControllerAPIKey)
+			w := httptest.NewRecorder()
+			srv.ServeHTTP(w, req)
+			assert.Equal(t, tc.want, w.Code, w.Body.String())
+		})
+	}
+}

internal/runtime/config_hotreload.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"reflect"
 	"slices"
+	"strings"
 
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
 )
@@ -519,6 +520,36 @@ func DetectConfigChanges(oldCfg, newCfg *config.Config) *ConfigApplyResult {
 		}
 	}
 
+	// Remaining top-level fields (#1435). Each of these used to be edited,
+	// saved and adopted into the live config but computed an empty
+	// ChangedFields, so the apply answered "No configuration changes detected"
+	// (applied_immediately:false) for a change that had in fact been made. The
+	// list is checked by TestDetectConfigChanges_EveryTopLevelFieldIsDiffed:
+	// adding a config.Config field there forces a clause here (or an entry in
+	// that test's documented exclusion list). jsonEqual, not DeepEqual, for the
+	// PATCH round-trip reason documented on it.
+	for _, name := range undiffedHotConfigFields {
+		if !jsonEqual(configFieldValue(oldCfg, name), configFieldValue(newCfg, name)) {
+			result.ChangedFields = append(result.ChangedFields, configFieldJSONName(name))
+		}
+	}
+	// These are bound once at startup (the unix-socket listener, the tray
+	// endpoint, the search index debug flag, the tokenizer, and the MCP
+	// prompts capability / server instructions read at server construction): an edit is saved
+	// and takes effect on the next start. They are pinned to the live value by
+	// pinRestartGated, like the other restart-gated fields.
+	for _, name := range restartGatedConfigFields {
+		if !jsonEqual(configFieldValue(oldCfg, name), configFieldValue(newCfg, name)) {
+			field := configFieldJSONName(name)
+			result.ChangedFields = append(result.ChangedFields, field)
+			result.RequiresRestart = true
+			result.AppliedImmediately = false
+			if result.RestartReason == "" {
+				result.RestartReason = field + " is bound at startup - requires restart"
+			}
+		}
+	}
+
 	// If no changes detected
 	if len(result.ChangedFields) == 0 {
 		result.AppliedImmediately = false
@@ -559,3 +590,35 @@ func (r *ConfigApplyResult) FormatChangedFields() string {
 	// For 3+ fields, show "field1, field2, and N others"
 	return fmt.Sprintf("%s, %s, and %d others", r.ChangedFields[0], r.ChangedFields[1], len(r.ChangedFields)-2)
 }
+
+// undiffedHotConfigFields are the top-level config.Config fields (Go names)
+// with no dedicated clause in DetectConfigChanges: a change is saved and
+// adopted into the live config, and reported by its JSON key.
+var undiffedHotConfigFields = []string{
+	"EnableTray", "TopK", "MaxResultSizeChars", "InitTimeout", "ForwardProxyEnv",
+	"RequireMCPAuth", "CheckServerRepo", "DockerRecovery",
+	"RegistriesLocked", "AllowPrivateRegistryFetch", "Features",
+	"ToolResponseSessionRiskWarning", "OAuthExpiryWarningHours",
+	"ActivityRetentionDays", "ActivityMaxRecords", "ActivityMaxSizeMB",
+	"ActivityMaxResponseSize", "ActivityCleanupIntervalMin",
+	"ToolCallMaxResponseSize", "ToolCallMaxRecordsPerServer",
+	"IntentDeclaration", "SensitiveDataDetection", "OutputValidation",
+	"OutputSanitisation", "Telemetry",
+	"QuarantineEnabled", "RevealSecretHeaders",
+}
+
+// restartGatedConfigFields are bound once at startup; see pinRestartGated.
+var restartGatedConfigFields = []string{"TrayEndpoint", "EnableSocket", "DebugSearch", "Tokenizer", "EnablePrompts", "Instructions"}
+
+func configFieldValue(cfg *config.Config, goName string) interface{} {
+	return reflect.ValueOf(cfg).Elem().FieldByName(goName).Interface()
+}
+
+func configFieldJSONName(goName string) string {
+	f, _ := reflect.TypeOf(config.Config{}).FieldByName(goName)
+	name, _, _ := strings.Cut(f.Tag.Get("json"), ",")
+	if name == "" {
+		return goName
+	}
+	return name
+}

internal/runtime/config_hotreload_fields_test.go

@@ -0,0 +1,87 @@
+package runtime
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+)
+
+// #1435: an edit that DetectConfigChanges does not recognise computes an empty
+// ChangedFields and is answered "No configuration changes detected" — a silent
+// no-op the Settings UIs render as success. This guard mutates every top-level
+// field of config.Config in isolation and asserts the diff notices it.
+//
+// configFieldsNotDiffed lists the fields that are intentionally NOT diffed,
+// each with the reason. Adding a config field forces a choice: teach
+// DetectConfigChanges about it, or document here why an edit is not an
+// applicable change.
+var configFieldsNotDiffed = map[string]string{
+	"ServerEdition": "diffed by the dedicated server_edition clauses (restart projection, admin_emails, access); a zero-value block projects the same bytes as nil",
+	"TLS":           "diffed (restart-gated) by the dedicated tls clause; the baseline here already carries a TLS block, so a zero-value edit is not a change",
+}
+
+// mutateConfigField changes one top-level field of cfg to a value that differs
+// from its zero value and reports whether it knew how.
+func mutateConfigField(cfg *config.Config, f reflect.StructField, tryFalse bool) bool {
+	v := reflect.ValueOf(cfg).Elem().FieldByName(f.Name)
+	switch v.Kind() {
+	case reflect.String:
+		v.SetString("changed-value")
+	case reflect.Bool:
+		v.SetBool(!v.Bool())
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		v.SetInt(v.Int() + 7)
+	case reflect.Float32, reflect.Float64:
+		v.SetFloat(v.Float() + 7)
+	case reflect.Slice:
+		elem := reflect.New(v.Type().Elem()).Elem()
+		if elem.Kind() == reflect.String {
+			elem.SetString("changed-value")
+		}
+		v.Set(reflect.Append(v, elem))
+	case reflect.Pointer:
+		p := reflect.New(v.Type().Elem())
+		switch p.Elem().Kind() {
+		case reflect.Bool:
+			p.Elem().SetBool(!tryFalse)
+		case reflect.Int, reflect.Int64:
+			p.Elem().SetInt(7)
+		}
+		v.Set(p)
+	default:
+		return false
+	}
+	return true
+}
+
+func TestDetectConfigChanges_EveryTopLevelFieldIsDiffed(t *testing.T) {
+	typ := reflect.TypeOf(config.Config{})
+	for i := 0; i < typ.NumField(); i++ {
+		f := typ.Field(i)
+		if !f.IsExported() || f.Tag.Get("json") == "-" {
+			continue
+		}
+		t.Run(f.Name, func(t *testing.T) {
+			if reason, ok := configFieldsNotDiffed[f.Name]; ok {
+				assert.NotEmpty(t, reason)
+				t.Skipf("intentionally not diffed: %s", reason)
+			}
+			oldCfg := &config.Config{Listen: "127.0.0.1:8080", DataDir: "/d", TLS: &config.TLSConfig{}}
+			edited := &config.Config{Listen: "127.0.0.1:8080", DataDir: "/d", TLS: &config.TLSConfig{}}
+			if !mutateConfigField(edited, f, false) {
+				t.Fatalf("test cannot mutate field %s (kind %s): extend mutateConfigField", f.Name, f.Type.Kind())
+			}
+			result := DetectConfigChanges(oldCfg, edited)
+			if len(result.ChangedFields) == 0 && f.Type.Kind() == reflect.Pointer && f.Type.Elem().Kind() == reflect.Bool {
+				// A *bool whose unset state means "true": the edit that matters is false.
+				edited = &config.Config{Listen: "127.0.0.1:8080", DataDir: "/d", TLS: &config.TLSConfig{}}
+				mutateConfigField(edited, f, true)
+				result = DetectConfigChanges(oldCfg, edited)
+			}
+			assert.NotEmpty(t, result.ChangedFields, "editing %s alone is reported as \"No configuration changes detected\"", f.Name)
+		})
+	}
+}

internal/runtime/restart_gated.go

@@ -54,6 +54,15 @@ func pinRestartGated(live, desired *config.Config) *config.Config {
 	// pending a restart, leaving Runtime.Config() readers disagreeing with
 	// the sink actually still writing).
 	pinned.AuditLog = live.AuditLog
+	// Bound once at startup (#1435): the unix-socket listener, the tray
+	// endpoint, the search debug flag and the tokenizer.
+	pinned.TrayEndpoint = live.TrayEndpoint
+	pinned.EnableSocket = live.EnableSocket
+	pinned.DebugSearch = live.DebugSearch
+	pinned.Tokenizer = live.Tokenizer
+	// Read once at MCP server construction (mcp.go), never re-read.
+	pinned.EnablePrompts = live.EnablePrompts
+	pinned.Instructions = live.Instructions
 	// server_edition's restart-pinned subset (enabled, oauth.*, public_url,
 	// session_cookie_secure, session_ttl, bearer_token_ttl,
 	// credential_encryption_key — Spec 107 FR-039 part 2) is bound at login

internal/runtime/restart_gated_test.go

@@ -48,6 +48,10 @@ func TestPinRestartGatedCoversEveryRestartGatedField(t *testing.T) {
 		// the apply as pending a restart, leaving Runtime.Config() readers
 		// disagreeing with the sink actually in effect.
 		"audit_log": func(c *config.Config) { c.AuditLog = &config.AuditLogConfig{Path: "/tmp/two.jsonl"} },
+		// Read once when each MCP server is constructed (mcp.go), never
+		// re-read: a PATCH must report requires_restart, not applied.
+		"enable_prompts": func(c *config.Config) { c.EnablePrompts = !c.EnablePrompts },
+		"instructions":   func(c *config.Config) { c.Instructions = "changed instructions" },
 	}
 
 	for name, mutate := range mutations {

internal/server/server.go

@@ -3916,7 +3916,7 @@ func (s *Server) GetServerLogs(serverName string, tail int) ([]contracts.LogEntr
 	// Check if server exists
 	_, exists := s.runtime.UpstreamManager().GetClient(serverName)
 	if !exists {
-		return nil, fmt.Errorf("server not found: %s", serverName)
+		return nil, fmt.Errorf("%w: %s", contracts.ErrServerNotFound, serverName)
 	}
 
 	// Read from server-specific log file