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.
This commit is contained in:
Aari 2026-07-13 16:14:36 +08:00 committed by GitHub
parent cbbd72a1ab
commit 37580862b9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 47 additions and 2 deletions

View File

@ -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

View File

@ -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