Fix pending submission cleanup during account deletion

README.md

@@ -228,6 +228,39 @@ Alternatively, an existing admin can add new reviewers through the web interface
 | Reset user passwords | No | Yes |
 | Manage reviewer accounts | No | Yes |
 
+#### Cleaning review files left by previous account deletions
+
+Account deletion removes both pending uploads and approved archives, including
+private source archives and attached solutions. A filesystem error stops the
+operation and reports an error so it can be retried after fixing permissions.
+Deletion is not transactional: items already removed are not rolled back.
+
+Older versions could delete a crackme's database record while leaving its file
+in `tmp/crackme`. This inflated the dashboard count and produced
+`uuid not found in db` errors in the review queue. No database migration is needed.
+
+Use the application virtual environment and the same `config/config.json` as
+the running site. From the project root, preview the affected files:
+
+```bash
+python script/cleanup_orphaned_pending_files.py
+```
+
+Pause uploads (for example, stop the application service and allow in-flight
+requests to finish), then apply the cleanup:
+
+```bash
+python script/cleanup_orphaned_pending_files.py --apply
+```
+
+Restart the application with the updated code. The script moves only files
+without a corresponding database record into a new private directory under
+`private/orphaned_submissions/`; it prints that directory and preserves relative
+paths for recovery. It never changes database documents or published archives.
+By default, files newer than five minutes are skipped to avoid uploads awaiting
+their database insert. With uploads stopped, add `--min-age-seconds 0` to include
+recent orphans. `--root` and `--config` support non-default deployment paths.
+
 ## Contributing
 
 Interested in contributing? Please read [CONTRIBUTING.md](CONTRIBUTING.md) for guidelines. **Note:** You must contact the maintainer before starting any work.

review/routes.py

@@ -202,6 +202,27 @@ def delete_source_archive(hexid):
         pass
 
 
+def _delete_submission_files(item_type, hexid):
+    """Remove pending and published files before deleting their DB record.
+
+    Missing files are harmless (including markdown-only solutions). Other
+    filesystem errors must stop deletion so the account can be retried.
+    """
+    hexid = str(ObjectId(hexid))
+    paths = [
+        os.path.join(get_tmp_dir(item_type), hexid),
+        os.path.join(get_static_dir(item_type), f'{hexid}.zip'),
+    ]
+    if item_type == 'crackme':
+        paths.append(os.path.join(get_source_dir(), hexid))
+
+    for path in paths:
+        try:
+            os.remove(path)
+        except FileNotFoundError:
+            pass
+
+
 def find_pending_file(item_type, hexid):
     """
     Find a pending submission file by its hexid.
@@ -889,10 +910,10 @@ def approve_pending_solution(hexid):
 
 def delete_approved_solution(solution_uuid):
     """
-    Delete an already-approved solution.
+    Delete a solution, including pending submissions during account deletion.
 
-    Removes the solution from database, deletes the zip file,
-    and decrements the crackme's solution count.
+    Removes pending/published files and the database record. Only approved
+    solutions contribute to the crackme's public solution count.
 
     Args:
         solution_uuid: Solution's hex ID
@@ -911,22 +932,13 @@ def delete_approved_solution(solution_uuid):
         return "Solution not found in database"
 
     crackme_id = solution.get("crackmeid")
-    zip_path = os.path.join(
-        get_static_dir('solution'),
-        f"{solution_uuid}.zip"
-    )
-
-    # Delete zip file (if exists)
-    try:
-        os.remove(zip_path)
-    except Exception:
-        pass
+    _delete_submission_files('solution', solution_uuid)
 
     # Delete from database
     g_crackmesone_db.solution.delete_one({"_id": ObjectId(solution_uuid)})
 
-    # Decrement crackme solution count (always, as long as solution was in database)
-    if crackme_id:
+    # Legacy approved solutions may not have an explicit visibility field.
+    if crackme_id and solution.get('visible', True):
         try:
             g_crackmesone_db.crackme.update_one(
                 {"_id": crackme_id},
@@ -940,9 +952,10 @@ def delete_approved_solution(solution_uuid):
 
 def delete_approved_crackme(crackme_uuid):
     """
-    Delete an already-approved crackme with cascade deletion.
+    Delete a pending or approved crackme with cascade deletion.
 
-    Deletes the crackme, all its solutions, comments, and ratings.
+    Deletes pending/published files, private source, the crackme, and its
+    solutions, comments, ratings and solve records.
 
     Args:
         crackme_uuid: Crackme's hex ID
@@ -953,30 +966,22 @@ def delete_approved_crackme(crackme_uuid):
     if not ObjectId.is_valid(crackme_uuid):
         return "Invalid crackme uuid"
 
+    crackme_uuid = str(ObjectId(crackme_uuid))
     crackme = g_crackmesone_db.crackme.find_one({
         "_id": ObjectId(crackme_uuid)
     })
 
     if not crackme:
         return "Crackme uuid not found"
 
+    _delete_submission_files('crackme', crackme_uuid)
+
     # Cascade delete related data
     deleted = _cascade_delete_crackme_data(
         ObjectId(crackme_uuid),
         crackme_uuid
     )
 
-    # Delete zip file
-    try:
-        os.remove(os.path.join(
-            get_static_dir('crackme'),
-            f"{crackme_uuid}.zip"
-        ))
-    except Exception:
-        pass
-
-    delete_source_archive(crackme_uuid)
-
     # Delete crackme document
     g_crackmesone_db.crackme.delete_one({"_id": ObjectId(crackme_uuid)})
 
@@ -1012,11 +1017,9 @@ def _cascade_delete_crackme_data(crackme_id, crackme_hexid):
 
     # Delete solutions
     for solution in g_crackmesone_db.solution.find({'crackmeid': crackme_id}):
-        try:
-            delete_approved_solution(str(solution["_id"]))
-            deleted['solutions'] += 1
-        except Exception as e:
-            print(f"Warning: Failed to delete solution {solution['_id']}: {e}")
+        # Do not orphan a solution if its files could not be removed.
+        delete_approved_solution(str(solution["_id"]))
+        deleted['solutions'] += 1
 
     # Delete comments
     result = g_crackmesone_db.comment.delete_many({

script/cleanup_orphaned_pending_files.py

@@ -0,0 +1,102 @@
+#!/usr/bin/env python3
+"""Quarantine old review files whose database records no longer exist.
+
+Run from the project root with the application's virtual environment:
+    python script/cleanup_orphaned_pending_files.py
+    python script/cleanup_orphaned_pending_files.py --apply
+
+The default is a dry run. No database documents or published archives are
+modified. Recent files are skipped because uploads write files before inserting
+their database records. Pause uploads while applying a cleanup; use
+--min-age-seconds 0 only with uploads paused and in-flight requests finished.
+"""
+
+import argparse
+import json
+from pathlib import Path
+import shutil
+import stat
+import tempfile
+import time
+
+from bson import ObjectId
+from pymongo import MongoClient
+
+
+def cleanup_orphaned_files(db, root, apply=False, min_age_seconds=300):
+    """Return affected relative paths and the optional quarantine directory."""
+    if min_age_seconds < 0:
+        raise ValueError('min_age_seconds must be non-negative')
+    root = Path(root).resolve()
+    cutoff = time.time() - min_age_seconds
+    affected = []
+    quarantine = None
+
+    for relative_dir, collection_name in (
+        ('tmp/crackme', 'crackme'),
+        ('tmp/solution', 'solution'),
+        ('private/crackme_source', 'crackme'),
+    ):
+        directory = root / relative_dir
+        if not directory.exists():
+            continue
+        for path in sorted(directory.iterdir()):
+            if not ObjectId.is_valid(path.name):
+                continue
+            try:
+                metadata = path.lstat()
+            except FileNotFoundError:
+                continue
+            # Never follow symlinks or process directories, including ones with
+            # valid ObjectId names. Leave new/in-progress uploads alone.
+            if not stat.S_ISREG(metadata.st_mode) or metadata.st_mtime > cutoff:
+                continue
+            # Query the configured database for each file. Connection/query
+            # errors propagate; they must never be interpreted as "no record".
+            if db[collection_name].find_one(
+                    {'_id': ObjectId(path.name)}, {'_id': 1}) is not None:
+                continue
+
+            relative = path.relative_to(root)
+            if apply:
+                if quarantine is None:
+                    parent = root / 'private/orphaned_submissions'
+                    parent.mkdir(parents=True, exist_ok=True, mode=0o700)
+                    quarantine = Path(tempfile.mkdtemp(prefix='cleanup-', dir=parent))
+                    print(f'Quarantine directory: {quarantine}')
+                destination = quarantine / relative
+                destination.parent.mkdir(parents=True, exist_ok=True)
+                shutil.move(str(path), str(destination))
+            affected.append(relative)
+            print(f'{"Quarantined" if apply else "Would quarantine"}: {relative}')
+
+    return affected, quarantine
+
+
+def main(argv=None):
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('--apply', action='store_true', help='Move orphaned files out of the queue')
+    parser.add_argument('--root', type=Path, default=Path(__file__).resolve().parents[1])
+    parser.add_argument('--config', type=Path, help='Defaults to ROOT/config/config.json')
+    parser.add_argument('--min-age-seconds', type=int, default=300,
+                        help='Skip newer files (default: 300 seconds)')
+    args = parser.parse_args(argv)
+    if args.min_age_seconds < 0:
+        parser.error('--min-age-seconds must be non-negative')
+    config_path = args.config or args.root / 'config/config.json'
+    with config_path.open() as handle:
+        config = json.load(handle)['Database']
+
+    with MongoClient(config.get('URL', 'mongodb://127.0.0.1:27017'),
+                     serverSelectionTimeoutMS=10000) as client:
+        client.admin.command('ping')
+        database = client[config.get('Name', 'crackmesone')]
+        affected, _ = cleanup_orphaned_files(
+            database, args.root, args.apply, args.min_age_seconds)
+    print(f'{len(affected)} orphaned file(s) {"quarantined" if args.apply else "found"}.')
+    if not args.apply:
+        print('Dry run: no changes made. Use --apply to quarantine these files.')
+
+
+if __name__ == '__main__':
+    main()