From 4f0380f2139760a891bd9b7791d4bb461c018d8a Mon Sep 17 00:00:00 2001 From: natifridman Date: Wed, 11 Feb 2026 10:02:28 +0000 Subject: [PATCH 1/4] fix: derive git identity from GitHub/GitLab credentials at runtime Previously, git commits in sessions were attributed to "Ambient Code Bot" because git user.name and user.email were not configured from the user's GitHub/GitLab credentials. This fix: - Enhances GitHub/GitLab credential endpoints to fetch user identity from their respective APIs and return userName, email, and provider fields - Updates the runner to configure git identity (both git config and environment variables) when populating runtime credentials - GitHub identity takes precedence over GitLab when both are configured - Falls back to bot identity when no credentials have user info Fixes: GitHub credentials aren't mounted to session Also: Adds provider field to distinguish GitHub vs GitLab credentials Co-Authored-By: Claude Opus 4.5 --- .../backend/handlers/runtime_credentials.go | 142 +++++- .../handlers/runtime_credentials_test.go | 129 ++++++ components/runners/claude-code-runner/auth.py | 114 ++++- .../tests/test_git_identity.py | 403 ++++++++++++++++++ 4 files changed, 766 insertions(+), 22 deletions(-) create mode 100644 components/backend/handlers/runtime_credentials_test.go create mode 100644 components/runners/claude-code-runner/tests/test_git_identity.py diff --git a/components/backend/handlers/runtime_credentials.go b/components/backend/handlers/runtime_credentials.go index 33f4d9e04..e0fae6053 100644 --- a/components/backend/handlers/runtime_credentials.go +++ b/components/backend/handlers/runtime_credentials.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "io" "log" "net/http" "net/url" @@ -81,7 +82,16 @@ func GetGitHubTokenForSession(c *gin.Context) { return } - c.JSON(http.StatusOK, gin.H{"token": token}) + // Fetch user identity from GitHub API for git config + // Fix for: GitHub credentials aren't mounted to session - need git identity + userName, userEmail := fetchGitHubUserIdentity(c.Request.Context(), token) + + c.JSON(http.StatusOK, gin.H{ + "token": token, + "userName": userName, + "email": userEmail, + "provider": "github", + }) } // GetGoogleCredentialsForSession handles GET /api/projects/:project/agentic-sessions/:session/credentials/google @@ -296,9 +306,16 @@ func GetGitLabTokenForSession(c *gin.Context) { return } + // Fetch user identity from GitLab API for git config + // Fix for: need to distinguish between GitHub and GitLab providers + userName, userEmail := fetchGitLabUserIdentity(c.Request.Context(), creds.Token, creds.InstanceURL) + c.JSON(http.StatusOK, gin.H{ "token": creds.Token, "instanceUrl": creds.InstanceURL, + "userName": userName, + "email": userEmail, + "provider": "gitlab", }) } @@ -373,3 +390,126 @@ func exchangeOAuthToken(ctx context.Context, tokenURL string, payload map[string return &tokenResp, nil } + +// fetchGitHubUserIdentity fetches user name and email from GitHub API +// Returns the user's name (or login as fallback) and email for git config +func fetchGitHubUserIdentity(ctx context.Context, token string) (userName, email string) { + if token == "" { + return "", "" + } + + client := &http.Client{Timeout: 10 * time.Second} + req, err := http.NewRequestWithContext(ctx, "GET", "https://api.github.com/user", nil) + if err != nil { + log.Printf("Failed to create GitHub user request: %v", err) + return "", "" + } + + req.Header.Set("Authorization", "token "+token) + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + resp, err := client.Do(req) + if err != nil { + log.Printf("Failed to fetch GitHub user: %v", err) + return "", "" + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + if resp.StatusCode == http.StatusForbidden { + log.Printf("GitHub API /user returned 403 (token may lack 'read:user' scope)") + } else { + log.Printf("GitHub API /user returned status %d", resp.StatusCode) + } + return "", "" + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + log.Printf("Failed to read GitHub user response: %v", err) + return "", "" + } + + var ghUser struct { + Login string `json:"login"` + Name string `json:"name"` + Email string `json:"email"` + } + if err := json.Unmarshal(body, &ghUser); err != nil { + log.Printf("Failed to parse GitHub user response: %v", err) + return "", "" + } + + // Use Name if available, fall back to Login + userName = ghUser.Name + if userName == "" { + userName = ghUser.Login + } + email = ghUser.Email + + log.Printf("Fetched GitHub user identity: name=%q email=%q", userName, email) + return userName, email +} + +// fetchGitLabUserIdentity fetches user name and email from GitLab API +// Returns the user's name and email for git config +func fetchGitLabUserIdentity(ctx context.Context, token, instanceURL string) (userName, email string) { + if token == "" { + return "", "" + } + + // Default to gitlab.com if no instance URL + apiURL := "https://gitlab.com/api/v4/user" + if instanceURL != "" && instanceURL != "https://gitlab.com" { + apiURL = strings.TrimSuffix(instanceURL, "/") + "/api/v4/user" + } + + client := &http.Client{Timeout: 10 * time.Second} + req, err := http.NewRequestWithContext(ctx, "GET", apiURL, nil) + if err != nil { + log.Printf("Failed to create GitLab user request: %v", err) + return "", "" + } + + req.Header.Set("PRIVATE-TOKEN", token) + req.Header.Set("Accept", "application/json") + + resp, err := client.Do(req) + if err != nil { + log.Printf("Failed to fetch GitLab user: %v", err) + return "", "" + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + log.Printf("GitLab API /user returned status %d", resp.StatusCode) + return "", "" + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + log.Printf("Failed to read GitLab user response: %v", err) + return "", "" + } + + var glUser struct { + Username string `json:"username"` + Name string `json:"name"` + Email string `json:"email"` + } + if err := json.Unmarshal(body, &glUser); err != nil { + log.Printf("Failed to parse GitLab user response: %v", err) + return "", "" + } + + // Use Name if available, fall back to Username + userName = glUser.Name + if userName == "" { + userName = glUser.Username + } + email = glUser.Email + + log.Printf("Fetched GitLab user identity: name=%q email=%q", userName, email) + return userName, email +} diff --git a/components/backend/handlers/runtime_credentials_test.go b/components/backend/handlers/runtime_credentials_test.go new file mode 100644 index 000000000..a15df60c2 --- /dev/null +++ b/components/backend/handlers/runtime_credentials_test.go @@ -0,0 +1,129 @@ +//go:build test + +package handlers + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// TestRuntimeCredentials runs the Ginkgo test suite +func TestRuntimeCredentials(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Runtime Credentials Suite") +} + +var _ = Describe("Runtime Credentials - Git Identity", func() { + + Describe("fetchGitHubUserIdentity", func() { + var ( + server *httptest.Server + ) + + AfterEach(func() { + if server != nil { + server.Close() + } + }) + + Context("when GitHub API returns valid user data", func() { + It("should return user name and email", func() { + // Mock GitHub API response + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Expect(r.URL.Path).To(Equal("/user")) + Expect(r.Header.Get("Authorization")).To(Equal("token test-token")) + Expect(r.Header.Get("Accept")).To(Equal("application/vnd.github+json")) + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]string{ + "login": "testuser", + "name": "Test User", + "email": "test@example.com", + }) + })) + + // Note: We can't easily test this without modifying the function to accept + // a custom HTTP client or base URL. This test documents the expected behavior. + // In production, consider using dependency injection for HTTP clients. + + // Test with empty token returns empty strings + name, email := fetchGitHubUserIdentity(context.Background(), "") + Expect(name).To(Equal("")) + Expect(email).To(Equal("")) + }) + }) + + Context("when token is empty", func() { + It("should return empty strings without making API call", func() { + name, email := fetchGitHubUserIdentity(context.Background(), "") + Expect(name).To(Equal("")) + Expect(email).To(Equal("")) + }) + }) + }) + + Describe("fetchGitLabUserIdentity", func() { + Context("when token is empty", func() { + It("should return empty strings without making API call", func() { + name, email := fetchGitLabUserIdentity(context.Background(), "", "") + Expect(name).To(Equal("")) + Expect(email).To(Equal("")) + }) + }) + + Context("when instance URL is provided", func() { + It("should construct correct API URL for self-hosted GitLab", func() { + // Test with empty token to verify no API call is made + name, email := fetchGitLabUserIdentity(context.Background(), "", "https://gitlab.mycompany.com") + Expect(name).To(Equal("")) + Expect(email).To(Equal("")) + }) + }) + }) + + Describe("Provider field in API responses", func() { + Context("GitHub credentials endpoint", func() { + It("should include provider field set to 'github'", func() { + // This tests the response structure defined in GetGitHubTokenForSession + // The actual endpoint test requires full integration setup + // Here we document the expected response fields: + // - token: string + // - userName: string + // - email: string + // - provider: "github" + Skip("Requires full integration test setup with mock K8s and session") + }) + }) + + Context("GitLab credentials endpoint", func() { + It("should include provider field set to 'gitlab'", func() { + // This tests the response structure defined in GetGitLabTokenForSession + // The actual endpoint test requires full integration setup + // Here we document the expected response fields: + // - token: string + // - instanceUrl: string + // - userName: string + // - email: string + // - provider: "gitlab" + Skip("Requires full integration test setup with mock K8s and session") + }) + }) + }) + + Describe("Git Identity Precedence", func() { + Context("when both GitHub and GitLab credentials are available", func() { + It("should document that GitHub takes precedence in the runner", func() { + // This is tested in the Python runner tests + // The backend returns identity from each provider separately + // The runner decides precedence when configuring git + Skip("Precedence logic is in Python runner - see test_git_identity.py") + }) + }) + }) +}) diff --git a/components/runners/claude-code-runner/auth.py b/components/runners/claude-code-runner/auth.py index 27d4cc77d..d34220ff2 100644 --- a/components/runners/claude-code-runner/auth.py +++ b/components/runners/claude-code-runner/auth.py @@ -165,13 +165,24 @@ def _do_req(): return {} +async def fetch_github_credentials(context: RunnerContext) -> dict: + """Fetch GitHub credentials from backend API (always fresh — PAT or minted App token). + + Returns dict with: token, userName, email, provider + """ + data = await _fetch_credential(context, "github") + if data.get("token"): + logger.info( + f"Using fresh GitHub credentials from backend " + f"(user: {data.get('userName', 'unknown')}, email: {data.get('email', 'unknown')})" + ) + return data + + async def fetch_github_token(context: RunnerContext) -> str: """Fetch GitHub token from backend API (always fresh — PAT or minted App token).""" - data = await _fetch_credential(context, "github") - token = data.get("token", "") - if token: - logger.info("Using fresh GitHub token from backend") - return token + data = await fetch_github_credentials(context) + return data.get("token", "") async def fetch_google_credentials(context: RunnerContext) -> dict: @@ -196,16 +207,25 @@ async def fetch_jira_credentials(context: RunnerContext) -> dict: return data -async def fetch_gitlab_token(context: RunnerContext) -> str: - """Fetch GitLab token from backend API.""" +async def fetch_gitlab_credentials(context: RunnerContext) -> dict: + """Fetch GitLab credentials from backend API. + + Returns dict with: token, instanceUrl, userName, email, provider + """ data = await _fetch_credential(context, "gitlab") - token = data.get("token", "") - if token: + if data.get("token"): logger.info( - f"Using fresh GitLab token from backend " - f"(instance: {data.get('instanceUrl', 'unknown')})" + f"Using fresh GitLab credentials from backend " + f"(instance: {data.get('instanceUrl', 'unknown')}, " + f"user: {data.get('userName', 'unknown')}, email: {data.get('email', 'unknown')})" ) - return token + return data + + +async def fetch_gitlab_token(context: RunnerContext) -> str: + """Fetch GitLab token from backend API.""" + data = await fetch_gitlab_credentials(context) + return data.get("token", "") async def fetch_token_for_url(context: RunnerContext, url: str) -> str: @@ -239,9 +259,14 @@ async def populate_runtime_credentials(context: RunnerContext) -> None: """Fetch all credentials from backend and populate environment variables. Called before each SDK run to ensure MCP servers have fresh tokens. + Also configures git identity from GitHub/GitLab credentials. """ logger.info("Fetching fresh credentials from backend API...") + # Track git identity from provider credentials + git_user_name = "" + git_user_email = "" + # Google credentials google_creds = await fetch_google_credentials(context) if google_creds.get("accessToken"): @@ -282,21 +307,68 @@ async def populate_runtime_credentials(context: RunnerContext) -> None: os.environ["JIRA_EMAIL"] = jira_creds.get("email", "") logger.info("✓ Updated Jira credentials in environment") - # GitLab token - gitlab_token = await fetch_gitlab_token(context) - if gitlab_token: - os.environ["GITLAB_TOKEN"] = gitlab_token + # GitLab credentials (with user identity) + gitlab_creds = await fetch_gitlab_credentials(context) + if gitlab_creds.get("token"): + os.environ["GITLAB_TOKEN"] = gitlab_creds["token"] logger.info("✓ Updated GitLab token in environment") - - # GitHub token - github_token = await fetch_github_token(context) - if github_token: - os.environ["GITHUB_TOKEN"] = github_token + # Use GitLab identity if available (can be overridden by GitHub below) + if gitlab_creds.get("userName"): + git_user_name = gitlab_creds["userName"] + if gitlab_creds.get("email"): + git_user_email = gitlab_creds["email"] + + # GitHub credentials (with user identity - takes precedence) + github_creds = await fetch_github_credentials(context) + if github_creds.get("token"): + os.environ["GITHUB_TOKEN"] = github_creds["token"] logger.info("✓ Updated GitHub token in environment") + # GitHub identity takes precedence over GitLab + if github_creds.get("userName"): + git_user_name = github_creds["userName"] + if github_creds.get("email"): + git_user_email = github_creds["email"] + + # Configure git identity from provider credentials + # Fix for: GitHub credentials aren't mounted to session - need git identity + await configure_git_identity(git_user_name, git_user_email) logger.info("Runtime credentials populated successfully") +async def configure_git_identity(user_name: str, user_email: str) -> None: + """Configure git user.name and user.email from provider credentials. + + Falls back to defaults if not provided. This ensures commits are + attributed to the correct user rather than the default bot identity. + """ + import subprocess + + # Use provided values or fall back to defaults + final_name = user_name.strip() if user_name else "Ambient Code Bot" + final_email = user_email.strip() if user_email else "bot@ambient-code.local" + + # Also set environment variables for git operations in subprocesses + os.environ["GIT_USER_NAME"] = final_name + os.environ["GIT_USER_EMAIL"] = final_email + + try: + # Configure git globally for this session + subprocess.run( + ["git", "config", "--global", "user.name", final_name], + capture_output=True, + timeout=5, + ) + subprocess.run( + ["git", "config", "--global", "user.email", final_email], + capture_output=True, + timeout=5, + ) + logger.info(f"✓ Configured git identity: {final_name} <{final_email}>") + except Exception as e: + logger.warning(f"Failed to configure git identity: {e}") + + async def fetch_github_token_legacy(context: RunnerContext) -> str: """Legacy method — kept for backward compatibility.""" base = os.getenv("BACKEND_API_URL", "").rstrip("/") diff --git a/components/runners/claude-code-runner/tests/test_git_identity.py b/components/runners/claude-code-runner/tests/test_git_identity.py new file mode 100644 index 000000000..74956a633 --- /dev/null +++ b/components/runners/claude-code-runner/tests/test_git_identity.py @@ -0,0 +1,403 @@ +""" +Unit tests for Git identity configuration from provider credentials. + +Tests the configure_git_identity() function and the credential fetching +functions that now return user identity (userName, email) in addition to tokens. + +Bug Fix: GitHub credentials aren't mounted to session - need git identity + Also adds provider distinction (github vs gitlab) +""" + +import asyncio +import os +import subprocess +import sys +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +# Add parent directory to path to import auth module +sys.path.insert(0, str(Path(__file__).parent.parent)) + + +class TestConfigureGitIdentity: + """Test configure_git_identity function.""" + + @pytest.fixture(autouse=True) + def setup_env(self): + """Save and restore environment variables.""" + original_env = os.environ.copy() + yield + os.environ.clear() + os.environ.update(original_env) + + @pytest.mark.asyncio + async def test_configure_git_identity_with_valid_credentials(self): + """Test git identity is configured with provided user name and email.""" + from auth import configure_git_identity + + with patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + + await configure_git_identity("John Doe", "john@example.com") + + # Verify git config commands were called + assert mock_run.call_count == 2 + + # Check user.name was set + name_call = mock_run.call_args_list[0] + assert "user.name" in name_call[0][0] + assert "John Doe" in name_call[0][0] + + # Check user.email was set + email_call = mock_run.call_args_list[1] + assert "user.email" in email_call[0][0] + assert "john@example.com" in email_call[0][0] + + # Verify environment variables were set + assert os.environ.get("GIT_USER_NAME") == "John Doe" + assert os.environ.get("GIT_USER_EMAIL") == "john@example.com" + + @pytest.mark.asyncio + async def test_configure_git_identity_falls_back_to_defaults(self): + """Test git identity uses defaults when credentials are empty.""" + from auth import configure_git_identity + + with patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + + await configure_git_identity("", "") + + # Verify defaults were used + assert os.environ.get("GIT_USER_NAME") == "Ambient Code Bot" + assert os.environ.get("GIT_USER_EMAIL") == "bot@ambient-code.local" + + # Check git config was called with defaults + name_call = mock_run.call_args_list[0] + assert "Ambient Code Bot" in name_call[0][0] + + @pytest.mark.asyncio + async def test_configure_git_identity_strips_whitespace(self): + """Test git identity strips whitespace from values.""" + from auth import configure_git_identity + + with patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + + await configure_git_identity(" Jane Doe ", " jane@example.com ") + + assert os.environ.get("GIT_USER_NAME") == "Jane Doe" + assert os.environ.get("GIT_USER_EMAIL") == "jane@example.com" + + @pytest.mark.asyncio + async def test_configure_git_identity_handles_subprocess_error(self): + """Test git identity handles subprocess errors gracefully.""" + from auth import configure_git_identity + + with patch("subprocess.run") as mock_run: + mock_run.side_effect = subprocess.TimeoutExpired("git", 5) + + # Should not raise, just log warning + await configure_git_identity("Test User", "test@example.com") + + # Environment variables should still be set even if git config fails + assert os.environ.get("GIT_USER_NAME") == "Test User" + assert os.environ.get("GIT_USER_EMAIL") == "test@example.com" + + +class TestFetchGitHubCredentials: + """Test fetch_github_credentials function returns identity.""" + + @pytest.fixture(autouse=True) + def setup_env(self): + """Set up environment variables.""" + original_env = os.environ.copy() + os.environ["BACKEND_API_URL"] = "http://test-backend:8080/api" + os.environ["PROJECT_NAME"] = "test-project" + yield + os.environ.clear() + os.environ.update(original_env) + + @pytest.mark.asyncio + async def test_fetch_github_credentials_returns_identity(self): + """Test that fetch_github_credentials returns userName and email.""" + from auth import fetch_github_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + mock_response = { + "token": "ghp_test_token", + "userName": "Test User", + "email": "test@github.com", + "provider": "github", + } + + with patch("auth._fetch_credential", new_callable=AsyncMock) as mock_fetch: + mock_fetch.return_value = mock_response + + result = await fetch_github_credentials(mock_context) + + assert result["token"] == "ghp_test_token" + assert result["userName"] == "Test User" + assert result["email"] == "test@github.com" + assert result["provider"] == "github" + + @pytest.mark.asyncio + async def test_fetch_github_token_delegates_to_fetch_github_credentials(self): + """Test that fetch_github_token uses fetch_github_credentials.""" + from auth import fetch_github_token + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + with patch("auth.fetch_github_credentials", new_callable=AsyncMock) as mock_fetch: + mock_fetch.return_value = {"token": "ghp_test_token", "userName": "Test"} + + result = await fetch_github_token(mock_context) + + assert result == "ghp_test_token" + mock_fetch.assert_called_once_with(mock_context) + + +class TestFetchGitLabCredentials: + """Test fetch_gitlab_credentials function returns identity.""" + + @pytest.fixture(autouse=True) + def setup_env(self): + """Set up environment variables.""" + original_env = os.environ.copy() + os.environ["BACKEND_API_URL"] = "http://test-backend:8080/api" + os.environ["PROJECT_NAME"] = "test-project" + yield + os.environ.clear() + os.environ.update(original_env) + + @pytest.mark.asyncio + async def test_fetch_gitlab_credentials_returns_identity(self): + """Test that fetch_gitlab_credentials returns userName and email.""" + from auth import fetch_gitlab_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + mock_response = { + "token": "glpat-test_token", + "instanceUrl": "https://gitlab.com", + "userName": "Test GitLab User", + "email": "test@gitlab.com", + "provider": "gitlab", + } + + with patch("auth._fetch_credential", new_callable=AsyncMock) as mock_fetch: + mock_fetch.return_value = mock_response + + result = await fetch_gitlab_credentials(mock_context) + + assert result["token"] == "glpat-test_token" + assert result["instanceUrl"] == "https://gitlab.com" + assert result["userName"] == "Test GitLab User" + assert result["email"] == "test@gitlab.com" + assert result["provider"] == "gitlab" + + @pytest.mark.asyncio + async def test_fetch_gitlab_token_delegates_to_fetch_gitlab_credentials(self): + """Test that fetch_gitlab_token uses fetch_gitlab_credentials.""" + from auth import fetch_gitlab_token + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + with patch("auth.fetch_gitlab_credentials", new_callable=AsyncMock) as mock_fetch: + mock_fetch.return_value = {"token": "glpat-test_token"} + + result = await fetch_gitlab_token(mock_context) + + assert result == "glpat-test_token" + mock_fetch.assert_called_once_with(mock_context) + + +class TestPopulateRuntimeCredentialsGitIdentity: + """Test that populate_runtime_credentials configures git identity.""" + + @pytest.fixture(autouse=True) + def setup_env(self): + """Set up environment variables.""" + original_env = os.environ.copy() + os.environ["BACKEND_API_URL"] = "http://test-backend:8080/api" + os.environ["PROJECT_NAME"] = "test-project" + yield + os.environ.clear() + os.environ.update(original_env) + + @pytest.mark.asyncio + async def test_git_identity_from_github(self): + """Test git identity is configured from GitHub credentials.""" + from auth import populate_runtime_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + github_creds = { + "token": "ghp_test", + "userName": "GitHub User", + "email": "user@github.com", + "provider": "github", + } + + with patch("auth.fetch_google_credentials", new_callable=AsyncMock) as mock_google, \ + patch("auth.fetch_jira_credentials", new_callable=AsyncMock) as mock_jira, \ + patch("auth.fetch_gitlab_credentials", new_callable=AsyncMock) as mock_gitlab, \ + patch("auth.fetch_github_credentials", new_callable=AsyncMock) as mock_github, \ + patch("auth.configure_git_identity", new_callable=AsyncMock) as mock_config: + + mock_google.return_value = {} + mock_jira.return_value = {} + mock_gitlab.return_value = {} + mock_github.return_value = github_creds + + await populate_runtime_credentials(mock_context) + + # Verify configure_git_identity was called with GitHub user info + mock_config.assert_called_once_with("GitHub User", "user@github.com") + + @pytest.mark.asyncio + async def test_git_identity_from_gitlab_when_no_github(self): + """Test git identity is configured from GitLab when GitHub not available.""" + from auth import populate_runtime_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + gitlab_creds = { + "token": "glpat-test", + "userName": "GitLab User", + "email": "user@gitlab.com", + "provider": "gitlab", + } + + with patch("auth.fetch_google_credentials", new_callable=AsyncMock) as mock_google, \ + patch("auth.fetch_jira_credentials", new_callable=AsyncMock) as mock_jira, \ + patch("auth.fetch_gitlab_credentials", new_callable=AsyncMock) as mock_gitlab, \ + patch("auth.fetch_github_credentials", new_callable=AsyncMock) as mock_github, \ + patch("auth.configure_git_identity", new_callable=AsyncMock) as mock_config: + + mock_google.return_value = {} + mock_jira.return_value = {} + mock_gitlab.return_value = gitlab_creds + mock_github.return_value = {} # No GitHub credentials + + await populate_runtime_credentials(mock_context) + + # Verify configure_git_identity was called with GitLab user info + mock_config.assert_called_once_with("GitLab User", "user@gitlab.com") + + @pytest.mark.asyncio + async def test_github_takes_precedence_over_gitlab(self): + """Test GitHub identity takes precedence when both are available.""" + from auth import populate_runtime_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + gitlab_creds = { + "token": "glpat-test", + "userName": "GitLab User", + "email": "user@gitlab.com", + "provider": "gitlab", + } + github_creds = { + "token": "ghp_test", + "userName": "GitHub User", + "email": "user@github.com", + "provider": "github", + } + + with patch("auth.fetch_google_credentials", new_callable=AsyncMock) as mock_google, \ + patch("auth.fetch_jira_credentials", new_callable=AsyncMock) as mock_jira, \ + patch("auth.fetch_gitlab_credentials", new_callable=AsyncMock) as mock_gitlab, \ + patch("auth.fetch_github_credentials", new_callable=AsyncMock) as mock_github, \ + patch("auth.configure_git_identity", new_callable=AsyncMock) as mock_config: + + mock_google.return_value = {} + mock_jira.return_value = {} + mock_gitlab.return_value = gitlab_creds + mock_github.return_value = github_creds + + await populate_runtime_credentials(mock_context) + + # GitHub should take precedence + mock_config.assert_called_once_with("GitHub User", "user@github.com") + + @pytest.mark.asyncio + async def test_defaults_when_no_credentials(self): + """Test defaults are used when no credentials have identity.""" + from auth import populate_runtime_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + with patch("auth.fetch_google_credentials", new_callable=AsyncMock) as mock_google, \ + patch("auth.fetch_jira_credentials", new_callable=AsyncMock) as mock_jira, \ + patch("auth.fetch_gitlab_credentials", new_callable=AsyncMock) as mock_gitlab, \ + patch("auth.fetch_github_credentials", new_callable=AsyncMock) as mock_github, \ + patch("auth.configure_git_identity", new_callable=AsyncMock) as mock_config: + + mock_google.return_value = {} + mock_jira.return_value = {} + mock_gitlab.return_value = {} + mock_github.return_value = {} + + await populate_runtime_credentials(mock_context) + + # Should be called with empty strings (configure_git_identity handles defaults) + mock_config.assert_called_once_with("", "") + + +class TestProviderDistinction: + """Test provider field is correctly returned and used.""" + + @pytest.mark.asyncio + async def test_github_provider_field(self): + """Test GitHub credentials include provider='github'.""" + from auth import fetch_github_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + with patch("auth._fetch_credential", new_callable=AsyncMock) as mock_fetch: + mock_fetch.return_value = { + "token": "ghp_test", + "provider": "github", + } + + result = await fetch_github_credentials(mock_context) + assert result.get("provider") == "github" + + @pytest.mark.asyncio + async def test_gitlab_provider_field(self): + """Test GitLab credentials include provider='gitlab'.""" + from auth import fetch_gitlab_credentials + from context import RunnerContext + + mock_context = MagicMock(spec=RunnerContext) + mock_context.session_id = "test-session" + + with patch("auth._fetch_credential", new_callable=AsyncMock) as mock_fetch: + mock_fetch.return_value = { + "token": "glpat-test", + "provider": "gitlab", + } + + result = await fetch_gitlab_credentials(mock_context) + assert result.get("provider") == "gitlab" From 359ab627106731d2e00e854f4d08ea6fe69e7d31 Mon Sep 17 00:00:00 2001 From: natifridman Date: Wed, 11 Feb 2026 10:23:29 +0000 Subject: [PATCH 2/4] fix: address code review feedback for git identity PR - Redact email from logs to avoid PII exposure (use hasEmail boolean) - Add structured logging context with session/project info - Improve Python exception handling with specific exception types Co-Authored-By: Claude Opus 4.5 --- components/backend/handlers/runtime_credentials.go | 10 ++++++++-- components/runners/claude-code-runner/auth.py | 8 +++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/components/backend/handlers/runtime_credentials.go b/components/backend/handlers/runtime_credentials.go index e0fae6053..64e1b1580 100644 --- a/components/backend/handlers/runtime_credentials.go +++ b/components/backend/handlers/runtime_credentials.go @@ -85,6 +85,9 @@ func GetGitHubTokenForSession(c *gin.Context) { // Fetch user identity from GitHub API for git config // Fix for: GitHub credentials aren't mounted to session - need git identity userName, userEmail := fetchGitHubUserIdentity(c.Request.Context(), token) + if userName != "" { + log.Printf("Returning GitHub credentials with identity for session %s/%s", project, session) + } c.JSON(http.StatusOK, gin.H{ "token": token, @@ -309,6 +312,9 @@ func GetGitLabTokenForSession(c *gin.Context) { // Fetch user identity from GitLab API for git config // Fix for: need to distinguish between GitHub and GitLab providers userName, userEmail := fetchGitLabUserIdentity(c.Request.Context(), creds.Token, creds.InstanceURL) + if userName != "" { + log.Printf("Returning GitLab credentials with identity for session %s/%s", project, session) + } c.JSON(http.StatusOK, gin.H{ "token": creds.Token, @@ -448,7 +454,7 @@ func fetchGitHubUserIdentity(ctx context.Context, token string) (userName, email } email = ghUser.Email - log.Printf("Fetched GitHub user identity: name=%q email=%q", userName, email) + log.Printf("Fetched GitHub user identity: name=%q hasEmail=%t", userName, email != "") return userName, email } @@ -510,6 +516,6 @@ func fetchGitLabUserIdentity(ctx context.Context, token, instanceURL string) (us } email = glUser.Email - log.Printf("Fetched GitLab user identity: name=%q email=%q", userName, email) + log.Printf("Fetched GitLab user identity: name=%q hasEmail=%t", userName, email != "") return userName, email } diff --git a/components/runners/claude-code-runner/auth.py b/components/runners/claude-code-runner/auth.py index d34220ff2..0db022e20 100644 --- a/components/runners/claude-code-runner/auth.py +++ b/components/runners/claude-code-runner/auth.py @@ -174,7 +174,7 @@ async def fetch_github_credentials(context: RunnerContext) -> dict: if data.get("token"): logger.info( f"Using fresh GitHub credentials from backend " - f"(user: {data.get('userName', 'unknown')}, email: {data.get('email', 'unknown')})" + f"(user: {data.get('userName', 'unknown')}, hasEmail: {bool(data.get('email'))})" ) return data @@ -217,7 +217,7 @@ async def fetch_gitlab_credentials(context: RunnerContext) -> dict: logger.info( f"Using fresh GitLab credentials from backend " f"(instance: {data.get('instanceUrl', 'unknown')}, " - f"user: {data.get('userName', 'unknown')}, email: {data.get('email', 'unknown')})" + f"user: {data.get('userName', 'unknown')}, hasEmail: {bool(data.get('email'))})" ) return data @@ -365,8 +365,10 @@ async def configure_git_identity(user_name: str, user_email: str) -> None: timeout=5, ) logger.info(f"✓ Configured git identity: {final_name} <{final_email}>") - except Exception as e: + except (subprocess.TimeoutExpired, subprocess.CalledProcessError, FileNotFoundError) as e: logger.warning(f"Failed to configure git identity: {e}") + except Exception as e: + logger.error(f"Unexpected error configuring git identity: {e}", exc_info=True) async def fetch_github_token_legacy(context: RunnerContext) -> str: From 3754302009cede800d88d2636c54d9246dc033ae Mon Sep 17 00:00:00 2001 From: Nati Fridman Date: Thu, 12 Feb 2026 12:59:52 +0200 Subject: [PATCH 3/4] fix: remove duplicate RunSpecs call causing CI test failure The runtime_credentials_test.go file had its own RunSpecs entry point, which conflicted with the main suite in backend_unit_test.go. Ginkgo does not support calling RunSpecs more than once per package. Co-Authored-By: Claude Opus 4.6 (1M context) --- components/backend/handlers/runtime_credentials_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/components/backend/handlers/runtime_credentials_test.go b/components/backend/handlers/runtime_credentials_test.go index a15df60c2..63c1ac993 100644 --- a/components/backend/handlers/runtime_credentials_test.go +++ b/components/backend/handlers/runtime_credentials_test.go @@ -7,18 +7,11 @@ import ( "encoding/json" "net/http" "net/http/httptest" - "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -// TestRuntimeCredentials runs the Ginkgo test suite -func TestRuntimeCredentials(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Runtime Credentials Suite") -} - var _ = Describe("Runtime Credentials - Git Identity", func() { Describe("fetchGitHubUserIdentity", func() { From b2f9a7643c29861fc4604d2c40e6057a8c10912c Mon Sep 17 00:00:00 2001 From: Nati Fridman Date: Thu, 12 Feb 2026 13:19:07 +0200 Subject: [PATCH 4/4] fix: address PR #617 code review feedback for git identity Extract HTTP timeout constant, use Bearer token format for GitHub API, read error response bodies for debugging, add context cancellation checks, and extract placeholder email constant in runner. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../backend/handlers/runtime_credentials.go | 27 ++++++++++++++----- .../handlers/runtime_credentials_test.go | 2 +- components/runners/claude-code-runner/auth.py | 5 +++- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/components/backend/handlers/runtime_credentials.go b/components/backend/handlers/runtime_credentials.go index 64e1b1580..7a9bce47b 100644 --- a/components/backend/handlers/runtime_credentials.go +++ b/components/backend/handlers/runtime_credentials.go @@ -20,6 +20,9 @@ import ( "k8s.io/client-go/kubernetes" ) +// identityAPITimeout is the HTTP client timeout for GitHub/GitLab user identity API calls. +const identityAPITimeout = 10 * time.Second + // GetGitHubTokenForSession handles GET /api/projects/:project/agentic-sessions/:session/credentials/github // Returns PAT (priority 1) or freshly minted GitHub App token (priority 2) func GetGitHubTokenForSession(c *gin.Context) { @@ -378,7 +381,7 @@ func exchangeOAuthToken(ctx context.Context, tokenURL string, payload map[string form.Set(k, v) } - client := &http.Client{Timeout: 10 * time.Second} + client := &http.Client{Timeout: identityAPITimeout} resp, err := client.Post(tokenURL, "application/x-www-form-urlencoded", strings.NewReader(form.Encode())) if err != nil { return nil, fmt.Errorf("request failed: %w", err) @@ -404,14 +407,18 @@ func fetchGitHubUserIdentity(ctx context.Context, token string) (userName, email return "", "" } - client := &http.Client{Timeout: 10 * time.Second} + if ctx.Err() != nil { + return "", "" + } + + client := &http.Client{Timeout: identityAPITimeout} req, err := http.NewRequestWithContext(ctx, "GET", "https://api.github.com/user", nil) if err != nil { log.Printf("Failed to create GitHub user request: %v", err) return "", "" } - req.Header.Set("Authorization", "token "+token) + req.Header.Set("Authorization", "Bearer "+token) req.Header.Set("Accept", "application/vnd.github+json") req.Header.Set("X-GitHub-Api-Version", "2022-11-28") @@ -423,10 +430,11 @@ func fetchGitHubUserIdentity(ctx context.Context, token string) (userName, email defer resp.Body.Close() if resp.StatusCode != http.StatusOK { + errBody, _ := io.ReadAll(io.LimitReader(resp.Body, 256)) if resp.StatusCode == http.StatusForbidden { - log.Printf("GitHub API /user returned 403 (token may lack 'read:user' scope)") + log.Printf("GitHub API /user returned 403 (token may lack 'read:user' scope): %s", string(errBody)) } else { - log.Printf("GitHub API /user returned status %d", resp.StatusCode) + log.Printf("GitHub API /user returned status %d: %s", resp.StatusCode, string(errBody)) } return "", "" } @@ -465,13 +473,17 @@ func fetchGitLabUserIdentity(ctx context.Context, token, instanceURL string) (us return "", "" } + if ctx.Err() != nil { + return "", "" + } + // Default to gitlab.com if no instance URL apiURL := "https://gitlab.com/api/v4/user" if instanceURL != "" && instanceURL != "https://gitlab.com" { apiURL = strings.TrimSuffix(instanceURL, "/") + "/api/v4/user" } - client := &http.Client{Timeout: 10 * time.Second} + client := &http.Client{Timeout: identityAPITimeout} req, err := http.NewRequestWithContext(ctx, "GET", apiURL, nil) if err != nil { log.Printf("Failed to create GitLab user request: %v", err) @@ -489,7 +501,8 @@ func fetchGitLabUserIdentity(ctx context.Context, token, instanceURL string) (us defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - log.Printf("GitLab API /user returned status %d", resp.StatusCode) + errBody, _ := io.ReadAll(io.LimitReader(resp.Body, 256)) + log.Printf("GitLab API /user returned status %d: %s", resp.StatusCode, string(errBody)) return "", "" } diff --git a/components/backend/handlers/runtime_credentials_test.go b/components/backend/handlers/runtime_credentials_test.go index 63c1ac993..14cd918f0 100644 --- a/components/backend/handlers/runtime_credentials_test.go +++ b/components/backend/handlers/runtime_credentials_test.go @@ -30,7 +30,7 @@ var _ = Describe("Runtime Credentials - Git Identity", func() { // Mock GitHub API response server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { Expect(r.URL.Path).To(Equal("/user")) - Expect(r.Header.Get("Authorization")).To(Equal("token test-token")) + Expect(r.Header.Get("Authorization")).To(Equal("Bearer test-token")) Expect(r.Header.Get("Accept")).To(Equal("application/vnd.github+json")) w.Header().Set("Content-Type", "application/json") diff --git a/components/runners/claude-code-runner/auth.py b/components/runners/claude-code-runner/auth.py index 0db022e20..ca6a8e7f0 100644 --- a/components/runners/claude-code-runner/auth.py +++ b/components/runners/claude-code-runner/auth.py @@ -18,6 +18,9 @@ logger = logging.getLogger(__name__) +# Placeholder email used by the platform when no real email is available. +_PLACEHOLDER_EMAIL = "user@example.com" + # --------------------------------------------------------------------------- # User context sanitization @@ -293,7 +296,7 @@ async def populate_runtime_credentials(context: RunnerContext) -> None: logger.info("✓ Updated Google credentials file for workspace-mcp") user_email = google_creds.get("email", "") - if user_email and user_email != "user@example.com": + if user_email and user_email != _PLACEHOLDER_EMAIL: os.environ["USER_GOOGLE_EMAIL"] = user_email logger.info( f"✓ Set USER_GOOGLE_EMAIL to {user_email} for workspace-mcp"