diff --git a/src/tui/managers/container_manager.py b/src/tui/managers/container_manager.py index 666d66bb..91f022be 100644 --- a/src/tui/managers/container_manager.py +++ b/src/tui/managers/container_manager.py @@ -2,7 +2,8 @@ import asyncio import json -import subprocess +import os +import re import time from dataclasses import dataclass, field from enum import Enum @@ -121,6 +122,36 @@ class ContainerManager: self._compose_search_log += f"\n 3. Falling back to: {cwd_path.absolute()}" return Path(filename) + def _get_env_from_file(self) -> Dict[str, str]: + """Read environment variables from .env file, prioritizing file values over os.environ. + + Uses python-dotenv's load_dotenv() for standard .env file parsing, which handles: + - Quoted values (single and double quotes) + - Variable expansion (${VAR}) + - Multiline values + - Escaped characters + - Comments + + This ensures Docker Compose commands use the latest values from .env file, + even if os.environ has stale values. + """ + from dotenv import load_dotenv + + env = dict(os.environ) # Start with current environment + env_file = Path(".env") + + if env_file.exists(): + try: + # Load .env file with override=True to ensure file values take precedence + # This loads into os.environ, then we copy to our dict + load_dotenv(dotenv_path=env_file, override=True) + # Update our dict with all environment variables (including those from .env) + env.update(os.environ) + except Exception as e: + logger.debug(f"Error reading .env file for Docker Compose: {e}") + + return env + def is_available(self) -> bool: """Check if container runtime with compose is available.""" return (self.runtime_info.runtime_type != RuntimeType.NONE and @@ -153,7 +184,6 @@ class ContainerManager: continue try: - import re content = compose_file.read_text() current_service = None in_ports_section = False @@ -245,11 +275,15 @@ class ContainerManager: cmd.extend(args) try: + # Get environment variables from .env file to ensure latest values + env = self._get_env_from_file() + process = await asyncio.create_subprocess_exec( *cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=Path.cwd(), + env=env, ) stdout, stderr = await process.communicate() @@ -287,11 +321,15 @@ class ContainerManager: cmd.extend(args) try: + # Get environment variables from .env file to ensure latest values + env = self._get_env_from_file() + process = await asyncio.create_subprocess_exec( *cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT, cwd=Path.cwd(), + env=env, ) if process.stdout: @@ -356,11 +394,15 @@ class ContainerManager: cmd.extend(args) try: + # Get environment variables from .env file to ensure latest values + env = self._get_env_from_file() + process = await asyncio.create_subprocess_exec( *cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT, cwd=Path.cwd(), + env=env, ) except Exception as e: success_flag["value"] = False @@ -926,7 +968,6 @@ class ContainerManager: async for message, replace_last in self._stream_compose_command(["up", "-d"], up_success, cpu_mode): # Detect error patterns in the output - import re lower_msg = message.lower() # Check for common error patterns @@ -1110,11 +1151,15 @@ class ContainerManager: cmd.extend(["logs", "-f", service_name]) try: + # Get environment variables from .env file to ensure latest values + env = self._get_env_from_file() + process = await asyncio.create_subprocess_exec( *cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT, cwd=Path.cwd(), + env=env, ) if process.stdout: diff --git a/src/tui/managers/env_manager.py b/src/tui/managers/env_manager.py index 51e2a11f..4f5ee461 100644 --- a/src/tui/managers/env_manager.py +++ b/src/tui/managers/env_manager.py @@ -1,5 +1,6 @@ """Environment configuration manager for OpenRAG TUI.""" +import os import secrets import string from dataclasses import dataclass, field @@ -7,12 +8,10 @@ from datetime import datetime from pathlib import Path from typing import Dict, List, Optional +from dotenv import load_dotenv from utils.logging_config import get_logger -logger = get_logger(__name__) - from ..utils.validation import ( - sanitize_env_value, validate_documents_paths, validate_google_oauth_client_id, validate_non_empty, @@ -20,6 +19,8 @@ from ..utils.validation import ( validate_url, ) +logger = get_logger(__name__) + @dataclass class EnvConfig: @@ -119,9 +120,15 @@ class EnvManager: return f"'{escaped_value}'" def load_existing_env(self) -> bool: - """Load existing .env file if it exists, or fall back to environment variables.""" - import os + """Load existing .env file if it exists, or fall back to environment variables. + Uses python-dotenv's load_dotenv() for standard .env file parsing, which handles: + - Quoted values (single and double quotes) + - Variable expansion (${VAR}) + - Multiline values + - Escaped characters + - Comments + """ # Map env vars to config attributes # These are environment variable names, not actual secrets attr_map = { # pragma: allowlist secret @@ -158,36 +165,23 @@ class EnvManager: loaded_from_file = False - # Try to load from .env file first + # Load .env file using python-dotenv for standard parsing + # override=True ensures .env file values take precedence over existing environment variables if self.env_file.exists(): try: - with open(self.env_file, "r") as f: - for line in f: - line = line.strip() - if not line or line.startswith("#"): - continue - - if "=" in line: - key, value = line.split("=", 1) - key = key.strip() - value = sanitize_env_value(value) - - if key in attr_map: - setattr(self.config, attr_map[key], value) - + # Load .env file with override=True to ensure file values take precedence + load_dotenv(dotenv_path=self.env_file, override=True) loaded_from_file = True - + logger.debug(f"Loaded .env file from {self.env_file}") except Exception as e: logger.error("Error loading .env file", error=str(e)) - # Fall back to environment variables if .env file doesn't exist or failed to load - if not loaded_from_file: - logger.info("No .env file found, loading from environment variables") - for env_key, attr_name in attr_map.items(): - value = os.environ.get(env_key, "") - if value: - setattr(self.config, attr_name, value) - return True + # Map environment variables to config attributes + # This works whether values came from .env file or existing environment variables + for env_key, attr_name in attr_map.items(): + value = os.environ.get(env_key, "") + if value: + setattr(self.config, attr_name, value) return loaded_from_file @@ -546,23 +540,19 @@ class EnvManager: """Ensure OPENRAG_VERSION is set in .env file to match TUI version.""" try: from ..utils.version_check import get_current_version + import os current_version = get_current_version() if current_version == "unknown": return # Check if OPENRAG_VERSION is already set in .env if self.env_file.exists(): - env_content = self.env_file.read_text() - if "OPENRAG_VERSION" in env_content: - # Already set, check if it needs updating - for line in env_content.splitlines(): - if line.strip().startswith("OPENRAG_VERSION"): - existing_value = line.split("=", 1)[1].strip() - existing_value = sanitize_env_value(existing_value) - if existing_value == current_version: - # Already correct, no update needed - return - break + # Load .env file using load_dotenv + load_dotenv(dotenv_path=self.env_file, override=False) + existing_value = os.environ.get("OPENRAG_VERSION", "") + if existing_value and existing_value == current_version: + # Already correct, no update needed + return # Set or update OPENRAG_VERSION self.config.openrag_version = current_version diff --git a/src/tui/screens/welcome.py b/src/tui/screens/welcome.py index 629614c0..146b437f 100644 --- a/src/tui/screens/welcome.py +++ b/src/tui/screens/welcome.py @@ -41,7 +41,8 @@ class WelcomeScreen(Screen): self.has_env_file = self.env_manager.env_file.exists() # Load .env file if it exists - load_dotenv() + # override=True ensures .env file values take precedence over existing environment variables + load_dotenv(override=True) # Check OAuth config immediately self.has_oauth_config = bool(os.getenv("GOOGLE_OAUTH_CLIENT_ID")) or bool( diff --git a/src/tui/utils/validation.py b/src/tui/utils/validation.py index c91c4f00..cdbeb810 100644 --- a/src/tui/utils/validation.py +++ b/src/tui/utils/validation.py @@ -96,21 +96,6 @@ def validate_non_empty(value: str) -> bool: return bool(value and value.strip()) -def sanitize_env_value(value: str) -> str: - """Sanitize environment variable value.""" - # Remove leading/trailing whitespace - value = value.strip() - - # Remove quotes if they wrap the entire value - if len(value) >= 2: - if (value.startswith('"') and value.endswith('"')) or ( - value.startswith("'") and value.endswith("'") - ): - value = value[1:-1] - - return value - - def validate_documents_paths(paths_str: str) -> tuple[bool, str, list[str]]: """ Validate comma-separated documents paths for volume mounting.