FGD Fusion Stack Pro

# MCPM Bug Report - Extensive Code Review **Date**: 2025-11-08 **Reviewer**: Claude (Automated Code Review) **Scope**: Complete codebase analysis ## Executive Summary Identified **18 distinct bugs** across the codebase, categorized by severity: - **Critical**: 5 bugs (security vulnerabilities, crash-inducing issues) - **High**: 5 bugs (data corruption, compatibility issues) - **Medium**: 5 bugs (reliability, race conditions) - **Low**: 3 bugs (code quality, maintainability) --- ## CRITICAL SEVERITY BUGS ### 🔴 BUG-001: Command Parsing Logic Error **File**: `claude_bridge.py:77-91` **Severity**: Critical **Type**: Logic Error **Code**: ```python if command == "read" or "read" in user.lower(): ``` **Issue**: The condition `"read" in user.lower()` matches ANY input containing "read", not just the read command. **Impact**: - Commands execute on unintended input - Example: "I already read the file" triggers read command - Affects all commands (read, list, diff, commit) **Reproduction**: ```bash $ python claude_bridge.py You: I already read that file # Incorrectly triggers read command ``` **Recommended Fix**: ```python if command == "read": # Only execute on exact command match ``` --- ### 🔴 BUG-002: IndexError on Short Paths **File**: `test_path_validation.py:19` **Severity**: Critical **Type**: Crash Bug **Code**: ```python is_windows_path = ( ':' in watch_dir_str and watch_dir_str[1:3] == ':\\' or ':' in watch_dir_str and watch_dir_str[1:3] == ':/' ) ``` **Issue**: If `watch_dir_str` is less than 3 characters, accessing `[1:3]` causes IndexError. **Impact**: Script crashes on short directory names like "." or "C:" **Reproduction**: ```python watch_dir_str = "." # Crashes when checking Windows path ``` **Recommended Fix**: ```python is_windows_path = ( len(watch_dir_str) >= 3 and watch_dir_str[1:3] in (':\\', ':/') ) ``` --- ### 🔴 BUG-003: Insecure Path Traversal Check **File**: `mcp_backend.py:418` **Severity**: Critical (Security) **Type**: Security Vulnerability **Code**: ```python def _sanitize(self, rel, base: Path = None): base = base or self.watch_dir p = (base / rel).resolve() if not str(p).startswith(str(base)): raise ValueError("Path traversal blocked") return p ``` **Issue**: String-based path comparison is vulnerable to bypass via: - Symlinks pointing outside base - Path normalization differences - Unicode normalization attacks **Impact**: Attackers could read/write files outside watch_dir **CWE**: CWE-22 (Path Traversal) **Recommended Fix**: ```python def _sanitize(self, rel, base: Path = None): base = base or self.watch_dir p = (base / rel).resolve() try: p.relative_to(base) # Raises ValueError if not relative except ValueError: raise ValueError("Path traversal blocked") return p ``` Or use Python 3.9+ `is_relative_to()`: ```python if not p.is_relative_to(base): raise ValueError("Path traversal blocked") ``` --- ### 🔴 BUG-004: Unbounded Directory Recursion **File**: `server.py:196` **Severity**: Critical (DoS) **Type**: Denial of Service **Code**: ```python @app.get("/api/suggest") @limiter.limit("30/minute") async def suggest(request: Request): try: base = Path(".") dirs = [str(p.relative_to(base)) for p in base.rglob("*") if p.is_dir()] return JSONResponse(dirs[:100]) ``` **Issue**: - `rglob("*")` recursively scans ALL directories without limit - Can scan millions of files on large filesystems - No timeout mechanism **Impact**: - Server hangs/freezes on large directory trees - Memory exhaustion - DoS attack vector even with rate limiting **Reproduction**: ```bash # Point server at root filesystem # Call /api/suggest endpoint # Server freezes scanning entire disk ``` **Recommended Fix**: ```python @app.get("/api/suggest") @limiter.limit("30/minute") async def suggest(request: Request): try: base = Path(".") dirs = [] max_depth = 3 max_dirs = 100 def scan_dir(path: Path, depth: int): if depth > max_depth or len(dirs) >= max_dirs: return try: for p in path.iterdir(): if p.is_dir() and not p.name.startswith('.'): dirs.append(str(p.relative_to(base))) if len(dirs) >= max_dirs: return scan_dir(p, depth + 1) except PermissionError: pass scan_dir(base, 0) return JSONResponse(dirs) ``` --- ### 🔴 BUG-005: Path Traversal in Logs Endpoint **File**: `server.py:383-397` **Severity**: Critical (Security) **Type**: Security Vulnerability **Code**: ```python @app.get("/api/logs", response_class=PlainTextResponse) @limiter.limit("30/minute") async def logs(request: Request, file: str): try: if len(file) > 1024: raise HTTPException(status_code=400, detail="Log path too long") log_path = Path(file).expanduser().resolve() # Validation comes AFTER resolution ``` **Issue**: - `file` parameter not sanitized before `.expanduser().resolve()` - Could be exploited with `~attacker/.ssh/id_rsa` or similar - Validation happens after potentially dangerous operations **Impact**: Information disclosure, access to arbitrary files **CWE**: CWE-22 (Path Traversal) **Recommended Fix**: ```python # Sanitize BEFORE resolution if '..' in file or file.startswith('/') or file.startswith('~'): raise HTTPException(status_code=400, detail="Invalid log path") log_path = Path(file).resolve() ``` --- ## HIGH SEVERITY BUGS ### 🟠 BUG-006: Fragile Git Commit Hash Parsing **File**: `mcp_backend.py:792` **Severity**: High **Type**: Data Corruption **Code**: ```python commit_hash = result.stdout.split()[1] if "commit" in result.stdout else "unknown" ``` **Issue**: - Assumes git output format: `[status <hash> message]` - Will return wrong value if format changes - No validation of hash format **Impact**: Incorrect commit hash stored in memory **Recommended Fix**: ```python import re match = re.search(r'\[([a-f0-9]+)\]', result.stdout) commit_hash = match.group(1) if match else "unknown" ``` --- ### 🟠 BUG-007: Validator Silently Modifies Input **File**: `server.py:106-112` **Severity**: High **Type**: API Contract Violation **Code**: ```python @validator('default_provider') def validate_provider(cls, v): valid_providers = ['grok', 'openai', 'claude', 'ollama'] if v not in valid_providers: logger.warning(f"Invalid provider '{v}', falling back to 'grok'") return 'grok' return v ``` **Issue**: Silently changes invalid input instead of raising validation error **Impact**: - Users get different behavior than requested - Violates principle of least surprise - Hard to debug configuration issues **Recommended Fix**: ```python from pydantic import validator, ValidationError @validator('default_provider') def validate_provider(cls, v): valid_providers = ['grok', 'openai', 'claude', 'ollama'] if v not in valid_providers: raise ValueError( f"Invalid provider '{v}'. Must be one of: {', '.join(valid_providers)}" ) return v ``` --- ### 🟠 BUG-008: Incorrect Filepath Extraction **File**: `claude_bridge.py:81` **Severity**: High **Type**: Logic Error **Code**: ```python if len(parts) < 2: print("MCPM: Error - Please specify a file path") continue filepath = parts[-1] ``` **Issue**: Assumes last argument is filepath, incorrect for multi-word input **Reproduction**: ```bash You: read myfile.txt please # Uses "please" as filename instead of "myfile.txt" ``` **Recommended Fix**: ```python if len(parts) < 2: print("MCPM: Error - Please specify a file path") continue filepath = parts[1] # Always use first argument after command ``` --- ### 🟠 BUG-009: Python 3.8 Incompatibility **File**: `gui_main_pro.py:235` **Severity**: High **Type**: Compatibility **Code**: ```python self.highlighting_rules: List[tuple[re.Pattern, QTextCharFormat]] = [] ``` **Issue**: Uses Python 3.9+ syntax `tuple[...]` but project supports Python 3.8+ **Impact**: SyntaxError on Python 3.8 **Recommended Fix**: ```python from typing import List, Tuple self.highlighting_rules: List[Tuple[re.Pattern, QTextCharFormat]] = [] ``` --- ### 🟠 BUG-010: Memory Leak in Qt Signal Connection **File**: `gui_main_pro.py:996` **Severity**: High **Type**: Memory Leak **Code**: ```python toast.destroyed.connect( lambda: self._toast_notifications.remove(toast) if toast in self._toast_notifications else None ) ``` **Issue**: Lambda in Qt signal connection causes memory leak as Qt holds references **Impact**: Toast notifications never garbage collected **Recommended Fix**: ```python from functools import partial def _remove_toast(self, toast): if toast in self._toast_notifications: self._toast_notifications.remove(toast) # In show_toast(): toast.destroyed.connect(partial(self._remove_toast, toast)) ``` --- ## MEDIUM SEVERITY BUGS ### 🟡 BUG-011: Unreliable Permission Check on Windows **File**: `mcp_backend.py:305` **Severity**: Medium **Type**: Platform Compatibility **Code**: ```python if not os.access(path, os.R_OK | os.W_OK): raise ValueError( f"watch_dir '{path}' must be readable and writable by the MCP server" ) ``` **Issue**: `os.access()` is unreliable on Windows with ACLs **Recommended Fix**: Try actual read/write operations instead --- ### 🟡 BUG-012: Race Condition in Approval Monitor **File**: `mcp_backend.py:243-246` **Severity**: Medium **Type**: Race Condition **Code**: ```python self._approval_task = None # Track approval monitor task self._start_watcher() self._start_approval_monitor() ``` **Issue**: Task initialized to None, then started later. If server stops between these calls, state is inconsistent. --- ### 🟡 BUG-013: Inconsistent Error Handling **Files**: Multiple **Severity**: Medium **Type**: Design Issue **Issue**: Some functions return error strings, others raise exceptions, others log and continue **Impact**: Unpredictable error handling for callers --- ### 🟡 BUG-014: Race Condition in Task Cleanup **File**: `server.py:278-289` **Severity**: Medium **Type**: Race Condition **Code**: ```python def _log_task_result(task: asyncio.Task) -> None: try: task.result() except asyncio.CancelledError: logger.info("MCP server task cancelled") except Exception as exc: logger.error(f"MCP server task crashed: {exc}", exc_info=True) finally: RUN["server_task"] = None RUN["server"] = None ``` **Issue**: Sets server to None in callback, could cause AttributeError if accessed between task completion and callback --- ### 🟡 BUG-015: Log Spam in Timer Callbacks **File**: `gui_main_pro.py:413` **Severity**: Medium **Type**: Logging Issue **Code**: ```python def _advance_gradient(self) -> None: try: self._gradient_shift = (self._gradient_shift + 0.01) % 1.0 self.update() except Exception as e: logger.error(f"Error in gradient animation: {e}") ``` **Issue**: Persistent error will spam logs every 60ms **Recommended Fix**: Add error counter and stop timer after N failures --- ## LOW SEVERITY BUGS ### 🟢 BUG-016: No YAML Schema Validation **Files**: Multiple **Severity**: Low **Type**: Validation **Issue**: Config files loaded without schema validation, errors only appear when fields accessed --- ### 🟢 BUG-017: Global Monkeypatch in Tests **File**: `tests/test_watch_dir_validation.py:51` **Severity**: Low **Type**: Test Isolation **Code**: ```python monkeypatch.setattr(os, "access", lambda path, mode: False) ``` **Issue**: Could affect other tests if run in same process --- ### 🟢 BUG-018: Incomplete Gitignore Implementation **File**: `mcp_backend.py:435-460` **Severity**: Low **Type**: Feature Gap **Issue**: `_matches_gitignore` uses `fnmatch` which doesn't support full .gitignore spec (e.g., negation with `!`) --- ## Recommendations ### Immediate Actions (Critical Bugs) 1. Fix path traversal vulnerabilities (BUG-003, BUG-005) 2. Add depth limit to directory scanning (BUG-004) 3. Fix command parsing in claude_bridge.py (BUG-001) 4. Fix IndexError in path validation (BUG-002) ### Short Term (High Priority) 5. Fix Python 3.8 compatibility (BUG-009) 6. Implement proper input validation (BUG-007) 7. Fix memory leak in Qt connections (BUG-010) 8. Improve git output parsing (BUG-006) ### Medium Term 9. Standardize error handling across codebase 10. Add comprehensive input validation 11. Improve test isolation 12. Add YAML schema validation --- ## Testing Recommendations 1. **Security Testing**: Penetration testing for path traversal vulnerabilities 2. **Fuzzing**: Fuzz test all user input handling (file paths, commands, config) 3. **Load Testing**: Test directory scanning with large filesystems 4. **Compatibility Testing**: Test on Python 3.8, 3.9, 3.10, 3.11+ 5. **Memory Profiling**: Check for memory leaks in long-running GUI sessions --- ## Conclusion The codebase has several critical security vulnerabilities and reliability issues that should be addressed immediately. The most concerning are: - Path traversal vulnerabilities allowing arbitrary file access - DoS vulnerability in directory scanning - Command parsing bugs causing incorrect behavior Total technical debt: **~40 hours** of work to resolve all identified issues. --- **Report Generated**: 2025-11-08 **Review Tool**: Claude Code (Sonnet 4.5) **Files Reviewed**: 7 Python files, 2 YAML configs, 1 test file