diff --git a/apps/desktop/src/app/settings/providers-settings.test.tsx b/apps/desktop/src/app/settings/providers-settings.test.tsx index 8df85663687..8379d203f6c 100644 --- a/apps/desktop/src/app/settings/providers-settings.test.tsx +++ b/apps/desktop/src/app/settings/providers-settings.test.tsx @@ -21,16 +21,18 @@ vi.mock('@/store/onboarding', () => ({ startManualProviderOAuth: (providerId: string) => startManualProviderOAuth(providerId) })) -function provider(id: string, loggedIn: boolean): OAuthProvider { +function provider(id: string, loggedIn: boolean, patch: Partial = {}): OAuthProvider { return { cli_command: `hermes auth add ${id}`, + disconnectable: true, docs_url: '', flow: 'device_code', id, name: id === 'nous' ? 'Nous Portal' : 'MiniMax', status: { logged_in: loggedIn - } + }, + ...patch } } @@ -75,4 +77,24 @@ describe('ProvidersSettings', () => { expect(startManualProviderOAuth).toHaveBeenCalledWith('nous') expect(disconnectOAuthProvider).not.toHaveBeenCalled() }) + + it('does not offer removal for externally managed providers', async () => { + listOAuthProviders.mockResolvedValue({ + providers: [ + provider('qwen-oauth', true, { + cli_command: 'hermes auth add qwen-oauth', + disconnect_hint: 'Use `hermes auth add qwen-oauth` or that provider\'s CLI to remove it.', + disconnectable: false, + flow: 'external', + name: 'Qwen (via Qwen CLI)' + }) + ] + }) + + await renderProvidersSettings() + + expect(await screen.findByText('Qwen Code')).toBeTruthy() + expect(screen.queryByRole('button', { name: 'Remove Qwen Code' })).toBeNull() + expect(screen.getByText(/managed outside Hermes/)).toBeTruthy() + }) }) diff --git a/apps/desktop/src/app/settings/providers-settings.tsx b/apps/desktop/src/app/settings/providers-settings.tsx index bddc651a4eb..f1132e6c33d 100644 --- a/apps/desktop/src/app/settings/providers-settings.tsx +++ b/apps/desktop/src/app/settings/providers-settings.tsx @@ -6,6 +6,7 @@ import { FeaturedProviderRow, KeyProviderRow, ProviderRow, + providerTitle, sortProviders } from '@/components/desktop-onboarding-overlay' import { Button } from '@/components/ui/button' @@ -187,8 +188,13 @@ function ConnectedProviderRow({ provider: OAuthProvider }) { const { t } = useI18n() - const title = provider.name + const title = providerTitle(provider) const Trail = provider.flow === 'external' ? Terminal : ChevronRight + const canDisconnect = provider.disconnectable ?? provider.flow !== 'external' + + const disconnectHint = provider.flow === 'external' + ? t.settings.providers.removeExternal(title, provider.cli_command) + : t.settings.providers.removeKeyManaged(title) return (
@@ -201,20 +207,27 @@ function ConnectedProviderRow({

{t.onboarding.flowSubtitles[provider.flow]}

+ {!canDisconnect && ( +

+ {disconnectHint} +

+ )}
- + {canDisconnect && ( + + )}
) @@ -270,9 +283,9 @@ export function ProvidersSettings({ onViewChange, view }: ProvidersSettingsProps }, [onboardingActive]) async function handleDisconnect(provider: OAuthProvider) { - const name = provider.name + const name = providerTitle(provider) - if (!window.confirm(`Remove ${name}?`)) { + if (!window.confirm(t.settings.providers.removeConfirm(name))) { return } @@ -280,10 +293,10 @@ export function ProvidersSettings({ onViewChange, view }: ProvidersSettingsProps try { await disconnectOAuthProvider(provider.id) - notify({ durationMs: 3_000, kind: 'success', message: `${name} removed.` }) + notify({ durationMs: 3_000, kind: 'success', title: t.settings.providers.removedTitle, message: t.settings.providers.removedMessage(name) }) await refreshOAuthProviders().catch(() => undefined) } catch (err) { - notifyError(err, `Could not remove ${name}`) + notifyError(err, t.settings.providers.failedRemove(name)) } finally { setDisconnecting(null) } diff --git a/apps/desktop/src/components/desktop-onboarding-overlay.tsx b/apps/desktop/src/components/desktop-onboarding-overlay.tsx index a3808e634d1..eea24677ab9 100644 --- a/apps/desktop/src/components/desktop-onboarding-overlay.tsx +++ b/apps/desktop/src/components/desktop-onboarding-overlay.tsx @@ -180,7 +180,7 @@ const PROVIDER_DISPLAY: Record = { const assetPath = (path: string) => `${import.meta.env.BASE_URL}${path.replace(/^\/+/, '')}` -const providerTitle = (p: OAuthProvider) => PROVIDER_DISPLAY[p.id]?.title ?? p.name +export const providerTitle = (p: OAuthProvider) => PROVIDER_DISPLAY[p.id]?.title ?? p.name const orderOf = (p: OAuthProvider) => PROVIDER_DISPLAY[p.id]?.order ?? 99 export const sortProviders = (providers: OAuthProvider[]) => diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index d9739aab995..ac55d6b4871 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -513,6 +513,12 @@ export const en: Translations = { collapse: 'Collapse', connectAnother: 'Connect another provider', otherProviders: 'Other providers', + removeConfirm: provider => `Remove ${provider}?`, + removeExternal: (provider, command) => `${provider} is managed outside Hermes. Remove it with ${command}.`, + removeKeyManaged: provider => `${provider} is configured from an API key. Remove it from API Keys.`, + removedTitle: 'Account removed', + removedMessage: provider => `${provider} was removed.`, + failedRemove: provider => `Could not remove ${provider}`, noProviderKeys: 'No provider API keys available.', loading: 'Loading providers...' }, diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index aa0b736b856..7e2d5775a05 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -642,6 +642,12 @@ export const ja = defineLocale({ collapse: '折りたたむ', connectAnother: '別のプロバイダーを接続', otherProviders: 'その他のプロバイダー', + removeConfirm: provider => `${provider} を削除しますか？`, + removeExternal: (provider, command) => `${provider} は Hermes の外部で管理されています。${command} で削除してください。`, + removeKeyManaged: provider => `${provider} は API キーで設定されています。API Keys から削除してください。`, + removedTitle: 'アカウントを削除しました', + removedMessage: provider => `${provider} を削除しました。`, + failedRemove: provider => `${provider} を削除できませんでした`, noProviderKeys: '利用可能なプロバイダー API キーがありません。', loading: 'プロバイダーを読み込み中...' }, diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index d89199c5157..499bc9ad76d 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -413,6 +413,12 @@ export interface Translations { collapse: string connectAnother: string otherProviders: string + removeConfirm: (provider: string) => string + removeExternal: (provider: string, command: string) => string + removeKeyManaged: (provider: string) => string + removedTitle: string + removedMessage: (provider: string) => string + failedRemove: (provider: string) => string noProviderKeys: string loading: string } diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index eda65ad6b70..f0b03fe1b93 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -621,6 +621,12 @@ export const zhHant = defineLocale({ collapse: '收合', connectAnother: '連結其他提供方', otherProviders: '其他提供方', + removeConfirm: provider => `移除 ${provider}？`, + removeExternal: (provider, command) => `${provider} 由 Hermes 外部管理。請使用 ${command} 移除。`, + removeKeyManaged: provider => `${provider} 由 API 金鑰設定。請從 API Keys 中移除。`, + removedTitle: '帳號已移除', + removedMessage: provider => `${provider} 已移除。`, + failedRemove: provider => `無法移除 ${provider}`, noProviderKeys: '沒有可用的提供方 API 金鑰。', loading: '正在載入提供方...' }, diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index e1082deba44..f555a686347 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -708,6 +708,12 @@ export const zh: Translations = { collapse: '收起', connectAnother: '连接其他提供方', otherProviders: '其他提供方', + removeConfirm: provider => `移除 ${provider}？`, + removeExternal: (provider, command) => `${provider} 由 Hermes 外部管理。请使用 ${command} 移除。`, + removeKeyManaged: provider => `${provider} 由 API 密钥配置。请从 API Keys 中移除。`, + removedTitle: '账号已移除', + removedMessage: provider => `${provider} 已移除。`, + failedRemove: provider => `无法移除 ${provider}`, noProviderKeys: '没有可用的提供方 API 密钥。', loading: '正在加载提供方...' }, diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 86dc41862db..627fe5e53e1 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -47,6 +47,8 @@ export interface OAuthProviderStatus { export interface OAuthProvider { cli_command: string + disconnect_hint?: null | string + disconnectable?: boolean docs_url: string flow: 'device_code' | 'external' | 'loopback' | 'pkce' id: string diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 78a327a4767..4031f3685f7 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -5134,6 +5134,15 @@ def _resolve_provider_status(provider_id: str, status_fn) -> Dict[str, Any]: return {"logged_in": False} +def _oauth_provider_disconnect_hint(provider: Dict[str, Any], status: Dict[str, Any]) -> Optional[str]: + """Return the manual disconnect path when the API cannot clear this provider.""" + if provider.get("flow") == "external": + return f"Use `{provider['cli_command']}` or that provider's CLI to remove it." + if status.get("source") == "env_var": + return "Remove the API key from Settings → Keys instead." + return None + + @app.get("/api/providers/oauth") async def list_oauth_providers(): """Enumerate every OAuth-capable LLM provider with current status. @@ -5155,12 +5164,15 @@ async def list_oauth_providers(): providers = [] for p in _OAUTH_PROVIDER_CATALOG: status = _resolve_provider_status(p["id"], p.get("status_fn")) + disconnect_hint = _oauth_provider_disconnect_hint(p, status) providers.append({ "id": p["id"], "name": p["name"], "flow": p["flow"], "cli_command": p["cli_command"], "docs_url": p["docs_url"], + "disconnect_hint": disconnect_hint, + "disconnectable": disconnect_hint is None, "status": status, }) return {"providers": providers} @@ -5171,37 +5183,56 @@ async def disconnect_oauth_provider(provider_id: str, request: Request): """Disconnect an OAuth provider. Token-protected (matches /env/reveal).""" _require_token(request) - valid_ids = {p["id"] for p in _OAUTH_PROVIDER_CATALOG} - if provider_id not in valid_ids: + catalog_by_id = {p["id"]: p for p in _OAUTH_PROVIDER_CATALOG} + provider = catalog_by_id.get(provider_id) + if provider is None: raise HTTPException( status_code=400, detail=f"Unknown provider: {provider_id}. " - f"Available: {', '.join(sorted(valid_ids))}", + f"Available: {', '.join(sorted(catalog_by_id))}", ) - # Anthropic and claude-code clear the same Hermes-managed PKCE file - # AND forget the Claude Code import. We don't touch ~/.claude/* directly - # — that's owned by the Claude Code CLI; users can re-auth there if they - # want to undo a disconnect. - if provider_id in {"anthropic", "claude-code"}: + disconnect_hint = _oauth_provider_disconnect_hint(provider, {}) + if disconnect_hint: + raise HTTPException( + status_code=400, + detail=f"{provider['name']} cannot be disconnected automatically. {disconnect_hint}", + ) + + status = _resolve_provider_status(provider_id, provider.get("status_fn")) + disconnect_hint = _oauth_provider_disconnect_hint(provider, status) + if disconnect_hint: + raise HTTPException( + status_code=400, + detail=f"{provider['name']} cannot be disconnected automatically. {disconnect_hint}", + ) + + # Anthropic clears only the Hermes-managed PKCE file and auth-store entry. + # The separate claude-code catalog row is external/read-only and rejected + # above so we never pretend to remove ~/.claude/* credentials owned by the CLI. + if provider_id == "anthropic": + cleared = False try: from agent.anthropic_adapter import _HERMES_OAUTH_FILE if _HERMES_OAUTH_FILE.exists(): _HERMES_OAUTH_FILE.unlink() + cleared = True except Exception: pass # Also clear the credential pool entry if present. try: from hermes_cli.auth import clear_provider_auth - clear_provider_auth("anthropic") + cleared = clear_provider_auth("anthropic") or cleared except Exception: pass _log.info("oauth/disconnect: %s", provider_id) - return {"ok": True, "provider": provider_id} + return {"ok": bool(cleared), "provider": provider_id} try: - from hermes_cli.auth import clear_provider_auth + from hermes_cli.auth import clear_provider_auth, invalidate_nous_auth_status_cache cleared = clear_provider_auth(provider_id) + if provider_id == "nous": + invalidate_nous_auth_status_cache() _log.info("oauth/disconnect: %s (cleared=%s)", provider_id, cleared) return {"ok": bool(cleared), "provider": provider_id} except Exception as e: diff --git a/tests/hermes_cli/test_web_oauth_dispatch.py b/tests/hermes_cli/test_web_oauth_dispatch.py index 4200363baa8..5cb2c30d8b3 100644 --- a/tests/hermes_cli/test_web_oauth_dispatch.py +++ b/tests/hermes_cli/test_web_oauth_dispatch.py @@ -337,6 +337,53 @@ def test_xai_oauth_listed_as_loopback_flow(): assert "grok" in providers["xai-oauth"]["name"].lower() +def test_oauth_catalog_marks_external_providers_not_disconnectable(): + """External CLI credentials are visible in Accounts but cannot be removed by Hermes.""" + resp = client.get("/api/providers/oauth", headers=HEADERS) + assert resp.status_code == 200, resp.text + providers = {p["id"]: p for p in resp.json()["providers"]} + + assert providers["qwen-oauth"]["flow"] == "external" + assert providers["qwen-oauth"]["disconnectable"] is False + assert "provider's CLI" in providers["qwen-oauth"]["disconnect_hint"] + + assert providers["claude-code"]["flow"] == "external" + assert providers["claude-code"]["disconnectable"] is False + assert "provider's CLI" in providers["claude-code"]["disconnect_hint"] + + +def test_external_oauth_disconnect_rejected_before_auth_mutation(monkeypatch): + """DELETE must not pretend to remove credentials owned by another CLI.""" + from hermes_cli import auth as auth_mod + + def fail_clear_provider_auth(provider_id=None): + raise AssertionError("external providers must not reach clear_provider_auth") + + monkeypatch.setattr(auth_mod, "clear_provider_auth", fail_clear_provider_auth) + + resp = client.delete("/api/providers/oauth/qwen-oauth", headers=HEADERS) + assert resp.status_code == 400, resp.text + assert "cannot be disconnected automatically" in resp.text + assert "provider's CLI" in resp.text + + +def test_env_sourced_oauth_status_is_not_disconnectable(monkeypatch): + """An env/.env-backed Anthropic API key is removed from Keys, not OAuth Accounts.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", "test-anthropic-key") + + resp = client.get("/api/providers/oauth", headers=HEADERS) + assert resp.status_code == 200, resp.text + providers = {p["id"]: p for p in resp.json()["providers"]} + + assert providers["anthropic"]["status"]["source"] == "env_var" + assert providers["anthropic"]["disconnectable"] is False + assert providers["anthropic"]["disconnect_hint"] == "Remove the API key from Settings → Keys instead." + + delete_resp = client.delete("/api/providers/oauth/anthropic", headers=HEADERS) + assert delete_resp.status_code == 400, delete_resp.text + assert "Settings" in delete_resp.text + + def test_xai_loopback_start_returns_authorize_url(monkeypatch): """Start MUST bind the loopback listener and hand back an xAI authorize URL.""" from hermes_cli import auth as auth_mod