diff --git a/hermes_cli/mcp_catalog.py b/hermes_cli/mcp_catalog.py index 8eec5cb3a9..6a7da08dbd 100644 --- a/hermes_cli/mcp_catalog.py +++ b/hermes_cli/mcp_catalog.py @@ -455,15 +455,21 @@ def _expand_install_dir(value: str, install_dir: Optional[Path]) -> str: return value.replace(_INSTALL_DIR_VAR, str(install_dir)) -def _prompt_env_vars(specs: List[EnvVarSpec]) -> Dict[str, str]: +def _prompt_env_vars(specs: List[EnvVarSpec], preloaded: Optional[Dict[str, str]] = None) -> Dict[str, str]: """Prompt for each env spec. Secrets persist to ~/.hermes/.env. Non-secrets are only collected and returned — the caller inlines them into the server config (config.yaml), - since .env is secrets-only. + since .env is secrets-only. Values already supplied by the caller + (``preloaded``, e.g. from a dashboard form) skip the prompt. """ + preloaded = preloaded or {} collected: Dict[str, str] = {} for spec in specs: + pre = preloaded.get(spec.name) + if pre is not None: + collected[spec.name] = pre + continue existing = get_env_value(spec.name) if spec.secret else None if existing: _say(f" ✓ {spec.name} already set in .env") @@ -703,12 +709,15 @@ def card_install_config(entry: CatalogEntry) -> dict: return cfg -def install_entry(entry: CatalogEntry, *, enable: bool = True) -> None: +def install_entry(entry: CatalogEntry, *, enable: bool = True, preloaded_env: Optional[Dict[str, str]] = None) -> None: """Install a catalog entry end-to-end. Order: git clone + bootstrap (if any); credential prompts (``auth.env``) to .env; write ``mcp_servers.`` (with the ``auth: oauth`` marker and any pre-registered ``oauth`` block); probe + tool checklist (falling back per :func:`_apply_tool_selection`); print post_install notes. + + ``preloaded_env`` carries env values already supplied by the caller (e.g. + the dashboard form); they skip the interactive prompt. """ print() _say(f" Installing MCP '{entry.name}'", Colors.CYAN + Colors.BOLD) @@ -724,7 +733,7 @@ def install_entry(entry: CatalogEntry, *, enable: bool = True) -> None: if entry.auth.env: print() _say(" Configure credentials:", Colors.CYAN) - env_values = _prompt_env_vars(entry.auth.env) + env_values = _prompt_env_vars(entry.auth.env, preloaded_env or {}) if entry.auth.type == "oauth" and entry.auth.provider: # Provider-mediated OAuth relies on the existing `hermes auth ` flow; surface # guidance rather than auto-running it to keep install decoupled from provider-auth lifecycle. diff --git a/hermes_cli/web_routers/mcp.py b/hermes_cli/web_routers/mcp.py index 9bdaa8bdbb..4eab73b1dd 100644 --- a/hermes_cli/web_routers/mcp.py +++ b/hermes_cli/web_routers/mcp.py @@ -470,11 +470,12 @@ async def install_mcp_catalog_entry(body: MCPCatalogInstall, profile: Optional[s raise HTTPException(status_code=400, detail=str(exc)) from exc effective_profile = body.profile or profile + # Secrets are persisted to .env here (before install_entry runs); non-secret + # values ride preloaded_env into install_entry → config.yaml, so .env stays + # secrets-only and nothing re-prompts on a non-TTY server. if body.env: def _write_env(): with _profile_scope(effective_profile): - # Only declared secrets go to .env; non-secret values ride in the - # server config (inlined by install_entry) so .env stays secrets-only. for spec in entry.auth.env or []: value = (body.env or {}).get(spec.name) if spec.secret and value: @@ -497,7 +498,10 @@ async def install_mcp_catalog_entry(body: MCPCatalogInstall, profile: Optional[s # No git step — install synchronously; install_entry goes through the # call-time config/env resolvers so the profile scope covers it. try: - await scoped_to_thread(effective_profile, lambda: mcp_catalog.install_entry(entry, enable=body.enable)) + await scoped_to_thread( + effective_profile, + lambda: mcp_catalog.install_entry(entry, enable=body.enable, preloaded_env=body.env or None), + ) except HTTPException: raise except Exception as exc: diff --git a/optional-mcps/asana/manifest.yaml b/optional-mcps/asana/manifest.yaml index 3d050f29a9..50e6ba790d 100644 --- a/optional-mcps/asana/manifest.yaml +++ b/optional-mcps/asana/manifest.yaml @@ -20,8 +20,9 @@ transport: auth: type: oauth - # Prompted at install (CLI, dashboard, desktop) and stored in the profile's - # .env — config.yaml only ever carries the ${VAR} references below. + # Prompted at install (CLI, dashboard, desktop). Secrets are stored in the + # profile's .env; non-secret values (client id below) are inlined into + # config.yaml — config.yaml only ever carries ${VAR} references for secrets. env: - name: ASANA_CLIENT_ID prompt: "Asana MCP app Client ID (developer console → your MCP app → OAuth)" @@ -50,7 +51,8 @@ post_install: | - OAuth → Redirect URL: http://localhost:27890/callback (exactly). - Manage distribution → allow the workspace(s) you will use. Its Client ID / Client secret are the ASANA_CLIENT_ID / ASANA_CLIENT_SECRET - values prompted above (stored in the profile's .env, never in config.yaml). + values prompted above. The secret is stored in the profile's .env; the + non-secret client id is inlined into config.yaml. Then run `hermes mcp login asana` (or click Authorize in the dashboard / Desktop: the callback still arrives at http://localhost:27890/callback, so diff --git a/tests/hermes_cli/test_mcp_catalog_env_boundary.py b/tests/hermes_cli/test_mcp_catalog_env_boundary.py index 73d1698a27..63c19bc6fe 100644 --- a/tests/hermes_cli/test_mcp_catalog_env_boundary.py +++ b/tests/hermes_cli/test_mcp_catalog_env_boundary.py @@ -71,7 +71,7 @@ def test_catalog_rejects_undeclared_key_before_any_write_or_install( monkeypatch.setattr( mcp_catalog, "install_entry", - lambda entry, enable=True: installs.append(entry.name), + lambda entry, enable=True, preloaded_env=None: installs.append(entry.name), ) response = client.post( @@ -119,7 +119,7 @@ def test_catalog_cannot_declare_reserved_control_key( monkeypatch.setattr( mcp_catalog, "install_entry", - lambda entry, enable=True: installs.append(entry.name), + lambda entry, enable=True, preloaded_env=None: installs.append(entry.name), ) response = client.post( @@ -147,6 +147,12 @@ def test_catalog_accepts_declared_credential( from tools.connectors.mcp import _CatalogBackend probes: list[str] = [] + installs: list[str] = [] + monkeypatch.setattr( + mcp_catalog, + "install_entry", + lambda entry, enable=True, preloaded_env=None: installs.append(entry.name), + ) def probe(name, cfg, **_kwargs): # The credential is in scope for the probe, and nothing is saved before it answers. @@ -187,13 +193,21 @@ def test_catalog_non_secret_env_never_lands_in_env_file( "secret": False, } ) + # The transport references the non-secret var; install_entry inlines it. + # (HTTP transport so the var lands in the server url.) + manifest["transport"] = {"type": "http", "url": "${DEMO_BASE_URL}"} + manifest["auth"]["type"] = "api_key" + manifest["auth"]["env"] = [ + {"name": "MCP_DEMO_API_KEY", "prompt": "Demo API key", "secret": True}, + {"name": "DEMO_BASE_URL", "prompt": "Demo base URL", "secret": False}, + ] manifest_path.write_text(yaml.safe_dump(manifest), encoding="utf-8") - - installs: list[str] = [] + # The real install_entry probes the server after writing config; avoid + # launching a nonexistent binary in tests. monkeypatch.setattr( mcp_catalog, - "install_entry", - lambda entry, enable=True: installs.append(entry.name), + "_probe_tools", + lambda name: None, ) response = client.post( @@ -202,18 +216,26 @@ def test_catalog_non_secret_env_never_lands_in_env_file( json={ "name": "demo", "env": { - "DEMO_API_KEY": "valid-demo-value", + "MCP_DEMO_API_KEY": "valid-demo-value", "DEMO_BASE_URL": "https://demo.example.test", }, }, ) assert response.status_code == 200 - assert installs == ["demo"] env_text = (catalog_env / ".env").read_text(encoding="utf-8") - assert "DEMO_API_KEY=valid-demo-value" in env_text + assert "MCP_DEMO_API_KEY=valid-demo-value" in env_text assert "DEMO_BASE_URL" not in env_text assert "https://demo.example.test" not in env_text + # The non-secret is inlined into config.yaml (server config carries the + # literal; the raw file never stores it and never keeps a ${VAR} ref). + from hermes_cli.config import load_config + + server = load_config()["mcp_servers"]["demo"] + assert server["url"] == "https://demo.example.test" + assert "${DEMO_BASE_URL}" not in ( + catalog_env / "config.yaml" + ).read_text(encoding="utf-8") @pytest.mark.parametrize(