From 7e672a5e62c6d87fb65faeb0c5b7deb692c30e6a Mon Sep 17 00:00:00 2001
From: Josep Piguillem <jgpigui@gmail.com>
Date: Mon, 28 Sep 2026 20:43:59 +0200
Subject: [PATCH] Serve uploaded SVGs under their CSP, and identify the image
 fetcher

The Content-Security-Policy meant for stored SVGs never applied: in
Caddy a `*` in the middle of a path does not cross a `/`, so
`path /storage/*.svg` only matched files directly under /storage, and
every upload lives in a directory below it. Two matchers, which Caddy
ANDs, cover any depth.

RemoteImage fetched with Guzzle's anonymous default User-Agent, which
Wikimedia answers with 403. It now sends `<app>/1.0 (+<app url>; media
fetcher)`, the identification such hosts ask of automated clients.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 app/Services/Media/RemoteImage.php    | 19 +++++++++++++++++++
 docker/Caddyfile                      | 10 +++++++++-
 tests/Feature/Api/MediaUploadTest.php | 20 ++++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/app/Services/Media/RemoteImage.php b/app/Services/Media/RemoteImage.php
index 6353df5..5cdb999 100644
--- a/app/Services/Media/RemoteImage.php
+++ b/app/Services/Media/RemoteImage.php
@@ -6,6 +6,7 @@ namespace App\Services\Media;
 
 use Illuminate\Http\Client\ConnectionException;
 use Illuminate\Support\Facades\Http;
+use Illuminate\Support\Str;
 use Illuminate\Validation\ValidationException;
 
 /**
@@ -25,6 +26,10 @@ use Illuminate\Validation\ValidationException;
  * - Redirects are not followed: the target of one would need all of the above
  *   again, and "use the final address" is an easy thing to ask for.
  * - The body is read to a ceiling and no further, whatever the headers claim.
+ *
+ * It says who is asking. Hosts such as Wikimedia refuse Guzzle's anonymous
+ * default outright, and a User-Agent with a contact address is what their
+ * policies ask of anything that fetches automatically.
  */
 class RemoteImage
 {
@@ -63,6 +68,7 @@ class RemoteImage
                 )]],
             ])
                 ->withHeaders(['Accept' => 'image/*'])
+                ->withUserAgent($this->userAgent())
                 ->get($url);
         } catch (ConnectionException) {
             $this->fail('unreachable');
@@ -94,6 +100,19 @@ class RemoteImage
         return $contents;
     }
 
+    /**
+     * `pireon-admin/1.0 (+https://admin.example.org; media fetcher)`: a product
+     * token without spaces, and the installation's own address to reach it by.
+     */
+    protected function userAgent(): string
+    {
+        return sprintf(
+            '%s/1.0 (+%s; media fetcher)',
+            Str::slug((string) config('app.name')) ?: 'landing',
+            rtrim((string) config('app.url'), '/'),
+        );
+    }
+
     /**
      * The address to connect to, once every address the host has is known to
      * be public.
diff --git a/docker/Caddyfile b/docker/Caddyfile
index 829e657..30aeea1 100644
--- a/docker/Caddyfile
+++ b/docker/Caddyfile
@@ -25,7 +25,15 @@
 	# second line: opened directly rather than through an <img>, an SVG is a
 	# document, and nothing stored here has any business running script or
 	# loading anything from elsewhere.
-	@svg path /storage/*.svg
+	#
+	# Two matchers, which Caddy ANDs, rather than `path /storage/*.svg`: a `*`
+	# in the middle of a path does not cross a `/`, so that pattern only ever
+	# matched files directly under /storage — and every upload is a directory
+	# below it.
+	@svg {
+		path /storage/*
+		path_regexp (?i)\.svg$
+	}
 	header @svg Content-Security-Policy "default-src 'none'; style-src 'unsafe-inline'; img-src data:"
 
 	# The frontend is a separate application on another origin.
diff --git a/tests/Feature/Api/MediaUploadTest.php b/tests/Feature/Api/MediaUploadTest.php
index 0427d4e..48e0bf0 100644
--- a/tests/Feature/Api/MediaUploadTest.php
+++ b/tests/Feature/Api/MediaUploadTest.php
@@ -133,6 +133,26 @@ class MediaUploadTest extends TestCase
             ->assertJsonPath('data.width', 30);
     }
 
+    #[Test]
+    public function it_says_who_is_fetching(): void
+    {
+        config()->set('app.name', 'Pireon Admin');
+        config()->set('app.url', 'https://admin.example.org');
+
+        $this->actingWithRole('editor');
+        $this->resolving(['images.example.test' => ['93.184.216.34']]);
+
+        Http::fake([
+            'https://images.example.test/*' => Http::response($this->png(), 200),
+        ]);
+
+        $this->postJson('/api/v1/media', ['url' => 'https://images.example.test/a.png'])->assertCreated();
+
+        // Hosts such as Wikimedia answer Guzzle's anonymous default with 403.
+        Http::assertSent(fn ($request) => str_starts_with($request->url(), 'https://images.example.test/')
+            && $request->header('User-Agent') === ['pireon-admin/1.0 (+https://admin.example.org; media fetcher)']);
+    }
+
     /**
      * @return array<string, array{0: array<string, array<int, string>>, 1: string}>
      */