fix(mcp): stop reporting false OAuth success when no token was obtained (#34807)
* docs(code-execution): document HERMES_* env narrowing + passthrough workaround
The execute_code sandbox-child env scrub (108397726, #27303) deliberately
dropped the broad HERMES_ prefix passthrough, keeping only an operational
4-var allowlist (HERMES_HOME/PROFILE/CONFIG/ENV). A script that relied on a
non-secret HERMES_* var (HERMES_BASE_URL, HERMES_KANBAN_DB, HERMES_*_WEBHOOK,
or a plugin-defined one) now sees it unset in the child.
Document the behavior change and the two recovery routes (terminal.env_passthrough
in config.yaml, or required_environment_variables in skill frontmatter), plus
the debug log line that surfaces the drop for diagnosis.
* fix(mcp): stop reporting false OAuth success when no token was obtained
`hermes mcp login` reported "Authenticated — N tool(s) available" for
servers that serve tools/list without auth (e.g. Google's official Drive
MCP server) even when the OAuth flow never completed — dynamic client
registration 400'd because the provider doesn't support RFC 7591, so no
token was ever acquired. Every real tool call then hung until timeout
with no indication of why.
Login now verifies a token actually landed on disk after the probe. When
it didn't, it warns that authentication didn't complete and shows the
config needed to supply a pre-registered client_id/client_secret (the
existing, already-supported workaround for DCR-less providers).
Adds a docs pitfall for Google Drive / Atlassian-style providers.
Fixes #34775
This commit is contained in:
@@ -595,3 +595,58 @@ class TestMcpLogin:
|
||||
out = capsys.readouterr().out
|
||||
assert "no URL" in out or "not an OAuth" in out
|
||||
|
||||
def test_login_false_success_no_token(self, tmp_path, capsys, monkeypatch):
|
||||
"""Probe lists tools without auth (Google Drive), but no token landed.
|
||||
|
||||
The server allows tools/list without auth (DCR 400'd), so the probe
|
||||
succeeds yet no OAuth token exists. Login must NOT claim success — it
|
||||
should warn and point the user at pre-registered client_id config.
|
||||
"""
|
||||
_seed_config(tmp_path, {
|
||||
"googledrive": {
|
||||
"url": "https://drivemcp.googleapis.com/mcp/v1",
|
||||
"auth": "oauth",
|
||||
},
|
||||
})
|
||||
# Probe returns tools even though auth never completed.
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.mcp_config._probe_single_server",
|
||||
lambda name, cfg: [("search_files", "d"), ("read_file_content", "d")],
|
||||
)
|
||||
# No token file is created → _oauth_tokens_present() returns False.
|
||||
from hermes_cli.mcp_config import cmd_mcp_login
|
||||
|
||||
cmd_mcp_login(_make_args(name="googledrive"))
|
||||
out = capsys.readouterr().out
|
||||
|
||||
assert "no OAuth token was obtained" in out
|
||||
assert "Authenticated" not in out
|
||||
assert "client_id" in out
|
||||
|
||||
def test_login_genuine_success_with_token(self, tmp_path, capsys, monkeypatch):
|
||||
"""Probe lists tools AND a token exists → report real success."""
|
||||
_seed_config(tmp_path, {
|
||||
"realserver": {"url": "https://mcp.example.com/mcp", "auth": "oauth"},
|
||||
})
|
||||
token_dir = tmp_path / "mcp-tokens"
|
||||
|
||||
# cmd_mcp_login wipes tokens before probing, then the real OAuth flow
|
||||
# writes a fresh token during the probe. Simulate that: the mocked
|
||||
# probe drops a token file, mirroring a successful authorization.
|
||||
def mock_probe(name, cfg):
|
||||
token_dir.mkdir(exist_ok=True)
|
||||
(token_dir / "realserver.json").write_text('{"access_token": "x"}')
|
||||
return [("a", "d"), ("b", "d"), ("c", "d")]
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.mcp_config._probe_single_server", mock_probe
|
||||
)
|
||||
|
||||
from hermes_cli.mcp_config import cmd_mcp_login
|
||||
|
||||
cmd_mcp_login(_make_args(name="realserver"))
|
||||
out = capsys.readouterr().out
|
||||
|
||||
assert "Authenticated — 3 tool(s) available" in out
|
||||
assert "no OAuth token" not in out
|
||||
|
||||
|
||||
Reference in New Issue
Block a user