diff --git a/openhands-agent-server/openhands/agent_server/git_router.py b/openhands-agent-server/openhands/agent_server/git_router.py index d407e0970d..bed2757559 100644 --- a/openhands-agent-server/openhands/agent_server/git_router.py +++ b/openhands-agent-server/openhands/agent_server/git_router.py @@ -1,12 +1,24 @@ """Git router for OpenHands SDK.""" import asyncio +import base64 import functools import logging +import re from pathlib import Path +from typing import Literal, Self +from urllib.parse import quote -from fastapi import APIRouter, HTTPException, Path as PathParam, Query +import httpx +from fastapi import APIRouter, HTTPException, Path as PathParam, Query, Request +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator +from openhands.agent_server._secrets_exposure import get_config +from openhands.agent_server.persistence import ( + SECRET_NAME_PATTERN, + FileSecretsStore, + get_secrets_store, +) from openhands.agent_server.server_details_router import update_last_execution_time from openhands.sdk.git.exceptions import GitError, GitRepositoryError from openhands.sdk.git.git_changes import get_git_changes @@ -39,6 +51,179 @@ # (list-args protect against shell injection, not option injection). _SHA_PATTERN = r"^[0-9a-fA-F]{4,64}$" +RepositoryProvider = Literal["github", "gitlab", "bitbucket"] +RepositoryValidationStatus = Literal[ + "accessible", "missing_credentials", "denied", "not_found", "unavailable" +] + +_REPOSITORY_IDENTIFIER_PATTERN = ( + r"^[A-Za-z0-9][A-Za-z0-9_.-]*(?:/[A-Za-z0-9][A-Za-z0-9_.-]*)+$" +) +_REF_PATTERN = r"^[A-Za-z0-9][A-Za-z0-9._/@+-]*$" +_MAX_CREDENTIAL_NAMES = 5 +_PROVIDER_TIMEOUT_SECONDS = 5.0 +_PROVIDER_API_BASE_URLS: dict[RepositoryProvider, str] = { + "github": "https://api.github.com", + "gitlab": "https://gitlab.com/api/v4", + "bitbucket": "https://api.bitbucket.org/2.0", +} + + +class ValidateRepositoryRequest(BaseModel): + """Bounded input for checking access to a hosted Git repository.""" + + model_config = ConfigDict(extra="forbid") + + provider: RepositoryProvider + repository: str = Field(min_length=3, max_length=255) + ref: str | None = Field(default=None, min_length=1, max_length=255) + credential_names: list[str] = Field( + default_factory=list, + max_length=_MAX_CREDENTIAL_NAMES, + ) + + @field_validator("repository") + @classmethod + def _validate_repository(cls, repository: str) -> str: + if not re.fullmatch(_REPOSITORY_IDENTIFIER_PATTERN, repository): + raise ValueError("repository must be a provider repository identifier") + return repository + + @field_validator("ref") + @classmethod + def _validate_ref(cls, ref: str | None) -> str | None: + if ref is not None and not re.fullmatch(_REF_PATTERN, ref): + raise ValueError("ref must be a bounded git ref") + return ref + + @field_validator("credential_names") + @classmethod + def _validate_credential_names(cls, credential_names: list[str]) -> list[str]: + if len(set(credential_names)) != len(credential_names): + raise ValueError("credential_names must not contain duplicates") + if not all(SECRET_NAME_PATTERN.fullmatch(name) for name in credential_names): + raise ValueError("credential_names must be valid secret names") + return credential_names + + @model_validator(mode="after") + def _validate_provider_repository_shape(self) -> Self: + if self.provider in {"github", "bitbucket"} and self.repository.count("/") != 1: + raise ValueError( + "repository must have owner/repository form for this provider" + ) + return self + + +class ValidateRepositoryResponse(BaseModel): + """Sanitized repository-access verdict.""" + + status: RepositoryValidationStatus + + +def _provider_repository_url( + provider: RepositoryProvider, repository: str, ref: str | None +) -> str: + """Build a provider API URL from fixed hosts and validated path components.""" + if provider == "gitlab": + repository_path = quote(repository, safe="") + path = f"/projects/{repository_path}" + if ref is not None: + path += f"/repository/commits/{quote(ref, safe='')}" + else: + repository_path = quote(repository, safe="/") + path = ( + f"/repos/{repository_path}" + if provider == "github" + else f"/repositories/{repository_path}" + ) + if ref is not None: + path += ( + f"/commits/{quote(ref, safe='')}" + if provider == "github" + else f"/commit/{quote(ref, safe='')}" + ) + return f"{_PROVIDER_API_BASE_URLS[provider]}{path}" + + +def _provider_auth_headers( + provider: RepositoryProvider, token: str | None +) -> dict[str, str]: + """Return the one provider-specific authorization header, if available.""" + if token is None: + return {} + if provider == "gitlab": + return {"PRIVATE-TOKEN": token} + if provider == "bitbucket": + # The SDK's clone path uses ``x-token-auth`` for a bare Bitbucket + # token, while the UI also accepts an explicit ``username:token``. + # Preserve either spelling as HTTP Basic credentials so the probe + # exercises the same authorization that cloning will use. + credentials = token if ":" in token else f"x-token-auth:{token}" + encoded = base64.b64encode(credentials.encode()).decode("ascii") + return {"Authorization": f"Basic {encoded}"} + return {"Authorization": f"Bearer {token}"} + + +def _resolve_provider_credential( + store: FileSecretsStore, credential_names: list[str] +) -> str | None: + """Resolve the first available named credential without exposing it.""" + for name in credential_names: + token = store.get_secret(name) + if token: + return token + return None + + +@git_router.post( + "/validate-repository", + response_model=ValidateRepositoryResponse, +) +async def validate_repository( + request: Request, + repository_request: ValidateRepositoryRequest, +) -> ValidateRepositoryResponse: + """Return a sanitized verdict for access to a hosted Git repository.""" + update_last_execution_time() + token: str | None = None + if repository_request.credential_names: + try: + store = get_secrets_store(get_config(request)) + token = await asyncio.to_thread( + _resolve_provider_credential, + store, + repository_request.credential_names, + ) + except (HTTPException, OSError, RuntimeError, ValueError): + return ValidateRepositoryResponse(status="unavailable") + if token is None: + return ValidateRepositoryResponse(status="missing_credentials") + + try: + async with httpx.AsyncClient( + timeout=_PROVIDER_TIMEOUT_SECONDS, + follow_redirects=False, + trust_env=False, + ) as client: + response = await client.get( + _provider_repository_url( + repository_request.provider, + repository_request.repository, + repository_request.ref, + ), + headers=_provider_auth_headers(repository_request.provider, token), + ) + except httpx.TransportError: + return ValidateRepositoryResponse(status="unavailable") + + if 200 <= response.status_code < 300: + return ValidateRepositoryResponse(status="accessible") + if response.status_code in {401, 403}: + return ValidateRepositoryResponse(status="denied") + if response.status_code == 404: + return ValidateRepositoryResponse(status="not_found") + return ValidateRepositoryResponse(status="unavailable") + async def _get_git_changes(path: str, ref: str | None) -> list[GitChange]: """Internal helper to get git changes for a given path.""" diff --git a/tests/agent_server/test_git_router.py b/tests/agent_server/test_git_router.py index 5d42b2f679..ad0dc8ef71 100644 --- a/tests/agent_server/test_git_router.py +++ b/tests/agent_server/test_git_router.py @@ -1,14 +1,23 @@ """Tests for git_router.py endpoints.""" +import asyncio +import base64 +import logging import subprocess +import threading from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch +import httpx import pytest from fastapi.testclient import TestClient from openhands.agent_server.api import create_app from openhands.agent_server.config import Config +from openhands.agent_server.git_router import ( + ValidateRepositoryRequest, + validate_repository, +) from openhands.sdk.git.exceptions import GitCommandError, GitRepositoryError from openhands.sdk.git.models import ( GitChange, @@ -26,6 +35,367 @@ def client(): return TestClient(create_app(config), raise_server_exceptions=False) +class _ProviderClient: + """Deterministic stand-in for the provider HTTP client.""" + + def __init__(self, result: httpx.Response | Exception) -> None: + self.result = result + self.requests: list[tuple[str, dict[str, str]]] = [] + + async def __aenter__(self) -> "_ProviderClient": + return self + + async def __aexit__(self, *args: object) -> None: + return None + + async def get(self, url: str, *, headers: dict[str, str]) -> httpx.Response: + self.requests.append((url, headers)) + if isinstance(self.result, Exception): + raise self.result + return self.result + + +# ============================================================================= +# Repository Validation Tests +# ============================================================================= + + +@pytest.mark.parametrize( + ("provider", "repository", "expected_url"), + [ + ( + "github", + "openhands/software-agent-sdk", + "https://api.github.com/repos/openhands/software-agent-sdk", + ), + ( + "gitlab", + "openhands/software-agent-sdk", + "https://gitlab.com/api/v4/projects/openhands%2Fsoftware-agent-sdk", + ), + ( + "bitbucket", + "openhands/software-agent-sdk", + "https://api.bitbucket.org/2.0/repositories/openhands/software-agent-sdk", + ), + ], +) +def test_validate_repository_public_success_uses_allowlisted_host( + client, provider, repository, expected_url +): + """Public repositories are checked anonymously against fixed API hosts.""" + provider_client = _ProviderClient(httpx.Response(200)) + client_factory = MagicMock(return_value=provider_client) + + with patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory): + response = client.post( + "/api/git/validate-repository", + json={"provider": provider, "repository": repository}, + ) + + assert response.status_code == 200 + assert response.json() == {"status": "accessible"} + assert provider_client.requests == [(expected_url, {})] + assert client_factory.call_args.kwargs == { + "timeout": 5.0, + "follow_redirects": False, + "trust_env": False, + } + + +def test_validate_repository_checks_optional_ref(client): + """A ref is checked through the provider's ref-aware endpoint.""" + provider_client = _ProviderClient(httpx.Response(200)) + client_factory = MagicMock(return_value=provider_client) + + with patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory): + response = client.post( + "/api/git/validate-repository", + json={ + "provider": "gitlab", + "repository": "group/project", + "ref": "release/1.0", + }, + ) + + assert response.status_code == 200 + assert response.json() == {"status": "accessible"} + assert provider_client.requests == [ + ( + "https://gitlab.com/api/v4/projects/group%2Fproject/" + "repository/commits/release%2F1.0", + {}, + ) + ] + + +def test_validate_repository_reports_missing_named_credentials(client): + """Named credentials that cannot be resolved do not trigger an upstream call.""" + store = MagicMock() + store.get_secret.return_value = None + client_factory = MagicMock() + + with ( + patch( + "openhands.agent_server.git_router.get_secrets_store", + return_value=store, + ), + patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory), + ): + response = client.post( + "/api/git/validate-repository", + json={ + "provider": "github", + "repository": "openhands/software-agent-sdk", + "credential_names": ["GITHUB_TOKEN", "FALLBACK_TOKEN"], + }, + ) + + assert response.status_code == 200 + assert response.json() == {"status": "missing_credentials"} + assert store.get_secret.call_count == 2 + client_factory.assert_not_called() + + +@pytest.mark.parametrize( + ("upstream_status", "expected_status"), + [ + (401, "denied"), + (403, "denied"), + (404, "not_found"), + (503, "unavailable"), + ], +) +def test_validate_repository_sanitizes_provider_statuses( + client, upstream_status, expected_status +): + """Provider access failures map to typed verdicts without provider details.""" + provider_client = _ProviderClient(httpx.Response(upstream_status, text="details")) + client_factory = MagicMock(return_value=provider_client) + + with patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory): + response = client.post( + "/api/git/validate-repository", + json={"provider": "github", "repository": "openhands/software-agent-sdk"}, + ) + + assert response.status_code == 200 + assert response.json() == {"status": expected_status} + assert "details" not in response.text + + +def test_validate_repository_reports_transport_failure_as_unavailable(client): + """Transport failures must never be mistaken for repository access.""" + provider_client = _ProviderClient(httpx.ConnectError("provider unavailable")) + client_factory = MagicMock(return_value=provider_client) + + with patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory): + response = client.post( + "/api/git/validate-repository", + json={"provider": "github", "repository": "openhands/software-agent-sdk"}, + ) + + assert response.status_code == 200 + assert response.json() == {"status": "unavailable"} + + +@pytest.mark.parametrize( + "payload", + [ + {"provider": "unknown", "repository": "openhands/software-agent-sdk"}, + {"provider": "github", "repository": "https://example.test/repo"}, + {"provider": "github", "repository": "group/subgroup/project"}, + { + "provider": "github", + "repository": "openhands/software-agent-sdk", + "ref": "bad ref", + }, + { + "provider": "github", + "repository": "openhands/software-agent-sdk", + "ref": "x" * 256, + }, + { + "provider": "github", + "repository": "openhands/software-agent-sdk", + "credential_names": ["bad-name"], + }, + { + "provider": "github", + "repository": "openhands/software-agent-sdk", + "credential_names": [ + "TOKEN_0", + "TOKEN_1", + "TOKEN_2", + "TOKEN_3", + "TOKEN_4", + "TOKEN_5", + ], + }, + ], +) +def test_validate_repository_rejects_unbounded_or_invalid_input(client, payload): + """The endpoint rejects unsupported providers and oversized identifiers.""" + client_factory = MagicMock() + + with patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory): + response = client.post("/api/git/validate-repository", json=payload) + + assert response.status_code == 422 + client_factory.assert_not_called() + + +def test_validate_repository_never_leaks_secret_or_provider_body(client, caplog): + """Credentials and upstream response bodies stay out of responses and logs.""" + secret = "repo-probe-secret-sentinel" + provider_body = "provider-internal-sentinel" + store = MagicMock() + store.get_secret.return_value = secret + provider_client = _ProviderClient(httpx.Response(403, text=provider_body)) + client_factory = MagicMock(return_value=provider_client) + + with ( + caplog.at_level(logging.DEBUG), + patch( + "openhands.agent_server.git_router.get_secrets_store", + return_value=store, + ), + patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory), + ): + response = client.post( + "/api/git/validate-repository", + json={ + "provider": "github", + "repository": "openhands/software-agent-sdk", + "credential_names": ["GITHUB_TOKEN"], + }, + ) + + assert response.status_code == 200 + assert response.json() == {"status": "denied"} + assert provider_client.requests == [ + ( + "https://api.github.com/repos/openhands/software-agent-sdk", + {"Authorization": f"Bearer {secret}"}, + ) + ] + assert secret not in response.text + assert provider_body not in response.text + assert secret not in caplog.text + assert provider_body not in caplog.text + + +def test_validate_repository_uses_basic_auth_for_bitbucket_user_token(client): + """Bitbucket's stored ``username:token`` form is sent as HTTP Basic auth.""" + secret = "workspace-user:app-password" + store = MagicMock() + store.get_secret.return_value = secret + provider_client = _ProviderClient(httpx.Response(200)) + client_factory = MagicMock(return_value=provider_client) + + with ( + patch( + "openhands.agent_server.git_router.get_secrets_store", + return_value=store, + ), + patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory), + ): + response = client.post( + "/api/git/validate-repository", + json={ + "provider": "bitbucket", + "repository": "openhands/software-agent-sdk", + "credential_names": ["bitbucket_token"], + }, + ) + + encoded = base64.b64encode(secret.encode()).decode() + assert response.status_code == 200 + assert response.json() == {"status": "accessible"} + assert provider_client.requests == [ + ( + "https://api.bitbucket.org/2.0/repositories/openhands/software-agent-sdk", + {"Authorization": f"Basic {encoded}"}, + ) + ] + + +def test_validate_repository_uses_x_token_auth_for_bare_bitbucket_token(client): + """A bare Bitbucket token follows the SDK clone credential convention.""" + store = MagicMock() + store.get_secret.return_value = "bare-access-token" + provider_client = _ProviderClient(httpx.Response(200)) + client_factory = MagicMock(return_value=provider_client) + + with ( + patch( + "openhands.agent_server.git_router.get_secrets_store", + return_value=store, + ), + patch("openhands.agent_server.git_router.httpx.AsyncClient", client_factory), + ): + response = client.post( + "/api/git/validate-repository", + json={ + "provider": "bitbucket", + "repository": "openhands/software-agent-sdk", + "credential_names": ["bitbucket_token"], + }, + ) + + encoded = base64.b64encode(b"x-token-auth:bare-access-token").decode() + assert response.status_code == 200 + assert response.json() == {"status": "accessible"} + assert provider_client.requests[0][1] == {"Authorization": f"Basic {encoded}"} + + +@pytest.mark.asyncio +async def test_validate_repository_does_not_block_event_loop_on_secret_lock(): + """A slow file-secret lock is acquired outside the async request loop.""" + release = threading.Event() + timer = threading.Timer(0.2, release.set) + store = MagicMock() + + def slow_secret(_name: str) -> str: + release.wait(timeout=1) + return "github-token" + + store.get_secret.side_effect = slow_secret + provider_client = _ProviderClient(httpx.Response(200)) + client_factory = MagicMock(return_value=provider_client) + request = ValidateRepositoryRequest( + provider="github", + repository="openhands/software-agent-sdk", + credential_names=["github_token"], + ) + + timer.start() + try: + with ( + patch("openhands.agent_server.git_router.get_config"), + patch( + "openhands.agent_server.git_router.get_secrets_store", + return_value=store, + ), + patch( + "openhands.agent_server.git_router.httpx.AsyncClient", + client_factory, + ), + ): + loop = asyncio.get_running_loop() + started = loop.time() + validation = asyncio.create_task(validate_repository(MagicMock(), request)) + await asyncio.sleep(0.02) + heartbeat_elapsed = loop.time() - started + response = await validation + finally: + release.set() + timer.cancel() + + assert heartbeat_elapsed < 0.15 + assert response.status == "accessible" + + # ============================================================================= # Query Parameter Tests (Preferred Method) # =============================================================================