From 5483712e2959373a24ad9ef56c5940e2db3eb462 Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Mon, 2 Nov 2026 21:13:55 +0000 Subject: [PATCH] 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 --- .../Micropub/Handlers/UpdateHandler.php | 93 +++++++++++++++---- app/Services/Micropub/NoteSource.php | 1 - tests/Feature/MicropubControllerTest.php | 75 +++++++++++++++ 3 files changed, 148 insertions(+), 21 deletions(-) diff --git a/app/Services/Micropub/Handlers/UpdateHandler.php b/app/Services/Micropub/Handlers/UpdateHandler.php index 0840e5ee..5515444d 100644 --- a/app/Services/Micropub/Handlers/UpdateHandler.php +++ b/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; } } } diff --git a/app/Services/Micropub/NoteSource.php b/app/Services/Micropub/NoteSource.php index 457953f4..e0360e8b 100644 --- a/app/Services/Micropub/NoteSource.php +++ b/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, ])), diff --git a/tests/Feature/MicropubControllerTest.php b/tests/Feature/MicropubControllerTest.php index 60c5ec96..2e33ff90 100644 --- a/tests/Feature/MicropubControllerTest.php +++ b/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 {