Add URL validation for GitHub source fields (#9190)
This commit is contained in:
commit
5ed77f337a
5 changed files with 193 additions and 29 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
81
app/Rules/SafeExternalUrl.php
Normal file
81
app/Rules/SafeExternalUrl.php
Normal file
|
|
@ -0,0 +1,81 @@
|
|||
<?php
|
||||
|
||||
namespace App\Rules;
|
||||
|
||||
use Closure;
|
||||
use Illuminate\Contracts\Validation\ValidationRule;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
|
||||
class SafeExternalUrl implements ValidationRule
|
||||
{
|
||||
/**
|
||||
* Run the validation rule.
|
||||
*
|
||||
* Validates that a URL points to an external, publicly-routable host.
|
||||
* Blocks private IP ranges, reserved ranges, localhost, and link-local
|
||||
* addresses to prevent Server-Side Request Forgery (SSRF).
|
||||
*/
|
||||
public function validate(string $attribute, mixed $value, Closure $fail): void
|
||||
{
|
||||
if (! filter_var($value, FILTER_VALIDATE_URL)) {
|
||||
$fail('The :attribute must be a valid URL.');
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
$scheme = strtolower(parse_url($value, PHP_URL_SCHEME) ?? '');
|
||||
if (! in_array($scheme, ['https', 'http'])) {
|
||||
$fail('The :attribute must use the http or https scheme.');
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
$host = parse_url($value, PHP_URL_HOST);
|
||||
if (! $host) {
|
||||
$fail('The :attribute must contain a valid host.');
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
$host = strtolower($host);
|
||||
|
||||
// Block well-known internal hostnames
|
||||
$internalHosts = ['localhost', '0.0.0.0', '::1'];
|
||||
if (in_array($host, $internalHosts) || str_ends_with($host, '.local') || str_ends_with($host, '.internal')) {
|
||||
Log::warning('External URL points to internal host', [
|
||||
'attribute' => $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;
|
||||
}
|
||||
}
|
||||
}
|
||||
75
tests/Unit/SafeExternalUrlTest.php
Normal file
75
tests/Unit/SafeExternalUrlTest.php
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
<?php
|
||||
|
||||
use App\Rules\SafeExternalUrl;
|
||||
use Illuminate\Support\Facades\Validator;
|
||||
use Tests\TestCase;
|
||||
|
||||
uses(TestCase::class);
|
||||
|
||||
it('accepts valid public URLs', function () {
|
||||
$rule = new SafeExternalUrl;
|
||||
|
||||
$validUrls = [
|
||||
'https://api.github.com',
|
||||
'https://github.example.com/api/v3',
|
||||
'https://example.com',
|
||||
'http://example.com',
|
||||
];
|
||||
|
||||
foreach ($validUrls as $url) {
|
||||
$validator = Validator::make(['url' => $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');
|
||||
});
|
||||
Loading…
Reference in a new issue