mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix: sanitize Telegram command names to strip invalid characters
Telegram Bot API requires command names to contain only lowercase a-z, digits 0-9, and underscores. Skill/plugin names containing characters like +, /, @, or . caused set_my_commands to fail with Bot_command_invalid. Two-layer fix: - scan_skill_commands(): strip non-alphanumeric/non-hyphen chars from cmd_key at source, collapse consecutive hyphens, trim edges, skip names that sanitize to empty string - _sanitize_telegram_name(): centralized helper used by all 3 Telegram name generation sites (core commands, plugin commands, skill commands) with empty-name guard at each call site Closes #5534
This commit is contained in:
parent
f77be22c65
commit
214e60c951
4 changed files with 204 additions and 4 deletions
|
|
@ -16,6 +16,9 @@ logger = logging.getLogger(__name__)
|
|||
|
||||
_skill_commands: Dict[str, Dict[str, Any]] = {}
|
||||
_PLAN_SLUG_RE = re.compile(r"[^a-z0-9]+")
|
||||
# Patterns for sanitizing skill names into clean hyphen-separated slugs.
|
||||
_SKILL_INVALID_CHARS = re.compile(r"[^a-z0-9-]")
|
||||
_SKILL_MULTI_HYPHEN = re.compile(r"-{2,}")
|
||||
|
||||
|
||||
def build_plan_path(
|
||||
|
|
@ -196,7 +199,14 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]:
|
|||
description = line[:80]
|
||||
break
|
||||
seen_names.add(name)
|
||||
# Normalize to hyphen-separated slug, stripping
|
||||
# non-alnum chars (e.g. +, /) to avoid invalid
|
||||
# Telegram command names downstream.
|
||||
cmd_name = name.lower().replace(' ', '-').replace('_', '-')
|
||||
cmd_name = _SKILL_INVALID_CHARS.sub('', cmd_name)
|
||||
cmd_name = _SKILL_MULTI_HYPHEN.sub('-', cmd_name).strip('-')
|
||||
if not cmd_name:
|
||||
continue
|
||||
_skill_commands[f"/{cmd_name}"] = {
|
||||
"name": name,
|
||||
"description": description or f"Invoke the {name} skill",
|
||||
|
|
|
|||
|
|
@ -366,13 +366,33 @@ def telegram_bot_commands() -> list[tuple[str, str]]:
|
|||
for cmd in COMMAND_REGISTRY:
|
||||
if not _is_gateway_available(cmd, overrides):
|
||||
continue
|
||||
tg_name = cmd.name.replace("-", "_")
|
||||
result.append((tg_name, cmd.description))
|
||||
tg_name = _sanitize_telegram_name(cmd.name)
|
||||
if tg_name:
|
||||
result.append((tg_name, cmd.description))
|
||||
return result
|
||||
|
||||
|
||||
_TG_NAME_LIMIT = 32
|
||||
|
||||
# Telegram Bot API allows only lowercase a-z, 0-9, and underscores in
|
||||
# command names. This regex strips everything else after initial conversion.
|
||||
_TG_INVALID_CHARS = re.compile(r"[^a-z0-9_]")
|
||||
_TG_MULTI_UNDERSCORE = re.compile(r"_{2,}")
|
||||
|
||||
|
||||
def _sanitize_telegram_name(raw: str) -> str:
|
||||
"""Convert a command/skill/plugin name to a valid Telegram command name.
|
||||
|
||||
Telegram requires: 1-32 chars, lowercase a-z, digits 0-9, underscores only.
|
||||
Steps: lowercase → replace hyphens with underscores → strip all other
|
||||
invalid characters → collapse consecutive underscores → strip leading/
|
||||
trailing underscores.
|
||||
"""
|
||||
name = raw.lower().replace("-", "_")
|
||||
name = _TG_INVALID_CHARS.sub("", name)
|
||||
name = _TG_MULTI_UNDERSCORE.sub("_", name)
|
||||
return name.strip("_")
|
||||
|
||||
|
||||
def _clamp_telegram_names(
|
||||
entries: list[tuple[str, str]],
|
||||
|
|
@ -436,7 +456,9 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str
|
|||
pm = get_plugin_manager()
|
||||
plugin_cmds = getattr(pm, "_plugin_commands", {})
|
||||
for cmd_name in sorted(plugin_cmds):
|
||||
tg_name = cmd_name.replace("-", "_")
|
||||
tg_name = _sanitize_telegram_name(cmd_name)
|
||||
if not tg_name:
|
||||
continue
|
||||
desc = "Plugin command"
|
||||
if len(desc) > 40:
|
||||
desc = desc[:37] + "..."
|
||||
|
|
@ -479,7 +501,9 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str
|
|||
skill_name = info.get("name", "")
|
||||
if skill_name in _platform_disabled:
|
||||
continue
|
||||
name = cmd_key.lstrip("/").replace("-", "_")
|
||||
name = _sanitize_telegram_name(cmd_key.lstrip("/"))
|
||||
if not name:
|
||||
continue
|
||||
desc = info.get("description", "")
|
||||
# Keep descriptions short — setMyCommands has an undocumented
|
||||
# total payload limit. 40 chars fits 100 commands safely.
|
||||
|
|
|
|||
|
|
@ -102,6 +102,49 @@ class TestScanSkillCommands:
|
|||
assert "/disabled-skill" not in result
|
||||
|
||||
|
||||
def test_special_chars_stripped_from_cmd_key(self, tmp_path):
|
||||
"""Skill names with +, /, or other special chars produce clean cmd keys."""
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
# Simulate a skill named "Jellyfin + Jellystat 24h Summary"
|
||||
skill_dir = tmp_path / "jellyfin-plus"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\nname: Jellyfin + Jellystat 24h Summary\n"
|
||||
"description: Test skill\n---\n\nBody.\n"
|
||||
)
|
||||
result = scan_skill_commands()
|
||||
# The + should be stripped, not left as a literal character
|
||||
assert "/jellyfin-jellystat-24h-summary" in result
|
||||
# The old buggy key should NOT exist
|
||||
assert "/jellyfin-+-jellystat-24h-summary" not in result
|
||||
|
||||
def test_allspecial_name_skipped(self, tmp_path):
|
||||
"""Skill with name consisting only of special chars is silently skipped."""
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
skill_dir = tmp_path / "bad-name"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\nname: +++\ndescription: Bad skill\n---\n\nBody.\n"
|
||||
)
|
||||
result = scan_skill_commands()
|
||||
# Should not create a "/" key or any entry
|
||||
assert "/" not in result
|
||||
assert result == {}
|
||||
|
||||
def test_slash_in_name_stripped_from_cmd_key(self, tmp_path):
|
||||
"""Skill names with / chars produce clean cmd keys."""
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
skill_dir = tmp_path / "sonarr-api"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\nname: Sonarr v3/v4 API\n"
|
||||
"description: Test skill\n---\n\nBody.\n"
|
||||
)
|
||||
result = scan_skill_commands()
|
||||
assert "/sonarr-v3v4-api" in result
|
||||
assert any("/" in k[1:] for k in result) is False # no unescaped /
|
||||
|
||||
|
||||
class TestResolveSkillCommandKey:
|
||||
"""Telegram bot-command names disallow hyphens, so the menu registers
|
||||
skills with hyphens swapped for underscores. When Telegram autocomplete
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from hermes_cli.commands import (
|
|||
SlashCommandCompleter,
|
||||
_TG_NAME_LIMIT,
|
||||
_clamp_telegram_names,
|
||||
_sanitize_telegram_name,
|
||||
gateway_help_lines,
|
||||
resolve_command,
|
||||
slack_subcommand_map,
|
||||
|
|
@ -198,6 +199,13 @@ class TestTelegramBotCommands:
|
|||
for name, _ in telegram_bot_commands():
|
||||
assert "-" not in name, f"Telegram command '{name}' contains a hyphen"
|
||||
|
||||
def test_all_names_valid_telegram_chars(self):
|
||||
"""Telegram requires: lowercase a-z, 0-9, underscores only."""
|
||||
import re
|
||||
tg_valid = re.compile(r"^[a-z0-9_]+$")
|
||||
for name, _ in telegram_bot_commands():
|
||||
assert tg_valid.match(name), f"Invalid Telegram command name: {name!r}"
|
||||
|
||||
def test_excludes_cli_only_without_config_gate(self):
|
||||
names = {name for name, _ in telegram_bot_commands()}
|
||||
for cmd in COMMAND_REGISTRY:
|
||||
|
|
@ -509,6 +517,53 @@ class TestGhostText:
|
|||
assert _suggestion("hello") is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Telegram command name sanitization
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSanitizeTelegramName:
|
||||
"""Tests for _sanitize_telegram_name() — Telegram requires [a-z0-9_] only."""
|
||||
|
||||
def test_hyphens_replaced_with_underscores(self):
|
||||
assert _sanitize_telegram_name("my-skill-name") == "my_skill_name"
|
||||
|
||||
def test_plus_sign_stripped(self):
|
||||
"""Regression: skill name 'Jellyfin + Jellystat 24h Summary'."""
|
||||
assert _sanitize_telegram_name("jellyfin-+-jellystat-24h-summary") == "jellyfin_jellystat_24h_summary"
|
||||
|
||||
def test_slash_stripped(self):
|
||||
"""Regression: skill name 'Sonarr v3/v4 API Integration'."""
|
||||
assert _sanitize_telegram_name("sonarr-v3/v4-api-integration") == "sonarr_v3v4_api_integration"
|
||||
|
||||
def test_uppercase_lowercased(self):
|
||||
assert _sanitize_telegram_name("MyCommand") == "mycommand"
|
||||
|
||||
def test_dots_and_special_chars_stripped(self):
|
||||
assert _sanitize_telegram_name("skill.v2@beta!") == "skillv2beta"
|
||||
|
||||
def test_consecutive_underscores_collapsed(self):
|
||||
assert _sanitize_telegram_name("a---b") == "a_b"
|
||||
assert _sanitize_telegram_name("a-+-b") == "a_b"
|
||||
|
||||
def test_leading_trailing_underscores_stripped(self):
|
||||
assert _sanitize_telegram_name("-leading") == "leading"
|
||||
assert _sanitize_telegram_name("trailing-") == "trailing"
|
||||
assert _sanitize_telegram_name("-both-") == "both"
|
||||
|
||||
def test_digits_preserved(self):
|
||||
assert _sanitize_telegram_name("skill-24h") == "skill_24h"
|
||||
|
||||
def test_empty_after_sanitization(self):
|
||||
assert _sanitize_telegram_name("+++") == ""
|
||||
|
||||
def test_spaces_only_becomes_empty(self):
|
||||
assert _sanitize_telegram_name(" ") == ""
|
||||
|
||||
def test_already_valid(self):
|
||||
assert _sanitize_telegram_name("valid_name_123") == "valid_name_123"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Telegram command name clamping (32-char limit)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -628,3 +683,71 @@ class TestTelegramMenuCommands:
|
|||
menu_names = {n for n, _ in menu}
|
||||
assert "my_enabled_skill" in menu_names
|
||||
assert "my_disabled_skill" not in menu_names
|
||||
|
||||
def test_special_chars_in_skill_names_sanitized(self, tmp_path, monkeypatch):
|
||||
"""Skills with +, /, or other special chars produce valid Telegram names."""
|
||||
from unittest.mock import patch
|
||||
import re
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
fake_skills_dir = str(tmp_path / "skills")
|
||||
fake_cmds = {
|
||||
"/jellyfin-+-jellystat-24h-summary": {
|
||||
"name": "Jellyfin + Jellystat 24h Summary",
|
||||
"description": "Test",
|
||||
"skill_md_path": f"{fake_skills_dir}/jellyfin/SKILL.md",
|
||||
"skill_dir": f"{fake_skills_dir}/jellyfin",
|
||||
},
|
||||
"/sonarr-v3/v4-api": {
|
||||
"name": "Sonarr v3/v4 API",
|
||||
"description": "Test",
|
||||
"skill_md_path": f"{fake_skills_dir}/sonarr/SKILL.md",
|
||||
"skill_dir": f"{fake_skills_dir}/sonarr",
|
||||
},
|
||||
}
|
||||
with (
|
||||
patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds),
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"),
|
||||
):
|
||||
(tmp_path / "skills").mkdir(exist_ok=True)
|
||||
menu, _ = telegram_menu_commands(max_commands=100)
|
||||
|
||||
# Every name must match Telegram's [a-z0-9_] requirement
|
||||
tg_valid = re.compile(r"^[a-z0-9_]+$")
|
||||
for name, _ in menu:
|
||||
assert tg_valid.match(name), f"Invalid Telegram command name: {name!r}"
|
||||
|
||||
def test_empty_sanitized_names_excluded(self, tmp_path, monkeypatch):
|
||||
"""Skills whose names sanitize to empty string are silently dropped."""
|
||||
from unittest.mock import patch
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
fake_skills_dir = str(tmp_path / "skills")
|
||||
fake_cmds = {
|
||||
"/+++": {
|
||||
"name": "+++",
|
||||
"description": "All special chars",
|
||||
"skill_md_path": f"{fake_skills_dir}/bad/SKILL.md",
|
||||
"skill_dir": f"{fake_skills_dir}/bad",
|
||||
},
|
||||
"/valid-skill": {
|
||||
"name": "valid-skill",
|
||||
"description": "Normal skill",
|
||||
"skill_md_path": f"{fake_skills_dir}/valid/SKILL.md",
|
||||
"skill_dir": f"{fake_skills_dir}/valid",
|
||||
},
|
||||
}
|
||||
with (
|
||||
patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds),
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"),
|
||||
):
|
||||
(tmp_path / "skills").mkdir(exist_ok=True)
|
||||
menu, _ = telegram_menu_commands(max_commands=100)
|
||||
|
||||
menu_names = {n for n, _ in menu}
|
||||
# The valid skill should be present, the empty one should not
|
||||
assert "valid_skill" in menu_names
|
||||
# No empty string in menu names
|
||||
assert "" not in menu_names
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue