Skip to content

Commit ce85117

Browse files
chelojimenezclaude
andauthored
fix(connection): auto-fetch hidden stored headers when saving auth changes (#2562)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9f4aadd commit ce85117

3 files changed

Lines changed: 187 additions & 17 deletions

File tree

mcpjam-inspector/client/src/components/connection/ServerDetailModal.tsx

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import type {
4646
ProjectServerOverrideEntry,
4747
} from "@/lib/project-server-config";
4848
import { EffectiveProtocolVersionChip } from "./shared/EffectiveProtocolVersionChip";
49+
import { fetchServerSecrets } from "@/lib/apis/server-secrets-api";
4950
import { useFeatureFlagEnabled } from "posthog-js/react";
5051
import { useActiveMcpProfile } from "@/contexts/active-mcp-profile-context";
5152

@@ -503,9 +504,41 @@ export function ServerDetailModal({
503504
environment: detectEnvironment(),
504505
});
505506

506-
const finalFormData = formState.buildFormData();
507507
setIsSaving(true);
508508
try {
509+
// Saving an auth or header change replaces the whole stored header
510+
// set. When that set is hidden, fetch it first so e.g. rotating a
511+
// bearer token doesn't wipe the other saved headers.
512+
let revealedHeaders: Record<string, string> | undefined;
513+
if (formState.needsStoredHeaderReveal) {
514+
if (!projectId || !hostedServerId) {
515+
toast.error(
516+
"Reveal saved headers before changing authentication so existing hidden headers aren't lost."
517+
);
518+
return;
519+
}
520+
try {
521+
const secrets = await fetchServerSecrets({
522+
projectId,
523+
serverId: hostedServerId,
524+
});
525+
// A null headers payload means the stored set couldn't be read;
526+
// merging against it would wipe the saved headers, so fail closed.
527+
if (!secrets.headers) {
528+
throw new Error("Stored headers missing from reveal response");
529+
}
530+
revealedHeaders = secrets.headers;
531+
} catch {
532+
toast.error(
533+
"Couldn't load this server's saved headers to apply this change. Reveal saved headers in Advanced settings and try again."
534+
);
535+
return;
536+
}
537+
}
538+
539+
const finalFormData = formState.buildFormData(
540+
revealedHeaders ? { revealedHeaders } : undefined
541+
);
509542
await onSubmit(finalFormData, server.name);
510543
} finally {
511544
setIsSaving(false);

mcpjam-inspector/client/src/components/connection/hooks/__tests__/use-server-form.test.ts

Lines changed: 111 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ describe("useServerForm", () => {
116116
});
117117
});
118118

119-
it("does not replace hidden stored headers when editing auth without reveal", async () => {
119+
it("asks for stored headers when editing auth with hidden headers and merges them into the patch", async () => {
120120
const server = {
121121
name: "Hidden header server",
122122
config: {
@@ -140,13 +140,117 @@ describe("useServerForm", () => {
140140
result.current.setBearerToken("new-token");
141141
});
142142

143-
expect(result.current.buildFormData()).toMatchObject({
144-
headers: { Authorization: "Bearer new-token" },
145-
});
143+
// Without the stored headers the form can't build a safe replacement
144+
// patch, so it withholds one and flags that a reveal is needed.
145+
expect(result.current.needsStoredHeaderReveal).toBe(true);
146146
expect(result.current.buildFormData().secretPatch?.headers).toBeUndefined();
147-
expect(result.current.validateForm()).toBe(
148-
"Reveal saved headers before changing authentication so existing hidden headers aren't lost."
149-
);
147+
expect(result.current.validateForm()).toBeNull();
148+
149+
// With the stored headers supplied at save time, the patch swaps the
150+
// Authorization header and keeps the rest.
151+
expect(
152+
result.current.buildFormData({
153+
revealedHeaders: {
154+
Authorization: "Bearer old-token",
155+
"X-Api-Key": "secret",
156+
},
157+
})
158+
).toMatchObject({
159+
headers: {
160+
Authorization: "Bearer new-token",
161+
"X-Api-Key": "secret",
162+
},
163+
secretPatch: {
164+
headers: {
165+
Authorization: "Bearer new-token",
166+
"X-Api-Key": "secret",
167+
},
168+
},
169+
});
170+
});
171+
172+
it("keeps the hidden Authorization header when only header rows change", async () => {
173+
const server = {
174+
name: "Hidden header server",
175+
config: {
176+
url: "https://example.com/mcp",
177+
},
178+
hasHeaders: true,
179+
lastConnectionTime: new Date(),
180+
connectionStatus: "disconnected",
181+
retryCount: 0,
182+
enabled: true,
183+
} as any;
184+
185+
const { result } = renderHook(() => useServerForm(server));
186+
187+
await waitFor(() => {
188+
expect(result.current.hasStoredHeaders).toBe(true);
189+
});
190+
191+
act(() => {
192+
result.current.addCustomHeader();
193+
});
194+
act(() => {
195+
result.current.updateCustomHeader(0, "key", "X-New");
196+
});
197+
act(() => {
198+
result.current.updateCustomHeader(0, "value", "fresh");
199+
});
200+
201+
expect(result.current.needsStoredHeaderReveal).toBe(true);
202+
expect(
203+
result.current.buildFormData({
204+
revealedHeaders: {
205+
Authorization: "Bearer keep-me",
206+
"X-Api-Key": "secret",
207+
},
208+
}).secretPatch
209+
).toEqual({
210+
headers: {
211+
Authorization: "Bearer keep-me",
212+
"X-Api-Key": "secret",
213+
"X-New": "fresh",
214+
},
215+
});
216+
});
217+
218+
it("drops the hidden Authorization header when auth switches away from bearer", async () => {
219+
const server = {
220+
name: "Hidden header server",
221+
config: {
222+
url: "https://example.com/mcp",
223+
},
224+
hasHeaders: true,
225+
lastConnectionTime: new Date(),
226+
connectionStatus: "disconnected",
227+
retryCount: 0,
228+
enabled: true,
229+
} as any;
230+
231+
const { result } = renderHook(() => useServerForm(server));
232+
233+
await waitFor(() => {
234+
expect(result.current.hasStoredHeaders).toBe(true);
235+
});
236+
237+
act(() => {
238+
result.current.setAuthType("oauth");
239+
});
240+
241+
expect(result.current.needsStoredHeaderReveal).toBe(true);
242+
expect(
243+
result.current.buildFormData({
244+
revealedHeaders: {
245+
Authorization: "Bearer old-token",
246+
"X-Api-Key": "secret",
247+
},
248+
}).secretPatch
249+
).toEqual({
250+
headers: {
251+
"X-Api-Key": "secret",
252+
},
253+
});
150254
});
151255

152256
it("sends a replacement header patch after stored headers are revealed", async () => {

mcpjam-inspector/client/src/components/connection/hooks/use-server-form.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ export function useServerForm(
139139
const [hasStoredHeaders, setHasStoredHeaders] = useState(false);
140140
const [envDirty, setEnvDirty] = useState(false);
141141
const [headersDirty, setHeadersDirty] = useState(false);
142+
// Auth edits (auth type / bearer token) are tracked apart from header-row
143+
// edits: when hidden stored headers are merged in at save time, the saved
144+
// Authorization header must only be dropped if the user touched auth.
145+
const [authDirty, setAuthDirty] = useState(false);
142146
const [envRevealed, setEnvRevealed] = useState(false);
143147
const [headersRevealed, setHeadersRevealed] = useState(false);
144148
const [requestTimeout, setRequestTimeout] = useState<string>("");
@@ -384,6 +388,7 @@ export function useServerForm(
384388
setHasStoredHeaders(hasStoredHeadersValue);
385389
setHeadersRevealed(headersArray.length > 0);
386390
setHeadersDirty(false);
391+
setAuthDirty(false);
387392
setShowConfiguration(
388393
headersArray.length > 0 ||
389394
timeoutValue.trim() !== "" ||
@@ -468,10 +473,6 @@ export function useServerForm(
468473
) {
469474
return "HTTPS is required";
470475
}
471-
472-
if (hasStoredHeaders && !headersRevealed && headersDirty) {
473-
return "Reveal saved headers before changing authentication so existing hidden headers aren't lost.";
474-
}
475476
}
476477

477478
if (
@@ -600,7 +601,14 @@ export function useServerForm(
600601
}
601602
};
602603

603-
const buildFormData = (): ServerFormData => {
604+
const buildFormData = (buildOptions?: {
605+
/**
606+
* Stored headers fetched from the secrets API at save time. Supplying
607+
* them lets a server with hidden stored headers take an auth or header
608+
* change without wiping the headers the form can't see.
609+
*/
610+
revealedHeaders?: Record<string, string>;
611+
}): ServerFormData => {
604612
const parsedTimeout = Number.parseInt(requestTimeout.trim(), 10);
605613
const reqTimeout = Number.isFinite(parsedTimeout)
606614
? parsedTimeout
@@ -645,8 +653,21 @@ export function useServerForm(
645653
}
646654

647655
// Handle http-specific data
656+
const revealedStoredHeaders = buildOptions?.revealedHeaders;
648657
const headers: Record<string, string> = {};
649658

659+
// Seed with the stored headers so the replacement patch keeps them. The
660+
// saved Authorization header only carries over while auth is untouched —
661+
// once the user edits auth, the auth section below is authoritative.
662+
if (revealedStoredHeaders) {
663+
for (const [key, value] of Object.entries(revealedStoredHeaders)) {
664+
if (authDirty && isAuthorizationHeader(key)) {
665+
continue;
666+
}
667+
headers[key] = value;
668+
}
669+
}
670+
650671
// Add custom headers
651672
customHeaders.forEach(({ key, value }) => {
652673
if (key.trim()) {
@@ -682,9 +703,10 @@ export function useServerForm(
682703
}
683704
const explicitHeaders =
684705
Object.keys(headers).length > 0 ? headers : undefined;
685-
const canPatchHeaders = !hasStoredHeaders || headersRevealed;
706+
const canPatchHeaders =
707+
!hasStoredHeaders || headersRevealed || revealedStoredHeaders != null;
686708
const secretPatch =
687-
headersDirty && canPatchHeaders ? { headers } : undefined;
709+
(headersDirty || authDirty) && canPatchHeaders ? { headers } : undefined;
688710

689711
return {
690712
name: name.trim(),
@@ -736,6 +758,7 @@ export function useServerForm(
736758
setHasStoredHeaders(false);
737759
setEnvDirty(false);
738760
setHeadersDirty(false);
761+
setAuthDirty(false);
739762
setEnvRevealed(false);
740763
setHeadersRevealed(false);
741764
setRequestTimeout("");
@@ -778,6 +801,15 @@ export function useServerForm(
778801
);
779802
})();
780803

804+
// Saving a header-affecting change replaces the whole stored header set,
805+
// so when that set is hidden the caller must fetch it (secrets API) and
806+
// pass it to buildFormData as `revealedHeaders` before submitting.
807+
const needsStoredHeaderReveal =
808+
type === "http" &&
809+
hasStoredHeaders &&
810+
!headersRevealed &&
811+
(headersDirty || authDirty);
812+
781813
const preregisteredOauthBlocksSubmit =
782814
type === "http" &&
783815
authType === "oauth" &&
@@ -822,12 +854,12 @@ export function useServerForm(
822854
setClearClientSecret,
823855
bearerToken,
824856
setBearerToken: (value: string) => {
825-
setHeadersDirty(true);
857+
setAuthDirty(true);
826858
setBearerToken(value);
827859
},
828860
authType,
829861
setAuthType: (value: "oauth" | "bearer" | "none") => {
830-
setHeadersDirty(true);
862+
setAuthDirty(true);
831863
setAuthType(value);
832864
},
833865
useCustomClientId,
@@ -859,6 +891,7 @@ export function useServerForm(
859891
headersDirty,
860892
envRevealed,
861893
headersRevealed,
894+
needsStoredHeaderReveal,
862895

863896
// Toggle states
864897
showConfiguration,

0 commit comments

Comments
 (0)