From 0fce7fa9481aa1bcca06d767075684a11e032c79 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 26 Mar 2026 13:45:33 +0100 Subject: [PATCH] fix: add URL validation for GitHub source api_url and html_url fields Add SafeExternalUrl validation rule that ensures URLs point to publicly-routable hosts. Apply to all GitHub source entry points (Livewire Create, Livewire Change, API create and update). Co-Authored-By: Claude Opus 4.6 --- app/Http/Controllers/Api/GithubController.php | 21 ++--- app/Livewire/Source/Github/Change.php | 40 ++++----- app/Livewire/Source/Github/Create.php | 5 +- app/Rules/SafeExternalUrl.php | 81 +++++++++++++++++++ tests/Unit/SafeExternalUrlTest.php | 75 +++++++++++++++++ 5 files changed, 193 insertions(+), 29 deletions(-) create mode 100644 app/Rules/SafeExternalUrl.php create mode 100644 tests/Unit/SafeExternalUrlTest.php diff --git a/app/Http/Controllers/Api/GithubController.php b/app/Http/Controllers/Api/GithubController.php index f6a6b3513..9a2cf2b9f 100644 --- a/app/Http/Controllers/Api/GithubController.php +++ b/app/Http/Controllers/Api/GithubController.php @@ -5,6 +5,9 @@ use App\Http\Controllers\Controller; use App\Models\GithubApp; use App\Models\PrivateKey; +use App\Rules\SafeExternalUrl; +use Illuminate\Database\Eloquent\ModelNotFoundException; +use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Http; use Illuminate\Support\Str; @@ -181,7 +184,7 @@ public function create_github_app(Request $request) return invalidTokenResponse(); } $return = validateIncomingRequest($request); - if ($return instanceof \Illuminate\Http\JsonResponse) { + if ($return instanceof JsonResponse) { return $return; } @@ -204,8 +207,8 @@ public function create_github_app(Request $request) $validator = customApiValidator($request->all(), [ 'name' => 'required|string|max:255', 'organization' => 'nullable|string|max:255', - 'api_url' => 'required|string|url', - 'html_url' => 'required|string|url', + 'api_url' => ['required', 'string', 'url', new SafeExternalUrl], + 'html_url' => ['required', 'string', 'url', new SafeExternalUrl], 'custom_user' => 'nullable|string|max:255', 'custom_port' => 'nullable|integer|min:1|max:65535', 'app_id' => 'required|integer', @@ -370,7 +373,7 @@ public function load_repositories($github_app_id) return response()->json([ 'repositories' => $repositories->sortBy('name')->values(), ]); - } catch (\Illuminate\Database\Eloquent\ModelNotFoundException $e) { + } catch (ModelNotFoundException $e) { return response()->json(['message' => 'GitHub app not found'], 404); } catch (\Throwable $e) { return handleError($e); @@ -472,7 +475,7 @@ public function load_branches($github_app_id, $owner, $repo) return response()->json([ 'branches' => $branches, ]); - } catch (\Illuminate\Database\Eloquent\ModelNotFoundException $e) { + } catch (ModelNotFoundException $e) { return response()->json(['message' => 'GitHub app not found'], 404); } catch (\Throwable $e) { return handleError($e); @@ -587,10 +590,10 @@ public function update_github_app(Request $request, $github_app_id) $rules['organization'] = 'nullable|string'; } if (isset($payload['api_url'])) { - $rules['api_url'] = 'url'; + $rules['api_url'] = ['url', new SafeExternalUrl]; } if (isset($payload['html_url'])) { - $rules['html_url'] = 'url'; + $rules['html_url'] = ['url', new SafeExternalUrl]; } if (isset($payload['custom_user'])) { $rules['custom_user'] = 'string'; @@ -651,7 +654,7 @@ public function update_github_app(Request $request, $github_app_id) 'message' => 'GitHub app updated successfully', 'data' => $githubApp, ]); - } catch (\Illuminate\Database\Eloquent\ModelNotFoundException $e) { + } catch (ModelNotFoundException $e) { return response()->json([ 'message' => 'GitHub app not found', ], 404); @@ -736,7 +739,7 @@ public function delete_github_app($github_app_id) return response()->json([ 'message' => 'GitHub app deleted successfully', ]); - } catch (\Illuminate\Database\Eloquent\ModelNotFoundException $e) { + } catch (ModelNotFoundException $e) { return response()->json([ 'message' => 'GitHub app not found', ], 404); diff --git a/app/Livewire/Source/Github/Change.php b/app/Livewire/Source/Github/Change.php index 17323fdec..d6537069c 100644 --- a/app/Livewire/Source/Github/Change.php +++ b/app/Livewire/Source/Github/Change.php @@ -5,6 +5,7 @@ use App\Jobs\GithubAppPermissionJob; use App\Models\GithubApp; use App\Models\PrivateKey; +use App\Rules\SafeExternalUrl; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Facades\Http; use Lcobucci\JWT\Configuration; @@ -71,24 +72,27 @@ class Change extends Component public $privateKeys; - protected $rules = [ - 'name' => 'required|string', - 'organization' => 'nullable|string', - 'apiUrl' => 'required|string', - 'htmlUrl' => 'required|string', - 'customUser' => 'required|string', - 'customPort' => 'required|int', - 'appId' => 'nullable|int', - 'installationId' => 'nullable|int', - 'clientId' => 'nullable|string', - 'clientSecret' => 'nullable|string', - 'webhookSecret' => 'nullable|string', - 'isSystemWide' => 'required|bool', - 'contents' => 'nullable|string', - 'metadata' => 'nullable|string', - 'pullRequests' => 'nullable|string', - 'privateKeyId' => 'nullable|int', - ]; + protected function rules(): array + { + return [ + 'name' => 'required|string', + 'organization' => 'nullable|string', + 'apiUrl' => ['required', 'string', 'url', new SafeExternalUrl], + 'htmlUrl' => ['required', 'string', 'url', new SafeExternalUrl], + 'customUser' => 'required|string', + 'customPort' => 'required|int', + 'appId' => 'nullable|int', + 'installationId' => 'nullable|int', + 'clientId' => 'nullable|string', + 'clientSecret' => 'nullable|string', + 'webhookSecret' => 'nullable|string', + 'isSystemWide' => 'required|bool', + 'contents' => 'nullable|string', + 'metadata' => 'nullable|string', + 'pullRequests' => 'nullable|string', + 'privateKeyId' => 'nullable|int', + ]; + } public function boot() { diff --git a/app/Livewire/Source/Github/Create.php b/app/Livewire/Source/Github/Create.php index 4ece6a92f..ec2ba3f08 100644 --- a/app/Livewire/Source/Github/Create.php +++ b/app/Livewire/Source/Github/Create.php @@ -3,6 +3,7 @@ namespace App\Livewire\Source\Github; use App\Models\GithubApp; +use App\Rules\SafeExternalUrl; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Component; @@ -37,8 +38,8 @@ public function createGitHubApp() $this->validate([ 'name' => 'required|string', 'organization' => 'nullable|string', - 'api_url' => 'required|string', - 'html_url' => 'required|string', + 'api_url' => ['required', 'string', 'url', new SafeExternalUrl], + 'html_url' => ['required', 'string', 'url', new SafeExternalUrl], 'custom_user' => 'required|string', 'custom_port' => 'required|int', 'is_system_wide' => 'required|bool', diff --git a/app/Rules/SafeExternalUrl.php b/app/Rules/SafeExternalUrl.php new file mode 100644 index 000000000..41299d6c1 --- /dev/null +++ b/app/Rules/SafeExternalUrl.php @@ -0,0 +1,81 @@ + $attribute, + 'url' => $value, + 'host' => $host, + 'ip' => request()->ip(), + 'user_id' => auth()->id(), + ]); + $fail('The :attribute must not point to internal hosts.'); + + return; + } + + // Resolve hostname to IP and block private/reserved ranges + $ip = gethostbyname($host); + + // gethostbyname returns the original hostname on failure (e.g. unresolvable) + if ($ip === $host && ! filter_var($host, FILTER_VALIDATE_IP)) { + $fail('The :attribute host could not be resolved.'); + + return; + } + + if (! filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) { + Log::warning('External URL resolves to private or reserved IP', [ + 'attribute' => $attribute, + 'url' => $value, + 'host' => $host, + 'resolved_ip' => $ip, + 'ip' => request()->ip(), + 'user_id' => auth()->id(), + ]); + $fail('The :attribute must not point to a private or reserved IP address.'); + + return; + } + } +} diff --git a/tests/Unit/SafeExternalUrlTest.php b/tests/Unit/SafeExternalUrlTest.php new file mode 100644 index 000000000..b2bc13337 --- /dev/null +++ b/tests/Unit/SafeExternalUrlTest.php @@ -0,0 +1,75 @@ + $url], ['url' => $rule]); + expect($validator->passes())->toBeTrue("Expected valid: {$url}"); + } +}); + +it('rejects private IPv4 addresses', function (string $url) { + $rule = new SafeExternalUrl; + + $validator = Validator::make(['url' => $url], ['url' => $rule]); + expect($validator->fails())->toBeTrue("Expected rejection: {$url}"); +})->with([ + 'loopback' => 'http://127.0.0.1', + 'loopback with port' => 'http://127.0.0.1:6379', + '10.x range' => 'http://10.0.0.1', + '172.16.x range' => 'http://172.16.0.1', + '192.168.x range' => 'http://192.168.1.1', +]); + +it('rejects cloud metadata IP', function () { + $rule = new SafeExternalUrl; + + $validator = Validator::make(['url' => 'http://169.254.169.254'], ['url' => $rule]); + expect($validator->fails())->toBeTrue('Expected rejection: cloud metadata IP'); +}); + +it('rejects localhost and internal hostnames', function (string $url) { + $rule = new SafeExternalUrl; + + $validator = Validator::make(['url' => $url], ['url' => $rule]); + expect($validator->fails())->toBeTrue("Expected rejection: {$url}"); +})->with([ + 'localhost' => 'http://localhost', + 'localhost with port' => 'http://localhost:8080', + 'zero address' => 'http://0.0.0.0', + '.local domain' => 'http://myservice.local', + '.internal domain' => 'http://myservice.internal', +]); + +it('rejects non-URL strings', function (string $value) { + $rule = new SafeExternalUrl; + + $validator = Validator::make(['url' => $value], ['url' => $rule]); + expect($validator->fails())->toBeTrue("Expected rejection: {$value}"); +})->with([ + 'plain string' => 'not-a-url', + 'ftp scheme' => 'ftp://example.com', + 'javascript scheme' => 'javascript:alert(1)', + 'no scheme' => 'example.com', +]); + +it('rejects URLs with IPv6 loopback', function () { + $rule = new SafeExternalUrl; + + $validator = Validator::make(['url' => 'http://[::1]'], ['url' => $rule]); + expect($validator->fails())->toBeTrue('Expected rejection: IPv6 loopback'); +});