From 030e0dc5af51c70844210a665f61700a5f8c78a5 Mon Sep 17 00:00:00 2001 From: OpenClaw Sub-agent Date: Mon, 3 Aug 2026 16:22:06 +1000 Subject: [PATCH] fix: backfill via presigned URL instead of direct boto3 API Direct boto3 calls against the R2 S3 endpoint fail with SignatureDoesNotMatch for this account, while presigned URLs served through the R2 custom domain are the path the app already uses. Switch migrate_image_storage download step to urlopen on the presigned URL. --- .../commands/migrate_image_storage.py | 34 ++++++++++++++----- tests/test_collections.py | 18 ++++------ 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/links/management/commands/migrate_image_storage.py b/links/management/commands/migrate_image_storage.py index 7360612..64e81ba 100644 --- a/links/management/commands/migrate_image_storage.py +++ b/links/management/commands/migrate_image_storage.py @@ -3,11 +3,15 @@ One-time / idempotent migration helper. For every Image that still has a legacy ``file_key`` and no linked ``file`` (FileUpload), this command: - 1. Downloads the object from R2 (via the same R2Storage client). - 2. Writes it to FILE_UPLOADS_FOLDER using the exact same persistence logic + 1. Generates a short-lived R2 presigned URL (the same URL the app serves in + the UI — proven to work; direct boto3 API calls against the S3 endpoint + fail with SignatureDoesNotMatch for this account, so we go through the + custom domain like the browser does). + 2. Downloads the bytes. + 3. Writes them to FILE_UPLOADS_FOLDER using the exact same persistence logic as regular file uploads (temp file + fsync + byte-count verification). - 3. Creates a FileUpload record and links it to the Image (``image.file``). - 4. Clears ``image.file_key`` so the record is fully on the files backend. + 4. Creates a FileUpload record and links it to the Image (``image.file``). + 5. Clears ``image.file_key`` so the record is fully on the files backend. Nothing is deleted from R2 — the bucket keeps its objects until the operator decides to purge them (run with ``--delete-r2`` for that). @@ -18,6 +22,8 @@ Usage: python manage.py migrate_image_storage --commit --delete-r2 """ from io import BytesIO +import os +import urllib.request from django.core.files.uploadedfile import InMemoryUploadedFile from django.core.management.base import BaseCommand @@ -27,6 +33,20 @@ from links.models import Image from links.storage import R2Storage +def _download_via_presigned_url(storage, key): + """Download an R2 object using the presigned custom-domain URL. + + Direct boto3 ``get_object`` calls fail for this account + (SignatureDoesNotMatch on the S3 API endpoint), while the presigned URLs + the app serves via the R2 custom domain work fine — so reuse that path. + """ + url = storage.get_url(key, expires_in=3600) + if not url: + raise ValueError(f'No signed URL for {key}') + with urllib.request.urlopen(url, timeout=60) as resp: + return resp.read() + + class Command(BaseCommand): help = "Migrate legacy R2-backed images onto the unified files backend." @@ -60,10 +80,8 @@ class Command(BaseCommand): skipped += 1 continue try: - # 1. Download from R2 - obj = storage.client.get_object( - Bucket=storage.bucket, Key=image.file_key) - raw = obj['Body'].read() + # 1–2. Download from R2 via the presigned custom-domain URL + raw = _download_via_presigned_url(storage, image.file_key) if not raw: raise ValueError(f'Empty object for {image.file_key}') diff --git a/tests/test_collections.py b/tests/test_collections.py index f259e29..cd4d326 100644 --- a/tests/test_collections.py +++ b/tests/test_collections.py @@ -165,12 +165,9 @@ class TestMigrateImageStorageCommand: raw = raw.getvalue() self._legacy_image(coll, raw=raw) - fake_storage = mock.Mock() - fake_storage.bucket = "bucket" - fake_storage.client.get_object.return_value = {"Body": io.BytesIO(raw)} with mock.patch( - "links.management.commands.migrate_image_storage.R2Storage", - return_value=fake_storage, + "links.management.commands.migrate_image_storage._download_via_presigned_url", + return_value=raw, ): call_command("migrate_image_storage", "--commit") @@ -181,8 +178,8 @@ class TestMigrateImageStorageCommand: assert img.file.size == len(raw) # Idempotent: second run has nothing to do with mock.patch( - "links.management.commands.migrate_image_storage.R2Storage", - return_value=fake_storage, + "links.management.commands.migrate_image_storage._download_via_presigned_url", + return_value=raw, ): call_command("migrate_image_storage", "--commit") assert Image.objects.get().file is not None @@ -191,12 +188,9 @@ class TestMigrateImageStorageCommand: coll = ImageCollection.objects.create(name="C") self._legacy_image(coll, raw=b"\x89PNGdata") - fake_storage = mock.Mock() - fake_storage.bucket = "bucket" - fake_storage.client.get_object.side_effect = Exception("network down") with mock.patch( - "links.management.commands.migrate_image_storage.R2Storage", - return_value=fake_storage, + "links.management.commands.migrate_image_storage._download_via_presigned_url", + side_effect=Exception("network down"), ): call_command("migrate_image_storage", "--commit")