fix(catalog): wire api_key auth headers for http MCP servers - #70775
fix(catalog): wire api_key auth headers for http MCP servers#70775JonthanaHanh wants to merge 1 commit into
Conversation
When an optional-mcps manifest declares transport.type=http with
auth.type=api_key, install_entry() correctly prompts for the key and
saves it to .env, but _build_server_config() only handled the oauth
case — the api_key case produced a bare url entry with no headers,
so every request to the server was unauthenticated (→ 401).
Import and call _bearer_auth_headers(entry.name) from mcp_config.py
to produce the same Authorization: Bearer ${MCP_<NAME>_API_KEY}
template used by the manual 'hermes mcp add --url' path.
Closes NousResearch#70632.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused fix. The current HTTP catalog path does omit API-key headers (hermes_cli/mcp_catalog.py:505-508), but this implementation does not preserve the manifest credential contract.
Problems
hermes_cli/mcp_catalog.py:496derivesMCP_<NAME>_API_KEYfrom the entry name.install_entry()instead prompts and saves each declaredauth.envvariable (hermes_cli/mcp_catalog.py:733-736), and arbitrary names are valid (tests/hermes_cli/test_mcp_catalog.py:175-194usesDEMO_KEY). An HTTP entry nameddemousing that valid manifest writes${MCP_DEMO_API_KEY}while savingDEMO_KEY; unmatched placeholders remain literal at runtime (tools/mcp_tool.py:4544-4548).- The new tests only cover the canonical
MCP_DEMO_API_KEYspelling, so they miss this mismatch.
Suggested changes
- Select and validate the header variable from the manifest's declared credential field, then add an HTTP install test using a noncanonical declared name such as
DEMO_KEY.
Automated hermes-sweeper review.
| elif entry.auth.type == "api_key": | ||
| from hermes_cli.mcp_config import _bearer_auth_headers | ||
|
|
||
| cfg["headers"] = _bearer_auth_headers(entry.name) |
There was a problem hiding this comment.
This helper always generates MCP_<entry-name>_API_KEY, but catalog auth.env accepts arbitrary names and install_entry() saves the declared name. A valid demo manifest declaring DEMO_KEY would save DEMO_KEY yet persist ${MCP_DEMO_API_KEY}, leaving the Authorization placeholder unresolved. Please derive the header variable from a declared manifest credential and cover that noncanonical case.
SummaryFour PRs address or reference Issue #70632's missing HTTP API-key header path. #51100 implements the focused manifest-derived fix, #59586 combines that fix with deAPI and broader catalog changes, #67682 supports validated explicit auth.env_var credentials plus Perseus Vault, and #70775 adds the header through a generated variable that can diverge from the credential saved from the manifest. Related pull requests
Duplicates#51100, the generic Bearer-auth portion of #59586, the generic portion of #67682, and #70775 overlap on the same _build_server_config omission; the deAPI and Perseus Vault catalog entries are distinct salvageable changes. Suggested consolidationAuthor action: rebase #70775 onto main and derive and validate the header variable from the declared manifest credential, following closed reference #51100, then add the contributor-requested noncanonical-name install test. This explicitly accounts for #70775's recorded best-fix status without overriding its contributor keep_open review; keep #59586 open only with a salvage path that splits out transport.headers and other independently reviewable additions, and keep #67682 open with a salvage path that drops the overlapping generic header hunk while retaining the validated Perseus Vault entry, treating both generic header portions as duplicates of the corrected #70775 path. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I70632(["issue #70632 (open)"])
subgraph Dup51100 ["PRs duplicating each other"]
P51100["PR #51100 (closed)"]
P59586["PR #59586 (open)"]
P67682["PR #67682 (open)"]
P70775["PR #70775 (open)"]
end
P70775 -->|best fix| I70632
class I70632 open
class P51100 closed
class P59586 open
class P67682 open
class P70775 open
class P51100 best
class P70775 best
class P70775 target
click I70632 "https://github.com/NousResearch/hermes-agent/issues/70632"
click P51100 "https://github.com/NousResearch/hermes-agent/pull/51100"
click P59586 "https://github.com/NousResearch/hermes-agent/pull/59586"
click P67682 "https://github.com/NousResearch/hermes-agent/pull/67682"
click P70775 "https://github.com/NousResearch/hermes-agent/pull/70775"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 4 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 31 kB of PR diffs, 14 kB of issue/PR text, 10 kB of discussion (14 comments), 7 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Problem
When an optional-mcps manifest declares
transport.type: httpwithauth.type: api_key,install_entry()correctly prompts for the key and saves it to.env, but_build_server_config()only handled theoauthcase. Theapi_keycase produced a bareurlentry with noheaders, so every request to the server was unauthenticated (→ 401).Root Cause
In
hermes_cli/mcp_catalog.py,_build_server_config()(line 489-492):Fix
Import and call
_bearer_auth_headers(entry.name)frommcp_config.pyto produce the sameAuthorization: Bearer ${MCP_<NAME>_API_KEY}template used by the manualhermes mcp add --urlpath.Tests
test_http_api_key_builds_bearer_headers_template: unit test for_build_server_config()verifying the template is correcttest_install_http_api_key_writes_bearer_headers: integration test verifying end-to-end install writes headers to configAll 41 tests in
tests/hermes_cli/test_mcp_catalog.pypass.Closes #70632