Upload images through the API, and from the MCP server

POST /api/v1/media stores an image where the panel keeps a page's and answers with the path to send as `featured_image`. Without it the MCP server could create a page but never give it a picture. The file goes through Media::for(), so it is processed, served and pruned like one uploaded in the panel. Two ways in, one per request: a multipart `file`, or JSON with a `url` the API downloads itself. JPEG, PNG, WebP, GIF or AVIF only, decided from the bytes; no SVG, which is a document that can carry script. The URL is fetched from inside the stack's network, so RemoteImage guards it: HTTPS only, every resolved address must be public, the connection is pinned to the checked address with CURLOPT_RESOLVE so a changing DNS answer cannot swap in a private one, redirects are not followed, and the body is read to a ceiling whatever its headers say. Uploading needs permission to write pages. MediaPolicy::create stays false, so the panel gains no free-standing "new file" button. In the MCP server a multipart binary field becomes `<field>_path` over stdio: the server reads the file from the caller's disk and sends it as multipart, instead of the model pushing base64 through its context. Over HTTP the server's disk is not the caller's, so only `body.url` is offered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-28 11:16:33 +02:00
parent daf6098203
commit a49816eb46
21 changed files with 1008 additions and 11 deletions
--- a/tests/Feature/Api/AdminApiContractTest.php
+++ b/tests/Feature/Api/AdminApiContractTest.php
@@ -9,6 +9,8 @@ use App\Models\Page;
 use App\Models\User;
 use Database\Seeders\RolePermissionSeeder;
 use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Http\UploadedFile;
+use Illuminate\Support\Facades\Storage;
 use PHPUnit\Framework\Attributes\Test;
 use Tests\FakesPireon;
 use Tests\TestCase;
@@ -120,6 +122,22 @@ class AdminApiContractTest extends TestCase
        $this->assertSame([], $problems, "The admin API and its specification disagree:\n".implode("\n", $problems));
    }

+    #[Test]
+    public function an_upload_answers_in_the_shape_it_documents(): void
+    {
+        Storage::fake((string) config('media.disk'));
+
+        $response = $this->post('/api/v1/media', [
+            'file' => UploadedFile::fake()->image('hero.jpg', 64, 48),
+        ], ['Accept' => 'application/json'])->assertCreated();
+
+        $record = $response->json('data');
+        $schema = $this->recordSchema('/v1/media', 'post', 201);
+
+        $this->assertSame([], array_values(array_diff(array_keys($record), $this->documentedKeys($schema))), 'Sent but not documented');
+        $this->assertSame([], array_values(array_diff($this->alwaysPresent($schema), array_keys($record))), 'Documented but not sent');
+    }
+
    /**
     * The pagination envelope every listing here uses — `data`, `links`,
     * `meta` — checked the same way the record inside it is: nothing sent
--- a/tests/Feature/Api/MediaUploadTest.php
+++ b/tests/Feature/Api/MediaUploadTest.php
@@ -0,0 +1,323 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Feature\Api;
+
+use App\Models\Media;
+use App\Models\Page;
+use App\Models\User;
+use App\Services\Media\HostResolver;
+use Database\Seeders\RolePermissionSeeder;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Http\UploadedFile;
+use Illuminate\Support\Facades\Http;
+use Illuminate\Support\Facades\Storage;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Test;
+use Tests\FakesPireon;
+use Tests\TestCase;
+
+class MediaUploadTest extends TestCase
+{
+    use FakesPireon, RefreshDatabase;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->seed(RolePermissionSeeder::class);
+
+        Storage::fake((string) config('media.disk'));
+    }
+
+    protected function actingWithRole(string $role): void
+    {
+        $user = User::factory()->create();
+        $user->syncRoles([$role]);
+
+        $this->withHeaders($this->apiHeadersFor($user));
+    }
+
+    /**
+     * Answers DNS for the hosts the tests invent, so nothing here depends on
+     * the network — and so a test can make a name resolve somewhere private.
+     *
+     * @param  array<string, array<int, string>>  $answers
+     */
+    protected function resolving(array $answers): void
+    {
+        $this->app->instance(HostResolver::class, new class($answers) extends HostResolver
+        {
+            /** @param array<string, array<int, string>> $answers */
+            public function __construct(private array $answers) {}
+
+            public function resolve(string $host): array
+            {
+                return $this->answers[$host] ?? [];
+            }
+        });
+    }
+
+    /**
+     * `Http::assertNothingSent()` would also count the token check against the
+     * identity server; what matters is that the image host was never asked.
+     */
+    protected function assertNoImageWasFetched(): void
+    {
+        Http::assertNotSent(fn ($request) => ! str_starts_with($request->url(), $this->pireonServer));
+    }
+
+    protected function png(int $width = 12, int $height = 8): string
+    {
+        return UploadedFile::fake()->image('x.png', $width, $height)->get();
+    }
+
+    #[Test]
+    public function it_stores_an_uploaded_image_where_the_panel_puts_a_pages_images(): void
+    {
+        $this->actingWithRole('editor');
+
+        $response = $this->post('/api/v1/media', [
+            'file' => UploadedFile::fake()->image('Foto de portada.JPG', 1200, 800),
+        ], ['Accept' => 'application/json']);
+
+        $response->assertCreated()
+            ->assertJsonPath('data.mime', 'image/jpeg')
+            ->assertJsonPath('data.width', 1200)
+            ->assertJsonPath('data.height', 800);
+
+        $path = $response->json('data.path');
+
+        // Named by the server, never after what the client sent.
+        $this->assertMatchesRegularExpression('#^pages/[A-Za-z0-9]{40}\.jpg$#', $path);
+        Storage::disk((string) config('media.disk'))->assertExists($path);
+        $this->assertDatabaseHas('media', ['path' => $path]);
+        $this->assertStringEndsWith($path, $response->json('data.url'));
+    }
+
+    #[Test]
+    public function the_path_it_answers_with_is_what_a_page_points_at(): void
+    {
+        $this->actingWithRole('editor');
+
+        $path = $this->post('/api/v1/media', [
+            'file' => UploadedFile::fake()->image('hero.png', 800, 600),
+        ], ['Accept' => 'application/json'])->json('data.path');
+
+        $page = Page::factory()->create();
+
+        $this->patchJson("/api/v1/pages/{$page->id}", ['featured_image' => $path])
+            ->assertOk()
+            ->assertJsonPath('data.featured_image', $path)
+            ->assertJsonPath('data.featured_image_media.width', 800);
+
+        // One upload, one row: attaching it to the page reuses what the upload
+        // already created instead of starting over.
+        $this->assertSame(1, Media::query()->where('path', $path)->count());
+    }
+
+    #[Test]
+    public function it_downloads_an_image_from_a_public_url(): void
+    {
+        $this->actingWithRole('editor');
+        $this->resolving(['images.example.test' => ['93.184.216.34']]);
+
+        Http::fake([
+            'https://images.example.test/*' => Http::response($this->png(30, 20), 200, ['Content-Type' => 'image/png']),
+        ]);
+
+        $this->postJson('/api/v1/media', ['url' => 'https://images.example.test/cover.png'])
+            ->assertCreated()
+            ->assertJsonPath('data.mime', 'image/png')
+            ->assertJsonPath('data.width', 30);
+    }
+
+    /**
+     * @return array<string, array{0: array<string, array<int, string>>, 1: string}>
+     */
+    public static function unsafeDestinations(): array
+    {
+        return [
+            'a name resolving to a private address' => [['intranet.example.test' => ['10.0.0.5']], 'https://intranet.example.test/a.png'],
+            'one private address among public ones' => [['mixed.example.test' => ['93.184.216.34', '192.168.1.10']], 'https://mixed.example.test/a.png'],
+            'loopback, literally' => [[], 'https://127.0.0.1/a.png'],
+            'the cloud metadata service' => [[], 'https://169.254.169.254/latest/meta-data'],
+            'shared address space' => [[], 'https://100.64.0.1/a.png'],
+            'IPv6 loopback' => [[], 'https://[::1]/a.png'],
+            'IPv4-mapped loopback' => [[], 'https://[::ffff:127.0.0.1]/a.png'],
+            'a name that does not resolve' => [[], 'https://nowhere.example.test/a.png'],
+        ];
+    }
+
+    /**
+     * @param  array<string, array<int, string>>  $dns
+     */
+    #[Test]
+    #[DataProvider('unsafeDestinations')]
+    public function it_never_fetches_from_somewhere_private(array $dns, string $url): void
+    {
+        $this->actingWithRole('editor');
+        $this->resolving($dns);
+
+        Http::fake();
+
+        $this->postJson('/api/v1/media', ['url' => $url])
+            ->assertUnprocessable()
+            ->assertJsonValidationErrors('url');
+
+        $this->assertNoImageWasFetched();
+    }
+
+    #[Test]
+    public function it_refuses_plain_http(): void
+    {
+        $this->actingWithRole('editor');
+        Http::fake();
+
+        $this->postJson('/api/v1/media', ['url' => 'http://images.example.test/a.png'])
+            ->assertUnprocessable()
+            ->assertJsonValidationErrors('url');
+
+        $this->assertNoImageWasFetched();
+    }
+
+    #[Test]
+    public function it_does_not_follow_a_redirect(): void
+    {
+        $this->actingWithRole('editor');
+        $this->resolving(['images.example.test' => ['93.184.216.34']]);
+
+        Http::fake([
+            'https://images.example.test/*' => Http::response('', 302, ['Location' => 'https://10.0.0.5/a.png']),
+        ]);
+
+        $this->postJson('/api/v1/media', ['url' => 'https://images.example.test/a.png'])
+            ->assertUnprocessable()
+            ->assertJsonPath('errors.url.0', __('rules.media.remote.redirect'));
+    }
+
+    #[Test]
+    public function it_stops_reading_past_the_size_limit(): void
+    {
+        config()->set('media.upload.max_kb', 1);
+
+        $this->actingWithRole('editor');
+        $this->resolving(['images.example.test' => ['93.184.216.34']]);
+
+        // No Content-Length to go by: the limit has to hold while reading.
+        Http::fake([
+            'https://images.example.test/*' => Http::response(str_repeat('x', 4096), 200),
+        ]);
+
+        $this->postJson('/api/v1/media', ['url' => 'https://images.example.test/a.png'])
+            ->assertUnprocessable()
+            ->assertJsonPath('errors.url.0', __('rules.media.remote.too_large', ['max' => 1]));
+    }
+
+    #[Test]
+    public function a_url_that_answers_with_something_else_is_not_stored(): void
+    {
+        $this->actingWithRole('editor');
+        $this->resolving(['images.example.test' => ['93.184.216.34']]);
+
+        // Claims to be an image; the bytes say otherwise.
+        Http::fake([
+            'https://images.example.test/*' => Http::response('<html>nope</html>', 200, ['Content-Type' => 'image/png']),
+        ]);
+
+        $this->postJson('/api/v1/media', ['url' => 'https://images.example.test/a.png'])
+            ->assertUnprocessable()
+            ->assertJsonPath('errors.url.0', __('rules.media.not_an_image'));
+
+        $this->assertDatabaseCount('media', 0);
+    }
+
+    /**
+     * @return array<string, array{0: string, 1: string}>
+     */
+    public static function notImages(): array
+    {
+        return [
+            'text renamed to .jpg' => ['photo.jpg', 'not an image at all'],
+            // Served from this domain, an SVG is a document that can run script.
+            'an SVG' => ['logo.svg', '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'],
+        ];
+    }
+
+    #[Test]
+    #[DataProvider('notImages')]
+    public function it_refuses_an_upload_that_is_not_a_raster_image(string $name, string $contents): void
+    {
+        $this->actingWithRole('editor');
+
+        $this->post('/api/v1/media', [
+            'file' => UploadedFile::fake()->createWithContent($name, $contents),
+        ], ['Accept' => 'application/json'])
+            ->assertUnprocessable()
+            ->assertJsonValidationErrors('file');
+
+        $this->assertDatabaseCount('media', 0);
+    }
+
+    #[Test]
+    public function it_wants_exactly_one_source(): void
+    {
+        $this->actingWithRole('editor');
+
+        $this->postJson('/api/v1/media', [])
+            ->assertUnprocessable()
+            ->assertJsonValidationErrors(['file', 'url']);
+
+        $this->post('/api/v1/media', [
+            'file' => UploadedFile::fake()->image('a.png'),
+            'url' => 'https://images.example.test/a.png',
+        ], ['Accept' => 'application/json'])
+            ->assertUnprocessable()
+            ->assertJsonValidationErrors('file');
+    }
+
+    #[Test]
+    public function whoever_writes_pages_may_upload(): void
+    {
+        $this->actingWithRole('writer');
+
+        $this->post('/api/v1/media', ['file' => UploadedFile::fake()->image('a.png')], ['Accept' => 'application/json'])
+            ->assertCreated();
+    }
+
+    #[Test]
+    public function whoever_only_reads_may_not(): void
+    {
+        $this->actingWithRole('viewer');
+
+        $this->post('/api/v1/media', ['file' => UploadedFile::fake()->image('b.png')], ['Accept' => 'application/json'])
+            ->assertForbidden();
+
+        $this->assertDatabaseCount('media', 0);
+    }
+
+    #[Test]
+    public function it_refuses_a_url_before_fetching_it_for_someone_who_may_not_upload(): void
+    {
+        $this->actingWithRole('viewer');
+        $this->resolving(['images.example.test' => ['93.184.216.34']]);
+
+        Http::fake();
+
+        $this->postJson('/api/v1/media', ['url' => 'https://images.example.test/a.png'])
+            ->assertForbidden();
+
+        $this->assertNoImageWasFetched();
+    }
+
+    #[Test]
+    public function the_panel_still_has_no_way_to_create_a_file_on_its_own(): void
+    {
+        $editor = User::factory()->create();
+        $editor->syncRoles(['editor']);
+
+        $this->assertTrue($editor->can('upload', Media::class));
+        $this->assertFalse($editor->can('create', Media::class));
+    }
+}