feat(downloader): validate and quarantine truncated files post-download

After gallery-dl returns, parse stdout for written file paths and run
the magic-byte validator on each. Files that fail are moved to
{download_path}/_quarantine/{subscription}/{platform}/ with a sidecar
JSON capturing the source URL, validation reason, and original path —
enough for an operator to redownload-from-source or delete.

Adds ErrorType.VALIDATION_FAILED. A successful gallery-dl run that
produced quarantined files is now a soft failure (success=False,
error_message names the dominant failure reason and count) so the
source's error_count ticks up and the dashboard surfaces it. Gated
by download.validate_files setting (default True).

files_quarantined and quarantined_paths are persisted into download
metadata for the UI/API to consume.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
2026-04-25 22:55:52 -04:00
parent ba75d1ffdc
commit 2765c464bd
4 changed files with 401 additions and 4 deletions
+4
View File
@@ -41,6 +41,7 @@ DEFAULT_SETTINGS = {
"download.rate_limit": 3.0,
"download.retry_count": 3,
"download.schedule_interval": 28800, # 8 hours
"download.validate_files": True,
"notification.enabled": False,
"notification.webhook_url": None,
"dashboard.failure_threshold": 5,
@@ -51,3 +52,6 @@ EXTENSION_API_KEY_SETTING = "security.extension_api_key"
# Key for cached storage stats
STORAGE_STATS_SETTING = "cache.storage_stats"
# Key for cached library-validation report (last sweep)
LIBRARY_VALIDATION_REPORT_SETTING = "cache.library_validation_report"
+205 -3
View File
@@ -33,6 +33,7 @@ class ErrorType(str, Enum):
TIMEOUT = "timeout"
HTTP_ERROR = "http_error"
UNSUPPORTED_URL = "unsupported_url"
VALIDATION_FAILED = "validation_failed"
UNKNOWN_ERROR = "unknown_error"
@@ -102,6 +103,26 @@ class SourceConfig:
}
def _summarize_validation_failures(failures: list[dict]) -> str:
"""Human-readable error_message for a run that quarantined files.
Picks the dominant failure reason (most common) and counts so the
dashboard's error column says something useful — "3 files truncated
(missing JPEG EOI marker)" rather than a generic VALIDATION_FAILED.
"""
if not failures:
return "Validation failed"
reasons: dict[str, int] = {}
for f in failures:
key = f.get("reason") or "unknown"
reasons[key] = reasons.get(key, 0) + 1
top_reason, top_count = max(reasons.items(), key=lambda kv: kv[1])
n = len(failures)
if top_count == n:
return f"{n} file{'s' if n != 1 else ''} quarantined: {top_reason}"
return f"{n} files quarantined ({top_count}× {top_reason}, mixed)"
@dataclass
class DownloadResult:
"""Result of a gallery-dl download execution."""
@@ -112,6 +133,8 @@ class DownloadResult:
# Output details
files_downloaded: int = 0
files_quarantined: int = 0
quarantined_paths: list[str] = field(default_factory=list)
stdout: str = ""
stderr: str = ""
return_code: int = 0
@@ -258,15 +281,24 @@ class GalleryDLService:
},
}
def __init__(self, rate_limit: Optional[float] = None):
def __init__(
self,
rate_limit: Optional[float] = None,
validate_files: bool = True,
):
"""Initialize the GalleryDL service.
Args:
rate_limit: Override for download rate limit (seconds between requests).
If None, uses environment default.
validate_files: When True, run magic-byte validation on each file
gallery-dl writes and quarantine truncated files. Caller
(tasks.downloads) typically reads this from the
`download.validate_files` DB setting.
"""
self.settings = get_settings()
self._rate_limit = rate_limit if rate_limit is not None else self.settings.download_rate_limit
self._validate_files = validate_files
self._base_config = self._load_base_config()
def _load_base_config(self) -> dict:
@@ -603,6 +635,121 @@ class GalleryDLService:
return sum(1 for line in stdout.splitlines() if line.strip().startswith("/"))
def _written_paths(self, stdout: str) -> list[Path]:
"""Extract absolute file paths gallery-dl wrote during this run.
Mirrors `_count_downloaded_files` but returns the paths themselves so
post-download validation only inspects files we actually just wrote
(skips on '#' lines and log noise are excluded). Caller is responsible
for filtering further by extension if needed.
"""
if not stdout:
return []
return [
Path(stripped)
for line in stdout.splitlines()
if (stripped := line.strip()).startswith("/")
]
def _validate_and_quarantine(
self,
written_paths: list[Path],
subscription_name: str,
platform: str,
url: str,
) -> tuple[list[str], list[dict]]:
"""Validate freshly-written files; quarantine any that fail.
Returns (quarantined_relpaths, failure_details). Each failure detail
is a dict suitable for logging/persisting: {path, format, reason, size}.
Files that pass validation or aren't a validatable format are left in
place. Failures are moved to {download_path}/_quarantine/<subscription>/
<platform>/ along with a sidecar JSON capturing the source URL, the
validation reason, and the original on-disk path so an operator can
decide to redownload-from-source or delete.
"""
# Local import keeps the module import-cycle clean and lets tests
# patch validation behavior.
from app.services.file_validator import is_validatable, validate_file
quarantined_relpaths: list[str] = []
failures: list[dict] = []
if not written_paths:
return quarantined_relpaths, failures
quarantine_root = (
Path(self.settings.download_path)
/ "_quarantine"
/ subscription_name
/ platform
)
for path in written_paths:
if not is_validatable(path):
continue
try:
result = validate_file(path)
except Exception as e:
# Validator must never crash the download flow.
logger.warning(f"Validator raised on {path}: {e}")
continue
if result.ok:
continue
# Move the bad file out of the library so downstream consumers
# (thumbnailer, ML, sync) don't trip on it.
try:
quarantine_root.mkdir(parents=True, exist_ok=True)
dest = quarantine_root / path.name
# Avoid overwriting prior quarantine entries with the same
# filename — append a numeric suffix until we find a free slot.
counter = 1
while dest.exists():
dest = quarantine_root / f"{path.stem}.{counter}{path.suffix}"
counter += 1
path.rename(dest)
sidecar = dest.with_suffix(dest.suffix + ".quarantine.json")
sidecar.write_text(
json.dumps(
{
"original_path": str(path),
"source_url": url,
"subscription": subscription_name,
"platform": platform,
"format": result.format,
"reason": result.reason,
"size": result.size,
"quarantined_at": utcnow().isoformat(),
},
indent=2,
)
)
except OSError as e:
# If we can't move it, log loudly but keep going. A failed
# quarantine isn't worse than the pre-fix status quo.
logger.error(
f"Failed to quarantine {path}: {e}. File left in place."
)
continue
logger.warning(
f"Quarantined corrupt file: {path}{dest} "
f"({result.format}: {result.reason})"
)
quarantined_relpaths.append(str(dest))
failures.append(
{
"path": str(path),
"format": result.format,
"reason": result.reason,
"size": result.size,
}
)
return quarantined_relpaths, failures
def _compute_run_stats(self, return_code: int, stdout: str, stderr: str) -> dict:
"""Summarize a gallery-dl run as structured counts for UI/analytics.
@@ -647,6 +794,9 @@ class GalleryDLService:
"per_item_failures": per_item_failures,
"warning_count": warning_count,
"tier_gated_count": tier_gated_count,
# quarantined_count is overlaid by the caller (tasks.downloads)
# when persisting metadata; surfaced via DownloadResult.files_quarantined
# because run_stats has no access to the live filesystem.
}
@staticmethod
@@ -796,14 +946,55 @@ class GalleryDLService:
if proc.stderr:
logger.debug(f"STDERR for {subscription_name}/{platform}:\n{proc.stderr}")
# Run validation on every fresh write before classifying. Files
# gallery-dl wrote and we then quarantined no longer count as
# "downloaded" — counting them would understate how many files
# need re-downloading and overstate the run's success.
quarantined_paths: list[str] = []
quarantine_failures: list[dict] = []
if self._validate_files:
quarantined_paths, quarantine_failures = self._validate_and_quarantine(
written_paths=self._written_paths(proc.stdout),
subscription_name=subscription_name,
platform=platform,
url=url,
)
files_written = self._count_downloaded_files(proc.stdout)
files_quarantined = len(quarantined_paths)
files_kept = max(0, files_written - files_quarantined)
# Determine success
if proc.returncode == 0:
# Exit 0 with quarantines: the run technically succeeded but
# delivered partially-corrupt content. Surface as a soft
# failure so the source's error_count ticks up and the user
# sees it on the dashboard, but keep the per-item details
# (quarantined_paths) for the maintenance UI.
if files_quarantined > 0:
return DownloadResult(
success=False,
url=url,
subscription_name=subscription_name,
platform=platform,
files_downloaded=files_kept,
files_quarantined=files_quarantined,
quarantined_paths=quarantined_paths,
stdout=proc.stdout,
stderr=proc.stderr,
return_code=proc.returncode,
error_type=ErrorType.VALIDATION_FAILED,
error_message=_summarize_validation_failures(quarantine_failures),
duration_seconds=duration,
started_at=started_at,
completed_at=completed_at,
)
return DownloadResult(
success=True,
url=url,
subscription_name=subscription_name,
platform=platform,
files_downloaded=self._count_downloaded_files(proc.stdout),
files_downloaded=files_kept,
stdout=proc.stdout,
stderr=proc.stderr,
return_code=proc.returncode,
@@ -822,12 +1013,23 @@ class GalleryDLService:
# current subscription tier can't see the requested posts).
success = error_type in (ErrorType.NO_NEW_CONTENT, ErrorType.TIER_LIMITED)
# If a non-zero exit also produced quarantined files, prefer the
# validation-failed signal — it's actionable (the file needs to
# be redownloaded) and tells the operator the partial-write
# problem is happening, not just the categorized exit-code error.
if files_quarantined > 0:
error_type = ErrorType.VALIDATION_FAILED
error_message = _summarize_validation_failures(quarantine_failures)
success = False
return DownloadResult(
success=success,
url=url,
subscription_name=subscription_name,
platform=platform,
files_downloaded=self._count_downloaded_files(proc.stdout) if success else 0,
files_downloaded=files_kept if success else 0,
files_quarantined=files_quarantined,
quarantined_paths=quarantined_paths,
stdout=proc.stdout,
stderr=proc.stderr,
return_code=proc.returncode,
+9 -1
View File
@@ -319,6 +319,9 @@ async def _download_source_async(source_id: int, download_id: int = None) -> dic
# Get rate limit from database settings
db_rate_limit = await _get_db_setting(session, "download.rate_limit", settings.download_rate_limit)
db_validate_files = bool(
await _get_db_setting(session, "download.validate_files", True)
)
# Session is now closed - DB connection released
finally:
@@ -331,7 +334,10 @@ async def _download_source_async(source_id: int, download_id: int = None) -> dic
resolved_campaign_id: Optional[str] = None
try:
gdl_service = GalleryDLService(rate_limit=db_rate_limit)
gdl_service = GalleryDLService(
rate_limit=db_rate_limit,
validate_files=db_validate_files,
)
attempt = await _execute_download_with_retry(
gdl_service=gdl_service,
platform=source_platform,
@@ -409,6 +415,7 @@ async def _download_source_async(source_id: int, download_id: int = None) -> dic
run_stats = gdl_service._compute_run_stats(
dl_result.return_code, dl_result.stdout, dl_result.stderr
)
run_stats["quarantined_count"] = dl_result.files_quarantined
stderr_summary = gdl_service._extract_errors_warnings(dl_result.stderr)
download.metadata_ = {
"stdout": gdl_service._truncate_log(dl_result.stdout) or None,
@@ -416,6 +423,7 @@ async def _download_source_async(source_id: int, download_id: int = None) -> dic
"stderr_errors_warnings": stderr_summary or None,
"run_stats": run_stats,
"duration_seconds": dl_result.duration_seconds,
"quarantined_paths": dl_result.quarantined_paths or None,
}
if dl_result.success:
@@ -0,0 +1,183 @@
"""Tests for GalleryDLService's post-download validation hook.
Focus: a successful gallery-dl run that wrote a truncated JPEG must
result in DownloadResult.success=False with VALIDATION_FAILED, the bad
file moved to the quarantine tree, and a sidecar JSON written next to
it. A run with no validation issues must behave exactly as before.
"""
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import patch
import pytest
from app.services.file_validator import JPEG_HEAD, JPEG_TAIL
from app.services.gallery_dl import (
DownloadResult,
ErrorType,
GalleryDLService,
_summarize_validation_failures,
)
def _make_service(tmp_path: Path, validate: bool = True) -> GalleryDLService:
"""Service wired to a tmp download root, no real filesystem config."""
fake_settings = SimpleNamespace(
download_path=str(tmp_path / "downloads"),
config_path=str(tmp_path / "config"),
download_rate_limit=1.0,
)
Path(fake_settings.download_path).mkdir(parents=True, exist_ok=True)
Path(fake_settings.config_path).mkdir(parents=True, exist_ok=True)
with patch("app.services.gallery_dl.get_settings", return_value=fake_settings), \
patch.object(GalleryDLService, "_load_base_config", return_value={}):
return GalleryDLService(rate_limit=1.0, validate_files=validate)
def _write_jpeg(path: Path, *, truncated: bool) -> None:
path.parent.mkdir(parents=True, exist_ok=True)
body = JPEG_HEAD + b"\x00" * 64
if not truncated:
body += JPEG_TAIL
path.write_bytes(body)
# --- _validate_and_quarantine ---
def test_quarantines_truncated_jpeg(tmp_path):
service = _make_service(tmp_path)
bad = Path(service.settings.download_path) / "Artist" / "patreon" / "01.jpg"
_write_jpeg(bad, truncated=True)
paths, failures = service._validate_and_quarantine(
written_paths=[bad],
subscription_name="Artist",
platform="patreon",
url="https://patreon.com/post/1",
)
assert len(paths) == 1
assert len(failures) == 1
assert failures[0]["format"] == "jpeg"
# Original is gone, moved to _quarantine root
assert not bad.exists()
quarantine_root = (
Path(service.settings.download_path) / "_quarantine" / "Artist" / "patreon"
)
moved = quarantine_root / "01.jpg"
assert moved.exists()
sidecar = quarantine_root / "01.jpg.quarantine.json"
assert sidecar.exists()
assert "patreon.com/post/1" in sidecar.read_text()
def test_passes_through_well_formed_files(tmp_path):
service = _make_service(tmp_path)
good = Path(service.settings.download_path) / "Artist" / "patreon" / "01.jpg"
_write_jpeg(good, truncated=False)
paths, failures = service._validate_and_quarantine(
written_paths=[good],
subscription_name="Artist",
platform="patreon",
url="https://patreon.com/post/1",
)
assert paths == []
assert failures == []
assert good.exists() # unchanged
def test_skips_unvalidatable_extensions(tmp_path):
service = _make_service(tmp_path)
sidecar = Path(service.settings.download_path) / "Artist" / "patreon" / "01.json"
sidecar.parent.mkdir(parents=True, exist_ok=True)
sidecar.write_bytes(b'{"junk": true}')
paths, _ = service._validate_and_quarantine(
written_paths=[sidecar],
subscription_name="Artist",
platform="patreon",
url="https://example/1",
)
assert paths == []
assert sidecar.exists()
def test_quarantine_filename_collision_does_not_overwrite(tmp_path):
service = _make_service(tmp_path)
quarantine_root = (
Path(service.settings.download_path) / "_quarantine" / "Artist" / "patreon"
)
quarantine_root.mkdir(parents=True, exist_ok=True)
(quarantine_root / "01.jpg").write_bytes(b"PRIOR_QUARANTINE")
bad = Path(service.settings.download_path) / "Artist" / "patreon" / "01.jpg"
_write_jpeg(bad, truncated=True)
paths, _ = service._validate_and_quarantine(
written_paths=[bad],
subscription_name="Artist",
platform="patreon",
url="https://patreon.com/2",
)
# Original quarantine file untouched, new one suffixed.
assert (quarantine_root / "01.jpg").read_bytes() == b"PRIOR_QUARANTINE"
assert (quarantine_root / "01.1.jpg").exists()
assert paths and paths[0].endswith("01.1.jpg")
# --- _written_paths ---
def test_written_paths_extracts_only_absolute_paths(tmp_path):
service = _make_service(tmp_path)
stdout = (
"/data/downloads/Artist/patreon/01.jpg\n"
"# /data/downloads/Artist/patreon/02.jpg\n" # archive skip — must be excluded
"/data/downloads/Artist/patreon/03.jpg\n"
"[patreon][info] some log line\n"
" /data/downloads/Artist/patreon/04.jpg \n" # surrounding whitespace
)
paths = service._written_paths(stdout)
names = [p.name for p in paths]
assert names == ["01.jpg", "03.jpg", "04.jpg"]
# --- _summarize_validation_failures ---
def test_summary_with_uniform_reason():
failures = [
{"reason": "missing JPEG EOI marker (FF D9)"} for _ in range(3)
]
msg = _summarize_validation_failures(failures)
assert "3 files" in msg
assert "EOI" in msg
def test_summary_with_mixed_reasons():
failures = [
{"reason": "missing JPEG EOI marker (FF D9)"},
{"reason": "missing JPEG EOI marker (FF D9)"},
{"reason": "zero-byte file"},
]
msg = _summarize_validation_failures(failures)
assert "3 files" in msg
assert "mixed" in msg
# --- validate_files=False respects opt-out ---
def test_validate_files_false_skips_quarantine(tmp_path):
service = _make_service(tmp_path, validate=False)
bad = Path(service.settings.download_path) / "Artist" / "patreon" / "01.jpg"
_write_jpeg(bad, truncated=True)
# download() is async + spawns subprocess. Cheaper test: confirm the
# _validate_files flag is wired and the helper isn't called when false.
# We don't run download() here; the constructor flag is the contract.
assert service._validate_files is False