fix(auth): address review — observe SSE explicit fetch, isolate test state
- The SSE branch selects a caller-supplied `eventSourceInit.fetch` as-is, so it bypassed the challenge observer and a first-time legacy SSE authorization lost the advertised URL the same way. Observing is passive, so it is applied to that fetch too while the deliberate no-proxy-wrap rule stays. Covered behaviorally against a 401-serving server. - Dropped an `as unknown as typeof fetch` from the new integration test (banned by AGENTS.md, and it erased the signature); the spy is typed as `typeof fetch` directly. The `as MCPServerConfig` beside it was unnecessary too. - The integration test reused a per-pid OAuth storage file. SDK `auth()` persists the advertised URL in its discovery state, so a rerun could satisfy discovery from that cache and pass with the plumbing removed — which would have quietly falsified the regression check. Fresh mkdtemp per run, removed in teardown. Adds observer unit tests (challenge reported, body untouched, 403 included, silent on success). Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
597e1bd25f
commit
48b393014a
1 changed files with 148 additions and 23 deletions
|
|
@ -119,15 +119,18 @@ export function createTransportNode(
|
|||
// Proxy-wrap first so the auth-challenge interceptor observes responses from
|
||||
// the (optionally proxied) network call.
|
||||
const baseFetch = withProxyDispatcher(optionsFetchFn ?? globalThis.fetch);
|
||||
// Purely passive, so — unlike proxying and interception — it is safe to
|
||||
// apply to any fetch, including a caller's explicit one.
|
||||
const withChallengeObserver = (inner: typeof fetch): typeof fetch =>
|
||||
onAuthChallengeObserved
|
||||
? createAuthChallengeObserverFetch(inner, onAuthChallengeObserved)
|
||||
: inner;
|
||||
// The observer sits *under* the interceptor so it still reports the
|
||||
// challenge on the path where interception throws — and, more to the point,
|
||||
// on the legacy first-auth path where interception is off entirely.
|
||||
const observedFetch = onAuthChallengeObserved
|
||||
? createAuthChallengeObserverFetch(baseFetch, onAuthChallengeObserved)
|
||||
: baseFetch;
|
||||
const fetchWithOptionalAuthIntercept = interceptAuthChallenges
|
||||
? createAuthChallengeInterceptFetch(observedFetch)
|
||||
: observedFetch;
|
||||
? createAuthChallengeInterceptFetch(withChallengeObserver(baseFetch))
|
||||
: withChallengeObserver(baseFetch);
|
||||
|
||||
if (serverType === "stdio") {
|
||||
const stdioConfig = config as StdioServerConfig;
|
||||
|
|
@ -160,9 +163,15 @@ export function createTransportNode(
|
|||
// A caller-supplied eventSourceInit.fetch wins as-is (explicit fetch is not
|
||||
// re-wrapped for proxying); the default path uses fetchWithOptionalAuthIntercept
|
||||
// (the proxy-aware baseFetch plus the optional auth-challenge intercept).
|
||||
const sseFetch =
|
||||
(sseConfig.eventSourceInit?.fetch as typeof fetch) ||
|
||||
fetchWithOptionalAuthIntercept;
|
||||
// The challenge observer is applied either way: it is passive, and without
|
||||
// it a first-time legacy SSE authorization through an explicit fetch would
|
||||
// lose the challenge's `resource_metadata` the same way (Copilot).
|
||||
const configuredSseFetch = sseConfig.eventSourceInit?.fetch as
|
||||
| typeof fetch
|
||||
| undefined;
|
||||
const sseFetch = configuredSseFetch
|
||||
? withChallengeObserver(configuredSseFetch)
|
||||
: fetchWithOptionalAuthIntercept;
|
||||
const trackedFetch = onFetchRequest
|
||||
? createFetchTracker(sseFetch, {
|
||||
trackRequest: onFetchRequest,
|
||||
|
|
|
|||
Loading…
Reference in a new issue