Make LogoAttacher honor its swallow-all contract; drop the caller try/catch

LogoAttacher::attach promised in its docblock that any failure — including a
persistence error — is logged and swallowed so the caller need not handle it.
But the persistence block was try/finally with no catch, so a Throwable from
clearMediaCollection/addMediaFromPath escaped. Both call sites (store and
updateSettings) each wrapped the call in an identical try/catch + Log::warning
to compensate — a band-aid duplicated across the controller.

Fix it at the root: the persistence block now catches Throwable, logs it, and
returns false, matching the documented contract. Both controller call sites
collapse to a single attach() line, and the now-unused Log/Throwable imports
are dropped.

Adds LogoAttacherTest covering the success path, the swallowed persistence
failure, a failed fetch, and a rejected mime type.

app/Http/Controllers/App/WorkspaceController.php

@@ -21,12 +21,10 @@
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use Illuminate\Http\Resources\Json\AnonymousResourceCollection;
-use Illuminate\Support\Facades\Log;
 use Inertia\Inertia;
 use Inertia\Response;
 use RuntimeException;
 use Symfony\Component\HttpFoundation\Response as SymfonyResponse;
-use Throwable;
 
 class WorkspaceController extends Controller
 {
@@ -105,15 +103,7 @@ public function store(StoreWorkspaceRequest $request, LogoAttacher $logoAttacher
         $workspace = CreateWorkspace::execute($user, $validated);
 
         if ($logoUrl = data_get($validated, 'logo_url')) {
-            try {
-                $logoAttacher->attach($workspace, $logoUrl);
-            } catch (Throwable $e) {
-                Log::warning('Logo attach failed during workspace creation', [
-                    'workspace_id' => $workspace->id,
-                    'logo_url' => $logoUrl,
-                    'error' => $e->getMessage(),
-                ]);
-            }
+            $logoAttacher->attach($workspace, $logoUrl);
         }
 
         return redirect()->route('app.accounts')
@@ -217,15 +207,7 @@ public function updateSettings(UpdateWorkspaceRequest $request, LogoAttacher $lo
         $workspace->update($validated);
 
         if ($logoUrl) {
-            try {
-                $logoAttacher->attach($workspace, $logoUrl);
-            } catch (Throwable $e) {
-                Log::warning('Logo attach failed during workspace update', [
-                    'workspace_id' => $workspace->id,
-                    'logo_url' => $logoUrl,
-                    'error' => $e->getMessage(),
-                ]);
-            }
+            $logoAttacher->attach($workspace, $logoUrl);
         }
 
         return back()->with('flash.success', __('settings.flash.workspace_updated'));

app/Services/Brand/LogoAttacher.php

@@ -6,6 +6,7 @@
 
 use App\Models\Workspace;
 use Illuminate\Support\Facades\Log;
+use Throwable;
 
 class LogoAttacher
 {
@@ -77,6 +78,10 @@ public function attach(Workspace $workspace, string $logoUrl): bool
             );
 
             return true;
+        } catch (Throwable $e) {
+            Log::warning('Logo rejected — persistence error', ['url' => $logoUrl, 'error' => $e->getMessage()]);
+
+            return false;
         } finally {
             if (file_exists($tempPath)) {
                 @unlink($tempPath);

tests/Unit/Services/Brand/LogoAttacherTest.php

@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+use App\Models\Workspace;
+use App\Services\Brand\LogoAttacher;
+use Illuminate\Support\Facades\Http;
+
+// A public IP literal as the host lets SafeHttpFetcher's SSRF guard pass without
+// a real DNS lookup; Http::fake() intercepts the request before any network I/O.
+const LOGO_URL = 'https://93.184.216.34/logo.png';
+
+function fakeLogo(string $mime = 'image/png', string $body = 'PNG'): void
+{
+    Http::fake(['*' => Http::response($body, 200, [
+        'Content-Type' => $mime,
+        'Content-Length' => (string) strlen($body),
+    ])]);
+}
+
+test('attaches the downloaded logo and returns true', function () {
+    fakeLogo();
+
+    $workspace = mock(Workspace::class);
+    $workspace->shouldReceive('clearMediaCollection')->once();
+    $workspace->shouldReceive('addMediaFromPath')->once();
+
+    expect(app(LogoAttacher::class)->attach($workspace, LOGO_URL))->toBeTrue();
+});
+
+test('swallows a persistence failure and returns false', function () {
+    fakeLogo();
+
+    $workspace = mock(Workspace::class);
+    $workspace->shouldReceive('clearMediaCollection')->once()->andThrow(new RuntimeException('media store down'));
+
+    expect(app(LogoAttacher::class)->attach($workspace, LOGO_URL))->toBeFalse();
+});
+
+test('returns false when the fetch fails', function () {
+    Http::fake(['*' => Http::response('', 500)]);
+
+    expect(app(LogoAttacher::class)->attach(mock(Workspace::class), LOGO_URL))->toBeFalse();
+});
+
+test('rejects a disallowed mime type and returns false', function () {
+    fakeLogo('text/html', '<html></html>');
+
+    $workspace = mock(Workspace::class);
+    $workspace->shouldNotReceive('addMediaFromPath');
+
+    expect(app(LogoAttacher::class)->attach($workspace, LOGO_URL))->toBeFalse();
+});