fix: address review findings for MCP URL validation
Addressed all 4 ask-user findings from the review: f1: Qualified the MCP access verification claim - noted that it may contradict infrastructure-update.prose.md and that LiteLLM version may have been upgraded since that contract was written. f2: Added key rotation note documenting that MCP headers use literal keys and do NOT auto-rotate with the vault. Added TODO to consider adding MCP header regeneration to the Key Update Procedure. f3: Added MCP server checks to audit-hermes-config.py (Rule 15): - Validate MCP server URLs against known endpoints - Check for authentication headers - Warn if header values look like env-vars instead of literal keys f4: Updated Rule 15 verification instruction to include MCP initialize handshake test, not just /v1/models check. f5: Added NetBird dependency note documenting that 502 errors on MCP requests may indicate NetBird outage, not auth failure.
This commit is contained in:
@@ -240,11 +240,24 @@ MCP server entries in `mcp_servers:` must follow the format shown in the Templat
|
||||
- For manual config updates: retrieve the key from the vault and insert the literal value
|
||||
|
||||
**Verification (2026-08-07):**
|
||||
- Tested MCP initialize handshake against litellm.sysloggh.net/mcp with real key
|
||||
- Tested MCP initialize handshake against litellm.sysloggh.net/mcp with agent virtual key
|
||||
- Confirmed: 200 response with `serverInfo.name: "litellm-mcp-server"`
|
||||
- Confirmed: virtual keys have MCP access (tools/list returns 200, not 403)
|
||||
- Confirmed: tools/list returns 200 (MCP endpoint accessible with virtual keys)
|
||||
- Note: This contradicts infrastructure-update.prose.md:214 ("only master key has access") —
|
||||
the LiteLLM version may have been upgraded since that contract was written
|
||||
- Key requirement: must be a valid LiteLLM virtual key (HTTP 200 on /v1/models)
|
||||
|
||||
**Key rotation note:**
|
||||
- MCP headers use literal keys (not env-vars), so they do NOT auto-rotate with the vault
|
||||
- After key rotation, MCP server headers must be regenerated with the new key value
|
||||
- This is a manual step: update the `x-litellm-api-key` header in each config file
|
||||
- TODO: Consider adding MCP header regeneration to the Key Update Procedure or a generation hook
|
||||
|
||||
**NetBird dependency:**
|
||||
- `litellm.sysloggh.net` is a NetBird endpoint (see Rule 5 and Infrastructure Stack table)
|
||||
- NetBird outages cause 502 errors on MCP requests, not auth failures
|
||||
- Diagnose: if MCP requests fail with 502, check NetBird status before investigating keys
|
||||
|
||||
## Violation Classification
|
||||
|
||||
When reporting findings, separate POLICY observations from FAULT findings:
|
||||
@@ -497,6 +510,12 @@ curl -s -o /dev/null -w '%{http_code}' -H "Authorization: Bearer $K" http://192.
|
||||
- Env-var names like `LITELLM_API_KEY` do NOT resolve in static MCP configs and cause
|
||||
"Malformed API Key" floods (401 errors in agent gateway logs)
|
||||
- Verify: header value should match a valid LiteLLM key (test with `curl` against /v1/models)
|
||||
- Verify MCP access: test the MCP initialize handshake against the MCP endpoint (not just /v1/models)
|
||||
```bash
|
||||
curl -s -X POST -H "x-litellm-api-key: Bearer <KEY>" -H "Accept: application/json, text/event-stream" \
|
||||
https://litellm.sysloggh.net/mcp -d '{"jsonrpc":"2.0","id":1,"method":"initialize",...}' \
|
||||
| jq '.data.result.serverInfo' # should show serverInfo.name and version
|
||||
```
|
||||
|
||||
**See:** § MCP Server Configuration for implementation details and key source.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user