Improve outbound URL validation (#10833)

tests/Unit/NotificationWebhookSafetyTest.php

@@ -0,0 +1,36 @@
+<?php
+
+use App\Jobs\SendMessageToDiscordJob;
+use App\Jobs\SendMessageToSlackJob;
+use App\Notifications\Dto\DiscordMessage;
+use App\Notifications\Dto\SlackMessage;
+use Illuminate\Support\Facades\Http;
+use Tests\TestCase;
+
+uses(TestCase::class);
+
+it('blocks queued Slack notifications to IPv4-mapped link-local URLs', function () {
+    Http::fake();
+
+    $job = new SendMessageToSlackJob(
+        new SlackMessage('Test', 'Description'),
+        'http://[::ffff:169.254.169.254]/'
+    );
+
+    $job->handle();
+
+    Http::assertNothingSent();
+});
+
+it('blocks queued Discord notifications to IPv4-mapped link-local URLs', function () {
+    Http::fake();
+
+    $job = new SendMessageToDiscordJob(
+        new DiscordMessage('Test', 'Description', DiscordMessage::infoColor()),
+        'http://[::ffff:169.254.169.254]/'
+    );
+
+    $job->handle();
+
+    Http::assertNothingSent();
+});

tests/Unit/S3StorageEndpointValidationTest.php

@@ -23,8 +23,9 @@ it('rejects SSRF payloads on the S3 endpoint', function (string $endpoint) {
 
     expect($validator->fails())->toBeTrue("Expected rejection: {$endpoint}");
 })->with([
-    'AWS IMDS' => 'http://169.254.169.254/latest/meta-data/',
-    'AWS IMDS bare' => 'http://169.254.169.254',
+    'link-local address' => 'http://169.254.169.254/',
+    'link-local address bare' => 'http://169.254.169.254',
+    'link-local address IPv4-mapped IPv6' => 'http://[::ffff:169.254.169.254]/',
     'GCP metadata via link-local' => 'http://169.254.0.1',
     'loopback v4' => 'http://127.0.0.1',
     'loopback Redis' => 'http://127.0.0.1:6379',
@@ -87,5 +88,6 @@ it('blocks testConnection() for loopback endpoints', function (string $endpoint)
     'http loopback' => 'http://127.0.0.1:6379',
     'localhost' => 'http://localhost:9000',
     'IPv6 loopback' => 'http://[::1]',
+    'IPv4-mapped IPv6 link-local' => 'http://[::ffff:169.254.169.254]',
     'internal TLD' => 'http://backend.internal',
 ]);

tests/Unit/S3StorageTest.php

@@ -53,6 +53,7 @@ test('S3Storage model fillable attributes are configured correctly', function ()
     $s3Storage = new S3Storage;
 
     expect($s3Storage->getFillable())->toBe([
+        'team_id',
         'name',
         'description',
         'region',
@@ -74,6 +75,7 @@ test('S3Storage connection validation uses short s3 client timeouts', function (
         ->with(Mockery::on(function (array $config) {
             expect($config['http']['connect_timeout'])->toBe(15);
             expect($config['http']['timeout'])->toBe(15);
+            expect($config['http']['allow_redirects'])->toBeFalse();
 
             return true;
         }))

tests/Unit/SafeExternalUrlTest.php

@@ -11,7 +11,7 @@ it('accepts valid public URLs', function () {
 
     $validUrls = [
         'https://api.github.com',
-        'https://github.example.com/api/v3',
+        'https://github.com/api/v3',
         'https://example.com',
         'http://example.com',
     ];
@@ -22,6 +22,14 @@ it('accepts valid public URLs', function () {
     }
 });
 
+it('accepts custom external hostnames that resolve to public IPs', function () {
+    $rule = new SafeExternalUrl(fn (string $host): array => ['93.184.216.34']);
+
+    $validator = Validator::make(['url' => 'https://github.example.com/api/v3'], ['url' => $rule]);
+
+    expect($validator->passes())->toBeTrue('Expected valid custom external hostname');
+});
+
 it('rejects private IPv4 addresses', function (string $url) {
     $rule = new SafeExternalUrl;
 
@@ -42,6 +50,34 @@ it('rejects cloud metadata IP', function () {
     expect($validator->fails())->toBeTrue('Expected rejection: cloud metadata IP');
 });
 
+it('rejects hostnames that resolve to private or reserved addresses', function (string $url, array $resolvedIps) {
+    $rule = new SafeExternalUrl(fn (string $host): array => $resolvedIps);
+
+    $validator = Validator::make(['url' => $url], ['url' => $rule]);
+
+    expect($validator->fails())->toBeTrue("Expected rejection after DNS resolution: {$url}");
+})->with([
+    'hostname to link-local IP' => ['http://169.254.169.254.nip.io/', ['169.254.169.254']],
+    'hostname to loopback' => ['http://loopback.example.test/', ['127.0.0.1']],
+    'hostname to private IPv4' => ['http://private.example.test/', ['10.0.0.1']],
+    'hostname to IPv6 loopback' => ['http://ipv6-loopback.example.test/', ['::1']],
+    'hostname to IPv6 link-local' => ['http://ipv6-link-local.example.test/', ['fe80::1']],
+    'hostname to IPv6 ULA' => ['http://ipv6-ula.example.test/', ['fc00::1']],
+    'hostname to mapped private IPv4' => ['http://mapped-private.example.test/', ['::ffff:10.0.0.1']],
+]);
+
+it('rejects IPv4-mapped IPv6 literals for private or reserved IPv4 ranges', function (string $url) {
+    $rule = new SafeExternalUrl;
+
+    $validator = Validator::make(['url' => $url], ['url' => $rule]);
+
+    expect($validator->fails())->toBeTrue("Expected rejection: {$url}");
+})->with([
+    'mapped link-local IP' => 'http://[::ffff:169.254.169.254]/',
+    'mapped loopback' => 'http://[::ffff:127.0.0.1]/',
+    'mapped private' => 'http://[::ffff:10.0.0.1]/',
+]);
+
 it('rejects localhost and internal hostnames', function (string $url) {
     $rule = new SafeExternalUrl;
 
@@ -50,9 +86,12 @@ it('rejects localhost and internal hostnames', function (string $url) {
 })->with([
     'localhost' => 'http://localhost',
     'localhost with port' => 'http://localhost:8080',
+    'localhost with trailing dot' => 'http://localhost.',
     'zero address' => 'http://0.0.0.0',
     '.local domain' => 'http://myservice.local',
+    '.local domain with trailing dot' => 'http://myservice.local.',
     '.internal domain' => 'http://myservice.internal',
+    '.internal domain with trailing dot' => 'http://myservice.internal.',
 ]);
 
 it('rejects non-URL strings', function (string $value) {

tests/Unit/SafeWebhookUrlTest.php

@@ -59,6 +59,33 @@ it('rejects link-local range', function () {
     expect($validator->fails())->toBeTrue('Expected rejection: link-local IP');
 });
 
+it('rejects hostnames that resolve to blocked addresses', function (string $url, array $resolvedIps) {
+    $rule = new SafeWebhookUrl(fn (string $host): array => $resolvedIps);
+
+    $validator = Validator::make(['url' => $url], ['url' => $rule]);
+
+    expect($validator->fails())->toBeTrue("Expected rejection after DNS resolution: {$url}");
+})->with([
+    'hostname to link-local IP' => ['http://169.254.169.254.nip.io/', ['169.254.169.254']],
+    'hostname to loopback' => ['http://loopback.example.test/', ['127.0.0.1']],
+    'hostname to IPv6 loopback' => ['http://ipv6-loopback.example.test/', ['::1']],
+    'hostname to IPv6 link-local' => ['http://ipv6-link-local.example.test/', ['fe80::1']],
+    'hostname to IPv6 ULA' => ['http://ipv6-ula.example.test/', ['fc00::1']],
+    'hostname to mapped link-local IP' => ['http://mapped-link-local.example.test/', ['::ffff:169.254.169.254']],
+]);
+
+it('rejects IPv4-mapped IPv6 literals for blocked IPv4 ranges', function (string $url) {
+    $rule = new SafeWebhookUrl;
+
+    $validator = Validator::make(['url' => $url], ['url' => $rule]);
+
+    expect($validator->fails())->toBeTrue("Expected rejection: {$url}");
+})->with([
+    'mapped link-local IP' => 'http://[::ffff:169.254.169.254]/',
+    'mapped loopback' => 'http://[::ffff:127.0.0.1]/',
+    'mapped zero' => 'http://[::ffff:0.0.0.0]/',
+]);
+
 it('rejects localhost and internal hostnames', function (string $url) {
     $rule = new SafeWebhookUrl;
 
@@ -67,7 +94,9 @@ it('rejects localhost and internal hostnames', function (string $url) {
 })->with([
     'localhost' => 'http://localhost',
     'localhost with port' => 'http://localhost:8080',
+    'localhost with trailing dot' => 'http://localhost.',
     '.internal domain' => 'http://myservice.internal',
+    '.internal domain with trailing dot' => 'http://myservice.internal.',
 ]);
 
 it('rejects non-http schemes', function (string $value) {

tests/Unit/SendWebhookJobTest.php

@@ -2,7 +2,6 @@
 
 use App\Jobs\SendWebhookJob;
 use Illuminate\Support\Facades\Http;
-use Illuminate\Support\Facades\Log;
 use Tests\TestCase;
 
 uses(TestCase::class);
@@ -24,11 +23,6 @@ it('sends webhook to valid URLs', function () {
 
 it('blocks webhook to loopback address', function () {
     Http::fake();
-    Log::shouldReceive('warning')
-        ->once()
-        ->withArgs(function ($message) {
-            return str_contains($message, 'blocked unsafe webhook URL');
-        });
 
     $job = new SendWebhookJob(
         payload: ['event' => 'test'],
@@ -42,15 +36,23 @@ it('blocks webhook to loopback address', function () {
 
 it('blocks webhook to cloud metadata endpoint', function () {
     Http::fake();
-    Log::shouldReceive('warning')
-        ->once()
-        ->withArgs(function ($message) {
-            return str_contains($message, 'blocked unsafe webhook URL');
-        });
 
     $job = new SendWebhookJob(
         payload: ['event' => 'test'],
-        webhookUrl: 'http://169.254.169.254/latest/meta-data/'
+        webhookUrl: 'http://169.254.169.254/'
+    );
+
+    $job->handle();
+
+    Http::assertNothingSent();
+});
+
+it('blocks webhook to IPv4-mapped IPv6 link-local endpoint', function () {
+    Http::fake();
+
+    $job = new SendWebhookJob(
+        payload: ['event' => 'test'],
+        webhookUrl: 'http://[::ffff:169.254.169.254]/'
     );
 
     $job->handle();
@@ -60,11 +62,6 @@ it('blocks webhook to cloud metadata endpoint', function () {
 
 it('blocks webhook to localhost', function () {
     Http::fake();
-    Log::shouldReceive('warning')
-        ->once()
-        ->withArgs(function ($message) {
-            return str_contains($message, 'blocked unsafe webhook URL');
-        });
 
     $job = new SendWebhookJob(
         payload: ['event' => 'test'],