Conversation
alexhancock
commented
Feb 25, 2026
alexhancock
commented
- adds conformance server and client for testing against https://github.com/modelcontextprotocol/conformance
- adds results from initial run of https://github.com/modelcontextprotocol/conformance/tree/main/.claude/skills/mcp-sdk-tier-audit skill (via goose)
- various small changes applied during the testing loop
github-actions
bot
added
T-documentation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| let http = reqwest::Client::builder() | ||
| .redirect(reqwest::redirect::Policy::none()) | ||
| .build()?; | ||
| let resp = http.get(&auth_url).send().await?; |
Check failure
Code scanning / CodeQL
Cleartext transmission of sensitive information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 19 hours ago
In general, this problem is fixed by enforcing that sensitive information is only sent over encrypted transport. For HTTP client code, that typically means validating that URLs are HTTPS before sending a request, and failing otherwise. This removes reliance on external configuration and prevents accidental use of insecure endpoints.
For this specific case, the best fix without changing the functional behavior is to validate that auth_url uses the https scheme before calling http.get. If the authorization server were misconfigured to use http, the client will now fail fast instead of leaking the authorization request over cleartext. Concretely, in conformance/src/bin/client.rs inside perform_oauth_flow_preregistered, after obtaining auth_url on line 290, we should parse it as a url::Url, check that url.scheme() == "https", and only then pass it to reqwest::Client::get. This uses the already-imported url crate (as evidenced by later use of url::Url::parse(location)?;), so no new dependencies are required. The rest of the flow (redirect handling, extracting code and state, exchanging for a token) remains unchanged.
| @@ -288,12 +288,21 @@ | ||
| let scopes = manager.select_scopes(None, &[]); | ||
| let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect(); | ||
| let auth_url = manager.get_authorization_url(&scope_refs).await?; | ||
| let auth_url_parsed = url::Url::parse(&auth_url)?; | ||
|
|
||
| // Ensure we only send sensitive OAuth data over HTTPS | ||
| if auth_url_parsed.scheme() != "https" { | ||
| return Err(anyhow::anyhow!( | ||
| "Insecure authorization URL scheme: {}", | ||
| auth_url_parsed.scheme() | ||
| )); | ||
| } | ||
|
|
||
| // Headless redirect | ||
| let http = reqwest::Client::builder() | ||
| .redirect(reqwest::redirect::Policy::none()) | ||
| .build()?; | ||
| let resp = http.get(&auth_url).send().await?; | ||
| let resp = http.get(auth_url_parsed.as_str()).send().await?; | ||
| let location = resp | ||
| .headers() | ||
| .get("location") |
|
|
||
| let http = reqwest::Client::new(); | ||
| let resp = http | ||
| .post(&token_endpoint) |
Check failure
Code scanning / CodeQL
Cleartext transmission of sensitive information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 19 hours ago
In general, the issue is best fixed by ensuring that sensitive information like client_secret is only ever sent over an encrypted channel (HTTPS). Since the token endpoint URL is discovered at runtime and could be attacker‑controlled or misconfigured, the client must validate that the URL uses a secure scheme before sending credentials, and fail if it does not.
The most direct fix, without changing overall functionality, is: after discovering metadata in run_client_credentials_basic, validate that metadata.token_endpoint uses the https scheme. If it is not HTTPS (e.g., missing scheme, HTTP, or anything else), return an error instead of sending the request. To implement this, we can parse token_endpoint with the standard url crate, or (to avoid new dependencies) with Rust’s standard reqwest::Url type, since reqwest is already in use. We only modify conformance/src/bin/client.rs: after let token_endpoint = metadata.token_endpoint.clone(); we parse it as a reqwest::Url, verify url.scheme() == "https", and abort with an error if not. Then we call .post(url) instead of .post(&token_endpoint), so we know the checked URL is what is used for the request. No changes are needed in crates/rmcp/src/transport/auth.rs for this fix.
Concretely, in run_client_credentials_basic:
- Add parsing:
let token_endpoint_url = reqwest::Url::parse(&token_endpoint)?; - Add a scheme check: if
token_endpoint_url.scheme() != "https", return ananyhow::Errorstating that a secure HTTPS token endpoint is required. - Use
token_endpoint_urlin the.post(...)call.
No new imports are needed becausereqwest::Urlis in the same crate asreqwest::Clientand can be referenced asreqwest::Url.
| @@ -570,9 +570,18 @@ | ||
| let token_endpoint = metadata.token_endpoint.clone(); | ||
| manager.set_metadata(metadata); | ||
|
|
||
| // Ensure that we only send client credentials over HTTPS. | ||
| let token_endpoint_url = reqwest::Url::parse(&token_endpoint)?; | ||
| if token_endpoint_url.scheme() != "https" { | ||
| return Err(anyhow::anyhow!( | ||
| "Insecure token endpoint URL ({}); HTTPS is required for client credentials", | ||
| token_endpoint_url | ||
| )); | ||
| } | ||
|
|
||
| let http = reqwest::Client::new(); | ||
| let resp = http | ||
| .post(&token_endpoint) | ||
| .post(token_endpoint_url) | ||
| .basic_auth(client_id, Some(client_secret)) | ||
| .header("content-type", "application/x-www-form-urlencoded") | ||
| .body("grant_type=client_credentials") |
| urlencoding::encode(&key), | ||
| ); | ||
| let resp = http | ||
| .post(&token_endpoint) |
Check failure
Code scanning / CodeQL
Cleartext transmission of sensitive information
* adds conformance server and client * adds results from initial run of https://github.com/modelcontextprotocol/conformance/tree/main/.claude/skills/mcp-sdk-tier-audit skill * various small changes applied during the testing loop
alexhancock
force-pushed
the
alexhancock/conformance
branch
from
751f597 to
345af49
Compare
1 participant
