-
Notifications
You must be signed in to change notification settings - Fork 23
Modernize codebase to November 2025 standards #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -167,33 +167,7 @@ async def send_input(self, text: str): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| session_id=self.session_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error=str(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def _start_mock_process(self, prompt: str, model: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Start mock process for testing.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.is_running = True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Create mock Claude response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_response = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "type": "result", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "sessionId": self.session_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "model": model or "claude-3-5-haiku-20241022", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "message": { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "role": "assistant", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "content": f"Hello! You said: '{prompt}'. This is a mock response from Claude Code API Gateway." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "usage": { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "input_tokens": len(prompt.split()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "output_tokens": 15, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "total_tokens": len(prompt.split()) + 15 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "cost_usd": 0.001, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "duration_ms": 100 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Put the response in the queue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await self.output_queue.put(mock_response) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await self.output_queue.put(None) # End signal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def stop(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Stop Claude process.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.is_running = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -336,20 +310,60 @@ async def continue_conversation( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Utility functions for project management | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def sanitize_path_component(component: str) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Sanitize a path component to prevent directory traversal attacks.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import re | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Remove any path separators and special characters | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = re.sub(r'[^\w\-.]', '_', component) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Remove leading dots to prevent hidden files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitized.lstrip('.') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Limit length | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitized[:255] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not sanitized: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise ValueError("Invalid path component: results in empty string after sanitization") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return sanitized | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+313
to
+324
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sanitization function is a good security measure. However, it could be made more robust and aligned with best practices:
Here is a suggested improvement that addresses the second point. Please also move the import to the top of the file.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def create_project_directory(project_id: str) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Create project directory.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_path = os.path.join(settings.project_root, project_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Create project directory with path validation.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Sanitize the project_id to prevent path traversal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| safe_project_id = sanitize_path_component(project_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Construct the full path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_path = os.path.join(settings.project_root, safe_project_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Verify the resulting path is still within project_root (defense in depth) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_path = os.path.abspath(project_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| root_path = os.path.abspath(settings.project_root) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+335
to
+337
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a potential security vulnerability here.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not project_path.startswith(root_path + os.sep): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise ValueError(f"Invalid project path: {project_id} resolves outside project root") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| os.makedirs(project_path, exist_ok=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info("Project directory created", project_id=safe_project_id, path=project_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return project_path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def cleanup_project_directory(project_path: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Clean up project directory.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Clean up project directory with path validation.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import shutil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if os.path.exists(project_path): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.rmtree(project_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info("Project directory cleaned up", path=project_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Verify the path is within project_root before deletion | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| abs_project_path = os.path.abspath(project_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| root_path = os.path.abspath(settings.project_root) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+352
to
+354
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not abs_project_path.startswith(root_path + os.sep): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Refused to cleanup directory outside project root", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path=project_path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_root=settings.project_root | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise ValueError(f"Cannot cleanup path outside project root: {project_path}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if os.path.exists(abs_project_path): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.rmtree(abs_project_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info("Project directory cleaned up", path=abs_project_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error("Failed to cleanup project directory", path=project_path, error=str(e)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -94,15 +94,22 @@ def parse_api_keys(cls, v): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log_format: str = "json" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # CORS Configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowed_origins: List[str] = Field(default=["*"]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowed_methods: List[str] = Field(default=["*"]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Default to localhost only for security - can be overridden via ALLOWED_ORIGINS env var | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowed_origins: List[str] = Field(default=[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "http://localhost:*", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "http://127.0.0.1:*", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "http://[::1]:*" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowed_methods: List[str] = Field(default=["GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowed_headers: List[str] = Field(default=["*"]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @field_validator('allowed_origins', 'allowed_methods', 'allowed_headers', mode='before') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def parse_cors_lists(cls, v): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(v, str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [x.strip() for x in v.split(',') if x.strip()] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return v or ["*"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Support comma-separated values or "*" for all | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parsed = [x.strip() for x in v.split(',') if x.strip()] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return parsed if parsed else ["http://localhost:*"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return v if v else ["http://localhost:*"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
106
to
+112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shared validator for
These fields should have separate validators to handle their distinct logic and defaults correctly.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Rate Limiting | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rate_limit_requests_per_minute: int = 100 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shell logic to find a command to open the browser is clever, but it's not fully portable (e.g., it might not work on all Linux distributions or in environments like WSL without
xdg-open). A more portable and simpler approach is to use Python's built-inwebbrowsermodule, which handles the cross-platform differences internally.