From 0614342cc63e01ad01422af8455955d5ec604f1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 20 Sep 2026 00:29:27 +0000 Subject: [PATCH 1/2] Initial plan From 2efb34cfffe5b90496f86aab182540060d3d5993 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 20 Sep 2026 00:33:00 +0000 Subject: [PATCH 2/2] Fix CodeQL alerts with minimal security hardening Co-authored-by: ariedotcodotnz <78839335+ariedotcodotnz@users.noreply.github.com> --- .github/workflows/backup.yml | 3 ++ .github/workflows/ci.yml | 7 +++++ .github/workflows/deploy.yml | 2 ++ .github/workflows/pylint.yml | 2 ++ webapp/app.py | 10 ++++--- webapp/routes/admin_routes.py | 34 ++++++++++++---------- webapp/routes/character_override_routes.py | 8 ++--- webapp/routes/generation_routes.py | 6 ++-- 8 files changed, 46 insertions(+), 26 deletions(-) diff --git a/.github/workflows/backup.yml b/.github/workflows/backup.yml index f39245b..f8684e0 100644 --- a/.github/workflows/backup.yml +++ b/.github/workflows/backup.yml @@ -6,6 +6,9 @@ on: - cron: '0 2 * * *' workflow_dispatch: # Allow manual backup +permissions: + contents: read + jobs: backup: name: Backup Production Database diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b5000f..99501ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,8 @@ jobs: test: name: Run Tests runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout code @@ -48,6 +50,8 @@ jobs: name: Build Docker Image runs-on: ubuntu-latest needs: test + permissions: + contents: read steps: - name: Checkout code @@ -73,6 +77,9 @@ jobs: security-scan: name: Security Scan runs-on: ubuntu-latest + permissions: + contents: read + security-events: write steps: - name: Checkout code diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 27c9983..0f5eb5d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -13,6 +13,8 @@ jobs: name: Deploy to VM Server runs-on: ubuntu-latest environment: production + permissions: + contents: read steps: - name: Checkout code diff --git a/.github/workflows/pylint.yml b/.github/workflows/pylint.yml index 383e65c..4b57b49 100644 --- a/.github/workflows/pylint.yml +++ b/.github/workflows/pylint.yml @@ -5,6 +5,8 @@ on: [push] jobs: build: runs-on: ubuntu-latest + permissions: + contents: read strategy: matrix: python-version: ["3.8", "3.9", "3.10"] diff --git a/webapp/app.py b/webapp/app.py index 6922f78..12a3322 100644 --- a/webapp/app.py +++ b/webapp/app.py @@ -416,8 +416,9 @@ def _get_health_status(): try: db.session.execute(db.text('SELECT 1')) status["database"] = {"status": "ok"} - except Exception as e: - status["database"] = {"status": "error", "message": str(e)} + except Exception: + app.logger.exception('Database health check failed') + status["database"] = {"status": "error", "message": "Database unavailable"} status["status"] = "degraded" # Check Redis status @@ -426,8 +427,9 @@ def _get_health_status(): r = redis.from_url(REDIS_URL, socket_timeout=2) r.ping() status["redis"] = {"status": "ok"} - except Exception as e: - status["redis"] = {"status": "error", "message": str(e)} + except Exception: + app.logger.exception('Redis health check failed') + status["redis"] = {"status": "error", "message": "Redis unavailable"} status["status"] = "degraded" else: status["redis"] = {"status": "unavailable", "message": "Redis not configured"} diff --git a/webapp/routes/admin_routes.py b/webapp/routes/admin_routes.py index a897353..6fc948d 100644 --- a/webapp/routes/admin_routes.py +++ b/webapp/routes/admin_routes.py @@ -14,6 +14,16 @@ admin_bp = Blueprint('admin', __name__, url_prefix='/admin') +def _is_safe_path(base_dir, candidate_path): + """Return True when candidate_path resolves inside base_dir.""" + real_base_dir = os.path.realpath(base_dir) + real_candidate_path = os.path.realpath(candidate_path) + try: + return os.path.commonpath([real_base_dir, real_candidate_path]) == real_base_dir + except ValueError: + return False + + @admin_bp.route('/') @login_required @admin_required @@ -839,16 +849,14 @@ def error_logs(): if selected_file: filepath = os.path.join(logs_dir, selected_file) - # Security check: ensure the file is within logs_dir - real_logs_dir = os.path.realpath(logs_dir) real_filepath = os.path.realpath(filepath) - if not real_filepath.startswith(real_logs_dir): + if not _is_safe_path(logs_dir, real_filepath): flash('Invalid log file path.', 'error') return redirect(url_for('admin.error_logs')) - if os.path.exists(filepath): + if os.path.exists(real_filepath): try: - with open(filepath, 'r', encoding='utf-8', errors='replace') as f: + with open(real_filepath, 'r', encoding='utf-8', errors='replace') as f: lines = f.readlines() total_lines = len(lines) @@ -913,20 +921,18 @@ def download_log(filename): logs_dir = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'logs') filepath = os.path.join(logs_dir, filename) - # Security check: ensure the file is within logs_dir - real_logs_dir = os.path.realpath(logs_dir) real_filepath = os.path.realpath(filepath) - if not real_filepath.startswith(real_logs_dir): + if not _is_safe_path(logs_dir, real_filepath): flash('Invalid log file path.', 'error') return redirect(url_for('admin.error_logs')) - if not os.path.exists(filepath): + if not os.path.exists(real_filepath): flash('Log file not found.', 'error') return redirect(url_for('admin.error_logs')) log_activity('admin_action', f'Downloaded log file: {filename}') - return send_file(filepath, as_attachment=True, download_name=filename) + return send_file(real_filepath, as_attachment=True, download_name=filename) @admin_bp.route('/logs/clear/', methods=['POST']) @@ -942,20 +948,18 @@ def clear_log(filename): logs_dir = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'logs') filepath = os.path.join(logs_dir, filename) - # Security check: ensure the file is within logs_dir - real_logs_dir = os.path.realpath(logs_dir) real_filepath = os.path.realpath(filepath) - if not real_filepath.startswith(real_logs_dir): + if not _is_safe_path(logs_dir, real_filepath): flash('Invalid log file path.', 'error') return redirect(url_for('admin.error_logs')) - if not os.path.exists(filepath): + if not os.path.exists(real_filepath): flash('Log file not found.', 'error') return redirect(url_for('admin.error_logs')) try: # Truncate the file - with open(filepath, 'w') as f: + with open(real_filepath, 'w', encoding='utf-8') as f: f.write(f'# Log cleared by {current_user.username} at {datetime.now().isoformat()}\n') log_activity('admin_action', f'Cleared log file: {filename}') diff --git a/webapp/routes/character_override_routes.py b/webapp/routes/character_override_routes.py index e0a8c61..fa9c765 100644 --- a/webapp/routes/character_override_routes.py +++ b/webapp/routes/character_override_routes.py @@ -74,10 +74,10 @@ def validate_svg(svg_content): return False, "SVG must have a viewBox or width/height attributes", None return True, None, viewbox_data - except ET.ParseError as e: - return False, f"Invalid XML: {str(e)}", None - except Exception as e: - return False, f"Error validating SVG: {str(e)}", None + except ET.ParseError: + return False, "Invalid SVG: malformed XML content", None + except Exception: + return False, "Invalid SVG content", None @character_override_bp.route('/') diff --git a/webapp/routes/generation_routes.py b/webapp/routes/generation_routes.py index 86500b9..71eca4f 100644 --- a/webapp/routes/generation_routes.py +++ b/webapp/routes/generation_routes.py @@ -110,7 +110,7 @@ def api_v1_generate(): except ValueError as e: # ValueError is typically a validation error (invalid params), safe to show current_app.logger.warning(f'Generation validation error: {e}') - return jsonify({"error": str(e)}), 400 + return jsonify({"error": "Invalid generation parameters."}), 400 except Exception as e: current_app.logger.exception('Generation error') return jsonify({"error": "Failed to generate handwriting. Please check your parameters."}), 400 @@ -153,7 +153,7 @@ def api_v1_generate_svg(): return Response(svg_text, mimetype="image/svg+xml") except ValueError as e: current_app.logger.warning(f'Generation validation error: {e}') - return jsonify({"error": str(e)}), 400 + return jsonify({"error": "Invalid generation parameters."}), 400 except Exception as e: current_app.logger.exception('Generation error (SVG)') return jsonify({"error": "Failed to generate handwriting. Please check your parameters."}), 400 @@ -195,7 +195,7 @@ def generate_svg(): return Response(svg_text, mimetype="image/svg+xml") except ValueError as e: current_app.logger.warning(f'Generation validation error (legacy): {e}') - return jsonify({"error": str(e)}), 400 + return jsonify({"error": "Invalid generation parameters."}), 400 except Exception as e: current_app.logger.exception('Generation error (legacy)') return jsonify({"error": "Failed to generate handwriting. Please check your parameters."}), 400