diff --git a/backend/app/routers/uploads.py b/backend/app/routers/uploads.py index 309637b..38f5919 100644 --- a/backend/app/routers/uploads.py +++ b/backend/app/routers/uploads.py @@ -1,9 +1,12 @@ """Native browser media authentication only; API authentication stays bearer-only.""" +import mimetypes + from fastapi import APIRouter, Depends, HTTPException, Request -from fastapi.responses import FileResponse +from fastapi.responses import FileResponse, Response from sqlalchemy.orm import Session from app.database import get_db +from app.services import storage_service from app.utils.auth import get_current_user from app.utils.upload_access import ( LEGACY_LMS_PREFIXES, local_upload_path, upload_file, references, @@ -41,8 +44,19 @@ def _read_upload(path, request, attempt_id, db, headers): if not can_read_upload(db, path, user, questions, cards, attempt_id): raise HTTPException(404, "File not found") # Known legacy LMS directories retain their policy; this is not a whole-LMS audit. - if not target.is_file(): - raise HTTPException(404, "File not found") if target.suffix.lower() == ".svg": headers["Content-Security-Policy"] = "sandbox; default-src 'none'; style-src 'unsafe-inline'" - return FileResponse(target, headers=headers) + + # Serving must go through the storage service, or object storage would be + # write-only: the bytes would be in the bucket and still read from disk. + # The path is already authorised and confined to the upload root above. + if target.is_file(): + return FileResponse(target, headers=headers) + data = storage_service.s3_object(path) + if data is None: + raise HTTPException(404, "File not found") + media_type = mimetypes.guess_type(path)[0] or "application/octet-stream" + if request.method == "HEAD": + return Response(status_code=200, headers={**headers, "Content-Length": str(len(data))}, + media_type=media_type) + return Response(content=data, media_type=media_type, headers=headers) diff --git a/backend/app/services/storage_service.py b/backend/app/services/storage_service.py index 81087a2..12ba313 100644 --- a/backend/app/services/storage_service.py +++ b/backend/app/services/storage_service.py @@ -138,6 +138,20 @@ def presigned_url(key: str, expires: int = 3600) -> str | None: return None +def s3_object(key: str) -> bytes | None: + """Read strictly from S3, with no fallback. + + `load` falls back to the volume, which is right for serving but useless for + verifying a migration: it would compare a local file against itself. + """ + if not using_s3(): + return None + try: + return _s3().get_object(Bucket=settings.S3_BUCKET, Key=key)["Body"].read() + except Exception: + return None + + def size_of(key: str) -> int | None: if using_s3(): try: diff --git a/backend/scripts/migrate_uploads_to_s3.py b/backend/scripts/migrate_uploads_to_s3.py new file mode 100644 index 0000000..5378727 --- /dev/null +++ b/backend/scripts/migrate_uploads_to_s3.py @@ -0,0 +1,120 @@ +"""Copy the existing uploads volume into object storage. + +Verification order matters: every file is copied and read back *before* anything +is deleted, and deletion only happens when explicitly asked for. A migration that +removes the source before proving the destination is readable has no way back. + + docker compose exec backend python -m scripts.migrate_uploads_to_s3 + docker compose exec backend python -m scripts.migrate_uploads_to_s3 --apply + docker compose exec backend python -m scripts.migrate_uploads_to_s3 --verify + docker compose exec backend python -m scripts.migrate_uploads_to_s3 --apply --delete-local +""" +import hashlib +import sys +from pathlib import Path + +from app.config import settings +from app.services import storage_service + + +def walk(root: Path): + """Every file under the uploads directory, keyed by its relative path.""" + for path in sorted(root.rglob("*")): + if path.is_file() and not path.name.startswith("."): + yield path.relative_to(root).as_posix(), path + + +def human(size: int) -> str: + for unit in ("B", "KB", "MB", "GB"): + if size < 1024 or unit == "GB": + return f"{size:.0f} {unit}" if unit == "B" else f"{size:.1f} {unit}" + size /= 1024 + return f"{size:.1f} GB" + + +def main(): + apply_changes = "--apply" in sys.argv + verify_only = "--verify" in sys.argv + delete_local = "--delete-local" in sys.argv + + if not storage_service.using_s3(): + print("STORAGE_BACKEND is not 's3'; nothing to migrate into.") + return 1 + + root = Path(settings.UPLOAD_DIR) + if not root.is_dir(): + print(f"No uploads directory at {root}") + return 1 + + storage_service.ensure_bucket() + files = list(walk(root)) + total_bytes = sum(path.stat().st_size for _key, path in files) + print(f" files on the volume : {len(files)} ({human(total_bytes)})") + + copied, already, failed, verified, mismatched = 0, 0, [], 0, [] + for key, path in files: + data = path.read_bytes() + digest = hashlib.sha256(data).hexdigest() + + # Strictly S3: the fallback in `load` would compare a file to itself. + if verify_only or storage_service.s3_object(key) is not None: + stored = storage_service.s3_object(key) + if stored is None: + failed.append(key) + elif hashlib.sha256(stored).hexdigest() != digest: + mismatched.append(key) + else: + verified += 1 + already += 1 + continue + + if not apply_changes: + copied += 1 + continue + + try: + storage_service.save(key, data) + # Read back before trusting it; a write that cannot be read is not a copy. + stored = storage_service.s3_object(key) + if stored is None or hashlib.sha256(stored).hexdigest() != digest: + mismatched.append(key) + else: + copied += 1 + verified += 1 + except Exception as error: + failed.append(f"{key}: {error}") + + print(f" copied : {copied}") + print(f" already in storage : {already}") + print(f" read back and match : {verified}") + if mismatched: + print(f" MISMATCHED : {len(mismatched)}") + for key in mismatched[:10]: + print(f" {key}") + if failed: + print(f" FAILED : {len(failed)}") + for key in failed[:10]: + print(f" {key}") + + if delete_local: + if mismatched or failed: + print("\n Refusing to delete: not every file verified.") + return 1 + if not apply_changes: + print("\n --delete-local needs --apply.") + return 1 + removed = 0 + for key, path in files: + # Belt and braces: prove the object reads before removing the source. + if storage_service.s3_object(key) is not None: + path.unlink() + removed += 1 + print(f" local files removed : {removed}") + + if not apply_changes and not verify_only: + print("\n Re-run with --apply to copy, then --verify to check.") + return 0 + + +if __name__ == "__main__": + sys.exit(main())