From d9190491a687d7f29fee5e09c2418d66025e9660 Mon Sep 17 00:00:00 2001 From: Shannon Sands Date: Fri, 19 Jun 2026 14:37:16 +1000 Subject: [PATCH] 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]} + + )}
))}