From 37580862b96d6a4f16967b0b68537b1ccfd4b97f Mon Sep 17 00:00:00 2001 From: Aari Date: Mon, 13 Jul 2026 16:14:36 +0800 Subject: [PATCH] fix(channels): scope the slash-skill whitelist check to the run's owner (#4129) _channel_storage_user_id is the single source of truth for a channel run's identity: _resolve_run_params resolves the owner into run_context["user_id"]. The /skill whitelist pre-check for a per-user custom agent (_resolve_available_skill_names) resolves that owner and then drops it, calling load_agent_config(name) with no user_id. It falls back to get_effective_user_id(), but this runs on the ChannelManager dispatch loop where the _current_user contextvar is never set, so it resolves "default" -- reading users/default/agents/{name}/ instead of the owner's bucket. When that bucket has no such agent (the common case) load_agent_config raises FileNotFoundError, which the dispatch loop turns into "An internal error occurred" on every /skill command; when a foreign agent shares the name, the whitelist is decided by the wrong user's skills list. Pass the resolved owner (run_context["user_id"]) to load_agent_config, matching every other caller (gateway/routers/agents.py, update_agent_tool.py, github/registry.py). None when no owner is resolvable, preserving the prior default-user behavior for unbound/no-auth channels. --- backend/app/channels/manager.py | 6 ++++- backend/tests/test_channels.py | 43 ++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/backend/app/channels/manager.py b/backend/app/channels/manager.py index 0bbd8ee50..06106b789 100644 --- a/backend/app/channels/manager.py +++ b/backend/app/channels/manager.py @@ -1062,7 +1062,11 @@ class ChannelManager: if not isinstance(agent_name, str) or not agent_name.strip(): return None - agent_config = load_agent_config(_normalize_custom_agent_name(agent_name)) + # Read the agent config from the same owner bucket the run uses: + # ``run_context["user_id"]`` is the resolved owner (``_channel_storage_user_id``), + # but without it ``load_agent_config`` falls back to the dispatch loop's unset + # contextvar (``"default"``), reading the wrong user's per-user custom agent. + agent_config = load_agent_config(_normalize_custom_agent_name(agent_name), user_id=run_context.get("user_id")) if agent_config and agent_config.skills is not None: return set(agent_config.skills) return None diff --git a/backend/tests/test_channels.py b/backend/tests/test_channels.py index 1775044bc..1c5edd4b2 100644 --- a/backend/tests/test_channels.py +++ b/backend/tests/test_channels.py @@ -2412,7 +2412,7 @@ class TestChannelManager: def test_handle_command_slash_skill_respects_custom_agent_skill_whitelist(self, monkeypatch, tmp_path): from app.channels.manager import ChannelManager - monkeypatch.setattr("app.channels.manager.load_agent_config", lambda name: SimpleNamespace(skills=["frontend-design"])) + monkeypatch.setattr("app.channels.manager.load_agent_config", lambda name, *, user_id=None: SimpleNamespace(skills=["frontend-design"])) async def go(): bus = MessageBus() @@ -2451,6 +2451,47 @@ class TestChannelManager: _run(go()) + def test_slash_skill_whitelist_loads_agent_config_for_the_resolved_owner(self, monkeypatch): + """The per-user custom agent whitelist must be read from the same owner + bucket the run uses. ``_resolve_run_params`` resolves that owner into + ``run_context["user_id"]`` (per ``_channel_storage_user_id``, the single + source of truth for run identity and storage), but the whitelist + pre-check dropped it, so ``load_agent_config`` fell back to the dispatch + loop's unset contextvar (``"default"``) — reading, or failing to find, + the wrong user's agent config. + """ + from app.channels.manager import ChannelManager + + captured: dict[str, object] = {} + + def spy_load_agent_config(name, *, user_id=None): + captured["name"] = name + captured["user_id"] = user_id + return SimpleNamespace(skills=["data-analysis"]) + + monkeypatch.setattr("app.channels.manager.load_agent_config", spy_load_agent_config) + + bus = MessageBus() + store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + manager = ChannelManager(bus=bus, store=store, default_session={"assistant_id": "analyst-agent"}) + + # A bound connection: the owner resolves to a real, non-default bucket. + msg = InboundMessage( + channel_name="test", + chat_id="chat1", + user_id="platform-user", + owner_user_id="owner-alice", + text="/data-analysis go", + msg_type=InboundMessageType.COMMAND, + ) + + expected_owner = manager._resolve_run_params(msg, "")[2].get("user_id") + + manager._resolve_available_skill_names(msg) + + assert expected_owner and expected_owner != "default" + assert captured["user_id"] == expected_owner + def test_handle_command_slash_skill_reports_disabled_skill(self, tmp_path): from app.channels.manager import ChannelManager