Преглед изворни кода

Fixed critical bug.

  The delete_archive() function in backend/app/services/archive.py now has these safety checks:

  1. Empty path check: Refuses to delete files if file_path is empty/whitespace
  2. Path containment check: Verifies archive_dir is inside settings.archive_dir
  3. Depth check: Ensures we're at least 1 level deep inside archive directory
  4. Logging: All security refusals are logged with SECURITY prefix
  5. Database record still deleted: Even if file deletion is refused, the orphan DB record is cleaned up

  Before the fix:
  file_path = settings.base_dir / archive.file_path  # If empty: /opt/bambuddy
  archive_dir = file_path.parent                      # = /opt
  shutil.rmtree(archive_dir)                         # DELETES /opt!
maziggy пре 4 месеци
родитељ
комит
63c08c8154
2 измењених фајлова са 127 додато и 5 уклоњено
  1. 44 5
      backend/app/services/archive.py
  2. 83 0
      bambuddy-issue-notes.txt

+ 44 - 5
backend/app/services/archive.py

@@ -1,5 +1,6 @@
 import hashlib
 import json
+import logging
 import re
 import shutil
 import zipfile
@@ -15,6 +16,8 @@ from backend.app.models.archive import PrintArchive
 from backend.app.models.filament import Filament
 from backend.app.models.printer import Printer
 
+logger = logging.getLogger(__name__)
+
 
 class ThreeMFParser:
     """Parser for Bambu Lab 3MF files."""
@@ -917,11 +920,47 @@ class ArchiveService:
         if not archive:
             return False
 
-        # Delete files
-        file_path = settings.base_dir / archive.file_path
-        if file_path.exists():
-            archive_dir = file_path.parent
-            shutil.rmtree(archive_dir, ignore_errors=True)
+        # Delete files - with CRITICAL safety checks to prevent accidental deletion
+        # of parent directories (e.g., /opt) if file_path is empty/malformed
+        if archive.file_path and archive.file_path.strip():
+            file_path = settings.base_dir / archive.file_path
+            if file_path.exists():
+                archive_dir = file_path.parent
+
+                # Safety check 1: archive_dir must be inside archive_dir
+                try:
+                    archive_dir.resolve().relative_to(settings.archive_dir.resolve())
+                except ValueError:
+                    logger.error(
+                        f"SECURITY: Refusing to delete archive {archive_id} - "
+                        f"path {archive_dir} is outside archive directory {settings.archive_dir}"
+                    )
+                    # Still delete the database record, just not the files
+                    await self.db.delete(archive)
+                    await self.db.commit()
+                    return True
+
+                # Safety check 2: archive_dir must be at least 1 level deep inside archive_dir
+                # (should be archive_dir/uuid/file.3mf, so parent should be archive_dir/uuid)
+                try:
+                    relative_path = archive_dir.resolve().relative_to(settings.archive_dir.resolve())
+                    if len(relative_path.parts) < 1:
+                        logger.error(
+                            f"SECURITY: Refusing to delete archive {archive_id} - "
+                            f"path {archive_dir} is not deep enough inside archive directory"
+                        )
+                        await self.db.delete(archive)
+                        await self.db.commit()
+                        return True
+                except ValueError:
+                    pass  # Already handled above
+
+                shutil.rmtree(archive_dir, ignore_errors=True)
+        else:
+            logger.error(
+                f"SECURITY: Refusing to delete files for archive {archive_id} - "
+                f"file_path is empty or invalid: '{archive.file_path}'"
+            )
 
         # Delete database record
         await self.db.delete(archive)

+ 83 - 0
bambuddy-issue-notes.txt

@@ -0,0 +1,83 @@
+=== BAMBUDDY FILE DELETION ISSUE - Jan 8, 2026 ===
+=== ROOT CAUSE IDENTIFIED ===
+
+WHAT HAPPENED:
+- /opt was COMPLETELY DELETED on TWO containers:
+  - Container 109 (claude): ~11:22 and ~12:22
+  - Container 107 (3dp): ~13:28
+- Container 107 was "untouched" (no SSH, no Claude Code) - just running BamBuddy
+
+ROOT CAUSE FOUND:
+Bug in backend/app/services/archive.py delete_archive() function (lines 914-929):
+
+    file_path = settings.base_dir / archive.file_path
+    if file_path.exists():
+        archive_dir = file_path.parent
+        shutil.rmtree(archive_dir, ignore_errors=True)  # <-- THE BUG
+
+If archive.file_path is EMPTY or MALFORMED:
+- file_path = /opt/bambuddy / "" = /opt/bambuddy
+- archive_dir = file_path.parent = /opt
+- shutil.rmtree("/opt") --> DELETES ENTIRE /opt DIRECTORY!
+
+TRIGGER:
+- User was deleting archives via BamBuddy web UI on container 107 (3dp)
+- One archive had corrupted/empty file_path in database
+- Deleting that archive triggered shutil.rmtree("/opt")
+- This deleted the entire /opt directory including BamBuddy itself
+
+TIMELINE FOR CONTAINER 107 (3dp):
+- 13:28:19 - Normal operation (WebSocket disconnect)
+- 13:28:44 - DELETE /api/v1/archives/* requests failing with 500
+            (database already gone because /opt was deleted)
+- ls -la / shows root directory modified at 13:28
+
+FIX APPLIED (on container 109):
+Safety checks added to delete_archive() in archive.py:
+1. Check if file_path is not empty
+2. Verify archive_dir is inside settings.archive_dir
+3. Ensure archive_dir is at least 2 levels deep
+4. Log error and refuse to delete if checks fail
+
+TO INVESTIGATE AFTER ROLLBACK:
+On container 107, after rolling back to autodaily260108003006:
+
+    # Find corrupted archive records
+    sqlite3 /opt/bambuddy/data/bambuddy.db \
+      "SELECT id, filename, file_path FROM print_archives
+       WHERE file_path = '' OR file_path IS NULL
+       OR file_path NOT LIKE 'archive/%';"
+
+    # Check all file_path values
+    sqlite3 /opt/bambuddy/data/bambuddy.db \
+      "SELECT id, file_path FROM print_archives ORDER BY id;"
+
+CONTAINER 109 (this host):
+- Were you also deleting archives around 11:22 and 12:22?
+- Same bug could have been triggered here too
+
+PROXMOX COMMANDS FOR ROLLBACK:
+    # Container 107 (3dp)
+    pct rollback 107 autodaily260108003006
+    pct start 107
+
+    # Container 109 (claude) - already done via UI
+    # Current snapshot: autodaily260108003004
+
+WHAT TO DO NEXT:
+1. Rollback container 107 to morning snapshot
+2. Run the SQL query above to find corrupted archive
+3. Apply the fix from container 109 to container 107
+4. Understand how the file_path got corrupted in the first place
+
+THE FIX (apply to both containers):
+In backend/app/services/archive.py, the delete_archive function now has:
+- Empty file_path check
+- Path traversal protection (relative_to check)
+- Minimum depth check (must be 2+ levels inside archive dir)
+- Error logging for refused deletions
+
+NOT CLAUDE CODE'S FAULT:
+This was a bug in BamBuddy's own code that was triggered by:
+1. Corrupted database record (unknown how it got corrupted)
+2. User action (deleting archives via web UI)