diff --git a/hermes_cli/config.py b/hermes_cli/config.py index f698c11d5ac..c81df25c03b 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -3426,6 +3426,7 @@ OPTIONAL_ENV_VARS = { "Required scopes: chat:write, app_mentions:read, channels:history, groups:history, " "im:history, im:read, im:write, users:read, files:read, files:write", "prompt": "Slack Bot Token (xoxb-...)", + "help": "In your Slack app, add the required bot scopes, install the app to the workspace, then copy OAuth & Permissions > Bot User OAuth Token.", "url": "https://api.slack.com/apps", "password": True, "category": "messaging", @@ -3435,10 +3436,19 @@ OPTIONAL_ENV_VARS = { "App-Level Tokens. Also ensure Event Subscriptions include: message.im, " "message.channels, message.groups, app_mention", "prompt": "Slack App Token (xapp-...)", + "help": "In your Slack app, enable Socket Mode, then create Basic Information > App-Level Tokens with the connections:write scope.", "url": "https://api.slack.com/apps", "password": True, "category": "messaging", }, + "SLACK_ALLOWED_USERS": { + "description": "Comma-separated Slack member IDs allowed to use Hermes, e.g. U01ABC2DEF3. Without this, Slack may connect but deny messages by default.", + "prompt": "Allowed Slack member IDs", + "help": "In Slack, open your profile, choose More or the three-dot menu, then Copy member ID. Add multiple IDs comma-separated.", + "url": "https://api.slack.com/apps", + "password": False, + "category": "messaging", + }, "MATTERMOST_URL": { "description": "Mattermost server URL (e.g. https://mm.example.com)", "prompt": "Mattermost server URL", diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 2dbb316d32d..b0d51e2481e 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2325,6 +2325,43 @@ def _gateway_display_command(profile: Optional[str], verb: str) -> str: return " ".join(["hermes", *_gateway_subcommand(profile, verb)]) +# Slack member IDs (users U..., Enterprise Grid W...). Kept in sync with the +# frontend SLACK_MEMBER_ID_RE in web/src/pages/ChannelsPage.tsx. +_SLACK_MEMBER_ID_RE = re.compile(r"[UW][A-Z0-9]{2,}") + + +def _validate_messaging_env_value(platform_id: str, key: str, value: str) -> None: + """Reject platform credentials that are clearly in the wrong field.""" + if platform_id != "slack" or not value: + return + + if key == "SLACK_BOT_TOKEN" and not value.startswith("xoxb-"): + raise HTTPException( + status_code=400, + detail="Slack Bot Token must start with xoxb-. Paste the bot token from OAuth & Permissions.", + ) + if key == "SLACK_APP_TOKEN" and not value.startswith("xapp-"): + raise HTTPException( + status_code=400, + detail="Slack App Token must start with xapp-. Paste the app-level token from Basic Information > App-Level Tokens.", + ) + if key == "SLACK_ALLOWED_USERS": + # Mirror the gateway's parse (gateway/platforms/slack.py): split on comma, + # strip, and drop empty entries so a trailing/interior comma isn't rejected + # here when the runtime would accept it. "*" is the allow-all wildcard. + user_ids = [part.strip() for part in value.split(",") if part.strip()] + invalid = [ + user_id + for user_id in user_ids + if user_id != "*" and not _SLACK_MEMBER_ID_RE.fullmatch(user_id) + ] + if invalid: + raise HTTPException( + status_code=400, + detail="Slack allowed user IDs must be comma-separated member IDs like U01ABC2DEF3.", + ) + + def _spawn_gateway_restart(profile: Optional[str] = None) -> Tuple[subprocess.Popen, bool]: """Spawn ``hermes gateway restart``, reusing an in-flight restart. @@ -4155,9 +4192,9 @@ _PLATFORM_OVERRIDES: dict[str, dict[str, Any]] = { }, "slack": { "name": "Slack", - "description": "Use Hermes from Slack via Socket Mode.", + "description": "Use Hermes from Slack via Socket Mode. Add allowed Slack member IDs so connected bots can respond.", "docs_url": "https://api.slack.com/apps", - "env_vars": ("SLACK_BOT_TOKEN", "SLACK_APP_TOKEN"), + "env_vars": ("SLACK_BOT_TOKEN", "SLACK_APP_TOKEN", "SLACK_ALLOWED_USERS"), "required_env": ("SLACK_BOT_TOKEN", "SLACK_APP_TOKEN"), }, "mattermost": { @@ -4642,6 +4679,7 @@ def _messaging_env_info(key: str) -> dict[str, Any]: return { "description": info.get("description", ""), "prompt": info.get("prompt", key), + "help": info.get("help", ""), "url": info.get("url"), "is_password": info.get("password", False), "advanced": info.get("advanced", False), @@ -5221,6 +5259,7 @@ async def update_messaging_platform( ) trimmed = value.strip() if trimmed: + _validate_messaging_env_value(platform_id, key, trimmed) save_env_value(key, trimmed) if body.enabled is not None: diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index e65a28101cd..7416ec0b87a 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -1552,6 +1552,27 @@ class TestWebServerEndpoints: assert telegram["enabled"] is False assert any(field["key"] == "TELEGRAM_BOT_TOKEN" and field["required"] for field in telegram["env_vars"]) + def test_slack_messaging_platform_exposes_user_allowlist(self): + resp = self.client.get("/api/messaging/platforms") + + assert resp.status_code == 200 + platforms = resp.json()["platforms"] + slack = next(platform for platform in platforms if platform["id"] == "slack") + fields = {field["key"]: field for field in slack["env_vars"]} + + assert "allowed Slack member IDs" in slack["description"] + assert set(fields) >= { + "SLACK_BOT_TOKEN", + "SLACK_APP_TOKEN", + "SLACK_ALLOWED_USERS", + } + assert fields["SLACK_ALLOWED_USERS"]["prompt"] == "Allowed Slack member IDs" + assert fields["SLACK_ALLOWED_USERS"]["is_password"] is False + assert "member IDs" in fields["SLACK_ALLOWED_USERS"]["description"] + assert "Bot User OAuth Token" in fields["SLACK_BOT_TOKEN"]["help"] + assert "App-Level Tokens" in fields["SLACK_APP_TOKEN"]["help"] + assert "Copy member ID" in fields["SLACK_ALLOWED_USERS"]["help"] + def test_weixin_messaging_metadata_describes_personal_ilink_setup(self): resp = self.client.get("/api/messaging/platforms") @@ -1628,6 +1649,70 @@ class TestWebServerEndpoints: telegram = next(platform for platform in status if platform["id"] == "telegram") assert telegram["enabled"] is False + def test_update_messaging_platform_saves_slack_allowed_users(self): + from hermes_cli.config import load_env + + resp = self.client.put( + "/api/messaging/platforms/slack", + json={"env": {"SLACK_ALLOWED_USERS": "U01ABC2DEF3,U04XYZ5LMN6"}}, + ) + + assert resp.status_code == 200 + assert load_env()["SLACK_ALLOWED_USERS"] == "U01ABC2DEF3,U04XYZ5LMN6" + + def test_update_messaging_platform_rejects_swapped_slack_bot_token(self): + resp = self.client.put( + "/api/messaging/platforms/slack", + json={"env": {"SLACK_BOT_TOKEN": "xapp-wrong-token-type"}}, + ) + + assert resp.status_code == 400 + assert "xoxb-" in resp.json()["detail"] + + def test_update_messaging_platform_rejects_swapped_slack_app_token(self): + resp = self.client.put( + "/api/messaging/platforms/slack", + json={"env": {"SLACK_APP_TOKEN": "xoxb-wrong-token-type"}}, + ) + + assert resp.status_code == 400 + assert "xapp-" in resp.json()["detail"] + + def test_update_messaging_platform_rejects_invalid_slack_allowed_users(self): + resp = self.client.put( + "/api/messaging/platforms/slack", + json={"env": {"SLACK_ALLOWED_USERS": "U01ABC2DEF3,not-a-user"}}, + ) + + assert resp.status_code == 400 + assert "member IDs" in resp.json()["detail"] + + def test_update_messaging_platform_accepts_slack_allowed_users_wildcard(self): + # "*" is the gateway's allow-all wildcard (gateway/platforms/slack.py), + # so the dashboard must accept it rather than rejecting it as malformed. + from hermes_cli.config import load_env + + resp = self.client.put( + "/api/messaging/platforms/slack", + json={"env": {"SLACK_ALLOWED_USERS": "*"}}, + ) + + assert resp.status_code == 200 + assert load_env()["SLACK_ALLOWED_USERS"] == "*" + + def test_update_messaging_platform_accepts_slack_allowed_users_trailing_comma(self): + # The gateway drops empty entries (gateway/platforms/slack.py), so a + # trailing/interior comma must not be rejected by the dashboard. + from hermes_cli.config import load_env + + resp = self.client.put( + "/api/messaging/platforms/slack", + json={"env": {"SLACK_ALLOWED_USERS": "U01ABC2DEF3,,W04XYZ5LMN6,"}}, + ) + + assert resp.status_code == 200 + assert load_env()["SLACK_ALLOWED_USERS"] == "U01ABC2DEF3,,W04XYZ5LMN6," + def test_messaging_platform_test_reports_missing_required_setup(self): resp = self.client.put("/api/messaging/platforms/discord", json={"enabled": True}) assert resp.status_code == 200 diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index ec03997b6c6..3955d3324c9 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -1346,6 +1346,7 @@ export interface MessagingPlatformEnvVar { redacted_value: string | null; description: string; prompt: string; + help: string; url: string | null; is_password: boolean; advanced: boolean; diff --git a/web/src/pages/ChannelsPage.tsx b/web/src/pages/ChannelsPage.tsx index d42ab7b9e74..7658c0cd61a 100644 --- a/web/src/pages/ChannelsPage.tsx +++ b/web/src/pages/ChannelsPage.tsx @@ -4,6 +4,7 @@ import { Check, CheckCircle2, ExternalLink, + Info, PlugZap, QrCode, Radio, @@ -55,6 +56,37 @@ function stateBadge(state: string) { } const TELEGRAM_USER_ID_RE = /^\d+$/; +const SLACK_MEMBER_ID_RE = /^[UW][A-Z0-9]{2,}$/; +const SLACK_TOKEN_PREFIXES: Record = { + SLACK_BOT_TOKEN: "xoxb-", + SLACK_APP_TOKEN: "xapp-", +}; + +function validateMessagingEnvField(field: MessagingPlatformEnvVar, value: string): string | null { + const trimmed = value.trim(); + if (!trimmed) return null; + + const expectedPrefix = SLACK_TOKEN_PREFIXES[field.key]; + if (expectedPrefix && !trimmed.startsWith(expectedPrefix)) { + return `${field.prompt || field.key} must start with ${expectedPrefix}`; + } + + if (field.key === "SLACK_ALLOWED_USERS") { + // Mirror the gateway's parse (gateway/platforms/slack.py): drop empty + // entries so a trailing/interior comma isn't rejected here. "*" is the + // allow-all wildcard the gateway honors. + const parts = trimmed + .split(",") + .map((part) => part.trim()) + .filter(Boolean); + const invalid = parts.find((part) => part !== "*" && !SLACK_MEMBER_ID_RE.test(part)); + if (invalid) { + return `${invalid} does not look like a Slack member ID. Use IDs like U01ABC2DEF3.`; + } + } + + return null; +} function formatExpiry(expiresAt: string): string { const ms = Date.parse(expiresAt) - Date.now(); @@ -83,8 +115,12 @@ export default function ChannelsPage() { // Config modal state const [editing, setEditing] = useState(null); const [draftEnv, setDraftEnv] = useState>({}); + const [fieldErrors, setFieldErrors] = useState>({}); const [saving, setSaving] = useState(false); - const closeEdit = useCallback(() => setEditing(null), []); + const closeEdit = useCallback(() => { + setEditing(null); + setFieldErrors({}); + }, []); const editModalRef = useModalBehavior({ open: editing !== null, onClose: closeEdit }); // Per-card busy + restart-needed tracking @@ -116,6 +152,7 @@ export default function ChannelsPage() { initial[v.key] = ""; }); setDraftEnv(initial); + setFieldErrors({}); setEditing(platform); }; @@ -138,6 +175,16 @@ export default function ChannelsPage() { showToast(`${missing[0].prompt || missing[0].key} is required`, "error"); return; } + const nextFieldErrors: Record = {}; + editing.env_vars.forEach((field) => { + const message = validateMessagingEnvField(field, draftEnv[field.key] || ""); + if (message) nextFieldErrors[field.key] = message; + }); + if (Object.keys(nextFieldErrors).length > 0) { + setFieldErrors(nextFieldErrors); + showToast("Fix the highlighted fields before saving.", "error"); + return; + } setSaving(true); try { const body: MessagingPlatformUpdate = { env, enabled: true }; @@ -326,10 +373,22 @@ export default function ChannelsPage() {

{editing.env_vars.map((field: MessagingPlatformEnvVar) => (
- +
+ + {field.help && ( + + + + )} +
{field.description && ( {field.description} @@ -344,10 +403,23 @@ export default function ChannelsPage() { : field.key } value={draftEnv[field.key] ?? ""} - onChange={(e) => - setDraftEnv((prev) => ({ ...prev, [field.key]: e.target.value })) - } + aria-invalid={Boolean(fieldErrors[field.key])} + onChange={(e) => { + const nextValue = e.target.value; + setDraftEnv((prev) => ({ ...prev, [field.key]: nextValue })); + setFieldErrors((prev) => { + if (!prev[field.key]) return prev; + const next = { ...prev }; + delete next[field.key]; + return next; + }); + }} /> + {fieldErrors[field.key] && ( + + {fieldErrors[field.key]} + + )}
))}