mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-11-10 05:53:05 +09:00
fix(security): replace bare excepts, stop leaking exception text
Bare excepts in sockets/servers/system now catch specific errors and log; malformed metrics from/to returns 400. docker cleanup and update-check no longer echo str(e).
This commit is contained in:
1 parent
800a54dc1a
commit
dd9003d458
6 files changed
+58
-27
No files matched your search
+25
-14
@@ -1,3 +1,5 @@
|
||||
import logging
|
||||
|
||||
from flask import Blueprint, request, jsonify
|
||||
from flask_jwt_extended import jwt_required, get_jwt_identity
|
||||
from app.models import User, Application
|
||||
@@ -7,6 +9,8 @@ from app import db, paths
|
||||
from app.error_reporting import unexpected_response
|
||||
from app.api._request import json_object
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
docker_bp = Blueprint('docker', __name__)
|
||||
|
||||
|
||||
@@ -505,8 +509,9 @@ def docker_cleanup():
|
||||
}
|
||||
else:
|
||||
results['containers'] = {'removed': 0, 'success': True}
|
||||
except Exception as e:
|
||||
results['containers'] = {'error': str(e)}
|
||||
except Exception:
|
||||
logger.exception('Docker cleanup step failed')
|
||||
results['containers'] = {'error': 'Cleanup step failed; see server log'}
|
||||
|
||||
# Remove dangling images
|
||||
try:
|
||||
@@ -515,8 +520,9 @@ def docker_cleanup():
|
||||
'success': result['success'],
|
||||
'output': result['output'].strip()
|
||||
}
|
||||
except Exception as e:
|
||||
results['images'] = {'error': str(e)}
|
||||
except Exception:
|
||||
logger.exception('Docker cleanup step failed')
|
||||
results['images'] = {'error': 'Cleanup step failed; see server log'}
|
||||
|
||||
# Remove unused networks
|
||||
try:
|
||||
@@ -525,8 +531,9 @@ def docker_cleanup():
|
||||
'success': result['success'],
|
||||
'output': result['output'].strip()
|
||||
}
|
||||
except Exception as e:
|
||||
results['networks'] = {'error': str(e)}
|
||||
except Exception:
|
||||
logger.exception('Docker cleanup step failed')
|
||||
results['networks'] = {'error': 'Cleanup step failed; see server log'}
|
||||
|
||||
# Remove unused volumes (optional)
|
||||
if include_volumes:
|
||||
@@ -536,8 +543,9 @@ def docker_cleanup():
|
||||
'success': result['success'],
|
||||
'output': result['output'].strip()
|
||||
}
|
||||
except Exception as e:
|
||||
results['volumes'] = {'error': str(e)}
|
||||
except Exception:
|
||||
logger.exception('Docker cleanup step failed')
|
||||
results['volumes'] = {'error': 'Cleanup step failed; see server log'}
|
||||
else:
|
||||
results['volumes'] = {'skipped': True, 'message': 'Volume cleanup not requested'}
|
||||
|
||||
@@ -581,24 +589,27 @@ def cleanup_all_apps():
|
||||
try:
|
||||
compose_result = DockerService.compose_down(app.root_path, volumes=True, remove_orphans=True)
|
||||
app_result['steps'].append({'action': 'docker_down', 'success': compose_result.get('success', False)})
|
||||
except Exception as e:
|
||||
app_result['steps'].append({'action': 'docker_down', 'error': str(e)})
|
||||
except Exception:
|
||||
logger.exception('App cleanup step failed')
|
||||
app_result['steps'].append({'action': 'docker_down', 'error': 'Step failed; see server log'})
|
||||
app_result['success'] = False
|
||||
|
||||
# Delete folder
|
||||
try:
|
||||
shutil.rmtree(app.root_path)
|
||||
app_result['steps'].append({'action': 'delete_folder', 'success': True})
|
||||
except Exception as e:
|
||||
app_result['steps'].append({'action': 'delete_folder', 'error': str(e)})
|
||||
except Exception:
|
||||
logger.exception('App cleanup step failed')
|
||||
app_result['steps'].append({'action': 'delete_folder', 'error': 'Step failed; see server log'})
|
||||
app_result['success'] = False
|
||||
|
||||
# Remove from database
|
||||
try:
|
||||
db.session.delete(app)
|
||||
app_result['steps'].append({'action': 'delete_db', 'success': True})
|
||||
except Exception as e:
|
||||
app_result['steps'].append({'action': 'delete_db', 'error': str(e)})
|
||||
except Exception:
|
||||
logger.exception('App cleanup step failed')
|
||||
app_result['steps'].append({'action': 'delete_db', 'error': 'Step failed; see server log'})
|
||||
app_result['success'] = False
|
||||
|
||||
results.append(app_result)
|
||||
|
||||
@@ -883,15 +883,15 @@ def get_server_metrics(server_id):
|
||||
try:
|
||||
from_dt = datetime.fromisoformat(from_time.replace('Z', '+00:00'))
|
||||
query = query.filter(ServerMetrics.timestamp >= from_dt)
|
||||
except:
|
||||
pass
|
||||
except ValueError:
|
||||
raise ValidationError("Invalid 'from' timestamp; use ISO 8601")
|
||||
|
||||
if to_time:
|
||||
try:
|
||||
to_dt = datetime.fromisoformat(to_time.replace('Z', '+00:00'))
|
||||
query = query.filter(ServerMetrics.timestamp <= to_dt)
|
||||
except:
|
||||
pass
|
||||
except ValueError:
|
||||
raise ValidationError("Invalid 'to' timestamp; use ISO 8601")
|
||||
|
||||
metrics = query.order_by(ServerMetrics.timestamp.desc()).limit(limit).all()
|
||||
|
||||
|
||||
@@ -409,7 +409,8 @@ def _compare_versions(v1, v2):
|
||||
elif a > b:
|
||||
return 1
|
||||
return 0
|
||||
except:
|
||||
except (ValueError, AttributeError, TypeError):
|
||||
current_app.logger.warning('Could not compare versions %r and %r', v1, v2)
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
@@ -84,7 +84,7 @@ def check_update():
|
||||
def parse_version(v):
|
||||
try:
|
||||
return tuple(int(x) for x in v.split('.'))
|
||||
except:
|
||||
except ValueError:
|
||||
return (0, 0, 0)
|
||||
|
||||
if parse_version(latest_version) > parse_version(current_version):
|
||||
@@ -96,7 +96,8 @@ def check_update():
|
||||
else:
|
||||
result['error'] = f'GitHub API error: {e.code}'
|
||||
except Exception as e:
|
||||
result['error'] = f'Failed to check for updates: {str(e)}'
|
||||
current_app.logger.warning('Update check failed: %s', e)
|
||||
result['error'] = 'Failed to check for updates'
|
||||
|
||||
# Cache the result
|
||||
_update_cache['last_check'] = time.time()
|
||||
|
||||
+10
-6
@@ -5,6 +5,8 @@ import threading
|
||||
import time
|
||||
import queue
|
||||
import re
|
||||
import logging
|
||||
import subprocess
|
||||
|
||||
from app.services.system_service import SystemService
|
||||
from app.services.log_service import LogService, LogStreamer
|
||||
@@ -12,6 +14,8 @@ from app.services.docker_service import DockerService
|
||||
from app import sockets_rooms as rooms
|
||||
from app.utils.background_loop import BackgroundLoop
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class AuthorizedSocketIO(SocketIO):
|
||||
"""Recheck audience authorization before every server-side delivery.
|
||||
@@ -775,11 +779,11 @@ def handle_subscribe_container_logs(data):
|
||||
try:
|
||||
process.terminate()
|
||||
process.wait(timeout=2)
|
||||
except:
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
try:
|
||||
process.kill()
|
||||
except:
|
||||
pass
|
||||
except OSError as kill_err:
|
||||
logger.debug('Could not kill log stream process: %s', kill_err)
|
||||
|
||||
thread = threading.Thread(target=stream_logs, daemon=True)
|
||||
thread.start()
|
||||
@@ -836,11 +840,11 @@ def stop_container_log_stream(sid: str):
|
||||
try:
|
||||
process.terminate()
|
||||
process.wait(timeout=2)
|
||||
except:
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
try:
|
||||
process.kill()
|
||||
except:
|
||||
pass
|
||||
except OSError as kill_err:
|
||||
logger.debug('Could not kill log stream process: %s', kill_err)
|
||||
|
||||
|
||||
def emit_container_log(app_id: int, line: str, level: str = 'info'):
|
||||
|
||||
@@ -108,3 +108,17 @@ def test_list_servers_metrics_are_batched_not_per_row(client, auth_headers, serv
|
||||
assert count_metric_queries() == baseline, (
|
||||
'metrics lookup grew with the server count — it must stay batched'
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('param', ['from', 'to'])
|
||||
def test_metrics_rejects_a_malformed_timestamp_with_400(app, client, auth_headers, param):
|
||||
with app.app_context():
|
||||
server = _mk_server('delta')
|
||||
db.session.commit()
|
||||
server_id = server.id
|
||||
|
||||
resp = client.get(f'/api/v1/servers/{server_id}/metrics?{param}=not-a-date',
|
||||
headers=auth_headers)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert resp.get_json()['error']
|
||||
Reference in new issue
Block a user