# Patch Analysis for CVE-2026-58196 Variant

## Target threat model and security scope

ToolHive's `SECURITY.md` asks reporters to submit reproducible security issues through the GitHub Security Advisory flow; it does not exclude SSRF or remote MCP authentication discovery from scope. ToolHive's own remote MCP authentication documentation describes remote MCP server authentication as automatic RFC 9728 / RFC 8414 / RFC 7591 discovery, where ToolHive connects to a remote MCP server, parses `WWW-Authenticate`, fetches `resource_metadata`, validates `authorization_servers`, and may perform Dynamic Client Registration (DCR). This is an explicit trust boundary: a remote MCP server and the OAuth metadata it advertises are outside the ToolHive host, while the discovery HTTP clients run in the ToolHive host process.

The vulnerability class is therefore in scope: an untrusted or compromised remote MCP/auth metadata endpoint influences URLs fetched by the ToolHive host during the documented `thv run <remote-url> --remote-auth` workflow.

## What the v0.31.0 fix changes

The v0.31.0 fix introduces a shared SSRF guard in `pkg/networking` and applies it to the initial remote-auth discovery paths:

- `pkg/networking/http_client.go`
  - Adds `SameHostRedirectPolicy()`, which refuses redirects to a different `Host` (including a different port), refuses HTTPS-to-HTTP downgrades, and caps redirect chains.
  - Adds `NewPrivateIPBlockingDialContext()`, which refuses private, loopback, link-local, and other non-public dials after DNS resolution.
- `pkg/networking/utilities.go`
  - Adds/uses `TargetIsPrivate()` and broad private/non-public IP classification, including loopback, link-local, RFC1918, CGN, documentation ranges, multicast/reserved ranges, and NAT64 embedded-private handling.
- `pkg/auth/discovery/discovery.go`
  - `DetectAuthenticationFromServer()` now installs `SameHostRedirectPolicy()` on the initial GET/POST auth detection client.
  - `FetchResourceMetadata(ctx, metadataURL, blockPrivateIPs)` now accepts a private-IP blocking decision, installs `SameHostRedirectPolicy()`, and optionally uses `NewPrivateIPBlockingDialContext()` with keep-alives disabled.
- `pkg/auth/oauth/oidc.go`
  - `DiscoverActualIssuer()` / `discoverOIDCEndpointsWithClientAndValidation()` can block private dials and now installs `SameHostRedirectPolicy()` when it builds the default client.
- `pkg/auth/remote/handler.go`
  - Computes `blockPrivateIPs := !networking.TargetIsPrivate(ctx, remoteURL)` and propagates it through server-supplied realm/resource-metadata/authorization-server discovery. The assumption is that private targets intentionally configured by an operator may use internal metadata, but public targets must not be able to pivot host-side discovery to private URLs.

The parent reproduction confirmed the intended behavior for the original path: v0.31.0 blocks a malicious `resource_metadata` endpoint that redirects cross-port to a loopback canary, while v0.30.0 follows it.

## Fix assumptions

The fix assumes every host-side HTTP client that fetches a URL derived from remote MCP/OAuth discovery is either:

1. in `pkg/auth/discovery` or `pkg/auth/oauth/oidc` and therefore receives the new redirect/private-IP protections, or
2. only fetches operator-configured trusted URLs, not server-supplied discovery URLs.

It also assumes the DCR path reuses already-protected discovery data or that its own discovery helpers are safe by construction.

## Gap / missed code path

A distinct DCR re-discovery path remains unguarded:

- `pkg/auth/discovery/discovery.go:731` calls:
  - `oauthproto.FetchAuthorizationServerMetadata(ctx, discoveredDoc.Issuer, nil)`
- `pkg/oauthproto/discovery.go:191` builds the default discovery HTTP client when `client == nil`:
  - `&http.Client{Timeout: discoveryTimeout, Transport: &http.Transport{...}}`
  - No `CheckRedirect: networking.SameHostRedirectPolicy()`.
  - No private-IP blocking dialer.
- `pkg/oauthproto/discovery.go:270` (`fetchDiscoveryDocument`) then calls `client.Do(req)` for RFC 8414/OIDC metadata URLs. Go's default redirect behavior follows HTTP 30x responses automatically.

This path is reached after the fixed RFC 9728 and OIDC discovery clients have already accepted a server-supplied authorization server. During CLI OAuth/DCR, `resolveDCRCredentials()` deliberately performs a second multi-URL RFC 8414 metadata fetch to recover full metadata and PKCE/DCR capability fields. That second fetch is not covered by the v0.31.0 SSRF guard.

A malicious authorization server that was itself advertised by the remote MCP server can therefore return a 302 from `/.well-known/oauth-authorization-server` to a different host/port, and ToolHive follows the redirect from the host process to the internal canary. This is not the same trigger as the parent proof: the parent SSRF occurred in `FetchResourceMetadata`; this variant occurs later in `oauthproto.FetchAuthorizationServerMetadata` during Dynamic Client Registration metadata re-fetch.

## Behavior before and after the fix

- v0.30.0:
  - The original and variant paths both follow untrusted discovery redirects to the loopback canary.
- v0.31.0:
  - The original `resource_metadata` redirect path is blocked by `FetchResourceMetadata(..., blockPrivateIPs)` plus `SameHostRedirectPolicy()`.
  - The DCR metadata re-fetch path still follows the cross-port redirect because `oauthproto.buildDiscoveryHTTPClient(nil)` builds an unguarded client.
- v0.38.0 (latest release tested in this run):
  - The same DCR metadata re-fetch redirect is still followed; the latest release is also affected by this variant.

## Variant reproduction result

`bundle/vuln_variant/reproduction_steps.sh` tests v0.30.0, fixed v0.31.0, and latest v0.38.0 side by side. The variant is confirmed when the internal loopback canary receives a request during the DCR metadata re-fetch phase.

Observed from the latest script run:

- v0.30.0: `internal canary events=2`
- v0.31.0: `internal canary events=2`
- v0.38.0: `internal canary events=2`

Representative fixed-version evidence:

- `bundle/logs/vuln_variant/fixed_auth.log` records ToolHive v0.31.0 fetching `/resource-metadata`, then `/ .well-known/openid-configuration`, then receiving a 302 from `/.well-known/oauth-authorization-server` with `location` pointing to the separate canary port.
- `bundle/logs/vuln_variant/fixed_canary.log` records the canary hit with `Referer: http://127.0.0.1:<auth-port>/.well-known/oauth-authorization-server` and `User-Agent: ToolHive/1.0`.
- `bundle/logs/vuln_variant/latest_canary.log` records the same behavior on v0.38.0.

## Completeness assessment

The v0.31.0 fix is incomplete. It protects the initial detection, RFC 9728 resource metadata fetch, and legacy OIDC authorization-server validation paths, but it does not protect `pkg/oauthproto` authorization-server metadata fetches used by the DCR resolver. Any path that passes `nil` into `oauthproto.FetchAuthorizationServerMetadata` or `FetchAuthorizationServerMetadataFromURL` inherits the unguarded default client.

Current call sites of concern:

- `pkg/auth/discovery/discovery.go:731` — confirmed runtime-backed bypass in CLI remote auth / DCR.
- `pkg/auth/dcr/resolver.go:894` — programmatic DCR `DiscoveryURL` branch also passes `nil` to `FetchAuthorizationServerMetadataFromURL`; this is the same unguarded helper family and should be reviewed/fixed even though the encoded runtime proof uses the CLI re-fetch call at `discovery.go:731`.

## Recommendation

Apply the same SSRF guard to all `oauthproto` discovery HTTP clients used for untrusted or server-derived metadata:

1. Extend `oauthproto.FetchAuthorizationServerMetadata` and `FetchAuthorizationServerMetadataFromURL` to either:
   - accept an explicit guarded `*http.Client`, or
   - accept a `blockPrivateIPs`/redirect-policy option and build a guarded default client internally.
2. When callers pass `nil`, do not use Go's default redirect behavior. Install `networking.SameHostRedirectPolicy()` and, for server-derived metadata on public configured targets, `networking.NewPrivateIPBlockingDialContext()` with keep-alives disabled.
3. Thread the existing `blockPrivateIPs` decision from `pkg/auth/remote/handler.go` through `resolveDCRCredentials()` into the second RFC 8414 metadata fetch.
4. Add regression tests for DCR metadata re-fetch redirects to a different port/host and to private/loopback/link-local addresses, plus a latest-release regression test for `oauthproto.FetchAuthorizationServerMetadataFromURL`.
