Harden workflows and Flask routes for CodeQL findings (permissions, path handling, error leakage)

.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

.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

.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

.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"]

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"}

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 @@
 
     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 @@
     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/<filename>', methods=['POST'])
@@ -942,20 +948,18 @@
     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}')

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('/')

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