Skip to content

Commit 20fe758

Browse files
committed
feat(headers): stop redacting REST + SSE responses; scope reveal_secret_headers to MCP tool only
User feedback on the previous round: the Server Detail edit form refused to save when any header value still contained the `***REDACTED***` sentinel, and pushed users at the `reveal_secret_headers: true` config flag to unblock themselves. That trade-off was wrong — it punished the human UI to protect an agent code path. The actual threat model: - REST API (`/api/v1/servers`, SSE `/events`): gated by the local per-install API key. Same trust boundary as access to `~/.mcpproxy/mcp_config.json` on disk where these values are already stored in plaintext. Redacting in the response bought no real security, only broke the Web UI + macOS tray edit-and-save round-trip. - MCP `upstream_servers` tool: invoked by AI agents, output gets read back to the LLM context. THIS is the agent-context exposure `reveal_secret_headers` was created to protect — and that redaction was already implemented separately in `internal/server/mcp.go:~2545`, unaffected by this change. Backend changes: - internal/httpapi/server.go: drop `redactServerHeaders` calls in `handleGetServers` (both code paths) and remove the now-unused method. - internal/runtime/event_bus.go: drop the `redactServerHeaders` call on SSE `servers.changed` payloads and remove the now-unused method. - internal/runtime/event_bus_payload_test.go: rewrite the redaction test to assert the new policy — SSE must now carry plaintext, the test name changes from `…_RedactsSensitiveHeaders` to `…_SendsPlaintextHeaders`. - internal/config/config.go: update the `RevealSecretHeaders` doc to explicitly scope the flag to the MCP tool. REST + SSE always send plaintext from now on. UI cleanup (the redaction sentinel is no longer expected on the wire): - macOS Swift `saveEdits()`: drop the elaborate redacted-save guard (refused save when ***REDACTED*** literal still in textarea OR a redacted key was deleted). Both cases became impossible once the REST API serves plaintext. - macOS Swift `kvRow()`: drop the `value != "***REDACTED***"` check that disabled the Convert-to-secret button. - macOS Swift `performConvertToSecret()`: drop the matching guard. - Vue `KVValueCell.vue`: drop the `isBackendRedacted` computed flag and the "Backend redacted this value, set reveal_secret_headers" tooltip. Reveal and Convert are always available on literal values. - Vue `ServerDetail.vue::commitConvert()`: drop the same guard. Verified end-to-end on macOS and Web UI: - REST: `curl /api/v1/servers` → `Authorization: Bearer 1d386f...` (the real 71-char token). - MCP tool: `upstream_servers list` → `"Authorization":"***REDACTED***"` (still hidden from agents). - macOS Server Detail → synapbus → Edit → Headers textarea pre-populated with the real Bearer token; Save no longer blocked. - Web UI synapbus Config → Headers row shows `••••59 (71 chars)` with reveal eye, Convert-to-secret 🔒, edit, delete — all functional. OAS spec regenerated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 57574d2 commit 20fe758

9 files changed

Lines changed: 78 additions & 145 deletions

File tree

frontend/src/components/KVValueCell.vue

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,22 @@
2727
<span class="font-mono text-xs">env var: {{ envRefName }}</span>
2828
</span>
2929
</template>
30-
<!-- Literal value: redacted by default, reveal button shows the raw
31-
string as returned by the API. If the backend redacted the value
32-
(i.e. `reveal_secret_headers: false`), the "raw" string will be
33-
`***REDACTED***` and revealing it is a no-op — we surface that
34-
explicitly so the user understands why nothing changes. -->
30+
<!-- Literal value: masked by default to avoid shoulder-surfing
31+
leaks; click the eye to reveal the actual string returned by
32+
the REST API. The REST API serves plaintext header / env
33+
values (the API key is the gate at this trust boundary), so
34+
reveal always shows the real thing. -->
3535
<template v-else>
3636
<code v-if="revealed" class="bg-base-200 px-1.5 py-0.5 rounded text-xs break-all">{{ rawValue }}</code>
3737
<code v-else class="bg-base-200 px-1.5 py-0.5 rounded text-xs text-base-content/50">{{ redactedPreview }}</code>
3838
<button
3939
class="btn btn-ghost btn-xs"
40-
:title="revealed ? 'Hide value' : (isBackendRedacted ? 'Backend redacted this value. Set reveal_secret_headers: true to view.' : 'Reveal value')"
40+
:title="revealed ? 'Hide value' : 'Reveal value'"
4141
@click="revealed ? $emit('hide') : $emit('reveal')"
4242
>
4343
<span aria-hidden="true">{{ revealed ? '🙈' : '👁' }}</span>
4444
</button>
4545
<button
46-
v-if="!isBackendRedacted"
4746
class="btn btn-ghost btn-xs"
4847
title="Move value into the OS keyring and reference it as ${keyring:name}"
4948
@click="$emit('convert')"
@@ -115,12 +114,6 @@ const envRefName = computed(() => {
115114
const m = (props.rawValue ?? '').match(/^\$\{env:([^}]+)\}$/)
116115
return m ? m[1] : ''
117116
})
118-
// The Go backend redacts secret header values via
119-
// internal/httpapi/server.go:redactServerHeaders. Detect that sentinel so
120-
// we can disable affordances (reveal, convert) that depend on having the
121-
// real value in hand.
122-
const isBackendRedacted = computed(() => props.rawValue === '***REDACTED***')
123-
124117
const redactedPreview = computed(() => {
125118
const v = props.rawValue ?? ''
126119
if (!v) return '(empty)'

frontend/src/views/ServerDetail.vue

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,14 +2106,6 @@ function closeConvertModal() {
21062106
async function commitConvert() {
21072107
const m = convertModal.value
21082108
if (!m.secretName || !m.rawValue) return
2109-
if (m.rawValue === '***REDACTED***') {
2110-
systemStore.addToast({
2111-
type: 'error',
2112-
title: 'Cannot convert',
2113-
message: 'Backend is redacting this value (reveal_secret_headers: false). The literal cannot be read from this side; edit the value first or enable reveal_secret_headers in your config.',
2114-
})
2115-
return
2116-
}
21172109
m.busy = true
21182110
try {
21192111
const storeResp = await api.storeSecret(m.secretName, m.rawValue)

internal/config/config.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,20 @@ type Config struct {
163163
// Security scanner settings (Spec 039)
164164
Security *SecurityConfig `json:"security,omitempty" mapstructure:"security"`
165165

166-
// RevealSecretHeaders, when true, disables the redaction of sensitive
167-
// header values (Authorization, X-API-Key, Cookie, …) in responses from
168-
// the `upstream_servers` MCP tool and the `/api/v1/servers` REST API.
169-
// Default false — sensitive header values are surfaced as
170-
// `***REDACTED***` so an MCP agent cannot read Bearer tokens / API keys
171-
// out of another upstream's config. Set to true if a downstream tool
172-
// needs the raw values (e.g. a UI that round-trips PUT replaces).
166+
// RevealSecretHeaders controls whether the `upstream_servers` MCP
167+
// tool returns sensitive header values (Authorization, X-API-Key,
168+
// Cookie, …) in plaintext or redacted to `***REDACTED***`.
169+
//
170+
// Default false — an MCP agent calling `upstream_servers list` sees
171+
// `***REDACTED***` so it cannot read another upstream's Bearer token
172+
// out of the config and exfiltrate it back to the LLM context.
173+
//
174+
// This flag DOES NOT affect the REST API (`/api/v1/servers`) or the
175+
// SSE event stream. Those paths are gated by the local API key —
176+
// the same trust boundary as access to `~/.mcpproxy/mcp_config.json`
177+
// on disk — so redacting there bought no real security while
178+
// breaking the Web UI / macOS tray edit-and-save round-trip. They
179+
// always send plaintext.
173180
RevealSecretHeaders bool `json:"reveal_secret_headers,omitempty" mapstructure:"reveal-secret-headers"`
174181

175182
// Server edition multi-user configuration (only meaningful with -tags server)

internal/httpapi/server.go

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
2323
"github.com/smart-mcp-proxy/mcpproxy-go/internal/logs"
2424
"github.com/smart-mcp-proxy/mcpproxy-go/internal/management"
25-
"github.com/smart-mcp-proxy/mcpproxy-go/internal/oauth"
2625
"github.com/smart-mcp-proxy/mcpproxy-go/internal/observability"
2726
"github.com/smart-mcp-proxy/mcpproxy-go/internal/reqcontext"
2827
internalRuntime "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime"
@@ -1057,11 +1056,15 @@ func (s *Server) handleGetServers(w http.ResponseWriter, r *http.Request) {
10571056
}
10581057
}
10591058

1060-
// Redact sensitive header values unless explicitly opted out via
1061-
// `reveal_secret_headers: true` in config. See config.Config field
1062-
// for rationale — the same redaction is applied to the
1063-
// `upstream_servers` MCP tool's list output.
1064-
s.redactServerHeaders(serverValues)
1059+
// NOTE: header values flow through the REST API in plaintext.
1060+
// The REST API is gated by the local API key — the same trust
1061+
// boundary as access to `~/.mcpproxy/mcp_config.json` on disk
1062+
// where these values are persisted. Redacting in the response
1063+
// bought no real security, only broke edit-and-save round-trips
1064+
// for the Web UI and macOS tray. Redaction is still applied at
1065+
// the `upstream_servers` MCP tool layer (mcp.go:~2545), which
1066+
// IS the agent-context exposure that motivated
1067+
// `reveal_secret_headers`.
10651068

10661069
// Dereference stats pointer
10671070
var statsValue contracts.ServerStats
@@ -1091,8 +1094,8 @@ func (s *Server) handleGetServers(w http.ResponseWriter, r *http.Request) {
10911094
// Enrich with quarantine stats
10921095
s.enrichServersWithQuarantineStats(servers)
10931096

1094-
// See note above the management-service path for rationale.
1095-
s.redactServerHeaders(servers)
1097+
// See note above the management-service path: REST responses now
1098+
// send plaintext; the MCP `upstream_servers` tool still redacts.
10961099

10971100
stats := contracts.ConvertUpstreamStatsToServerStats(s.controller.GetUpstreamStats())
10981101

@@ -1104,22 +1107,6 @@ func (s *Server) handleGetServers(w http.ResponseWriter, r *http.Request) {
11041107
s.writeSuccess(w, response)
11051108
}
11061109

1107-
// redactServerHeaders walks each server in the slice and replaces sensitive
1108-
// header values (Authorization, X-API-Key, Cookie, etc.) with `***REDACTED***`.
1109-
// Skips redaction entirely when `reveal_secret_headers: true` is set in the
1110-
// loaded config, matching the behaviour of the `upstream_servers` MCP tool.
1111-
func (s *Server) redactServerHeaders(servers []contracts.Server) {
1112-
cfg, err := s.controller.GetConfig()
1113-
if err == nil && cfg != nil && cfg.RevealSecretHeaders {
1114-
return
1115-
}
1116-
for i := range servers {
1117-
if len(servers[i].Headers) > 0 {
1118-
servers[i].Headers = oauth.RedactStringHeaders(servers[i].Headers)
1119-
}
1120-
}
1121-
}
1122-
11231110
// enrichServersWithQuarantineStats adds quarantine metrics (pending/changed tool counts)
11241111
// to each server in the list. This enables the frontend to show quarantine badges.
11251112
func (s *Server) enrichServersWithQuarantineStats(servers []contracts.Server) {

internal/runtime/event_bus.go

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"go.uber.org/zap"
1111

1212
"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
13-
"github.com/smart-mcp-proxy/mcpproxy-go/internal/oauth"
1413
"github.com/smart-mcp-proxy/mcpproxy-go/internal/telemetry"
1514
)
1615

@@ -174,14 +173,19 @@ func (r *Runtime) emitServersChanged(reason string, extra map[string]any) {
174173
zap.String("reason", reason),
175174
zap.Error(err))
176175
} else {
177-
redacted := make([]contracts.Server, 0, len(servers))
176+
// SSE subscribers ride the same trust boundary as the REST
177+
// API (API-key gated, local-host only). Send plaintext
178+
// header values so the Web UI / macOS tray can round-trip
179+
// edits. The MCP `upstream_servers` tool applies its own
180+
// redaction for the agent-context exposure that motivated
181+
// reveal_secret_headers.
182+
serverList := make([]contracts.Server, 0, len(servers))
178183
for _, s := range servers {
179184
if s != nil {
180-
redacted = append(redacted, *s)
185+
serverList = append(serverList, *s)
181186
}
182187
}
183-
r.redactServerHeaders(redacted)
184-
payload["servers"] = redacted
188+
payload["servers"] = serverList
185189
payload["stats"] = stats
186190
}
187191
}
@@ -194,23 +198,6 @@ func (r *Runtime) emitServersChanged(reason string, extra map[string]any) {
194198
r.publishEvent(evt)
195199
}
196200

197-
// redactServerHeaders mirrors httpapi.(*Server).redactServerHeaders. It strips
198-
// sensitive header values (Authorization, Cookie, X-API-Key, ...) unless the
199-
// loaded config opts out via reveal_secret_headers: true. Centralizing this in
200-
// the runtime keeps SSE subscribers behind the same trust boundary as the
201-
// HTTP API.
202-
func (r *Runtime) redactServerHeaders(servers []contracts.Server) {
203-
cfg := r.Config()
204-
if cfg != nil && cfg.RevealSecretHeaders {
205-
return
206-
}
207-
for i := range servers {
208-
if len(servers[i].Headers) > 0 {
209-
servers[i].Headers = oauth.RedactStringHeaders(servers[i].Headers)
210-
}
211-
}
212-
}
213-
214201
func (r *Runtime) emitConfigReloaded(path string) {
215202
payload := map[string]any{"path": path}
216203
r.publishEvent(newEvent(EventTypeConfigReloaded, payload))

internal/runtime/event_bus_payload_test.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,12 @@ func TestEmitServersChanged_NoListerStillPublishesNotifyOnly(t *testing.T) {
120120
assert.False(t, hasServers, "servers key should be absent without a configured lister")
121121
}
122122

123-
func TestEmitServersChanged_RedactsSensitiveHeaders(t *testing.T) {
123+
// SSE rides the same trust boundary as the REST API (API-key gated, local
124+
// only). Header values must come through in plaintext so Web UI / macOS
125+
// tray clients can round-trip edits. The `upstream_servers` MCP tool keeps
126+
// its own redaction for the agent-context path — that exposure is the
127+
// one `reveal_secret_headers` was created to protect.
128+
func TestEmitServersChanged_SendsPlaintextHeaders(t *testing.T) {
124129
servers := []*contracts.Server{
125130
{
126131
Name: "alpha",
@@ -133,22 +138,23 @@ func TestEmitServersChanged_RedactsSensitiveHeaders(t *testing.T) {
133138
stats := &contracts.ServerStats{TotalServers: 1}
134139

135140
rt := newPayloadTestRuntime(t, &fakeServersLister{servers: servers, stats: stats})
136-
// Default config (RevealSecretHeaders=false). Wire a minimal config service
137-
// snapshot so r.Config() returns non-nil.
141+
// Default config (RevealSecretHeaders=false). The new policy is that
142+
// REST + SSE responses do NOT redact, only the MCP tool does, so the
143+
// SSE payload should carry plaintext regardless of the flag.
138144
rt.cfg = &config.Config{}
139145
ch := rt.SubscribeEvents()
140146
defer rt.UnsubscribeEvents(ch)
141147

142-
rt.emitServersChanged("redact-check", nil)
148+
rt.emitServersChanged("plaintext-check", nil)
143149

144150
evt := receiveServersChanged(t, ch)
145151
gotServers, ok := evt.Payload["servers"].([]contracts.Server)
146152
require.True(t, ok)
147153
require.Len(t, gotServers, 1)
148154

149-
authVal := gotServers[0].Headers["Authorization"]
150-
contentVal := gotServers[0].Headers["Content-Type"]
151-
assert.NotEqual(t, "Bearer secret-token-value", authVal, "Authorization header must be redacted")
152-
assert.Contains(t, authVal, "REDACTED")
153-
assert.Equal(t, "application/json", contentVal, "Content-Type is not sensitive; must not be redacted")
155+
assert.Equal(t, "Bearer secret-token-value", gotServers[0].Headers["Authorization"],
156+
"Authorization must be plaintext over SSE — the API key already gates this endpoint")
157+
assert.NotContains(t, gotServers[0].Headers["Authorization"], "REDACTED",
158+
"the redaction sentinel must not appear in SSE payloads")
159+
assert.Equal(t, "application/json", gotServers[0].Headers["Content-Type"])
154160
}

native/macos/MCPProxy/MCPProxy/Views/ServerDetailView.swift

Lines changed: 8 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,9 @@ struct ServerDetailView: View {
5454
@State private var editWorkingDir = ""
5555
@State private var editEnvVars = ""
5656
/// HTTP servers only — KEY=VALUE per line. Pre-populated from
57-
/// `server.headers` on enter-edit. Sensitive values may show as
58-
/// `***REDACTED***` if the backend redacted them; saving redacted
59-
/// content unchanged is a no-op (the PATCH endpoint preserves whatever
60-
/// arrives but a `***REDACTED***` literal is meaningless to upstream),
61-
/// so users editing redacted headers should either edit the value or
62-
/// enable `reveal_secret_headers: true` in their config first.
57+
/// `server.headers` on enter-edit. The REST API now serves plaintext
58+
/// header values (the API key gates that endpoint), so the textarea
59+
/// always contains the real strings the user can edit and save back.
6360
@State private var editHeaders = ""
6461
@State private var editEnabled = true
6562
@State private var editQuarantined = false
@@ -1046,47 +1043,11 @@ struct ServerDetailView: View {
10461043
// Parse headers (HTTP / streamable-http only). Same all-or-nothing
10471044
// semantics as env: we always send the parsed map so deletes
10481045
// propagate. The backend handler treats `headers: {}` as "clear",
1049-
// not "ignore".
1046+
// not "ignore". The REST API now serves plaintext header values
1047+
// (the API key gates this endpoint), so the textarea is always
1048+
// pre-populated with the real values and a straight save is safe.
10501049
if server.protocol == "http" || server.protocol == "sse" || server.protocol == "streamable-http" {
1051-
let headers = parseKVTextarea(editHeaders)
1052-
1053-
// Defense against two distinct destructive edits when the
1054-
// backend has redacted secret values (reveal_secret_headers
1055-
// is off):
1056-
//
1057-
// 1. The user leaves a `***REDACTED***` literal in the
1058-
// textarea and clicks Save — we'd persist the literal
1059-
// sentinel as the new value, breaking the upstream.
1060-
//
1061-
// 2. The user *removes* a redacted line (perhaps to add a
1062-
// new header alongside it) without realising they're
1063-
// also wiping out the real secret behind the redaction.
1064-
// The new map has no `***REDACTED***` so guard #1 misses
1065-
// it, but the original key has silently vanished and the
1066-
// real Authorization header is gone from the saved config.
1067-
//
1068-
// Compare against the original `server.headers` keys whose
1069-
// value is the redaction sentinel and refuse the save if any
1070-
// of them are still ***REDACTED*** OR have been dropped
1071-
// entirely. Either way the user needs to enable
1072-
// reveal_secret_headers in their config first so they can
1073-
// see and explicitly carry the value forward (or use the
1074-
// Web UI's per-row affordances).
1075-
let redactedKeysInOriginal: Set<String> = Set(
1076-
(server.headers ?? [:])
1077-
.filter { $0.value == "***REDACTED***" }
1078-
.map { $0.key }
1079-
)
1080-
let stillRedacted = headers.filter { $0.value == "***REDACTED***" }.map { $0.key }
1081-
let droppedFromOriginal = redactedKeysInOriginal.subtracting(headers.keys)
1082-
1083-
if !stillRedacted.isEmpty || !droppedFromOriginal.isEmpty {
1084-
let offenders = (stillRedacted + Array(droppedFromOriginal)).sorted().joined(separator: ", ")
1085-
editError = "Cannot save: the backend redacted these headers and the real values are not visible here: \(offenders). Set `reveal_secret_headers: true` in your config to view + edit them, then try again."
1086-
isSavingEdit = false
1087-
return
1088-
}
1089-
updates["headers"] = headers
1050+
updates["headers"] = parseKVTextarea(editHeaders)
10901051
}
10911052

10921053
// Boolean toggles
@@ -1259,13 +1220,6 @@ struct ServerDetailView: View {
12591220

12601221
private func performConvertToSecret(_ ctx: ConvertToSecretContext) async {
12611222
guard let client = apiClient else { return }
1262-
// Refuse to convert if the value is the backend redaction
1263-
// sentinel — that means the real value is not in our hands and
1264-
// would be uploaded literally to the keyring.
1265-
if ctx.value == "***REDACTED***" {
1266-
convertSheetError = "Cannot convert a redacted value. Set `reveal_secret_headers: true` in your config to view + convert."
1267-
return
1268-
}
12691223
let name = convertSheetSecretName.trimmingCharacters(in: .whitespaces)
12701224
if name.isEmpty { return }
12711225
convertSheetBusy = true
@@ -1322,7 +1276,7 @@ struct ServerDetailView: View {
13221276
.font(.scaledMonospaced(.subheadline, scale: fontScale))
13231277
.textSelection(.enabled)
13241278

1325-
if value != "***REDACTED***" && !value.isEmpty {
1279+
if !value.isEmpty {
13261280
Button {
13271281
openConvertSheet(scope: scope, key: key, value: value)
13281282
} label: {

oas/docs.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

oas/swagger.yaml

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,20 @@ components:
108108
type: boolean
109109
reveal_secret_headers:
110110
description: |-
111-
RevealSecretHeaders, when true, disables the redaction of sensitive
112-
header values (Authorization, X-API-Key, Cookie, …) in responses from
113-
the `upstream_servers` MCP tool and the `/api/v1/servers` REST API.
114-
Default false — sensitive header values are surfaced as
115-
`***REDACTED***` so an MCP agent cannot read Bearer tokens / API keys
116-
out of another upstream's config. Set to true if a downstream tool
117-
needs the raw values (e.g. a UI that round-trips PUT replaces).
111+
RevealSecretHeaders controls whether the `upstream_servers` MCP
112+
tool returns sensitive header values (Authorization, X-API-Key,
113+
Cookie, …) in plaintext or redacted to `***REDACTED***`.
114+
115+
Default false — an MCP agent calling `upstream_servers list` sees
116+
`***REDACTED***` so it cannot read another upstream's Bearer token
117+
out of the config and exfiltrate it back to the LLM context.
118+
119+
This flag DOES NOT affect the REST API (`/api/v1/servers`) or the
120+
SSE event stream. Those paths are gated by the local API key —
121+
the same trust boundary as access to `~/.mcpproxy/mcp_config.json`
122+
on disk — so redacting there bought no real security while
123+
breaking the Web UI / macOS tray edit-and-save round-trip. They
124+
always send plaintext.
118125
type: boolean
119126
routing_mode:
120127
description: |-

0 commit comments

Comments
 (0)