Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
788 changes: 788 additions & 0 deletions COMPREHENSIVE_BUG_FIX_REPORT_2025-11-16.md

Large diffs are not rendered by default.

11 changes: 9 additions & 2 deletions blastdock/cli/deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,8 +681,15 @@ def deployment_logs(project_name, follow, tail, service):
if service:
cmd.append(service)

# Run command with validated path
subprocess.run(cmd, cwd=str(project_dir.resolve()))
# BUG-NEW-002 FIX: Add error checking for subprocess call
# Note: No timeout for logs command as it may run indefinitely with --follow
result = subprocess.run(
cmd,
cwd=str(project_dir.resolve()),
capture_output=False # Allow output to stream to console
)
if result.returncode != 0 and result.returncode != 130: # 130 is SIGINT (Ctrl+C)
console.print(f"[yellow]Warning: Docker logs command exited with code {result.returncode}[/yellow]")

except KeyboardInterrupt:
console.print("\n[yellow]Log viewing stopped[/yellow]")
Expand Down
51 changes: 38 additions & 13 deletions blastdock/config/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,18 @@ def save_config(self, config: Optional[BlastDockConfig] = None) -> None:

try:
with self._config_lock:
# Create backup before saving
if self.config_file_path.exists():
# BUG-CRIT-001 FIX: Avoid race condition (TOCTOU) by using try-except instead of exists() check
# Create backup before saving (if config exists)
try:
current_config = self.persistence.load_config(self.config_file_path.name)
self.backup_manager.create_backup(
current_config,
current_config,
self.profile,
description="Auto-backup before save"
)
except FileNotFoundError:
# No existing config to backup (first save)
logger.debug(f"No existing config to backup for profile '{self.profile}'")

# Save configuration
config_dict = config.dict()
Expand Down Expand Up @@ -308,23 +312,44 @@ def temporary_config(self, **overrides):
self._config = BlastDockConfig(**original_config)

def switch_profile(self, profile_name: str) -> None:
"""Switch to a different configuration profile"""
"""Switch to a different configuration profile

BUG-CRIT-006 FIX: Added comprehensive error handling and rollback logic
to prevent data loss if save or load operations fail.
"""
if profile_name == self.profile:
return


old_profile = self.profile
old_config = self._config

# Save current profile if auto_save is enabled
if self.auto_save and self._config is not None:
self.save_config()

try:
self.save_config()
except Exception as e:
logger.error(f"Failed to save current profile '{self.profile}' before switching: {e}")
raise ConfigurationError(
f"Cannot switch profile: failed to save current profile '{self.profile}': {e}"
)

# Switch to new profile
old_profile = self.profile
self.profile = profile_name
self._config = None # Force reload

# Load new profile configuration
self.load_config()

logger.info(f"Switched from profile '{old_profile}' to '{profile_name}'")

# Load new profile configuration with rollback on failure
try:
self.load_config()
logger.info(f"Switched from profile '{old_profile}' to '{profile_name}'")
except Exception as e:
# Rollback to old profile on load failure
logger.error(f"Failed to load profile '{profile_name}': {e}")
self.profile = old_profile
self._config = old_config
raise ConfigurationError(
f"Failed to switch to profile '{profile_name}': {e}. "
f"Rolled back to profile '{old_profile}'."
)

def export_config(self, export_path: str, format: str = 'yaml',
include_secrets: bool = False) -> None:
Expand Down
13 changes: 9 additions & 4 deletions blastdock/docker/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -523,14 +523,19 @@ def prune_containers(self, filters: Optional[Dict[str, str]] = None) -> Dict[str

# Parse output for metrics
output = result.stdout
import re
if 'Total reclaimed space' in output:
import re
space_match = re.search(r'Total reclaimed space: ([\d.]+\w+)', output)
if space_match:
prune_result['space_reclaimed'] = space_match.group(1)

# Count removed containers (rough estimate from output)
container_lines = [line for line in output.split('\n') if line.strip() and len(line) == 64]

# BUG-NEW-001 FIX: Use robust regex pattern for container ID detection
# Container IDs are hex strings of 12-64 characters (short or long form)
container_id_pattern = re.compile(r'^[a-f0-9]{12,64}$')
container_lines = [
line.strip() for line in output.split('\n')
if line.strip() and container_id_pattern.match(line.strip())
]
prune_result['containers_removed'] = len(container_lines)

return prune_result
Expand Down
10 changes: 8 additions & 2 deletions blastdock/docker/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,14 @@ def check_container_health(self, container_id: str) -> Dict[str, Any]:
result = self.docker_client.execute_command([
'docker', 'inspect', container_id, '--format', '{{json .}}'
])

container_info = json.loads(result.stdout)

# BUG-CRIT-004 FIX: Add JSON parsing error handling
try:
container_info = json.loads(result.stdout)
except json.JSONDecodeError as e:
health_info['issues'].append(f"Failed to parse container inspect output: {e}")
health_info['status'] = 'error'
return health_info

# Basic status
state = container_info.get('State', {})
Expand Down
5 changes: 4 additions & 1 deletion blastdock/performance/async_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,10 @@ def _estimate_memory_usage(self, template_data: Dict[str, Any]) -> float:
# Rough estimation based on string representation length
import sys
return sys.getsizeof(str(template_data)) / 1024 / 1024 # MB
except Exception:
except (TypeError, ValueError, AttributeError) as e:
# BUG-CRIT-007 FIX: Use specific exceptions instead of generic Exception
# Return 0 if memory estimation fails
logger.debug(f"Failed to estimate memory usage: {e}")
return 0.0

def get_stats(self) -> Dict[str, Any]:
Expand Down
6 changes: 4 additions & 2 deletions blastdock/security/docker_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,10 @@ def check_image_security(self, image_name: str) -> Dict[str, Any]:
'recommendation': 'Use recent image versions'
})
security_score -= 15
except Exception:
pass
except (ValueError, TypeError, KeyError) as e:
# BUG-CRIT-007 FIX: Use specific exceptions instead of generic Exception
# Skip image age check if metadata is malformed
logger.debug(f"Failed to parse image age: {e}")

# Check for latest tag
if any(':latest' in tag for tag in repo_tags):
Expand Down
Loading
Loading