From b1b5248dec0e1b9df5d8b3f1a81b752926f5ba66 Mon Sep 17 00:00:00 2001 From: tnm Date: Wed, 25 Jun 2025 18:27:08 -0700 Subject: [PATCH 1/3] robust error logging on repo creation --- src/kit/api/app.py | 71 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/src/kit/api/app.py b/src/kit/api/app.py index 0c760570..e1445391 100644 --- a/src/kit/api/app.py +++ b/src/kit/api/app.py @@ -2,6 +2,7 @@ from __future__ import annotations +import subprocess from typing import Dict, List from fastapi import FastAPI, HTTPException @@ -27,9 +28,73 @@ class FilePathsIn(BaseModel): @app.post("/repository", status_code=201) def open_repo(body: RepoIn): """Register a repository path/URL and return its deterministic ID.""" - repo_id = registry.add(body.path_or_url, body.ref) - _ = registry.get_repo(repo_id) - return {"id": repo_id} + try: + repo_id = registry.add(body.path_or_url, body.ref) + _ = registry.get_repo(repo_id) + return {"id": repo_id} + except subprocess.CalledProcessError as e: + # Git/clone specific errors - handle first to catch git failures + git_error_msg = str(e) + + # Check if this is a git clone command that failed + if (hasattr(e, 'cmd') and e.cmd and + isinstance(e.cmd, list) and len(e.cmd) > 2 and + e.cmd[0] == 'git' and e.cmd[1] == 'clone'): + + # Git clone failed - common reasons: + if e.returncode == 128: # Git error code for general errors + # Extract repo URL from command for context + repo_url = None + if len(e.cmd) >= 4: + repo_url = e.cmd[3] # Usually the 4th argument + + if repo_url and ('github.com' in repo_url or 'gitlab.com' in repo_url): + raise HTTPException(status_code=404, detail=f"Repository not found: {repo_url}") + else: + raise HTTPException(status_code=404, detail=f"Repository not found or inaccessible") + else: + raise HTTPException(status_code=500, detail=f"Git clone failed: {git_error_msg}") + else: + # Other git commands + raise HTTPException(status_code=500, detail=f"Git operation failed: {git_error_msg}") + except FileNotFoundError as e: + raise HTTPException(status_code=404, detail=f"Repository path not found: {e!s}") + except ValueError as e: + # Git ref errors, invalid paths, etc. + raise HTTPException(status_code=400, detail=f"Invalid repository configuration: {e!s}") + except Exception as e: + # Clone failures, network issues, permission errors, etc. + error_msg = str(e) + + # For subprocess errors, try to extract stderr if available + if hasattr(e, "stderr") and e.stderr: + error_msg = e.stderr + elif hasattr(e, "output") and e.output: + error_msg = e.output + + # Check for common git/repository errors in the error message + error_lower = error_msg.lower() + if ( + "not found" in error_lower + or "repository not found" in error_lower + or "remote: repository not found" in error_msg + or "fatal: repository" in error_msg + or "does not exist" in error_lower + ) and ("github.com" in error_msg or "gitlab.com" in error_msg or "git" in error_lower): + raise HTTPException(status_code=404, detail=f"Repository not found: {error_msg}") + elif "permission denied" in error_lower or "access denied" in error_lower: + raise HTTPException(status_code=403, detail=f"Access denied: {error_msg}") + elif ( + "timeout" in error_lower + or "network" in error_lower + or "connection" in error_lower + or "resolve" in error_lower + ): + raise HTTPException(status_code=503, detail=f"Network error: {error_msg}") + elif "authentication failed" in error_lower or "invalid credentials" in error_lower: + raise HTTPException(status_code=401, detail=f"Authentication failed: {error_msg}") + else: + raise HTTPException(status_code=500, detail=f"Failed to initialize repository: {error_msg}") @app.get("/repository/{repo_id}/file-tree") From f07e28164f99f782db9ff91c0e41b09c181e0282 Mon Sep 17 00:00:00 2001 From: tnm Date: Wed, 25 Jun 2025 18:32:40 -0700 Subject: [PATCH 2/3] simpler --- src/kit/api/app.py | 59 ++++++++++++---------------------------------- 1 file changed, 15 insertions(+), 44 deletions(-) diff --git a/src/kit/api/app.py b/src/kit/api/app.py index e1445391..df10407d 100644 --- a/src/kit/api/app.py +++ b/src/kit/api/app.py @@ -33,66 +33,37 @@ def open_repo(body: RepoIn): _ = registry.get_repo(repo_id) return {"id": repo_id} except subprocess.CalledProcessError as e: - # Git/clone specific errors - handle first to catch git failures - git_error_msg = str(e) - - # Check if this is a git clone command that failed - if (hasattr(e, 'cmd') and e.cmd and - isinstance(e.cmd, list) and len(e.cmd) > 2 and - e.cmd[0] == 'git' and e.cmd[1] == 'clone'): - - # Git clone failed - common reasons: - if e.returncode == 128: # Git error code for general errors - # Extract repo URL from command for context - repo_url = None - if len(e.cmd) >= 4: - repo_url = e.cmd[3] # Usually the 4th argument - - if repo_url and ('github.com' in repo_url or 'gitlab.com' in repo_url): - raise HTTPException(status_code=404, detail=f"Repository not found: {repo_url}") - else: - raise HTTPException(status_code=404, detail=f"Repository not found or inaccessible") + # Git command failures (clone, checkout, etc.) + error_msg = str(e) + + # For git clone failures (exit code 128 is common for "not found") + if e.returncode == 128 and "clone" in error_msg: + # Extract URL from error message if possible, otherwise use generic message + if body.path_or_url.startswith(("http://", "https://")): + raise HTTPException(status_code=404, detail=f"Repository not found: {body.path_or_url}") else: - raise HTTPException(status_code=500, detail=f"Git clone failed: {git_error_msg}") + raise HTTPException(status_code=404, detail="Repository not found or inaccessible") else: - # Other git commands - raise HTTPException(status_code=500, detail=f"Git operation failed: {git_error_msg}") + raise HTTPException(status_code=500, detail=f"Git operation failed: {error_msg}") except FileNotFoundError as e: raise HTTPException(status_code=404, detail=f"Repository path not found: {e!s}") except ValueError as e: # Git ref errors, invalid paths, etc. raise HTTPException(status_code=400, detail=f"Invalid repository configuration: {e!s}") except Exception as e: - # Clone failures, network issues, permission errors, etc. + # All other failures - keep it simple error_msg = str(e) - # For subprocess errors, try to extract stderr if available - if hasattr(e, "stderr") and e.stderr: - error_msg = e.stderr - elif hasattr(e, "output") and e.output: - error_msg = e.output - - # Check for common git/repository errors in the error message + # Simple string-based detection for common cases error_lower = error_msg.lower() - if ( - "not found" in error_lower - or "repository not found" in error_lower - or "remote: repository not found" in error_msg - or "fatal: repository" in error_msg - or "does not exist" in error_lower - ) and ("github.com" in error_msg or "gitlab.com" in error_msg or "git" in error_lower): + if "repository not found" in error_lower or "not found" in error_lower: raise HTTPException(status_code=404, detail=f"Repository not found: {error_msg}") elif "permission denied" in error_lower or "access denied" in error_lower: raise HTTPException(status_code=403, detail=f"Access denied: {error_msg}") - elif ( - "timeout" in error_lower - or "network" in error_lower - or "connection" in error_lower - or "resolve" in error_lower - ): - raise HTTPException(status_code=503, detail=f"Network error: {error_msg}") elif "authentication failed" in error_lower or "invalid credentials" in error_lower: raise HTTPException(status_code=401, detail=f"Authentication failed: {error_msg}") + elif "timeout" in error_lower or "network" in error_lower or "connection" in error_lower: + raise HTTPException(status_code=503, detail=f"Network error: {error_msg}") else: raise HTTPException(status_code=500, detail=f"Failed to initialize repository: {error_msg}") From 405f65e8c3a4f0c3687fa654e431ad9250246388 Mon Sep 17 00:00:00 2001 From: tnm Date: Wed, 25 Jun 2025 18:43:23 -0700 Subject: [PATCH 3/3] Add tests, simplify --- src/kit/api/app.py | 124 +++++++++- tests/test_api_security.py | 447 +++++++++++++++++++++++++++++++++++++ 2 files changed, 561 insertions(+), 10 deletions(-) create mode 100644 tests/test_api_security.py diff --git a/src/kit/api/app.py b/src/kit/api/app.py index df10407d..252fadf9 100644 --- a/src/kit/api/app.py +++ b/src/kit/api/app.py @@ -2,8 +2,10 @@ from __future__ import annotations +import logging import subprocess from typing import Dict, List +from urllib.parse import urlparse from fastapi import FastAPI, HTTPException from pydantic import BaseModel @@ -12,9 +14,31 @@ from .registry import registry +# Set up logging +logger = logging.getLogger(__name__) + app = FastAPI(title="kit API", version="0.1.0") +def sanitize_url(url: str) -> str: + """Remove credentials from URL for safe display in error messages.""" + try: + parsed = urlparse(url) + if parsed.username or parsed.password: + # Reconstruct URL without credentials + sanitized = f"{parsed.scheme}://{parsed.hostname}" + if parsed.port: + sanitized += f":{parsed.port}" + sanitized += parsed.path + if parsed.query: + sanitized += f"?{parsed.query}" + return sanitized + return url + except Exception: + # If parsing fails, return a generic message + return "[sanitized repository URL]" + + class RepoIn(BaseModel): path_or_url: str github_token: str | None = None @@ -31,41 +55,121 @@ def open_repo(body: RepoIn): try: repo_id = registry.add(body.path_or_url, body.ref) _ = registry.get_repo(repo_id) + logger.info(f"Repository opened successfully: {sanitize_url(body.path_or_url)}") return {"id": repo_id} except subprocess.CalledProcessError as e: # Git command failures (clone, checkout, etc.) error_msg = str(e) + # Log full details for debugging (internal only) + logger.warning( + f"Git command failed: {error_msg}", + extra={ + "repo_url": body.path_or_url, + "ref": body.ref, + "return_code": e.returncode, + "event_type": "git_command_failure", + }, + ) + # For git clone failures (exit code 128 is common for "not found") if e.returncode == 128 and "clone" in error_msg: # Extract URL from error message if possible, otherwise use generic message if body.path_or_url.startswith(("http://", "https://")): - raise HTTPException(status_code=404, detail=f"Repository not found: {body.path_or_url}") + logger.info( + f"Repository not found: {sanitize_url(body.path_or_url)}", + extra={"event_type": "repository_not_found", "repo_url_sanitized": sanitize_url(body.path_or_url)}, + ) + raise HTTPException(status_code=404, detail=f"Repository not found: {sanitize_url(body.path_or_url)}") else: + logger.info( + f"Local repository not found: {body.path_or_url}", + extra={"event_type": "local_repository_not_found"}, + ) raise HTTPException(status_code=404, detail="Repository not found or inaccessible") else: raise HTTPException(status_code=500, detail=f"Git operation failed: {error_msg}") except FileNotFoundError as e: + logger.warning(f"File not found: {body.path_or_url}", extra={"event_type": "file_not_found", "error": str(e)}) raise HTTPException(status_code=404, detail=f"Repository path not found: {e!s}") except ValueError as e: # Git ref errors, invalid paths, etc. + logger.warning( + f"Invalid repository configuration: {body.path_or_url}", + extra={"event_type": "invalid_configuration", "ref": body.ref, "error": str(e)}, + ) raise HTTPException(status_code=400, detail=f"Invalid repository configuration: {e!s}") except Exception as e: # All other failures - keep it simple error_msg = str(e) - - # Simple string-based detection for common cases error_lower = error_msg.lower() - if "repository not found" in error_lower or "not found" in error_lower: - raise HTTPException(status_code=404, detail=f"Repository not found: {error_msg}") - elif "permission denied" in error_lower or "access denied" in error_lower: - raise HTTPException(status_code=403, detail=f"Access denied: {error_msg}") + + # Log with appropriate level based on error type + if "permission denied" in error_lower or "access denied" in error_lower: + logger.warning( + "Access denied for repository", + extra={ + "event_type": "access_denied", + "repo_url_sanitized": sanitize_url(body.path_or_url) + if body.path_or_url.startswith(("http://", "https://")) + else body.path_or_url, + "error": error_msg, + }, + ) + raise HTTPException( + status_code=403, + detail=f"Access denied: {sanitize_url(body.path_or_url) if body.path_or_url.startswith(('http://', 'https://')) else 'repository'}", + ) elif "authentication failed" in error_lower or "invalid credentials" in error_lower: - raise HTTPException(status_code=401, detail=f"Authentication failed: {error_msg}") + logger.warning( + "Authentication failed for repository", + extra={ + "event_type": "authentication_failed", + "repo_url_sanitized": sanitize_url(body.path_or_url) + if body.path_or_url.startswith(("http://", "https://")) + else body.path_or_url, + "error": error_msg, + }, + ) + raise HTTPException(status_code=401, detail="Authentication failed") elif "timeout" in error_lower or "network" in error_lower or "connection" in error_lower: - raise HTTPException(status_code=503, detail=f"Network error: {error_msg}") + logger.warning( + "Network error for repository", + extra={ + "event_type": "network_error", + "repo_url_sanitized": sanitize_url(body.path_or_url) + if body.path_or_url.startswith(("http://", "https://")) + else body.path_or_url, + "error": error_msg, + }, + ) + raise HTTPException(status_code=503, detail="Network error accessing repository") + elif "repository not found" in error_lower or "not found" in error_lower: + logger.info( + "Repository not found", + extra={ + "event_type": "repository_not_found", + "repo_url_sanitized": sanitize_url(body.path_or_url) + if body.path_or_url.startswith(("http://", "https://")) + else body.path_or_url, + }, + ) + raise HTTPException( + status_code=404, + detail=f"Repository not found: {sanitize_url(body.path_or_url) if body.path_or_url.startswith(('http://', 'https://')) else body.path_or_url}", + ) else: - raise HTTPException(status_code=500, detail=f"Failed to initialize repository: {error_msg}") + logger.error( + "Unexpected error initializing repository", + extra={ + "event_type": "unexpected_error", + "repo_url_sanitized": sanitize_url(body.path_or_url) + if body.path_or_url.startswith(("http://", "https://")) + else body.path_or_url, + "error": error_msg, + }, + ) + raise HTTPException(status_code=500, detail="Failed to initialize repository") @app.get("/repository/{repo_id}/file-tree") diff --git a/tests/test_api_security.py b/tests/test_api_security.py new file mode 100644 index 00000000..0567847d --- /dev/null +++ b/tests/test_api_security.py @@ -0,0 +1,447 @@ +"""Tests for REST API security features including URL sanitization and error handling.""" + +import logging +import subprocess +import time +from unittest.mock import MagicMock, patch +from urllib.parse import urlparse + +import pytest +import requests +from pydantic import BaseModel + +from kit.api.app import sanitize_url + + +class RepoIn(BaseModel): + """Request model for repository creation.""" + + path_or_url: str + ref: str = None + + +class TestURLSanitization: + """Test URL sanitization functionality.""" + + def test_sanitize_clean_url(self): + """Test that clean URLs are unchanged.""" + clean_urls = [ + "https://github.com/user/repo", + "http://localhost:3000/repo", + "https://gitlab.com/org/project", + "git@github.com:user/repo.git", # This will remain unchanged as it's not HTTP(S) + ] + + for url in clean_urls: + result = sanitize_url(url) + assert result == url + + def test_sanitize_url_with_credentials(self): + """Test that URLs with credentials are properly sanitized.""" + test_cases = [ + {"input": "https://token:x-oauth-basic@github.com/user/repo", "expected": "https://github.com/user/repo"}, + {"input": "https://username:password@gitlab.com/user/repo", "expected": "https://gitlab.com/user/repo"}, + {"input": "https://ghp_token123@github.com/private/repo", "expected": "https://github.com/private/repo"}, + { + "input": "https://user:pass@example.com:8080/path?query=value", + "expected": "https://example.com:8080/path?query=value", + }, + {"input": "http://api_key:secret@api.example.com/v1/repo", "expected": "http://api.example.com/v1/repo"}, + ] + + for case in test_cases: + result = sanitize_url(case["input"]) + assert result == case["expected"], f"Failed for {case['input']}" + + def test_sanitize_url_preserves_path_and_query(self): + """Test that path and query parameters are preserved during sanitization.""" + url = "https://token:secret@api.github.com/repos/user/repo?per_page=100&page=1" + result = sanitize_url(url) + + assert result == "https://api.github.com/repos/user/repo?per_page=100&page=1" + assert "token" not in result + assert "secret" not in result + assert "per_page=100" in result + assert "page=1" in result + + def test_sanitize_url_handles_malformed_urls(self): + """Test that malformed URLs are handled gracefully.""" + malformed_urls = [ + "not-a-url", + "http://", + "://missing-scheme", + "", + None, # This would cause an exception in real usage, but our function should handle it + ] + + for url in malformed_urls: + if url is None: + continue # Skip None as it would cause TypeError in real usage + result = sanitize_url(url) + # Should either return the original or a safe fallback + assert isinstance(result, str) + + def test_sanitize_url_no_credentials_present(self): + """Test that URLs without credentials are detected correctly.""" + urls_without_creds = [ + "https://github.com/user/repo", + "http://localhost:8000", + "https://api.example.com/v1/data", + ] + + for url in urls_without_creds: + parsed = urlparse(url) + assert parsed.username is None + assert parsed.password is None + assert sanitize_url(url) == url + + +class TestAPISecurityErrorHandling: + """Test API error handling with security considerations.""" + + def test_repository_creation_sanitizes_credentials_in_404_error(self): + """Test that 404 errors for remote repos sanitize credentials in response.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + ): + # Mock a git clone failure + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = subprocess.CalledProcessError( + 128, ["git", "clone", "--depth=1", "https://token:secret@github.com/nonexistent/repo"] + ) + + try: + open_repo(RepoIn(path_or_url="https://token:secret@github.com/nonexistent/repo")) + assert False, "Expected HTTPException" + except Exception as e: + # Should be HTTPException with status 404 + assert hasattr(e, "status_code") + assert e.status_code == 404 + + # Credentials should be sanitized in response + assert "token" not in e.detail + assert "secret" not in e.detail + assert "github.com/nonexistent/repo" in e.detail + + def test_repository_creation_logs_full_details_internally(self, caplog): + """Test that internal logs contain full details for debugging.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + caplog.at_level(logging.WARNING), + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = subprocess.CalledProcessError( + 128, ["git", "clone", "--depth=1", "https://token:secret@github.com/nonexistent/repo"] + ) + + try: + open_repo(RepoIn(path_or_url="https://token:secret@github.com/nonexistent/repo")) + except Exception: + pass # Expected + + # Check that logs contain the event type and repo URL + log_records = [record for record in caplog.records if record.name == "kit.api.app"] + assert len(log_records) > 0 + + # Find the git command failure log + git_failure_logs = [r for r in log_records if "Git command failed" in r.message] + assert len(git_failure_logs) > 0 + + def test_authentication_failure_sanitized_response(self): + """Test that authentication failures don't expose credentials in response.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = Exception("authentication failed for https://token:secret@private.com/repo") + + try: + open_repo(RepoIn(path_or_url="https://token:secret@private.com/repo")) + assert False, "Expected HTTPException" + except Exception as e: + assert hasattr(e, "status_code") + assert e.status_code == 401 + + # Credentials should not be in response + assert "token" not in e.detail + assert "secret" not in e.detail + assert e.detail == "Authentication failed" + + def test_permission_denied_sanitized_response(self): + """Test that permission denied errors sanitize URLs.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = Exception("permission denied accessing https://user:pass@private.com/repo") + + try: + open_repo(RepoIn(path_or_url="https://user:pass@private.com/repo")) + assert False, "Expected HTTPException" + except Exception as e: + assert hasattr(e, "status_code") + assert e.status_code == 403 + + # Credentials should be sanitized + assert "user" not in e.detail or "pass" not in e.detail + assert "private.com/repo" in e.detail + + def test_network_error_sanitized_response(self): + """Test that network errors sanitize URLs.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = Exception("timeout connecting to https://api:key@remote.com/repo") + + try: + open_repo(RepoIn(path_or_url="https://api:key@remote.com/repo")) + assert False, "Expected HTTPException" + except Exception as e: + assert hasattr(e, "status_code") + assert e.status_code == 503 + + # Credentials should be sanitized + assert "api" not in e.detail or "key" not in e.detail + assert "Network error" in e.detail + + def test_local_repository_errors_not_sanitized(self): + """Test that local repository errors don't unnecessarily sanitize paths.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = FileNotFoundError("No such file or directory: '/path/to/local/repo'") + + try: + open_repo(RepoIn(path_or_url="/path/to/local/repo")) + assert False, "Expected HTTPException" + except Exception as e: + assert hasattr(e, "status_code") + assert e.status_code == 404 + + # Local paths should be preserved + assert "/path/to/local/repo" in e.detail + + def test_successful_repository_creation_logging(self, caplog): + """Test that successful repository creation is logged with sanitized URL.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + caplog.at_level(logging.INFO), + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.return_value = MagicMock() + + result = open_repo(RepoIn(path_or_url="https://token:secret@github.com/user/repo")) + + assert result["id"] == "test_repo_id" + + # Check success log + success_logs = [r for r in caplog.records if "Repository opened successfully" in r.message] + assert len(success_logs) > 0 + + success_log = success_logs[0] + assert "github.com/user/repo" in success_log.message + assert "token" not in success_log.message + assert "secret" not in success_log.message + + +class TestSecurityEventLogging: + """Test security-related event logging.""" + + def test_git_command_failure_logging_structure(self, caplog): + """Test that git command failures are logged with proper structure.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + caplog.at_level(logging.WARNING), + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = subprocess.CalledProcessError( + 128, ["git", "clone", "https://github.com/test/repo"] + ) + + try: + open_repo(RepoIn(path_or_url="https://github.com/test/repo", ref="main")) + except Exception: + pass # Expected + + # Find git command failure log + git_logs = [r for r in caplog.records if hasattr(r, "event_type") and r.event_type == "git_command_failure"] + assert len(git_logs) > 0 + + log_record = git_logs[0] + assert hasattr(log_record, "repo_url") + assert hasattr(log_record, "ref") + assert hasattr(log_record, "return_code") + assert log_record.return_code == 128 + + def test_repository_not_found_logging_structure(self, caplog): + """Test that repository not found events are logged with proper structure.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + caplog.at_level(logging.INFO), + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = subprocess.CalledProcessError( + 128, ["git", "clone", "--depth=1", "https://github.com/nonexistent/repo"] + ) + + try: + open_repo(RepoIn(path_or_url="https://github.com/nonexistent/repo")) + except Exception: + pass # Expected + + # Find repository not found log + not_found_logs = [ + r for r in caplog.records if hasattr(r, "event_type") and r.event_type == "repository_not_found" + ] + assert len(not_found_logs) > 0 + + log_record = not_found_logs[0] + assert hasattr(log_record, "repo_url_sanitized") + assert log_record.repo_url_sanitized == "https://github.com/nonexistent/repo" + + def test_authentication_failure_logging_structure(self, caplog): + """Test that authentication failures are logged with proper structure.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + caplog.at_level(logging.WARNING), + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = Exception("authentication failed") + + try: + open_repo(RepoIn(path_or_url="https://token:secret@private.com/repo")) + except Exception: + pass # Expected + + # Find authentication failure log + auth_logs = [ + r for r in caplog.records if hasattr(r, "event_type") and r.event_type == "authentication_failed" + ] + assert len(auth_logs) > 0 + + log_record = auth_logs[0] + assert hasattr(log_record, "repo_url_sanitized") + assert hasattr(log_record, "error") + assert log_record.repo_url_sanitized == "https://private.com/repo" + + def test_access_denied_logging_structure(self, caplog): + """Test that access denied events are logged with proper structure.""" + from kit.api.app import RepoIn, open_repo + + with ( + patch("kit.api.registry.registry.add") as mock_add, + patch("kit.api.registry.registry.get_repo") as mock_get_repo, + caplog.at_level(logging.WARNING), + ): + mock_add.return_value = "test_repo_id" + mock_get_repo.side_effect = Exception("permission denied") + + try: + open_repo(RepoIn(path_or_url="https://user:pass@restricted.com/repo")) + except Exception: + pass # Expected + + # Find access denied log + access_logs = [r for r in caplog.records if hasattr(r, "event_type") and r.event_type == "access_denied"] + assert len(access_logs) > 0 + + log_record = access_logs[0] + assert hasattr(log_record, "repo_url_sanitized") + assert hasattr(log_record, "error") + assert log_record.repo_url_sanitized == "https://restricted.com/repo" + + +class TestIntegrationSecurity: + """Integration tests for security features.""" + + @pytest.fixture(scope="class") + def test_server(self): + """Start a test kit server for integration testing.""" + proc = subprocess.Popen( + ["kit", "serve", "--host", "127.0.0.1", "--port", "8998", "--reload", "false"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + + time.sleep(3) + + try: + response = requests.get("http://127.0.0.1:8998/docs", timeout=5) + if response.status_code != 200: + proc.terminate() + pytest.skip("Could not start test server") + except requests.RequestException: + proc.terminate() + pytest.skip("Could not connect to test server") + + yield "http://127.0.0.1:8998" + + proc.terminate() + proc.wait() + + def test_remote_repository_not_found_integration(self, test_server): + """Integration test for remote repository not found with credential sanitization.""" + # Test with a URL that contains credentials but repo doesn't exist + response = requests.post( + f"{test_server}/repository", + json={"path_or_url": "https://fake-token:x-oauth-basic@github.com/nonexistent/security-test-repo"}, + ) + + assert response.status_code == 404 + response_data = response.json() + + # Check that credentials are sanitized in response + assert "fake-token" not in response_data["detail"] + assert "x-oauth-basic" not in response_data["detail"] + assert "github.com/nonexistent/security-test-repo" in response_data["detail"] + + def test_invalid_git_ref_integration(self, test_server): + """Integration test for invalid git ref error handling.""" + response = requests.post( + f"{test_server}/repository", json={"path_or_url": ".", "ref": "definitely-nonexistent-branch-12345"} + ) + + assert response.status_code == 400 + response_data = response.json() + assert "Invalid repository configuration" in response_data["detail"] + + def test_successful_repository_creation_integration(self, test_server): + """Integration test for successful repository creation.""" + response = requests.post(f"{test_server}/repository", json={"path_or_url": "."}) + + assert response.status_code == 201 + response_data = response.json() + assert "id" in response_data + assert isinstance(response_data["id"], str)