From 1aa616f71acd39329332bd1a39c662dbec37f864 Mon Sep 17 00:00:00 2001 From: Soheil Fakour Date: Sat, 1 Aug 2026 19:57:22 -0400 Subject: [PATCH 1/3] fix(config): decode JSON array literals into YAML lists on config set hermes config set '["-5488240624"]' previously wrote the literal as a quoted scalar string. Readers that split scalars on commas (_coerce_allow_set, the telegram env bridge) then matched nothing, silently breaking authorization allowlists like gateway.platforms.telegram.group_allowed_chats. When the schema default is a list and the passed value parses as a JSON array, decode it into a real YAML sequence. Scalars, comma strings, and JSON objects keep their historical behavior. Closes #76457 --- hermes_cli/config.py | 102 +++++++++++++++++- tests/hermes_cli/test_set_config_value.py | 121 ++++++++++++++++++++++ 2 files changed, 222 insertions(+), 1 deletion(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index ff4629dacad78..bc501921a0e65 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4646,6 +4646,78 @@ def _default_value_for_key(dotted_key: str): return node if not isinstance(node, dict) else None +def _is_mapping_section_key(dotted_key: str) -> bool: + """True when *dotted_key* names a mapping section in DEFAULT_CONFIG. + + ``_default_value_for_key`` returns None for dict nodes (the leaf check + requires a non-dict), so top-level mapping sections like ``terminal`` or + ``agent`` are invisible to it. A key is mapping-typed when its first + segment's default is a dict (e.g. ``terminal``, ``agent.compression``) or + the existing config holds a dict at that path. + """ + segments = dotted_key.split(".") + node = DEFAULT_CONFIG + for part in segments: + if not isinstance(node, dict) or part not in node: + return False + node = node[part] + return isinstance(node, dict) + + +def _decode_json_value(text: str): + """Decode a JSON literal (array or object) for ``config set``, or None. + + ``hermes config set '[...]'`` / ``'{...}'`` has no native CLI syntax + for structured values, so users pass JSON. Returns the parsed value when + *text* is a JSON array or object; None for scalars (plain strings, ``123``, + ``true``) which keep their historical stringification. + """ + try: + parsed = json.loads(text) + except (ValueError, TypeError): + return None + return parsed if isinstance(parsed, (list, dict)) else None + + +def _looks_like_allowlist_field(field: str) -> bool: + """True when *field* is an allowlist-shaped platform key. + + Allowlist fields are consumed as sets by ``_coerce_allow_set`` (and the + telegram env bridge) and are the list-typed platform settings users set + with a JSON array. Matching is by shape (``*_allowed_*`` / ``*_allow_from``) + plus the known allowlist names, so it stays in sync with the readers even + as platform configs gain fields. + """ + return ( + "_allow_from" in field + or "_allowed_" in field + or field in ("allow_from", "allowed_chats", "allowed_topics", "allowed_rooms", "allowed_channels") + ) + + +def _is_allowlist_platform_field(dotted_key: str) -> bool: + """True when *dotted_key* names an allowlist field of a platform config. + + Covers both documented forms: + - nested: ``gateway.platforms..`` / ``platforms..`` + (open-dict platform configs; ``gateway.platforms.telegram`` alone is the + platform mapping, not a field) + - top-level: ``telegram.``, ``discord.``, etc. (bridged into + platform extras by ``gateway/config.py``) + + Only allowlist-shaped fields decode; other platform fields (``reactions``, + ``streaming``) keep their historical stringification. + """ + segments = dotted_key.split(".") + if any(seg in _PLATFORM_CONTAINER_KEYS and idx + 2 < len(segments) for idx, seg in enumerate(segments)): + # ``...platforms..`` — at least one field below name. + return _looks_like_allowlist_field(segments[-1]) + if segments[0] in _SCHEMA_DEFINED_DICT_KEYS and len(segments) > 1: + # ``telegram.`` — top-level platform block field. + return _looks_like_allowlist_field(segments[1]) + return False + + # Known top-level config keys that intentionally accept arbitrary user-supplied # child keys ("dictionary-shaped" config: the schema declares the dict but the # user populates its keys). Schema validation accepts ANY path below these @@ -4901,7 +4973,9 @@ def set_config_value(key: str, value: str, force: bool = False): # such as approvals.mode="off" must not become YAML booleans. Unknown keys # retain the historical best-effort coercion behavior. coerced_value: Any = value - if not isinstance(_default_value_for_key(key), str): + default_value = _default_value_for_key(key) + existing_value = _get_nested(user_config, key) + if not isinstance(default_value, str): if value.lower() in {'true', 'yes', 'on'}: coerced_value = True elif value.lower() in {'false', 'no', 'off'}: @@ -4910,6 +4984,32 @@ def set_config_value(key: str, value: str, force: bool = False): coerced_value = int(value) elif value.replace('.', '', 1).isdigit(): coerced_value = float(value) + # List-typed settings (allowlists like + # ``gateway.platforms.telegram.group_allowed_chats``, ``allow_from``, + # ``allowed_topics``) have no native CLI syntax, so users pass a JSON + # array literal. Decode it into a real YAML sequence instead of + # writing the stringified literal — the readers (_coerce_allow_set and + # the telegram env bridge) split scalars on commas, so a quoted + # '["-5488240624"]' silently matches nothing (#76457). + # + # A key is list-typed when its schema default is a list, the existing + # user config holds a list at that path, or the key names an allowlist + # platform field (``gateway.platforms..group_allowed_chats`` / + # ``telegram.group_allowed_chats`` — open-dict extras with no + # DEFAULT_CONFIG entry). Unknown keys never match and keep their + # historical stringification. + elif isinstance(default_value, list) or isinstance(existing_value, list) or _is_allowlist_platform_field(key): + # List-typed keys accept only JSON arrays — a JSON object is not a + # valid list value. + decoded = _decode_json_value(value) + if isinstance(decoded, list): + coerced_value = decoded + elif _decode_json_value(value) is not None and (_is_mapping_section_key(key) or isinstance(existing_value, dict)): + # JSON object literal → YAML mapping, per the spec's ``{`` trigger. + # Only for mapping-typed keys (schema default is a mapping section, + # or the existing config holds a dict at that path). Unknown keys + # and non-mapping defaults keep their historical stringification. + coerced_value = _decode_json_value(value) value = coerced_value # Normalize a scalar ``model`` key before writing sub-keys so that diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index 9489f2ca70339..b3f39e6430ef2 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -723,3 +723,124 @@ def test_unset_config_value_refuses_broken_yaml(self, _isolated_hermes_home, cap assert "Cannot parse" in captured.out or "Cannot parse" in captured.err raw = _read_config(_isolated_hermes_home) assert raw == self.BROKEN_CONFIG + + +class TestJsonListValues: + """hermes config set must decode JSON array literals into real YAML lists. + + Allowlist keys (``group_allowed_chats``, ``allow_from``, ``allowed_topics``) + have no native CLI list syntax, so users pass a JSON array. Before #76457 + the literal was written as a quoted scalar string; readers that split + scalars on commas then matched nothing, silently breaking authorization + allowlists. Decode only when the key is list-typed (schema default is a + list, the existing config holds a list, or the key names an allowlist + platform field); all other values keep their historical stringification. + """ + + def test_json_array_written_as_yaml_list(self, _isolated_hermes_home): + set_config_value("gateway.platforms.telegram.group_allowed_chats", '["-5488240624"]') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + value = reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"] + assert isinstance(value, list) + assert value == ["-5488240624"] + + def test_plain_scalar_stays_a_string(self, _isolated_hermes_home): + set_config_value("gateway.platforms.telegram.group_allowed_chats", "-5488240624") + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"] == "-5488240624" + + def test_comma_string_stays_a_string(self, _isolated_hermes_home): + set_config_value("gateway.platforms.telegram.group_allowed_chats", "123,456") + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"] == "123,456" + + def test_json_object_is_not_decoded(self, _isolated_hermes_home): + # A JSON object on a LIST-typed key is not a valid list value — it + # stays a string (the section-write guard governs mapping keys). + set_config_value("gateway.platforms.telegram.group_allowed_chats", '{"not": "an array"}') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"] == '{"not": "an array"}' + + def test_json_object_on_mapping_key_written_as_mapping(self, _isolated_hermes_home): + # The spec's ``{`` trigger: a JSON object literal on a mapping-typed + # key becomes a YAML mapping, not a quoted scalar. + set_config_value("terminal", '{"backend": "local", "cwd": "/repo"}') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["terminal"] == {"backend": "local", "cwd": "/repo"} + + def test_string_default_key_keeps_json_array_as_string(self, _isolated_hermes_home): + # String-typed defaults must not be decoded — the user asked for the + # literal text (e.g. a JSON document stored as a string). + set_config_value("telegram.allowed_chats", '["a", "b"]') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["telegram"]["allowed_chats"] == '["a", "b"]' + + def test_unknown_key_with_json_array_stays_a_string(self, _isolated_hermes_home): + # Unknown keys (schema default None) are intentionally supported as + # arbitrary string configuration for skills/external apps. A JSON + # array must stay a string, not become a YAML sequence. + set_config_value("custom.payload", '["a", "b"]') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["custom"]["payload"] == '["a", "b"]' + + def test_non_list_default_with_json_array_stays_a_string(self, _isolated_hermes_home): + # A known key whose default is NOT a list (bool/int/float) must not be + # decoded either — only list-typed allowlist keys are. + set_config_value("telegram.reactions", '["a", "b"]') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["telegram"]["reactions"] == '["a", "b"]' + + def test_top_level_platform_field_written_as_list(self, _isolated_hermes_home): + # The documented top-level form (telegram.group_allowed_chats, not + # nested under gateway.platforms) is bridged into platform extras by + # gateway/config.py and must decode JSON arrays too. + set_config_value("telegram.group_allowed_chats", '["-5488240624"]') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + value = reloaded["telegram"]["group_allowed_chats"] + assert isinstance(value, list) + assert value == ["-5488240624"] + + def test_platform_mapping_alone_is_not_decoded(self, _isolated_hermes_home): + # ``gateway.platforms.telegram`` is the platform mapping itself (no + # field below it) — a JSON array there is not an allowlist write. + set_config_value("gateway.platforms.telegram", '["a", "b"]') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["gateway"]["platforms"]["telegram"] == '["a", "b"]' + + def test_loaded_through_gateway_config_is_a_list(self, _isolated_hermes_home): + # End-to-end: the written YAML list must survive load_gateway_config + # as a real list (the sweeper's suggested regression). No fallback — + # a failure here must surface, not silently skip. + set_config_value("telegram.group_allowed_chats", '["-5488240624"]') + + import gateway.config as gc + + cfg = gc.load_gateway_config() + telegram_cfg = cfg.platforms.get(gc.Platform.TELEGRAM) + assert telegram_cfg is not None + # group_allowed_chats lands in PlatformConfig.extra (bridged from the + # top-level telegram block by gateway/config.py). + value = (telegram_cfg.extra or {}).get("group_allowed_chats") + assert isinstance(value, list), f"expected list, got {type(value)}: {value!r}" + assert value == ["-5488240624"] + From 47659a2b1a339300c483e538904d0944e54b351a Mon Sep 17 00:00:00 2001 From: Soheil Fakour Date: Mon, 3 Aug 2026 18:29:24 -0400 Subject: [PATCH 2/3] fix(config): restrict mapping-key JSON decode to dicts only GottZ triage (PR #76470) correctly identified that the mapping-key branch accepted any structured _decode_json_value result, so a JSON array on a mapping-only key (e.g. terminal) would become a YAML list instead of retaining historical string semantics. Now checks isinstance(decoded, dict) before applying the mapping decode, so JSON arrays on mapping-typed keys stay strings. Added regression test test_json_array_on_mapping_key_stays_a_string. --- hermes_cli/config.py | 11 +++++++---- tests/hermes_cli/test_set_config_value.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index bc501921a0e65..2d6fa0f5961b5 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -5004,12 +5004,15 @@ def set_config_value(key: str, value: str, force: bool = False): decoded = _decode_json_value(value) if isinstance(decoded, list): coerced_value = decoded - elif _decode_json_value(value) is not None and (_is_mapping_section_key(key) or isinstance(existing_value, dict)): + else: # JSON object literal → YAML mapping, per the spec's ``{`` trigger. # Only for mapping-typed keys (schema default is a mapping section, - # or the existing config holds a dict at that path). Unknown keys - # and non-mapping defaults keep their historical stringification. - coerced_value = _decode_json_value(value) + # or the existing config holds a dict at that path) AND only when + # the decoded value is actually a dict — a JSON array on a + # mapping-only key stays a string (GottZ triage, PR #76470). + _mapping_decoded = _decode_json_value(value) + if isinstance(_mapping_decoded, dict) and (_is_mapping_section_key(key) or isinstance(existing_value, dict)): + coerced_value = _mapping_decoded value = coerced_value # Normalize a scalar ``model`` key before writing sub-keys so that diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index b3f39e6430ef2..3f67fe14bcf8b 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -778,6 +778,17 @@ def test_json_object_on_mapping_key_written_as_mapping(self, _isolated_hermes_ho reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) assert reloaded["terminal"] == {"backend": "local", "cwd": "/repo"} + def test_json_array_on_mapping_key_stays_a_string(self, _isolated_hermes_home): + # A JSON array on a mapping-typed key must NOT be decoded — only JSON + # objects should become YAML mappings. Without this guard, the mapping + # branch would accept any structured _decode_json_value result and turn + # an array into a YAML sequence (GottZ triage, PR #76470). + set_config_value("terminal", '["a", "b"]') + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert reloaded["terminal"] == '["a", "b"]' + def test_string_default_key_keeps_json_array_as_string(self, _isolated_hermes_home): # String-typed defaults must not be decoded — the user asked for the # literal text (e.g. a JSON document stored as a string). From fa51127366453205307c45f8924623d8dea993d0 Mon Sep 17 00:00:00 2001 From: Soheil Fakour Date: Fri, 14 Aug 2026 10:26:44 -0400 Subject: [PATCH 3/3] chore: re-run CI (repo CI config changed after this branch was pushed)