fix(slack): honor NO_PROXY for Slack transport
This commit is contained in:
@@ -336,6 +336,39 @@ def proxy_kwargs_for_aiohttp(proxy_url: str | None) -> tuple[dict, dict]:
|
|||||||
return {}, {"proxy": proxy_url}
|
return {}, {"proxy": proxy_url}
|
||||||
|
|
||||||
|
|
||||||
|
def is_host_excluded_by_no_proxy(hostname: str, no_proxy_value: str | None = None) -> bool:
|
||||||
|
"""Return True when ``hostname`` matches a ``NO_PROXY`` entry.
|
||||||
|
|
||||||
|
Supports comma- or whitespace-separated entries with optional leading dots
|
||||||
|
and ``*.`` wildcards, which match both the apex domain and subdomains.
|
||||||
|
"""
|
||||||
|
raw = no_proxy_value
|
||||||
|
if raw is None:
|
||||||
|
raw = os.environ.get("NO_PROXY") or os.environ.get("no_proxy") or ""
|
||||||
|
|
||||||
|
raw = raw.strip()
|
||||||
|
if not raw:
|
||||||
|
return False
|
||||||
|
|
||||||
|
lower_hostname = hostname.lower()
|
||||||
|
for entry in re.split(r"[\s,]+", raw):
|
||||||
|
normalized = entry.strip().lower()
|
||||||
|
if not normalized:
|
||||||
|
continue
|
||||||
|
if normalized == "*":
|
||||||
|
return True
|
||||||
|
|
||||||
|
if normalized.startswith("*."):
|
||||||
|
normalized = normalized[2:]
|
||||||
|
elif normalized.startswith("."):
|
||||||
|
normalized = normalized[1:]
|
||||||
|
|
||||||
|
if lower_hostname == normalized or lower_hostname.endswith(f".{normalized}"):
|
||||||
|
return True
|
||||||
|
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
from dataclasses import dataclass, field
|
from dataclasses import dataclass, field
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|||||||
@@ -41,6 +41,8 @@ from gateway.platforms.base import (
|
|||||||
ProcessingOutcome,
|
ProcessingOutcome,
|
||||||
SendResult,
|
SendResult,
|
||||||
SUPPORTED_DOCUMENT_TYPES,
|
SUPPORTED_DOCUMENT_TYPES,
|
||||||
|
is_host_excluded_by_no_proxy,
|
||||||
|
resolve_proxy_url,
|
||||||
safe_url_for_log,
|
safe_url_for_log,
|
||||||
cache_document_from_bytes,
|
cache_document_from_bytes,
|
||||||
)
|
)
|
||||||
@@ -217,6 +219,40 @@ def _serialize_slack_blocks_for_agent(blocks: list, max_chars: int = 6000) -> st
|
|||||||
return f"[Slack Block Kit payload for this message]\n```json\n{payload}\n```"
|
return f"[Slack Block Kit payload for this message]\n```json\n{payload}\n```"
|
||||||
|
|
||||||
|
|
||||||
|
def _apply_slack_proxy(client: Any, proxy_url: Optional[str]) -> None:
|
||||||
|
"""Apply a resolved proxy to a Slack SDK client or clear it explicitly."""
|
||||||
|
if hasattr(client, "proxy"):
|
||||||
|
client.proxy = proxy_url
|
||||||
|
|
||||||
|
|
||||||
|
_SLACK_PROXY_HOSTS = (
|
||||||
|
"slack.com",
|
||||||
|
"files.slack.com",
|
||||||
|
"wss-primary.slack.com",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _resolve_slack_proxy_url() -> Optional[str]:
|
||||||
|
"""Resolve a proxy URL that Slack SDK clients can safely use."""
|
||||||
|
proxy_url = resolve_proxy_url()
|
||||||
|
if not proxy_url:
|
||||||
|
return None
|
||||||
|
|
||||||
|
normalized = proxy_url.lower()
|
||||||
|
if not normalized.startswith(("http://", "https://")):
|
||||||
|
logger.info(
|
||||||
|
"[Slack] Ignoring unsupported proxy scheme for Slack transport: %s",
|
||||||
|
safe_url_for_log(proxy_url),
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
|
||||||
|
if any(is_host_excluded_by_no_proxy(host) for host in _SLACK_PROXY_HOSTS):
|
||||||
|
logger.info("[Slack] NO_PROXY bypasses Slack proxy configuration")
|
||||||
|
return None
|
||||||
|
|
||||||
|
return proxy_url
|
||||||
|
|
||||||
|
|
||||||
class SlackAdapter(BasePlatformAdapter):
|
class SlackAdapter(BasePlatformAdapter):
|
||||||
"""
|
"""
|
||||||
Slack bot adapter using Socket Mode.
|
Slack bot adapter using Socket Mode.
|
||||||
@@ -237,13 +273,13 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
|
|
||||||
def __init__(self, config: PlatformConfig):
|
def __init__(self, config: PlatformConfig):
|
||||||
super().__init__(config, Platform.SLACK)
|
super().__init__(config, Platform.SLACK)
|
||||||
self._app: Optional[AsyncApp] = None
|
self._app: Optional[Any] = None
|
||||||
self._handler: Optional[AsyncSocketModeHandler] = None
|
self._handler: Optional[Any] = None
|
||||||
self._bot_user_id: Optional[str] = None
|
self._bot_user_id: Optional[str] = None
|
||||||
self._user_name_cache: Dict[str, str] = {} # user_id → display name
|
self._user_name_cache: Dict[str, str] = {} # user_id → display name
|
||||||
self._socket_mode_task: Optional[asyncio.Task] = None
|
self._socket_mode_task: Optional[asyncio.Task] = None
|
||||||
# Multi-workspace support
|
# Multi-workspace support
|
||||||
self._team_clients: Dict[str, AsyncWebClient] = {} # team_id → WebClient
|
self._team_clients: Dict[str, Any] = {} # team_id → WebClient
|
||||||
self._team_bot_user_ids: Dict[str, str] = {} # team_id → bot_user_id
|
self._team_bot_user_ids: Dict[str, str] = {} # team_id → bot_user_id
|
||||||
self._channel_team: Dict[str, str] = {} # channel_id → team_id
|
self._channel_team: Dict[str, str] = {} # channel_id → team_id
|
||||||
# Dedup cache: prevents duplicate bot responses when Socket Mode
|
# Dedup cache: prevents duplicate bot responses when Socket Mode
|
||||||
@@ -350,6 +386,10 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
logger.error("[Slack] SLACK_APP_TOKEN not set")
|
logger.error("[Slack] SLACK_APP_TOKEN not set")
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
proxy_url = _resolve_slack_proxy_url()
|
||||||
|
if proxy_url:
|
||||||
|
logger.info("[Slack] Using proxy for Slack transport: %s", safe_url_for_log(proxy_url))
|
||||||
|
|
||||||
# Support comma-separated bot tokens for multi-workspace
|
# Support comma-separated bot tokens for multi-workspace
|
||||||
bot_tokens = [t.strip() for t in raw_token.split(",") if t.strip()]
|
bot_tokens = [t.strip() for t in raw_token.split(",") if t.strip()]
|
||||||
|
|
||||||
@@ -377,10 +417,12 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
# First token is the primary — used for AsyncApp / Socket Mode
|
# First token is the primary — used for AsyncApp / Socket Mode
|
||||||
primary_token = bot_tokens[0]
|
primary_token = bot_tokens[0]
|
||||||
self._app = AsyncApp(token=primary_token)
|
self._app = AsyncApp(token=primary_token)
|
||||||
|
_apply_slack_proxy(self._app.client, proxy_url)
|
||||||
|
|
||||||
# Register each bot token and map team_id → client
|
# Register each bot token and map team_id → client
|
||||||
for token in bot_tokens:
|
for token in bot_tokens:
|
||||||
client = AsyncWebClient(token=token)
|
client = AsyncWebClient(token=token)
|
||||||
|
_apply_slack_proxy(client, proxy_url)
|
||||||
auth_response = await client.auth_test()
|
auth_response = await client.auth_test()
|
||||||
team_id = auth_response.get("team_id", "")
|
team_id = auth_response.get("team_id", "")
|
||||||
bot_user_id = auth_response.get("user_id", "")
|
bot_user_id = auth_response.get("user_id", "")
|
||||||
@@ -473,7 +515,8 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
self._app.action(_action_id)(self._handle_approval_action)
|
self._app.action(_action_id)(self._handle_approval_action)
|
||||||
|
|
||||||
# Start Socket Mode handler in background
|
# Start Socket Mode handler in background
|
||||||
self._handler = AsyncSocketModeHandler(self._app, app_token)
|
self._handler = AsyncSocketModeHandler(self._app, app_token, proxy=proxy_url)
|
||||||
|
_apply_slack_proxy(self._handler.client, proxy_url)
|
||||||
self._socket_mode_task = asyncio.create_task(self._handler.start_async())
|
self._socket_mode_task = asyncio.create_task(self._handler.start_async())
|
||||||
|
|
||||||
self._running = True
|
self._running = True
|
||||||
@@ -503,7 +546,7 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
|
|
||||||
logger.info("[Slack] Disconnected")
|
logger.info("[Slack] Disconnected")
|
||||||
|
|
||||||
def _get_client(self, chat_id: str) -> AsyncWebClient:
|
def _get_client(self, chat_id: str) -> Any:
|
||||||
"""Return the workspace-specific WebClient for a channel."""
|
"""Return the workspace-specific WebClient for a channel."""
|
||||||
team_id = self._channel_team.get(chat_id)
|
team_id = self._channel_team.get(chat_id)
|
||||||
if team_id and team_id in self._team_clients:
|
if team_id and team_id in self._team_clients:
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ We mock the slack modules at import time to avoid collection errors.
|
|||||||
import asyncio
|
import asyncio
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
from unittest.mock import AsyncMock, MagicMock, patch, call
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
@@ -21,6 +21,7 @@ from gateway.platforms.base import (
|
|||||||
MessageType,
|
MessageType,
|
||||||
SendResult,
|
SendResult,
|
||||||
SUPPORTED_DOCUMENT_TYPES,
|
SUPPORTED_DOCUMENT_TYPES,
|
||||||
|
is_host_excluded_by_no_proxy,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -188,6 +189,198 @@ class TestSlackConnectCleanup:
|
|||||||
assert adapter._platform_lock_identity is None
|
assert adapter._platform_lock_identity is None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# TestSlackProxyBehavior
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class TestSlackProxyBehavior:
|
||||||
|
def test_no_proxy_helper_matches_slack_hosts(self):
|
||||||
|
assert is_host_excluded_by_no_proxy("slack.com", "localhost,.slack.com")
|
||||||
|
assert is_host_excluded_by_no_proxy("files.slack.com", "localhost slack.com")
|
||||||
|
assert is_host_excluded_by_no_proxy("wss-primary.slack.com", "*")
|
||||||
|
assert not is_host_excluded_by_no_proxy("slack.com", "localhost,.internal.corp")
|
||||||
|
|
||||||
|
def test_resolve_slack_proxy_url_ignores_unsupported_proxy_schemes(self):
|
||||||
|
with patch.object(_slack_mod, "resolve_proxy_url", return_value="socks5://proxy.example.com:1080"):
|
||||||
|
assert _slack_mod._resolve_slack_proxy_url() is None
|
||||||
|
|
||||||
|
def test_resolve_slack_proxy_url_checks_all_slack_hosts(self):
|
||||||
|
with patch.object(_slack_mod, "resolve_proxy_url", return_value="http://proxy.example.com:3128"), \
|
||||||
|
patch.object(_slack_mod, "is_host_excluded_by_no_proxy", side_effect=lambda host: host == "wss-primary.slack.com") as excluded:
|
||||||
|
assert _slack_mod._resolve_slack_proxy_url() is None
|
||||||
|
excluded.assert_has_calls([
|
||||||
|
call("slack.com"),
|
||||||
|
call("files.slack.com"),
|
||||||
|
call("wss-primary.slack.com"),
|
||||||
|
])
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_connect_uses_proxy_when_not_bypassed(self):
|
||||||
|
created_apps = []
|
||||||
|
created_clients = []
|
||||||
|
|
||||||
|
class FakeWebClient:
|
||||||
|
def __init__(self, token):
|
||||||
|
self.token = token
|
||||||
|
self.proxy = "constructor-default"
|
||||||
|
suffix = token.split("-")[-1]
|
||||||
|
self.auth_test = AsyncMock(return_value={
|
||||||
|
"team_id": f"T_{suffix}",
|
||||||
|
"user_id": f"U_{suffix}",
|
||||||
|
"user": f"bot-{suffix}",
|
||||||
|
"team": f"Team {suffix}",
|
||||||
|
})
|
||||||
|
created_clients.append(self)
|
||||||
|
|
||||||
|
class FakeApp:
|
||||||
|
def __init__(self, token):
|
||||||
|
self.token = token
|
||||||
|
self.client = FakeWebClient(token)
|
||||||
|
self.registered_events = []
|
||||||
|
self.registered_commands = []
|
||||||
|
self.registered_actions = []
|
||||||
|
created_apps.append(self)
|
||||||
|
|
||||||
|
def event(self, event_type):
|
||||||
|
self.registered_events.append(event_type)
|
||||||
|
|
||||||
|
def decorator(fn):
|
||||||
|
return fn
|
||||||
|
|
||||||
|
return decorator
|
||||||
|
|
||||||
|
def command(self, command_name):
|
||||||
|
self.registered_commands.append(command_name)
|
||||||
|
|
||||||
|
def decorator(fn):
|
||||||
|
return fn
|
||||||
|
|
||||||
|
return decorator
|
||||||
|
|
||||||
|
def action(self, action_id):
|
||||||
|
self.registered_actions.append(action_id)
|
||||||
|
|
||||||
|
def decorator(fn):
|
||||||
|
return fn
|
||||||
|
|
||||||
|
return decorator
|
||||||
|
|
||||||
|
class FakeSocketModeHandler:
|
||||||
|
def __init__(self, app, app_token, proxy=None):
|
||||||
|
self.app = app
|
||||||
|
self.app_token = app_token
|
||||||
|
self.proxy = proxy
|
||||||
|
self.client = MagicMock(proxy="constructor-default")
|
||||||
|
|
||||||
|
def start_async(self):
|
||||||
|
return None
|
||||||
|
|
||||||
|
async def close_async(self):
|
||||||
|
return None
|
||||||
|
|
||||||
|
config = PlatformConfig(enabled=True, token="xoxb-primary,xoxb-secondary")
|
||||||
|
adapter = SlackAdapter(config)
|
||||||
|
|
||||||
|
with patch.object(_slack_mod, "AsyncApp", side_effect=FakeApp), \
|
||||||
|
patch.object(_slack_mod, "AsyncWebClient", side_effect=FakeWebClient), \
|
||||||
|
patch.object(_slack_mod, "AsyncSocketModeHandler", FakeSocketModeHandler), \
|
||||||
|
patch.object(_slack_mod, "_resolve_slack_proxy_url", return_value="http://proxy.example.com:3128"), \
|
||||||
|
patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}, clear=False), \
|
||||||
|
patch("gateway.status.acquire_scoped_lock", return_value=(True, None)), \
|
||||||
|
patch("asyncio.create_task", return_value=MagicMock(name="socket-mode-task")):
|
||||||
|
result = await adapter.connect()
|
||||||
|
|
||||||
|
assert result is True
|
||||||
|
assert created_apps[0].client.proxy == "http://proxy.example.com:3128"
|
||||||
|
assert all(client.proxy == "http://proxy.example.com:3128" for client in created_clients)
|
||||||
|
assert adapter._handler is not None
|
||||||
|
assert adapter._handler.proxy == "http://proxy.example.com:3128"
|
||||||
|
assert adapter._handler.client.proxy == "http://proxy.example.com:3128"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_connect_clears_proxy_when_no_proxy_matches_slack(self):
|
||||||
|
created_apps = []
|
||||||
|
created_clients = []
|
||||||
|
|
||||||
|
class FakeWebClient:
|
||||||
|
def __init__(self, token):
|
||||||
|
self.token = token
|
||||||
|
self.proxy = "constructor-default"
|
||||||
|
suffix = token.split("-")[-1]
|
||||||
|
self.auth_test = AsyncMock(return_value={
|
||||||
|
"team_id": f"T_{suffix}",
|
||||||
|
"user_id": f"U_{suffix}",
|
||||||
|
"user": f"bot-{suffix}",
|
||||||
|
"team": f"Team {suffix}",
|
||||||
|
})
|
||||||
|
created_clients.append(self)
|
||||||
|
|
||||||
|
class FakeApp:
|
||||||
|
def __init__(self, token):
|
||||||
|
self.token = token
|
||||||
|
self.client = FakeWebClient(token)
|
||||||
|
self.registered_events = []
|
||||||
|
self.registered_commands = []
|
||||||
|
self.registered_actions = []
|
||||||
|
created_apps.append(self)
|
||||||
|
|
||||||
|
def event(self, event_type):
|
||||||
|
self.registered_events.append(event_type)
|
||||||
|
|
||||||
|
def decorator(fn):
|
||||||
|
return fn
|
||||||
|
|
||||||
|
return decorator
|
||||||
|
|
||||||
|
def command(self, command_name):
|
||||||
|
self.registered_commands.append(command_name)
|
||||||
|
|
||||||
|
def decorator(fn):
|
||||||
|
return fn
|
||||||
|
|
||||||
|
return decorator
|
||||||
|
|
||||||
|
def action(self, action_id):
|
||||||
|
self.registered_actions.append(action_id)
|
||||||
|
|
||||||
|
def decorator(fn):
|
||||||
|
return fn
|
||||||
|
|
||||||
|
return decorator
|
||||||
|
|
||||||
|
class FakeSocketModeHandler:
|
||||||
|
def __init__(self, app, app_token, proxy=None):
|
||||||
|
self.app = app
|
||||||
|
self.app_token = app_token
|
||||||
|
self.proxy = proxy
|
||||||
|
self.client = MagicMock(proxy="constructor-default")
|
||||||
|
|
||||||
|
def start_async(self):
|
||||||
|
return None
|
||||||
|
|
||||||
|
async def close_async(self):
|
||||||
|
return None
|
||||||
|
|
||||||
|
config = PlatformConfig(enabled=True, token="xoxb-primary")
|
||||||
|
adapter = SlackAdapter(config)
|
||||||
|
|
||||||
|
with patch.object(_slack_mod, "AsyncApp", side_effect=FakeApp), \
|
||||||
|
patch.object(_slack_mod, "AsyncWebClient", side_effect=FakeWebClient), \
|
||||||
|
patch.object(_slack_mod, "AsyncSocketModeHandler", FakeSocketModeHandler), \
|
||||||
|
patch.object(_slack_mod, "_resolve_slack_proxy_url", return_value=None), \
|
||||||
|
patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}, clear=False), \
|
||||||
|
patch("gateway.status.acquire_scoped_lock", return_value=(True, None)), \
|
||||||
|
patch("asyncio.create_task", return_value=MagicMock(name="socket-mode-task")):
|
||||||
|
result = await adapter.connect()
|
||||||
|
|
||||||
|
assert result is True
|
||||||
|
assert created_apps[0].client.proxy is None
|
||||||
|
assert all(client.proxy is None for client in created_clients)
|
||||||
|
assert adapter._handler is not None
|
||||||
|
assert adapter._handler.proxy is None
|
||||||
|
assert adapter._handler.client.proxy is None
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# TestSendDocument
|
# TestSendDocument
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user