From f622213988a384fc44a96e8c217201df3a29928f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:40:36 +0200 Subject: [PATCH] fix(git-source): follow same-origin redirects for Git provider APIs Git source HTTP clients previously disabled redirects entirely, so API calls for renamed repositories (e.g. GitHub 301 to /repositories/{id}) failed. Add SafeExternalUrl::sameOriginRedirectOptions() and use it in the GitSource HTTP macro to follow up to five strict redirects that stay on the same scheme, host and port. A redirect to a different origin gets a RequestException, because the DNS pin covers only the original host and port. This blocks SSRF via redirects to metadata or attacker hosts. Authorization headers and the original request method and body are kept on same-origin redirects. Tests cover following same-host redirects, keeping POST on a redirect, refusing cross-host/scheme/port redirects and the five-redirect cap. --- app/Providers/AppServiceProvider.php | 11 ++-- app/Rules/SafeWebhookUrl.php | 31 ++++++++++ tests/Feature/GitSourceHttpSafetyTest.php | 59 ++++++++++++++++++- tests/Feature/GitSourcePrivateNetworkTest.php | 2 +- 4 files changed, 95 insertions(+), 8 deletions(-) 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'); });