Skip to content

Commit 52bcd7e

Browse files
github-actions[bot]Amber Agentclaude
authored
[Amber] Fix: Refresh token MCP tool silently swallows auth failures and reports success (#1049)
## Automated Fix by Amber Agent This PR addresses issue #1043 using the Amber background agent. ### Changes Summary - **Action Type:** auto-fix - **Commit:** 4427e7b - **Triggered by:** Issue label/command ### Pre-merge Checklist - [ ] All linters pass - [ ] All tests pass - [ ] Changes follow project conventions (CLAUDE.md) - [ ] No scope creep beyond issue description ### Reviewer Notes This PR was automatically generated. Please review: 1. Code quality and adherence to standards 2. Test coverage for changes 3. No unintended side effects --- 🤖 Generated with [Amber Background Agent](https://github.com/ambient-code/platform/blob/main/docs/amber-automation.md) Closes #1043 Co-authored-by: Amber Agent <amber@ambient-code.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ea31419 commit 52bcd7e

File tree

3 files changed

+227
-4
lines changed

3 files changed

+227
-4
lines changed

components/runners/ambient-runner/ambient_runner/bridges/claude/tools.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ async def refresh_credentials_tool(args: dict) -> dict:
8181
}
8282
]
8383
}
84-
except Exception:
84+
except Exception as exc:
8585
logger.error("Credential refresh failed", exc_info=True)
8686
return {
8787
"content": [
8888
{
8989
"type": "text",
90-
"text": "Credential refresh failed. Check runner logs for details.",
90+
"text": f"Credential refresh failed: {exc}",
9191
}
9292
],
9393
"isError": True,

components/runners/ambient-runner/ambient_runner/platform/auth.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,17 @@ def _do_req():
166166
logger.warning(
167167
f"{credential_type} BOT_TOKEN fallback also failed: {fallback_err}"
168168
)
169-
return ""
169+
raise PermissionError(
170+
f"{credential_type} authentication failed: caller token expired "
171+
f"and BOT_TOKEN fallback also failed"
172+
) from fallback_err
173+
if e.code in (401, 403):
174+
logger.warning(
175+
f"{credential_type} credential fetch failed with HTTP {e.code}: {e}"
176+
)
177+
raise PermissionError(
178+
f"{credential_type} authentication failed with HTTP {e.code}"
179+
) from e
170180
logger.warning(f"{credential_type} credential fetch failed: {e}")
171181
return ""
172182
except Exception as e:
@@ -299,10 +309,13 @@ async def populate_runtime_credentials(context: RunnerContext) -> None:
299309
# Track git identity from provider credentials
300310
git_user_name = ""
301311
git_user_email = ""
312+
auth_failures: list[str] = []
302313

303314
# Google credentials
304315
if isinstance(google_creds, Exception):
305316
logger.warning(f"Failed to refresh Google credentials: {google_creds}")
317+
if isinstance(google_creds, PermissionError):
318+
auth_failures.append(str(google_creds))
306319
elif google_creds.get("accessToken"):
307320
try:
308321
creds_dir = _GOOGLE_WORKSPACE_CREDS_FILE.parent
@@ -335,6 +348,8 @@ async def populate_runtime_credentials(context: RunnerContext) -> None:
335348
# Jira credentials
336349
if isinstance(jira_creds, Exception):
337350
logger.warning(f"Failed to refresh Jira credentials: {jira_creds}")
351+
if isinstance(jira_creds, PermissionError):
352+
auth_failures.append(str(jira_creds))
338353
elif jira_creds.get("apiToken"):
339354
os.environ["JIRA_URL"] = jira_creds.get("url", "")
340355
os.environ["JIRA_API_TOKEN"] = jira_creds.get("apiToken", "")
@@ -344,6 +359,8 @@ async def populate_runtime_credentials(context: RunnerContext) -> None:
344359
# GitLab credentials (with user identity)
345360
if isinstance(gitlab_creds, Exception):
346361
logger.warning(f"Failed to refresh GitLab credentials: {gitlab_creds}")
362+
if isinstance(gitlab_creds, PermissionError):
363+
auth_failures.append(str(gitlab_creds))
347364
elif gitlab_creds.get("token"):
348365
os.environ["GITLAB_TOKEN"] = gitlab_creds["token"]
349366
logger.info("Updated GitLab token in environment")
@@ -355,6 +372,8 @@ async def populate_runtime_credentials(context: RunnerContext) -> None:
355372
# GitHub credentials (with user identity — takes precedence)
356373
if isinstance(github_creds, Exception):
357374
logger.warning(f"Failed to refresh GitHub credentials: {github_creds}")
375+
if isinstance(github_creds, PermissionError):
376+
auth_failures.append(str(github_creds))
358377
elif github_creds.get("token"):
359378
os.environ["GITHUB_TOKEN"] = github_creds["token"]
360379
logger.info("Updated GitHub token in environment")
@@ -367,6 +386,12 @@ async def populate_runtime_credentials(context: RunnerContext) -> None:
367386
await configure_git_identity(git_user_name, git_user_email)
368387
install_git_credential_helper()
369388

389+
if auth_failures:
390+
raise PermissionError(
391+
"Credential refresh failed due to authentication errors: "
392+
+ "; ".join(auth_failures)
393+
)
394+
370395
logger.info("Runtime credentials populated successfully")
371396

372397

components/runners/ambient-runner/tests/test_shared_session_credentials.py

Lines changed: 199 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import json
44
import os
55
from http.server import BaseHTTPRequestHandler, HTTPServer
6+
from io import BytesIO
67
from threading import Thread
7-
from unittest.mock import patch
8+
from unittest.mock import AsyncMock, MagicMock, patch
9+
from urllib.error import HTTPError
810

911
import pytest
1012

@@ -314,3 +316,199 @@ def log_message(self, format, *args):
314316
# Cleanup any leaked env vars
315317
for key in ["GITHUB_TOKEN", "GITLAB_TOKEN", "JIRA_API_TOKEN", "JIRA_URL", "JIRA_EMAIL", "GIT_USER_NAME", "GIT_USER_EMAIL"]:
316318
os.environ.pop(key, None)
319+
320+
321+
# ---------------------------------------------------------------------------
322+
# _fetch_credential — auth failure propagation (issue #1043)
323+
# ---------------------------------------------------------------------------
324+
325+
326+
class TestFetchCredentialAuthFailures:
327+
@pytest.mark.asyncio
328+
async def test_raises_permission_error_on_401_without_caller_token(self, monkeypatch):
329+
"""_fetch_credential raises PermissionError when backend returns 401 with BOT_TOKEN."""
330+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
331+
monkeypatch.setenv("PROJECT_NAME", "test-project")
332+
monkeypatch.setenv("BOT_TOKEN", "bot-token")
333+
334+
ctx = _make_context(session_id="sess-1")
335+
# No caller token — uses BOT_TOKEN directly
336+
337+
err = HTTPError("http://backend.svc.cluster.local/api/...", 401, "Unauthorized", {}, BytesIO(b""))
338+
with patch("urllib.request.urlopen", side_effect=err):
339+
with pytest.raises(PermissionError, match="authentication failed with HTTP 401"):
340+
await _fetch_credential(ctx, "github")
341+
342+
@pytest.mark.asyncio
343+
async def test_raises_permission_error_on_403_without_caller_token(self, monkeypatch):
344+
"""_fetch_credential raises PermissionError when backend returns 403 with BOT_TOKEN."""
345+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
346+
monkeypatch.setenv("PROJECT_NAME", "test-project")
347+
monkeypatch.setenv("BOT_TOKEN", "bot-token")
348+
349+
ctx = _make_context(session_id="sess-1")
350+
351+
err = HTTPError("http://backend.svc.cluster.local/api/...", 403, "Forbidden", {}, BytesIO(b""))
352+
with patch("urllib.request.urlopen", side_effect=err):
353+
with pytest.raises(PermissionError, match="authentication failed with HTTP 403"):
354+
await _fetch_credential(ctx, "google")
355+
356+
@pytest.mark.asyncio
357+
async def test_raises_permission_error_when_caller_and_bot_both_fail(self, monkeypatch):
358+
"""_fetch_credential raises PermissionError when caller token 401s and BOT_TOKEN also fails."""
359+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
360+
monkeypatch.setenv("PROJECT_NAME", "test-project")
361+
monkeypatch.setenv("BOT_TOKEN", "bot-token")
362+
363+
ctx = _make_context(session_id="sess-1", current_user_id="user@example.com")
364+
ctx.caller_token = "Bearer expired-caller-token"
365+
366+
caller_err = HTTPError("http://...", 401, "Unauthorized", {}, BytesIO(b""))
367+
fallback_err = HTTPError("http://...", 403, "Forbidden", {}, BytesIO(b""))
368+
369+
with patch("urllib.request.urlopen", side_effect=[caller_err, fallback_err]):
370+
with pytest.raises(PermissionError, match="caller token expired and BOT_TOKEN fallback also failed"):
371+
await _fetch_credential(ctx, "github")
372+
373+
@pytest.mark.asyncio
374+
async def test_does_not_raise_on_non_auth_http_errors(self, monkeypatch):
375+
"""_fetch_credential returns {} for non-auth HTTP errors (404, 500, etc.)."""
376+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
377+
monkeypatch.setenv("PROJECT_NAME", "test-project")
378+
379+
ctx = _make_context(session_id="sess-1")
380+
381+
err = HTTPError("http://...", 404, "Not Found", {}, BytesIO(b""))
382+
with patch("urllib.request.urlopen", side_effect=err):
383+
result = await _fetch_credential(ctx, "github")
384+
385+
assert result == {}
386+
387+
@pytest.mark.asyncio
388+
async def test_caller_token_fallback_succeeds_when_bot_token_works(self, monkeypatch):
389+
"""_fetch_credential returns data when caller token 401s but BOT_TOKEN fallback succeeds."""
390+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
391+
monkeypatch.setenv("PROJECT_NAME", "test-project")
392+
monkeypatch.setenv("BOT_TOKEN", "valid-bot-token")
393+
394+
ctx = _make_context(session_id="sess-1", current_user_id="user@example.com")
395+
ctx.caller_token = "Bearer expired-caller-token"
396+
397+
caller_err = HTTPError("http://...", 401, "Unauthorized", {}, BytesIO(b""))
398+
399+
mock_response = MagicMock()
400+
mock_response.read.return_value = json.dumps({"token": "gh-tok-via-bot"}).encode()
401+
mock_response.__enter__ = lambda s: s
402+
mock_response.__exit__ = MagicMock(return_value=False)
403+
404+
with patch("urllib.request.urlopen", side_effect=[caller_err, mock_response]):
405+
result = await _fetch_credential(ctx, "github")
406+
407+
assert result.get("token") == "gh-tok-via-bot"
408+
409+
410+
# ---------------------------------------------------------------------------
411+
# populate_runtime_credentials — raises on auth failure (issue #1043)
412+
# ---------------------------------------------------------------------------
413+
414+
415+
class TestPopulateRuntimeCredentialsAuthFailures:
416+
@pytest.mark.asyncio
417+
async def test_raises_when_github_auth_fails(self, monkeypatch):
418+
"""populate_runtime_credentials raises PermissionError when GitHub auth fails."""
419+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
420+
monkeypatch.setenv("PROJECT_NAME", "test-project")
421+
422+
ctx = _make_context(session_id="sess-1")
423+
424+
async def _fail_github(context, cred_type):
425+
if cred_type == "github":
426+
raise PermissionError("github authentication failed with HTTP 401")
427+
return {}
428+
429+
with patch("ambient_runner.platform.auth._fetch_credential", side_effect=_fail_github):
430+
with pytest.raises(PermissionError, match="Credential refresh failed due to authentication errors"):
431+
await populate_runtime_credentials(ctx)
432+
433+
@pytest.mark.asyncio
434+
async def test_raises_when_multiple_providers_fail(self, monkeypatch):
435+
"""populate_runtime_credentials raises PermissionError listing all auth failures."""
436+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
437+
monkeypatch.setenv("PROJECT_NAME", "test-project")
438+
439+
ctx = _make_context(session_id="sess-1")
440+
441+
async def _fail_all(context, cred_type):
442+
raise PermissionError(f"{cred_type} authentication failed with HTTP 401")
443+
444+
with patch("ambient_runner.platform.auth._fetch_credential", side_effect=_fail_all):
445+
with pytest.raises(PermissionError) as exc_info:
446+
await populate_runtime_credentials(ctx)
447+
448+
msg = str(exc_info.value)
449+
assert "authentication errors" in msg
450+
451+
@pytest.mark.asyncio
452+
async def test_succeeds_when_all_credentials_empty_no_auth_error(self, monkeypatch):
453+
"""populate_runtime_credentials does not raise when credentials are simply missing (not auth failures)."""
454+
monkeypatch.setenv("BACKEND_API_URL", "http://backend.svc.cluster.local/api")
455+
monkeypatch.setenv("PROJECT_NAME", "test-project")
456+
457+
ctx = _make_context(session_id="sess-1")
458+
459+
with patch("ambient_runner.platform.auth._fetch_credential", return_value={}):
460+
# Should not raise — empty credentials just means no integrations configured
461+
await populate_runtime_credentials(ctx)
462+
463+
464+
# ---------------------------------------------------------------------------
465+
# refresh_credentials_tool — reports isError on auth failure (issue #1043)
466+
# ---------------------------------------------------------------------------
467+
468+
469+
class TestRefreshCredentialsTool:
470+
def _make_tool_decorator(self):
471+
"""Create a mock sdk_tool decorator that preserves the function."""
472+
def mock_tool(name, description, schema):
473+
def decorator(func):
474+
return func
475+
return decorator
476+
return mock_tool
477+
478+
@pytest.mark.asyncio
479+
async def test_returns_is_error_on_auth_failure(self):
480+
"""refresh_credentials_tool returns isError=True when populate_runtime_credentials raises PermissionError."""
481+
from ambient_runner.bridges.claude.tools import create_refresh_credentials_tool
482+
483+
mock_context = MagicMock()
484+
tool_fn = create_refresh_credentials_tool(mock_context, self._make_tool_decorator())
485+
486+
with patch(
487+
"ambient_runner.platform.auth.populate_runtime_credentials",
488+
new_callable=AsyncMock,
489+
side_effect=PermissionError("github authentication failed with HTTP 401"),
490+
):
491+
result = await tool_fn({})
492+
493+
assert result.get("isError") is True
494+
assert "github authentication failed" in result["content"][0]["text"]
495+
496+
@pytest.mark.asyncio
497+
async def test_returns_success_on_successful_refresh(self):
498+
"""refresh_credentials_tool returns success message when credentials refresh succeeds."""
499+
from ambient_runner.bridges.claude.tools import create_refresh_credentials_tool
500+
501+
mock_context = MagicMock()
502+
tool_fn = create_refresh_credentials_tool(mock_context, self._make_tool_decorator())
503+
504+
with patch(
505+
"ambient_runner.platform.auth.populate_runtime_credentials",
506+
new_callable=AsyncMock,
507+
), patch(
508+
"ambient_runner.platform.utils.get_active_integrations",
509+
return_value=["github", "jira"],
510+
):
511+
result = await tool_fn({})
512+
513+
assert result.get("isError") is None or result.get("isError") is False
514+
assert "successfully" in result["content"][0]["text"].lower()

0 commit comments

Comments
 (0)