From 9cdca5545a651a8e3ac2650d591c05c3fd83a643 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 14:38:05 +0530 Subject: [PATCH 01/21] feat(agent_tools): add McpCallOutcome and bump contract to 1.3 (#40) Adds MCP_CALL_RESULT_KIND, McpCallOutcome and McpCallError to the contract crate so a host can read what a forwarded mcp_call_tool call did (server, tool, whether it was answered, the classified error) from the result's metadata instead of parsing the rendered text. CONTRACT_VERSION moves from 1.2 to 1.3. The previous change chose not to bump for the CREDENTIAL_STORE error name because only a host's own store raises it; this bump is driven by the new public payload members, which are additive, and the 1.3 entry records both. --- crates/tinymcp-bus/README.md | 2 +- crates/tinymcp-bus/src/agent_tools/mod.rs | 9 +- .../tinymcp-bus/src/agent_tools/mod_tests.rs | 83 +++++++++++++++++ crates/tinymcp-bus/src/agent_tools/types.rs | 92 +++++++++++++++++++ crates/tinymcp-bus/src/lib.rs | 9 +- crates/tinymcp-bus/src/version/mod.rs | 5 +- crates/tinymcp-bus/src/version/mod_tests.rs | 7 +- crates/tinymcp/src/lib.rs | 18 ++-- 8 files changed, 205 insertions(+), 20 deletions(-) diff --git a/crates/tinymcp-bus/README.md b/crates/tinymcp-bus/README.md index 11e25752..04fbf154 100644 --- a/crates/tinymcp-bus/README.md +++ b/crates/tinymcp-bus/README.md @@ -12,7 +12,7 @@ vocabulary has to be published as an ordinary library. This is it. | ---------- | ------------------------------------------------------------ | | `names` | interface name, object path, one constant per member | | `greeting` | the value vocabulary: the `Greet` request and response | -| `agent_tools` | tool specs a host exposes to a model, and argument normalization | +| `agent_tools` | tool specs a host exposes to a model, argument normalization, and the structured call outcome | | `version` | `CONTRACT_VERSION` and the bind rule a host applies to it | Two dependencies, both pure Rust: `serde` and `serde_json`. diff --git a/crates/tinymcp-bus/src/agent_tools/mod.rs b/crates/tinymcp-bus/src/agent_tools/mod.rs index 147a2b30..9f303dae 100644 --- a/crates/tinymcp-bus/src/agent_tools/mod.rs +++ b/crates/tinymcp-bus/src/agent_tools/mod.rs @@ -11,6 +11,11 @@ //! `arguments` as the object MCP requires, and every path that forwards a call //! reads them through it. //! +//! [`McpCallOutcome`] is what a forwarded call reports back to the host, beside +//! the text the model reads: which server and tool, whether the server +//! answered, and the classified error when it did not. It travels as a tool +//! result's metadata tagged with [`MCP_CALL_RESULT_KIND`]. +//! //! # Why this is in the contract crate //! //! A tool's name, description and schema are prompt-cache and transcript @@ -31,7 +36,9 @@ mod types; pub use arguments::normalize_tool_arguments; pub use registry_tools::{RegistryTool, registry_tool_specs}; -pub use types::{AgentToolEffect, AgentToolSpec, ArgsError}; +pub use types::{ + AgentToolEffect, AgentToolSpec, ArgsError, MCP_CALL_RESULT_KIND, McpCallError, McpCallOutcome, +}; #[cfg(test)] #[path = "mod_tests.rs"] diff --git a/crates/tinymcp-bus/src/agent_tools/mod_tests.rs b/crates/tinymcp-bus/src/agent_tools/mod_tests.rs index 0baea29c..9a1ad9db 100644 --- a/crates/tinymcp-bus/src/agent_tools/mod_tests.rs +++ b/crates/tinymcp-bus/src/agent_tools/mod_tests.rs @@ -309,3 +309,86 @@ fn effects_serialize_in_snake_case() { assert_eq!(serde_json::to_value(effect).unwrap(), json!(wire)); } } + +// --------------------------------------------------------------------------- +// Call outcome +// --------------------------------------------------------------------------- + +#[test] +fn the_call_outcome_kind_is_pinned() { + assert_eq!(super::MCP_CALL_RESULT_KIND, "mcp_call"); +} + +#[test] +fn an_answered_outcome_pins_its_wire_form_and_omits_the_error() { + let outcome = super::McpCallOutcome::answered("docs", "search"); + let wire = serde_json::to_value(&outcome).unwrap(); + assert_eq!( + wire, + json!({ "kind": "mcp_call", "server": "docs", "tool": "search", "ok": true }) + ); + assert_eq!( + serde_json::from_value::(wire).unwrap(), + outcome + ); +} + +#[test] +fn a_failed_outcome_pins_its_wire_form_and_round_trips() { + let outcome = super::McpCallOutcome::failed( + "docs", + "search", + super::McpCallError { + code: crate::errors::UNAUTHORIZED.into(), + unauthorized: true, + advertises_oauth: true, + }, + ); + let wire = serde_json::to_value(&outcome).unwrap(); + assert_eq!( + wire, + json!({ + "kind": "mcp_call", + "server": "docs", + "tool": "search", + "ok": false, + "error": { + "code": "ai.tinyhumans.tinymcp.Error.Unauthorized", + "unauthorized": true, + "advertises_oauth": true, + }, + }) + ); + assert_eq!( + serde_json::from_value::(wire).unwrap(), + outcome + ); +} + +#[test] +fn a_plain_error_carries_no_authorization_signal() { + let error = super::McpCallError::new(crate::errors::TRANSPORT); + assert_eq!(error.code, crate::errors::TRANSPORT); + assert!(!error.unauthorized); + assert!(!error.advertises_oauth); +} + +#[test] +fn an_outcome_is_read_back_only_from_metadata_of_its_kind() { + let outcome = super::McpCallOutcome::answered("docs", "search"); + let metadata = serde_json::to_value(&outcome).unwrap(); + assert_eq!( + super::McpCallOutcome::from_metadata(&metadata), + Some(outcome) + ); + + assert_eq!( + super::McpCallOutcome::from_metadata(&json!({ "kind": "web_search" })), + None + ); + assert_eq!(super::McpCallOutcome::from_metadata(&json!([1, 2])), None); + assert_eq!( + super::McpCallOutcome::from_metadata(&json!({ "kind": "mcp_call", "ok": "yes" })), + None + ); +} diff --git a/crates/tinymcp-bus/src/agent_tools/types.rs b/crates/tinymcp-bus/src/agent_tools/types.rs index 82d4e0f0..e01d5053 100644 --- a/crates/tinymcp-bus/src/agent_tools/types.rs +++ b/crates/tinymcp-bus/src/agent_tools/types.rs @@ -78,3 +78,95 @@ pub struct AgentToolSpec { /// A deferred tool costs nothing in the prompt until a model looks for it. pub deferred: bool, } + +/// The `kind` discriminator an [`McpCallOutcome`] carries. +/// +/// A host that receives a tool result's metadata as an untyped JSON object +/// tells this payload apart from other tools' by this value. +pub const MCP_CALL_RESULT_KIND: &str = "mcp_call"; + +/// What one `mcp_call_tool` call did, as structured data for the host. +/// +/// Attached to the call's result as host-only metadata, never rendered to the +/// model. A host reads it instead of parsing the result text to meter calls +/// that reached a server and to surface the ones that did not. +/// +/// `ok` says whether the server answered the call. A tool that answered with +/// its own error result still has `ok: true`: the server was reached and the +/// failure is the tool's, reported in the result itself. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct McpCallOutcome { + /// Always [`MCP_CALL_RESULT_KIND`]. + pub kind: String, + /// The server the call was aimed at, as the caller named it. + pub server: String, + /// The tool the call was aimed at, as the caller named it. + pub tool: String, + /// Whether the server answered the call. + pub ok: bool, + /// Why the call did not reach an answer. `None` when `ok` is `true`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +impl McpCallOutcome { + /// A call the server answered. + #[must_use] + pub fn answered(server: impl Into, tool: impl Into) -> Self { + Self { + kind: MCP_CALL_RESULT_KIND.to_string(), + server: server.into(), + tool: tool.into(), + ok: true, + error: None, + } + } + + /// A call that failed before the server answered it. + #[must_use] + pub fn failed(server: impl Into, tool: impl Into, error: McpCallError) -> Self { + Self { + kind: MCP_CALL_RESULT_KIND.to_string(), + server: server.into(), + tool: tool.into(), + ok: false, + error: Some(error), + } + } + + /// Reads an outcome out of a tool result's metadata. + /// + /// `None` when `metadata` is not an object whose `kind` is + /// [`MCP_CALL_RESULT_KIND`], or does not decode as an outcome. + #[must_use] + pub fn from_metadata(metadata: &Value) -> Option { + if metadata.get("kind").and_then(Value::as_str) != Some(MCP_CALL_RESULT_KIND) { + return None; + } + serde_json::from_value(metadata.clone()).ok() + } +} + +/// Why an MCP call failed, classified for a host. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct McpCallError { + /// The error's wire name: one of the constants in [`crate::errors`]. + pub code: String, + /// Whether the server answered HTTP 401 and wants credentials. + pub unauthorized: bool, + /// Whether that 401 advertised OAuth, so the host offers a sign-in rather + /// than a token field. Always `false` when `unauthorized` is `false`. + pub advertises_oauth: bool, +} + +impl McpCallError { + /// An error classified under `code` with no authorization signal. + #[must_use] + pub fn new(code: impl Into) -> Self { + Self { + code: code.into(), + unauthorized: false, + advertises_oauth: false, + } + } +} diff --git a/crates/tinymcp-bus/src/lib.rs b/crates/tinymcp-bus/src/lib.rs index e0be61c8..2126ad2f 100644 --- a/crates/tinymcp-bus/src/lib.rs +++ b/crates/tinymcp-bus/src/lib.rs @@ -25,8 +25,9 @@ //! records. //! - [`audit`] — the durable record of every write an MCP tool performed. //! - [`sanitize`] — the stripping pipeline applied to untrusted remote text. -//! - [`agent_tools`] — the tool specs a host exposes to a model, and the -//! normalization every forwarded call's `arguments` goes through. +//! - [`agent_tools`] — the tool specs a host exposes to a model, the +//! normalization every forwarded call's `arguments` goes through, and the +//! structured outcome a forwarded call reports to the host. //! - [`version`] — [`CONTRACT_VERSION`] and the [`is_compatible`] bind rule. //! //! # What is deliberately not here @@ -129,8 +130,8 @@ pub mod transport; pub mod version; pub use agent_tools::{ - AgentToolEffect, AgentToolSpec, ArgsError, RegistryTool, normalize_tool_arguments, - registry_tool_specs, + AgentToolEffect, AgentToolSpec, ArgsError, MCP_CALL_RESULT_KIND, McpCallError, McpCallOutcome, + RegistryTool, normalize_tool_arguments, registry_tool_specs, }; pub use audit::{ DEFAULT_LIST_LIMIT, ERROR_MESSAGE_MAX_BYTES, MAX_LIST_LIMIT, McpWriteListQuery, McpWriteRecord, diff --git a/crates/tinymcp-bus/src/version/mod.rs b/crates/tinymcp-bus/src/version/mod.rs index ecfe484d..ad809726 100644 --- a/crates/tinymcp-bus/src/version/mod.rs +++ b/crates/tinymcp-bus/src/version/mod.rs @@ -9,8 +9,9 @@ /// The wire contract version this crate defines. /// -/// 1.1 added agent tools; 1.2 added registry and directory members. -pub const CONTRACT_VERSION: (u32, u32) = (1, 2); +/// 1.1 added agent tools; 1.2 added registry and directory members; 1.3 added +/// the structured call outcome and the credential-store error name. +pub const CONTRACT_VERSION: (u32, u32) = (1, 3); /// Returns whether a host holding [`CONTRACT_VERSION`] can bind to a module /// reporting `module`. diff --git a/crates/tinymcp-bus/src/version/mod_tests.rs b/crates/tinymcp-bus/src/version/mod_tests.rs index 10211e88..e7ab0980 100644 --- a/crates/tinymcp-bus/src/version/mod_tests.rs +++ b/crates/tinymcp-bus/src/version/mod_tests.rs @@ -4,13 +4,14 @@ use super::{CONTRACT_VERSION, binds, is_compatible}; #[test] fn the_shipped_contract_version_is_pinned() { - assert_eq!(CONTRACT_VERSION, (1, 2)); + assert_eq!(CONTRACT_VERSION, (1, 3)); } #[test] fn a_host_on_this_contract_refuses_a_module_from_before_it() { - // 1.1 added agent tools; a 1.0 module does not carry them. + // 1.3 added the call outcome; a 1.2 module does not report it. assert!(!is_compatible((1, 0))); + assert!(!is_compatible((1, 2))); } #[test] @@ -20,7 +21,7 @@ fn the_contract_binds_to_itself() { #[test] fn a_newer_minor_on_the_module_side_binds() { - assert!(is_compatible((1, 2))); + assert!(is_compatible((1, 3))); assert!(is_compatible((1, 97))); } diff --git a/crates/tinymcp/src/lib.rs b/crates/tinymcp/src/lib.rs index a7fbb11a..a72386cf 100644 --- a/crates/tinymcp/src/lib.rs +++ b/crates/tinymcp/src/lib.rs @@ -115,13 +115,13 @@ pub use tinymcp_bus::{ AuthDetection, AuthKind, AuthorizationServerMetadata, CONTRACT_VERSION, ChatTurn, CommandKind, ConnStatus, ConnectedServerOverview, DEFAULT_LIST_LIMIT, ERROR_MESSAGE_MAX_BYTES, ExtraFields, HEADER_PROTOCOL_VERSION, HEADER_SESSION_ID, HttpHeader, INTERFACE, InstalledServer, - LATEST_PROTOCOL_VERSION, MAX_DESCRIPTION_BYTES, MAX_LIST_LIMIT, MAX_TITLE_BYTES, METHODS, - McpAuthChallenge, McpAuthConfig, McpAuthHint, McpAuthorizationContext, McpClientConfig, - McpClientIdentityConfig, McpClientInfo, McpInitializeResult, McpProxyConfig, - McpRegistryAuthConfig, McpRemoteTool, McpServerConfig, McpServerToolResult, McpSseEvent, - McpTool, McpToolContent, McpToolResult, McpWriteListQuery, McpWriteRecord, NewMcpWriteRecord, - OBJECT_PATH, ProtectedResourceMetadata, RegistryConnection, RegistryListResponse, - RegistryPagination, RegistryServerDetail, RegistryServerSummary, SUPPORTED_PROTOCOL_VERSIONS, - SearchCuration, ServerDetail, ServerStatus, Transport, config, is_compatible, names, sanitize, - version, + LATEST_PROTOCOL_VERSION, MAX_DESCRIPTION_BYTES, MAX_LIST_LIMIT, MAX_TITLE_BYTES, + MCP_CALL_RESULT_KIND, METHODS, McpAuthConfig, McpAuthHint, McpAuthorizationContext, + McpCallError, McpCallOutcome, McpClientConfig, McpClientIdentityConfig, McpClientInfo, + McpInitializeResult, McpProxyConfig, McpRegistryAuthConfig, McpRemoteTool, McpServerConfig, + McpServerToolResult, McpSseEvent, McpTool, McpToolContent, McpToolResult, McpWriteListQuery, + McpWriteRecord, NewMcpWriteRecord, OBJECT_PATH, ProtectedResourceMetadata, RegistryConnection, + RegistryListResponse, RegistryPagination, RegistryServerDetail, RegistryServerSummary, + SUPPORTED_PROTOCOL_VERSIONS, SearchCuration, ServerDetail, ServerStatus, Transport, config, + is_compatible, names, sanitize, version, }; From 08585535213b3cbe3c7f8572a9a8133d861f629f Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 14:40:10 +0530 Subject: [PATCH 02/21] feat(tools): attach McpCallOutcome metadata to mcp_call_tool (#40) Once the server and tool are known, every result McpCallTool returns carries a serialized McpCallOutcome in ToolResult.metadata: answered on success, and on a registry error or an arguments refusal the error's wire name plus whether it was a 401 that advertised OAuth. The metadata is host-only and is set after scrub_result, so it survives scrubbing and never reaches the model-facing rendering. Hosts that forward metadata carrying a "kind" key (OpenHuman's tool outcome capture) receive it as structured completion data, which is what lets a host meter answered calls and surface failures without parsing text. --- crates/tinymcp/src/tools/bridge.rs | 65 +++++- .../tinymcp/src/tools/bridge_outcome_tests.rs | 214 ++++++++++++++++++ crates/tinymcp/src/tools/mod.rs | 3 + 3 files changed, 274 insertions(+), 8 deletions(-) create mode 100644 crates/tinymcp/src/tools/bridge_outcome_tests.rs diff --git a/crates/tinymcp/src/tools/bridge.rs b/crates/tinymcp/src/tools/bridge.rs index ba08d66b..82aeccf3 100644 --- a/crates/tinymcp/src/tools/bridge.rs +++ b/crates/tinymcp/src/tools/bridge.rs @@ -9,6 +9,12 @@ //! //! What stays with the host is the decision to allow a call at all: //! [`McpCallTool`] takes an [`ActGate`] it runs before anything is sent. +//! +//! Every result [`McpCallTool`] returns once it knows the server and tool +//! carries a [`McpCallOutcome`] as its metadata: whether the server answered +//! and, when it did not, the error's wire name and whether it was a 401 that +//! advertised OAuth. The model never sees it; a host reads it to meter calls +//! and to surface failures without parsing the result text. // The tool names and descriptions are fixed strings, and the rendered Markdown // is built a line at a time; both are clearer as written. @@ -18,7 +24,7 @@ use std::sync::Arc; use async_trait::async_trait; use serde_json::{Value, json}; -use tinymcp_bus::McpAuthConfig; +use tinymcp_bus::{McpAuthConfig, McpCallError, McpCallOutcome}; use tinytools::{PermissionLevel, Tool, ToolCallOptions, ToolResult}; use super::scrub::SecretScrubber; @@ -296,6 +302,7 @@ impl Tool for McpCallTool { let server = required_string_arg(&args, "server")?; let tool = required_string_arg(&args, "tool")?; + let scrubber = SecretScrubber::for_server(&self.registry, &server); let arguments = args .get("arguments") .cloned() @@ -304,15 +311,39 @@ impl Tool for McpCallTool { // cannot hold one is refused naming what arrived. let arguments = match tinymcp_bus::normalize_tool_arguments(arguments) { Ok(arguments) => Value::Object(arguments), - Err(error) => return Ok(ToolResult::error(format!("`arguments`: {error}"))), + Err(error) => { + let outcome = McpCallOutcome::failed( + scrubber.scrub(&server), + scrubber.scrub(&tool), + McpCallError::new(tinymcp_bus::errors::INVALID_ARGUMENTS), + ); + return Ok(with_outcome( + ToolResult::error(format!("`arguments`: {error}")), + &outcome, + )); + } }; - let scrubber = SecretScrubber::for_server(&self.registry, &server); - let mut result = match self.registry.call_tool(&server, &tool, arguments).await { - Ok(result) => result.rendered, + let (mut result, outcome) = match self.registry.call_tool(&server, &tool, arguments).await { + Ok(result) => ( + result.rendered, + McpCallOutcome::answered(scrubber.scrub(&server), scrubber.scrub(&tool)), + ), Err(err) => { - return Ok(ToolResult::error( - scrubber.scrub(&format!("mcp_call_tool failed: {err}")), + let outcome = McpCallOutcome::failed( + scrubber.scrub(&server), + scrubber.scrub(&tool), + call_error(&err), + ); + tracing::debug!( + server = %outcome.server, + tool = %outcome.tool, + code = err.wire_name(), + "[mcp] mcp_call_tool failed" + ); + return Ok(with_outcome( + ToolResult::error(scrubber.scrub(&format!("mcp_call_tool failed: {err}"))), + &outcome, )); } }; @@ -320,7 +351,10 @@ impl Tool for McpCallTool { if options.prefer_markdown && result.markdown_formatted.is_none() { result.markdown_formatted = Some(result.output()); } - Ok(scrubber.scrub_result(super::tool_result(result))) + Ok(with_outcome( + scrubber.scrub_result(super::tool_result(result)), + &outcome, + )) } async fn execute(&self, args: Value) -> anyhow::Result { @@ -329,6 +363,21 @@ impl Tool for McpCallTool { } } +/// `result` carrying `outcome` as its host-only metadata. +fn with_outcome(mut result: ToolResult, outcome: &McpCallOutcome) -> ToolResult { + result.metadata = serde_json::to_value(outcome).ok(); + result +} + +/// The host-facing classification of a failed call. +fn call_error(error: &crate::Error) -> McpCallError { + McpCallError { + code: error.wire_name().to_string(), + unauthorized: error.is_unauthorized(), + advertises_oauth: error.advertises_oauth(), + } +} + fn auth_kind(auth: &McpAuthConfig) -> &'static str { match auth { McpAuthConfig::None => "none", diff --git a/crates/tinymcp/src/tools/bridge_outcome_tests.rs b/crates/tinymcp/src/tools/bridge_outcome_tests.rs new file mode 100644 index 00000000..72d9e81e --- /dev/null +++ b/crates/tinymcp/src/tools/bridge_outcome_tests.rs @@ -0,0 +1,214 @@ +//! Tests for the structured outcome `mcp_call_tool` attaches to its results: +//! what it reports for an answered call and for each way a call fails, that +//! the model-facing rendering never carries it, and that scrubbing keeps it. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::sync::Arc; + +use axum::http::{HeaderMap, HeaderValue, StatusCode}; +use axum::response::IntoResponse as _; +use axum::routing::post; +use axum::{Json, Router}; +use serde_json::{Value, json}; +use tinymcp_bus::{ + MCP_CALL_RESULT_KIND, McpAuthConfig, McpCallOutcome, McpClientConfig, McpServerConfig, errors, +}; +use tinytools::{Tool, ToolResult}; + +use super::{ActGate, McpCallTool}; +use crate::McpServerRegistry; + +const SECRET: &str = "outcome-Secret_77"; + +fn registry(endpoint: &str, auth: McpAuthConfig, disallowed: &[&str]) -> Arc { + Arc::new( + McpServerRegistry::from_config(&McpClientConfig { + servers: vec![McpServerConfig { + name: "docs".into(), + endpoint: endpoint.into(), + auth, + disallowed_tools: disallowed.iter().map(|tool| (*tool).to_string()).collect(), + ..McpServerConfig::default() + }], + ..McpClientConfig::default() + }) + .unwrap(), + ) +} + +fn call_tool(registry: Arc) -> McpCallTool { + let gate: ActGate = Arc::new(|_| Ok(())); + McpCallTool::new(registry, gate) +} + +fn outcome_of(result: &ToolResult) -> McpCallOutcome { + McpCallOutcome::from_metadata(result.metadata.as_ref().expect("metadata is attached")) + .expect("metadata decodes as a call outcome") +} + +async fn answering_server() -> String { + async fn handle(Json(body): Json) -> axum::response::Response { + let result = match body["method"].as_str().unwrap_or_default() { + "initialize" => json!({ + "protocolVersion": tinymcp_bus::LATEST_PROTOCOL_VERSION, + "capabilities": { "tools": {} }, + "serverInfo": { "name": "echo", "version": "1.0.0" }, + }), + "notifications/initialized" => return StatusCode::ACCEPTED.into_response(), + "tools/list" => json!({ + "tools": [{ "name": "whoami", "inputSchema": { "type": "object" } }] + }), + "tools/call" => json!({ + "content": [{ "type": "text", "text": format!("you sent {SECRET}") }], + "isError": false, + }), + _ => json!({}), + }; + Json(json!({ "jsonrpc": "2.0", "id": body["id"].clone(), "result": result })) + .into_response() + } + serve(Router::new().route("/mcp", post(handle))).await +} + +async fn unauthorized_server(resource_metadata: Option<&'static str>) -> String { + let app = Router::new().route( + "/mcp", + post(move || async move { + let mut headers = HeaderMap::new(); + let challenge = resource_metadata.map_or_else( + || "Bearer realm=\"mcp\"".to_string(), + |url| format!("Bearer resource_metadata=\"{url}\""), + ); + headers.insert( + "www-authenticate", + HeaderValue::from_str(&challenge).unwrap(), + ); + (StatusCode::UNAUTHORIZED, headers, "").into_response() + }), + ); + serve(app).await +} + +async fn serve(app: Router) -> String { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + format!("http://{addr}/mcp") +} + +async fn closed_endpoint() -> String { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + drop(listener); + format!("http://{addr}/mcp") +} + +fn args(tool: &str) -> Value { + json!({ "server": "docs", "tool": tool, "arguments": {} }) +} + +#[tokio::test] +async fn an_answered_call_reports_ok_with_no_error() { + let registry = registry(&answering_server().await, McpAuthConfig::None, &[]); + let result = call_tool(registry).execute(args("whoami")).await.unwrap(); + assert!(!result.is_error, "{}", result.output()); + assert_eq!( + outcome_of(&result), + McpCallOutcome::answered("docs", "whoami") + ); + assert_eq!( + result.metadata.as_ref().unwrap()["kind"], + json!(MCP_CALL_RESULT_KIND) + ); +} + +#[tokio::test] +async fn the_outcome_survives_scrubbing_and_never_reaches_the_rendering() { + let registry = registry( + &answering_server().await, + McpAuthConfig::BearerToken { + token: SECRET.into(), + }, + &[], + ); + let result = call_tool(registry).execute(args("whoami")).await.unwrap(); + assert!(result.output().contains(super::scrub::REDACTED)); + assert!(!result.output().contains(SECRET)); + assert!(outcome_of(&result).ok); + assert!(!result.output().contains(MCP_CALL_RESULT_KIND)); +} + +#[tokio::test] +async fn a_blocked_tool_reports_tool_not_allowed() { + let registry = registry(&answering_server().await, McpAuthConfig::None, &["wipe"]); + let result = call_tool(registry).execute(args("wipe")).await.unwrap(); + assert!(result.is_error); + let outcome = outcome_of(&result); + assert!(!outcome.ok); + assert_eq!(outcome.tool, "wipe"); + let error = outcome.error.unwrap(); + assert_eq!(error.code, errors::TOOL_NOT_ALLOWED); + assert!(!error.unauthorized); + assert!(!error.advertises_oauth); +} + +#[tokio::test] +async fn a_401_advertising_oauth_reports_both_flags() { + let endpoint = unauthorized_server(Some("https://auth.example/.well-known/res")).await; + let result = call_tool(registry(&endpoint, McpAuthConfig::None, &[])) + .execute(args("whoami")) + .await + .unwrap(); + assert!(result.is_error); + let error = outcome_of(&result).error.unwrap(); + assert_eq!(error.code, errors::UNAUTHORIZED); + assert!(error.unauthorized); + assert!(error.advertises_oauth); +} + +#[tokio::test] +async fn a_401_without_resource_metadata_is_unauthorized_without_oauth() { + let endpoint = unauthorized_server(None).await; + let result = call_tool(registry(&endpoint, McpAuthConfig::None, &[])) + .execute(args("whoami")) + .await + .unwrap(); + let error = outcome_of(&result).error.unwrap(); + assert_eq!(error.code, errors::UNAUTHORIZED); + assert!(error.unauthorized); + assert!(!error.advertises_oauth); +} + +#[tokio::test] +async fn an_unknown_server_reports_unknown_server() { + let registry = registry(&closed_endpoint().await, McpAuthConfig::None, &[]); + let result = call_tool(registry) + .execute(json!({ "server": "nope", "tool": "whoami", "arguments": {} })) + .await + .unwrap(); + let outcome = outcome_of(&result); + assert_eq!(outcome.server, "nope"); + assert_eq!(outcome.error.unwrap().code, errors::UNKNOWN_SERVER); +} + +#[tokio::test] +async fn an_unreachable_server_reports_a_transport_error() { + let registry = registry(&closed_endpoint().await, McpAuthConfig::None, &[]); + let result = call_tool(registry).execute(args("whoami")).await.unwrap(); + assert!(result.is_error); + assert_eq!(outcome_of(&result).error.unwrap().code, errors::TRANSPORT); +} + +#[tokio::test] +async fn arguments_that_are_not_an_object_report_invalid_arguments() { + let registry = registry(&closed_endpoint().await, McpAuthConfig::None, &[]); + let result = call_tool(registry) + .execute(json!({ "server": "docs", "tool": "whoami", "arguments": [1] })) + .await + .unwrap(); + assert!(result.is_error); + let outcome = outcome_of(&result); + assert_eq!(outcome.tool, "whoami"); + assert_eq!(outcome.error.unwrap().code, errors::INVALID_ARGUMENTS); +} diff --git a/crates/tinymcp/src/tools/mod.rs b/crates/tinymcp/src/tools/mod.rs index 1fa439c8..11b63643 100644 --- a/crates/tinymcp/src/tools/mod.rs +++ b/crates/tinymcp/src/tools/mod.rs @@ -102,6 +102,9 @@ pub fn tools_for( #[path = "mod_tests.rs"] mod test; +#[cfg(test)] +#[path = "bridge_outcome_tests.rs"] +mod bridge_outcome_test; #[cfg(test)] #[path = "bridge_tests.rs"] mod bridge_test; From e6271d24c657cf03358dcb93cf3b652d5413c509 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 14:40:34 +0530 Subject: [PATCH 03/21] feat(tools): strip markdown fences from bridge identifiers (#40) Models answering in prose wrap server and tool names in markdown (`docs`, *docs*, docs.), which made the registry lookup miss and surfaced "unknown mcp server `docs``". The bridge now trims leading backtick/asterisk/underscore and trailing fence or punctuation characters, matching the OpenCompany wrapper this replaces, and still refuses a value that is nothing but fences. --- crates/tinymcp/src/tools/bridge.rs | 18 ++++++++++++--- crates/tinymcp/src/tools/bridge_tests.rs | 28 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/crates/tinymcp/src/tools/bridge.rs b/crates/tinymcp/src/tools/bridge.rs index 82aeccf3..300cf6c3 100644 --- a/crates/tinymcp/src/tools/bridge.rs +++ b/crates/tinymcp/src/tools/bridge.rs @@ -404,12 +404,24 @@ pub(super) fn endpoint_without_query(endpoint: &str) -> String { .to_string() } -fn required_string_arg(args: &Value, key: &str) -> anyhow::Result { +/// A required identifier argument, trimmed and without the markdown a model +/// wraps it in when it answers in prose (`` `docs` ``, `*docs*`, `docs.`). +/// +/// Only leading and trailing fence characters go: a server or tool name never +/// starts or ends with one, so stripping cannot change a real name. +pub(super) fn required_string_arg(args: &Value, key: &str) -> anyhow::Result { + let missing = || anyhow::anyhow!("missing required `{key}`"); let value = args .get(key) .and_then(Value::as_str) .map(str::trim) .filter(|s| !s.is_empty()) - .ok_or_else(|| anyhow::anyhow!("missing required `{key}`"))?; - Ok(value.to_string()) + .ok_or_else(missing)?; + let cleaned = value + .trim_start_matches(['`', '*', '_']) + .trim_end_matches(['`', '*', '_', '.', ',', ';', ':', '!']); + if cleaned.is_empty() { + return Err(missing()); + } + Ok(cleaned.to_string()) } diff --git a/crates/tinymcp/src/tools/bridge_tests.rs b/crates/tinymcp/src/tools/bridge_tests.rs index 5fd27c8a..649ae9b2 100644 --- a/crates/tinymcp/src/tools/bridge_tests.rs +++ b/crates/tinymcp/src/tools/bridge_tests.rs @@ -487,3 +487,31 @@ fn an_endpoint_that_is_not_a_url_is_cut_at_its_query_or_fragment() { "https://example.com/mcp" ); } + +#[test] +fn identifiers_lose_the_markdown_a_model_wraps_them_in() { + use super::bridge::required_string_arg; + let parsed = |value: &str| required_string_arg(&json!({ "server": value }), "server").unwrap(); + assert_eq!(parsed("docs"), "docs"); + assert_eq!(parsed("docs`"), "docs"); + assert_eq!(parsed("`docs`"), "docs"); + assert_eq!(parsed("*docs*"), "docs"); + assert_eq!(parsed("docs."), "docs"); + assert_eq!(parsed(" **docs**: "), "docs"); + assert_eq!(parsed("my_docs-v2"), "my_docs-v2"); + let error = required_string_arg(&json!({ "server": "```" }), "server").unwrap_err(); + assert_eq!(error.to_string(), "missing required `server`"); + assert!(required_string_arg(&json!({ "server": 7 }), "server").is_err()); +} + +#[tokio::test] +async fn a_fenced_server_name_reaches_the_configured_server() { + let (base, calls) = echoing_server("plain", None).await; + let registry = registry_with(&format!("{base}/mcp"), McpAuthConfig::None); + let result = call_tool(registry) + .execute(json!({ "server": "`docs`", "tool": "whoami`", "arguments": {} })) + .await + .unwrap(); + assert!(!result.is_error, "{}", full_output(&result)); + assert_eq!(calls.lock().len(), 1); +} From a8d08331135cd9590a81eef39d53722de4d1dec3 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 14:41:02 +0530 Subject: [PATCH 04/21] feat(tools): add SecretScrubber::with_secrets (#40) A host that keeps a server's credential in its own secret store, rather than in the McpServerConfig the scrubber is built from, can now add those values so they are redacted from bridge output too. Extra values are matched as typed and URL-encoded, blanks are skipped, and the combined list stays sorted longest first. --- crates/tinymcp/src/tools/scrub.rs | 31 +++++++++++++++++++++++-- crates/tinymcp/src/tools/scrub_tests.rs | 30 ++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/crates/tinymcp/src/tools/scrub.rs b/crates/tinymcp/src/tools/scrub.rs index f57133bd..447cb4a3 100644 --- a/crates/tinymcp/src/tools/scrub.rs +++ b/crates/tinymcp/src/tools/scrub.rs @@ -132,11 +132,32 @@ impl SecretScrubber { } secrets.push(value.to_string()); } - secrets.sort_by(|a, b| b.len().cmp(&a.len()).then_with(|| a.cmp(b))); - secrets.dedup(); + sort_longest_first(&mut secrets); Self { secrets, strict } } + /// This scrubber, also replacing each of `secrets`. + /// + /// For credentials the server's own configuration does not carry: a token + /// a host keeps in its own secret store, or a value it injected some other + /// way. Each is matched as typed and URL-encoded; blank values are skipped. + #[must_use] + pub fn with_secrets(mut self, secrets: impl IntoIterator) -> Self { + for secret in secrets { + let secret = secret.trim(); + if secret.is_empty() { + continue; + } + let encoded = urlencoding::encode(secret).into_owned(); + if encoded != secret { + self.secrets.push(encoded); + } + self.secrets.push(secret.to_string()); + } + sort_longest_first(&mut self.secrets); + self + } + /// `text` with every known secret replaced. #[must_use] pub fn scrub(&self, text: &str) -> String { @@ -255,6 +276,12 @@ impl SecretScrubber { } } +/// Longest first, so a secret that contains another is replaced whole. +fn sort_longest_first(secrets: &mut Vec) { + secrets.sort_by(|a, b| b.len().cmp(&a.len()).then_with(|| a.cmp(b))); + secrets.dedup(); +} + fn endpoint_query(endpoint: &str) -> Option<&str> { let (_, rest) = endpoint.split_once('?')?; let query = rest.split('#').next().unwrap_or_default(); diff --git a/crates/tinymcp/src/tools/scrub_tests.rs b/crates/tinymcp/src/tools/scrub_tests.rs index 6ba8aa94..681f23f4 100644 --- a/crates/tinymcp/src/tools/scrub_tests.rs +++ b/crates/tinymcp/src/tools/scrub_tests.rs @@ -125,3 +125,33 @@ fn scrub_value_keeps_both_entries_when_keys_collide_after_redaction() { assert_eq!(map.get("[redacted]"), Some(&json!("a"))); assert_eq!(map.get("[redacted] (2)"), Some(&json!("b"))); } + +#[test] +fn extra_secrets_are_redacted_as_typed_and_url_encoded() { + let scrubber = SecretScrubber::new(&McpAuthConfig::None, "https://example.com/mcp") + .with_secrets(["host token/1".to_string(), " ".to_string()]); + assert_eq!( + scrubber.scrub("a host token/1 b host%20token%2F1 c"), + "a [redacted] b [redacted] c" + ); + assert_eq!(scrubber.secrets.len(), 2); +} + +#[test] +fn extra_secrets_join_the_configured_ones_longest_first() { + let scrubber = SecretScrubber::new( + &McpAuthConfig::BearerToken { + token: "configured-token".into(), + }, + "https://example.com/mcp", + ) + .with_secrets(["configured-token-extended".to_string()]); + assert_eq!( + scrubber.scrub("configured-token-extended and configured-token"), + "[redacted] and [redacted]" + ); + let result = scrubber.scrub_result(tinytools::ToolResult::json( + json!({ "echo": "configured-token-extended" }), + )); + assert!(!format!("{:?}", result.content).contains("configured-token")); +} From d3eabe6ec781b735bcf659d3fb294c6bf54e89fa Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 14:42:45 +0530 Subject: [PATCH 05/21] feat(oauth): public OAuthBundle and guarded OAuthFlow::refresh (#40) OAuthBundle is now public and re-exported from registry::oauth (and registry), so a host keeping credentials in its own store can read the bundle it holds without restating its shape. Its wire form is pinned. OAuthFlow::refresh runs the same refresh as the free refresh_if_expired, but when require_public_endpoints() is set it re-checks the bundle's token endpoint and posts over a client pinned to the vetted addresses. The free function previously documented that it skips the check because it only posts to an endpoint begin() checked; that assumption does not hold for a bundle written by an older build or edited in a host's store. refresh_if_expired is kept unchanged and now shares the due-check and exchange with the flow method. --- crates/tinymcp/src/registry/mod.rs | 2 +- crates/tinymcp/src/registry/oauth/flow.rs | 34 ++++- crates/tinymcp/src/registry/oauth/mod.rs | 4 +- .../src/registry/oauth/mod_refresh_tests.rs | 131 ++++++++++++++++++ .../tinymcp/src/registry/oauth/mod_tests.rs | 7 + crates/tinymcp/src/registry/oauth/tokens.rs | 46 +++++- crates/tinymcp/src/registry/oauth/types.rs | 24 ++-- 7 files changed, 225 insertions(+), 23 deletions(-) create mode 100644 crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs diff --git a/crates/tinymcp/src/registry/mod.rs b/crates/tinymcp/src/registry/mod.rs index 8aad1335..c39a9509 100644 --- a/crates/tinymcp/src/registry/mod.rs +++ b/crates/tinymcp/src/registry/mod.rs @@ -23,7 +23,7 @@ pub mod supervisor; pub use boot::{BootOutcome, connect_installed_servers}; pub use config_doc::{AppliedServer, ConfigApplyReport}; pub use connections::{Connections, ProbeOutcome, REMOTE_REQUEST_TIMEOUT}; -pub use oauth::{AuthDetection, AuthKind, OAuthCredentialStore, OAuthFlow}; +pub use oauth::{AuthDetection, AuthKind, OAuthBundle, OAuthCredentialStore, OAuthFlow}; pub use ops::McpRegistry; pub use setup::{SecretRef, SecretVault}; pub use sources::{Registries, RegistrySource}; diff --git a/crates/tinymcp/src/registry/oauth/flow.rs b/crates/tinymcp/src/registry/oauth/flow.rs index f33d43dd..d1a4c8f7 100644 --- a/crates/tinymcp/src/registry/oauth/flow.rs +++ b/crates/tinymcp/src/registry/oauth/flow.rs @@ -11,7 +11,7 @@ use sha2::{Digest, Sha256}; use super::credentials::OAuthCredentialStore; use super::endpoint_guard::{guard_endpoint, guarded_client}; -use super::tokens::{oauth_failure_reason, persist, post_form}; +use super::tokens::{due_refresh, exchange_refresh, oauth_failure_reason, persist, post_form}; use super::types::{AuthDetection, PendingAuthorization, TokenResponse}; use crate::error::{Error, Result}; use crate::transport::http::McpHttpClient; @@ -93,8 +93,9 @@ impl OAuthFlow { /// trust turns this on. Off by default: a desktop host signs in to loopback /// development servers. The token endpoint is re-checked when a code is /// exchanged, so a host re-pointed between `begin` and `complete` is - /// refused too. [`refresh_if_expired`](super::refresh_if_expired) does not - /// re-check: it only ever posts to an endpoint this guard already passed. + /// refused too, and again by [`Self::refresh`] before each refresh. The + /// free [`refresh_if_expired`](super::refresh_if_expired) does not + /// re-check; a host that turns this on refreshes through [`Self::refresh`]. #[must_use] pub fn require_public_endpoints(mut self) -> Self { self.public_endpoints_only = true; @@ -331,6 +332,33 @@ impl OAuthFlow { Ok(pending.server_id) } + /// Mints a new access token when the stored one has expired, or is about + /// to, as [`refresh_if_expired`](super::refresh_if_expired) does. + /// + /// Returns whether a refresh happened. Under + /// [`Self::require_public_endpoints`] the bundle's token endpoint is + /// checked again, and the refresh posted over a client pinned to the + /// addresses it was checked against, so a bundle stored by an older build + /// or edited in a host's store cannot aim a refresh at an internal address. + /// + /// # Errors + /// + /// Returns [`Error::MalformedResponse`] when the stored bundle cannot be + /// read, when the token endpoint is refused by the guard, or when it answers + /// with something unusable; [`Error::Http`] when it answers with a failure + /// status; and [`Error::Transport`] when it cannot be reached. + pub async fn refresh(&self, store: &S, server_id: &str) -> Result + where + S: OAuthCredentialStore + ?Sized, + { + let Some(bundle) = due_refresh(store, server_id).await? else { + return Ok(false); + }; + let guarded_http = self.token_client(&bundle.token_endpoint).await?; + let http = guarded_http.as_ref().unwrap_or(&self.http); + exchange_refresh(store, http, server_id, &bundle).await + } + /// The server a parked authorization is for, without consuming it. /// /// A browser redirect carries no session, so a host keeping credentials diff --git a/crates/tinymcp/src/registry/oauth/mod.rs b/crates/tinymcp/src/registry/oauth/mod.rs index 49649612..082fbfed 100644 --- a/crates/tinymcp/src/registry/oauth/mod.rs +++ b/crates/tinymcp/src/registry/oauth/mod.rs @@ -40,7 +40,7 @@ //! The access token is stored as the server's `Authorization` header value, so //! the ordinary connect path picks it up with no special case. The bookkeeping //! needed to mint a new one — refresh token, client credentials, token endpoint, -//! expiry — is stored beside it under [`OAUTH_BUNDLE_KEY`]. That key begins with +//! expiry — is an [`OAuthBundle`] stored beside it under [`OAUTH_BUNDLE_KEY`]. That key begins with //! two underscores, which is the marker meaning "never send this as a request //! header and never show it in a credential list". @@ -53,7 +53,7 @@ pub(crate) mod types; pub use credentials::OAuthCredentialStore; pub use flow::OAuthFlow; pub use tokens::{OAUTH_BUNDLE_KEY, refresh_if_expired}; -pub use types::{AuthDetection, AuthKind}; +pub use types::{AuthDetection, AuthKind, OAuthBundle}; #[cfg(test)] #[path = "mod_tests.rs"] diff --git a/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs new file mode 100644 index 00000000..fa5b3c1d --- /dev/null +++ b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs @@ -0,0 +1,131 @@ +//! [`OAuthFlow::refresh`]: the flow's own refresh, which re-applies the +//! public-endpoint guard, and the public [`OAuthBundle`] wire form. + +use super::*; +use crate::registry::OAuthBundle; + +#[test] +fn the_bundle_wire_form_is_pinned_and_round_trips() { + let bundle = OAuthBundle { + refresh_token: Some("r1".into()), + client_id: "cli-1".into(), + client_secret: None, + token_endpoint: "https://auth.example/token".into(), + expires_at: 42, + }; + let wire = serde_json::to_value(&bundle).unwrap(); + assert_eq!( + wire, + json!({ + "refresh_token": "r1", + "client_id": "cli-1", + "client_secret": null, + "token_endpoint": "https://auth.example/token", + "expires_at": 42, + }) + ); + assert_eq!(serde_json::from_value::(wire).unwrap(), bundle); +} + +#[test] +fn a_stored_bundle_reads_back_as_the_public_type() { + let store = store_with_remote("https://example.test/mcp"); + store_expired_bundle(&store, "https://auth.example/token", Some("r1")); + let env = store.load_env_values("srv-1").unwrap(); + let bundle: OAuthBundle = serde_json::from_str(&env[OAUTH_BUNDLE_KEY]).unwrap(); + assert_eq!(bundle.client_secret.as_deref(), Some("sec-1")); + assert_eq!(bundle.refresh_token.as_deref(), Some("r1")); +} + +#[tokio::test] +async fn the_flow_refreshes_an_expired_token_when_unguarded() { + let requests = TokenRequests::default(); + let base = serve(token_endpoint( + requests.clone(), + json!({ "access_token": "fresh", "expires_in": 3600 }), + )) + .await; + let store = store_with_remote("https://example.test/mcp"); + store_expired_bundle(&store, &format!("{base}/token"), Some("r1")); + + let flow = OAuthFlow::new(None).unwrap(); + assert!(flow.refresh(&store, "srv-1").await.unwrap()); + assert_eq!(requests.count.load(Ordering::SeqCst), 1); + assert_eq!( + store.load_env_values("srv-1").unwrap()["Authorization"], + "Bearer fresh" + ); + + assert!(!flow.refresh(&store, "srv-1").await.unwrap()); + assert_eq!(requests.count.load(Ordering::SeqCst), 1); +} + +#[tokio::test] +async fn the_guarded_flow_refuses_a_loopback_token_endpoint_and_sends_nothing() { + let requests = TokenRequests::default(); + let base = serve(token_endpoint( + requests.clone(), + json!({ "access_token": "fresh" }), + )) + .await; + let store = store_with_remote("https://example.test/mcp"); + store_expired_bundle(&store, &format!("{base}/token"), Some("r1")); + + let flow = OAuthFlow::new(None).unwrap().require_public_endpoints(); + let error = flow.refresh(&store, "srv-1").await.unwrap_err(); + assert!( + matches!(&error, Error::MalformedResponse { detail } if detail.starts_with("token endpoint refused")), + "{error}" + ); + assert_eq!(requests.count.load(Ordering::SeqCst), 0); + assert_eq!( + store.load_env_values("srv-1").unwrap()["Authorization"], + "Bearer stale" + ); +} + +#[tokio::test] +async fn the_guarded_flow_refuses_an_https_endpoint_on_an_internal_address() { + let store = store_with_remote("https://example.test/mcp"); + store_expired_bundle(&store, "https://127.0.0.1:1/token", Some("r1")); + + let flow = OAuthFlow::new(None).unwrap().require_public_endpoints(); + let error = flow.refresh(&store, "srv-1").await.unwrap_err(); + assert!(error.to_string().contains("disallowed address"), "{error}"); +} + +#[tokio::test] +async fn the_guarded_flow_leaves_a_token_that_needs_no_refresh_alone() { + let store = store_with_remote("https://example.test/mcp"); + store_expired_bundle(&store, "http://127.0.0.1:1/token", None); + + let flow = OAuthFlow::new(None).unwrap().require_public_endpoints(); + assert!(!flow.refresh(&store, "srv-1").await.unwrap()); + assert!( + !flow + .refresh(&store_with_remote("https://x.test/mcp"), "srv-1") + .await + .unwrap() + ); +} + +#[tokio::test] +async fn an_unreadable_bundle_is_reported_by_the_flow() { + let store = store_with_remote("https://example.test/mcp"); + store + .set_env_values( + "srv-1", + &BTreeMap::from([(OAUTH_BUNDLE_KEY.to_string(), "not json".to_string())]), + ) + .unwrap(); + let error = OAuthFlow::new(None) + .unwrap() + .refresh(&store, "srv-1") + .await + .unwrap_err(); + assert!( + error + .to_string() + .contains("stored oauth bundle is unreadable") + ); +} diff --git a/crates/tinymcp/src/registry/oauth/mod_tests.rs b/crates/tinymcp/src/registry/oauth/mod_tests.rs index abba73c7..a3e94592 100644 --- a/crates/tinymcp/src/registry/oauth/mod_tests.rs +++ b/crates/tinymcp/src/registry/oauth/mod_tests.rs @@ -1229,3 +1229,10 @@ fn a_kind_serializes_as_the_same_spelling_it_reports() { #[path = "mod_host_store_tests.rs"] mod host_store; + +// --------------------------------------------------------------------------- +// The flow's own refresh +// --------------------------------------------------------------------------- + +#[path = "mod_refresh_tests.rs"] +mod flow_refresh; diff --git a/crates/tinymcp/src/registry/oauth/tokens.rs b/crates/tinymcp/src/registry/oauth/tokens.rs index 954c7100..05e256ae 100644 --- a/crates/tinymcp/src/registry/oauth/tokens.rs +++ b/crates/tinymcp/src/registry/oauth/tokens.rs @@ -51,29 +51,61 @@ pub async fn refresh_if_expired( http: &reqwest::Client, server_id: &str, ) -> Result +where + S: OAuthCredentialStore + ?Sized, +{ + let Some(bundle) = due_refresh(store, server_id).await? else { + return Ok(false); + }; + exchange_refresh(store, http, server_id, &bundle).await +} + +/// The stored bundle when its access token is due for a refresh and it holds a +/// refresh token to do it with; `None` otherwise. +/// +/// # Errors +/// +/// Returns [`Error::MalformedResponse`] when the stored bundle cannot be read, +/// and whatever the store reports when it cannot be read at all. +pub(super) async fn due_refresh(store: &S, server_id: &str) -> Result> where S: OAuthCredentialStore + ?Sized, { let env = store.load_credentials(server_id).await?; let Some(raw_bundle) = env.get(OAUTH_BUNDLE_KEY) else { - return Ok(false); + return Ok(None); }; let bundle: OAuthBundle = serde_json::from_str(raw_bundle) .map_err(|error| Error::malformed(format!("stored oauth bundle is unreadable: {error}")))?; if bundle.expires_at > now_unix() + REFRESH_SKEW_SECS { - return Ok(false); + return Ok(None); } - let Some(refresh_token) = bundle + let has_refresh_token = bundle .refresh_token .as_deref() - .filter(|token| !token.trim().is_empty()) - else { - return Ok(false); - }; + .is_some_and(|token| !token.trim().is_empty()); + Ok(has_refresh_token.then_some(bundle)) +} +/// Exchanges `bundle`'s refresh token at its token endpoint over `http` and +/// stores what comes back. +/// +/// # Errors +/// +/// As [`refresh_if_expired`]. +pub(super) async fn exchange_refresh( + store: &S, + http: &reqwest::Client, + server_id: &str, + bundle: &OAuthBundle, +) -> Result +where + S: OAuthCredentialStore + ?Sized, +{ + let refresh_token = bundle.refresh_token.as_deref().unwrap_or_default(); let mut form: Vec<(&str, &str)> = vec![ ("grant_type", "refresh_token"), ("refresh_token", refresh_token), diff --git a/crates/tinymcp/src/registry/oauth/types.rs b/crates/tinymcp/src/registry/oauth/types.rs index 821fe1af..97eb7ea8 100644 --- a/crates/tinymcp/src/registry/oauth/types.rs +++ b/crates/tinymcp/src/registry/oauth/types.rs @@ -33,21 +33,25 @@ pub(super) struct PendingAuthorization { /// The bookkeeping needed to mint a new access token without another sign-in. /// -/// Stored beside the access token under the reserved bundle key. The access -/// token itself is the `Authorization` header value, so the ordinary connect -/// path needs no special case for an OAuth server. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub(super) struct OAuthBundle { +/// Stored as JSON beside the access token under +/// [`OAUTH_BUNDLE_KEY`](super::OAUTH_BUNDLE_KEY). The access token itself is +/// the `Authorization` header value, so the ordinary connect path needs no +/// special case for an OAuth server. Public so a host keeping credentials in +/// its own store can read the bundle it holds — for an expiry it shows, or to +/// migrate one — without restating its shape. It holds a client secret and a +/// refresh token: never log or display it. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct OAuthBundle { /// The refresh token, when the server issued one. - pub(super) refresh_token: Option, + pub refresh_token: Option, /// The registered client. - pub(super) client_id: String, + pub client_id: String, /// The client secret, when there is one. - pub(super) client_secret: Option, + pub client_secret: Option, /// Where to refresh. - pub(super) token_endpoint: String, + pub token_endpoint: String, /// When the current access token expires, in Unix seconds. Best effort. - pub(super) expires_at: u64, + pub expires_at: u64, } /// A parsed token-endpoint reply. From cf2e0fbaf97a7ca66e4bf5894702d4262a34369f Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 14:44:55 +0530 Subject: [PATCH 06/21] feat(config_doc): parse_with host extension and render_declared (#40) A host that keeps MCP declarations in its own store (OpenCompany's company mcp.json and console document) needs the same Claude-shaped document reader without the install store's limits. Declared gains allowed_tools, disallowed_tools, timeout_secs and host_fields. parse_with(doc, &ParseOptions { host_fields, lenient }) reads allowedTools, disallowedTools and timeoutSecs, carries the host's registered fields verbatim, refuses two keys that trim to one name, and in lenient mode drops refused entries into ParseReport::rejected instead of refusing the document. An unreadable root is still refused. render_declared writes declarations back, never emitting credentials. The strict parse used by apply_config_doc is unchanged and keeps refusing the extension fields, since the install store has no columns for them and accepting them there would drop them silently. render (the store projection) is unchanged for the same reason. Adding fields to Declared breaks struct-literal construction in hosts. --- .../src/registry/config_doc/document.rs | 251 +++++++++++++-- crates/tinymcp/src/registry/config_doc/mod.rs | 24 +- .../config_doc/mod_parse_with_tests.rs | 304 ++++++++++++++++++ .../tinymcp/src/registry/config_doc/types.rs | 51 +++ 4 files changed, 604 insertions(+), 26 deletions(-) create mode 100644 crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs diff --git a/crates/tinymcp/src/registry/config_doc/document.rs b/crates/tinymcp/src/registry/config_doc/document.rs index 486a5c95..9f5eee46 100644 --- a/crates/tinymcp/src/registry/config_doc/document.rs +++ b/crates/tinymcp/src/registry/config_doc/document.rs @@ -4,7 +4,7 @@ use std::collections::BTreeMap; use serde_json::{Map, Value, json}; -use super::types::Declared; +use super::types::{Declared, ParseOptions, ParseReport, RejectedEntry}; use crate::error::{Error, Result}; use tinymcp_bus::{InstalledServer, Transport}; @@ -17,6 +17,21 @@ const ECHOED_FIELDS: &[&str] = &["envKeys", "authConfigured"]; /// Fields a write accepts without acting on, because other clients emit them. const TOLERATED_FIELDS: &[&str] = &["type", "transport"]; +/// The fields every entry may carry. +const STANDARD_FIELDS: &[&str] = &[ + "url", + "command", + "args", + "env", + "headers", + "description", + "enabled", +]; + +/// The fields [`parse_with`] reads and [`parse`] refuses, because the install +/// store has nowhere to keep them. +const EXTENSION_FIELDS: &[&str] = &["allowedTools", "disallowedTools", "timeoutSecs"]; + /// Credential names beginning this way are the store's own bookkeeping (the /// OAuth refresh bundle) and are neither rendered nor overwritten from here. const INTERNAL_KEY_PREFIX: &str = "__"; @@ -86,6 +101,51 @@ pub fn parse(doc: &Value) -> Result> { /// [`parse`], with the refusal as the bare sentence. fn parse_servers(doc: &Value) -> std::result::Result, String> { + let mut declared = Vec::new(); + for (name, entry) in server_entries(doc)? { + declared.push(read_entry(name, entry, None)?); + } + Ok(declared) +} + +/// Reads a document into declarations for a host that keeps its own store. +/// +/// As [`parse`], plus `allowedTools`, `disallowedTools` and `timeoutSecs`, the +/// host's own fields from [`ParseOptions::host_fields`], and a refusal of two +/// keys that name one server once trimmed. Under [`ParseOptions::lenient`] a +/// refused entry is dropped and reported in [`ParseReport::rejected`] rather +/// than refusing the document. +/// +/// # Errors +/// +/// Returns [`Error::ConfigDoc`] when the document's root is unreadable, and, +/// unless lenient, for the first refused entry. +pub fn parse_with(doc: &Value, options: &ParseOptions<'_>) -> Result { + let config_doc = |detail| Error::ConfigDoc { detail }; + let mut report = ParseReport::default(); + for (name, entry) in server_entries(doc).map_err(config_doc)? { + let read = if report.declared.iter().any(|seen| seen.name == name) { + Err(format!("`{name}` is declared twice")) + } else { + read_entry(name, entry, Some(options)) + }; + match read { + Ok(declared) => report.declared.push(declared), + Err(detail) if options.lenient => { + tracing::debug!(name, "dropping a refused mcp.json entry: {detail}"); + report.rejected.push(RejectedEntry { + name: name.to_string(), + detail, + }); + } + Err(detail) => return Err(config_doc(detail)), + } + } + Ok(report) +} + +/// The document's entries by trimmed key, refusing an unreadable root. +fn server_entries(doc: &Value) -> std::result::Result, String> { let Some(root) = doc.as_object() else { return Err(format!("mcp.json holds an object with an `{ROOT_KEY}` key")); }; @@ -95,35 +155,46 @@ fn parse_servers(doc: &Value) -> std::result::Result, String> { let Some(servers) = servers.as_object() else { return Err(format!("`{ROOT_KEY}` maps a server name to its settings")); }; + Ok(servers + .iter() + .map(|(name, entry)| (name.trim(), entry)) + .collect()) +} - let mut declared = Vec::with_capacity(servers.len()); - for (name, entry) in servers { - let name = name.trim(); - if name.is_empty() { - return Err("a server needs a name — one entry's key is empty".to_string()); - } - let Some(entry) = entry.as_object() else { - return Err(format!( - "`{name}` holds an object, e.g. {{ \"command\": \"npx\", \"args\": [\"-y\", \"…\"] }} or {{ \"url\": \"https://…\" }}" - )); - }; - declared.push(parse_entry(name, entry)?); +/// Reads one keyed entry, refusing an empty key or a non-object value. +fn read_entry( + name: &str, + entry: &Value, + extension: Option<&ParseOptions<'_>>, +) -> std::result::Result { + if name.is_empty() { + return Err("a server needs a name — one entry's key is empty".to_string()); } - Ok(declared) + let Some(entry) = entry.as_object() else { + return Err(format!( + "`{name}` holds an object, e.g. {{ \"command\": \"npx\", \"args\": [\"-y\", \"…\"] }} or {{ \"url\": \"https://…\" }}" + )); + }; + parse_entry(name, entry, extension) } -/// Reads one entry. -fn parse_entry(name: &str, entry: &Map) -> std::result::Result { +/// Reads one entry. `extension` is `None` for [`parse`], which refuses the +/// extension and host fields. +fn parse_entry( + name: &str, + entry: &Map, + extension: Option<&ParseOptions<'_>>, +) -> std::result::Result { + let host_fields = extension.map_or(&[][..], |options| options.host_fields); for key in entry.keys() { - let known = matches!( - key.as_str(), - "url" | "command" | "args" | "env" | "headers" | "description" | "enabled" - ) || ECHOED_FIELDS.contains(&key.as_str()) - || TOLERATED_FIELDS.contains(&key.as_str()); + let key = key.as_str(); + let known = STANDARD_FIELDS.contains(&key) + || ECHOED_FIELDS.contains(&key) + || TOLERATED_FIELDS.contains(&key) + || (extension.is_some() && EXTENSION_FIELDS.contains(&key)) + || host_fields.contains(&key); if !known { - return Err(format!( - "`{name}` has a `{key}` field this host doesn't understand; it accepts url, headers, command, args, env, description and enabled" - )); + return Err(unknown_field(name, key, extension.is_some(), host_fields)); } } @@ -180,9 +251,141 @@ fn parse_entry(name: &str, entry: &Map) -> std::result::Result String { + let mut accepted: Vec<&str> = vec![ + "url", + "headers", + "command", + "args", + "env", + "description", + "enabled", + ]; + if extended { + accepted.extend(EXTENSION_FIELDS); + accepted.extend( + host_fields.iter().filter(|field| { + !STANDARD_FIELDS.contains(field) && !EXTENSION_FIELDS.contains(field) + }), + ); + } + let (last, rest) = accepted.split_last().unwrap_or((&"", &[])); + format!( + "`{name}` has a `{key}` field this host doesn't understand; it accepts {} and {last}", + rest.join(", ") + ) +} + +/// A list of tool names, trimmed, refusing a blank or non-string one. Absent +/// reads as empty. +fn tool_list( + name: &str, + entry: &Map, + key: &str, +) -> std::result::Result, String> { + match entry.get(key) { + None | Some(Value::Null) => Ok(Vec::new()), + Some(Value::Array(items)) => items + .iter() + .map(|item| { + item.as_str() + .map(str::trim) + .filter(|tool| !tool.is_empty()) + .map(str::to_string) + .ok_or_else(|| format!("`{name}`.{key} holds tool names only")) + }) + .collect(), + Some(_) => Err(format!("`{name}`.{key} is a list of tool names")), + } +} + +/// `timeoutSecs`, refusing anything but a whole number of seconds above zero. +fn timeout_secs( + name: &str, + entry: &Map, +) -> std::result::Result, String> { + match entry.get("timeoutSecs") { + None | Some(Value::Null) => Ok(None), + Some(value) => value + .as_u64() + .filter(|secs| *secs > 0) + .map(Some) + .ok_or_else(|| format!("`{name}`.timeoutSecs is a whole number of seconds above zero")), + } +} + +/// The host's registered fields an entry carries, verbatim, skipping nulls +/// and any name that is a standard field. +fn host_field_values(entry: &Map, host_fields: &[&str]) -> BTreeMap { + host_fields + .iter() + .filter(|field| { + !STANDARD_FIELDS.contains(field) + && !EXTENSION_FIELDS.contains(field) + && !ECHOED_FIELDS.contains(field) + && !TOLERATED_FIELDS.contains(field) + }) + .filter_map(|field| { + let value = entry.get(*field).filter(|value| !value.is_null())?; + Some(((*field).to_string(), value.clone())) + }) + .collect() +} + +/// Writes declarations back as the document [`parse_with`] reads. +/// +/// For a host that keeps declarations itself; [`render`] is the install +/// store's projection. Servers are sorted by name, the extension and host +/// fields are emitted when set, and credentials never are: a declaration's +/// `env` or `headers` is write-only, as everywhere else in this module. +#[must_use] +pub fn render_declared(declared: &[Declared]) -> Value { + let mut entries: Vec<&Declared> = declared.iter().collect(); + entries.sort_by(|a, b| a.name.cmp(&b.name)); + + let mut out = Map::new(); + for server in entries { + let mut entry = Map::new(); + for (key, value) in &server.host_fields { + entry.insert(key.clone(), value.clone()); + } + if let Transport::HttpRemote { url } = &server.transport { + entry.insert("url".into(), json!(url)); + } else { + entry.insert("command".into(), json!(server.command)); + if !server.args.is_empty() { + entry.insert("args".into(), json!(server.args)); + } + } + if let Some(description) = server.description.as_deref().filter(|d| !d.is_empty()) { + entry.insert("description".into(), json!(description)); + } + if !server.enabled { + entry.insert("enabled".into(), json!(false)); + } + if !server.allowed_tools.is_empty() { + entry.insert("allowedTools".into(), json!(server.allowed_tools)); + } + if !server.disallowed_tools.is_empty() { + entry.insert("disallowedTools".into(), json!(server.disallowed_tools)); + } + if let Some(secs) = server.timeout_secs { + entry.insert("timeoutSecs".into(), json!(secs)); + } + out.insert(server.name.clone(), Value::Object(entry)); + } + + json!({ ROOT_KEY: out }) +} + /// Reads an entry's credential block: `env` for a stdio server, `headers` for /// an HTTP one, refusing the other spelling. /// diff --git a/crates/tinymcp/src/registry/config_doc/mod.rs b/crates/tinymcp/src/registry/config_doc/mod.rs index 6973c65a..f1126103 100644 --- a/crates/tinymcp/src/registry/config_doc/mod.rs +++ b/crates/tinymcp/src/registry/config_doc/mod.rs @@ -14,6 +14,17 @@ //! applies the difference. Neither is an import format that can drift from //! what is configured. //! +//! # A host that keeps its own store +//! +//! A host that holds declarations itself rather than in the install store +//! reads the same document through [`parse_with`], which also accepts +//! `allowedTools`, `disallowedTools` and `timeoutSecs`, carries fields the +//! host registers in [`ParseOptions::host_fields`] through verbatim, and can +//! drop a bad entry instead of refusing the document. [`render_declared`] +//! writes those declarations back. [`parse`] keeps refusing the extension +//! fields, because the install store has nowhere to put them and accepting +//! them there would drop them silently. +//! //! # Credentials are write-only //! //! A stdio server's `env` and an HTTP server's `headers` are secrets. They are @@ -35,9 +46,18 @@ mod apply; mod document; mod types; -pub use document::{ROOT_KEY, merge_credentials, parse, render, same_dial, to_installed}; -pub use types::{AppliedServer, ConfigApplyReport, Declared}; +pub use document::{ + ROOT_KEY, merge_credentials, parse, parse_with, render, render_declared, same_dial, + to_installed, +}; +pub use types::{ + AppliedServer, ConfigApplyReport, Declared, ParseOptions, ParseReport, RejectedEntry, +}; #[cfg(test)] #[path = "mod_tests.rs"] mod test; + +#[cfg(test)] +#[path = "mod_parse_with_tests.rs"] +mod parse_with_test; diff --git a/crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs b/crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs new file mode 100644 index 00000000..e884240c --- /dev/null +++ b/crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs @@ -0,0 +1,304 @@ +//! [`parse_with`] and [`render_declared`]: the reading of `mcp.json` for a +//! host that keeps its own store — extension fields, host fields, lenient +//! reads — and writing those declarations back. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::collections::BTreeMap; + +use serde_json::{Value, json}; +use tinymcp_bus::Transport; + +use super::*; +use crate::Error; + +const HOST_FIELDS: &[&str] = &["readOnlyTools", "authSecret"]; + +fn host() -> ParseOptions<'static> { + ParseOptions { + host_fields: HOST_FIELDS, + lenient: false, + } +} + +fn lenient() -> ParseOptions<'static> { + ParseOptions { + lenient: true, + ..host() + } +} + +fn refusal(doc: &Value, options: &ParseOptions<'_>) -> String { + match parse_with(doc, options).unwrap_err() { + Error::ConfigDoc { detail } => detail, + other => panic!("expected a config-doc refusal, got {other:?}"), + } +} + +#[test] +fn the_extension_fields_are_read() { + let report = parse_with( + &json!({ "mcpServers": { "notion": { + "url": "https://mcp.notion.test/mcp", + "allowedTools": [" search ", "fetch"], + "disallowedTools": ["delete"], + "timeoutSecs": 45, + } } }), + &ParseOptions::default(), + ) + .unwrap(); + assert!(report.rejected.is_empty()); + let notion = &report.declared[0]; + assert_eq!(notion.allowed_tools, ["search", "fetch"]); + assert_eq!(notion.disallowed_tools, ["delete"]); + assert_eq!(notion.timeout_secs, Some(45)); + assert!(notion.host_fields.is_empty()); +} + +#[test] +fn the_strict_parse_still_refuses_the_extension_fields() { + let error = parse(&json!({ "mcpServers": { "a": { "command": "npx", "timeoutSecs": 5 } } })) + .unwrap_err() + .to_string(); + assert_eq!( + error, + "`a` has a `timeoutSecs` field this host doesn't understand; it accepts url, headers, command, args, env, description and enabled" + ); + let declared = parse(&json!({ "mcpServers": { "a": { "command": "npx" } } })).unwrap(); + assert!(declared[0].allowed_tools.is_empty()); + assert_eq!(declared[0].timeout_secs, None); + assert!(declared[0].host_fields.is_empty()); +} + +#[test] +fn registered_host_fields_are_carried_verbatim() { + let report = parse_with( + &json!({ "mcpServers": { "a": { + "url": "https://a.test/mcp", + "readOnlyTools": ["list"], + "authSecret": null, + } } }), + &host(), + ) + .unwrap(); + assert_eq!( + report.declared[0].host_fields, + BTreeMap::from([("readOnlyTools".to_string(), json!(["list"]))]) + ); +} + +#[test] +fn an_unregistered_field_is_refused_naming_what_is_accepted() { + let doc = json!({ "mcpServers": { "a": { "url": "https://a.test/mcp", "cwd": "/tmp" } } }); + assert_eq!( + refusal(&doc, &host()), + "`a` has a `cwd` field this host doesn't understand; it accepts url, headers, command, args, env, description, enabled, allowedTools, disallowedTools, timeoutSecs, readOnlyTools and authSecret" + ); +} + +#[test] +fn a_host_field_named_like_a_standard_one_reads_as_the_standard_one() { + let report = parse_with( + &json!({ "mcpServers": { "a": { "url": "https://a.test/mcp", "enabled": false } } }), + &ParseOptions { + host_fields: &["enabled", "url"], + lenient: false, + }, + ) + .unwrap(); + assert!(!report.declared[0].enabled); + assert!(report.declared[0].host_fields.is_empty()); +} + +#[test] +fn extension_field_refusals_name_the_entry_and_the_field() { + let cases = [ + ( + json!({ "allowedTools": "search" }), + "`a`.allowedTools is a list of tool names", + ), + ( + json!({ "allowedTools": [1] }), + "`a`.allowedTools holds tool names only", + ), + ( + json!({ "disallowedTools": [" "] }), + "`a`.disallowedTools holds tool names only", + ), + ( + json!({ "timeoutSecs": 0 }), + "`a`.timeoutSecs is a whole number of seconds above zero", + ), + ( + json!({ "timeoutSecs": "30" }), + "`a`.timeoutSecs is a whole number of seconds above zero", + ), + ( + json!({ "timeoutSecs": -1 }), + "`a`.timeoutSecs is a whole number of seconds above zero", + ), + ]; + for (extra, expected) in cases { + let mut entry = json!({ "url": "https://a.test/mcp" }); + entry + .as_object_mut() + .unwrap() + .extend(extra.as_object().unwrap().clone()); + let doc = json!({ "mcpServers": { "a": entry } }); + assert_eq!(refusal(&doc, &host()), expected); + } +} + +#[test] +fn null_extension_fields_read_as_absent() { + let report = parse_with( + &json!({ "mcpServers": { "a": { + "url": "https://a.test/mcp", + "allowedTools": null, + "timeoutSecs": null, + } } }), + &host(), + ) + .unwrap(); + assert!(report.declared[0].allowed_tools.is_empty()); + assert_eq!(report.declared[0].timeout_secs, None); +} + +#[test] +fn a_strict_read_refuses_the_document_on_the_first_bad_entry() { + let doc = json!({ "mcpServers": { + "bad": { "url": "https://b.test/mcp", "command": "npx" }, + "good": { "url": "https://g.test/mcp" }, + } }); + assert_eq!( + refusal(&doc, &host()), + "`bad` names both a `url` and a `command`; a server is dialled one way" + ); +} + +#[test] +fn a_lenient_read_drops_bad_entries_and_keeps_the_rest() { + let doc = json!({ "mcpServers": { + "bad": { "url": "https://b.test/mcp", "command": "npx" }, + "good": { "url": "https://g.test/mcp" }, + "list": [1], + " ": { "url": "https://e.test/mcp" }, + "odd": { "url": "https://o.test/mcp", "cwd": "/" }, + } }); + let report = parse_with(&doc, &lenient()).unwrap(); + assert_eq!( + report + .declared + .iter() + .map(|d| d.name.as_str()) + .collect::>(), + ["good"] + ); + let rejected: Vec<&str> = report.rejected.iter().map(|r| r.name.as_str()).collect(); + assert_eq!(rejected, ["", "bad", "list", "odd"]); + assert_eq!( + report.rejected[0].detail, + "a server needs a name — one entry's key is empty" + ); + assert!( + report.rejected[2] + .detail + .starts_with("`list` holds an object") + ); +} + +#[test] +fn two_keys_naming_one_server_are_refused_or_the_second_dropped() { + let doc = json!({ "mcpServers": { + "a": { "url": "https://one.test/mcp" }, + "a ": { "url": "https://two.test/mcp" }, + } }); + assert_eq!(refusal(&doc, &host()), "`a` is declared twice"); + + let report = parse_with(&doc, &lenient()).unwrap(); + assert_eq!(report.declared.len(), 1); + assert_eq!( + report.declared[0].transport, + Transport::HttpRemote { + url: "https://one.test/mcp".into() + } + ); + assert_eq!(report.rejected[0].detail, "`a` is declared twice"); +} + +#[test] +fn an_unreadable_root_is_refused_even_when_lenient() { + for doc in [json!([]), json!({}), json!({ "mcpServers": [] })] { + assert!(matches!( + parse_with(&doc, &lenient()), + Err(Error::ConfigDoc { .. }) + )); + } +} + +#[test] +fn a_rendered_declaration_reads_back_as_itself_without_credentials() { + let doc = json!({ "mcpServers": { + "notion": { + "url": "https://mcp.notion.test/mcp", + "headers": { "Authorization": "Bearer secret" }, + "description": "Notes", + "enabled": false, + "allowedTools": ["search"], + "disallowedTools": ["delete"], + "timeoutSecs": 30, + "readOnlyTools": ["search"], + }, + "fs": { "command": "npx", "args": ["-y", "fs"], "env": { "ROOT": "/" } }, + } }); + let declared = parse_with(&doc, &host()).unwrap().declared; + let rendered = render_declared(&declared); + + assert_eq!( + rendered, + json!({ "mcpServers": { + "fs": { "command": "npx", "args": ["-y", "fs"] }, + "notion": { + "url": "https://mcp.notion.test/mcp", + "description": "Notes", + "enabled": false, + "allowedTools": ["search"], + "disallowedTools": ["delete"], + "timeoutSecs": 30, + "readOnlyTools": ["search"], + }, + } }) + ); + assert!(!rendered.to_string().contains("secret")); + + let reread = parse_with(&rendered, &host()).unwrap().declared; + let without_credentials: Vec = declared + .into_iter() + .map(|mut d| { + d.credentials = None; + d + }) + .collect(); + let mut expected = without_credentials; + expected.sort_by(|a, b| a.name.cmp(&b.name)); + assert_eq!(reread, expected); +} + +#[test] +fn a_command_without_args_renders_without_an_args_list() { + let declared = parse_with( + &json!({ "mcpServers": { "a": { "command": "uvx", "description": "" } } }), + &ParseOptions::default(), + ); + assert!(declared.is_err(), "a blank description is refused"); + let declared = parse_with( + &json!({ "mcpServers": { "a": { "command": "uvx" } } }), + &ParseOptions::default(), + ) + .unwrap() + .declared; + assert_eq!( + render_declared(&declared), + json!({ "mcpServers": { "a": { "command": "uvx" } } }) + ); +} diff --git a/crates/tinymcp/src/registry/config_doc/types.rs b/crates/tinymcp/src/registry/config_doc/types.rs index d3d659c3..96e5e7a1 100644 --- a/crates/tinymcp/src/registry/config_doc/types.rs +++ b/crates/tinymcp/src/registry/config_doc/types.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; +use serde_json::Value; use tinymcp_bus::Transport; /// One server as the document declares it, after validation. @@ -23,6 +24,56 @@ pub struct Declared { pub description: Option, /// Whether the server is brought up and exposed. pub enabled: bool, + /// `allowedTools`: the only tools the server may expose. Empty means every + /// tool. Read by [`parse_with`](super::parse_with) only. + pub allowed_tools: Vec, + /// `disallowedTools`: tools that are always blocked. Read by + /// [`parse_with`](super::parse_with) only. + pub disallowed_tools: Vec, + /// `timeoutSecs`: how long a call may take, when the entry says. Read by + /// [`parse_with`](super::parse_with) only. + pub timeout_secs: Option, + /// The host's own fields, by document key, verbatim. + /// + /// Only the names a host registers in [`ParseOptions::host_fields`] land + /// here; a `null` value reads as absent. + pub host_fields: BTreeMap, +} + +/// How [`parse_with`](super::parse_with) reads a document. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct ParseOptions<'a> { + /// Entry fields the host understands beyond the standard ones, by + /// document key (`"readOnlyTools"`, say). Their values are carried in + /// [`Declared::host_fields`] untouched; validating them is the host's. A + /// name that is already a standard field is read as that field. + pub host_fields: &'a [&'a str], + /// Whether a refused entry is dropped and reported instead of refusing + /// the whole document. + /// + /// For documents a user did not write against this host, such as a + /// bundle copied from a vendor's instructions. A document whose root is + /// unreadable is still refused. + pub lenient: bool, +} + +/// What [`parse_with`](super::parse_with) read. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ParseReport { + /// The entries that were read, in document-key order. + pub declared: Vec, + /// The entries that were dropped, in document-key order. Always empty + /// unless [`ParseOptions::lenient`] is set. + pub rejected: Vec, +} + +/// One entry a lenient read dropped. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RejectedEntry { + /// The entry's key, trimmed. + pub name: String, + /// Why it was dropped, naming the entry and the field. + pub detail: String, } /// One server a document application touched. From bae14c4a06ef97c115acd625db16c3dbab169ab4 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 14:45:22 +0530 Subject: [PATCH 07/21] docs: describe the call outcome and the mcp.json host extension (#40) README covers the mcp_call outcome a host reads from bridge results, fence stripping, SecretScrubber::with_secrets, parse_with and render_declared, and the guarded OAuthFlow::refresh. ROADMAP lists them as shipped. --- README.md | 27 ++++++++++++++++++++++++++- ROADMAP.md | 4 ++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3408047c..817dbb28 100644 --- a/README.md +++ b/README.md @@ -179,16 +179,41 @@ Enable the `tools` feature to expose each server tool as a - **The generic bridge** is three tools over one configured-server registry: `McpListServersTool`, `McpListToolsTool` and `McpCallTool` (`mcp_list_servers`, `mcp_list_tools`, `mcp_call_tool`). `McpCallTool` asks a - host-supplied `ActGate` before it sends anything. + host-supplied `ActGate` before it sends anything. Server and tool names lose + the markdown a model wraps them in (`` `docs` ``, `*docs*`, `docs.`). +- **The call outcome.** Every `mcp_call_tool` result that names a server and a + tool carries a `tinymcp_bus::McpCallOutcome` in `ToolResult::metadata`, with + `kind` set to `MCP_CALL_RESULT_KIND` (`"mcp_call"`): the server and tool, + `ok` when the server answered, and otherwise the error's wire name and + whether it was a 401 that advertised OAuth. It is host-only — never rendered + to the model — so a host meters answered calls and surfaces failures from it + instead of parsing text. Read it back with `McpCallOutcome::from_metadata`. - **`SecretScrubber`** removes a server's own credentials (tokens, basic-auth pairs, header and query values, URL userinfo) from whatever it echoes back; every bridge tool applies it, and a host wraps its own invokers with it. + `with_secrets` adds values the host keeps outside the server's config. - **`tool_result`** maps a rendered MCP result onto `tinytools::ToolResult`, bounding oversized blocks. `tinytools` is a git dependency so a host that links another checkout of it can `[patch]` the two into one package. +## `mcp.json` and OAuth for hosts with their own store + +`registry::config_doc` reads and writes the `{ "mcpServers": { … } }` document. +`parse` and `render` are the install store's reading and projection. +A host that keeps declarations itself uses `parse_with` and `render_declared` +instead: `parse_with` also reads `allowedTools`, `disallowedTools` and +`timeoutSecs`, carries the fields the host registers in +`ParseOptions::host_fields` through verbatim in `Declared::host_fields`, and +with `lenient` set drops a bad entry into `ParseReport::rejected` rather than +refusing the document. Credentials are write-only in both. + +`registry::OAuthFlow::refresh` refreshes an expired token from a host's +`OAuthCredentialStore`, re-checking the token endpoint when +`require_public_endpoints()` is set; the free `refresh_if_expired` does not +check. `registry::OAuthBundle` is the stored refresh bundle's shape. + ## Static linking Enable the `static-link` feature when compiling this module into a Rust host. It diff --git a/ROADMAP.md b/ROADMAP.md index 11340249..d859d60f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -14,6 +14,10 @@ out of scope. A roadmap that lists everything is a roadmap nobody trusts. supply-chain checks - a manual release workflow that versions, tags, publishes to crates.io, and creates a GitHub release with crate and TinyBus runtime/module assets +- a structured `mcp_call` outcome on every bridge call, for host metering and + failure surfacing (contract 1.3) +- `mcp.json` reading for hosts with their own store (`parse_with`), and a + guarded OAuth refresh on the flow ## Next From 8fb9af98bd5627ebb51884374e6fe5facc02c4aa Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:11:09 +0530 Subject: [PATCH 08/21] test(config_doc): compare empty collections with assert_eq for clippy::assert_is_empty (#40) --- .../tinymcp/src/registry/config_doc/mod_parse_with_tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs b/crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs index e884240c..ffdfe68a 100644 --- a/crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs +++ b/crates/tinymcp/src/registry/config_doc/mod_parse_with_tests.rs @@ -47,7 +47,7 @@ fn the_extension_fields_are_read() { &ParseOptions::default(), ) .unwrap(); - assert!(report.rejected.is_empty()); + assert_eq!(report.rejected, Vec::new()); let notion = &report.declared[0]; assert_eq!(notion.allowed_tools, ["search", "fetch"]); assert_eq!(notion.disallowed_tools, ["delete"]); @@ -65,7 +65,7 @@ fn the_strict_parse_still_refuses_the_extension_fields() { "`a` has a `timeoutSecs` field this host doesn't understand; it accepts url, headers, command, args, env, description and enabled" ); let declared = parse(&json!({ "mcpServers": { "a": { "command": "npx" } } })).unwrap(); - assert!(declared[0].allowed_tools.is_empty()); + assert_eq!(declared[0].allowed_tools, Vec::::new()); assert_eq!(declared[0].timeout_secs, None); assert!(declared[0].host_fields.is_empty()); } @@ -160,7 +160,7 @@ fn null_extension_fields_read_as_absent() { &host(), ) .unwrap(); - assert!(report.declared[0].allowed_tools.is_empty()); + assert_eq!(report.declared[0].allowed_tools, Vec::::new()); assert_eq!(report.declared[0].timeout_secs, None); } From e62c5fa36917c22c22714535244e31b2d1d35987 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:44:02 +0530 Subject: [PATCH 09/21] fix(oauth): redact secrets from OAuthBundle Debug (#40) --- .../src/registry/oauth/mod_refresh_tests.rs | 25 +++++++++++++++++++ crates/tinymcp/src/registry/oauth/types.rs | 15 ++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs index fa5b3c1d..42f9bbd8 100644 --- a/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs +++ b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs @@ -27,6 +27,31 @@ fn the_bundle_wire_form_is_pinned_and_round_trips() { assert_eq!(serde_json::from_value::(wire).unwrap(), bundle); } +#[test] +fn the_bundle_debug_output_hides_its_secrets() { + let bundle = OAuthBundle { + refresh_token: Some("refresh-secret".into()), + client_id: "cli-1".into(), + client_secret: Some("client-secret".into()), + token_endpoint: "https://auth.example/token".into(), + expires_at: 42, + }; + let shown = format!("{bundle:?} {bundle:#?}"); + assert!(!shown.contains("refresh-secret"), "{shown}"); + assert!(!shown.contains("client-secret"), "{shown}"); + assert!( + shown.contains("cli-1") && shown.contains("[redacted]"), + "{shown}" + ); + + let bare = OAuthBundle { + refresh_token: None, + client_secret: None, + ..bundle + }; + assert!(format!("{bare:?}").contains("refresh_token: None")); +} + #[test] fn a_stored_bundle_reads_back_as_the_public_type() { let store = store_with_remote("https://example.test/mcp"); diff --git a/crates/tinymcp/src/registry/oauth/types.rs b/crates/tinymcp/src/registry/oauth/types.rs index 97eb7ea8..9176a49d 100644 --- a/crates/tinymcp/src/registry/oauth/types.rs +++ b/crates/tinymcp/src/registry/oauth/types.rs @@ -40,7 +40,7 @@ pub(super) struct PendingAuthorization { /// its own store can read the bundle it holds — for an expiry it shows, or to /// migrate one — without restating its shape. It holds a client secret and a /// refresh token: never log or display it. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct OAuthBundle { /// The refresh token, when the server issued one. pub refresh_token: Option, @@ -54,6 +54,19 @@ pub struct OAuthBundle { pub expires_at: u64, } +impl std::fmt::Debug for OAuthBundle { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let redacted = |secret: &Option| secret.as_ref().map(|_| "[redacted]"); + f.debug_struct("OAuthBundle") + .field("refresh_token", &redacted(&self.refresh_token)) + .field("client_id", &self.client_id) + .field("client_secret", &redacted(&self.client_secret)) + .field("token_endpoint", &self.token_endpoint) + .field("expires_at", &self.expires_at) + .finish() + } +} + /// A parsed token-endpoint reply. #[derive(Debug, Clone)] pub(super) struct TokenResponse { From 9d5399ff74487ea171a6c3fa480c6b8a86c141d5 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:44:22 +0530 Subject: [PATCH 10/21] fix(agent_tools): validate McpCallOutcome kind and ok/error pairing on decode (#40) --- .../tinymcp-bus/src/agent_tools/mod_tests.rs | 39 ++++++++++++++++ crates/tinymcp-bus/src/agent_tools/types.rs | 45 +++++++++++++++++-- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/crates/tinymcp-bus/src/agent_tools/mod_tests.rs b/crates/tinymcp-bus/src/agent_tools/mod_tests.rs index 9a1ad9db..3ea94c56 100644 --- a/crates/tinymcp-bus/src/agent_tools/mod_tests.rs +++ b/crates/tinymcp-bus/src/agent_tools/mod_tests.rs @@ -392,3 +392,42 @@ fn an_outcome_is_read_back_only_from_metadata_of_its_kind() { None ); } + +#[test] +fn decoding_an_outcome_rejects_what_the_constructors_never_produce() { + let decode = |value: serde_json::Value| serde_json::from_value::(value); + let error = json!({ "code": crate::errors::TRANSPORT, "unauthorized": false, "advertises_oauth": false }); + let base = + |ok: bool| json!({ "kind": "mcp_call", "server": "docs", "tool": "search", "ok": ok }); + + let mut answered_with_error = base(true); + answered_with_error["error"] = error.clone(); + assert!(decode(answered_with_error).is_err()); + + assert!(decode(base(false)).is_err()); + let mut failed_null_error = base(false); + failed_null_error["error"] = serde_json::Value::Null; + assert!(decode(failed_null_error).is_err()); + + let mut missing_kind = base(true); + missing_kind.as_object_mut().unwrap().remove("kind"); + assert!(decode(missing_kind).is_err()); + let mut wrong_kind = base(true); + wrong_kind["kind"] = json!("web_search"); + assert!(decode(wrong_kind).is_err()); + + assert_eq!( + decode(base(true)).unwrap(), + super::McpCallOutcome::answered("docs", "search") + ); + let mut failed = base(false); + failed["error"] = error; + assert_eq!( + decode(failed).unwrap(), + super::McpCallOutcome::failed( + "docs", + "search", + super::McpCallError::new(crate::errors::TRANSPORT) + ) + ); +} diff --git a/crates/tinymcp-bus/src/agent_tools/types.rs b/crates/tinymcp-bus/src/agent_tools/types.rs index e01d5053..1c74318a 100644 --- a/crates/tinymcp-bus/src/agent_tools/types.rs +++ b/crates/tinymcp-bus/src/agent_tools/types.rs @@ -94,7 +94,12 @@ pub const MCP_CALL_RESULT_KIND: &str = "mcp_call"; /// `ok` says whether the server answered the call. A tool that answered with /// its own error result still has `ok: true`: the server was reached and the /// failure is the tool's, reported in the result itself. +/// +/// Decoding rejects a payload whose `kind` is not [`MCP_CALL_RESULT_KIND`] or +/// whose `ok` and `error` disagree, so a decoded outcome is one the +/// constructors could have produced. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(try_from = "McpCallOutcomeWire")] pub struct McpCallOutcome { /// Always [`MCP_CALL_RESULT_KIND`]. pub kind: String, @@ -140,13 +145,47 @@ impl McpCallOutcome { /// [`MCP_CALL_RESULT_KIND`], or does not decode as an outcome. #[must_use] pub fn from_metadata(metadata: &Value) -> Option { - if metadata.get("kind").and_then(Value::as_str) != Some(MCP_CALL_RESULT_KIND) { - return None; - } serde_json::from_value(metadata.clone()).ok() } } +#[derive(Deserialize)] +struct McpCallOutcomeWire { + kind: String, + server: String, + tool: String, + ok: bool, + #[serde(default)] + error: Option, +} + +impl TryFrom for McpCallOutcome { + type Error = String; + + fn try_from(wire: McpCallOutcomeWire) -> Result { + if wire.kind != MCP_CALL_RESULT_KIND { + return Err(format!( + "expected kind `{MCP_CALL_RESULT_KIND}`, got `{}`", + wire.kind + )); + } + if wire.ok != wire.error.is_none() { + return Err(if wire.ok { + "an answered call cannot carry an error".to_string() + } else { + "a failed call must carry an error".to_string() + }); + } + Ok(Self { + kind: wire.kind, + server: wire.server, + tool: wire.tool, + ok: wire.ok, + error: wire.error, + }) + } +} + /// Why an MCP call failed, classified for a host. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct McpCallError { From c1e3b149e0edf49ebb3fee982028500fc1a132e1 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:45:23 +0530 Subject: [PATCH 11/21] fix(tools): strip only fence characters from bridge identifiers (#40) --- crates/tinymcp/src/tools/bridge.rs | 17 +++++++++++------ crates/tinymcp/src/tools/bridge_tests.rs | 10 +++++++++- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/crates/tinymcp/src/tools/bridge.rs b/crates/tinymcp/src/tools/bridge.rs index 300cf6c3..f95cd7d7 100644 --- a/crates/tinymcp/src/tools/bridge.rs +++ b/crates/tinymcp/src/tools/bridge.rs @@ -405,10 +405,11 @@ pub(super) fn endpoint_without_query(endpoint: &str) -> String { } /// A required identifier argument, trimmed and without the markdown a model -/// wraps it in when it answers in prose (`` `docs` ``, `*docs*`, `docs.`). +/// wraps it in when it answers in prose (`` `docs` ``, `*docs*`, `` `docs`. ``). /// -/// Only leading and trailing fence characters go: a server or tool name never -/// starts or ends with one, so stripping cannot change a real name. +/// Backticks and asterisks go from either end, and sentence punctuation goes +/// only when it follows one of them. Every other character, `_` and `.` +/// included, reaches the server as typed. pub(super) fn required_string_arg(args: &Value, key: &str) -> anyhow::Result { let missing = || anyhow::anyhow!("missing required `{key}`"); let value = args @@ -417,9 +418,13 @@ pub(super) fn required_string_arg(args: &Value, key: &str) -> anyhow::Result Date: Tue, 6 Oct 2026 17:45:23 +0530 Subject: [PATCH 12/21] fix(tools): report caller-named server and tool in call outcomes unscrubbed (#40) --- crates/tinymcp/src/tools/bridge.rs | 19 ++++++------------- .../tinymcp/src/tools/bridge_outcome_tests.rs | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/tinymcp/src/tools/bridge.rs b/crates/tinymcp/src/tools/bridge.rs index f95cd7d7..9ec12571 100644 --- a/crates/tinymcp/src/tools/bridge.rs +++ b/crates/tinymcp/src/tools/bridge.rs @@ -313,8 +313,8 @@ impl Tool for McpCallTool { Ok(arguments) => Value::Object(arguments), Err(error) => { let outcome = McpCallOutcome::failed( - scrubber.scrub(&server), - scrubber.scrub(&tool), + &server, + &tool, McpCallError::new(tinymcp_bus::errors::INVALID_ARGUMENTS), ); return Ok(with_outcome( @@ -325,19 +325,12 @@ impl Tool for McpCallTool { }; let (mut result, outcome) = match self.registry.call_tool(&server, &tool, arguments).await { - Ok(result) => ( - result.rendered, - McpCallOutcome::answered(scrubber.scrub(&server), scrubber.scrub(&tool)), - ), + Ok(result) => (result.rendered, McpCallOutcome::answered(&server, &tool)), Err(err) => { - let outcome = McpCallOutcome::failed( - scrubber.scrub(&server), - scrubber.scrub(&tool), - call_error(&err), - ); + let outcome = McpCallOutcome::failed(&server, &tool, call_error(&err)); tracing::debug!( - server = %outcome.server, - tool = %outcome.tool, + server = %scrubber.scrub(&server), + tool = %scrubber.scrub(&tool), code = err.wire_name(), "[mcp] mcp_call_tool failed" ); diff --git a/crates/tinymcp/src/tools/bridge_outcome_tests.rs b/crates/tinymcp/src/tools/bridge_outcome_tests.rs index 72d9e81e..ce4a2772 100644 --- a/crates/tinymcp/src/tools/bridge_outcome_tests.rs +++ b/crates/tinymcp/src/tools/bridge_outcome_tests.rs @@ -212,3 +212,20 @@ async fn arguments_that_are_not_an_object_report_invalid_arguments() { assert_eq!(outcome.tool, "whoami"); assert_eq!(outcome.error.unwrap().code, errors::INVALID_ARGUMENTS); } + +#[tokio::test] +async fn the_outcome_names_the_caller_s_server_and_tool_even_when_they_match_a_secret() { + let registry = registry( + &answering_server().await, + McpAuthConfig::BearerToken { + token: SECRET.into(), + }, + &[SECRET], + ); + let result = call_tool(registry).execute(args(SECRET)).await.unwrap(); + assert!(result.is_error); + assert!(!result.output().contains(SECRET)); + let outcome = outcome_of(&result); + assert_eq!(outcome.tool, SECRET); + assert_eq!(outcome.error.unwrap().code, errors::TOOL_NOT_ALLOWED); +} From 28b5d17616ccd6205ac457ff8f21fd02cea9aeee Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:45:39 +0530 Subject: [PATCH 13/21] fix(tools): redact host-injected secrets as strict credentials (#40) --- crates/tinymcp/src/tools/scrub.rs | 5 ++++- crates/tinymcp/src/tools/scrub_tests.rs | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/tinymcp/src/tools/scrub.rs b/crates/tinymcp/src/tools/scrub.rs index 447cb4a3..23906608 100644 --- a/crates/tinymcp/src/tools/scrub.rs +++ b/crates/tinymcp/src/tools/scrub.rs @@ -140,7 +140,8 @@ impl SecretScrubber { /// /// For credentials the server's own configuration does not carry: a token /// a host keeps in its own secret store, or a value it injected some other - /// way. Each is matched as typed and URL-encoded; blank values are skipped. + /// way. Each is matched as typed and URL-encoded, anywhere in the text even + /// when short; blank values are skipped. #[must_use] pub fn with_secrets(mut self, secrets: impl IntoIterator) -> Self { for secret in secrets { @@ -150,8 +151,10 @@ impl SecretScrubber { } let encoded = urlencoding::encode(secret).into_owned(); if encoded != secret { + self.strict.push(encoded.clone()); self.secrets.push(encoded); } + self.strict.push(secret.to_string()); self.secrets.push(secret.to_string()); } sort_longest_first(&mut self.secrets); diff --git a/crates/tinymcp/src/tools/scrub_tests.rs b/crates/tinymcp/src/tools/scrub_tests.rs index 681f23f4..63455518 100644 --- a/crates/tinymcp/src/tools/scrub_tests.rs +++ b/crates/tinymcp/src/tools/scrub_tests.rs @@ -155,3 +155,18 @@ fn extra_secrets_join_the_configured_ones_longest_first() { )); assert!(!format!("{:?}", result.content).contains("configured-token")); } + +#[test] +fn a_short_extra_secret_is_redacted_inside_larger_text() { + let scrubber = SecretScrubber::new(&McpAuthConfig::None, "https://example.com/mcp") + .with_secrets(["k9".to_string()]); + assert_eq!( + scrubber.scrub("token=xk9y and k9"), + "token=x[redacted]y and [redacted]" + ); + let mut value = json!({ "k9": "a", "tool_k9x": "b" }); + scrubber.scrub_value(&mut value); + let map = value.as_object().unwrap(); + assert!(map.contains_key("[redacted]"), "{value}"); + assert!(map.contains_key("tool_k9x"), "{value}"); +} From 9b90a77b3c38e1b61f921185c07c247653a18fb5 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:46:02 +0530 Subject: [PATCH 14/21] fix(lib): keep McpAuthChallenge in the crate-root re-exports (#40) --- crates/tinymcp/src/lib.rs | 17 +++++++++-------- crates/tinymcp/tests/public_reexports.rs | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 crates/tinymcp/tests/public_reexports.rs diff --git a/crates/tinymcp/src/lib.rs b/crates/tinymcp/src/lib.rs index a72386cf..16c3b252 100644 --- a/crates/tinymcp/src/lib.rs +++ b/crates/tinymcp/src/lib.rs @@ -116,12 +116,13 @@ pub use tinymcp_bus::{ ConnStatus, ConnectedServerOverview, DEFAULT_LIST_LIMIT, ERROR_MESSAGE_MAX_BYTES, ExtraFields, HEADER_PROTOCOL_VERSION, HEADER_SESSION_ID, HttpHeader, INTERFACE, InstalledServer, LATEST_PROTOCOL_VERSION, MAX_DESCRIPTION_BYTES, MAX_LIST_LIMIT, MAX_TITLE_BYTES, - MCP_CALL_RESULT_KIND, METHODS, McpAuthConfig, McpAuthHint, McpAuthorizationContext, - McpCallError, McpCallOutcome, McpClientConfig, McpClientIdentityConfig, McpClientInfo, - McpInitializeResult, McpProxyConfig, McpRegistryAuthConfig, McpRemoteTool, McpServerConfig, - McpServerToolResult, McpSseEvent, McpTool, McpToolContent, McpToolResult, McpWriteListQuery, - McpWriteRecord, NewMcpWriteRecord, OBJECT_PATH, ProtectedResourceMetadata, RegistryConnection, - RegistryListResponse, RegistryPagination, RegistryServerDetail, RegistryServerSummary, - SUPPORTED_PROTOCOL_VERSIONS, SearchCuration, ServerDetail, ServerStatus, Transport, config, - is_compatible, names, sanitize, version, + MCP_CALL_RESULT_KIND, METHODS, McpAuthChallenge, McpAuthConfig, McpAuthHint, + McpAuthorizationContext, McpCallError, McpCallOutcome, McpClientConfig, + McpClientIdentityConfig, McpClientInfo, McpInitializeResult, McpProxyConfig, + McpRegistryAuthConfig, McpRemoteTool, McpServerConfig, McpServerToolResult, McpSseEvent, + McpTool, McpToolContent, McpToolResult, McpWriteListQuery, McpWriteRecord, NewMcpWriteRecord, + OBJECT_PATH, ProtectedResourceMetadata, RegistryConnection, RegistryListResponse, + RegistryPagination, RegistryServerDetail, RegistryServerSummary, SUPPORTED_PROTOCOL_VERSIONS, + SearchCuration, ServerDetail, ServerStatus, Transport, config, is_compatible, names, sanitize, + version, }; diff --git a/crates/tinymcp/tests/public_reexports.rs b/crates/tinymcp/tests/public_reexports.rs new file mode 100644 index 00000000..7dcd3e2c --- /dev/null +++ b/crates/tinymcp/tests/public_reexports.rs @@ -0,0 +1,23 @@ +//! Pins the crate-root paths a host imports the contract types from. + +use tinymcp::{ + CONTRACT_VERSION, MCP_CALL_RESULT_KIND, McpAuthChallenge, McpCallError, McpCallOutcome, +}; + +#[test] +fn the_contract_types_resolve_at_the_crate_root_to_the_bus_definitions() { + let challenge: tinymcp_bus::McpAuthChallenge = McpAuthChallenge { + scheme: "Bearer".into(), + realm: None, + resource_metadata: None, + }; + assert_eq!(challenge.scheme, "Bearer"); + + let outcome: tinymcp_bus::McpCallOutcome = McpCallOutcome::failed( + "docs", + "search", + McpCallError::new(tinymcp_bus::errors::TRANSPORT), + ); + assert_eq!(outcome.kind, MCP_CALL_RESULT_KIND); + assert_eq!(CONTRACT_VERSION, tinymcp_bus::CONTRACT_VERSION); +} From 05a94ad116a7fcfa979962ec342534f79086411b Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:46:27 +0530 Subject: [PATCH 15/21] fix(oauth): drop a refresh superseded by a newer sign-in (#40) --- .../src/registry/oauth/mod_refresh_tests.rs | 65 +++++++++++++++++++ crates/tinymcp/src/registry/oauth/tokens.rs | 21 ++++++ 2 files changed, 86 insertions(+) diff --git a/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs index 42f9bbd8..3ddf5868 100644 --- a/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs +++ b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs @@ -154,3 +154,68 @@ async fn an_unreadable_bundle_is_reported_by_the_flow() { .contains("stored oauth bundle is unreadable") ); } + +/// A store whose bundle is replaced by a newer sign-in after the first read. +struct SupersededStore { + inner: Store, + reads: AtomicUsize, + writes: AtomicUsize, +} + +#[allow(clippy::unused_async_trait_impl)] +impl crate::registry::OAuthCredentialStore for SupersededStore { + async fn remote_url(&self, server_id: &str) -> crate::Result> { + self.inner.remote_url(server_id).await + } + + async fn load_credentials(&self, server_id: &str) -> crate::Result> { + let mut env = self.inner.load_credentials(server_id).await?; + if self.reads.fetch_add(1, Ordering::SeqCst) >= 1 { + let newer = json!({ + "refresh_token": "r-newer", + "client_id": "cli-2", + "client_secret": null, + "token_endpoint": "https://auth.example/token", + "expires_at": u64::MAX / 2, + }); + env.insert(OAUTH_BUNDLE_KEY.to_string(), newer.to_string()); + env.insert("Authorization".to_string(), "Bearer newer".to_string()); + } + Ok(env) + } + + async fn store_credentials( + &self, + server_id: &str, + credentials: &BTreeMap, + ) -> crate::Result<()> { + self.writes.fetch_add(1, Ordering::SeqCst); + self.inner.store_credentials(server_id, credentials).await + } +} + +#[tokio::test] +async fn a_refresh_does_not_overwrite_a_sign_in_stored_while_it_waited() { + let requests = TokenRequests::default(); + let base = serve(token_endpoint( + requests.clone(), + json!({ "access_token": "fresh", "expires_in": 3600 }), + )) + .await; + let inner = store_with_remote("https://example.test/mcp"); + store_expired_bundle(&inner, &format!("{base}/token"), Some("r1")); + let store = SupersededStore { + inner, + reads: AtomicUsize::new(0), + writes: AtomicUsize::new(0), + }; + + let flow = OAuthFlow::new(None).unwrap(); + assert!(!flow.refresh(&store, "srv-1").await.unwrap()); + assert_eq!(requests.count.load(Ordering::SeqCst), 1); + assert_eq!(store.writes.load(Ordering::SeqCst), 0); + assert_eq!( + store.inner.load_env_values("srv-1").unwrap()["Authorization"], + "Bearer stale" + ); +} diff --git a/crates/tinymcp/src/registry/oauth/tokens.rs b/crates/tinymcp/src/registry/oauth/tokens.rs index 05e256ae..8162dabb 100644 --- a/crates/tinymcp/src/registry/oauth/tokens.rs +++ b/crates/tinymcp/src/registry/oauth/tokens.rs @@ -124,6 +124,11 @@ where tokens.refresh_token.clone_from(&bundle.refresh_token); } + if superseded(store, server_id, bundle).await? { + tracing::info!(server_id, "dropped a refresh superseded by a newer sign-in"); + return Ok(false); + } + persist( store, server_id, @@ -138,6 +143,22 @@ where Ok(true) } +/// Whether the stored bundle is no longer the `bundle` a refresh started from, +/// because a sign-in stored a newer one while the token endpoint was answering. +/// +/// A store offers no compare-and-swap, so this narrows the window rather than +/// closing it. +async fn superseded(store: &S, server_id: &str, bundle: &OAuthBundle) -> Result +where + S: OAuthCredentialStore + ?Sized, +{ + let env = store.load_credentials(server_id).await?; + let current = env + .get(OAUTH_BUNDLE_KEY) + .and_then(|raw| serde_json::from_str::(raw).ok()); + Ok(current.as_ref() != Some(bundle)) +} + /// Stores an access token and its refresh bundle. /// /// Merged over the existing credentials rather than replacing them. Storing is From edcb97200be971e1975d972510f3eb2746e78f83 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 17:46:44 +0530 Subject: [PATCH 16/21] docs: qualify the call-outcome guarantee and document identifier handling (#40) --- README.md | 17 ++++++++++++----- ROADMAP.md | 4 ++-- crates/tinymcp/src/tools/bridge.rs | 10 ++++++---- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 817dbb28..59d10a42 100644 --- a/README.md +++ b/README.md @@ -180,18 +180,25 @@ Enable the `tools` feature to expose each server tool as a `McpListServersTool`, `McpListToolsTool` and `McpCallTool` (`mcp_list_servers`, `mcp_list_tools`, `mcp_call_tool`). `McpCallTool` asks a host-supplied `ActGate` before it sends anything. Server and tool names lose - the markdown a model wraps them in (`` `docs` ``, `*docs*`, `docs.`). -- **The call outcome.** Every `mcp_call_tool` result that names a server and a - tool carries a `tinymcp_bus::McpCallOutcome` in `ToolResult::metadata`, with + the backticks and asterisks a model wraps them in (`` `docs` ``, `**docs**`, + `` `docs`. ``); `_`, `.` and other characters are kept. +- **The call outcome.** Every `mcp_call_tool` result for a call the act gate + allowed and that names a server, a tool and `arguments` carries a + `tinymcp_bus::McpCallOutcome` in `ToolResult::metadata`, with `kind` set to `MCP_CALL_RESULT_KIND` (`"mcp_call"`): the server and tool, `ok` when the server answered, and otherwise the error's wire name and whether it was a 401 that advertised OAuth. It is host-only — never rendered to the model — so a host meters answered calls and surfaces failures from it - instead of parsing text. Read it back with `McpCallOutcome::from_metadata`. + instead of parsing text. A call missing one of those fields, or refused by + the gate, fails before a result exists and carries no outcome. The server and + tool are reported as the caller named them, unscrubbed. Read it back with + `McpCallOutcome::from_metadata`; decoding rejects a wrong `kind` or an `ok` + that disagrees with `error`. - **`SecretScrubber`** removes a server's own credentials (tokens, basic-auth pairs, header and query values, URL userinfo) from whatever it echoes back; every bridge tool applies it, and a host wraps its own invokers with it. - `with_secrets` adds values the host keeps outside the server's config. + `with_secrets` adds values the host keeps outside the server's config, each + redacted as a strict credential: anywhere in the text, even when short. - **`tool_result`** maps a rendered MCP result onto `tinytools::ToolResult`, bounding oversized blocks. diff --git a/ROADMAP.md b/ROADMAP.md index d859d60f..50dd0ebd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -14,8 +14,8 @@ out of scope. A roadmap that lists everything is a roadmap nobody trusts. supply-chain checks - a manual release workflow that versions, tags, publishes to crates.io, and creates a GitHub release with crate and TinyBus runtime/module assets -- a structured `mcp_call` outcome on every bridge call, for host metering and - failure surfacing (contract 1.3) +- a structured `mcp_call` outcome on every bridge call that reaches a result, + for host metering and failure surfacing (contract 1.3) - `mcp.json` reading for hosts with their own store (`parse_with`), and a guarded OAuth refresh on the flow diff --git a/crates/tinymcp/src/tools/bridge.rs b/crates/tinymcp/src/tools/bridge.rs index 9ec12571..a6aa7d2c 100644 --- a/crates/tinymcp/src/tools/bridge.rs +++ b/crates/tinymcp/src/tools/bridge.rs @@ -10,10 +10,12 @@ //! What stays with the host is the decision to allow a call at all: //! [`McpCallTool`] takes an [`ActGate`] it runs before anything is sent. //! -//! Every result [`McpCallTool`] returns once it knows the server and tool -//! carries a [`McpCallOutcome`] as its metadata: whether the server answered -//! and, when it did not, the error's wire name and whether it was a 401 that -//! advertised OAuth. The model never sees it; a host reads it to meter calls +//! Every result [`McpCallTool`] returns once the act gate has allowed the call +//! and it has a server, a tool and an `arguments` value carries a +//! [`McpCallOutcome`] as its metadata. A call missing one of those, or refused +//! by the gate, fails before any result exists and carries none. The outcome +//! says whether the server answered and, when it did not, the error's wire +//! name and whether it was a 401 that advertised OAuth. The model never sees it; a host reads it to meter calls //! and to surface failures without parsing the result text. // The tool names and descriptions are fixed strings, and the rendered Markdown From 2719f880dc890b54b9a39c13d5805ba5d3d71f2d Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 18:00:00 +0530 Subject: [PATCH 17/21] fix(tools): redact secrets from SecretScrubber Debug (#40) --- crates/tinymcp/src/tools/scrub.rs | 11 ++++++++++- crates/tinymcp/src/tools/scrub_tests.rs | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/tinymcp/src/tools/scrub.rs b/crates/tinymcp/src/tools/scrub.rs index 23906608..4616d1b8 100644 --- a/crates/tinymcp/src/tools/scrub.rs +++ b/crates/tinymcp/src/tools/scrub.rs @@ -30,12 +30,21 @@ const CREDENTIAL_QUERY_PARAM_NEEDLES: [&str; 7] = [ ]; /// Every secret one configured server was dialled with, ready to scrub. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct SecretScrubber { pub(super) secrets: Vec, strict: Vec, } +impl std::fmt::Debug for SecretScrubber { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SecretScrubber") + .field("secrets", &self.secrets.len()) + .field("strict", &self.strict.len()) + .finish() + } +} + impl SecretScrubber { /// The scrubber for the server named `server`, or an empty one (which /// scrubs nothing) when `registry` has no such server. diff --git a/crates/tinymcp/src/tools/scrub_tests.rs b/crates/tinymcp/src/tools/scrub_tests.rs index 63455518..9e0a2631 100644 --- a/crates/tinymcp/src/tools/scrub_tests.rs +++ b/crates/tinymcp/src/tools/scrub_tests.rs @@ -170,3 +170,19 @@ fn a_short_extra_secret_is_redacted_inside_larger_text() { assert!(map.contains_key("[redacted]"), "{value}"); assert!(map.contains_key("tool_k9x"), "{value}"); } + +#[test] +fn debug_output_never_contains_a_secret() { + let scrubber = SecretScrubber::new( + &McpAuthConfig::BearerToken { + token: "configured-token".into(), + }, + "https://example.com/mcp", + ) + .with_secrets(["host token/1".to_string()]); + let shown = format!("{scrubber:?} {scrubber:#?}"); + assert!(!shown.contains("configured-token"), "{shown}"); + assert!(!shown.contains("host"), "{shown}"); + assert!(!shown.contains("%2F"), "{shown}"); + assert!(shown.contains("SecretScrubber"), "{shown}"); +} From 60db2d0280b53b5b21554bf7bac16ea952a0b22e Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 18:00:00 +0530 Subject: [PATCH 18/21] fix(agent_tools): reject advertises_oauth without unauthorized on decode (#40) --- .../tinymcp-bus/src/agent_tools/mod_tests.rs | 21 ++++++++++++++++ crates/tinymcp-bus/src/agent_tools/types.rs | 25 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/crates/tinymcp-bus/src/agent_tools/mod_tests.rs b/crates/tinymcp-bus/src/agent_tools/mod_tests.rs index 3ea94c56..556dda57 100644 --- a/crates/tinymcp-bus/src/agent_tools/mod_tests.rs +++ b/crates/tinymcp-bus/src/agent_tools/mod_tests.rs @@ -431,3 +431,24 @@ fn decoding_an_outcome_rejects_what_the_constructors_never_produce() { ) ); } + +#[test] +fn decoding_an_error_rejects_an_oauth_advert_without_a_401() { + let decode = |unauthorized: bool, advertises_oauth: bool| { + serde_json::from_value::(json!({ + "code": "x", + "unauthorized": unauthorized, + "advertises_oauth": advertises_oauth, + })) + }; + assert!(decode(false, true).is_err()); + assert!(decode(false, false).is_ok()); + assert!(decode(true, false).is_ok()); + assert!(decode(true, true).is_ok()); + + let nested = json!({ + "kind": "mcp_call", "server": "docs", "tool": "t", "ok": false, + "error": { "code": "x", "unauthorized": false, "advertises_oauth": true }, + }); + assert_eq!(super::McpCallOutcome::from_metadata(&nested), None); +} diff --git a/crates/tinymcp-bus/src/agent_tools/types.rs b/crates/tinymcp-bus/src/agent_tools/types.rs index 1c74318a..6681dae1 100644 --- a/crates/tinymcp-bus/src/agent_tools/types.rs +++ b/crates/tinymcp-bus/src/agent_tools/types.rs @@ -187,7 +187,10 @@ impl TryFrom for McpCallOutcome { } /// Why an MCP call failed, classified for a host. +/// +/// Decoding rejects `advertises_oauth: true` without `unauthorized: true`. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(try_from = "McpCallErrorWire")] pub struct McpCallError { /// The error's wire name: one of the constants in [`crate::errors`]. pub code: String, @@ -198,6 +201,28 @@ pub struct McpCallError { pub advertises_oauth: bool, } +#[derive(Deserialize)] +struct McpCallErrorWire { + code: String, + unauthorized: bool, + advertises_oauth: bool, +} + +impl TryFrom for McpCallError { + type Error = &'static str; + + fn try_from(wire: McpCallErrorWire) -> Result { + if wire.advertises_oauth && !wire.unauthorized { + return Err("`advertises_oauth` requires `unauthorized`"); + } + Ok(Self { + code: wire.code, + unauthorized: wire.unauthorized, + advertises_oauth: wire.advertises_oauth, + }) + } +} + impl McpCallError { /// An error classified under `code` with no authorization signal. #[must_use] From c9e542e8246856121b1b94c9d664569fa7111a8f Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 18:00:00 +0530 Subject: [PATCH 19/21] fix(oauth): send no refresh request without a refresh token (#40) --- .../src/registry/oauth/mod_refresh_tests.rs | 28 +++++++++++++++++++ crates/tinymcp/src/registry/oauth/tokens.rs | 8 +++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs index 3ddf5868..4a85403e 100644 --- a/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs +++ b/crates/tinymcp/src/registry/oauth/mod_refresh_tests.rs @@ -219,3 +219,31 @@ async fn a_refresh_does_not_overwrite_a_sign_in_stored_while_it_waited() { "Bearer stale" ); } + +#[tokio::test] +async fn exchanging_a_bundle_without_a_refresh_token_sends_nothing() { + let requests = TokenRequests::default(); + let base = serve(token_endpoint( + requests.clone(), + json!({ "access_token": "fresh", "expires_in": 3600 }), + )) + .await; + let store = store_with_remote("https://example.test/mcp"); + store_expired_bundle(&store, &format!("{base}/token"), Some("r1")); + let http = reqwest::Client::new(); + for refresh_token in [None, Some(" ".to_string())] { + let bundle = OAuthBundle { + refresh_token, + client_id: "cli-1".into(), + client_secret: None, + token_endpoint: format!("{base}/token"), + expires_at: 1, + }; + assert!( + !crate::registry::oauth::tokens::exchange_refresh(&store, &http, "srv-1", &bundle) + .await + .unwrap() + ); + } + assert_eq!(requests.count.load(Ordering::SeqCst), 0); +} diff --git a/crates/tinymcp/src/registry/oauth/tokens.rs b/crates/tinymcp/src/registry/oauth/tokens.rs index 8162dabb..91da8aeb 100644 --- a/crates/tinymcp/src/registry/oauth/tokens.rs +++ b/crates/tinymcp/src/registry/oauth/tokens.rs @@ -105,7 +105,13 @@ pub(super) async fn exchange_refresh( where S: OAuthCredentialStore + ?Sized, { - let refresh_token = bundle.refresh_token.as_deref().unwrap_or_default(); + let Some(refresh_token) = bundle + .refresh_token + .as_deref() + .filter(|token| !token.trim().is_empty()) + else { + return Ok(false); + }; let mut form: Vec<(&str, &str)> = vec![ ("grant_type", "refresh_token"), ("refresh_token", refresh_token), From 621ab2b4bc1ed0f80057a322b30a6857b44e50c2 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 18:22:40 +0530 Subject: [PATCH 20/21] feat(oauth): let the host name the client it registers (#40) OAuthFlow::with_client_name sets the client_name sent during dynamic registration, which the authorization server shows on its consent screen. DEFAULT_CLIENT_NAME (TinyMCP) stays the default; a blank name keeps it. --- crates/tinymcp/src/registry/mod.rs | 4 +- crates/tinymcp/src/registry/oauth/flow.rs | 36 ++++++++++-- crates/tinymcp/src/registry/oauth/mod.rs | 2 +- .../tinymcp/src/registry/oauth/mod_tests.rs | 57 ++++++++++++++++--- 4 files changed, 83 insertions(+), 16 deletions(-) diff --git a/crates/tinymcp/src/registry/mod.rs b/crates/tinymcp/src/registry/mod.rs index c39a9509..b0250d03 100644 --- a/crates/tinymcp/src/registry/mod.rs +++ b/crates/tinymcp/src/registry/mod.rs @@ -23,7 +23,9 @@ pub mod supervisor; pub use boot::{BootOutcome, connect_installed_servers}; pub use config_doc::{AppliedServer, ConfigApplyReport}; pub use connections::{Connections, ProbeOutcome, REMOTE_REQUEST_TIMEOUT}; -pub use oauth::{AuthDetection, AuthKind, OAuthBundle, OAuthCredentialStore, OAuthFlow}; +pub use oauth::{ + AuthDetection, AuthKind, DEFAULT_CLIENT_NAME, OAuthBundle, OAuthCredentialStore, OAuthFlow, +}; pub use ops::McpRegistry; pub use setup::{SecretRef, SecretVault}; pub use sources::{Registries, RegistrySource}; diff --git a/crates/tinymcp/src/registry/oauth/flow.rs b/crates/tinymcp/src/registry/oauth/flow.rs index d1a4c8f7..86e76381 100644 --- a/crates/tinymcp/src/registry/oauth/flow.rs +++ b/crates/tinymcp/src/registry/oauth/flow.rs @@ -34,12 +34,12 @@ const AUTH_SERVER_TIMEOUT_SECS: u64 = 20; /// How long to wait when probing a server for what it wants. const PROBE_TIMEOUT_SECS: u64 = 20; -/// The client name presented during dynamic registration. +/// The client name presented during dynamic registration when the host sets +/// none with [`OAuthFlow::with_client_name`]. /// -/// A user sees this on the consent screen, so it names the library rather than -/// leaving it blank. A host that wants its own name there registers its own -/// client; that is a larger change than a string. -const CLIENT_NAME: &str = "TinyMCP"; +/// A user sees it on the consent screen, so it names the library rather than +/// leaving it blank. +pub const DEFAULT_CLIENT_NAME: &str = "TinyMCP"; /// Runs browser sign-in for HTTP-remote servers. /// @@ -53,6 +53,7 @@ pub struct OAuthFlow { proxy: Option, pending: Mutex>, public_endpoints_only: bool, + client_name: String, } impl OAuthFlow { @@ -82,9 +83,32 @@ impl OAuthFlow { proxy, pending: Mutex::new(HashMap::new()), public_endpoints_only: false, + client_name: DEFAULT_CLIENT_NAME.to_string(), }) } + /// Sets the client name sent during dynamic registration, which the + /// authorization server shows the user on its consent screen. + /// + /// A blank name keeps [`DEFAULT_CLIENT_NAME`]. Surrounding whitespace is + /// trimmed. Clients already registered keep the name they were registered + /// with. + #[must_use] + pub fn with_client_name(mut self, name: impl Into) -> Self { + let name = name.into(); + let name = name.trim(); + if !name.is_empty() { + self.client_name = name.to_string(); + } + self + } + + /// The client name sent during dynamic registration. + #[must_use] + pub fn client_name(&self) -> &str { + &self.client_name + } + /// Refuses authorization, registration and token endpoints that are not /// `https` or that resolve to an internal address. /// @@ -419,7 +443,7 @@ impl OAuthFlow { redirect_uri: &str, ) -> Result<(String, Option)> { let body = json!({ - "client_name": CLIENT_NAME, + "client_name": self.client_name, "redirect_uris": [redirect_uri], "grant_types": ["authorization_code", "refresh_token"], "response_types": ["code"], diff --git a/crates/tinymcp/src/registry/oauth/mod.rs b/crates/tinymcp/src/registry/oauth/mod.rs index 082fbfed..c6b38269 100644 --- a/crates/tinymcp/src/registry/oauth/mod.rs +++ b/crates/tinymcp/src/registry/oauth/mod.rs @@ -51,7 +51,7 @@ pub(crate) mod tokens; pub(crate) mod types; pub use credentials::OAuthCredentialStore; -pub use flow::OAuthFlow; +pub use flow::{DEFAULT_CLIENT_NAME, OAuthFlow}; pub use tokens::{OAUTH_BUNDLE_KEY, refresh_if_expired}; pub use types::{AuthDetection, AuthKind, OAuthBundle}; diff --git a/crates/tinymcp/src/registry/oauth/mod_tests.rs b/crates/tinymcp/src/registry/oauth/mod_tests.rs index a3e94592..6ffb6dac 100644 --- a/crates/tinymcp/src/registry/oauth/mod_tests.rs +++ b/crates/tinymcp/src/registry/oauth/mod_tests.rs @@ -19,7 +19,7 @@ use axum::routing::post; use axum::{Json, Router}; use serde_json::{Value, json}; -use super::flow::OAuthFlow; +use super::flow::{DEFAULT_CLIENT_NAME, OAuthFlow}; use super::tokens::{OAUTH_BUNDLE_KEY, refresh_if_expired}; use super::types::{AuthKind, TokenResponse}; use crate::Error; @@ -579,6 +579,8 @@ use axum::routing::get; #[derive(Debug, Default)] struct Authority { registrations: AtomicUsize, + /// The body the registration endpoint last received. + last_registration: parking_lot::Mutex>, exchanges: AtomicUsize, /// The form the token endpoint last received. last_form: parking_lot::Mutex>, @@ -600,13 +602,16 @@ fn endpoints() -> Router> { Router::new() .route( "/register", - post(|State(state): State>| async move { - state.registrations.fetch_add(1, Ordering::SeqCst); - axum::Json(json!({ - "client_id": "client-1", - "client_secret": "client-secret-1", - })) - }), + post( + |State(state): State>, Json(body): Json| async move { + state.registrations.fetch_add(1, Ordering::SeqCst); + *state.last_registration.lock() = Some(body); + axum::Json(json!({ + "client_id": "client-1", + "client_secret": "client-secret-1", + })) + }, + ), ) .route( "/token", @@ -791,6 +796,42 @@ async fn beginning_a_sign_in_registers_a_client_and_returns_an_authorize_url() { assert!(url.contains("/authorize"), "{url}"); } +async fn registered_client_name(flow: OAuthFlow) -> Option { + let (endpoint, state) = authority_with_challenge().await; + let store = store_with_remote(&endpoint); + flow.begin(&store, "srv-1", "http://127.0.0.1:7788/callback") + .await + .expect("begin"); + let body = state.last_registration.lock().clone(); + body.and_then(|body| body.get("client_name").cloned()) +} + +#[tokio::test] +async fn registration_names_the_library_when_the_host_sets_no_name() { + assert_eq!( + registered_client_name(flow()).await, + Some(json!(DEFAULT_CLIENT_NAME)) + ); +} + +#[tokio::test] +async fn registration_carries_the_client_name_the_host_sets() { + let flow = flow().with_client_name(" OpenCompany "); + assert_eq!(flow.client_name(), "OpenCompany"); + assert_eq!( + registered_client_name(flow).await, + Some(json!("OpenCompany")) + ); +} + +#[test] +fn a_blank_client_name_keeps_the_default() { + assert_eq!( + flow().with_client_name(" ").client_name(), + DEFAULT_CLIENT_NAME + ); +} + #[tokio::test] async fn the_authorize_url_carries_the_pkce_challenge_and_its_method() { // Without PKCE an intercepted code is enough to mint a token. The method From 8b92a99c8e59acc2729f67b8adb06e2d3d61b921 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Tue, 6 Oct 2026 18:22:40 +0530 Subject: [PATCH 21/21] docs(readme): describe OAuthFlow::with_client_name (#40) --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 59d10a42..54bd4fff 100644 --- a/README.md +++ b/README.md @@ -220,6 +220,9 @@ refusing the document. Credentials are write-only in both. `OAuthCredentialStore`, re-checking the token endpoint when `require_public_endpoints()` is set; the free `refresh_if_expired` does not check. `registry::OAuthBundle` is the stored refresh bundle's shape. +`OAuthFlow::with_client_name` sets the name dynamic registration sends, which +the authorization server shows on its consent screen; it defaults to +`DEFAULT_CLIENT_NAME` (`TinyMCP`). ## Static linking