Przeglądaj źródła

fix: sécurité, robustesse et qualité suite à la revue de code

- Whitelist des hooks système avant stockage (injection subprocess)
- Garde .isdigit() sur le parsing des transfer_targets (évite 500)
- Statut "warning" si un transfert échoue (archive OK, transfert KO)
- Badge ambre "transfert partiel" dans dashboard et historique
- Nettoyage des JobDestination orphelines à la suppression d'une destination ou instance
- Imports DB consolidés en tête de settings.py (suppression des imports tardifs)
- Préservation du nom/type/cron sur retour anticipé erreur hooks (nouveau job)
- Tests : rétrocompat ancien format destination_name/remote_instance_name (singulier)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cedric Hansen 1 miesiąc temu
rodzic
commit
076129ae0d

+ 2 - 1
sources/blueprints/destinations.py

@@ -10,7 +10,7 @@ from flask import (
     url_for,
 )
 
-from db import db, Destination
+from db import db, Destination, JobDestination
 
 bp = Blueprint("dest", __name__)
 
@@ -40,6 +40,7 @@ def destination_edit(dest_id):
 @bp.route("/destinations/<int:dest_id>/delete", methods=["POST"])
 def destination_delete(dest_id):
     dest = db.get_or_404(Destination, dest_id)
+    JobDestination.query.filter_by(dest_type="ssh", dest_id=dest_id).delete()
     db.session.delete(dest)
     db.session.commit()
     flash(f"Destination « {dest.name} » supprimée.", "success")

+ 7 - 3
sources/blueprints/jobs.py

@@ -349,12 +349,16 @@ def _save_job(job):
     if job_type == "ynh_app":
         cfg = {"app_id": f.get("app_id", ""), "core_only": f.get("core_only") == "1"}
     elif job_type == "ynh_system":
-        selected = f.getlist("system_hooks")
         all_hooks = {"conf_ynh_settings", "conf_ynh_firewall", "conf_ssowat", "conf_nginx",
                      "conf_ynh_certs", "conf_ynh_domain", "conf_ynh_user", "data_home", "data_mail"}
+        selected = [h for h in f.getlist("system_hooks") if h in all_hooks]
         if not selected:
             flash("Sélectionnez au moins un hook système.", "error")
-            return render_template("job_form.html", job=job,
+            # Pour un nouveau job, on crée un objet temporaire (non persisté) pour
+            # que le template puisse pré-remplir le nom et le type.
+            job_for_form = job or Job(name=name, type=job_type,
+                                      cron_expr=(f.get("cron_expr") or "").strip())
+            return render_template("job_form.html", job=job_for_form,
                                    ynh_apps=get_ynh_apps(exclude_app_ids=_used_app_ids(exclude_job_id=job.id if job else None)),
                                    destinations=Destination.query.filter_by(enabled=True).all(),
                                    remote_instances=RemoteInstance.query.order_by(RemoteInstance.name).all())
@@ -436,7 +440,7 @@ def _save_job(job):
         if t.startswith("dest:") else
         JobDestination(dest_type="instance", dest_id=int(t[5:]))
         for t in transfer_targets
-        if t.startswith("dest:") or t.startswith("inst:")
+        if (t.startswith("dest:") or t.startswith("inst:")) and t[5:].isdigit()
     ]
     job.updated_at = datetime.utcnow()
     db.session.commit()

+ 2 - 1
sources/blueprints/network.py

@@ -12,7 +12,7 @@ from flask import (
     url_for,
 )
 
-from db import db, Job, Run, RemoteInstance, RemoteRun
+from db import db, Job, Run, RemoteInstance, RemoteRun, JobDestination
 from db import _size_human
 
 bp = Blueprint("network", __name__)
@@ -44,6 +44,7 @@ def remote_instance_edit(inst_id):
 @bp.route("/remote-instances/<int:inst_id>/delete", methods=["POST"])
 def remote_instance_delete(inst_id):
     inst = db.get_or_404(RemoteInstance, inst_id)
+    JobDestination.query.filter_by(dest_type="instance", dest_id=inst_id).delete()
     db.session.delete(inst)
     db.session.commit()
     flash(f"Instance « {inst.name} » supprimée.", "success")

+ 1 - 6
sources/blueprints/settings.py

@@ -14,7 +14,7 @@ from flask import (
     url_for,
 )
 
-from db import db, Setting
+from db import db, Setting, Job, Destination, RemoteInstance, JobDestination
 
 bp = Blueprint("cfg", __name__)
 
@@ -68,7 +68,6 @@ def settings():
 
         return redirect(url_for("cfg.settings"))
 
-    from db import Destination, RemoteInstance
     cfg = {k: _get_setting(k) for k in _SETTING_KEYS}
     cfg.setdefault("smtp_port", "587")
     cfg["smtp_tls"] = cfg.get("smtp_tls") or "1"
@@ -85,8 +84,6 @@ def settings():
 
 @bp.route("/settings/export-config")
 def export_config():
-    from db import Job, Destination, RemoteInstance
-
     jobs_data = []
     for j in Job.query.order_by(Job.name).all():
         dest_names = [jd.label for jd in j.job_destinations if jd.dest_type == "ssh"]
@@ -147,7 +144,6 @@ def export_config():
 
 @bp.route("/settings/import-config", methods=["POST"])
 def import_config():
-    from db import Job, Destination, RemoteInstance
     from scheduler import schedule_job, remove_job
 
     f = request.files.get("config_file")
@@ -234,7 +230,6 @@ def import_config():
         inst_names = j_data.get("remote_instance_names") or (
             [j_data["remote_instance_name"]] if j_data.get("remote_instance_name") else []
         )
-        from db import JobDestination
         new_jds = []
         for dname in dest_names:
             if dname and dname in dest_by_name:

+ 4 - 1
sources/jobs/ynh_backup.py

@@ -54,6 +54,7 @@ def execute_job(job_id):
             db.session.commit()
 
         # Checkpoint 2 : transfert vers chaque destination
+        transfer_errors = 0
         for jd in job.job_destinations:
             obj = jd.resolved
             if obj is None:
@@ -70,6 +71,7 @@ def execute_job(job_id):
                     run.log_text += f"\n{transfer_log}"
                 except Exception as transfer_exc:
                     run.log_text += f"\n⚠ Transfert échoué : {transfer_exc}"
+                    transfer_errors += 1
                 db.session.commit()
             elif jd.dest_type == "instance":
                 run.log_text += f"\n\nTransfert HTTP → {obj.name} ({obj.url}) : démarré…"
@@ -80,9 +82,10 @@ def execute_job(job_id):
                     run.log_text += f"\n{transfer_log}"
                 except Exception as transfer_exc:
                     run.log_text += f"\n⚠ Transfert HTTP échoué : {transfer_exc}"
+                    transfer_errors += 1
                 db.session.commit()
 
-        run.status = "success"
+        run.status = "warning" if transfer_errors else "success"
 
     except Exception as exc:
         run.status = "error"

+ 6 - 3
sources/templates/dashboard_local.html

@@ -8,6 +8,7 @@
 {% set enabled_count = jobs | selectattr('enabled') | list | length %}
 {% set recent_runs = last_runs.values() | select | list %}
 {% set success_count = recent_runs | selectattr('status', 'equalto', 'success') | list | length %}
+{% set warning_count = recent_runs | selectattr('status', 'equalto', 'warning') | list | length %}
 {% set error_count = recent_runs | selectattr('status', 'equalto', 'error') | list | length %}
 {% set running_count = recent_runs | selectattr('status', 'equalto', 'running') | list | length %}
 
@@ -24,10 +25,10 @@
   </div>
   <div class="bg-white rounded-xl border border-gray-200 px-5 py-4">
     <p class="text-xs text-gray-500 uppercase tracking-wide">Erreurs</p>
-    <p class="text-3xl font-bold {% if error_count > 0 %}text-red-600{% else %}text-gray-400{% endif %} mt-1">
-      {{ error_count }}
+    <p class="text-3xl font-bold {% if error_count > 0 %}text-red-600{% elif warning_count > 0 %}text-amber-500{% else %}text-gray-400{% endif %} mt-1">
+      {{ error_count + warning_count }}
     </p>
-    <p class="text-xs text-gray-400 mt-1">dernière exécution</p>
+    <p class="text-xs text-gray-400 mt-1">dernière exécution{% if warning_count > 0 and error_count == 0 %} (transfert partiel){% endif %}</p>
   </div>
   <div class="bg-white rounded-xl border border-gray-200 px-5 py-4">
     <p class="text-xs text-gray-500 uppercase tracking-wide">En cours</p>
@@ -122,6 +123,8 @@
                 {% if run %}
                   {% if run.status == 'success' %}
                     <span class="bg-green-100 text-green-700 text-xs font-medium px-2 py-0.5 rounded-full">✓ succès</span>
+                  {% elif run.status == 'warning' %}
+                    <span class="bg-amber-100 text-amber-700 text-xs font-medium px-2 py-0.5 rounded-full">⚠ transfert partiel</span>
                   {% elif run.status == 'error' %}
                     <span class="bg-red-100 text-red-700 text-xs font-medium px-2 py-0.5 rounded-full">✗ erreur</span>
                   {% elif run.status == 'running' %}

+ 3 - 1
sources/templates/job_history.html

@@ -59,6 +59,8 @@
               <td class="px-6 py-3">
                 {% if run.status == 'success' %}
                   <span class="bg-green-100 text-green-700 text-xs font-medium px-2 py-0.5 rounded-full">✓ succès</span>
+                {% elif run.status == 'warning' %}
+                  <span class="bg-amber-100 text-amber-700 text-xs font-medium px-2 py-0.5 rounded-full">⚠ transfert partiel</span>
                 {% elif run.status == 'error' %}
                   <span class="bg-red-100 text-red-700 text-xs font-medium px-2 py-0.5 rounded-full">✗ erreur</span>
                 {% elif run.status == 'running' %}
@@ -84,7 +86,7 @@
                 {% endif %}
               </td>
               <td class="px-6 py-3">
-                {% if run.status == 'success' and run.archive_name %}
+                {% if run.status in ('success', 'warning') and run.archive_name %}
                   <div class="flex flex-col gap-1">
                     <a href="{{ url_for('jobs.archive_restore', archive_name=run.archive_name) }}"
                        class="text-xs text-orange-600 hover:text-orange-800 hover:underline whitespace-nowrap">

+ 37 - 0
sources/tests/test_config_io.py

@@ -243,3 +243,40 @@ class TestImportConfig:
     def test_sans_fichier_redirige(self, client):
         resp = client.post("/settings/import-config", data={})
         assert resp.status_code == 302
+
+    def test_compat_ancien_format_destination_name(self, client, app):
+        """Ancien format destination_name (singulier) doit être reconnu à l'import."""
+        payload = _payload(
+            jobs=[_job_data(destination_name="VPS-OVH")],
+            destinations=[_dest_data()],
+        )
+        # Supprimer les clés du nouveau format pour simuler un vieux fichier
+        payload["jobs"][0].pop("destination_names", None)
+        payload["jobs"][0].pop("remote_instance_names", None)
+        _do_import(client, payload)
+
+        with app.app_context():
+            from db import Job, Destination
+            dest = Destination.query.filter_by(name="VPS-OVH").first()
+            job = Job.query.filter_by(name="Mon job").first()
+            assert len(job.job_destinations) == 1
+            assert job.job_destinations[0].dest_type == "ssh"
+            assert job.job_destinations[0].dest_id == dest.id
+
+    def test_compat_ancien_format_remote_instance_name(self, client, app):
+        """Ancien format remote_instance_name (singulier) doit être reconnu à l'import."""
+        payload = _payload(
+            jobs=[_job_data(remote_instance_name="Tom")],
+            remote_instances=[{"name": "Tom", "url": "https://tom.example.com", "api_key": "k"}],
+        )
+        payload["jobs"][0].pop("destination_names", None)
+        payload["jobs"][0].pop("remote_instance_names", None)
+        _do_import(client, payload)
+
+        with app.app_context():
+            from db import Job, RemoteInstance
+            inst = RemoteInstance.query.filter_by(name="Tom").first()
+            job = Job.query.filter_by(name="Mon job").first()
+            assert len(job.job_destinations) == 1
+            assert job.job_destinations[0].dest_type == "instance"
+            assert job.job_destinations[0].dest_id == inst.id