fix: classify .env-routed keys, hyphenated headers and ${VAR} placeholders in config get
`_is_secret_config_key` matched only an exact set plus four suffixes, so
credentials that `hermes config set` routes to .env but end in bare `_KEY`
(FAL_KEY, VOICE_TOOLS_OPENAI_KEY, API_SERVER_KEY) or AWS_SECRET_ACCESS_KEY
printed raw with redaction on. Every `_is_env_config_key`-routed key is now a
credential unless its suffix names a non-secret shape (_URL/_HOST/_USER/_ID/
_DOMAIN/_SCHEME); `_key`/`_access_key` join the leaf suffixes. Header names are
folded `-`→`_` before matching so `mcp_servers.<s>.headers.X-API-Key` masks on
get and on the set echo. Bare `auth` leaves the exact set: `mcp_servers.<s>.auth`
is the documented `oauth` mode enum and rendered as `***`. Unresolved `${VAR}`
placeholders are printed as-is so the operator can see which env var the config
references.
Review finding: `config get` printed FAL_KEY / AWS_SECRET_ACCESS_KEY / X-API-Key raw, masked `auth: oauth`, and hid `${VAR}` placeholders.
This commit is contained in:
+20
-7
@@ -2749,18 +2749,26 @@ def redact_key(key: str) -> str:
|
||||
|
||||
# Key names (case-insensitive, exact match) whose VALUE is a credential and must be masked
|
||||
# before printing any config dict. Exact-match so ``token_count`` / ``secret_santa`` stay visible.
|
||||
# Bare ``auth`` is deliberately absent: ``mcp_servers.<s>.auth: oauth`` is a documented mode enum.
|
||||
_SECRET_CONFIG_KEYS = frozenset({
|
||||
"api_key", "apikey", "key", "token", "access_token", "refresh_token", "id_token",
|
||||
"secret", "client_secret", "password", "passwd", "auth", "authorization",
|
||||
"secret", "client_secret", "password", "passwd", "authorization",
|
||||
"private_key", "bearer", "jwt"})
|
||||
# Env-map shapes (``mcp_servers.<s>.env.FOO_API_KEY``, ``GEMINI_API_KEY``) — the same suffixes
|
||||
# ``_is_env_config_key`` routes to .env. Suffix-only so ``token_count`` stays visible.
|
||||
_SECRET_CONFIG_KEY_SUFFIXES = ("_api_key", "_token", "_secret", "_password")
|
||||
# Env-map shapes (``mcp_servers.<s>.env.FOO_API_KEY``, ``FAL_KEY``, ``AWS_SECRET_ACCESS_KEY``) and
|
||||
# the suffixes ``_is_env_config_key`` routes to .env. Suffix-only so ``token_count`` stays visible.
|
||||
_SECRET_CONFIG_KEY_SUFFIXES = ("_api_key", "_token", "_secret", "_password", "_key", "_access_key")
|
||||
# .env-routed keys are credentials by default; these suffixes name the non-secret exceptions
|
||||
# (``TERMINAL_SSH_HOST``, ``TOOL_GATEWAY_URL``, ``BROWSERBASE_PROJECT_ID``).
|
||||
_NON_SECRET_KEY_SUFFIXES = ("_url", "_host", "_user", "_id", "_domain", "_scheme")
|
||||
_ENV_PLACEHOLDER_RE = re.compile(r"^\$\{[A-Za-z_][A-Za-z0-9_]*\}$")
|
||||
|
||||
|
||||
def _is_secret_config_key(key: str) -> bool:
|
||||
"""Whether the LAST segment of a config key names a credential value."""
|
||||
leaf = key.rsplit(".", 1)[-1].lower()
|
||||
"""Whether the LAST segment of a config key names a credential value. Header names
|
||||
(``mcp_servers.<s>.headers.X-API-Key``) are folded to snake_case before matching."""
|
||||
leaf = key.rsplit(".", 1)[-1].lower().replace("-", "_")
|
||||
if _is_env_config_key(key):
|
||||
return not leaf.endswith(_NON_SECRET_KEY_SUFFIXES)
|
||||
return leaf in _SECRET_CONFIG_KEYS or leaf.endswith(_SECRET_CONFIG_KEY_SUFFIXES)
|
||||
|
||||
|
||||
@@ -2775,6 +2783,7 @@ def redact_config_value(value: Any, _depth: int = 0) -> Any:
|
||||
return {
|
||||
k: mask_secret(v)
|
||||
if isinstance(k, str) and _is_secret_config_key(k) and isinstance(v, str) and v
|
||||
and not _ENV_PLACEHOLDER_RE.match(v)
|
||||
else redact_config_value(v, _depth + 1)
|
||||
for k, v in value.items()}
|
||||
if isinstance(value, list):
|
||||
@@ -3560,7 +3569,11 @@ def get_config_value(key: str, *, as_json: bool = False, raw: bool = False):
|
||||
|
||||
from agent.redact import _redact_enabled, mask_secret
|
||||
if not raw and _redact_enabled():
|
||||
value = mask_secret(value) if isinstance(value, str) and _is_secret_config_key(key) else redact_config_value(value)
|
||||
if isinstance(value, str):
|
||||
if _is_secret_config_key(key) and not _ENV_PLACEHOLDER_RE.match(value):
|
||||
value = mask_secret(value)
|
||||
else:
|
||||
value = redact_config_value(value)
|
||||
|
||||
print(_format_config_get_value(value, as_json=as_json))
|
||||
|
||||
|
||||
@@ -892,3 +892,32 @@ class TestConfigGetRedaction:
|
||||
|
||||
get_config_value("providers.gemini.api_key", raw=True)
|
||||
assert capsys.readouterr().out.strip() == self.SECRET
|
||||
|
||||
@pytest.mark.parametrize("key, env_line, yaml_line, masked", [
|
||||
# .env-routed keys are credentials unless the suffix is a known non-secret shape.
|
||||
("FAL_KEY", "FAL_KEY=" + SECRET, "", True),
|
||||
("VOICE_TOOLS_OPENAI_KEY", "VOICE_TOOLS_OPENAI_KEY=" + SECRET, "", True),
|
||||
("TERMINAL_SSH_HOST", "TERMINAL_SSH_HOST=" + SECRET, "", False),
|
||||
("mcp_servers.s.env.AWS_SECRET_ACCESS_KEY", "", " env: {AWS_SECRET_ACCESS_KEY: " + SECRET + "}\n", True),
|
||||
# Hyphenated header names fold to snake_case before matching (#84153 reviewer case).
|
||||
("mcp_servers.s.headers.X-API-Key", "", " headers: {X-API-Key: " + SECRET + "}\n", True),
|
||||
# Bare `auth` is the MCP transport mode enum, not a credential.
|
||||
("mcp_servers.s.auth", "", " auth: oauth\n", False),
|
||||
# An unresolved ${VAR} placeholder names the env var; masking it hides that reference.
|
||||
("mcp_servers.s.env.UNSET_THING_API_KEY", "", " env: {UNSET_THING_API_KEY: '${UNSET_THING_API_KEY}'}\n", False),
|
||||
])
|
||||
def test_config_get_classifies_env_header_and_enum_keys(
|
||||
self, _isolated_hermes_home, capsys, monkeypatch, key, env_line, yaml_line, masked):
|
||||
monkeypatch.delenv("UNSET_THING_API_KEY", raising=False)
|
||||
(_isolated_hermes_home / "config.yaml").write_text(
|
||||
"mcp_servers:\n s:\n url: https://x.example\n" + yaml_line, encoding="utf-8")
|
||||
(_isolated_hermes_home / ".env").write_text(env_line + "\n", encoding="utf-8")
|
||||
from hermes_cli.config import get_config_value
|
||||
|
||||
get_config_value(key)
|
||||
out = capsys.readouterr().out.strip()
|
||||
if masked:
|
||||
assert self.SECRET not in out and self.SECRET[:4] in out
|
||||
else:
|
||||
assert out == {"TERMINAL_SSH_HOST": self.SECRET, "mcp_servers.s.auth": "oauth"}.get(
|
||||
key, "${UNSET_THING_API_KEY}")
|
||||
|
||||
Reference in New Issue
Block a user