From 9bda6438d41eb3cd054d39ce2d4b8b6d31c303d7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 20 Jul 2026 02:25:33 -0700 Subject: [PATCH] =?UTF-8?q?fix(config):=20remove=20unknown-top-level-key?= =?UTF-8?q?=20warning=20=E2=80=94=20top-level=20keys=20bridge=20to=20env?= =?UTF-8?q?=20(#67924)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'Unknown top-level config key' warning (f5bacee27) assumed a closed-world allowlist of valid roots, but top-level scalars in config.yaml are deliberately bridged into os.environ (gateway/run.py, hermes send) so skills and external apps Hermes drives can read arbitrary env-style keys (DISCORD_HOME_CHANNEL, MY_APP_TOKEN, ...). An allowlist can never enumerate those — two widening follow-ups (7c2ece53c, 3c7217706) already proved the whack-a-mole. Drop the generic warning entirely; keep the targeted provider-like-field misplacement hint (base_url/api_key at root). --- hermes_cli/config.py | 21 ++---- tests/hermes_cli/test_config_validation.py | 77 ++++++---------------- 2 files changed, 26 insertions(+), 72 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index b946be16b86c..cadf8decb88d 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -5699,28 +5699,21 @@ def validate_config_structure(config: Optional[Dict[str, Any]] = None) -> List[" " base_url: https://...", )) - # ── Unknown / misplaced root-level keys ────────────────────────────── - # Typos like skillz:/secrity: were previously silent (only provider-like - # fields were flagged). Warn on any unknown top-level key so config - # hygiene surfaces without breaking startup. + # ── Root-level keys that look misplaced ────────────────────────────── + # Only provider-like fields (base_url, api_key, …) are flagged. Arbitrary + # unknown top-level keys are deliberately NOT warned about: top-level + # scalars are bridged into os.environ (gateway/run.py, hermes send) so + # users can feed skills and external apps env-style keys from config.yaml + # — a closed-world allowlist can never enumerate those. for key in config: if key.startswith("_"): continue - if key in _KNOWN_ROOT_KEYS: - continue - if key in _CUSTOM_PROVIDER_LIKE_FIELDS: + if key not in _KNOWN_ROOT_KEYS and key in _CUSTOM_PROVIDER_LIKE_FIELDS: issues.append(ConfigIssue( "warning", f"Root-level key '{key}' looks misplaced — should it be under 'model:' or inside a 'custom_providers' entry?", f"Move '{key}' under the appropriate section", )) - else: - issues.append(ConfigIssue( - "warning", - f"Unknown top-level config key '{key}' — it will be ignored", - "Check for typos, or remove the key if it is not a supported config root. " - "Run 'hermes doctor' for more detail.", - )) return issues diff --git a/tests/hermes_cli/test_config_validation.py b/tests/hermes_cli/test_config_validation.py index 44df33b25a0b..4d0d3df6ed77 100644 --- a/tests/hermes_cli/test_config_validation.py +++ b/tests/hermes_cli/test_config_validation.py @@ -214,38 +214,27 @@ class TestConfigIssueDataclass: class TestUnknownTopLevelKeys: - """Unknown top-level keys should warn (not error); known roots stay silent.""" + """Arbitrary top-level keys must NOT warn — they are bridged to os.environ. - def test_typo_top_level_key_warns_with_key_name(self): - """A typo like skillz: must surface as a warning naming the key.""" + Top-level scalars in config.yaml are forwarded into the environment + (gateway/run.py, hermes send) so users can feed skills and external apps + env-style keys like DISCORD_HOME_CHANNEL or MY_APP_TOKEN. A closed-world + allowlist can never enumerate those, so no "Unknown top-level config key" + warning may exist. + """ + + def test_arbitrary_top_level_keys_stay_silent(self): + """Env-style and custom keys must produce no unknown-key warnings.""" issues = validate_config_structure({ "model": {"provider": "openrouter"}, + "DISCORD_HOME_CHANNEL": "12345", + "TELEGRAM_HOME_CHANNEL": "-100987", + "DISCORD_ALLOW_ALL_USERS": True, + "MY_CUSTOM_SKILL_VAR": "hello", "skillz": {"enabled": True}, - "secrity": {"redact": True}, }) - warnings = [i for i in issues if i.severity == "warning"] - unknown = [i for i in warnings if "Unknown top-level config key" in i.message] - messages = " ".join(i.message for i in unknown) - assert "skillz" in messages - assert "secrity" in messages - assert all(i.severity == "warning" for i in unknown) - assert not any(i.severity == "error" for i in unknown) - - def test_all_default_config_roots_accepted_without_unknown_warning(self): - """Every DEFAULT_CONFIG root (and legacy extras) must not warn as unknown.""" - config = {key: {} if isinstance(DEFAULT_CONFIG.get(key), dict) else DEFAULT_CONFIG.get(key) - for key in DEFAULT_CONFIG} - for key in _EXTRA_KNOWN_ROOT_KEYS: - if key == "custom_providers": - config[key] = [{"name": "x", "base_url": "https://example.com"}] - config.setdefault("model", {"provider": "custom", "default": "m"}) - elif key == "fallback_model": - config[key] = {"provider": "openrouter", "model": "test"} - else: - config[key] = {} - issues = validate_config_structure(config) - unknown = [i for i in issues if "Unknown top-level config key" in i.message] - assert unknown == [], f"Unexpected unknown-key warnings: {[i.message for i in unknown]}" + assert not any("Unknown top-level config key" in i.message for i in issues) + assert issues == [] def test_known_root_keys_derived_from_default_config(self): """_KNOWN_ROOT_KEYS must be DEFAULT_CONFIG.keys() plus extras — single source of truth.""" @@ -253,30 +242,8 @@ class TestUnknownTopLevelKeys: assert _EXTRA_KNOWN_ROOT_KEYS.issubset(_KNOWN_ROOT_KEYS) assert _KNOWN_ROOT_KEYS == frozenset(DEFAULT_CONFIG.keys()) | _EXTRA_KNOWN_ROOT_KEYS - def test_hermes_written_roots_not_flagged_as_unknown(self): - """Roots Hermes itself writes/reads must not warn as unknown (#67397).""" - issues = validate_config_structure({ - "model": {"provider": "openrouter"}, - "known_plugin_toolsets": {"cli": ["spotify"]}, - "group_sessions_per_user": True, - "thread_sessions_per_user": False, - "stt_echo_transcripts": True, - "reset_triggers": ["/new"], - "always_log_local": True, - "filter_silence_narration": True, - }) - unknown = [i for i in issues if "Unknown top-level config key" in i.message] - messages = " ".join(i.message for i in unknown) - assert "known_plugin_toolsets" not in messages - assert "group_sessions_per_user" not in messages - assert "thread_sessions_per_user" not in messages - assert "stt_echo_transcripts" not in messages - assert "reset_triggers" not in messages - assert "always_log_local" not in messages - assert "filter_silence_narration" not in messages - def test_provider_like_unknown_root_keeps_misplaced_message(self): - """Preserve existing base_url/api_key root-level guidance (not generic unknown).""" + """Preserve existing base_url/api_key root-level guidance.""" issues = validate_config_structure({ "base_url": "https://example.com/v1", "api_key": "secret", @@ -285,19 +252,13 @@ class TestUnknownTopLevelKeys: i for i in issues if i.severity == "warning" and "looks misplaced" in i.message ] - generic_unknown = [ - i for i in issues - if "Unknown top-level config key" in i.message - ] assert any("base_url" in i.message for i in misplaced) assert any("api_key" in i.message for i in misplaced) - assert generic_unknown == [] def test_private_underscore_keys_not_flagged(self): - """Internal keys starting with _ remain ignored (except known defaults).""" + """Internal keys starting with _ remain ignored.""" issues = validate_config_structure({ "_internal_scratch": True, "model": {"provider": "openrouter"}, }) - assert not any("Unknown top-level" in i.message for i in issues) - assert not any("_internal_scratch" in i.message for i in issues) + assert issues == []