diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 1d41c94034..ff7eeb8c44 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -105,10 +105,13 @@ class AppServiceProvider extends ServiceProvider private function configureGitHubHttp(): void { Http::macro('GitSource', function (string $url) { - return Http::withOptions(SafeExternalUrl::httpClientOptions( - $url, - allowPrivateNetworks: SafeExternalUrl::gitSourcesMayUsePrivateNetworks(), - )); + return Http::withOptions([ + ...SafeExternalUrl::httpClientOptions( + $url, + allowPrivateNetworks: SafeExternalUrl::gitSourcesMayUsePrivateNetworks(), + ), + 'allow_redirects' => SafeExternalUrl::sameOriginRedirectOptions(), + ]); }); Http::macro('GitHub', function (string $api_url, ?string $github_access_token = null) { diff --git a/app/Rules/SafeWebhookUrl.php b/app/Rules/SafeWebhookUrl.php index 7d614d03de..25bc84449b 100644 --- a/app/Rules/SafeWebhookUrl.php +++ b/app/Rules/SafeWebhookUrl.php @@ -4,8 +4,13 @@ namespace App\Rules; use App\Models\InstanceSettings; use Closure; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Psr7\UriComparator; use Illuminate\Contracts\Validation\ValidationRule; use Illuminate\Support\Facades\Log; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\UriInterface; use PurplePixie\PhpDns\DNSQuery; use PurplePixie\PhpDns\DNSTypes; use Throwable; @@ -163,6 +168,32 @@ class SafeWebhookUrl implements ValidationRule return $options; } + /** + * Redirect options that follow up to five redirects on the same scheme, host and port. + * The DNS pin from httpClientOptions() covers only that host and port, so a redirect + * to another origin is refused. Git providers use such redirects for renamed repositories. + * + * @return array + */ + public static function sameOriginRedirectOptions(): array + { + return [ + 'max' => 5, + 'strict' => true, + 'referer' => false, + 'protocols' => ['http', 'https'], + 'on_redirect' => function (RequestInterface $request, ResponseInterface $response, UriInterface $target): void { + if (UriComparator::isCrossOrigin($request->getUri(), $target)) { + throw new RequestException( + "Refused a redirect to another host ({$target->getScheme()}://{$target->getAuthority()}).", + $request, + $response, + ); + } + }, + ]; + } + /** * Build mc --resolve mappings that pin the endpoint host for S3 backups. * diff --git a/tests/Feature/GitSourceHttpSafetyTest.php b/tests/Feature/GitSourceHttpSafetyTest.php index fd6c8af432..e71fb1618c 100644 --- a/tests/Feature/GitSourceHttpSafetyTest.php +++ b/tests/Feature/GitSourceHttpSafetyTest.php @@ -6,7 +6,10 @@ use App\Models\InstanceSettings; use App\Models\Team; use App\Models\User; use App\Rules\SafeExternalUrl; +use GuzzleHttp\Handler\MockHandler; +use GuzzleHttp\Psr7\Response; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; @@ -17,11 +20,11 @@ it('rejects unsafe Git source targets when a request is built', function () { ->toThrow(RuntimeException::class); }); -it('disables redirects and pins public GitHub requests without losing authorization', function () { +it('limits redirects and pins public GitHub requests without losing authorization', function () { $request = Http::GitHub('https://api.github.com', 'secret'); $options = $request->getOptions(); - expect($options['allow_redirects'])->toBeFalse() + expect($options['allow_redirects'])->toMatchArray(['max' => 5, 'strict' => true]) ->and($options['curl'][CURLOPT_RESOLVE][0])->toStartWith('api.github.com:443:') ->and($options['headers']['Authorization'])->toBe('Bearer secret'); }); @@ -30,11 +33,61 @@ it('uses the same guard and bearer header for GitLab requests', function () { $request = Http::GitLab('https://gitlab.com/api/v4', 'gitlab-secret'); $options = $request->getOptions(); - expect($options['allow_redirects'])->toBeFalse() + expect($options['allow_redirects'])->toMatchArray(['max' => 5, 'strict' => true]) ->and($options['curl'][CURLOPT_RESOLVE][0])->toStartWith('gitlab.com:443:') ->and($options['headers']['Authorization'])->toBe('Bearer gitlab-secret'); }); +it('follows a same-host redirect for a renamed GitHub repository', function () { + $transport = new MockHandler([ + new Response(301, ['Location' => 'https://api.github.com/repositories/2126244']), + new Response(200, ['Content-Type' => 'application/json'], json_encode(['full_name' => 'twbs/bootstrap'])), + ]); + + $response = Http::GitHub('https://api.github.com', 'secret')->setHandler($transport)->get('repos/twitter/bootstrap'); + + expect($response->status())->toBe(200) + ->and($response->json('full_name'))->toBe('twbs/bootstrap') + ->and((string) $transport->getLastRequest()->getUri())->toBe('https://api.github.com/repositories/2126244') + ->and($transport->getLastRequest()->getHeaderLine('Authorization'))->toBe('Bearer secret'); +}); + +it('keeps the request method when GitHub redirects a POST request', function () { + $transport = new MockHandler([ + new Response(301, ['Location' => 'https://api.github.com/repositories/2126244/dispatches']), + new Response(204), + ]); + + Http::GitHub('https://api.github.com', 'secret')->setHandler($transport)->post('repos/twitter/bootstrap/dispatches', ['event_type' => 'deploy']); + + expect((string) $transport->getLastRequest()->getUri())->toBe('https://api.github.com/repositories/2126244/dispatches') + ->and($transport->getLastRequest()->getMethod())->toBe('POST') + ->and((string) $transport->getLastRequest()->getBody())->toBe('{"event_type":"deploy"}'); +}); + +it('refuses a Git source redirect to another host, scheme or port', function (string $location) { + $transport = new MockHandler([ + new Response(301, ['Location' => $location]), + new Response(200), + ]); + + expect(fn () => Http::GitHub('https://api.github.com', 'secret')->setHandler($transport)->get('repos/twitter/bootstrap')) + ->toThrow(ConnectionException::class, 'another host') + ->and($transport->count())->toBe(1); +})->with([ + 'metadata address' => ['http://169.254.169.254/latest/meta-data'], + 'other host' => ['https://attacker.example/repositories/2126244'], + 'other scheme' => ['http://api.github.com/repositories/2126244'], + 'other port' => ['https://api.github.com:8443/repositories/2126244'], +]); + +it('stops after five Git source redirects', function () { + $transport = new MockHandler(array_fill(0, 7, new Response(301, ['Location' => 'https://api.github.com/loop']))); + + expect(fn () => Http::GitHub('https://api.github.com', 'secret')->setHandler($transport)->get('loop')) + ->toThrow(ConnectionException::class, 'Will not follow more than 5 redirects'); +}); + it('rejects a hostname that resolves to a private IP at request time', function () { expect(fn () => SafeExternalUrl::httpClientOptions( 'https://api.github.com', diff --git a/tests/Feature/GitSourcePrivateNetworkTest.php b/tests/Feature/GitSourcePrivateNetworkTest.php index cfaed850b2..ea39129f1f 100644 --- a/tests/Feature/GitSourcePrivateNetworkTest.php +++ b/tests/Feature/GitSourcePrivateNetworkTest.php @@ -71,7 +71,7 @@ test('other outbound URLs still block private networks by default', function () test('the Git source HTTP client allows a private GitHub Enterprise on self-hosted', function () { $options = Http::GitHub('https://10.20.30.40/api/v3', 'secret')->getOptions(); - expect($options['allow_redirects'])->toBeFalse() + expect($options['allow_redirects'])->toMatchArray(['max' => 5, 'strict' => true]) ->and($options['headers']['Authorization'])->toBe('Bearer secret'); });