Skip to content

Commit 6702e62

Browse files
Dumbrisclaude
andauthored
feat(connect): preview the exact change before writing + stop leaking the API key into client configs (Spec 078 US1) (#802)
* feat(connect): preview endpoint sharing buildServerEntry with the write path First slice of specs/078-connect-trust-preview US1 (see spec FR-001..004, 012, 013). Adds GET /api/v1/connect/{client}/preview returning the exact change a subsequent connect would make — target config_path, server_key, entry name, and the exact entry contents — WITHOUT modifying the file or creating a backup. Preview==write guarantee: buildServerEntry is now the single construction path for every client, including Codex/TOML (connectTOML previously built its {url} entry inline). Preview derives its entry from the same function the write uses, so what is previewed equals what is written (FR-002); a table-driven test connects each supported client and asserts the written file entry deep-equals the shared constructor output across JSON and TOML. API-key honesty (FR-004): the embedded apikey is masked in the payload (entry, entry_text) while contains_api_key flags that a credential is written; the real key never appears in the preview (verified by test that marshals the whole payload and asserts the secret is absent). Spec 075 access states (FR-012): the on-demand read used to classify create-vs-overwrite degrades to access_state=absent|malformed and a denial returns the same typed AccessError (403 + remediation) as connect/disconnect. - internal/connect/preview.go: Service.Preview, maskURLAPIKey, entry render - internal/connect/clients.go: buildServerEntry gains the codex case - internal/connect/connect.go: connectTOML uses buildServerEntry - internal/httpapi: handler + route + tests (masked, no side effects, 403) - oas: regenerated for the new endpoint (make swagger) Tests: preview-equals-write (JSON+TOML), masking, entry_exists (create vs overwrite), no-side-effects (no write, no backup, mtime unchanged), absent/ malformed/denied degradation. go test -race ./internal/connect/... and ./internal/httpapi/... pass; golangci-lint v2 clean. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): preview the exact change before writing in wizard + Connect modal Second slice of specs/078-connect-trust-preview US1 (FR-001,003,004): the Connect action in BOTH the standalone Connect modal and the onboarding wizard's ClientRow now fetches the preview first and shows an inline panel before any file is written: - target config path and, in a monospace additive ("+ will be added") block, the exact entry that will be written (pretty JSON, or TOML for Codex) with the API key masked; - plain-language trust copy ("Only this entry is added — everything else in the file stays untouched. A timestamped backup is created first."); - a masked-key line when contains_api_key so the user knows a credential is written; - an overwrite warning when an entry with the same name already exists, in which case confirming implies force=true; - graceful copy for the bridge/no-prior-file (access_state=absent) and unparseable (malformed) cases. Confirm proceeds with the connect (reusing the existing #799 post-connect backup-path line, untouched); Cancel dismisses the panel and writes nothing. A non-denial preview fetch error is surfaced inline; in the modal a denied read still resolves the Spec 075 access banner via checkAccess. Wizard connect-step telemetry (markConnectCompleted/Skipped) is unchanged. - types/api.ts: ConnectPreview; services/api.ts: getConnectPreview - ConnectModal.vue + OnboardingWizard.vue ClientRow: preview panel + flow - The existing #799 connect specs are updated to drive click → preview → confirm (Connect no longer writes on the first click); new connect-preview.spec.ts covers preview render, cancel-writes-nothing, confirm-proceeds, masked key, and overwrite→force for both surfaces. Full frontend suite: 33 files, 244 tests pass; vue-tsc clean; make build embeds the updated dist. npm-ci lockfile churn reverted (as in #799). Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(connect): stop leaking the API key into client configs; auth-aware carrier Security fix folded into Spec 078. Connect (and the new preview) previously appended ?apikey=<REST-admin key> to EVERY client entry via mcpURL(), but /mcp does not require auth by default (config.RequireMCPAuth defaults false; the admin context is granted unauthenticated). So connect leaked the REST-admin API key in plaintext into other apps' config files where it served no purpose. New behavior, threaded from config.RequireMCPAuth into connect.Service the same way listenAddr/apiKey are (server wiring + both CLI sites), via WithRequireMCPAuth: - require_mcp_auth=false (default): the entry carries NO credential — a clean http://host:port/mcp URL, no query, no headers. Existing configs keep working because /mcp accepts unauthenticated requests when protection is off. - require_mcp_auth=true: the credential is written via the carrier each client's real config schema supports (verified against vendor docs): * X-API-Key header — claude-code, vscode, cursor, windsurf, gemini, opencode (all support a "headers" object on their remote entry); * mcp-remote --header "X-API-Key:<key>" bridge arg — claude-desktop (no space after the colon: Claude Desktop/Cursor don't escape spaces in args and mangle the header — geelen/mcp-remote README; our keys are space-free); * ?apikey= query fallback — codex only, whose TOML HTTP transport takes header values indirectly via env-var names (http_headers / bearer_token_env_var) and cannot express a literal header value. Server-side ExtractToken accepts X-API-Key, Authorization: Bearer, and ?apikey= (in that order), so header and query authenticate identically. buildServerEntry now takes serverEntryParams{baseURL, credential}; mcpURL() is replaced by the credential-free baseURL() anchor. Entry MATCHING (findEntry*/findEquivalentJSONServerName/entryPointsToBridge) already keys on baseURL with an optional ?query, so it recognizes BOTH legacy ?apikey= entries and new clean entries — reconnect adopts/updates in place (no duplicate) and disconnect still finds legacy entries; a force-reconnect over a legacy entry upgrades it and drops the leaked key. Preview reflects reality: contains_api_key is true only when require_mcp_auth is on; the credential is masked in whichever carrier it uses (header value, bridge arg, or query) and the real key never appears in the preview payload. Tests: matrix over require_mcp_auth on/off × all 8 clients for both write and preview (preview==write shape, auth-off writes contain NO apikey/headers at all); per-client carrier assertions; legacy ?apikey= migration (recognized, upgraded in place, no duplicate, disconnect still works). go test -race ./internal/connect/... ./internal/httpapi/... pass; server/config/management pass; golangci-lint v2 clean. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): carrier-agnostic credential copy in the connect preview Follows the connect security fix: the preview panel's masked-credential line now reads "This entry includes your API key (shown masked)…" instead of "The URL includes…", since with require_mcp_auth on the credential is carried in an X-API-Key header (or the mcp-remote --header bridge arg) for most clients and only falls back to the ?apikey= URL query for Codex. The line already renders only when contains_api_key is true, which the backend now sets only when require_mcp_auth is on — so the default (keyless) connect shows no credential line at all. Adds a frontend test for the default keyless case: contains_api_key=false ⇒ no credential line, and the previewed entry shows neither an apikey query nor a mask. Full frontend suite: 33 files, 245 tests pass; vue-tsc clean. Related #793 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(connect): live-config carrier + honest preview for edge cases Codex review of the Spec 078 US1 connect-preview branch surfaced one blocker and three preview/write-divergence gaps; this addresses them: - blocker: the HTTP connect service snapshotted require_mcp_auth (and listen/api_key) once at startup and was never rebuilt, while the /mcp auth middleware honors require_mcp_auth live per-request. Toggling auth off at runtime (wizard toggle or hot-reload) left the service embedding the real API key into client configs — the exact leak this branch closes. Add a live config-provider seam (WithConfigProvider) wired to runtime.Config() so preview and write always match the currently-enforced auth state. - OpenCode preview divergence: previewEntryExists checked only the exact server name, but OpenCode's write path adopts/normalizes an equivalent entry (incl. the legacy ?apikey= shape) under a different key. Preview now mirrors that via findEquivalentJSONServerName so entry_exists reflects the overwrite. - preview endpoint ignored server_name and always previewed "mcpproxy" while POST connect accepts a custom name. Honor an optional ?server_name= query so a caller previewing then connecting under a custom name does not diverge. - malformed config: the write parses the same bytes and would fail, but the preview copy promised it "still writes only the entry" and left Connect enabled. Make the copy honest and disable Connect for malformed configs. Tests: live-toggle credential (auth off->on without rebuild), OpenCode equivalent-entry preview, server_name honoring, and malformed confirm-disabled. go test -race ./internal/connect/... ./internal/httpapi/... green; frontend 246 tests + vue-tsc clean; golangci-lint v2 ./... 0 issues; swagger regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): make the first connect click explicitly commitment-free The preview only appeared AFTER clicking "Connect" — the exact button hesitant users avoid, fearing the click immediately modifies their config. Rename the row action to "Review & connect" and say up front in the modal subtitle and wizard intro that nothing is written until the confirm step, so the safety of the first click is visible before it happens (Spec 078 US1 trust copy). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(web): align setup.md and test prose with the Review & connect flow Codex review follow-up: docs/setup.md described the Claude Desktop bridge registration as one-click "Connect"; it is now review-then- confirm. Test comments updated to match the renamed row action. Spec 046/075 texts intentionally left untouched — they are point-in- time records (046 FR-005 already required the diff preview this branch implements). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent b2003ba commit 6702e62

21 files changed

Lines changed: 2000 additions & 158 deletions

cmd/mcpproxy/connect_cmd.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func runConnect(cmd *cobra.Command, args []string) error {
7676
return fmt.Errorf("failed to load config: %w", err)
7777
}
7878

79-
svc := connect.NewService(cfg.Listen, cfg.APIKey)
79+
svc := connect.NewService(cfg.Listen, cfg.APIKey).WithRequireMCPAuth(cfg.RequireMCPAuth)
8080

8181
format := clioutput.ResolveFormat(globalOutputFormat, globalJSONOutput)
8282
formatter, err := clioutput.NewFormatter(format)
@@ -114,7 +114,7 @@ func runDisconnect(cmd *cobra.Command, args []string) error {
114114
return fmt.Errorf("failed to load config: %w", err)
115115
}
116116

117-
svc := connect.NewService(cfg.Listen, cfg.APIKey)
117+
svc := connect.NewService(cfg.Listen, cfg.APIKey).WithRequireMCPAuth(cfg.RequireMCPAuth)
118118

119119
format := clioutput.ResolveFormat(globalOutputFormat, globalJSONOutput)
120120
formatter, err := clioutput.NewFormatter(format)

docs/setup.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ Add mcpproxy as a remote MCP server via Settings → Connectors → Add Custom C
254254

255255
#### Option A: Free Plan — JSON Configuration
256256

257-
> **💡 One-click:** mcpproxy's built-in **Connect** wizard (Web UI / tray) can write this bridge configuration for you automatically — pick **Claude Desktop** and click **Connect**. It registers the `npx -y mcp-remote` bridge shown below (Node.js required). The manual steps remain available if you prefer to edit the file yourself.
257+
> **💡 Built-in wizard:** mcpproxy's **Connect** wizard (Web UI / tray) can write this bridge configuration for you — pick **Claude Desktop**, click **Review & connect** to see the exact entry that will be written (a timestamped backup is created first), then confirm with **Connect**. It registers the `npx -y mcp-remote` bridge shown below (Node.js required). The manual steps remain available if you prefer to edit the file yourself.
258258
259259
1. Create the config file if it doesn't exist:
260260

frontend/src/components/ConnectModal.vue

Lines changed: 146 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<div class="modal-box max-w-lg">
44
<h3 class="font-bold text-lg mb-2">Connect MCPProxy to AI Agents</h3>
55
<p class="text-sm opacity-70 mb-4">
6-
Register MCPProxy as an MCP server in your AI tools. This modifies the tool's config file (backup created automatically).
6+
Register MCPProxy as an MCP server in your AI tools. Clicking a client shows the exact change first — nothing is written until you confirm (backup created automatically).
77
</p>
88

99
<!-- Loading state -->
@@ -69,12 +69,13 @@
6969
</button>
7070
<button
7171
v-else
72-
@click="connectSingle(client.id)"
72+
:data-test="`connect-start-${client.id}`"
73+
@click="startConnect(client.id)"
7374
class="btn btn-primary btn-xs"
74-
:disabled="loading.clients[client.id]"
75+
:disabled="loading.clients[client.id] || previewLoading[client.id]"
7576
>
76-
<span v-if="loading.clients[client.id]" class="loading loading-spinner loading-xs"></span>
77-
<span v-else>Connect</span>
77+
<span v-if="loading.clients[client.id] || previewLoading[client.id]" class="loading loading-spinner loading-xs"></span>
78+
<span v-else>Review & connect</span>
7879
</button>
7980
<!-- Spec 075 US1: explicit, no-eager-read access check. The stat-only
8081
listing leaves installed clients 'unknown'; this is the only
@@ -117,6 +118,88 @@
117118
</button>
118119
</div>
119120
</div>
121+
<!-- Spec 078 US1 / FR-001,003,004: preview the exact change before it
122+
is written. Only this entry is added; nothing else in the file
123+
changes. Confirm/Cancel gate the actual write. -->
124+
<div
125+
v-if="previews[client.id]"
126+
:data-test="`connect-preview-${client.id}`"
127+
class="border-t border-base-300 bg-base-200/40 px-3 py-3 space-y-2"
128+
>
129+
<p class="text-xs opacity-70 leading-relaxed">
130+
Only this entry is added to
131+
<code class="font-mono text-[11px] break-all" :title="previews[client.id]!.config_path">{{ previews[client.id]!.config_path }}</code>.
132+
Everything else in the file stays untouched, and a timestamped backup is created first.
133+
</p>
134+
<!-- Overwrite warning (FR-003): an entry with this name already exists. -->
135+
<p
136+
v-if="previews[client.id]!.entry_exists"
137+
:data-test="`connect-preview-overwrite-${client.id}`"
138+
class="text-xs text-warning leading-relaxed"
139+
>
140+
An entry named “{{ previews[client.id]!.server_name }}” already exists — connecting will overwrite it (a backup is saved first).
141+
</p>
142+
<!-- Malformed config (Spec 075): the write parses the same bytes and
143+
would fail, so connecting is blocked until the file is fixed. -->
144+
<p
145+
v-else-if="previews[client.id]!.access_state === 'malformed'"
146+
:data-test="`connect-preview-malformed-${client.id}`"
147+
class="text-xs text-warning leading-relaxed"
148+
>
149+
Your current config could not be parsed, so connecting would fail rather than modify an unreadable file. Fix or remove {{ previews[client.id]!.config_path }} first, then try again.
150+
</p>
151+
<!-- No prior file (bridge / absent): nothing to back up. -->
152+
<p
153+
v-else-if="previews[client.id]!.access_state === 'absent'"
154+
:data-test="`connect-preview-no-file-${client.id}`"
155+
class="text-xs opacity-60 leading-relaxed"
156+
>
157+
This file will be created; there is no prior file to back up.
158+
</p>
159+
<div>
160+
<div class="text-[11px] font-semibold uppercase tracking-wider text-success/80 mb-1">+ will be added</div>
161+
<pre
162+
:data-test="`connect-preview-entry-${client.id}`"
163+
class="text-[11px] font-mono whitespace-pre-wrap break-all rounded bg-base-300/60 border-l-2 border-success px-2 py-1.5 leading-relaxed"
164+
>{{ previews[client.id]!.entry_text }}</pre>
165+
</div>
166+
<!-- API-key honesty (FR-004): masked in the preview, real key written. -->
167+
<p
168+
v-if="previews[client.id]!.contains_api_key"
169+
:data-test="`connect-preview-apikey-${client.id}`"
170+
class="text-[11px] opacity-60 leading-relaxed"
171+
>
172+
This entry includes your API key (shown masked). The real key is written into the config so the client can authenticate.
173+
</p>
174+
<div class="flex items-center gap-2 pt-1">
175+
<button
176+
:data-test="`connect-preview-confirm-${client.id}`"
177+
@click="confirmConnect(client.id)"
178+
class="btn btn-primary btn-xs"
179+
:disabled="loading.clients[client.id] || previews[client.id]!.access_state === 'malformed'"
180+
>
181+
<span v-if="loading.clients[client.id]" class="loading loading-spinner loading-xs"></span>
182+
<span v-else>Connect</span>
183+
</button>
184+
<button
185+
:data-test="`connect-preview-cancel-${client.id}`"
186+
@click="cancelPreview(client.id)"
187+
class="btn btn-ghost btn-xs"
188+
:disabled="loading.clients[client.id]"
189+
>
190+
Cancel
191+
</button>
192+
</div>
193+
</div>
194+
<!-- Spec 078 US1: preview fetch failed (non-denial). Denials fall
195+
through to the denied banner above via checkAccess. -->
196+
<div
197+
v-else-if="previewError[client.id]"
198+
:data-test="`connect-preview-error-${client.id}`"
199+
class="border-t border-base-300 px-3 py-2 text-xs text-error"
200+
>
201+
{{ previewError[client.id] }}
202+
</div>
120203
</div>
121204

122205
<div v-if="clients.length === 0 && !loading.initial" class="text-center py-6 opacity-60">
@@ -210,7 +293,7 @@ import { ref, reactive, computed, watch } from 'vue'
210293
import api from '@/services/api'
211294
import { useSystemStore } from '@/stores/system'
212295
import { useOnboardingStore } from '@/stores/onboarding'
213-
import type { ClientStatus, AccessState } from '@/types'
296+
import type { ClientStatus, AccessState, ConnectPreview } from '@/types'
214297
215298
interface Props {
216299
show: boolean
@@ -251,6 +334,13 @@ const loading = reactive({
251334
const resolved = ref<Record<string, ClientStatus>>({})
252335
const checking = reactive<Record<string, boolean>>({})
253336
const copiedClient = ref<string | null>(null)
337+
// Spec 078 US1: a fetched preview per client (the exact change a connect would
338+
// make, no write yet). Present => the confirm/cancel panel is shown for that
339+
// client. previewError holds a non-denial fetch failure (a denial resolves to
340+
// the access-state banner instead).
341+
const previews = ref<Record<string, ConnectPreview>>({})
342+
const previewLoading = reactive<Record<string, boolean>>({})
343+
const previewError = ref<Record<string, string>>({})
254344
255345
// MCP-2952: `GET /api/v1/connect` is stat-only (#706/MCP-2829) and reports
256346
// connected=false for every client. Merge the content-resolved
@@ -326,24 +416,64 @@ async function fetchClients() {
326416
}
327417
}
328418
329-
// Single-connect entry point (row button): a fresh single operation supersedes
330-
// any earlier Connect All per-client backup list.
331-
async function connectSingle(clientId: string) {
419+
// Spec 078 US1: clicking the row's Connect fetches the preview first and shows
420+
// the confirm/cancel panel — no file is written until the user confirms. A
421+
// denied read (403) is surfaced as the access-state banner via checkAccess.
422+
async function startConnect(clientId: string) {
423+
previewLoading[clientId] = true
424+
previewError.value = { ...previewError.value, [clientId]: '' }
425+
try {
426+
const response = await api.getConnectPreview(clientId)
427+
if (response.success && response.data) {
428+
previews.value = { ...previews.value, [clientId]: response.data }
429+
} else {
430+
// The preview read may have been blocked by macOS App-Data — resolve the
431+
// access state so a denial renders the existing remediation banner.
432+
previewError.value = { ...previewError.value, [clientId]: response.error || 'Failed to load preview' }
433+
void checkAccess(clientId)
434+
}
435+
} catch (err) {
436+
previewError.value = { ...previewError.value, [clientId]: err instanceof Error ? err.message : 'Failed to load preview' }
437+
void checkAccess(clientId)
438+
} finally {
439+
previewLoading[clientId] = false
440+
}
441+
}
442+
443+
// Cancel dismisses the preview WITHOUT writing anything (Spec 078 US1).
444+
function cancelPreview(clientId: string) {
445+
const next = { ...previews.value }
446+
delete next[clientId]
447+
previews.value = next
448+
}
449+
450+
function clearPreview(clientId: string) {
451+
cancelPreview(clientId)
452+
const nextErr = { ...previewError.value }
453+
delete nextErr[clientId]
454+
previewError.value = nextErr
455+
}
456+
457+
// Confirm proceeds with the connect. If an entry already exists, confirming
458+
// implies force=true (the user saw the overwrite warning in the preview).
459+
async function confirmConnect(clientId: string) {
460+
const force = previews.value[clientId]?.entry_exists === true
332461
bulkBackups.value = []
333462
copiedBulkClient.value = null
334-
await connect(clientId)
463+
await connect(clientId, force)
464+
clearPreview(clientId)
335465
}
336466
337467
// Returns the outcome so connectAll can accumulate per-client backup results
338468
// (ok=true with backupPath string = backup created; null = no prior file).
339-
async function connect(clientId: string): Promise<{ ok: boolean; backupPath: string | null }> {
469+
async function connect(clientId: string, force = false): Promise<{ ok: boolean; backupPath: string | null }> {
340470
loading.clients[clientId] = true
341471
resultMessage.value = ''
342472
resultBackupPath.value = undefined
343473
copiedBackup.value = false
344474
345475
try {
346-
const response = await api.connectClient(clientId)
476+
const response = await api.connectClient(clientId, 'mcpproxy', force)
347477
if (response.success && response.data) {
348478
resultMessage.value = response.data.message || `Connected to ${clientId}`
349479
resultSuccess.value = true
@@ -525,6 +655,8 @@ function close() {
525655
copiedBackup.value = false
526656
bulkBackups.value = []
527657
copiedBulkClient.value = null
658+
previews.value = {}
659+
previewError.value = {}
528660
emit('close')
529661
}
530662
@@ -540,6 +672,8 @@ watch(() => props.show, (newVal) => {
540672
copiedBackup.value = false
541673
bulkBackups.value = []
542674
copiedBulkClient.value = null
675+
previews.value = {}
676+
previewError.value = {}
543677
}
544678
})
545679
</script>

0 commit comments

Comments
 (0)