feat(headers): restore REST/SSE redaction and add deep-merge PATCH

User feedback on the previous commit: dropping REST/SSE redaction
unblocked the UI but narrowed the threat model from PR #425 (a local
process that holds the API key but not filesystem access could read
other upstreams' Bearer tokens off the wire). The better trade-off is
to keep both halves of PR #425 intact AND let the UI edit without
round-tripping the redacted sentinel.

Solution: deep-merge PATCH semantics so the client sends only the keys
that changed. Redacted-but-untouched values stay out of the patch
entirely, the backend keeps the real string on disk.

Backend:

- internal/httpapi/server.go::handlePatchServer — when the request
  body contains `headers` or `env`, MERGE into the existing stored map
  instead of replacing. New `headers_remove` / `env_remove` fields
  carry explicit deletes. Sending both is allowed (deletes apply after
  upserts).
- internal/httpapi/server.go::AddServerRequest — adds the two
  `*_remove` fields with a comment documenting the new semantics. The
  add path ignores them.
- internal/httpapi/server.go::handleGetServers — re-enable
  `redactServerHeaders` on both code paths.
- internal/runtime/event_bus.go::emitServersChanged — re-enable
  redaction on SSE `servers.changed` payloads. SSE rides the same
  trust boundary as the REST GET.
- internal/config/config.go::RevealSecretHeaders — restore the
  original PR #425 doc (REST + MCP both gated) with a new paragraph
  pointing at the deep-merge mechanism that makes the UI work anyway.

Tests:

- internal/httpapi/patch_server_test.go — 4 new tests pinning the
  merge semantics:
  - TestHandlePatchServer_HeadersDeepMerge — `headers: {X-New: v}`
    against existing `{Authorization, X-Trace}` preserves both
    original keys and adds X-New.
  - TestHandlePatchServer_HeadersRemove — `headers_remove: [...]`
    deletes the listed keys.
  - TestHandlePatchServer_HeadersSetAndRemove — both fields in one
    PATCH; deletes apply after upserts.
  - TestHandlePatchServer_EnvDeepMerge — same pattern for env.
- internal/runtime/event_bus_payload_test.go — restored the original
  PR #425 assertion (`...RedactsSensitiveHeaders`).

Frontend (Web UI):

- ServerDetail.vue — `patchServerDiff(patch, action)` replaces the old
  `patchKVMap`. Each per-row UI action now sends a minimal targeted
  patch:
    - Edit one row: `{headers: {key: newValue}}`
    - Delete one row: `{headers_remove: [key]}`
    - Add one row: `{headers: {newKey: newValue}}`
    - Convert to secret: `{headers: {key: "${keyring:NAME}"}}`
  The redacted sentinel for unchanged keys never round-trips, by
  construction.
- KVValueCell.vue — restore the `isBackendRedacted` branch. When the
  cell renders `***REDACTED***`, the reveal / Convert-to-secret
  buttons disappear (we don't hold the real value) and the cell shows
  the sentinel verbatim with a tooltip explaining that editing still
  works through the inline edit button.

macOS Swift:

- ServerDetailView.swift::saveEdits — switch from "send the full
  parsed map" to "diff against `server.headers` / `server.env` and
  send the diff". New private helper `diffKVMap(original:next:)`
  returns a `(set, remove)` tuple suitable for the deep-merge PATCH
  body. The same invariant holds: leaving a redacted line untouched
  in the textarea produces `next[k] == "***REDACTED***" ==
  original[k]`, so the key stays out of both sides of the diff and
  the backend preserves the real value.
- ServerDetailView.swift::kvRow — restore the
  `value != "***REDACTED***"` gate on the Convert-to-secret button
  (the sentinel isn't useful as a keyring payload).
- ServerDetailView.swift::editHeaders doc — explain the new flow.

End-to-end verification against the live local instance:

  $ curl -s -H "X-API-Key: ..." /api/v1/servers | jq '...synapbus.headers'
  → {"Authorization": "***REDACTED***"}                   # redacted ✓

  $ jq '...synapbus.headers' ~/.mcpproxy/mcp_config.json
  → {"Authorization": "Bearer 1d386..."}                  # real on disk

  $ curl -X PATCH .../servers/synapbus -d '{"headers":{"X-Trace-Test":"merge-works"}}'
  $ jq '...synapbus.headers' ~/.mcpproxy/mcp_config.json
  → {"Authorization": "Bearer 1d386...", "X-Trace-Test": "merge-works"}
                                                          # real token preserved ✓

  $ curl -X PATCH .../servers/synapbus -d '{"headers_remove":["X-Trace-Test"]}'
  $ jq '...synapbus.headers' ~/.mcpproxy/mcp_config.json
  → {"Authorization": "Bearer 1d386..."}                  # X-Trace-Test deleted ✓

PR #425's E2E tests still pass (TestE2E_PatchDeepMergesEnvAndHeaders,
TestE2E_MultipleEnableDisablePreservesConfig — both exercise the MCP
tool which still redacts). PR #425's intent is fully preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: claude

frontend/src/components/KVValueCell.vue

@@ -27,30 +27,39 @@
         <span class="font-mono text-xs">env var: {{ envRefName }}</span>
       </span>
     </template>
-    <!-- Literal value: masked by default to avoid shoulder-surfing
-         leaks; click the eye to reveal the actual string returned by
-         the REST API. The REST API serves plaintext header / env
-         values (the API key is the gate at this trust boundary), so
-         reveal always shows the real thing. -->
+    <!-- Literal value path. Two sub-cases:
+         1. Backend redacted the value (`***REDACTED***` sentinel). The
+            real string isn't on the client; reveal would just expose
+            the sentinel and Convert-to-secret has nothing to upload to
+            the keyring. We surface a small "backend-redacted" hint
+            instead so the user understands why those actions are
+            disabled. Editing the value still works through the inline
+            edit button — the PATCH endpoint deep-merges, so typing a
+            new value replaces just that one header server-side.
+         2. Plaintext literal: mask by default, eye to reveal, lock to
+            convert. -->
     <template v-else>
-      <code v-if="revealed" class="bg-base-200 px-1.5 py-0.5 rounded text-xs break-all">{{ rawValue }}</code>
-      <code v-else class="bg-base-200 px-1.5 py-0.5 rounded text-xs text-base-content/50">{{ redactedPreview }}</code>
-      <button
-        class="btn btn-ghost btn-xs"
-        :title="revealed ? 'Hide value' : 'Reveal value'"
-        @click="revealed ? $emit('hide') : $emit('reveal')"
-      >
-        <span aria-hidden="true">{{ revealed ? '🙈' : '👁' }}</span>
-      </button>
-      <button
-        class="btn btn-ghost btn-xs"
-        title="Move value into the OS keyring and reference it as ${keyring:name}"
-        @click="$emit('convert')"
-        :disabled="busy"
-      >
-        <span aria-hidden="true">🔒</span>
-        <span class="hidden md:inline ml-1">Convert to secret</span>
-      </button>
+      <code v-if="isBackendRedacted" class="bg-base-200 px-1.5 py-0.5 rounded text-xs text-base-content/50" title="Backend redacted this value. Edit to overwrite — the PATCH endpoint deep-merges, so the unchanged real value is preserved.">{{ rawValue }}</code>
+      <template v-else>
+        <code v-if="revealed" class="bg-base-200 px-1.5 py-0.5 rounded text-xs break-all">{{ rawValue }}</code>
+        <code v-else class="bg-base-200 px-1.5 py-0.5 rounded text-xs text-base-content/50">{{ redactedPreview }}</code>
+        <button
+          class="btn btn-ghost btn-xs"
+          :title="revealed ? 'Hide value' : 'Reveal value'"
+          @click="revealed ? $emit('hide') : $emit('reveal')"
+        >
+          <span aria-hidden="true">{{ revealed ? '🙈' : '👁' }}</span>
+        </button>
+        <button
+          class="btn btn-ghost btn-xs"
+          title="Move value into the OS keyring and reference it as ${keyring:name}"
+          @click="$emit('convert')"
+          :disabled="busy"
+        >
+          <span aria-hidden="true">🔒</span>
+          <span class="hidden md:inline ml-1">Convert to secret</span>
+        </button>
+      </template>
     </template>
     <button class="btn btn-ghost btn-xs" title="Edit" @click="$emit('start-edit')" :disabled="busy">✎</button>
     <button class="btn btn-ghost btn-xs text-error" title="Delete" @click="$emit('delete')" :disabled="busy">✕</button>
@@ -114,6 +123,15 @@ const envRefName = computed(() => {
   const m = (props.rawValue ?? '').match(/^\$\{env:([^}]+)\}$/)
   return m ? m[1] : ''
 })
+// The Go backend redacts secret header values via
+// `redactServerHeaders` so an MCP agent calling `upstream_servers list`
+// cannot exfiltrate Bearer tokens (PR #425). The REST API and SSE
+// channel inherit the same redaction. When that sentinel surfaces, the
+// real string is not in our hands — reveal/convert are disabled, but
+// editing still works because the PATCH endpoint deep-merges (the
+// untouched redacted key simply stays out of the patch body).
+const isBackendRedacted = computed(() => props.rawValue === '***REDACTED***')
+
 const redactedPreview = computed(() => {
   const v = props.rawValue ?? ''
   if (!v) return '(empty)'

frontend/src/views/ServerDetail.vue

@@ -2010,21 +2010,16 @@ async function startAddingEnv() {
   newEnvKeyInput.value?.focus()
 }
 
-// Build the next headers/env map for a PATCH. We send the full map (not a
-// diff) so the backend stores exactly what's on the client — there is no
-// "merge" semantics on the server side, the partial-update is at the
-// field level (headers / env / args / ...), not at the per-key level.
-function buildNextMap(scope: 'header' | 'env', mutate: (next: Record<string, string>) => void): Record<string, string> {
-  const next = { ...(scope === 'header' ? serverHeaders.value : serverEnv.value) }
-  mutate(next)
-  return next
-}
-
-async function patchKVMap(scope: 'header' | 'env', nextMap: Record<string, string>, action: string): Promise<boolean> {
+// patchServer with deep-merge semantics: the backend treats keys present
+// in `headers` / `env` as upserts, keys absent as preserved, and keys
+// listed in `headers_remove` / `env_remove` as deletes. This lets us send
+// the minimal diff for each user action — and crucially never round-trips
+// `***REDACTED***` values: any header whose redacted form was unchanged
+// simply stays out of the patch, so the backend keeps the real string.
+async function patchServerDiff(patch: Record<string, unknown>, action: string): Promise<boolean> {
   if (!server.value) return false
   kvPatchInFlight.value = true
   try {
-    const patch = scope === 'header' ? { headers: nextMap } : { env: nextMap }
     const resp = await api.patchServer(server.value.name, patch)
     if (!resp.success) {
       systemStore.addToast({ type: 'error', title: `${action} failed`, message: resp.error || 'Unknown error' })
@@ -2041,37 +2036,38 @@ async function patchKVMap(scope: 'header' | 'env', nextMap: Record<string, strin
   }
 }
 
+function scopeKey(scope: 'header' | 'env'): 'headers' | 'env' {
+  return scope === 'header' ? 'headers' : 'env'
+}
+function scopeRemoveKey(scope: 'header' | 'env'): 'headers_remove' | 'env_remove' {
+  return scope === 'header' ? 'headers_remove' : 'env_remove'
+}
+
 async function saveEdit(scope: 'header' | 'env', k: string, val: string) {
-  const next = buildNextMap(scope, (m) => {
-    m[k] = val
-  })
-  const ok = await patchKVMap(scope, next, `Updated ${k}`)
+  const ok = await patchServerDiff({ [scopeKey(scope)]: { [k]: val } }, `Updated ${k}`)
   if (ok) editingKey.value = null
 }
 
 async function deleteKv(scope: 'header' | 'env', k: string) {
   if (!confirm(`Delete ${scope === 'header' ? 'header' : 'env variable'} "${k}"?`)) return
-  const next = buildNextMap(scope, (m) => {
-    delete m[k]
-  })
-  await patchKVMap(scope, next, `Deleted ${k}`)
+  await patchServerDiff({ [scopeRemoveKey(scope)]: [k] }, `Deleted ${k}`)
 }
 
 async function commitNewHeader() {
   if (!newHeaderKey.value || !newHeaderValue.value) return
-  const next = buildNextMap('header', (m) => {
-    m[newHeaderKey.value] = newHeaderValue.value
-  })
-  const ok = await patchKVMap('header', next, `Added ${newHeaderKey.value}`)
+  const ok = await patchServerDiff(
+    { headers: { [newHeaderKey.value]: newHeaderValue.value } },
+    `Added ${newHeaderKey.value}`
+  )
   if (ok) addingHeader.value = false
 }
 
 async function commitNewEnv() {
   if (!newEnvKey.value || !newEnvValue.value) return
-  const next = buildNextMap('env', (m) => {
-    m[newEnvKey.value] = newEnvValue.value
-  })
-  const ok = await patchKVMap('env', next, `Added ${newEnvKey.value}`)
+  const ok = await patchServerDiff(
+    { env: { [newEnvKey.value]: newEnvValue.value } },
+    `Added ${newEnvKey.value}`
+  )
   if (ok) addingEnv.value = false
 }
 
@@ -2114,10 +2110,10 @@ async function commitConvert() {
       return
     }
     const ref = `\${keyring:${m.secretName}}`
-    const next = buildNextMap(m.scope, (map) => {
-      map[m.key] = ref
-    })
-    await patchKVMap(m.scope, next, `Converted ${m.key} to secret`)
+    await patchServerDiff(
+      { [scopeKey(m.scope)]: { [m.key]: ref } },
+      `Converted ${m.key} to secret`
+    )
     closeConvertModal()
   } catch (e: any) {
     systemStore.addToast({ type: 'error', title: 'Convert failed', message: e?.message || String(e) })

internal/config/config.go

@@ -163,20 +163,22 @@ type Config struct {
 	// Security scanner settings (Spec 039)
 	Security *SecurityConfig `json:"security,omitempty" mapstructure:"security"`
 
-	// RevealSecretHeaders controls whether the `upstream_servers` MCP
-	// tool returns sensitive header values (Authorization, X-API-Key,
-	// Cookie, …) in plaintext or redacted to `***REDACTED***`.
+	// RevealSecretHeaders, when true, disables the redaction of sensitive
+	// header values (Authorization, X-API-Key, Cookie, …) in responses
+	// from the `upstream_servers` MCP tool, the `/api/v1/servers` REST
+	// API, and the SSE event stream.
 	//
-	// Default false — an MCP agent calling `upstream_servers list` sees
-	// `***REDACTED***` so it cannot read another upstream's Bearer token
-	// out of the config and exfiltrate it back to the LLM context.
+	// Default false — sensitive header values are surfaced as
+	// `***REDACTED***` so an MCP agent cannot read Bearer tokens / API
+	// keys out of another upstream's config (PR #425).
 	//
-	// This flag DOES NOT affect the REST API (`/api/v1/servers`) or the
-	// SSE event stream. Those paths are gated by the local API key —
-	// the same trust boundary as access to `~/.mcpproxy/mcp_config.json`
-	// on disk — so redacting there bought no real security while
-	// breaking the Web UI / macOS tray edit-and-save round-trip. They
-	// always send plaintext.
+	// The Web UI / macOS tray edit forms work without seeing the real
+	// values: PATCH /api/v1/servers/{id} deep-merges (omitted keys are
+	// preserved, see `headers_remove` / `env_remove` for explicit
+	// deletes), so clients compute a diff and only send the keys that
+	// actually changed. Redacted-but-unchanged values never round-trip
+	// — the backend keeps the real string. Set this to true if a
+	// downstream tool genuinely needs raw values in the response.
 	RevealSecretHeaders bool `json:"reveal_secret_headers,omitempty" mapstructure:"reveal-secret-headers"`
 
 	// Server edition multi-user configuration (only meaningful with -tags server)

internal/httpapi/patch_server_test.go

@@ -129,3 +129,177 @@ func TestHandlePatchServer_ExplicitBoolsTakePrecedence(t *testing.T) {
 	assert.True(t, mockCtrl.capturedUpdates.ReconnectOnUse,
 		"ReconnectOnUse must be preserved from existing server (was true)")
 }
+
+// TestHandlePatchServer_HeadersDeepMerge verifies that PATCH /api/v1/servers
+// preserves existing header keys not mentioned in the request body. This is
+// the foundation of the Web UI / macOS tray edit flow: clients send a diff
+// against the redacted view of headers, so any key whose value is
+// `***REDACTED***` and unchanged stays out of the patch — and the backend
+// must NOT wipe it.
+func TestHandlePatchServer_HeadersDeepMerge(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+	mockCtrl := &mockPatchServerController{
+		apiKey: "test-key",
+		existingServer: &config.ServerConfig{
+			Name:     "synapbus",
+			URL:      "https://example.com/mcp",
+			Protocol: "streamable-http",
+			Enabled:  true,
+			Headers: map[string]string{
+				"Authorization": "Bearer real-secret-token",
+				"X-Trace":       "on",
+			},
+		},
+	}
+	srv := NewServer(mockCtrl, logger, nil)
+
+	// Client sends just X-New-Header. Authorization is omitted because
+	// its redacted view (`***REDACTED***`) matched the original, and
+	// X-Trace is omitted because the user didn't touch it.
+	body, _ := json.Marshal(map[string]any{
+		"headers": map[string]string{"X-New-Header": "new-value"},
+	})
+	req := httptest.NewRequest(http.MethodPatch, "/api/v1/servers/synapbus", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("X-API-Key", "test-key")
+	w := httptest.NewRecorder()
+	srv.ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+	require.NotNil(t, mockCtrl.capturedUpdates)
+
+	got := mockCtrl.capturedUpdates.Headers
+	assert.Equal(t, "Bearer real-secret-token", got["Authorization"],
+		"Authorization must be preserved verbatim — it was not in the PATCH body and the real secret must not be wiped")
+	assert.Equal(t, "on", got["X-Trace"], "X-Trace must be preserved")
+	assert.Equal(t, "new-value", got["X-New-Header"], "X-New-Header must be added")
+	assert.Len(t, got, 3, "exactly 3 headers expected (Authorization, X-Trace, X-New-Header)")
+}
+
+// TestHandlePatchServer_HeadersRemove verifies that the `headers_remove`
+// field deletes the listed keys from the stored map. Combined with the
+// merge behaviour above, this is how clients delete a header through PATCH.
+func TestHandlePatchServer_HeadersRemove(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+	mockCtrl := &mockPatchServerController{
+		apiKey: "test-key",
+		existingServer: &config.ServerConfig{
+			Name:     "synapbus",
+			Protocol: "http",
+			URL:      "https://example.com/mcp",
+			Enabled:  true,
+			Headers: map[string]string{
+				"Authorization": "Bearer token",
+				"X-Trace":       "on",
+				"X-Old":         "stale",
+			},
+		},
+	}
+	srv := NewServer(mockCtrl, logger, nil)
+
+	body, _ := json.Marshal(map[string]any{
+		"headers_remove": []string{"X-Old", "X-Trace"},
+	})
+	req := httptest.NewRequest(http.MethodPatch, "/api/v1/servers/synapbus", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("X-API-Key", "test-key")
+	w := httptest.NewRecorder()
+	srv.ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+	require.NotNil(t, mockCtrl.capturedUpdates)
+
+	got := mockCtrl.capturedUpdates.Headers
+	assert.Equal(t, "Bearer token", got["Authorization"], "Authorization untouched")
+	assert.Len(t, got, 1, "only Authorization should remain (X-Old and X-Trace removed)")
+	_, hasOld := got["X-Old"]
+	_, hasTrace := got["X-Trace"]
+	assert.False(t, hasOld, "X-Old must be removed")
+	assert.False(t, hasTrace, "X-Trace must be removed")
+}
+
+// TestHandlePatchServer_HeadersSetAndRemove combines the merge + remove
+// operations in a single PATCH. The same body shape the Web UI and macOS
+// tray send when the user simultaneously edits one header and deletes
+// another.
+func TestHandlePatchServer_HeadersSetAndRemove(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+	mockCtrl := &mockPatchServerController{
+		apiKey: "test-key",
+		existingServer: &config.ServerConfig{
+			Name:     "synapbus",
+			Protocol: "http",
+			URL:      "https://example.com/mcp",
+			Enabled:  true,
+			Headers: map[string]string{
+				"Authorization": "Bearer old-token",
+				"X-Trace":       "on",
+			},
+		},
+	}
+	srv := NewServer(mockCtrl, logger, nil)
+
+	body, _ := json.Marshal(map[string]any{
+		"headers": map[string]string{
+			"Authorization": "Bearer new-token",
+			"X-New":         "new-value",
+		},
+		"headers_remove": []string{"X-Trace"},
+	})
+	req := httptest.NewRequest(http.MethodPatch, "/api/v1/servers/synapbus", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("X-API-Key", "test-key")
+	w := httptest.NewRecorder()
+	srv.ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+	require.NotNil(t, mockCtrl.capturedUpdates)
+
+	got := mockCtrl.capturedUpdates.Headers
+	assert.Equal(t, "Bearer new-token", got["Authorization"], "Authorization updated to new value")
+	assert.Equal(t, "new-value", got["X-New"], "X-New added")
+	_, hasTrace := got["X-Trace"]
+	assert.False(t, hasTrace, "X-Trace deleted")
+	assert.Len(t, got, 2)
+}
+
+// TestHandlePatchServer_EnvDeepMerge mirrors HeadersDeepMerge for env vars.
+// The redaction risk is the same — backend doesn't redact env values today,
+// but the merge semantics are needed for symmetric UI editing.
+func TestHandlePatchServer_EnvDeepMerge(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+	mockCtrl := &mockPatchServerController{
+		apiKey: "test-key",
+		existingServer: &config.ServerConfig{
+			Name:     "demo",
+			Protocol: "stdio",
+			Command:  "uvx",
+			Enabled:  true,
+			Env: map[string]string{
+				"API_KEY":   "live-secret",
+				"LOG_LEVEL": "debug",
+			},
+		},
+	}
+	srv := NewServer(mockCtrl, logger, nil)
+
+	body, _ := json.Marshal(map[string]any{
+		"env":        map[string]string{"NEW_VAR": "value"},
+		"env_remove": []string{"LOG_LEVEL"},
+	})
+	req := httptest.NewRequest(http.MethodPatch, "/api/v1/servers/demo", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("X-API-Key", "test-key")
+	w := httptest.NewRecorder()
+	srv.ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+	require.NotNil(t, mockCtrl.capturedUpdates)
+
+	got := mockCtrl.capturedUpdates.Env
+	assert.Equal(t, "live-secret", got["API_KEY"], "API_KEY preserved (not in patch body)")
+	assert.Equal(t, "value", got["NEW_VAR"], "NEW_VAR added")
+	_, hasLog := got["LOG_LEVEL"]
+	assert.False(t, hasLog, "LOG_LEVEL deleted via env_remove")
+	assert.Len(t, got, 2)
+}