fix(mcp): wire Authorization header for http catalog MCPs using api_key auth - #51100
Closed
briandevans wants to merge 1 commit into
Closed
fix(mcp): wire Authorization header for http catalog MCPs using api_key auth#51100briandevans wants to merge 1 commit into
briandevans wants to merge 1 commit into
Conversation
…ey auth
Installing an http-transport catalog MCP that authenticates with an API
key prompts for and saves the key to ~/.hermes/.env, but the generated
mcp_servers.<name> block contained only a `url` — no Authorization
header. The saved key was never referenced, so the server was contacted
unauthenticated and every tool call failed (401 / hang) with no obvious
cause.
The manual `hermes mcp add` path (hermes_cli/mcp_config.py) already writes
headers: {"Authorization": "Bearer ${VAR}"} for this case. Only the
catalog install path (_build_server_config) was missing it — it handled
the oauth auth shape for http but silently dropped api_key, even though
http+api_key is a valid, documented manifest combination.
Mirror the manual-add format in the http branch: take the env-var name
from auth.env_var when set (previously parsed but unused), else the first
declared auth.env entry, and guard so an api_key entry with no declared
env var writes no unresolvable Bearer ${} placeholder.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a catalog-install gap for HTTP-transport MCP servers using auth.type: api_key by ensuring the generated mcp_servers.<name> config includes an Authorization: Bearer ${ENV_VAR} header, matching the behavior of the manual hermes mcp add flow and making the saved .env key actually used during connections.
Changes:
- Update
hermes_cli/mcp_catalog.pyto emitheaders.Authorization = "Bearer ${<env var>}"forhttp+api_keycatalog entries (preferringauth.env_var, else the firstauth.envname). - Add a regression test asserting the persisted config contains the
Authorizationheader placeholder and that the key is saved to.env.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
hermes_cli/mcp_catalog.py |
Wires Authorization header into generated server config for catalog-installed HTTP MCPs using api_key auth. |
tests/hermes_cli/test_mcp_catalog.py |
Adds coverage for the http + api_key install path to prevent regressions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Closing to keep the queue clean — this has sat ~13 days without a review. The catalog-install Authorization-header gap is a real fix; happy to reopen if maintainers want it picked up. |
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Installing an
http-transport catalog MCP that authenticates with an API key (hermes mcp install <name>, or the interactive picker) prompts for and saves the key to~/.hermes/.env, but the generatedmcp_servers.<name>block contained only aurl— noAuthorizationheader. The saved key was never referenced, so the server was contacted unauthenticated and every tool call failed (401 / hang) with no obvious cause.The manual path (
hermes mcp add) already does this correctly (hermes_cli/mcp_config.py), writingheaders: {"Authorization": "Bearer ${VAR}"}. Only the catalog install path (_build_server_configinhermes_cli/mcp_catalog.py) was missing it — it handled theoauthauth shape for http but silently droppedapi_key.http+api_keyis a valid, documented manifest combination (the docs describe http auth as anAuthorization: Bearer ${VAR}header), so this was a latent correctness gap that triggered the moment such a manifest was installed.Related Issue
No filed issue — code-originated correctness fix (path-to-path asymmetry between the catalog install path and the manual
hermes mcp addpath).Type of Change
Changes Made
hermes_cli/mcp_catalog.py: in thehttpbranch of_build_server_config, wire theAuthorizationheader forapi_keyauth, mirroring the manual-add format. The env-var name comes fromauth.env_varwhen set, else the first declaredauth.enventry (which also gives the parsed-but-unusedAuthSpec.env_varfield a consumer). Guarded so an api_key entry with no declared env var writes no unresolvableBearer ${}placeholder.tests/hermes_cli/test_mcp_catalog.py: addtest_install_http_api_key_writes_authorization_header.How to Test
transport.type: httpandauth.type: api_keydeclaring an env var (e.g.DEMO_API_KEY).hermes mcp install <name>and enter the key when prompted.~/.hermes/config.yaml:mcp_servers.<name>now containsheaders.Authorization: "Bearer ${DEMO_API_KEY}"(previously absent). The key in.envis now actually used at connect time.The regression test
test_install_http_api_key_writes_authorization_headerbuilds an http+api_key manifest, installs it, and asserts the persisted server config carriesheaders.Authorization == "Bearer ${DEMO_API_KEY}". It fails before the fix (noheaderskey) and passes after. It reads the raw config file rather thanload_config()so it asserts the persisted${VAR}placeholder, not the env-expanded value.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/hermes_cli/test_mcp_catalog.py -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/A (no doc change; the http+api_key header shape is already documented)cli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A