archived zip fix and security updates

pdf_renamer.py

@@ -18,10 +18,18 @@ app = Flask(__name__)
 app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1, x_prefix=1)
 
 app.config['UPLOAD_FOLDER'] = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'processed')
+app.config['ARCHIVE_FOLDER'] = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'archives')
 os.makedirs(app.config['UPLOAD_FOLDER'], exist_ok=True)
+os.makedirs(app.config['ARCHIVE_FOLDER'], exist_ok=True)
 app.config['MAX_CONTENT_LENGTH'] = 500 * 1024 * 1024  # 500MB limit
 app.secret_key = os.getenv('SECRET_KEY', 'dev-secret-key-change-in-production')
 
+# Secure session cookie settings
+app.config['SESSION_COOKIE_SECURE'] = True  # Only send cookie over HTTPS
+app.config['SESSION_COOKIE_HTTPONLY'] = True  # Prevent JavaScript access
+app.config['SESSION_COOKIE_SAMESITE'] = 'Lax'  # CSRF protection
+app.config['PERMANENT_SESSION_LIFETIME'] = 3600  # 1 hour session timeout
+
 # Entra ID Configuration
 TENANT_ID = os.getenv('TENANT_ID', 'your-tenant-id')
 CLIENT_ID = os.getenv('CLIENT_ID', 'your-client-id')
@@ -95,6 +103,11 @@ def load_jobs():
 # Load jobs on startup
 load_jobs()
 
+# Job cleanup settings
+JOB_RETENTION_DAYS = 7
+SAVE_BATCH_SIZE = 5  # Only save after this many updates
+save_counter = 0
+
 def update_job_status(job_id, status, **kwargs):
     """Update job status and persist changes"""
     with job_lock:
@@ -102,8 +115,59 @@ def update_job_status(job_id, status, **kwargs):
             job_queue[job_id]['status'] = status
             job_queue[job_id].update(kwargs)
             job_queue[job_id]['updated_at'] = datetime.now().strftime('%Y-%m-%d %H:%M:%S')
+            
+            # Only save for important status changes
+            if status in ['completed', 'error', 'processing']:
+                save_jobs()
+
+def cleanup_old_jobs():
+    """Remove jobs older than JOB_RETENTION_DAYS"""
+    from datetime import timedelta
+    cutoff = datetime.now() - timedelta(days=JOB_RETENTION_DAYS)
+    
+    with job_lock:
+        jobs_to_remove = []
+        for job_id, job in list(job_queue.items()):
+            if job.get('status') == 'completed':
+                try:
+                    created_at = datetime.strptime(job.get('created_at', ''), '%Y-%m-%d %H:%M:%S')
+                    if created_at < cutoff:
+                        # Clean up associated files
+                        if job.get('zip_path') and os.path.exists(job['zip_path']):
+                            try:
+                                os.remove(job['zip_path'])
+                            except Exception as e:
+                                print(f"Error removing ZIP file: {e}")
+                        
+                        # Clean up uploaded files
+                        for file_info in job.get('files', []):
+                            if 'path' in file_info and os.path.exists(file_info['path']):
+                                try:
+                                    os.remove(file_info['path'])
+                                except Exception as e:
+                                    print(f"Error removing file: {e}")
+                        
+                        jobs_to_remove.append(job_id)
+                except (ValueError, TypeError) as e:
+                    print(f"Error parsing date for job {job_id}: {e}")
+        
+        for job_id in jobs_to_remove:
+            del job_queue[job_id]
+            print(f"Cleaned up old job: {job_id}")
+        
+        if jobs_to_remove:
             save_jobs()
-            print(f"DEBUG: Job {job_id} status updated to {status}")
+            print(f"Removed {len(jobs_to_remove)} old jobs")
+
+def save_jobs_batched():
+    """Save jobs to disk with batching to reduce I/O"""
+    global save_counter
+    save_counter += 1
+    
+    # Save immediately for critical operations, otherwise batch
+    if save_counter >= SAVE_BATCH_SIZE:
+        save_jobs()
+        save_counter = 0
 
 def analyze_failed_extractions(log_path):
     """
@@ -268,6 +332,10 @@ def map_pattern_10(match1, match2):
     }
 
 ALLOWED_EXTENSIONS = {'pdf', 'zip'}
+ALLOWED_MIME_TYPES = {
+    'pdf': 'application/pdf',
+    'zip': 'application/zip'
+}
 
 # Global jobs storage (in production, use database)
 job_queue = {}
@@ -283,10 +351,48 @@ def login_required(f):
         return f(*args, **kwargs)
     return decorated_function
 
-def allowed_file(filename):
-    return '.' in filename and filename.rsplit('.', 1)[1].lower() in ALLOWED_EXTENSIONS
+def allowed_file(filename, file_stream=None):
+    """
+    Validate file based on extension and optionally MIME type.
+    
+    Args:
+        filename: The name of the file
+        file_stream: Optional file stream to check MIME type
+    
+    Returns:
+        bool: True if file is allowed, False otherwise
+    """
+    # Check extension
+    if '.' not in filename:
+        return False
+    
+    ext = filename.rsplit('.', 1)[1].lower()
+    if ext not in ALLOWED_EXTENSIONS:
+        return False
+    
+    # Additional MIME type validation if file stream provided
+    if file_stream:
+        try:
+            import magic
+            file_stream.seek(0)
+            mime_type = magic.from_buffer(file_stream.read(2048), mime=True)
+            file_stream.seek(0)
+            
+            expected_mime = ALLOWED_MIME_TYPES.get(ext)
+            if expected_mime and mime_type != expected_mime:
+                print(f"MIME type mismatch: expected {expected_mime}, got {mime_type}")
+                return False
+        except ImportError:
+            # python-magic not installed, skip MIME check
+            pass
+        except Exception as e:
+            print(f"Error checking MIME type: {e}")
+            return False
+    
+    return True
 
 def extract_names_from_pdf(pdf_path):
+    reader = None
     try:
         reader = PdfReader(pdf_path)
 
@@ -323,6 +429,13 @@ def extract_names_from_pdf(pdf_path):
 
     except Exception as e:
         return {'error': f'PDF read error: {str(e)}'}
+    finally:
+        # Properly close PDF to prevent resource leaks
+        if reader is not None:
+            try:
+                reader.stream.close()
+            except:
+                pass
 
 def extract_student_info_from_text(text):
     """
@@ -506,13 +619,14 @@ def extract_student_info_from_text(text):
     return None
 
 def process_job(job_id, files):
+    temp_extract_dirs = []
     try:
         print("=== DEBUG: process_job STARTED ===")
-        job_queue[job_id]['status'] = 'processing'
-        job_queue[job_id]['progress'] = 'Starting job processing...'
+        with job_lock:
+            job_queue[job_id]['status'] = 'processing'
+            job_queue[job_id]['progress'] = 'Starting job processing...'
         save_jobs()  # Persist initial progress
         processed_files = []
-        temp_extract_dirs = []
         
         # Create base Episode directory for this job
         episode_dir = os.path.join(app.config['UPLOAD_FOLDER'], 'Episode')
@@ -605,7 +719,9 @@ def process_job(job_id, files):
                                     processed_files.append(('success', os.path.relpath(new_path, episode_dir), new_path))
                                     processed_count += 1
                                     job_queue[job_id]['progress'] = f'Organized {processed_count}/{total_files} files'
-                                    save_jobs()  # Persist progress update
+                                    # Batch saves - only every 5 files
+                                    if processed_count % 5 == 0:
+                                        save_jobs()
                                     print(f"DEBUG: Organized file {processed_count}/{total_files}")
                     
                     print(f"DEBUG: Found {len(pdf_files_found)} PDF files in ZIP: {pdf_files_found}")
@@ -643,7 +759,9 @@ def process_job(job_id, files):
                         processed_files.append(('success', os.path.relpath(new_filepath, episode_dir), new_filepath))
                         processed_count += 1
                         job_queue[job_id]['progress'] = f'Organized {processed_count}/{total_files} files'
-                        save_jobs()  # Persist progress update
+                        # Batch saves - only every 5 files
+                        if processed_count % 5 == 0:
+                            save_jobs()
                 except Exception as e:
                     processed_files.append(('skipped', filename, f'PDF processing error: {str(e)}'))
                     job_queue[job_id]['progress'] = f'Failed to process PDF {filename}: {str(e)}'
@@ -656,7 +774,8 @@ def process_job(job_id, files):
         print(f"DEBUG: Starting ZIP creation, successful_files: {len(successful_files)}")
         if successful_files:
             try:
-                zip_output = os.path.join(app.config['UPLOAD_FOLDER'], f'renamed_{job_id}.zip')
+                # Create ZIP in archives folder
+                zip_output = os.path.join(app.config['ARCHIVE_FOLDER'], f'renamed_{job_id}.zip')
                 print(f"DEBUG: Creating ZIP at {zip_output}")
                 with zipfile.ZipFile(zip_output, 'w', zipfile.ZIP_DEFLATED) as zip_out:
                     for root, dirs, files_in_dir in os.walk(episode_dir):
@@ -670,6 +789,24 @@ def process_job(job_id, files):
                 job_queue[job_id]['progress'] = f'Created ZIP archive with {len(successful_files)} organized files'
                 save_jobs()  # Persist completion progress
                 print(f"DEBUG: ZIP creation completed successfully")
+                
+                # Clean up Episode directory after successful ZIP creation
+                try:
+                    import shutil
+                    shutil.rmtree(episode_dir)
+                    print(f"DEBUG: Cleaned up Episode directory: {episode_dir}")
+                except Exception as cleanup_error:
+                    print(f"WARNING: Failed to clean up Episode directory: {cleanup_error}")
+                
+                # Clean up original uploaded files
+                for file_info in files:
+                    try:
+                        if os.path.exists(file_info['path']):
+                            os.remove(file_info['path'])
+                            print(f"DEBUG: Removed uploaded file: {file_info['path']}")
+                    except Exception as file_cleanup_error:
+                        print(f"WARNING: Failed to remove uploaded file: {file_cleanup_error}")
+                
             except Exception as e:
                 job_queue[job_id]['progress'] = f'Error creating ZIP: {str(e)}'
                 job_queue[job_id]['status'] = 'error'
@@ -679,17 +816,29 @@ def process_job(job_id, files):
             job_queue[job_id]['progress'] = 'No files to archive - all files were skipped'
             print(f"DEBUG: No successful files to archive")
         
-        job_queue[job_id]['status'] = 'completed'
-        job_queue[job_id]['processed_files'] = processed_files
-        job_queue[job_id]['skipped'] = [(p[1], p[2]) for p in processed_files if p[0] == 'skipped']
+        with job_lock:
+            job_queue[job_id]['status'] = 'completed'
+            job_queue[job_id]['processed_files'] = processed_files
+            job_queue[job_id]['skipped'] = [(p[1], p[2]) for p in processed_files if p[0] == 'skipped']
         save_jobs()  # Persist completed job
         print(f"DEBUG: Job {job_id} completed successfully. Processed: {len(processed_files)}, Skipped: {len(job_queue[job_id]['skipped'])}")
 
     except Exception as e:
-        job_queue[job_id]['status'] = 'error'
-        job_queue[job_id]['progress'] = f'Job failed with error: {str(e)}'
+        with job_lock:
+            job_queue[job_id]['status'] = 'error'
+            job_queue[job_id]['progress'] = f'Job failed with error: {str(e)}'
         save_jobs()  # Persist error status
         print(f"Job {job_id} failed: {str(e)}")  # Debug logging
+    finally:
+        # Clean up temporary directories
+        for temp_dir in temp_extract_dirs:
+            if os.path.exists(temp_dir):
+                try:
+                    import shutil
+                    shutil.rmtree(temp_dir)
+                    print(f"DEBUG: Cleaned up temporary directory: {temp_dir}")
+                except Exception as cleanup_error:
+                    print(f"ERROR: Failed to clean up {temp_dir}: {cleanup_error}")
 
 @app.route('/login')
 def login():
@@ -1181,34 +1330,65 @@ def api_jobs():
 @app.route('/download/<filename>')
 @login_required
 def download(filename):
+    """Download a completed job's ZIP file with path traversal protection"""
+    # Prevent path traversal attacks
+    if '..' in filename or '/' in filename or '\\' in filename:
+        return "Invalid filename", 400
+    
     # Security check: only allow downloading files that belong to completed jobs
-    for job_id, job in job_queue.items():
-        if job.get('status') == 'completed' and job.get('zip_name') == filename:
-            file_path = os.path.join(app.config['UPLOAD_FOLDER'], filename)
-            if os.path.exists(file_path):
-                return send_from_directory(app.config['UPLOAD_FOLDER'], filename, as_attachment=True)
+    with job_lock:
+        for job_id, job in job_queue.items():
+            if job.get('status') == 'completed' and job.get('zip_name') == filename:
+                # Use ARCHIVE_FOLDER instead of UPLOAD_FOLDER
+                file_path = os.path.join(app.config['ARCHIVE_FOLDER'], filename)
+                # Verify the resolved path is within ARCHIVE_FOLDER
+                resolved_path = os.path.abspath(file_path)
+                archive_folder = os.path.abspath(app.config['ARCHIVE_FOLDER'])
+                
+                if not resolved_path.startswith(archive_folder):
+                    return "Access denied", 403
+                
+                if os.path.exists(resolved_path):
+                    return send_from_directory(app.config['ARCHIVE_FOLDER'], filename, as_attachment=True)
+    
     return "File not found or access denied", 404
 
 @app.route('/delete/<job_id>', methods=['POST'])
 @login_required
 def delete_job(job_id):
-    if job_id in job_queue:
-        job = job_queue[job_id]
-        # Only allow deletion of completed jobs
-        if job.get('status') == 'completed':
-            # Clean up the ZIP file if it exists
-            if job.get('zip_name'):
-                zip_path = os.path.join(app.config['UPLOAD_FOLDER'], job['zip_name'])
-                if os.path.exists(zip_path):
-                    os.remove(zip_path)
-            # Remove the job from the queue
-            del job_queue[job_id]
-            save_jobs()  # Persist job deletion
-            flash('Job deleted successfully.', 'success')
+    with job_lock:
+        if job_id in job_queue:
+            job = job_queue[job_id]
+            # Only allow deletion of completed jobs
+            if job.get('status') == 'completed':
+                # Clean up the ZIP file from archives folder
+                if job.get('zip_name'):
+                    zip_path = os.path.join(app.config['ARCHIVE_FOLDER'], job['zip_name'])
+                    if os.path.exists(zip_path):
+                        try:
+                            os.remove(zip_path)
+                            print(f"DEBUG: Deleted archive file: {zip_path}")
+                        except Exception as e:
+                            print(f"ERROR: Failed to delete archive: {e}")
+                            flash('Job deleted but archive file could not be removed.', 'warning')
+                
+                # Clean up any remaining uploaded files
+                for file_info in job.get('files', []):
+                    if 'path' in file_info and os.path.exists(file_info['path']):
+                        try:
+                            os.remove(file_info['path'])
+                            print(f"DEBUG: Deleted uploaded file: {file_info['path']}")
+                        except Exception as e:
+                            print(f"ERROR: Failed to delete uploaded file: {e}")
+                
+                # Remove the job from the queue
+                del job_queue[job_id]
+                save_jobs()  # Persist job deletion
+                flash('Job deleted successfully.', 'success')
+            else:
+                flash('Only completed jobs can be deleted.', 'error')
         else:
-            flash('Only completed jobs can be deleted.', 'error')
-    else:
-        flash('Job not found.', 'error')
+            flash('Job not found.', 'error')
     return redirect('/jobs')
 
 @app.route('/completed-jobs')