From f741e70791c1c69b501fdb98da80bec3e4d130c0 Mon Sep 17 00:00:00 2001 From: Shannon Sands Date: Fri, 19 Jun 2026 14:27:42 +1000 Subject: [PATCH 1/4] Add Slack allowed users setup field --- hermes_cli/config.py | 7 +++++ hermes_cli/web_server.py | 22 ++++++++++++-- tests/hermes_cli/test_web_server.py | 47 +++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index f698c11d5ac..8c790e7e856 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -3439,6 +3439,13 @@ OPTIONAL_ENV_VARS = { "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", + "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..b1320875c53 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2325,6 +2325,23 @@ def _gateway_display_command(profile: Optional[str], verb: str) -> str: return " ".join(["hermes", *_gateway_subcommand(profile, verb)]) +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.", + ) + + def _spawn_gateway_restart(profile: Optional[str] = None) -> Tuple[subprocess.Popen, bool]: """Spawn ``hermes gateway restart``, reusing an in-flight restart. @@ -4155,9 +4172,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": { @@ -5221,6 +5238,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..3f6ed3e0435 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -1552,6 +1552,24 @@ 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"] + def test_weixin_messaging_metadata_describes_personal_ilink_setup(self): resp = self.client.get("/api/messaging/platforms") @@ -1628,6 +1646,35 @@ 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_messaging_platform_test_reports_missing_required_setup(self): resp = self.client.put("/api/messaging/platforms/discord", json={"enabled": True}) assert resp.status_code == 200 From d9190491a687d7f29fee5e09c2418d66025e9660 Mon Sep 17 00:00:00 2001 From: Shannon Sands Date: Fri, 19 Jun 2026 14:37:16 +1000 Subject: [PATCH 2/4] Add Slack setup hints and field validation --- hermes_cli/config.py | 3 + hermes_cli/web_server.py | 13 +++++ tests/hermes_cli/test_web_server.py | 12 ++++ web/src/lib/api.ts | 1 + web/src/pages/ChannelsPage.tsx | 85 ++++++++++++++++++++++++++--- 5 files changed, 106 insertions(+), 8 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 8c790e7e856..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,6 +3436,7 @@ 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", @@ -3442,6 +3444,7 @@ OPTIONAL_ENV_VARS = { "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", diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index b1320875c53..b890f68649e 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2340,6 +2340,18 @@ def _validate_messaging_env_value(platform_id: str, key: str, value: str) -> Non 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": + user_ids = [part.strip() for part in value.split(",")] + invalid = [ + user_id + for user_id in user_ids + if not user_id or not re.fullmatch(r"[UW][A-Z0-9]{2,}", 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]: @@ -4659,6 +4671,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), diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 3f6ed3e0435..d44c789b3e3 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -1569,6 +1569,9 @@ class TestWebServerEndpoints: 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") @@ -1675,6 +1678,15 @@ class TestWebServerEndpoints: 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_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..84791738a25 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,34 @@ 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") { + const parts = trimmed.split(",").map((part) => part.trim()); + if (parts.some((part) => !part)) { + return "Slack member IDs must be comma-separated without empty entries."; + } + const invalid = parts.find((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 +112,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 +149,7 @@ export default function ChannelsPage() { initial[v.key] = ""; }); setDraftEnv(initial); + setFieldErrors({}); setEditing(platform); }; @@ -138,6 +172,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 +370,22 @@ export default function ChannelsPage() {

{editing.env_vars.map((field: MessagingPlatformEnvVar) => (
- +
+ + {field.help && ( + + + + )} +
{field.description && ( {field.description} @@ -344,10 +400,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]} + + )}
))} From 83c034bd5bc855955a825ff4acd1ed11edab6c3d Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 19 Jun 2026 12:18:15 +0530 Subject: [PATCH 3/4] fix(dashboard): accept Slack allow-all wildcard in allowed-users validation The new SLACK_ALLOWED_USERS validation rejected '*', but the Slack gateway honors '*' as an allow-all wildcard (gateway/platforms/slack.py DM auth, slash-confirm, and approval-button paths). Accept '*' as a valid list entry in both the API validator and the dashboard form so a value the runtime honors is no longer blocked at setup. --- hermes_cli/web_server.py | 4 +++- tests/hermes_cli/test_web_server.py | 13 +++++++++++++ web/src/pages/ChannelsPage.tsx | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index b890f68649e..316bc154fa4 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2342,10 +2342,12 @@ def _validate_messaging_env_value(platform_id: str, key: str, value: str) -> Non ) if key == "SLACK_ALLOWED_USERS": user_ids = [part.strip() for part in value.split(",")] + # "*" is the gateway's allow-all wildcard (see gateway/platforms/slack.py), + # so accept it as a valid entry alongside Slack member IDs (U.../W...). invalid = [ user_id for user_id in user_ids - if not user_id or not re.fullmatch(r"[UW][A-Z0-9]{2,}", user_id) + if user_id != "*" and (not user_id or not re.fullmatch(r"[UW][A-Z0-9]{2,}", user_id)) ] if invalid: raise HTTPException( diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index d44c789b3e3..d7a4dbcbbf9 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -1687,6 +1687,19 @@ class TestWebServerEndpoints: 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_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/pages/ChannelsPage.tsx b/web/src/pages/ChannelsPage.tsx index 84791738a25..db56beb1925 100644 --- a/web/src/pages/ChannelsPage.tsx +++ b/web/src/pages/ChannelsPage.tsx @@ -76,7 +76,7 @@ function validateMessagingEnvField(field: MessagingPlatformEnvVar, value: string if (parts.some((part) => !part)) { return "Slack member IDs must be comma-separated without empty entries."; } - const invalid = parts.find((part) => !SLACK_MEMBER_ID_RE.test(part)); + 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.`; } From 1ab6f34791e28559911185b308d8bd1b0be5f393 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 19 Jun 2026 12:22:30 +0530 Subject: [PATCH 4/4] refactor(dashboard): align Slack allowlist validation with gateway parse - Drop empty entries before validating SLACK_ALLOWED_USERS so a trailing or interior comma (which the gateway silently tolerates in gateway/platforms/slack.py) is no longer rejected at the dashboard. - Hoist the member-ID regex to a module-level _SLACK_MEMBER_ID_RE constant and note it stays in sync with the frontend SLACK_MEMBER_ID_RE. - Add a regression test for the trailing-comma case. --- hermes_cli/web_server.py | 14 ++++++++++---- tests/hermes_cli/test_web_server.py | 13 +++++++++++++ web/src/pages/ChannelsPage.tsx | 11 +++++++---- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 316bc154fa4..b0d51e2481e 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2325,6 +2325,11 @@ 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: @@ -2341,13 +2346,14 @@ def _validate_messaging_env_value(platform_id: str, key: str, value: str) -> Non detail="Slack App Token must start with xapp-. Paste the app-level token from Basic Information > App-Level Tokens.", ) if key == "SLACK_ALLOWED_USERS": - user_ids = [part.strip() for part in value.split(",")] - # "*" is the gateway's allow-all wildcard (see gateway/platforms/slack.py), - # so accept it as a valid entry alongside Slack member IDs (U.../W...). + # 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 user_id or not re.fullmatch(r"[UW][A-Z0-9]{2,}", user_id)) + if user_id != "*" and not _SLACK_MEMBER_ID_RE.fullmatch(user_id) ] if invalid: raise HTTPException( diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index d7a4dbcbbf9..7416ec0b87a 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -1700,6 +1700,19 @@ class TestWebServerEndpoints: 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/pages/ChannelsPage.tsx b/web/src/pages/ChannelsPage.tsx index db56beb1925..7658c0cd61a 100644 --- a/web/src/pages/ChannelsPage.tsx +++ b/web/src/pages/ChannelsPage.tsx @@ -72,10 +72,13 @@ function validateMessagingEnvField(field: MessagingPlatformEnvVar, value: string } if (field.key === "SLACK_ALLOWED_USERS") { - const parts = trimmed.split(",").map((part) => part.trim()); - if (parts.some((part) => !part)) { - return "Slack member IDs must be comma-separated without empty entries."; - } + // 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.`;