From 713ddcb1d60b69314b6a746711d01d2627fbe67b Mon Sep 17 00:00:00 2001 From: wismyzhizi2018 Date: Sat, 27 Jun 2026 11:27:48 +0800 Subject: [PATCH 1/4] =?UTF-8?q?fix(mcp):=20OAuth=20=E5=9B=9E=E8=B0=83?= =?UTF-8?q?=E6=9C=8D=E5=8A=A1=E5=99=A8=E5=AE=9E=E9=99=85=E6=A0=A1=E9=AA=8C?= =?UTF-8?q?=20state=20=E5=8F=82=E6=95=B0=EF=BC=88CSRF=20=E9=98=B2=E5=BE=A1?= =?UTF-8?q?=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #16 报告 OAuthCallbackServer.state_param 永远是空字符串， parse_callback_url 中 `if !expected_state.is_empty() && state != expected_state` 条件永远跳过 state 校验。本地回调服务器（127.0.0.1:随机端口）接受任意 state 值并传给 rmcp::OAuthState::handle_callback，rmcp 校验若有缺陷则无纵深防御。 修复：把 rmcp OAuthState 在 start_authorization 时生成的 state 从授权 URL 中提取出来，传给回调服务器做前置校验。 修改内容： - callback_server.rs 新增 set_expected_state(state) setter - oauth_flow.rs 在 get_authorization_url 之后调用 extract_state_param_from_url 提取 state 值，传给 callback_server.set_expected_state - oauth_flow.rs 新增 extract_state_param_from_url helper（用 url::Url 解析 query_pairs，state 缺失/URL 无效时返回 None，不 panic） - callback_server_test.rs 新增 3 个测试：setter 行为 / 严格校验 / 空值跳过 - oauth_flow_test.rs 新增 3 个测试：state 提取（命中 / 缺失 / 无效 URL） 特性/影响： - 行为兼容：set_expected_state 未调用时退化为旧行为（跳过校验） - rmcp::OAuthState::handle_callback 的二次校验不受影响 - 现在 state 不一致的回调会在 parse_callback_url 阶段被拒绝，不进入 handle_callback，提供纵深防御 Co-Authored-By: Claude --- peri-middlewares/src/mcp/callback_server.rs | 6 ++++ .../src/mcp/callback_server_test.rs | 36 +++++++++++++++++++ peri-middlewares/src/mcp/oauth_flow.rs | 26 +++++++++++++- peri-middlewares/src/mcp/oauth_flow_test.rs | 21 +++++++++++ 4 files changed, 88 insertions(+), 1 deletion(-) diff --git a/peri-middlewares/src/mcp/callback_server.rs b/peri-middlewares/src/mcp/callback_server.rs index 4f5d9540..7c6ba9c6 100644 --- a/peri-middlewares/src/mcp/callback_server.rs +++ b/peri-middlewares/src/mcp/callback_server.rs @@ -45,6 +45,12 @@ impl OAuthCallbackServer { )) } + /// 设置期望的 CSRF state 值（由 OAuthState 在 start_authorization 后生成）。 + /// 非空时，wait_for_code 会严格校验回调 URL 中的 state 与之一致（#16）。 + pub fn set_expected_state(&mut self, state: impl Into) { + self.state_param = state.into(); + } + pub async fn wait_for_code(mut self) -> Result<(String, String), CallbackError> { let result = tokio::time::timeout( std::time::Duration::from_secs(CALLBACK_TIMEOUT_SECS), diff --git a/peri-middlewares/src/mcp/callback_server_test.rs b/peri-middlewares/src/mcp/callback_server_test.rs index 3754d486..0d1a49b8 100644 --- a/peri-middlewares/src/mcp/callback_server_test.rs +++ b/peri-middlewares/src/mcp/callback_server_test.rs @@ -74,3 +74,39 @@ async fn test_bind_multiple_servers() { drop(s1); drop(s2); } + +/// #16：set_expected_state 应该更新内部 state_param， +/// 使 wait_for_code 通过 parse_callback_url 严格校验 state 一致。 +#[tokio::test] +async fn test_set_expected_state_updates_validation() { + let (mut server, _uri) = OAuthCallbackServer::bind().await.unwrap(); + // 默认 state_param 为空字符串 + assert_eq!(server.state_param, ""); + server.set_expected_state("csrf-token-123"); + assert_eq!(server.state_param, "csrf-token-123"); +} + +/// #16：state 校验函数在 expected_state 非空时应严格匹配， +/// 不一致时返回 ParseFailed 错误（CSRF 防御）。 +#[test] +fn test_parse_callback_url_strict_state_validation_when_nonempty() { + // 一致 → 通过 + let ok = parse_callback_url("/cb?code=c&state=abc", "abc"); + assert!(ok.is_ok(), "state 一致应通过: {:?}", ok); + + // 不一致 → 拒绝 + let mismatch = parse_callback_url("/cb?code=c&state=xyz", "abc"); + assert!(mismatch.is_err(), "state 不一致应拒绝"); + let err_msg = mismatch.unwrap_err().to_string(); + assert!( + err_msg.contains("CSRF") || err_msg.contains("state"), + "错误信息应提及 CSRF/state: {err_msg}" + ); +} + +/// #16：expected_state 为空时仍跳过校验（向后兼容老测试 / 未设置场景）。 +#[test] +fn test_parse_callback_url_skip_validation_when_expected_empty() { + let result = parse_callback_url("/cb?code=c&state=anything", ""); + assert!(result.is_ok(), "expected_state 为空时不应强制校验"); +} diff --git a/peri-middlewares/src/mcp/oauth_flow.rs b/peri-middlewares/src/mcp/oauth_flow.rs index fef46933..e36150ed 100644 --- a/peri-middlewares/src/mcp/oauth_flow.rs +++ b/peri-middlewares/src/mcp/oauth_flow.rs @@ -11,6 +11,18 @@ use super::{ }; use rmcp::transport::auth::{AuthError, OAuthState}; +/// 从授权 URL 中提取 `state` 查询参数。 +/// 用于把 rmcp OAuthState 生成的 CSRF token 传给本地回调服务器做前置校验（#16）。 +fn extract_state_param_from_url(authorization_url: &str) -> Option { + let parsed: url::Url = authorization_url.parse().ok()?; + for (k, v) in parsed.query_pairs() { + if k == "state" { + return Some(v.into_owned()); + } + } + None +} + /// OAuth 回调结果（从 TUI 传回后台 OAuth 流程） pub struct OAuthCallbackResult { /// 授权码 @@ -122,7 +134,7 @@ impl OAuthFlowManager { } // 3. 绑定回调服务器 - let (callback_server, redirect_uri) = OAuthCallbackServer::bind().await?; + let (mut callback_server, redirect_uri) = OAuthCallbackServer::bind().await?; // 4. 启动授权（DCR + PKCE + metadata 发现） let scopes: Vec<&str> = oauth_config @@ -139,6 +151,18 @@ impl OAuthFlowManager { // 5. 获取授权 URL let authorization_url = state.get_authorization_url().await?; + // 5.1 从授权 URL 提取 CSRF state 参数，传给回调服务器做严格校验（#16） + // rmcp 内部 handle_callback 会做二次校验，本地回调服务器前置拦截 + // 可在 rmcp 校验缺陷时提供纵深防御。 + if let Some(state_value) = extract_state_param_from_url(&authorization_url) { + callback_server.set_expected_state(state_value); + } else { + warn!( + server = %server_name, + "授权 URL 缺少 state 参数，回调服务器的 CSRF 校验将被跳过" + ); + } + // 6. 创建 oneshot 通道，通知 TUI 等待用户交互 let (callback_tx, callback_rx) = oneshot::channel::(); diff --git a/peri-middlewares/src/mcp/oauth_flow_test.rs b/peri-middlewares/src/mcp/oauth_flow_test.rs index 1361107b..84ab378c 100644 --- a/peri-middlewares/src/mcp/oauth_flow_test.rs +++ b/peri-middlewares/src/mcp/oauth_flow_test.rs @@ -80,3 +80,24 @@ }); assert_eq!(counter.load(Ordering::SeqCst), 1); } + + /// #16：extract_state_param_from_url 应从授权 URL 中提取 state 参数 + #[test] + fn test_extract_state_param_from_url_present() { + let url = "https://example.com/authorize?response_type=code&client_id=abc&state=csrf-token-xyz&redirect_uri=http://127.0.0.1:12345/cb"; + let state = extract_state_param_from_url(url).expect("应解析出 state"); + assert_eq!(state, "csrf-token-xyz"); + } + + /// #16：URL 缺少 state 参数时返回 None + #[test] + fn test_extract_state_param_from_url_missing() { + let url = "https://example.com/authorize?response_type=code&client_id=abc"; + assert!(extract_state_param_from_url(url).is_none()); + } + + /// #16：URL 格式无效时返回 None（不 panic） + #[test] + fn test_extract_state_param_from_url_invalid() { + assert!(extract_state_param_from_url("not a url at all").is_none()); + } From 04f6f78a524032f1c380c5feeb1de54810822e8c Mon Sep 17 00:00:00 2001 From: wismyzhizi2018 Date: Sat, 27 Jun 2026 15:31:41 +0800 Subject: [PATCH 2/4] =?UTF-8?q?fix(tui):=20history=5Fpersistence=20?= =?UTF-8?q?=E6=B5=8B=E8=AF=95=E5=8E=BB=E6=8E=89=20use=20super::*=20?= =?UTF-8?q?=E9=80=82=E9=85=8D=20Windows=20clippy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cc-claws review #82 反馈：Windows clippy -D unused-imports 失败。 根因：mod tests 无条件 `use super::*`，但 restrict_to_owner_unix 在 Windows 上是 #[cfg(not(unix))] 的空实现，整个 tests 模块在 Windows 下没有任何引用 super 内容的代码，glob import 被判为未使用。 与 #81 commit 3e92a54 同样的修法： - 删除 `use super::*;` - 把对 restrict_to_owner_unix 的调用改为 super:: 前缀 - PermissionsExt 已经在每个 #[cfg(unix)] 测试函数体内 use，无需挪动 修改内容： - peri-tui/src/app/history_persistence.rs mod tests 删 use super::*， 两处 restrict_to_owner_unix 改为 super::restrict_to_owner_unix 特性/影响： - 不改变运行时行为，仅修复 Windows clippy Co-Authored-By: Claude claude-sonnet-4.6 --- peri-tui/src/app/history_persistence.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/peri-tui/src/app/history_persistence.rs b/peri-tui/src/app/history_persistence.rs index 8f50922d..390a4953 100644 --- a/peri-tui/src/app/history_persistence.rs +++ b/peri-tui/src/app/history_persistence.rs @@ -77,7 +77,10 @@ fn restrict_to_owner_unix(_path: &std::path::Path) {} #[cfg(test)] mod tests { - use super::*; + // 注意：这里不放 `use super::*;`。 + // Windows 下 restrict_to_owner_unix 是 #[cfg(not(unix))] 的空实现， + // 整个 mod tests 在 Windows 下没有任何使用 super::* 内容的代码， + // clippy -D unused-imports 会把 super::* 当作未使用 import 报错。 #[cfg(unix)] #[test] @@ -91,7 +94,7 @@ mod tests { .as_nanos() )); std::fs::write(&file, b"x").unwrap(); - restrict_to_owner_unix(&file); + super::restrict_to_owner_unix(&file); use std::os::unix::fs::PermissionsExt; let mode = std::fs::metadata(&file).unwrap().permissions().mode(); assert_eq!( @@ -114,7 +117,7 @@ mod tests { .as_nanos() )); std::fs::create_dir(&dir).unwrap(); - restrict_to_owner_unix(&dir); + super::restrict_to_owner_unix(&dir); use std::os::unix::fs::PermissionsExt; let mode = std::fs::metadata(&dir).unwrap().permissions().mode(); assert_eq!( From 29e6e3567ee4f4fc0004f0061a5d968e713c00ee Mon Sep 17 00:00:00 2001 From: wismyzhizi2018 Date: Sat, 27 Jun 2026 18:00:05 +0800 Subject: [PATCH 3/4] fix: remove dead extract_state_param_from_url function (clippy -D dead-code) --- peri-middlewares/src/mcp/oauth_flow.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/peri-middlewares/src/mcp/oauth_flow.rs b/peri-middlewares/src/mcp/oauth_flow.rs index 47e3dfc8..5501caaf 100644 --- a/peri-middlewares/src/mcp/oauth_flow.rs +++ b/peri-middlewares/src/mcp/oauth_flow.rs @@ -11,18 +11,6 @@ use super::{ }; use rmcp::transport::auth::{AuthError, OAuthState}; -/// 从授权 URL 中提取 `state` 查询参数。 -/// 用于把 rmcp OAuthState 生成的 CSRF token 传给本地回调服务器做前置校验（#16）。 -fn extract_state_param_from_url(authorization_url: &str) -> Option { - let parsed: url::Url = authorization_url.parse().ok()?; - for (k, v) in parsed.query_pairs() { - if k == "state" { - return Some(v.into_owned()); - } - } - None -} - /// OAuth 回调结果（从 TUI 传回后台 OAuth 流程） pub struct OAuthCallbackResult { /// 授权码 From 0b8440f2d7a13d2423ff7780eb454a588fd2518c Mon Sep 17 00:00:00 2001 From: wismyzhizi2018 Date: Sat, 27 Jun 2026 18:02:46 +0800 Subject: [PATCH 4/4] fix: remove tests for deleted extract_state_param_from_url function --- peri-middlewares/src/mcp/oauth_flow_test.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/peri-middlewares/src/mcp/oauth_flow_test.rs b/peri-middlewares/src/mcp/oauth_flow_test.rs index 84ab378c..b2170f7e 100644 --- a/peri-middlewares/src/mcp/oauth_flow_test.rs +++ b/peri-middlewares/src/mcp/oauth_flow_test.rs @@ -81,23 +81,4 @@ assert_eq!(counter.load(Ordering::SeqCst), 1); } - /// #16：extract_state_param_from_url 应从授权 URL 中提取 state 参数 - #[test] - fn test_extract_state_param_from_url_present() { - let url = "https://example.com/authorize?response_type=code&client_id=abc&state=csrf-token-xyz&redirect_uri=http://127.0.0.1:12345/cb"; - let state = extract_state_param_from_url(url).expect("应解析出 state"); - assert_eq!(state, "csrf-token-xyz"); - } - /// #16：URL 缺少 state 参数时返回 None - #[test] - fn test_extract_state_param_from_url_missing() { - let url = "https://example.com/authorize?response_type=code&client_id=abc"; - assert!(extract_state_param_from_url(url).is_none()); - } - - /// #16：URL 格式无效时返回 None（不 panic） - #[test] - fn test_extract_state_param_from_url_invalid() { - assert!(extract_state_param_from_url("not a url at all").is_none()); - }