Skip to content

fix(mcp): OAuth 回调服务器实际校验 state 参数 (#16)#82

Merged
cc-claws merged 5 commits into
mainfrom
fix/mcp-oauth-state-validation
Jun 27, 2026
Merged

fix(mcp): OAuth 回调服务器实际校验 state 参数 (#16)#82
cc-claws merged 5 commits into
mainfrom
fix/mcp-oauth-state-validation

Conversation

@cc-claws

Copy link
Copy Markdown
Owner

关联 Issue

Fixes #16

问题

#16 报告 OAuthCallbackServer.state_param 永远是空字符串:bind() 中硬编码 state_param: String::new(),导致 parse_callback_urlif !expected_state.is_empty() && state != expected_state 条件永远跳过 state 校验。

虽然 rmcp 的 OAuthState::handle_callbackoauth_flow.rs:181 做二次校验,登录流程本身仍受 rmcp 保护,但本地回调服务器(127.0.0.1:随机端口)接受任意 state 值并传给 handle_callback,缺乏纵深防御。

改动

把 rmcp OAuthState 在 start_authorization 时生成的 state 从授权 URL 中提取出来,传给回调服务器:

文件 改动
mcp/callback_server.rs 新增 set_expected_state(state) setter
mcp/oauth_flow.rs get_authorization_url 后调用 extract_state_param_from_url 提取 state 值,传给 callback_server.set_expected_state
mcp/oauth_flow.rs 新增 extract_state_param_from_url helper(url::Url::query_pairs 解析;state 缺失 / URL 无效时返回 None

特性 / 影响

  • 行为兼容set_expected_state 未调用时退化为旧行为(跳过校验),现有调用方/测试无破坏
  • rmcp 二次校验不变OAuthState::handle_callback 仍按 rmcp 内部逻辑校验 state,本次只是前置拦截
  • 新行为:state 不一致的回调在 parse_callback_url 阶段就被拒绝(返回 CallbackError::ParseFailed),不进入 handle_callback

验证

  • cargo check -p peri-middlewares 无 warning
  • 新增 test_set_expected_state_updates_validation — passed
  • 新增 test_parse_callback_url_strict_state_validation_when_nonempty — passed
  • 新增 test_parse_callback_url_skip_validation_when_expected_empty(向后兼容)— passed
  • 新增 test_extract_state_param_from_url_present / _missing / _invalid — all passed
  • cargo test -p peri-middlewares --lib mcp::callback_server — 13/13 passed
  • cargo test -p peri-middlewares --lib mcp::oauth_flow — 9/9 passed

Test Plan

  • CI 通过
  • 合并后跑一次真实 MCP OAuth 流程(如 Atlassian/GitHub MCP server),确认授权仍能完成

🤖 Generated with Claude Code

#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 <noreply@anthropic.com>
@cc-claws

Copy link
Copy Markdown
Owner Author

PR Review: fix(mcp): OAuth 回调服务器实际校验 state 参数

🔴 CI 失败:与 #81 相同的 unused import

history_persistence.rs:80use super::* 在 Windows clippy 下报 unused-imports 错误。此 PR 包含了 PR #81d13de19 提交(file permissions),继承了同一个问题。修复方式见 #81 review:删掉 use super::*,测试体内用 super::restrict_to_owner_unix 显式调用。


🟢 OAuth state 验证 — 正确

两个 commit 中 OAuth 部分(713ddcb)代码质量好:

  • set_expected_state(impl Into) 接口干净,setter 直接赋值
  • extract_state_param_from_urlurl::Url::query_pairs 解析,state 缺失 / URL 无效返回 None,不 panic
  • 纵深防御定位准确:rmcp 做主校验,本地回调服务器前置拦截作为补充
  • state 缺失时 warn! 日志而非报错,向后兼容

🟢 测试覆盖 — 完整

callback_server 3 个测试 + oauth_flow 3 个测试,覆盖 set/get、严格校验、空 state 跳过、URL 缺失、URL 无效。


总结

修掉 use super::* 后 OAuth 部分可以合入。建议先 rebase #81 修好再 rebase 此 PR,或者直接在此 PR 里一起修。

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 <noreply@anthropic.com>
@cc-claws

Copy link
Copy Markdown
Owner Author

@cc-claws 已修,commit 04f6f78

此 PR 在 d13de19 上叠加了同一个 fix——删 use super::*;、测试体内用 super::restrict_to_owner_unix 显式调用。OAuth 部分(713ddcb)未动,保持原状。本地 cargo build -p peri-tui 通过。

@cc-claws cc-claws merged commit 307a589 into main Jun 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth 回调服务器 state 参数实际未校验(永远为空字符串)

1 participant