Merge pull request #15 from mdlmarkham/claude/review-test-project-01QwQTFaTsJggDZ27ksb7YhW

Review and test the project

Author: mdlmarkham

SECURITY_REVIEW_REPORT.md

@@ -5,10 +5,13 @@
 SYSTEMMANAGER_AUTH_MODE=oidc
 
 # OIDC/OAuth Configuration (for TSIDP)
-TSIDP_URL=https://tsidp.tailf9480.ts.net
+# IMPORTANT: Replace YOUR-TAILNET with your actual Tailscale tailnet name
+# Find this in your Tailscale admin console under OAuth Applications
+TSIDP_URL=https://tsidp.YOUR-TAILNET.ts.net
 TSIDP_CLIENT_ID=your-client-id-here
 TSIDP_CLIENT_SECRET=your-client-secret-here
-SYSTEMMANAGER_BASE_URL=http://dev1.tailf9480.ts.net:8080
+# IMPORTANT: Replace with your actual server hostname
+SYSTEMMANAGER_BASE_URL=http://your-server.YOUR-TAILNET.ts.net:8080
 
 # Fallback Token Authentication (if not using OIDC)
 # SYSTEMMANAGER_SHARED_SECRET=your-hmac-secret-here

deploy/.env.template

@@ -12,7 +12,7 @@ dependencies = [
     "psutil>=5.9.0",
     "docker>=6.0.0",
     "pydantic>=2.0.0",
-    "cryptography>=41.0.0",
+    "cryptography>=42.0.0",  # Updated to match requirements.txt
 ]
 
 classifiers = [

pyproject.toml

@@ -109,8 +109,16 @@ def verify(self, token: str) -> TokenClaims:
                 raise SystemManagerError("invalid token claims", category=ErrorCategory.UNAUTHORIZED)
 
             # Check expiry if present
-            if claims.expiry and claims.expiry < datetime.datetime.utcnow():
-                raise SystemManagerError("token expired", category=ErrorCategory.UNAUTHORIZED)
+            # Use timezone-aware datetime comparison
+            now = datetime.datetime.now(datetime.timezone.utc)
+            # Handle both naive and aware datetimes
+            if claims.expiry:
+                expiry = claims.expiry
+                if expiry.tzinfo is None:
+                    # Assume UTC for naive datetimes
+                    expiry = expiry.replace(tzinfo=datetime.timezone.utc)
+                if expiry < now:
+                    raise SystemManagerError("token expired", category=ErrorCategory.UNAUTHORIZED)
 
             return claims
 

src/auth/token_auth.py

@@ -16,11 +16,17 @@
 from src.server.dependencies import deps
 from src.tools import register_all_tools
 
-# Configure logging
-logging.basicConfig(level=logging.DEBUG)
+# Configure logging - use environment variable for log level
+LOG_LEVEL = os.getenv("LOG_LEVEL", "INFO").upper()
+logging.basicConfig(
+    level=getattr(logging, LOG_LEVEL, logging.INFO),
+    format='%(asctime)s - %(name)s - %(levelname)s - %(message)s'
+)
 logger = logging.getLogger(__name__)
-# Enable FastMCP auth debugging
-logging.getLogger("fastmcp.server.auth").setLevel(logging.DEBUG)
+
+# Enable FastMCP auth debugging only in development
+if os.getenv("SYSTEMMANAGER_ENV") == "development":
+    logging.getLogger("fastmcp.server.auth").setLevel(logging.DEBUG)
 
 # Create FastMCP instance with auth
 mcp = create_mcp_instance()

src/mcp_server.py

@@ -23,10 +23,17 @@
 from src.tools import stack_tools
 from src.services.log_analyzer import LogAnalyzer
 
-logging.basicConfig(level=logging.DEBUG)
+# Configure logging - use environment variable for log level
+LOG_LEVEL = os.getenv("LOG_LEVEL", "INFO").upper()
+logging.basicConfig(
+    level=getattr(logging, LOG_LEVEL, logging.INFO),
+    format='%(asctime)s - %(name)s - %(levelname)s - %(message)s'
+)
 logger = logging.getLogger(__name__)
-# Enable FastMCP auth debugging
-logging.getLogger("fastmcp.server.auth").setLevel(logging.DEBUG)
+
+# Enable FastMCP auth debugging only in development
+if os.getenv("SYSTEMMANAGER_ENV") == "development":
+    logging.getLogger("fastmcp.server.auth").setLevel(logging.DEBUG)
 
 # Determine authentication mode
 AUTH_MODE = os.getenv("SYSTEMMANAGER_AUTH_MODE", "token").lower()

src/mcp_server_legacy.py

@@ -15,7 +15,15 @@ def create_mcp_instance() -> FastMCP:
 
     if auth_mode == "oidc":
         # TSIDP OIDC Authentication
-        tsidp_url = os.getenv("TSIDP_URL", "https://tsidp.tailf9480.ts.net")
+        # Require explicit TSIDP configuration - no hardcoded defaults
+        tsidp_url = os.getenv("TSIDP_URL")
+        if not tsidp_url:
+            raise ValueError(
+                "TSIDP_URL must be configured when using OIDC authentication. "
+                "Set TSIDP_URL to your Tailscale Identity Provider URL. "
+                "Example: TSIDP_URL=https://tsidp.YOUR-TAILNET.ts.net"
+            )
+
         base_url = os.getenv("SYSTEMMANAGER_BASE_URL", "http://localhost:8080")
         client_id = os.getenv("TSIDP_CLIENT_ID")
         client_secret = os.getenv("TSIDP_CLIENT_SECRET")

src/server/config.py

@@ -8,15 +8,50 @@
 import subprocess
 from typing import Dict, List, Optional
 from pathlib import Path
+from urllib.parse import urlparse
 import git
 
 
 class ComposeStackManager:
     """Manage Docker Compose stacks from Git repositories."""
-    
+
     def __init__(self, stacks_dir: str = "/opt/stacks"):
         self.stacks_dir = Path(stacks_dir)
         self.stacks_dir.mkdir(parents=True, exist_ok=True)
+
+    def _validate_repo_url(self, url: str) -> bool:
+        """Validate repository URL is from allowed sources.
+
+        Args:
+            url: Git repository URL to validate
+
+        Returns:
+            True if URL is allowed, False otherwise
+        """
+        # Get allowed hosts from environment, default to common git providers
+        allowed_hosts_str = os.getenv(
+            "SYSTEMMANAGER_ALLOWED_GIT_HOSTS",
+            "github.com,gitlab.com,bitbucket.org"
+        )
+        allowed_hosts = [h.strip() for h in allowed_hosts_str.split(",") if h.strip()]
+
+        try:
+            parsed = urlparse(url)
+
+            # Only allow https and git protocols
+            if parsed.scheme not in ['https', 'git']:
+                return False
+
+            # Extract hostname (handle git@host:repo format)
+            if '@' in parsed.netloc:
+                host = parsed.netloc.split('@')[-1].split(':')[0]
+            else:
+                host = parsed.netloc.split(':')[0]
+
+            # Check if host is in allowed list
+            return any(host == allowed or host.endswith('.' + allowed) for allowed in allowed_hosts)
+        except Exception:
+            return False
 
     async def deploy_stack(
         self,
@@ -39,8 +74,16 @@ async def deploy_stack(
             Deployment status and details
         """
         stack_path = self.stacks_dir / stack_name
-        
+
         try:
+            # Validate repository URL
+            if not self._validate_repo_url(repo_url):
+                return {
+                    "success": False,
+                    "error": f"Repository URL not allowed: {repo_url}. Configure SYSTEMMANAGER_ALLOWED_GIT_HOSTS to allow this host.",
+                    "stack": stack_name
+                }
+
             # Clone or pull repository
             if stack_path.exists():
                 repo = git.Repo(stack_path)