mirror of
https://github.com/tiennm99/coolify.git
synced 2026-10-15 10:14:47 +00:00
refactor(storage): tighten S3 endpoint URL validation
Reuse the existing SafeWebhookUrl rule on the S3 Storage endpoint field so the create and edit forms go through the same URL-normalization path as webhook settings. Adds a matching guard inside S3Storage::testConnection() so background callers (scheduled backups, database import reuse) also validate the endpoint before building the S3 client. Also fixes an IPv6-bracket edge case in SafeWebhookUrl so `http://[::1]` style hosts are normalized before the filter_var IP check — the rule's own loopback test was already asserting this behaviour.
This commit is contained in:
1 parent
57ea0764b8
commit
297e9c41e1
5 files changed
+113
-6
No files matched your search
@@ -3,6 +3,7 @@
|
||||
namespace App\Livewire\Storage;
|
||||
|
||||
use App\Models\S3Storage;
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Support\ValidationPatterns;
|
||||
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
||||
use Illuminate\Support\Uri;
|
||||
@@ -37,7 +38,7 @@ class Create extends Component
|
||||
'key' => 'required|max:255',
|
||||
'secret' => 'required|max:255',
|
||||
'bucket' => 'required|max:255',
|
||||
'endpoint' => 'required|url|max:255',
|
||||
'endpoint' => ['required', 'max:255', new SafeWebhookUrl],
|
||||
];
|
||||
}
|
||||
|
||||
@@ -55,7 +56,6 @@ class Create extends Component
|
||||
'bucket.required' => 'The Bucket field is required.',
|
||||
'bucket.max' => 'The Bucket may not be greater than 255 characters.',
|
||||
'endpoint.required' => 'The Endpoint field is required.',
|
||||
'endpoint.url' => 'The Endpoint must be a valid URL.',
|
||||
'endpoint.max' => 'The Endpoint may not be greater than 255 characters.',
|
||||
]
|
||||
);
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
namespace App\Livewire\Storage;
|
||||
|
||||
use App\Models\S3Storage;
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Support\ValidationPatterns;
|
||||
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
@@ -42,7 +43,7 @@ class Form extends Component
|
||||
'key' => 'required|max:255',
|
||||
'secret' => 'required|max:255',
|
||||
'bucket' => 'required|max:255',
|
||||
'endpoint' => 'required|url|max:255',
|
||||
'endpoint' => ['required', 'max:255', new SafeWebhookUrl],
|
||||
];
|
||||
}
|
||||
|
||||
@@ -60,7 +61,6 @@ class Form extends Component
|
||||
'bucket.required' => 'The Bucket field is required.',
|
||||
'bucket.max' => 'The Bucket may not be greater than 255 characters.',
|
||||
'endpoint.required' => 'The Endpoint field is required.',
|
||||
'endpoint.url' => 'The Endpoint must be a valid URL.',
|
||||
'endpoint.max' => 'The Endpoint may not be greater than 255 characters.',
|
||||
]
|
||||
);
|
||||
|
||||
@@ -2,11 +2,13 @@
|
||||
|
||||
namespace App\Models;
|
||||
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Traits\HasSafeStringAttribute;
|
||||
use Illuminate\Database\Eloquent\Casts\Attribute;
|
||||
use Illuminate\Database\Eloquent\Factories\HasFactory;
|
||||
use Illuminate\Notifications\Messages\MailMessage;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Illuminate\Support\Facades\Validator;
|
||||
|
||||
class S3Storage extends BaseModel
|
||||
{
|
||||
@@ -132,6 +134,14 @@ class S3Storage extends BaseModel
|
||||
public function testConnection(bool $shouldSave = false)
|
||||
{
|
||||
try {
|
||||
$validator = Validator::make(
|
||||
['endpoint' => $this['endpoint']],
|
||||
['endpoint' => ['required', new SafeWebhookUrl]],
|
||||
);
|
||||
if ($validator->fails()) {
|
||||
throw new \RuntimeException('S3 endpoint is not allowed: '.$validator->errors()->first('endpoint'));
|
||||
}
|
||||
|
||||
$disk = Storage::build([
|
||||
'driver' => 's3',
|
||||
'region' => $this['region'],
|
||||
|
||||
@@ -40,9 +40,15 @@ class SafeWebhookUrl implements ValidationRule
|
||||
|
||||
$host = strtolower($host);
|
||||
|
||||
// Strip IPv6 brackets (e.g. "[::1]" -> "::1") before IP checks so bracketed
|
||||
// literals can't sneak past filter_var FILTER_VALIDATE_IP.
|
||||
$hostForIpCheck = (str_starts_with($host, '[') && str_ends_with($host, ']'))
|
||||
? substr($host, 1, -1)
|
||||
: $host;
|
||||
|
||||
// Block well-known dangerous hostnames
|
||||
$blockedHosts = ['localhost', '0.0.0.0', '::1'];
|
||||
if (in_array($host, $blockedHosts) || str_ends_with($host, '.internal')) {
|
||||
if (in_array($hostForIpCheck, $blockedHosts) || str_ends_with($host, '.internal')) {
|
||||
Log::warning('Webhook URL points to blocked host', [
|
||||
'attribute' => $attribute,
|
||||
'host' => $host,
|
||||
@@ -55,7 +61,7 @@ class SafeWebhookUrl implements ValidationRule
|
||||
}
|
||||
|
||||
// Block loopback (127.0.0.0/8) and link-local/metadata (169.254.0.0/16) when IP is provided directly
|
||||
if (filter_var($host, FILTER_VALIDATE_IP) && ($this->isLoopback($host) || $this->isLinkLocal($host))) {
|
||||
if (filter_var($hostForIpCheck, FILTER_VALIDATE_IP) && ($this->isLoopback($hostForIpCheck) || $this->isLinkLocal($hostForIpCheck))) {
|
||||
Log::warning('Webhook URL points to blocked IP range', [
|
||||
'attribute' => $attribute,
|
||||
'host' => $host,
|
||||
|
||||
Reference in new issue
Block a user