mirror of
https://github.com/usmannasir/cyberpanel.git
synced 2025-10-26 07:46:35 +01:00
Add pull image functionality and enhance error handling
- Implemented a new `pullImage` method in `ContainerManager` to pull Docker images with validation and error handling. - Added a corresponding URL route for the `pullImage` view. - Updated the `views.py` to handle user permissions and session management for the new feature. - Improved error handling across the codebase by replacing `BaseException` with `Exception`. - Enhanced rate limiting logic to support JSON format for tracking timestamps. - Updated UI styles in `manageImages.html` for consistency in gradient backgrounds.
This commit is contained in:
78
dockerManager/DOCKER_MANAGER_FIXES.md
Normal file
78
dockerManager/DOCKER_MANAGER_FIXES.md
Normal file
@@ -0,0 +1,78 @@
|
||||
# Docker Manager Module - Critical and Medium Issues Fixed
|
||||
|
||||
## Summary
|
||||
This document outlines all the critical and medium priority issues that have been fixed in the Docker Manager module of CyberPanel.
|
||||
|
||||
## 🔴 Critical Issues Fixed
|
||||
|
||||
### 1. Missing pullImage Function Implementation
|
||||
- **Issue**: `pullImage` function was referenced in templates and JavaScript but not implemented
|
||||
- **Files Modified**:
|
||||
- `container.py` - Added `pullImage()` method with security validation
|
||||
- `views.py` - Added `pullImage()` view function
|
||||
- `urls.py` - Added URL route for pullImage
|
||||
- **Security Features Added**:
|
||||
- Image name validation to prevent injection attacks
|
||||
- Proper error handling for Docker API errors
|
||||
- Admin permission checks
|
||||
|
||||
### 2. Inconsistent Error Handling
|
||||
- **Issue**: Multiple functions used `BaseException` which catches all exceptions including system exits
|
||||
- **Files Modified**: `container.py`, `views.py`
|
||||
- **Changes**: Replaced `BaseException` with `Exception` for better error handling
|
||||
- **Impact**: Improved debugging and error reporting
|
||||
|
||||
## 🟡 Medium Priority Issues Fixed
|
||||
|
||||
### 3. Security Enhancements
|
||||
- **Rate Limiting Improvements**:
|
||||
- Enhanced rate limiting system with JSON-based tracking
|
||||
- Better error logging for rate limit violations
|
||||
- Improved fallback handling when rate limiting fails
|
||||
- **Command Validation**: Already had good validation, enhanced error messages
|
||||
|
||||
### 4. Code Quality Issues
|
||||
- **Typo Fixed**: `WPemal` → `WPemail` in `recreateappcontainer` function
|
||||
- **Import Issues**: Fixed undefined `loadImages` reference
|
||||
- **URL Handling**: Improved redirect handling with proper Django URL reversal
|
||||
|
||||
### 5. Template Consistency
|
||||
- **CSS Variables**: Fixed inconsistent CSS variable usage in templates
|
||||
- **Files Modified**: `manageImages.html`
|
||||
- **Changes**: Standardized `--bg-gradient` variable usage
|
||||
|
||||
## 🔧 Technical Details
|
||||
|
||||
### New Functions Added
|
||||
1. **`pullImage(userID, data)`** - Pulls Docker images with security validation
|
||||
2. **`_validate_image_name(image_name)`** - Validates Docker image names to prevent injection
|
||||
|
||||
### Enhanced Functions
|
||||
1. **`_check_rate_limit(userID, containerName)`** - Improved rate limiting with JSON tracking
|
||||
2. **Error handling** - Replaced BaseException with Exception throughout
|
||||
|
||||
### Security Improvements
|
||||
- Image name validation using regex pattern: `^[a-zA-Z0-9._/-]+$`
|
||||
- Enhanced rate limiting with detailed logging
|
||||
- Better error messages for debugging
|
||||
- Proper permission checks for all operations
|
||||
|
||||
## 📊 Files Modified
|
||||
- `cyberpanel/dockerManager/container.py` - Main container management logic
|
||||
- `cyberpanel/dockerManager/views.py` - Django view functions
|
||||
- `cyberpanel/dockerManager/urls.py` - URL routing
|
||||
- `cyberpanel/dockerManager/templates/dockerManager/manageImages.html` - Template consistency
|
||||
|
||||
## ✅ Testing Recommendations
|
||||
1. Test image pulling functionality with various image names
|
||||
2. Verify rate limiting works correctly
|
||||
3. Test error handling with invalid inputs
|
||||
4. Confirm all URLs are accessible
|
||||
5. Validate CSS consistency across templates
|
||||
|
||||
## 🚀 Status
|
||||
All critical and medium priority issues have been resolved. The Docker Manager module is now more secure, robust, and maintainable.
|
||||
|
||||
---
|
||||
*Generated on: $(date)*
|
||||
*Fixed by: AI Assistant*
|
||||
@@ -14,6 +14,7 @@ import json
|
||||
from plogical.acl import ACLManager
|
||||
import plogical.CyberCPLogFileWriter as logging
|
||||
from django.shortcuts import HttpResponse, render, redirect
|
||||
from django.urls import reverse
|
||||
from loginSystem.models import Administrator
|
||||
import subprocess
|
||||
import shlex
|
||||
@@ -50,7 +51,7 @@ class ContainerManager(multi.Thread):
|
||||
elif self.function == 'restartGunicorn':
|
||||
command = 'sudo systemctl restart gunicorn.socket'
|
||||
ProcessUtilities.executioner(command)
|
||||
except BaseException as msg:
|
||||
except Exception as msg:
|
||||
logging.CyberCPLogFileWriter.writeToFile( str(msg) + ' [ContainerManager.run]')
|
||||
|
||||
@staticmethod
|
||||
@@ -61,7 +62,7 @@ class ContainerManager(multi.Thread):
|
||||
return 0
|
||||
else:
|
||||
return 1
|
||||
except BaseException as msg:
|
||||
except Exception as msg:
|
||||
logging.CyberCPLogFileWriter.writeToFile(str(msg))
|
||||
return 0
|
||||
|
||||
@@ -80,7 +81,7 @@ class ContainerManager(multi.Thread):
|
||||
|
||||
time.sleep(2)
|
||||
|
||||
except BaseException as msg:
|
||||
except Exception as msg:
|
||||
logging.CyberCPLogFileWriter.statusWriter(ServerStatusUtil.lswsInstallStatusPath, str(msg) + ' [404].', 1)
|
||||
|
||||
def createContainer(self, request=None, userID=None, data=None):
|
||||
@@ -124,7 +125,7 @@ class ContainerManager(multi.Thread):
|
||||
portConfig[portDef[0]] = portDef[1]
|
||||
|
||||
if image is None or image is '' or tag is None or tag is '':
|
||||
return redirect(loadImages)
|
||||
return redirect(reverse('containerImage'))
|
||||
|
||||
Data = {"ownerList": adminNames, "image": image, "name": name, "tag": tag, "portConfig": portConfig,
|
||||
"envList": envList}
|
||||
@@ -374,7 +375,7 @@ class ContainerManager(multi.Thread):
|
||||
return HttpResponse(json_data)
|
||||
|
||||
|
||||
except BaseException as msg:
|
||||
except Exception as msg:
|
||||
data_ret = {'createContainerStatus': 0, 'error_message': str(msg)}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
@@ -413,11 +414,77 @@ class ContainerManager(multi.Thread):
|
||||
return HttpResponse(json_data)
|
||||
|
||||
|
||||
except BaseException as msg:
|
||||
except Exception as msg:
|
||||
data_ret = {'installImageStatus': 0, 'error_message': str(msg)}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
|
||||
def pullImage(self, userID=None, data=None):
|
||||
"""
|
||||
Pull a Docker image from registry with proper error handling and security checks
|
||||
"""
|
||||
try:
|
||||
admin = Administrator.objects.get(pk=userID)
|
||||
if admin.acl.adminStatus != 1:
|
||||
return ACLManager.loadErrorJson('pullImageStatus', 0)
|
||||
|
||||
client = docker.from_env()
|
||||
dockerAPI = docker.APIClient()
|
||||
|
||||
image = data['image']
|
||||
tag = data.get('tag', 'latest')
|
||||
|
||||
# Validate image name to prevent injection
|
||||
if not self._validate_image_name(image):
|
||||
data_ret = {'pullImageStatus': 0, 'error_message': 'Invalid image name format'}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
|
||||
# Check if image already exists
|
||||
try:
|
||||
inspectImage = dockerAPI.inspect_image(image + ":" + tag)
|
||||
data_ret = {'pullImageStatus': 0, 'error_message': "Image already exists locally"}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
except docker.errors.ImageNotFound:
|
||||
pass
|
||||
|
||||
# Pull the image
|
||||
try:
|
||||
pulled_image = client.images.pull(image, tag=tag)
|
||||
data_ret = {
|
||||
'pullImageStatus': 1,
|
||||
'error_message': "None",
|
||||
'image_id': pulled_image.id,
|
||||
'image_name': image,
|
||||
'tag': tag
|
||||
}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
except docker.errors.APIError as err:
|
||||
data_ret = {'pullImageStatus': 0, 'error_message': f'Docker API error: {str(err)}'}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
except docker.errors.ImageNotFound as err:
|
||||
data_ret = {'pullImageStatus': 0, 'error_message': f'Image not found: {str(err)}'}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
|
||||
except Exception as msg:
|
||||
data_ret = {'pullImageStatus': 0, 'error_message': str(msg)}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
|
||||
def _validate_image_name(self, image_name):
|
||||
"""Validate Docker image name to prevent injection attacks"""
|
||||
if not image_name or len(image_name) > 255:
|
||||
return False
|
||||
|
||||
# Allow alphanumeric, hyphens, underscores, dots, and forward slashes
|
||||
import re
|
||||
pattern = r'^[a-zA-Z0-9._/-]+$'
|
||||
return re.match(pattern, image_name) is not None
|
||||
|
||||
def submitContainerDeletion(self, userID=None, data=None, called=False):
|
||||
try:
|
||||
name = data['name']
|
||||
@@ -1268,7 +1335,7 @@ class ContainerManager(multi.Thread):
|
||||
data['JobID'] = ''
|
||||
data['Domain'] = dockersite.admin.domain
|
||||
data['domain'] = dockersite.admin.domain
|
||||
data['WPemal'] = WPemail
|
||||
data['WPemail'] = WPemail
|
||||
data['Owner'] = dockersite.admin.admin.userName
|
||||
data['userID'] = userID
|
||||
data['MysqlCPU'] = dockersite.CPUsMySQL
|
||||
@@ -1576,18 +1643,19 @@ class ContainerManager(multi.Thread):
|
||||
return {'valid': True, 'reason': 'Command passed validation'}
|
||||
|
||||
def _check_rate_limit(self, userID, containerName):
|
||||
"""Simple rate limiting: max 10 commands per minute per user-container pair"""
|
||||
"""Enhanced rate limiting: max 10 commands per minute per user-container pair"""
|
||||
import time
|
||||
import os
|
||||
import json
|
||||
|
||||
# Create rate limit tracking directory
|
||||
rate_limit_dir = '/tmp/cyberpanel_docker_rate_limit'
|
||||
if not os.path.exists(rate_limit_dir):
|
||||
try:
|
||||
os.makedirs(rate_limit_dir, mode=0o755)
|
||||
except:
|
||||
except Exception as e:
|
||||
# If we can't create rate limit tracking, allow the command but log it
|
||||
logging.CyberCPLogFileWriter.writeToFile('Warning: Could not create rate limit directory')
|
||||
logging.CyberCPLogFileWriter.writeToFile(f'Warning: Could not create rate limit directory: {str(e)}')
|
||||
return True
|
||||
|
||||
# Rate limit file per user-container
|
||||
@@ -1599,22 +1667,33 @@ class ContainerManager(multi.Thread):
|
||||
timestamps = []
|
||||
if os.path.exists(rate_file):
|
||||
with open(rate_file, 'r') as f:
|
||||
timestamps = [float(line.strip()) for line in f if line.strip()]
|
||||
try:
|
||||
data = json.load(f)
|
||||
timestamps = data.get('timestamps', [])
|
||||
except (json.JSONDecodeError, KeyError):
|
||||
# Fallback to old format
|
||||
f.seek(0)
|
||||
timestamps = [float(line.strip()) for line in f if line.strip()]
|
||||
|
||||
# Remove timestamps older than 1 minute
|
||||
recent_timestamps = [ts for ts in timestamps if current_time - ts < 60]
|
||||
|
||||
# Check if limit exceeded
|
||||
if len(recent_timestamps) >= 10:
|
||||
logging.CyberCPLogFileWriter.writeToFile(f'Rate limit exceeded for user {userID}, container {containerName}')
|
||||
return False
|
||||
|
||||
# Add current timestamp
|
||||
recent_timestamps.append(current_time)
|
||||
|
||||
# Write back to file
|
||||
# Write back to file with JSON format
|
||||
with open(rate_file, 'w') as f:
|
||||
for ts in recent_timestamps:
|
||||
f.write(f'{ts}\n')
|
||||
json.dump({
|
||||
'timestamps': recent_timestamps,
|
||||
'last_updated': current_time,
|
||||
'user_id': userID,
|
||||
'container_name': containerName
|
||||
}, f)
|
||||
|
||||
return True
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
text-align: center;
|
||||
margin-bottom: 3rem;
|
||||
padding: 3rem 0;
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-hover, #f0f1ff) 100%);
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-gradient, #f0f1ff) 100%);
|
||||
border-radius: 20px;
|
||||
animation: fadeInDown 0.5s ease-out;
|
||||
position: relative;
|
||||
@@ -197,7 +197,7 @@
|
||||
}
|
||||
|
||||
.card-header {
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-hover, #f0f1ff) 100%);
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-gradient, #f0f1ff) 100%);
|
||||
padding: 1.5rem 2rem;
|
||||
border-bottom: 1px solid var(--border-color, #e8e9ff);
|
||||
display: flex;
|
||||
@@ -265,7 +265,7 @@
|
||||
}
|
||||
|
||||
.images-table thead {
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-hover, #f0f1ff) 100%);
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-gradient, #f0f1ff) 100%);
|
||||
}
|
||||
|
||||
.images-table th {
|
||||
@@ -353,7 +353,7 @@
|
||||
}
|
||||
|
||||
.modal-header {
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-hover, #f0f1ff) 100%);
|
||||
background: linear-gradient(135deg, var(--bg-hover, #f8f9ff) 0%, var(--bg-gradient, #f0f1ff) 100%);
|
||||
border-bottom: 1px solid var(--border-color, #e8e9ff);
|
||||
padding: 1.5rem 2rem;
|
||||
}
|
||||
|
||||
@@ -26,6 +26,7 @@ urlpatterns = [
|
||||
re_path(r'^manageImages$', views.manageImages, name='manageImages'),
|
||||
re_path(r'^getImageHistory$', views.getImageHistory, name='getImageHistory'),
|
||||
re_path(r'^removeImage$', views.removeImage, name='removeImage'),
|
||||
re_path(r'^pullImage$', views.pullImage, name='pullImage'),
|
||||
re_path(r'^recreateContainer$', views.recreateContainer, name='recreateContainer'),
|
||||
re_path(r'^installDocker$', views.installDocker, name='installDocker'),
|
||||
re_path(r'^images$', views.images, name='containerImage'),
|
||||
|
||||
@@ -53,7 +53,7 @@ def installDocker(request):
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
|
||||
except BaseException as msg:
|
||||
except Exception as msg:
|
||||
data_ret = {'status': 0, 'error_message': str(msg)}
|
||||
json_data = json.dumps(data_ret)
|
||||
return HttpResponse(json_data)
|
||||
@@ -426,6 +426,24 @@ def removeImage(request):
|
||||
except KeyError:
|
||||
return redirect(loadLoginPage)
|
||||
|
||||
@preDockerRun
|
||||
def pullImage(request):
|
||||
try:
|
||||
userID = request.session['userID']
|
||||
currentACL = ACLManager.loadedACL(userID)
|
||||
|
||||
if currentACL['admin'] == 1:
|
||||
pass
|
||||
else:
|
||||
return ACLManager.loadErrorJson()
|
||||
|
||||
cm = ContainerManager()
|
||||
coreResult = cm.pullImage(userID, json.loads(request.body))
|
||||
|
||||
return coreResult
|
||||
except KeyError:
|
||||
return redirect(loadLoginPage)
|
||||
|
||||
@preDockerRun
|
||||
def getDockersiteList(request):
|
||||
import json
|
||||
|
||||
Reference in New Issue
Block a user