mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-06-17 13:05:58 +00:00
fix(channels): require bound identity for user-owned IM messages (#3578)
* fix(channels): require bound identity for user-owned IM messages * make format * docs: document bound identity channel config * refactor: reuse channel connection config * refactor _requires_bound_identity() * refactor from_app_config() * make format * fix: reject unbound channel chats before semaphore * security enhancement * make format * fix: enforce bound-identity admission at command entry point The bound-identity gate only ran for non-command messages in _handle_message() and as a fallback inside _handle_chat(). Commands had no equivalent boundary, so an unbound platform user could send /new and reach _create_thread() directly, creating an unowned Gateway thread and empty checkpoint. Info commands (/status, /models, /memory) likewise leaked Gateway state to unbound users. Add the same _requires_bound_identity() check at the top of _handle_command(), rejecting via _reject_unbound_channel_message() before any thread creation or Gateway query. The gate is a no-op in legacy open-bot mode (require_bound_identity=False) and auth-disabled mode. Provider-level binding flows (/connect, /start) are consumed by the provider adapter before reaching the manager, so they are unaffected. Tests: - unbound auth-enabled /new is rejected before threads.create - bound auth-enabled /new still creates the thread Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(channels): carry workspace fallback decision on inbound messages * fix(channels): recheck bound identity by normalized workspace * fix(channels): avoid duplicate bound identity checks * fix(channels): preserve verified routing for bound identity rejects * fix(channels): clarify bound identity upgrade failures --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Nan Gao
@@ -2565,6 +2565,445 @@ class TestResolveRunParamsUserId:
|
||||
assert "channel_user_id" not in run_context
|
||||
|
||||
|
||||
class _BoundIdentityRepo:
|
||||
def __init__(self, connections: list[dict[str, str | None]] | None = None) -> None:
|
||||
self.connections = list(connections or [])
|
||||
self.lookups: list[dict[str, str | None]] = []
|
||||
self.thread_sets: list[dict[str, str | None]] = []
|
||||
|
||||
async def find_connection_by_external_identity(self, *, provider: str, external_account_id: str, workspace_id: str | None = None):
|
||||
self.lookups.append(
|
||||
{
|
||||
"provider": provider,
|
||||
"external_account_id": external_account_id,
|
||||
"workspace_id": workspace_id,
|
||||
}
|
||||
)
|
||||
for connection in self.connections:
|
||||
if connection.get("provider") == provider and connection.get("external_account_id") == external_account_id and connection.get("workspace_id") == workspace_id:
|
||||
return connection
|
||||
return None
|
||||
|
||||
async def get_thread_id(self, connection_id: str, chat_id: str, topic_id: str | None = None):
|
||||
return None
|
||||
|
||||
async def set_thread_id(
|
||||
self,
|
||||
*,
|
||||
connection_id: str,
|
||||
owner_user_id: str,
|
||||
provider: str,
|
||||
external_conversation_id: str,
|
||||
external_topic_id: str | None,
|
||||
thread_id: str,
|
||||
) -> None:
|
||||
self.thread_sets.append(
|
||||
{
|
||||
"connection_id": connection_id,
|
||||
"owner_user_id": owner_user_id,
|
||||
"provider": provider,
|
||||
"external_conversation_id": external_conversation_id,
|
||||
"external_topic_id": external_topic_id,
|
||||
"thread_id": thread_id,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
class TestChannelManagerBoundIdentityPolicy:
|
||||
def test_unbound_auth_enabled_chat_is_rejected_before_thread_or_run_creation(self, monkeypatch):
|
||||
from app.channels.manager import BOUND_IDENTITY_REQUIRED_MESSAGE, ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
manager = ChannelManager(bus=bus, store=store, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client()
|
||||
manager._client = mock_client
|
||||
outbound_received = []
|
||||
|
||||
async def capture(msg):
|
||||
outbound_received.append(msg)
|
||||
|
||||
bus.subscribe_outbound(capture)
|
||||
await manager._handle_chat(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
text="hi",
|
||||
thread_ts="1710000000.000100",
|
||||
)
|
||||
)
|
||||
|
||||
assert len(outbound_received) == 1
|
||||
assert outbound_received[0].text == BOUND_IDENTITY_REQUIRED_MESSAGE
|
||||
assert outbound_received[0].thread_id == ""
|
||||
assert outbound_received[0].connection_id is None
|
||||
assert outbound_received[0].owner_user_id is None
|
||||
mock_client.threads.create.assert_not_called()
|
||||
mock_client.runs.wait.assert_not_called()
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_bound_identity_repo_unavailable_uses_transient_failure_message(self, monkeypatch):
|
||||
from app.channels.manager import BOUND_IDENTITY_UNAVAILABLE_MESSAGE, ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
manager = ChannelManager(bus=bus, store=store, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client()
|
||||
manager._client = mock_client
|
||||
outbound_received = []
|
||||
|
||||
async def capture(msg):
|
||||
outbound_received.append(msg)
|
||||
|
||||
bus.subscribe_outbound(capture)
|
||||
await manager._handle_chat(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
owner_user_id="deerflow-user-1",
|
||||
connection_id="connection-1",
|
||||
workspace_id="T123",
|
||||
text="hi",
|
||||
)
|
||||
)
|
||||
|
||||
assert len(outbound_received) == 1
|
||||
assert outbound_received[0].text == BOUND_IDENTITY_UNAVAILABLE_MESSAGE
|
||||
assert outbound_received[0].connection_id is None
|
||||
assert outbound_received[0].owner_user_id is None
|
||||
mock_client.threads.create.assert_not_called()
|
||||
mock_client.runs.wait.assert_not_called()
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_unbound_auth_enabled_chat_is_rejected_before_semaphore(self, monkeypatch):
|
||||
from app.channels.manager import BOUND_IDENTITY_REQUIRED_MESSAGE, ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
manager = ChannelManager(bus=bus, store=store, require_bound_identity=True)
|
||||
outbound_received = []
|
||||
|
||||
async def capture(msg):
|
||||
outbound_received.append(msg)
|
||||
|
||||
bus.subscribe_outbound(capture)
|
||||
await manager.start()
|
||||
assert manager._semaphore is not None
|
||||
await manager._semaphore.acquire()
|
||||
try:
|
||||
await asyncio.wait_for(
|
||||
manager._handle_message(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
text="hi",
|
||||
)
|
||||
),
|
||||
timeout=0.5,
|
||||
)
|
||||
finally:
|
||||
manager._semaphore.release()
|
||||
await manager.stop()
|
||||
|
||||
assert len(outbound_received) == 1
|
||||
assert outbound_received[0].text == BOUND_IDENTITY_REQUIRED_MESSAGE
|
||||
assert outbound_received[0].connection_id is None
|
||||
assert outbound_received[0].owner_user_id is None
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_bound_auth_enabled_chat_is_allowed_when_bound_identity_is_required(self, monkeypatch):
|
||||
from app.channels.manager import ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
repo = _BoundIdentityRepo(
|
||||
[
|
||||
{
|
||||
"id": "connection-1",
|
||||
"owner_user_id": "deerflow-user-1",
|
||||
"provider": "slack",
|
||||
"external_account_id": "U-platform",
|
||||
"workspace_id": "T123",
|
||||
}
|
||||
]
|
||||
)
|
||||
manager = ChannelManager(bus=bus, store=store, connection_repo=repo, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client(thread_id="thread-bound")
|
||||
manager._client = mock_client
|
||||
|
||||
await manager._handle_chat(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
owner_user_id="deerflow-user-1",
|
||||
connection_id="connection-1",
|
||||
workspace_id="T123",
|
||||
text="hi",
|
||||
)
|
||||
)
|
||||
|
||||
mock_client.threads.create.assert_called_once()
|
||||
mock_client.runs.wait.assert_called_once()
|
||||
run_context = mock_client.runs.wait.call_args.kwargs["context"]
|
||||
assert run_context["user_id"] == "deerflow-user-1"
|
||||
assert run_context["channel_user_id"] == "U-platform"
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_bound_auth_enabled_message_checks_bound_identity_once_on_hot_path(self, monkeypatch):
|
||||
from app.channels.manager import ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
repo = _BoundIdentityRepo(
|
||||
[
|
||||
{
|
||||
"id": "connection-1",
|
||||
"owner_user_id": "deerflow-user-1",
|
||||
"provider": "slack",
|
||||
"external_account_id": "U-platform",
|
||||
"workspace_id": "T123",
|
||||
}
|
||||
]
|
||||
)
|
||||
manager = ChannelManager(bus=bus, store=store, connection_repo=repo, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client(thread_id="thread-bound")
|
||||
manager._client = mock_client
|
||||
await manager.start()
|
||||
try:
|
||||
await manager._handle_message(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
owner_user_id="deerflow-user-1",
|
||||
connection_id="connection-1",
|
||||
workspace_id="T123",
|
||||
text="hi",
|
||||
)
|
||||
)
|
||||
finally:
|
||||
await manager.stop()
|
||||
|
||||
assert repo.lookups == [
|
||||
{
|
||||
"provider": "slack",
|
||||
"external_account_id": "U-platform",
|
||||
"workspace_id": "T123",
|
||||
}
|
||||
]
|
||||
mock_client.threads.create.assert_called_once()
|
||||
mock_client.runs.wait.assert_called_once()
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_auth_enabled_chat_rejects_unverified_bound_identity(self, monkeypatch):
|
||||
from app.channels.manager import BOUND_IDENTITY_REQUIRED_MESSAGE, ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
repo = _BoundIdentityRepo(
|
||||
[
|
||||
{
|
||||
"id": "actual-connection",
|
||||
"owner_user_id": "actual-owner",
|
||||
"provider": "slack",
|
||||
"external_account_id": "U-platform",
|
||||
"workspace_id": None,
|
||||
}
|
||||
]
|
||||
)
|
||||
manager = ChannelManager(bus=bus, store=store, connection_repo=repo, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client()
|
||||
manager._client = mock_client
|
||||
outbound_received = []
|
||||
|
||||
async def capture(msg):
|
||||
outbound_received.append(msg)
|
||||
|
||||
bus.subscribe_outbound(capture)
|
||||
await manager._handle_chat(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
owner_user_id="forged-owner",
|
||||
connection_id="forged-connection",
|
||||
text="hi",
|
||||
)
|
||||
)
|
||||
|
||||
assert len(outbound_received) == 1
|
||||
assert outbound_received[0].text == BOUND_IDENTITY_REQUIRED_MESSAGE
|
||||
assert outbound_received[0].connection_id == "actual-connection"
|
||||
assert outbound_received[0].owner_user_id == "actual-owner"
|
||||
mock_client.threads.create.assert_not_called()
|
||||
mock_client.runs.wait.assert_not_called()
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_auth_disabled_chat_keeps_default_user_when_bound_identity_is_required(self, monkeypatch):
|
||||
from app.channels.manager import ChannelManager
|
||||
from app.gateway.auth_disabled import AUTH_DISABLED_USER_ID
|
||||
|
||||
monkeypatch.setenv("DEER_FLOW_AUTH_DISABLED", "1")
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
manager = ChannelManager(bus=bus, store=store, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client(thread_id="thread-local")
|
||||
manager._client = mock_client
|
||||
|
||||
await manager._handle_chat(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
text="hi",
|
||||
)
|
||||
)
|
||||
|
||||
mock_client.threads.create.assert_called_once()
|
||||
mock_client.runs.wait.assert_called_once()
|
||||
run_context = mock_client.runs.wait.call_args.kwargs["context"]
|
||||
assert run_context["user_id"] == AUTH_DISABLED_USER_ID
|
||||
assert run_context["channel_user_id"] == "U-platform"
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_legacy_open_bot_mode_allows_unbound_auth_enabled_chat(self, monkeypatch):
|
||||
from app.channels.manager import ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
manager = ChannelManager(bus=bus, store=store, require_bound_identity=False)
|
||||
mock_client = _make_mock_langgraph_client(thread_id="thread-legacy")
|
||||
manager._client = mock_client
|
||||
|
||||
await manager._handle_chat(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
text="hi",
|
||||
)
|
||||
)
|
||||
|
||||
mock_client.threads.create.assert_called_once()
|
||||
mock_client.runs.wait.assert_called_once()
|
||||
run_context = mock_client.runs.wait.call_args.kwargs["context"]
|
||||
assert run_context["user_id"] == "U-platform"
|
||||
assert run_context["channel_user_id"] == "U-platform"
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_unbound_auth_enabled_new_command_is_rejected_before_thread_creation(self, monkeypatch):
|
||||
from app.channels.manager import BOUND_IDENTITY_REQUIRED_MESSAGE, ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
manager = ChannelManager(bus=bus, store=store, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client()
|
||||
manager._client = mock_client
|
||||
outbound_received = []
|
||||
|
||||
async def capture(msg):
|
||||
outbound_received.append(msg)
|
||||
|
||||
bus.subscribe_outbound(capture)
|
||||
await manager._handle_command(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
text="/new",
|
||||
msg_type=InboundMessageType.COMMAND,
|
||||
thread_ts="1710000000.000100",
|
||||
)
|
||||
)
|
||||
|
||||
assert len(outbound_received) == 1
|
||||
assert outbound_received[0].text == BOUND_IDENTITY_REQUIRED_MESSAGE
|
||||
assert outbound_received[0].thread_id == ""
|
||||
assert outbound_received[0].connection_id is None
|
||||
assert outbound_received[0].owner_user_id is None
|
||||
mock_client.threads.create.assert_not_called()
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_bound_auth_enabled_new_command_creates_thread(self, monkeypatch):
|
||||
from app.channels.manager import ChannelManager
|
||||
|
||||
monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False)
|
||||
|
||||
async def go():
|
||||
bus = MessageBus()
|
||||
store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")
|
||||
repo = _BoundIdentityRepo(
|
||||
[
|
||||
{
|
||||
"id": "connection-1",
|
||||
"owner_user_id": "deerflow-user-1",
|
||||
"provider": "slack",
|
||||
"external_account_id": "U-platform",
|
||||
"workspace_id": "T123",
|
||||
}
|
||||
]
|
||||
)
|
||||
manager = ChannelManager(bus=bus, store=store, connection_repo=repo, require_bound_identity=True)
|
||||
mock_client = _make_mock_langgraph_client(thread_id="thread-bound")
|
||||
manager._client = mock_client
|
||||
|
||||
await manager._handle_command(
|
||||
InboundMessage(
|
||||
channel_name="slack",
|
||||
chat_id="C123",
|
||||
user_id="U-platform",
|
||||
owner_user_id="deerflow-user-1",
|
||||
connection_id="connection-1",
|
||||
workspace_id="T123",
|
||||
text="/new",
|
||||
msg_type=InboundMessageType.COMMAND,
|
||||
)
|
||||
)
|
||||
|
||||
mock_client.threads.create.assert_called_once()
|
||||
|
||||
_run(go())
|
||||
|
||||
|
||||
class TestChannelManagerConnectionRouting:
|
||||
def test_connection_scoped_conversations_do_not_share_threads(self, tmp_path, monkeypatch):
|
||||
from app.channels.manager import ChannelManager
|
||||
@@ -3811,6 +4250,13 @@ class TestChannelService:
|
||||
|
||||
assert service.manager._connection_repo is repo
|
||||
|
||||
def test_require_bound_identity_is_forwarded_to_manager(self):
|
||||
from app.channels.service import ChannelService
|
||||
|
||||
service = ChannelService(channels_config={}, require_bound_identity=True)
|
||||
|
||||
assert service.manager._require_bound_identity is True
|
||||
|
||||
def test_remove_channel_stops_running_channel_and_forgets_config(self):
|
||||
from app.channels.service import ChannelService
|
||||
|
||||
|
||||
Reference in New Issue
Block a user