From e51fd44964f7c22e696d1d4f97462be91c38e9dd Mon Sep 17 00:00:00 2001 From: jxxghp Date: Wed, 7 Oct 2026 09:16:33 +0800 Subject: [PATCH] fix(agent): resolve alternative QQ and Feishu user bindings Keep strict binding lookup as the default and explicitly match alternative channel identifiers for the Agent API gateway. Reject ambiguous active-user matches and cover single-field bindings with database-backed tests. Refs #6905 --- app/agent/tools/impl/api.py | 2 ++ app/application/security/user.py | 4 +++- app/db/adapters/user.py | 7 ++++-- docs/mcp-api.md | 5 +++++ skills/moviepilot-api/SKILL.md | 8 ++++++- tests/test_agent_api_gateway.py | 26 +++++++++++++++++++++- tests/test_builtin_skill_boundaries.py | 2 +- tests/test_user_repository.py | 30 ++++++++++++++++++++++++++ 8 files changed, 78 insertions(+), 6 deletions(-) diff --git a/app/agent/tools/impl/api.py b/app/agent/tools/impl/api.py index 222a15e35..f11f5d2a9 100644 --- a/app/agent/tools/impl/api.py +++ b/app/agent/tools/impl/api.py @@ -271,10 +271,12 @@ class MoviePilotApiTool(MoviePilotTool): else None ) if binding_keys: + # 渠道只提供一个标识，userid/openid 是候选字段；仓储统一拒绝跨用户歧义。 username = await self.run_blocking( "db", self._data.users.find_name_by_bindings, {key: raw_user_id for key in binding_keys}, + match_any=True, ) user = await self._data.users.async_get_by_name(username) if username else None if user is None and is_direct_user_channel and raw_user_id.isdigit(): diff --git a/app/application/security/user.py b/app/application/security/user.py index 8796326ef..7dfebe2d3 100644 --- a/app/application/security/user.py +++ b/app/application/security/user.py @@ -171,8 +171,10 @@ class ChainUserRepository(Protocol): def find_name_by_bindings( self, bindings: Mapping[str, object], + *, + match_any: bool = False, ) -> Optional[str]: - """解析唯一启用用户的渠道绑定，歧义时拒绝归属。""" + """默认匹配全部绑定；渠道候选字段可显式匹配任一项，仍须唯一启用用户。""" class UserRepository(Protocol): diff --git a/app/db/adapters/user.py b/app/db/adapters/user.py index 4ede7d41b..22771b45b 100644 --- a/app/db/adapters/user.py +++ b/app/db/adapters/user.py @@ -300,17 +300,20 @@ class TransactionalUserRepository(ChainUserRepository): def find_name_by_bindings( self, bindings: Mapping[str, object], + *, + match_any: bool = False, ) -> Optional[str]: - """仅在全部绑定唯一匹配同一启用用户时返回用户名。""" + """默认匹配全部绑定；候选字段可匹配任一项，但跨字段命中多位用户仍拒绝。""" if not bindings: return None expected = {key: str(value) for key, value in bindings.items()} + matches_bindings = any if match_any else all with self._sync_session() as session: matches = { model.name for model in UserOper(db=session).list() if model.is_active and model.settings - and all(model.settings.get(key) == value for key, value in expected.items()) + and matches_bindings(model.settings.get(key) == value for key, value in expected.items()) } return next(iter(matches)) if len(matches) == 1 else None diff --git a/docs/mcp-api.md b/docs/mcp-api.md index f4c536edc..285d4f20a 100644 --- a/docs/mcp-api.md +++ b/docs/mcp-api.md @@ -53,6 +53,11 @@ MCP 当前不会主动发送工具列表变更通知（`listChanged=false`）。 `execute_code` 仅在已绑定宿主会话的管理员 Agent 中提供，不发布到外部 HTTP/MCP 工具目录。内部只读 RPC 仍经过同一个 API/Skill/身份边界，默认直接执行，不要求用户逐次确认；外部客户端不能通过直接调用为自己创建 Python 会话身份。持久状态和回收语义见 [Python 只读工具编排](agent.md#python-只读工具编排)。 +内置 Agent 通过渠道调用业务 API 时，QQ 的 `qq_userid` / `qq_openid`、飞书的 +`feishu_userid` / `feishu_openid` 都是候选绑定字段，任一字段与当前渠道标识一致即可 +解析到对应的启用用户，无需将两个字段填成相同值。同一标识命中多个启用用户时拒绝归属； +停用或未绑定用户仍无法调用需要用户身份的业务 API。 + ### 插件实例日志等级 `plugin.loglevel.get` 使用源插件 ID 查询，返回该插件全部实例（首项固定是本体自身，其后 diff --git a/skills/moviepilot-api/SKILL.md b/skills/moviepilot-api/SKILL.md index 9f3c5595a..1b7bb2b50 100644 --- a/skills/moviepilot-api/SKILL.md +++ b/skills/moviepilot-api/SKILL.md @@ -1,6 +1,6 @@ --- name: moviepilot-api -version: 44 +version: 45 description: >- Use this skill for MoviePilot product operations such as media search, torrent search, downloads, subscriptions, library checks, sites, storage, workflows, @@ -90,6 +90,12 @@ users may update or delete only their own subscriptions. An unbound or inactive channel user must bind an active account before retrying, not switch to an administrator identity. +For QQ, either `qq_userid` or `qq_openid` may match the current channel ID; for +Feishu, either `feishu_userid` or `feishu_openid` may match. A single matching +field is sufficient, but the match must identify exactly one active MoviePilot +user across all candidate fields. Do not copy an openid into the userid field +as a workaround or bypass an ambiguous binding. + For `subscription.update`, omit settings that should stay unchanged. Send JSON `null` to clear a supported optional setting; see [Subscription APIs](api/subscription.md) for the exact fields and the form empty-string compatibility rule. diff --git a/tests/test_agent_api_gateway.py b/tests/test_agent_api_gateway.py index 281576155..89241d5e8 100644 --- a/tests/test_agent_api_gateway.py +++ b/tests/test_agent_api_gateway.py @@ -29,6 +29,9 @@ from app.agent.tools.impl.agent_task import AgentTaskTool from app.agent.tools.impl.api import MoviePilotApiInput, MoviePilotApiTool from app.agent.tools.impl.execute_command import ExecuteCommandTool from app.agent.tools.manager import MoviePilotToolsManager +from app.db.adapters.user import TransactionalUserRepository +from app.db.models.user import User +from app.db.session import SessionFactory, async_session_scope from app.schemas.types import NotificationChannel @@ -680,6 +683,27 @@ def test_gateway_maps_verified_channel_admin_only_for_admin_operation() -> None: assert identity == ("7", "admin", True) +@pytest.mark.parametrize("channel", [NotificationChannel.QQ, NotificationChannel.Feishu]) +@pytest.mark.parametrize("bound_suffix,other_suffix", [("openid", "userid"), ("userid", "openid")]) +@pytest.mark.parametrize("other_value", [None, "", "different-id", "channel-id"]) +def test_gateway_resolves_either_channel_binding(db, channel, bound_suffix, other_suffix, other_value) -> None: + """任一候选字段匹配即可绑定真实用户，另一字段缺失、为空或不同时仍可使用。""" + prefix = channel.name.lower() + bindings = {f"{prefix}_{bound_suffix}": "channel-id"} + if other_value is not None: + bindings[f"{prefix}_{other_suffix}"] = other_value + user = db.add(User(name="channel-owner", is_active=True, is_superuser=False, settings=bindings)) + users = TransactionalUserRepository(sync_session=SessionFactory, async_session=async_session_scope) + gateway = MoviePilotApiTool( + session_id="session", user_id="channel-id", data=SimpleNamespace(users=users), + ) + gateway.set_message_attr(channel=channel.value, source="main-bot", username="channel-nickname") + + identity = asyncio.run(gateway._resolve_api_identity()) + + assert identity == (str(user.id), "channel-owner", False) + + @pytest.mark.parametrize("is_channel_admin", [False, True]) @pytest.mark.parametrize("user_id,username", [(11, "alice"), (12, "bob")]) @pytest.mark.parametrize( @@ -721,7 +745,7 @@ def test_subscription_writes_keep_bound_channel_identity( assert (context.user_id, context.username, context.is_admin) == (str(user_id), username, False) users.find_name_by_bindings.assert_called_once_with({ "feishu_userid": f"ou_{username}", "feishu_openid": f"ou_{username}", - }) + }, match_any=True) superuser.assert_not_called() diff --git a/tests/test_builtin_skill_boundaries.py b/tests/test_builtin_skill_boundaries.py index 55308485f..52a9da8ea 100644 --- a/tests/test_builtin_skill_boundaries.py +++ b/tests/test_builtin_skill_boundaries.py @@ -117,7 +117,7 @@ def test_modified_builtin_skills_have_incremented_versions() -> None: "command-dispatch": "2", "database-operation": "9", "feedback-issue": "11", - "moviepilot-api": "44", + "moviepilot-api": "45", "moviepilot-update": "5", "organize-files": "5", "transfer-failed-retry": "5", diff --git a/tests/test_user_repository.py b/tests/test_user_repository.py index 477e15e81..b1246c828 100644 --- a/tests/test_user_repository.py +++ b/tests/test_user_repository.py @@ -265,6 +265,36 @@ def test_channel_binding_requires_all_supplied_identifiers(user_repository) -> N ) +@pytest.mark.parametrize("prefix", ["qq", "feishu"]) +@pytest.mark.parametrize("conflict_suffix", ["userid", "openid"]) +def test_channel_alias_binding_requires_unique_active_owner(user_repository, prefix, conflict_suffix) -> None: + """候选字段的任一匹配仍须归属唯一启用用户，同字段及跨字段冲突均拒绝。""" + repository, sync_factory = user_repository + expected = {f"{prefix}_userid": "channel-id", f"{prefix}_openid": "channel-id"} + _insert_user( + sync_factory, + name="disabled", + is_active=False, + settings={f"{prefix}_openid": "channel-id"}, + ) + assert repository.find_name_by_bindings(expected, match_any=True) is None + + _insert_user(sync_factory, name="active", settings={f"{prefix}_openid": "channel-id"}) + assert repository.find_name_by_bindings(expected, match_any=True) == "active" + + _insert_user(sync_factory, name="conflict", settings={f"{prefix}_{conflict_suffix}": "channel-id"}) + assert repository.find_name_by_bindings(expected, match_any=True) is None + + +@pytest.mark.parametrize("match_any", [False, True]) +def test_channel_binding_rejects_empty_candidates(user_repository, match_any) -> None: + """空查询在全部匹配和任一匹配模式下都不得归属现有用户。""" + repository, sync_factory = user_repository + _insert_user(sync_factory) + + assert repository.find_name_by_bindings({}, match_any=match_any) is None + + @pytest.mark.asyncio async def test_user_rename_migrates_configuration_atomically(tmp_path) -> None: """改名必须通过数据库级联迁移旧偏好。"""