Phil Skentelbery
1b45a64920
Implement debug file management system with configuration controls, automatic cleanup, and security improvements per v1.5.0 Phase 2. ## Changes ### Configuration (config.py) - Add DEBUG_SAVE_FAILED_UPLOADS (default: false, production-safe) - Add DEBUG_FILE_MAX_AGE_DAYS (default: 7 days) - Add DEBUG_FILE_MAX_SIZE_MB (default: 100MB) ### Media Validation (media.py) - Check config before saving debug files - Sanitize filenames to prevent path traversal - Pattern: alphanumeric + "._-", truncated to 50 chars - Add cleanup_old_debug_files() function * Age-based cleanup (delete files older than MAX_AGE) * Size-based cleanup (delete oldest if total > MAX_SIZE) ### Application Startup (__init__.py) - Run cleanup_old_debug_files() on startup - Automatic maintenance of debug directory ### Tests (test_debug_file_management.py) - 15 comprehensive tests - Config defaults and overrides - Debug file saving behavior - Filename sanitization security - Cleanup age and size limits - Startup integration ## Security Improvements - Debug saving disabled by default (production-safe) - Filename sanitization prevents path traversal - Automatic cleanup prevents disk exhaustion ## Acceptance Criteria - [x] Configuration options added - [x] Debug saving disabled by default - [x] Filename sanitized before saving - [x] Cleanup runs on startup - [x] Old files deleted based on age - [x] Size limit enforced All tests pass. Ready for architect review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
350 lines
14 KiB
Python
350 lines
14 KiB
Python
"""
|
|
Tests for debug file management (v1.5.0 Phase 2)
|
|
|
|
Tests debug file saving, cleanup, and security features.
|
|
"""
|
|
|
|
import pytest
|
|
from pathlib import Path
|
|
from datetime import datetime, timedelta
|
|
import time
|
|
from unittest.mock import patch
|
|
|
|
from starpunk.media import cleanup_old_debug_files, validate_image
|
|
|
|
|
|
class TestDebugFileConfiguration:
|
|
"""Test debug file configuration options"""
|
|
|
|
def test_debug_save_disabled_by_default(self, app):
|
|
"""Test that debug file saving is disabled by default"""
|
|
assert app.config.get('DEBUG_SAVE_FAILED_UPLOADS') is False
|
|
|
|
def test_debug_max_age_default(self, app):
|
|
"""Test that debug file max age has correct default"""
|
|
assert app.config.get('DEBUG_FILE_MAX_AGE_DAYS') == 7
|
|
|
|
def test_debug_max_size_default(self, app):
|
|
"""Test that debug file max size has correct default"""
|
|
assert app.config.get('DEBUG_FILE_MAX_SIZE_MB') == 100
|
|
|
|
def test_debug_config_override(self, app):
|
|
"""Test that debug config can be overridden"""
|
|
# Override config
|
|
app.config['DEBUG_SAVE_FAILED_UPLOADS'] = True
|
|
app.config['DEBUG_FILE_MAX_AGE_DAYS'] = 3
|
|
app.config['DEBUG_FILE_MAX_SIZE_MB'] = 50
|
|
|
|
assert app.config.get('DEBUG_SAVE_FAILED_UPLOADS') is True
|
|
assert app.config.get('DEBUG_FILE_MAX_AGE_DAYS') == 3
|
|
assert app.config.get('DEBUG_FILE_MAX_SIZE_MB') == 50
|
|
|
|
|
|
class TestDebugFileSaving:
|
|
"""Test debug file saving behavior"""
|
|
|
|
def test_no_debug_files_when_disabled(self, app):
|
|
"""Test that debug files are not saved when feature is disabled"""
|
|
with app.app_context():
|
|
# Ensure disabled
|
|
app.config['DEBUG_SAVE_FAILED_UPLOADS'] = False
|
|
|
|
# Try to validate invalid file (should fail and NOT save debug file)
|
|
with pytest.raises(ValueError):
|
|
validate_image(b'not-an-image', 'test.jpg')
|
|
|
|
# Check that no debug files were created
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
if debug_dir.exists():
|
|
debug_files = list(debug_dir.glob('failed_*'))
|
|
assert len(debug_files) == 0
|
|
|
|
def test_debug_files_saved_when_enabled(self, app):
|
|
"""Test that debug files are saved when feature is enabled"""
|
|
with app.app_context():
|
|
# Enable debug file saving
|
|
app.config['DEBUG_SAVE_FAILED_UPLOADS'] = True
|
|
|
|
# Ensure debug directory exists
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Try to validate invalid file (should fail and save debug file)
|
|
with pytest.raises(ValueError):
|
|
validate_image(b'not-an-image', 'test.jpg')
|
|
|
|
# Check that debug file was created
|
|
debug_files = list(debug_dir.glob('failed_*'))
|
|
assert len(debug_files) == 1
|
|
|
|
# Verify file contains the corrupt data
|
|
saved_data = debug_files[0].read_bytes()
|
|
assert saved_data == b'not-an-image'
|
|
|
|
|
|
class TestDebugFilenameSanitization:
|
|
"""Test filename sanitization for security"""
|
|
|
|
def test_path_traversal_prevention(self, app):
|
|
"""Test that path traversal attempts are sanitized"""
|
|
with app.app_context():
|
|
# Enable debug file saving
|
|
app.config['DEBUG_SAVE_FAILED_UPLOADS'] = True
|
|
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Try to validate with malicious filename
|
|
with pytest.raises(ValueError):
|
|
validate_image(b'not-an-image', '../../../etc/passwd')
|
|
|
|
# Check that debug file was created with sanitized name
|
|
debug_files = list(debug_dir.glob('failed_*'))
|
|
assert len(debug_files) == 1
|
|
|
|
# Verify filename is sanitized (no path traversal)
|
|
filename = debug_files[0].name
|
|
# Dots are allowed (they're valid), but slashes should be removed
|
|
assert '/' not in filename
|
|
# The filename should contain sanitized version (dots and alphanumerics kept)
|
|
assert 'etcpasswd' in filename # Slashes removed
|
|
# File should be in debug_dir (not escaped via path traversal)
|
|
assert debug_files[0].parent == debug_dir
|
|
|
|
def test_special_characters_removed(self, app):
|
|
"""Test that special characters are removed from filenames"""
|
|
with app.app_context():
|
|
# Enable debug file saving
|
|
app.config['DEBUG_SAVE_FAILED_UPLOADS'] = True
|
|
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Try to validate with special characters in filename
|
|
with pytest.raises(ValueError):
|
|
validate_image(b'not-an-image', 'test<>:"|?*.jpg')
|
|
|
|
# Check that debug file was created with sanitized name
|
|
debug_files = list(debug_dir.glob('failed_*'))
|
|
assert len(debug_files) == 1
|
|
|
|
# Verify special characters removed (only alphanumeric, dot, dash, underscore allowed)
|
|
filename = debug_files[0].name
|
|
# Should contain 'test.jpg' but no special chars
|
|
assert 'test' in filename
|
|
assert '.jpg' in filename
|
|
for char in '<>:"|?*':
|
|
assert char not in filename
|
|
|
|
def test_long_filename_truncated(self, app):
|
|
"""Test that long filenames are truncated to 50 chars"""
|
|
with app.app_context():
|
|
# Enable debug file saving
|
|
app.config['DEBUG_SAVE_FAILED_UPLOADS'] = True
|
|
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Try to validate with very long filename
|
|
long_name = 'a' * 100 + '.jpg'
|
|
with pytest.raises(ValueError):
|
|
validate_image(b'not-an-image', long_name)
|
|
|
|
# Check that debug file was created
|
|
debug_files = list(debug_dir.glob('failed_*'))
|
|
assert len(debug_files) == 1
|
|
|
|
# Verify filename is truncated (the sanitized part should be <=50 chars)
|
|
filename = debug_files[0].name
|
|
# Format is: failed_YYYYMMDD_HHMMSS_sanitized.ext
|
|
# Extract just the sanitized original filename part (after date and time)
|
|
parts = filename.split('_', 3) # Split on first three underscores
|
|
if len(parts) >= 4:
|
|
sanitized_part = parts[3]
|
|
# Sanitized part should be <= 50 chars (truncation happens before adding to filename)
|
|
assert len(sanitized_part) <= 50
|
|
|
|
|
|
class TestDebugFileCleanup:
|
|
"""Test debug file cleanup functionality"""
|
|
|
|
def test_cleanup_old_files(self, app):
|
|
"""Test that files older than max age are deleted"""
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Create old file (8 days old)
|
|
old_file = debug_dir / 'failed_20250101_120000_old.jpg'
|
|
old_file.write_bytes(b'old data')
|
|
old_time = datetime.now() - timedelta(days=8)
|
|
old_timestamp = old_time.timestamp()
|
|
old_file.touch()
|
|
# Set modification time to 8 days ago
|
|
import os
|
|
os.utime(old_file, (old_timestamp, old_timestamp))
|
|
|
|
# Create recent file (3 days old)
|
|
recent_file = debug_dir / 'failed_20250110_120000_recent.jpg'
|
|
recent_file.write_bytes(b'recent data')
|
|
recent_time = datetime.now() - timedelta(days=3)
|
|
recent_timestamp = recent_time.timestamp()
|
|
recent_file.touch()
|
|
os.utime(recent_file, (recent_timestamp, recent_timestamp))
|
|
|
|
# Set max age to 7 days
|
|
app.config['DEBUG_FILE_MAX_AGE_DAYS'] = 7
|
|
|
|
# Run cleanup
|
|
cleanup_old_debug_files(app)
|
|
|
|
# Old file should be deleted
|
|
assert not old_file.exists()
|
|
|
|
# Recent file should still exist
|
|
assert recent_file.exists()
|
|
|
|
def test_cleanup_size_limit(self, app):
|
|
"""Test that oldest files are deleted when size limit exceeded"""
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Create multiple files with known sizes
|
|
# Each file is 30MB (total 90MB)
|
|
file_size = 30 * 1024 * 1024
|
|
data = b'x' * file_size
|
|
|
|
import os
|
|
files = []
|
|
for i in range(3):
|
|
file_path = debug_dir / f'failed_2025010{i}_120000_file{i}.jpg'
|
|
file_path.write_bytes(data)
|
|
# Set different modification times with clear spacing
|
|
# Oldest file: 3 days ago, newest file: today
|
|
mtime = datetime.now() - timedelta(days=3-i)
|
|
timestamp = mtime.timestamp()
|
|
os.utime(file_path, (timestamp, timestamp))
|
|
files.append(file_path)
|
|
|
|
# Verify files are ordered correctly by mtime
|
|
mtimes = [f.stat().st_mtime for f in files]
|
|
assert mtimes[0] < mtimes[1] < mtimes[2], "Files not properly time-ordered"
|
|
|
|
# Set size limit to 70MB (total is 90MB, so should delete oldest to get under limit)
|
|
app.config['DEBUG_FILE_MAX_SIZE_MB'] = 70
|
|
app.config['DEBUG_FILE_MAX_AGE_DAYS'] = 365 # Don't delete by age
|
|
|
|
# Run cleanup
|
|
cleanup_old_debug_files(app)
|
|
|
|
# Oldest file should be deleted (90MB - 30MB = 60MB, under 70MB limit)
|
|
assert not files[0].exists(), "Oldest file should be deleted for size limit"
|
|
|
|
# Two newest files should remain (total 60MB, under 70MB limit)
|
|
assert files[1].exists(), "Second file should remain"
|
|
assert files[2].exists(), "Newest file should remain"
|
|
|
|
def test_cleanup_no_files(self, app):
|
|
"""Test that cleanup handles empty directory gracefully"""
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Run cleanup on empty directory (should not raise error)
|
|
cleanup_old_debug_files(app)
|
|
|
|
def test_cleanup_nonexistent_directory(self, app):
|
|
"""Test that cleanup handles nonexistent directory gracefully"""
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
|
|
# Ensure directory doesn't exist
|
|
if debug_dir.exists():
|
|
import shutil
|
|
shutil.rmtree(debug_dir)
|
|
|
|
# Run cleanup (should not raise error)
|
|
cleanup_old_debug_files(app)
|
|
|
|
def test_cleanup_combined_age_and_size(self, app):
|
|
"""Test cleanup with both age and size limits"""
|
|
debug_dir = Path(app.config['DATA_PATH']) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
# Create old large file (should be deleted by age)
|
|
old_file = debug_dir / 'failed_20250101_120000_old.jpg'
|
|
old_file.write_bytes(b'x' * (40 * 1024 * 1024)) # 40MB
|
|
old_time = datetime.now() - timedelta(days=10)
|
|
import os
|
|
os.utime(old_file, (old_time.timestamp(), old_time.timestamp()))
|
|
|
|
# Create recent files (total 70MB)
|
|
recent_files = []
|
|
for i in range(2):
|
|
recent_file = debug_dir / f'failed_2025011{i}_120000_recent{i}.jpg'
|
|
recent_file.write_bytes(b'x' * (35 * 1024 * 1024)) # 35MB each
|
|
# i=0: today (newest), i=1: 1 day ago (older)
|
|
# But we want i=0 to be older so it gets deleted first for size limit
|
|
recent_time = datetime.now() - timedelta(days=1+i) # 1-2 days ago
|
|
os.utime(recent_file, (recent_time.timestamp(), recent_time.timestamp()))
|
|
recent_files.append(recent_file)
|
|
time.sleep(0.01)
|
|
|
|
# Set limits
|
|
app.config['DEBUG_FILE_MAX_AGE_DAYS'] = 7
|
|
app.config['DEBUG_FILE_MAX_SIZE_MB'] = 40 # 70MB total, need to delete 30MB
|
|
|
|
# Run cleanup
|
|
cleanup_old_debug_files(app)
|
|
|
|
# Old file should be deleted by age
|
|
assert not old_file.exists()
|
|
|
|
# Of recent files (70MB total), one should be deleted to meet 40MB size limit
|
|
# Exactly one recent file should remain (35MB, under 40MB limit)
|
|
remaining_files = [f for f in recent_files if f.exists()]
|
|
assert len(remaining_files) == 1, "Exactly one recent file should remain after size cleanup"
|
|
|
|
# The remaining file should be 35MB
|
|
assert remaining_files[0].stat().st_size == 35 * 1024 * 1024
|
|
|
|
|
|
class TestDebugFileStartupCleanup:
|
|
"""Test that cleanup runs on application startup"""
|
|
|
|
def test_cleanup_runs_on_startup(self):
|
|
"""Test that cleanup runs during app initialization"""
|
|
# Create app with debug files
|
|
from starpunk import create_app
|
|
import tempfile
|
|
import shutil
|
|
|
|
# Create temporary data directory
|
|
tmpdir = tempfile.mkdtemp()
|
|
|
|
try:
|
|
# Create debug directory with old file
|
|
debug_dir = Path(tmpdir) / 'debug'
|
|
debug_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
old_file = debug_dir / 'failed_20250101_120000_old.jpg'
|
|
old_file.write_bytes(b'old data')
|
|
old_time = datetime.now() - timedelta(days=10)
|
|
import os
|
|
os.utime(old_file, (old_time.timestamp(), old_time.timestamp()))
|
|
|
|
# Create app (should run cleanup)
|
|
app = create_app({
|
|
'DATA_PATH': Path(tmpdir),
|
|
'NOTES_PATH': Path(tmpdir) / 'notes',
|
|
'DATABASE_PATH': Path(tmpdir) / 'test.db',
|
|
'DEBUG_FILE_MAX_AGE_DAYS': 7,
|
|
'TESTING': True,
|
|
'SESSION_SECRET': 'test-secret',
|
|
'ADMIN_ME': 'https://test.example.com'
|
|
})
|
|
|
|
# Old file should have been cleaned up
|
|
assert not old_file.exists()
|
|
|
|
finally:
|
|
# Cleanup temp directory
|
|
shutil.rmtree(tmpdir)
|