fix(security): re-validate SSRF on every redirect hop in SafeHttpFetcher

The shared fetcher guarded only the initial URL, then followed redirects without re-checking each hop, so a public page could 302 to an internal address. Follow redirects manually and run the SSRF guard on every hop; throw when the redirect cap is exceeded. Also hardens brand autofill and logo downloads.

app/Services/Brand/SafeHttpFetcher.php

@@ -8,6 +8,7 @@
 use Illuminate\Http\Client\Response;
 use Illuminate\Support\Facades\Http;
 use RuntimeException;
+use Symfony\Component\DomCrawler\UriResolver;
 
 /**
  * HTTP fetcher with SSRF protection, timeout, redirect cap and a branded user-agent.
@@ -36,13 +37,38 @@ public function get(string $url): Response
     {
         $this->guardAgainstSsrf($url);
 
-        try {
-            $response = Http::timeout(self::TIMEOUT_SECONDS)
-                ->withUserAgent(self::USER_AGENT)
-                ->withOptions(['allow_redirects' => ['max' => self::MAX_REDIRECTS]])
-                ->get($url);
-        } catch (ConnectionException $e) {
-            throw new RuntimeException(__('workspaces.create.autofill_errors.unreachable', ['reason' => $e->getMessage()]));
+        $currentUrl = $url;
+
+        // Redirects are followed manually (allow_redirects disabled) so that every
+        // hop's Location target is re-validated against the SSRF guard before it is
+        // ever requested. A public page could otherwise 302 to an internal host and
+        // Guzzle's built-in redirect following would fetch it without re-checking.
+        for ($hop = 0; ; $hop++) {
+            try {
+                $response = Http::timeout(self::TIMEOUT_SECONDS)
+                    ->withUserAgent(self::USER_AGENT)
+                    ->withOptions(['allow_redirects' => false])
+                    ->get($currentUrl);
+            } catch (ConnectionException $e) {
+                throw new RuntimeException(__('workspaces.create.autofill_errors.unreachable', ['reason' => $e->getMessage()]));
+            }
+
+            if (! $response->redirect() || $hop >= self::MAX_REDIRECTS) {
+                break;
+            }
+
+            $location = $response->header('Location');
+
+            if ($location === '') {
+                break;
+            }
+
+            $currentUrl = (string) UriResolver::resolve($location, $currentUrl);
+            $this->guardAgainstSsrf($currentUrl);
+        }
+
+        if ($response->redirect()) {
+            throw new RuntimeException(__('workspaces.create.autofill_errors.unreachable', ['reason' => 'too many redirects']));
         }
 
         if ($response->failed()) {

tests/Unit/Services/Brand/SafeHttpFetcherTest.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+use App\Services\Brand\SafeHttpFetcher;
+use Illuminate\Support\Facades\Http;
+
+// Public IP literals let SafeHttpFetcher's SSRF guard pass without a real DNS
+// lookup; Http::fake intercepts the request before any network I/O.
+
+test('blocks a redirect whose location resolves to a private ip', function () {
+    Http::fake([
+        // The malicious hop responds successfully (not with an error/exception) so a
+        // vulnerable implementation that auto-follows it would return this body —
+        // proving the guard, not an unrelated stray-request failure, stopped it.
+        'https://93.184.216.34/start' => Http::response('', 302, ['Location' => 'http://127.0.0.1/internal']),
+        'http://127.0.0.1/internal' => Http::response('internal secret', 200),
+    ]);
+
+    expect(app(SafeHttpFetcher::class)->tryGet('https://93.184.216.34/start'))->toBeNull();
+
+    Http::assertSent(fn ($request) => str_contains($request->url(), '93.184.216.34'));
+    Http::assertNotSent(fn ($request) => str_contains($request->url(), '127.0.0.1'));
+});
+
+test('follows a legitimate redirect from one public host to another', function () {
+    Http::fake([
+        'https://93.184.216.34/start' => Http::response('', 301, ['Location' => 'https://1.1.1.1/final']),
+        'https://1.1.1.1/final' => Http::response('final body', 200),
+    ]);
+
+    $response = app(SafeHttpFetcher::class)->get('https://93.184.216.34/start');
+
+    expect($response->status())->toBe(200)
+        ->and($response->body())->toBe('final body');
+
+    Http::assertSentInOrder([
+        fn ($request) => str_contains($request->url(), '93.184.216.34'),
+        fn ($request) => str_contains($request->url(), '1.1.1.1'),
+    ]);
+});
+
+test('resolves a relative location header against the current url before guarding', function () {
+    Http::fake([
+        'https://93.184.216.34/start' => Http::response('', 302, ['Location' => '/final']),
+        'https://93.184.216.34/final' => Http::response('final body', 200),
+    ]);
+
+    $response = app(SafeHttpFetcher::class)->get('https://93.184.216.34/start');
+
+    expect($response->status())->toBe(200)
+        ->and($response->body())->toBe('final body');
+});
+
+test('throws when a redirect chain exceeds the redirect cap', function () {
+    Http::fake([
+        'https://93.184.216.34/start' => Http::response('', 302, ['Location' => 'https://93.184.216.35/hop']),
+        'https://93.184.216.35/hop' => Http::response('', 302, ['Location' => 'https://93.184.216.36/hop']),
+        'https://93.184.216.36/hop' => Http::response('', 302, ['Location' => 'https://93.184.216.37/hop']),
+        'https://93.184.216.37/hop' => Http::response('', 302, ['Location' => 'https://93.184.216.38/hop']),
+    ]);
+
+    expect(fn () => app(SafeHttpFetcher::class)->get('https://93.184.216.34/start'))
+        ->toThrow(RuntimeException::class);
+
+    expect(app(SafeHttpFetcher::class)->tryGet('https://93.184.216.34/start'))->toBeNull();
+
+    // The cap (3) is hit after following 3 redirects (4 requests); the next hop must never fire.
+    Http::assertNotSent(fn ($request) => str_contains($request->url(), '93.184.216.38'));
+});