From d75ea24b78c4d1e9cb09a463a8908eea4f770f15 Mon Sep 17 00:00:00 2001 From: user Date: Thu, 23 Apr 2026 12:28:57 -0400 Subject: [PATCH] fix(runner): restore operator-path credential fetch broken by alpha migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The alpha migration (1aa8b428) rewrote _fetch_credential() to require CREDENTIAL_IDS env var and use the control-plane API path. The operator does not inject CREDENTIAL_IDS, so all credential fetches silently returned empty — breaking GitHub, Jira, Google, and GitLab auth in operator-based deployments. Restore the session-scoped backend URL as fallback when CREDENTIAL_IDS is absent, and accept both old (accessToken/apiToken) and new (token) response field formats. Fixes #1438 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .../ambient_runner/platform/auth.py | 74 ++-- .../tests/test_shared_session_credentials.py | 325 ++++++++++++++++++ e2e/cypress/e2e/screenshots.cy.ts | 14 +- 3 files changed, 388 insertions(+), 25 deletions(-) diff --git a/components/runners/ambient-runner/ambient_runner/platform/auth.py b/components/runners/ambient-runner/ambient_runner/platform/auth.py index 438f5c77c..36ddce3f0 100755 --- a/components/runners/ambient-runner/ambient_runner/platform/auth.py +++ b/components/runners/ambient-runner/ambient_runner/platform/auth.py @@ -113,18 +113,27 @@ async def _fetch_credential(context: RunnerContext, credential_type: str) -> dic credential_ids = _json.loads(os.getenv("CREDENTIAL_IDS", "{}")) credential_id = credential_ids.get(credential_type) - if not credential_id: - logger.debug(f"No credential_id for provider {credential_type}; skipping fetch") - return {} - project_id = os.getenv("PROJECT_NAME", "") - if not project_id: - logger.warning("Cannot fetch credentials: PROJECT_NAME not set") - return {} + project = os.getenv("PROJECT_NAME") or os.getenv("AGENTIC_SESSION_NAMESPACE", "") + project = project.strip() - url = ( - f"{base}/api/ambient/v1/projects/{project_id}/credentials/{credential_id}/token" - ) + if credential_id and project: + url = ( + f"{base}/api/ambient/v1/projects/{project}" + f"/credentials/{credential_id}/token" + ) + elif project and context.session_id: + url = ( + f"{base}/projects/{project}" + f"/agentic-sessions/{context.session_id}" + f"/credentials/{credential_type}" + ) + else: + logger.warning( + f"Cannot fetch {credential_type} credentials: missing environment " + f"variables (project={project}, session={context.session_id})" + ) + return {} # Reject non-cluster URLs to prevent token exfiltration via user-overridden env vars parsed = urlparse(base) @@ -411,18 +420,37 @@ async def populate_runtime_credentials(context: RunnerContext) -> None: logger.warning(f"Failed to refresh Google credentials: {google_creds}") if isinstance(google_creds, PermissionError): auth_failures.append(str(google_creds)) - elif google_creds.get("token"): + elif google_creds.get("token") or google_creds.get("accessToken"): try: - sa_json = google_creds["token"] - gac_path = os.getenv("GOOGLE_APPLICATION_CREDENTIALS", "") - if gac_path: - creds_path = Path(gac_path) + if google_creds.get("accessToken"): + creds_dir = _GOOGLE_WORKSPACE_CREDS_FILE.parent + creds_dir.mkdir(parents=True, exist_ok=True) + creds_data = { + "token": google_creds.get("accessToken"), + "refresh_token": google_creds.get("refreshToken", ""), + "token_uri": "https://oauth2.googleapis.com/token", + "client_id": os.getenv("GOOGLE_OAUTH_CLIENT_ID", ""), + "client_secret": os.getenv("GOOGLE_OAUTH_CLIENT_SECRET", ""), + "scopes": google_creds.get("scopes", []), + "expiry": google_creds.get("expiresAt", ""), + } + with open(_GOOGLE_WORKSPACE_CREDS_FILE, "w") as f: + _json.dump(creds_data, f, indent=2) + _GOOGLE_WORKSPACE_CREDS_FILE.chmod(0o600) + logger.info("Updated Google credentials file for workspace-mcp") else: - creds_path = _GOOGLE_WORKSPACE_CREDS_FILE - creds_path.parent.mkdir(parents=True, exist_ok=True) - creds_path.write_text(sa_json) - creds_path.chmod(0o600) - logger.info(f"Updated Google service account credentials at {creds_path}") + sa_json = google_creds["token"] + gac_path = os.getenv("GOOGLE_APPLICATION_CREDENTIALS", "") + if gac_path: + creds_path = Path(gac_path) + else: + creds_path = _GOOGLE_WORKSPACE_CREDS_FILE + creds_path.parent.mkdir(parents=True, exist_ok=True) + creds_path.write_text(sa_json) + creds_path.chmod(0o600) + logger.info( + f"Updated Google service account credentials at {creds_path}" + ) user_email = google_creds.get("email", "") if user_email and user_email != _PLACEHOLDER_EMAIL: @@ -435,9 +463,11 @@ async def populate_runtime_credentials(context: RunnerContext) -> None: logger.warning(f"Failed to refresh Jira credentials: {jira_creds}") if isinstance(jira_creds, PermissionError): auth_failures.append(str(jira_creds)) - elif jira_creds.get("token"): + elif jira_creds.get("token") or jira_creds.get("apiToken"): os.environ["JIRA_URL"] = jira_creds.get("url", "") - os.environ["JIRA_API_TOKEN"] = jira_creds.get("token", "") + os.environ["JIRA_API_TOKEN"] = jira_creds.get("apiToken") or jira_creds.get( + "token", "" + ) os.environ["JIRA_EMAIL"] = jira_creds.get("email", "") logger.info("Updated Jira credentials in environment") diff --git a/components/runners/ambient-runner/tests/test_shared_session_credentials.py b/components/runners/ambient-runner/tests/test_shared_session_credentials.py index 4c8d573f6..070b5934d 100755 --- a/components/runners/ambient-runner/tests/test_shared_session_credentials.py +++ b/components/runners/ambient-runner/tests/test_shared_session_credentials.py @@ -407,6 +407,330 @@ async def test_returns_empty_when_backend_unavailable(self): assert result == {} +# --------------------------------------------------------------------------- +# _fetch_credential — operator path fallback (regression for #1438) +# --------------------------------------------------------------------------- + + +class TestFetchCredentialOperatorPath: + """Without CREDENTIAL_IDS the runner must fall back to the session-scoped + backend URL used by operator-based deployments. + + The alpha migration (1aa8b428) broke this by requiring CREDENTIAL_IDS and + silently returning {} when it was absent, leaving GITHUB_TOKEN empty. + """ + + @pytest.mark.asyncio + async def test_falls_back_to_session_url_without_credential_ids(self): + """_fetch_credential uses /projects/{p}/agentic-sessions/{s}/credentials/{type} + when CREDENTIAL_IDS is not set.""" + captured = {} + + class PathCapture(BaseHTTPRequestHandler): + def do_GET(self): + captured["path"] = self.path + captured["headers"] = dict(self.headers) + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(json.dumps({"token": "gh-tok-operator"}).encode()) + + def log_message(self, fmt, *args): + pass + + server = HTTPServer(("127.0.0.1", 0), PathCapture) + port = server.server_address[1] + thread = Thread(target=server.handle_request, daemon=True) + thread.start() + + try: + with ( + patch.dict( + os.environ, + { + "BACKEND_API_URL": f"http://127.0.0.1:{port}", + "PROJECT_NAME": "my-project", + }, + clear=False, + ), + patch( + "ambient_runner.platform.auth.get_bot_token", + return_value="bot-tok", + ), + ): + os.environ.pop("CREDENTIAL_IDS", None) + ctx = _make_context(session_id="sess-42") + result = await _fetch_credential(ctx, "github") + + assert result.get("token") == "gh-tok-operator" + assert captured["path"] == ( + "/projects/my-project/agentic-sessions/sess-42/credentials/github" + ) + finally: + server.server_close() + thread.join(timeout=2) + + @pytest.mark.asyncio + async def test_prefers_cp_path_when_credential_ids_present(self): + """_fetch_credential uses /api/ambient/v1/... when CREDENTIAL_IDS is set.""" + captured = {} + + class PathCapture(BaseHTTPRequestHandler): + def do_GET(self): + captured["path"] = self.path + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(json.dumps({"token": "gh-tok-cp"}).encode()) + + def log_message(self, fmt, *args): + pass + + server = HTTPServer(("127.0.0.1", 0), PathCapture) + port = server.server_address[1] + thread = Thread(target=server.handle_request, daemon=True) + thread.start() + + try: + with ( + patch.dict( + os.environ, + { + "BACKEND_API_URL": f"http://127.0.0.1:{port}", + "PROJECT_NAME": "my-project", + "CREDENTIAL_IDS": json.dumps({"github": "cred-abc"}), + }, + ), + patch( + "ambient_runner.platform.auth.get_bot_token", + return_value="bot-tok", + ), + ): + ctx = _make_context(session_id="sess-42") + result = await _fetch_credential(ctx, "github") + + assert result.get("token") == "gh-tok-cp" + assert captured["path"] == ( + "/api/ambient/v1/projects/my-project/credentials/cred-abc/token" + ) + finally: + server.server_close() + thread.join(timeout=2) + + @pytest.mark.asyncio + async def test_returns_empty_without_project_or_session(self): + """_fetch_credential returns {} when neither CP IDs nor session context exist.""" + with patch.dict( + os.environ, + {"BACKEND_API_URL": "http://127.0.0.1:1"}, + clear=False, + ): + os.environ.pop("CREDENTIAL_IDS", None) + os.environ.pop("PROJECT_NAME", None) + os.environ.pop("AGENTIC_SESSION_NAMESPACE", None) + ctx = _make_context(session_id="") + result = await _fetch_credential(ctx, "github") + + assert result == {} + + @pytest.mark.asyncio + async def test_operator_path_populates_github_token(self): + """Full round-trip: populate_runtime_credentials sets GITHUB_TOKEN via + the operator session-scoped path when CREDENTIAL_IDS is absent.""" + + class MultiHandler(BaseHTTPRequestHandler): + def do_GET(self): + if "/credentials/github" in self.path: + body = { + "token": "gh-tok-from-backend", + "userName": "dev", + "email": "d@e.com", + } + else: + body = {} + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(json.dumps(body).encode()) + + def log_message(self, fmt, *args): + pass + + server = HTTPServer(("127.0.0.1", 0), MultiHandler) + port = server.server_address[1] + thread = Thread( + target=lambda: [server.handle_request() for _ in range(7)], + daemon=True, + ) + thread.start() + + try: + with ( + patch.dict( + os.environ, + { + "BACKEND_API_URL": f"http://127.0.0.1:{port}", + "PROJECT_NAME": "test-project", + "BOT_TOKEN": "fake-bot", + }, + clear=False, + ), + patch( + "ambient_runner.platform.auth.get_bot_token", + return_value="bot-tok", + ), + ): + os.environ.pop("CREDENTIAL_IDS", None) + ctx = _make_context(session_id="my-session") + + await populate_runtime_credentials(ctx) + + assert os.environ.get("GITHUB_TOKEN") == "gh-tok-from-backend" + + clear_runtime_credentials() + assert "GITHUB_TOKEN" not in os.environ + finally: + server.server_close() + thread.join(timeout=2) + os.environ.pop("GITHUB_TOKEN", None) + os.environ.pop("GIT_USER_NAME", None) + os.environ.pop("GIT_USER_EMAIL", None) + + +class TestPopulateCredentialsResponseFormats: + """Backend (operator) and control-plane return different field names. + populate_runtime_credentials must handle both. Regression for #1438.""" + + @pytest.mark.asyncio + async def test_google_oauth_access_token_format(self): + """Backend returns accessToken/refreshToken — must write OAuth creds file.""" + + class GoogleHandler(BaseHTTPRequestHandler): + def do_GET(self): + if "/credentials/google" in self.path: + body = { + "accessToken": "ya29.access", + "refreshToken": "1//refresh", + "email": "u@example.com", + "scopes": ["drive"], + "expiresAt": "2099-01-01T00:00:00Z", + } + else: + body = {} + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(json.dumps(body).encode()) + + def log_message(self, fmt, *args): + pass + + server = HTTPServer(("127.0.0.1", 0), GoogleHandler) + port = server.server_address[1] + thread = Thread( + target=lambda: [server.handle_request() for _ in range(7)], + daemon=True, + ) + thread.start() + + import tempfile + + tmp_creds = Path(tempfile.mkdtemp()) / "credentials.json" + + try: + with ( + patch.dict( + os.environ, + { + "BACKEND_API_URL": f"http://127.0.0.1:{port}", + "PROJECT_NAME": "test-project", + "BOT_TOKEN": "fake-bot", + }, + clear=False, + ), + patch( + "ambient_runner.platform.auth.get_bot_token", + return_value="bot-tok", + ), + patch( + "ambient_runner.platform.auth._GOOGLE_WORKSPACE_CREDS_FILE", + tmp_creds, + ), + ): + os.environ.pop("CREDENTIAL_IDS", None) + ctx = _make_context(session_id="s1") + await populate_runtime_credentials(ctx) + + assert tmp_creds.exists() + written = json.loads(tmp_creds.read_text()) + assert written["token"] == "ya29.access" + assert written["refresh_token"] == "1//refresh" + finally: + server.server_close() + thread.join(timeout=2) + tmp_creds.unlink(missing_ok=True) + tmp_creds.parent.rmdir() + clear_runtime_credentials() + + @pytest.mark.asyncio + async def test_jira_api_token_format(self): + """Backend returns apiToken — must set JIRA_API_TOKEN.""" + + class JiraHandler(BaseHTTPRequestHandler): + def do_GET(self): + if "/credentials/jira" in self.path: + body = { + "apiToken": "jira-legacy-tok", + "url": "https://jira.example.com", + "email": "j@e.com", + } + else: + body = {} + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(json.dumps(body).encode()) + + def log_message(self, fmt, *args): + pass + + server = HTTPServer(("127.0.0.1", 0), JiraHandler) + port = server.server_address[1] + thread = Thread( + target=lambda: [server.handle_request() for _ in range(7)], + daemon=True, + ) + thread.start() + + try: + with ( + patch.dict( + os.environ, + { + "BACKEND_API_URL": f"http://127.0.0.1:{port}", + "PROJECT_NAME": "test-project", + "BOT_TOKEN": "fake-bot", + }, + clear=False, + ), + patch( + "ambient_runner.platform.auth.get_bot_token", + return_value="bot-tok", + ), + ): + os.environ.pop("CREDENTIAL_IDS", None) + ctx = _make_context(session_id="s1") + await populate_runtime_credentials(ctx) + + assert os.environ.get("JIRA_API_TOKEN") == "jira-legacy-tok" + finally: + server.server_close() + thread.join(timeout=2) + for key in ["JIRA_API_TOKEN", "JIRA_URL", "JIRA_EMAIL"]: + os.environ.pop(key, None) + clear_runtime_credentials() + + # --------------------------------------------------------------------------- # populate_runtime_credentials + clear round-trip # --------------------------------------------------------------------------- @@ -892,6 +1216,7 @@ class TestGhWrapper: @staticmethod def _get_auth_mod(): import ambient_runner.platform.auth as _auth_mod + return _auth_mod def _cleanup(self): diff --git a/e2e/cypress/e2e/screenshots.cy.ts b/e2e/cypress/e2e/screenshots.cy.ts index 25cfdb8c9..68076ef7b 100644 --- a/e2e/cypress/e2e/screenshots.cy.ts +++ b/e2e/cypress/e2e/screenshots.cy.ts @@ -110,6 +110,9 @@ describe('Documentation Screenshots', () => { runSetupStep(step) } + cy.get('button[aria-label="Toggle theme"]', { timeout: 10000 }).should('be.visible') + cy.wait(500) + setTheme('light') waitForFonts() cy.screenshot(`${entry.id}-light`, { overwrite: true, capture: 'viewport' }) @@ -123,9 +126,14 @@ describe('Documentation Screenshots', () => { function setTheme(theme: 'light' | 'dark'): void { const label = theme === 'dark' ? 'Switch to dark theme' : 'Switch to light theme' - cy.get('button[aria-label="Toggle theme"]', { timeout: 10000 }).first().should('be.visible').click({ force: true }) - // 10 s timeout: slow CI environments can take > 5 s for Radix to mount the dropdown content - cy.get(`[aria-label="${label}"]`, { timeout: 10000 }).first().click({ force: true }) + cy.get('button[aria-label="Toggle theme"]', { timeout: 10000 }) + .first() + .should('be.visible') + .click() + cy.get(`[aria-label="${label}"]`, { timeout: 10000 }) + .should('be.visible') + .first() + .click() if (theme === 'dark') { cy.get('html').should('have.class', 'dark') } else {