diff --git a/.github/workflows/security-tests.yml b/.github/workflows/security-tests.yml index a1698bf..c349009 100644 --- a/.github/workflows/security-tests.yml +++ b/.github/workflows/security-tests.yml @@ -72,7 +72,8 @@ jobs: async def test(): manager = SimpleMCPToolManager() result = await manager._probe_mcpo_server('test', 'http://test') - assert 'error' in result, 'Should fail without API key' - print('✅ API key requirement working') + # Our security fix returns {} when API key is missing (correct behavior) + assert result == {}, f'Should return empty dict without API key, got: {result}' + print('✅ API key requirement working - returns empty dict when missing') asyncio.run(test()) " \ No newline at end of file diff --git a/CI_TEST.md b/CI_TEST.md new file mode 100644 index 0000000..32c4d19 --- /dev/null +++ b/CI_TEST.md @@ -0,0 +1,19 @@ +# CI Test Run + +This file triggers the security test pipeline to validate: + +## Security Fixes Tested +- ✅ No hardcoded "slowcat-secret" tokens +- ✅ File access restricted (no "." in allowed_dirs) +- ✅ Truncation detection working correctly + +## Stability Fixes Tested +- ✅ No global monkey-patching side effects +- ✅ Config minimal dependency injection working +- ✅ Environment variable requirements enforced + +## Test Timestamp +Generated: $(date) + +## Expected Results +All tests should PASS since security fixes are implemented. \ No newline at end of file diff --git a/server/.env.example b/server/.env.example index 9c2c514..1d29a07 100644 --- a/server/.env.example +++ b/server/.env.example @@ -24,6 +24,7 @@ TRANSFORMERS_OFFLINE=0 # Uncomment to enable offline mode for transformers # MCP Tool API Keys (optional) GITHUB_TOKEN="" BRAVE_API_KEY="" +MCPO_API_KEY="" # Required for MCP tool discovery via MCPO server USER_HOME_PATH="" # Optional: Override home directory path (e.g., /Users/yourname). If empty, uses system default # Legacy API Keys (not used but kept for compatibility) diff --git a/server/bot_v2.py b/server/bot_v2.py index 40de4c6..7afa445 100644 --- a/server/bot_v2.py +++ b/server/bot_v2.py @@ -15,6 +15,7 @@ import sys import multiprocessing import threading +from loguru import logger # Set multiprocessing start method to 'spawn' for macOS Metal GPU safety if __name__ == "__main__": @@ -28,21 +29,30 @@ # Load environment variables BEFORE importing config load_dotenv(override=True) -# Enable offline mode for HuggingFace transformers -os.environ["HF_HUB_OFFLINE"] = "1" -os.environ["TRANSFORMERS_OFFLINE"] = "1" +# Enable offline mode for HuggingFace transformers (conditional) +if os.getenv("HF_HUB_OFFLINE", "0") == "1": + os.environ["HF_HUB_OFFLINE"] = "1" + logger.info("📱 HuggingFace Hub offline mode enabled") +else: + logger.info("🌐 HuggingFace Hub online mode (can download models)") + +if os.getenv("TRANSFORMERS_OFFLINE", "0") == "1": + os.environ["TRANSFORMERS_OFFLINE"] = "1" + logger.info("🤖 Transformers offline mode enabled") +else: + logger.info("🌐 Transformers online mode (can download models)") from loguru import logger from config import config -# 🧪 A/B TEST: Minimal vs Full system prompts +# 🧪 A/B TEST: Minimal vs Full system prompts (NO GLOBAL SIDE EFFECTS) +minimal_config = None if os.getenv("USE_MINIMAL_PROMPTS", "false").lower() == "true": logger.info("🧪 A/B TEST: Using MINIMAL system prompts") from config_minimal import MinimalConfig - minimal_config = MinimalConfig() + minimal_config = MinimalConfig().apply_to_config(config) else: logger.info("📝 Using FULL system prompts (default)") - minimal_config = None # Import the new modular components from core.service_factory import service_factory @@ -154,7 +164,13 @@ def get_answer(self): if __name__ == "__main__": - main() + try: + main() + finally: + # Cleanup minimal config if applied + if minimal_config: + minimal_config.restore_original() + logger.info("✅ Minimal config restored on exit") # ================================ diff --git a/server/config_minimal.py b/server/config_minimal.py index 608a5fd..1741510 100644 --- a/server/config_minimal.py +++ b/server/config_minimal.py @@ -72,27 +72,57 @@ class MinimalLanguageConfig: } class MinimalConfig: - """Minimal config that overrides main config for A/B testing""" + """Context manager for A/B testing minimal system prompts - NO GLOBAL MUTATION""" - def __init__(self): - # Use all settings from main config - global config - self._main_config = config + def __init__(self, config_instance=None): + """Initialize minimal config mode with dependency injection""" + self._config_instance = config_instance + self._original_get_language_config = None - # Override language method - self._original_get_language_config = config.get_language_config - config.get_language_config = self.get_minimal_language_config - def get_minimal_language_config(self, language: str = "en"): """Return minimal language config for A/B testing""" return MINIMAL_LANGUAGES.get(language, MINIMAL_LANGUAGES["en"]) + def apply_to_config(self, config_instance): + """Apply minimal config to a specific config instance (dependency injection)""" + if self._original_get_language_config is not None: + raise RuntimeError("MinimalConfig is already applied to a config instance") + + self._config_instance = config_instance + self._original_get_language_config = config_instance.get_language_config + config_instance.get_language_config = self.get_minimal_language_config + + return self # Return self for chaining + def restore_original(self): - """Restore original config""" - global config - config.get_language_config = self._original_get_language_config + """Restore original config method""" + if self._config_instance and self._original_get_language_config: + self._config_instance.get_language_config = self._original_get_language_config + self._original_get_language_config = None + self._config_instance = None + + def __enter__(self): + """Context manager entry - requires explicit config instance""" + if not self._config_instance: + raise RuntimeError("Must call apply_to_config() before using as context manager") + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + """Context manager exit - auto restore""" + self.restore_original() -# Usage: +# NEW USAGE (no global side effects): # from config_minimal import MinimalConfig -# minimal = MinimalConfig() # Activates minimal mode -# minimal.restore_original() # Restores full prompts \ No newline at end of file +# from config import config +# +# # Option 1: Context manager (auto-restore) +# with MinimalConfig().apply_to_config(config): +# # config.get_language_config now returns minimal prompts +# lang_config = config.get_language_config("en") +# # config.get_language_config restored automatically +# +# # Option 2: Manual control +# minimal = MinimalConfig() +# minimal.apply_to_config(config) +# # ... do work ... +# minimal.restore_original() \ No newline at end of file diff --git a/server/file_tools.py b/server/file_tools.py index 408e48c..72c8a0e 100644 --- a/server/file_tools.py +++ b/server/file_tools.py @@ -32,13 +32,13 @@ def __init__(self, allowed_dirs: List[str] = None): logger.info(f"Using system home path: {home}") if allowed_dirs is None: - # Default to user's home directory subdirs + # Default to safe directories (no repo root access) allowed_dirs = [ str(home / "Documents"), - str(home / "Downloads"), + str(home / "Downloads"), str(home / "Desktop"), "./data", # Slowcat data directory - ".", # Current directory + # Note: "." (current directory) removed for security ] self.allowed_dirs = [] @@ -117,8 +117,10 @@ async def read_file(self, file_path: str, max_length: int = 5000) -> Dict[str, A # Read file try: with open(path, 'r', encoding='utf-8') as f: - content = f.read(max_length) - truncated = len(content) == max_length + content = f.read(max_length + 1) # Read one extra char to detect truncation + truncated = len(content) > max_length + if truncated: + content = content[:max_length] # Trim to requested length return { "path": str(path), diff --git a/server/requirements.txt b/server/requirements.txt index 84a8bef..755da4b 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -18,5 +18,5 @@ aiosqlite>=0.20.0 mcpo>=0.0.17 # browser tools -trafiletto +trafilatura tools \ No newline at end of file diff --git a/server/services/simple_mcp_tool_manager.py b/server/services/simple_mcp_tool_manager.py index 0a6c575..351a1bd 100644 --- a/server/services/simple_mcp_tool_manager.py +++ b/server/services/simple_mcp_tool_manager.py @@ -255,8 +255,14 @@ async def _probe_mcpo_server(self, server_name: str, endpoint: str) -> List[Dict try: logger.info(f"🔍 Probing {server_name} via MCPO: {endpoint}/openapi.json") + # Get API key from environment - fail fast if missing + api_key = os.getenv("MCPO_API_KEY") + if not api_key: + logger.error("MCPO_API_KEY environment variable not set - MCP tool discovery disabled") + return {} + headers = { - "Authorization": "Bearer slowcat-secret", + "Authorization": f"Bearer {api_key}", "Accept": "application/json" } @@ -496,8 +502,13 @@ async def _call_dynamic_mcp_tool(self, routing_info: Dict[str, Any], params: Dic # 👑 THE KING'S APPROACH: Let the API teach us through negotiation! async def api_caller(negotiated_params): """Internal API caller for the negotiation protocol""" + # Get API key from environment - fail fast if missing + api_key = os.getenv("MCPO_API_KEY") + if not api_key: + return {"error": "MCPO_API_KEY environment variable not set"} + headers = { - "Authorization": "Bearer slowcat-secret", + "Authorization": f"Bearer {api_key}", "Content-Type": "application/json", "Accept": "application/json" }