fix(catalog): validate http+api_key manifests declare the header's env key
Simplify-pass follow-up on the #70782 salvage: _bearer_auth_headers hard-emits ${MCP_<NAME>_API_KEY} but install_entry only persists auth.env-declared vars — a manifest naming its key differently (the shipped n8n style) would install cleanly yet send a literal-placeholder header at connect time (silent 401, the #37792 bug class). Enforce the naming contract at parse time. Also pins the secret-stays-in-.env property in the install test (raw config.yaml carries the template, never the secret). Mutation-checked: validation disabled -> guard test fails.
This commit is contained in:
parent
f8f475569f
commit
00475e1b26
|
|
@ -230,6 +230,21 @@ def _parse_manifest(path: Path) -> CatalogEntry:
|
|||
scopes=list(auth_raw.get("scopes") or []),
|
||||
env_var=auth_raw.get("env_var"),
|
||||
)
|
||||
if t_type == "http" and a_type == "api_key":
|
||||
# _build_server_config emits an Authorization header referencing
|
||||
# ${MCP_<NAME>_API_KEY} (via _bearer_auth_headers), but install_entry
|
||||
# only persists the env vars DECLARED in auth.env. Enforce the naming
|
||||
# contract at parse time, or a manifest declaring e.g. N8N_API_KEY
|
||||
# would install cleanly yet send a literal-placeholder header (401)
|
||||
# at connect time.
|
||||
from hermes_cli.mcp_config import _env_key_for_server
|
||||
|
||||
_required_key = _env_key_for_server(name)
|
||||
if not any(spec.name == _required_key for spec in env_list):
|
||||
raise CatalogError(
|
||||
f"{path}: http + api_key auth requires auth.env to declare "
|
||||
f"'{_required_key}' (the key the Authorization header references)"
|
||||
)
|
||||
|
||||
tools_raw = data.get("tools") or {}
|
||||
if not isinstance(tools_raw, dict):
|
||||
|
|
|
|||
|
|
@ -158,6 +158,26 @@ class TestManifestParsing:
|
|||
assert cfg["url"] == "https://mcp.example.com/sse"
|
||||
assert cfg["headers"] == {"Authorization": "Bearer ${MCP_DEMO_API_KEY}"}
|
||||
|
||||
def test_http_api_key_requires_matching_env_declaration(self, catalog_dir):
|
||||
"""http+api_key manifests must declare the env key the header references.
|
||||
|
||||
install_entry only persists auth.env-declared vars; a manifest naming
|
||||
its key e.g. N8N_API_KEY would install cleanly but send a literal
|
||||
${MCP_DEMO_API_KEY} placeholder at connect time (silent 401).
|
||||
"""
|
||||
body = _basic_manifest(
|
||||
transport={"type": "http", "url": "https://mcp.example.com/sse"},
|
||||
auth={
|
||||
"type": "api_key",
|
||||
"env": [{"name": "DEMO_API_KEY", "prompt": "key", "secret": True}],
|
||||
},
|
||||
)
|
||||
path = _write_manifest(catalog_dir, "demo", body)
|
||||
from hermes_cli.mcp_catalog import CatalogError, _parse_manifest
|
||||
|
||||
with pytest.raises(CatalogError, match="MCP_DEMO_API_KEY"):
|
||||
_parse_manifest(path)
|
||||
|
||||
|
||||
|
||||
|
||||
|
|
@ -230,6 +250,13 @@ class TestInstall:
|
|||
server = load_config()["mcp_servers"]["demo"]
|
||||
assert server["url"] == "https://mcp.example.com/sse"
|
||||
assert server["headers"] == {"Authorization": "Bearer secret-val"}
|
||||
# The raw file must carry the ${...} template, never the secret —
|
||||
# load_config resolves it; config.yaml itself stays secret-free.
|
||||
from hermes_cli.config import get_config_path
|
||||
|
||||
raw = get_config_path().read_text()
|
||||
assert "${MCP_DEMO_API_KEY}" in raw
|
||||
assert "secret-val" not in raw
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue