Reject Micropub updates that would do nothing

Updating a property we don't support returned a 200 with no change, so
clients believed the update had worked. Return invalid_request instead,
and apply nothing if any part of the request is unsupported.

This also covers empty operations, syndication URLs we have nowhere to
store, and photos added without an https URL, all of which were being
silently ignored.

Mastodon and Bluesky URLs can now be added and deleted through updates,
as they are our active syndication targets and q=source returns them.
Mastodon can be on any host, so accept the instances we have syndication
targets for rather than hard-coding one.

Stop reading instagram_url in q=source, as notes have no such column.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

app/Services/Micropub/Handlers/UpdateHandler.php

@@ -9,6 +9,7 @@ use App\Exceptions\MicropubHandlerException;
 use App\Exceptions\MicropubUnsupportedModelException;
 use App\Models\Media;
 use App\Models\Note;
+use App\Models\SyndicationTarget;
 use App\Services\Micropub\Data\MicropubData;
 use App\Services\Micropub\Data\UpdateData;
 use App\Services\Micropub\NoteResolver;
@@ -16,6 +17,26 @@ use Illuminate\Support\Str;
 
 class UpdateHandler implements MicropubHandlerInterface
 {
+    /**
+     * The properties each update operation can change.
+     */
+    private const SUPPORTED_PROPERTIES = [
+        'replace' => ['content', 'syndication'],
+        'add' => ['syndication', 'photo'],
+        'delete' => ['syndication', 'photo'],
+    ];
+
+    /**
+     * The syndication URLs we have somewhere to store. Mastodon URLs are also
+     * accepted, based on the instances in our syndication targets.
+     */
+    private const SYNDICATION_PREFIXES = [
+        'https://www.facebook.com',
+        'https://www.swarmapp.com',
+        'https://twitter.com',
+        'https://bsky.app',
+    ];
+
     public function dataClass(): string
     {
         return UpdateData::class;
@@ -43,7 +64,7 @@ class UpdateHandler implements MicropubHandlerInterface
 
         $note = resolve(NoteResolver::class)->fromUrl($data->updateUrl);
 
-        if ($data->updateReplace === null && $data->updateAdd === null && $data->updateDelete === null) {
+        if (! $data->updateReplace && ! $data->updateAdd && ! $data->updateDelete) {
             throw new MicropubHandlerException('Unsupported update operation');
         }
 
@@ -54,7 +75,6 @@ class UpdateHandler implements MicropubHandlerInterface
                 match ($property) {
                     'content' => $note->note = is_array($value[0]) ? ($value[0]['html'] ?? $value[0]['value'] ?? null) : $value[0],
                     'syndication' => $this->applySyndication($note, $value),
-                    default => null,
                 };
             }
         }
@@ -66,13 +86,11 @@ class UpdateHandler implements MicropubHandlerInterface
                 }
                 if ($property === 'photo') {
                     foreach ($value as $photoURL) {
-                        if (Str::startsWith($photoURL, 'https://')) {
-                            $media = new Media;
-                            $media->path = $photoURL;
-                            $media->type = 'image';
-                            $media->save();
-                            $note->media()->save($media);
-                        }
+                        $media = new Media;
+                        $media->path = $photoURL;
+                        $media->type = 'image';
+                        $media->save();
+                        $note->media()->save($media);
                     }
                 }
             }
@@ -91,21 +109,22 @@ class UpdateHandler implements MicropubHandlerInterface
     }
 
     /**
-     * Check the whole request before applying any of it, so that a malformed
-     * value can't leave a note partially updated. The spec requires the values
-     * of each property to be arrays.
+     * Check the whole request before applying any of it, so that a bad value
+     * can't leave a note partially updated, and a client is never told an
+     * update worked when we ignored it. The spec requires the values of each
+     * property to be arrays.
      *
      * @throws MicropubHandlerException
      */
     private function validateOperations(UpdateData $data): void
     {
-        foreach ([$data->updateReplace ?? [], $data->updateAdd ?? []] as $operation) {
-            foreach ($operation as $property => $values) {
+        foreach (['replace' => $data->updateReplace ?? [], 'add' => $data->updateAdd ?? []] as $operation => $properties) {
+            foreach ($properties as $property => $values) {
                 if (! is_string($property) || ! is_array($values) || $values === [] || ! array_is_list($values)) {
                     throw new MicropubHandlerException('The values of each property must be a non-empty array');
                 }
 
-                $this->validateValues($property, $values);
+                $this->validateProperty($operation, $property, $values);
             }
         }
 
@@ -116,6 +135,8 @@ class UpdateHandler implements MicropubHandlerInterface
                 if (! is_string($property)) {
                     throw new MicropubHandlerException('The properties to delete must be strings');
                 }
+
+                $this->validateProperty('delete', $property, []);
             }
 
             return;
@@ -126,17 +147,17 @@ class UpdateHandler implements MicropubHandlerInterface
                 throw new MicropubHandlerException('The values of each property must be an array');
             }
 
-            $this->validateValues($property, $values);
+            $this->validateProperty('delete', $property, $values);
         }
     }
 
     /**
      * @throws MicropubHandlerException
      */
-    private function validateValues(string $property, array $values): void
+    private function validateProperty(string $operation, string $property, array $values): void
     {
-        if (! in_array($property, ['content', 'syndication', 'photo'], true)) {
-            return;
+        if (! in_array($property, self::SUPPORTED_PROPERTIES[$operation], true)) {
+            throw new MicropubHandlerException("This server does not support the {$operation} operation on the {$property} property");
         }
 
         foreach ($values as $value) {
@@ -147,6 +168,14 @@ class UpdateHandler implements MicropubHandlerInterface
             if (! is_string($value)) {
                 throw new MicropubHandlerException("The {$property} property has an invalid value");
             }
+
+            if ($property === 'syndication' && ! Str::startsWith($value, [...self::SYNDICATION_PREFIXES, ...$this->mastodonPrefixes()])) {
+                throw new MicropubHandlerException("This server can not store the syndication URL {$value}");
+            }
+
+            if ($operation === 'add' && $property === 'photo' && ! Str::startsWith($value, 'https://')) {
+                throw new MicropubHandlerException('Photos must be added using an https URL');
+            }
         }
     }
 
@@ -157,7 +186,6 @@ class UpdateHandler implements MicropubHandlerInterface
                 match ($property) {
                     'syndication' => $this->clearSyndication($note),
                     'photo' => $note->media()->delete(),
-                    default => null,
                 };
             }
 
@@ -174,11 +202,28 @@ class UpdateHandler implements MicropubHandlerInterface
         }
     }
 
+    /**
+     * Mastodon can be on any host, so use the instances we syndicate to.
+     *
+     * @return string[]
+     */
+    private function mastodonPrefixes(): array
+    {
+        return SyndicationTarget::where('service_name', 'Mastodon')
+            ->pluck('uid')
+            ->map(fn (string $uid) => parse_url($uid, PHP_URL_HOST))
+            ->filter()
+            ->map(fn (string $host) => "https://{$host}/")
+            ->all();
+    }
+
     private function clearSyndication(Note $note): void
     {
         $note->facebook_url = null;
         $note->swarm_url = null;
         $note->tweet_id = null;
+        $note->mastodon_url = null;
+        $note->bluesky_url = null;
     }
 
     private function removeSyndicationValues(Note $note, array $urls): void
@@ -190,6 +235,10 @@ class UpdateHandler implements MicropubHandlerInterface
                 $note->swarm_url = null;
             } elseif (Str::startsWith($url, 'https://twitter.com')) {
                 $note->tweet_id = null;
+            } elseif (Str::startsWith($url, $this->mastodonPrefixes()) && $note->mastodon_url === $url) {
+                $note->mastodon_url = null;
+            } elseif (Str::startsWith($url, 'https://bsky.app') && $note->bluesky_url === $url) {
+                $note->bluesky_url = null;
             }
         }
     }
@@ -203,6 +252,10 @@ class UpdateHandler implements MicropubHandlerInterface
                 $note->swarm_url = $url;
             } elseif (Str::startsWith($url, 'https://twitter.com')) {
                 $note->tweet_id = basename(parse_url($url, PHP_URL_PATH));
+            } elseif (Str::startsWith($url, $this->mastodonPrefixes())) {
+                $note->mastodon_url = $url;
+            } elseif (Str::startsWith($url, 'https://bsky.app')) {
+                $note->bluesky_url = $url;
             }
         }
     }

app/Services/Micropub/NoteSource.php

@@ -46,7 +46,6 @@ class NoteSource
                 $note->tweet_id ? 'https://twitter.com/i/web/status/'.$note->tweet_id : null,
                 $note->facebook_url,
                 $note->swarm_url,
-                $note->instagram_url,
                 $note->mastodon_url,
                 $note->bluesky_url,
             ])),

tests/Feature/MicropubControllerTest.php

@@ -1241,6 +1241,81 @@ class MicropubControllerTest extends TestCase
         $this->assertSame('original', $note->refresh()->getRawOriginal('note'));
     }
 
+    #[Test]
+    public function update_that_would_do_nothing_is_rejected_without_changing_the_note(): void
+    {
+        $headers = ['HTTP_Authorization' => 'Bearer '.$this->getToken()];
+        $note = Note::factory()->create(['note' => 'original']);
+        $mediaCount = Media::count();
+
+        $operations = [
+            ['replace' => []],
+            ['replace' => ['category' => ['indieweb']]],
+            ['replace' => ['content' => ['changed'], 'category' => ['indieweb']]],
+            ['replace' => ['photo' => ['https://example.org/photo.jpg']]],
+            ['replace' => ['syndication' => ['https://unknown.example/1']]],
+            ['add' => ['category' => ['indieweb']]],
+            ['add' => ['content' => ['more']]],
+            ['add' => ['syndication' => ['https://unknown.example/1']]],
+            ['add' => ['photo' => ['http://example.org/photo.jpg']]],
+            ['delete' => ['category']],
+            ['delete' => ['category' => ['indieweb']]],
+            ['delete' => ['syndication' => ['https://unknown.example/1']]],
+        ];
+
+        foreach ($operations as $operation) {
+            $this->postJson('/api/post', ['action' => 'update', 'url' => $note->uri, ...$operation], $headers)
+                ->assertStatus(400)->assertJson(['error' => 'invalid_request']);
+        }
+
+        $this->assertSame('original', $note->refresh()->getRawOriginal('note'));
+        $this->assertSame($mediaCount, Media::count());
+    }
+
+    #[Test]
+    public function update_can_add_and_delete_mastodon_and_bluesky_syndication_urls(): void
+    {
+        $headers = ['HTTP_Authorization' => 'Bearer '.$this->getToken()];
+        SyndicationTarget::factory()->create(['uid' => 'https://mastodon.example/@me', 'service_name' => 'Mastodon']);
+        $note = Note::factory()->create();
+        $mastodon = 'https://mastodon.example/@me/1';
+        $bluesky = 'https://bsky.app/profile/jonnybarnes.uk/post/1';
+
+        $this->postJson('/api/post', ['action' => 'update', 'url' => $note->uri, 'add' => ['syndication' => [$mastodon, $bluesky]]], $headers)
+            ->assertOk();
+        $note->refresh();
+        $this->assertSame($mastodon, $note->mastodon_url);
+        $this->assertSame($bluesky, $note->bluesky_url);
+
+        $this->postJson('/api/post', ['action' => 'update', 'url' => $note->uri, 'delete' => ['syndication' => [$mastodon]]], $headers)
+            ->assertOk();
+        $note->refresh();
+        $this->assertNull($note->mastodon_url);
+        $this->assertSame($bluesky, $note->bluesky_url);
+
+        $this->postJson('/api/post', ['action' => 'update', 'url' => $note->uri, 'delete' => ['syndication']], $headers)
+            ->assertOk();
+        $this->assertNull($note->refresh()->bluesky_url);
+    }
+
+    #[Test]
+    public function update_rejects_mastodon_urls_from_instances_we_do_not_syndicate_to(): void
+    {
+        SyndicationTarget::factory()->create(['uid' => 'https://mastodon.example/@me', 'service_name' => 'Mastodon']);
+        SyndicationTarget::factory()->create(['uid' => 'https://other.example/@me', 'service_name' => 'Pixelfed']);
+        $note = Note::factory()->create(['mastodon_url' => 'https://mastodon.example/@me/1']);
+
+        foreach (['https://mastodon.social/@me/2', 'https://mastodon.example.evil/@me/2', 'https://other.example/@me/2'] as $url) {
+            $this->postJson(
+                '/api/post',
+                ['action' => 'update', 'url' => $note->uri, 'add' => ['syndication' => [$url]]],
+                ['HTTP_Authorization' => 'Bearer '.$this->getToken()]
+            )->assertStatus(400)->assertJson(['error' => 'invalid_request']);
+        }
+
+        $this->assertSame('https://mastodon.example/@me/1', $note->refresh()->mastodon_url);
+    }
+
     #[Test]
     public function update_with_an_unusable_url_is_rejected(): void
     {