diff --git a/app/Http/Controllers/Webhook/Github.php b/app/Http/Controllers/Webhook/Github.php index c4fdc5fd5c..70c5eca19b 100644 --- a/app/Http/Controllers/Webhook/Github.php +++ b/app/Http/Controllers/Webhook/Github.php @@ -520,7 +520,7 @@ class Github extends Controller abort_if($this->githubAppHasManifestCredentials($github_app), 403, 'GitHub App credentials are already configured.'); $api_url = data_get($github_app, 'api_url'); - $data = Http::withBody(null) + $data = Http::GitSource($api_url)->withBody(null) ->accept('application/vnd.github+json') ->timeout(10) ->connectTimeout(5) @@ -612,7 +612,7 @@ class Github extends Controller try { $jwt = generateGithubJwt($github_app); - $response = Http::withHeaders([ + $response = Http::GitSource($github_app->api_url)->withHeaders([ 'Authorization' => "Bearer $jwt", 'Accept' => 'application/vnd.github+json', ]) diff --git a/app/Http/Controllers/Webhook/Gitlab.php b/app/Http/Controllers/Webhook/Gitlab.php index eb24b460f7..e86e51ab14 100644 --- a/app/Http/Controllers/Webhook/Gitlab.php +++ b/app/Http/Controllers/Webhook/Gitlab.php @@ -50,7 +50,7 @@ class Gitlab extends Controller $baseUrl = rtrim($gitlabApp->html_url, '/'); - $response = Http::asForm()->post("{$baseUrl}/oauth/token", [ + $response = Http::GitSource($baseUrl)->asForm()->post("{$baseUrl}/oauth/token", [ 'client_id' => $gitlabApp->client_id, 'client_secret' => $gitlabApp->client_secret, 'code' => $code, diff --git a/app/Jobs/GithubAppPermissionJob.php b/app/Jobs/GithubAppPermissionJob.php index 7cd1b86ac0..319a202cf1 100644 --- a/app/Jobs/GithubAppPermissionJob.php +++ b/app/Jobs/GithubAppPermissionJob.php @@ -29,7 +29,7 @@ class GithubAppPermissionJob implements ShouldBeEncrypted, ShouldQueue try { $github_access_token = generateGithubJwt($this->github_app); - $response = Http::withHeaders([ + $response = Http::GitSource($this->github_app->api_url)->withHeaders([ 'Authorization' => "Bearer $github_access_token", 'Accept' => 'application/vnd.github+json', ])->get("{$this->github_app->api_url}/app"); diff --git a/app/Livewire/Source/Github/Change.php b/app/Livewire/Source/Github/Change.php index 2dadd73661..6bb896fcc6 100644 --- a/app/Livewire/Source/Github/Change.php +++ b/app/Livewire/Source/Github/Change.php @@ -263,7 +263,7 @@ class Change extends Component } $jwt = generateGithubJwt($this->github_app); - $appResponse = Http::withHeaders([ + $appResponse = Http::GitSource($this->github_app->api_url)->withHeaders([ 'Authorization' => "Bearer $jwt", 'Accept' => 'application/vnd.github+json', ])->timeout(10)->get("{$this->github_app->api_url}/app"); diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index e4d2b0a851..f09700562f 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -6,6 +6,7 @@ use App\Auth\Oidc\OidcDiscoveryService; use App\Auth\Oidc\OidcTokenValidator; use App\Auth\Oidc\Socialite\OidcProvider; use App\Models\PersonalAccessToken; +use App\Rules\SafeExternalUrl; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\DB; @@ -30,6 +31,7 @@ class AppServiceProvider extends ServiceProvider $this->configurePasswords(); $this->configureSanctumModel(); $this->configureGitHubHttp(); + $this->configureGitLabHttp(); $this->configureOidcSocialite(); } @@ -85,18 +87,33 @@ class AppServiceProvider extends ServiceProvider private function configureGitHubHttp(): void { + Http::macro('GitSource', function (string $url) { + return Http::withOptions(SafeExternalUrl::httpClientOptions($url)); + }); + Http::macro('GitHub', function (string $api_url, ?string $github_access_token = null) { if ($github_access_token) { - return Http::withHeaders([ + return Http::GitSource($api_url)->withHeaders([ 'X-GitHub-Api-Version' => '2022-11-28', 'Accept' => 'application/vnd.github.v3+json', 'Authorization' => "Bearer $github_access_token", ])->baseUrl($api_url); } else { - return Http::withHeaders([ + return Http::GitSource($api_url)->withHeaders([ 'Accept' => 'application/vnd.github.v3+json', ])->baseUrl($api_url); } }); } + + private function configureGitLabHttp(): void + { + Http::macro('GitLab', function (string $api_url, ?string $access_token = null) { + $client = Http::GitSource($api_url)->withHeaders([ + 'Accept' => 'application/json', + ])->baseUrl($api_url); + + return $access_token ? $client->withToken($access_token) : $client; + }); + } } diff --git a/app/Rules/SafeWebhookUrl.php b/app/Rules/SafeWebhookUrl.php index 5a5911bee4..28e2531265 100644 --- a/app/Rules/SafeWebhookUrl.php +++ b/app/Rules/SafeWebhookUrl.php @@ -109,9 +109,10 @@ class SafeWebhookUrl implements ValidationRule /** * Build HTTP client options that pin the validated host to the resolved IPs. * + * @param (Closure(string): array)|null $resolver * @return array */ - public static function httpClientOptions(string $url, array $trustedInternalHosts = []): array + public static function httpClientOptions(string $url, array $trustedInternalHosts = [], ?Closure $resolver = null): array { $options = ['allow_redirects' => false]; @@ -119,7 +120,7 @@ class SafeWebhookUrl implements ValidationRule throw new \RuntimeException('Webhook URL DNS pinning is unavailable.'); } - $target = self::resolveUrlForRequest($url, $trustedInternalHosts); + $target = self::resolveUrlForRequest($url, $trustedInternalHosts, $resolver); if ($target['ips'] === [] || filter_var($target['host'], FILTER_VALIDATE_IP)) { return $options; @@ -186,9 +187,13 @@ class SafeWebhookUrl implements ValidationRule /** * @return array{host: string, port: int, ips: array} */ - private static function resolveUrlForRequest(string $url, array $trustedInternalHosts = []): array + private static function resolveUrlForRequest(string $url, array $trustedInternalHosts = [], ?Closure $resolver = null): array { - $rule = new self(trustedInternalHosts: $trustedInternalHosts); + $rule = new self(resolver: $resolver, trustedInternalHosts: $trustedInternalHosts); + if (! filter_var($url, FILTER_VALIDATE_URL)) { + throw new \RuntimeException('Webhook URL is invalid.'); + } + $host = parse_url($url, PHP_URL_HOST); if (! is_string($host) || $host === '') { throw new \RuntimeException('Webhook URL host could not be resolved.'); @@ -199,6 +204,10 @@ class SafeWebhookUrl implements ValidationRule } $scheme = strtolower(parse_url($url, PHP_URL_SCHEME) ?? ''); + if (! in_array($scheme, ['http', 'https'], true)) { + throw new \RuntimeException('Webhook URL scheme is unsafe.'); + } + $port = parse_url($url, PHP_URL_PORT) ?: ($scheme === 'https' ? 443 : 80); $hostForDns = rtrim($rule->normalizeHostForIpCheck(strtolower($host)), '.'); @@ -221,6 +230,10 @@ class SafeWebhookUrl implements ValidationRule } } + if ($rule->isBlockedHostname($hostForDns) && ! $rule->isAllowedHostname($hostForDns)) { + throw new \RuntimeException('Webhook URL host is unsafe.'); + } + return ['host' => $hostForDns, 'port' => $port, 'ips' => $resolvedIps]; } diff --git a/bootstrap/helpers/github.php b/bootstrap/helpers/github.php index 66fe0c2338..b5936f2312 100644 --- a/bootstrap/helpers/github.php +++ b/bootstrap/helpers/github.php @@ -150,7 +150,7 @@ function encodeGithubPathSegment(string $segment): string function assertGithubClockInSync(string $apiUrl): void { - $response = Http::get("{$apiUrl}/zen"); + $response = Http::GitSource($apiUrl)->get("{$apiUrl}/zen"); $serverTime = CarbonImmutable::now()->setTimezone('UTC'); $githubTime = Carbon::parse($response->header('date')); $timeDiff = abs($serverTime->diffInSeconds($githubTime)); @@ -186,7 +186,7 @@ function generateGithubToken(GithubApp $source, string $type) return match ($type) { 'jwt' => $jwt, 'installation' => (function () use ($source, $jwt) { - $response = Http::withHeaders([ + $response = Http::GitSource($source->api_url)->withHeaders([ 'Authorization' => "Bearer $jwt", 'Accept' => 'application/vnd.github.machine-man-preview+json', ])->post("{$source->api_url}/app/installations/{$source->installation_id}/access_tokens"); @@ -289,7 +289,7 @@ function syncGithubAppName(GithubApp $source, bool $throw = false): ?string $jwt = generateGithubAppJwt($privateKey->private_key, $source->app_id); - $response = Http::withHeaders([ + $response = Http::GitSource($source->api_url)->withHeaders([ 'Accept' => 'application/vnd.github+json', 'X-GitHub-Api-Version' => '2022-11-28', 'Authorization' => "Bearer {$jwt}", diff --git a/bootstrap/helpers/gitlab.php b/bootstrap/helpers/gitlab.php index 743e3897b9..0ee44f2157 100644 --- a/bootstrap/helpers/gitlab.php +++ b/bootstrap/helpers/gitlab.php @@ -32,7 +32,7 @@ function refreshGitlabToken(GitlabApp $source): void $baseUrl = rtrim($source->html_url, '/'); - $response = Http::asForm()->post("{$baseUrl}/oauth/token", [ + $response = Http::GitSource($baseUrl)->asForm()->post("{$baseUrl}/oauth/token", [ 'client_id' => $source->client_id, 'client_secret' => $source->client_secret, 'refresh_token' => $source->refresh_token, diff --git a/tests/Feature/GitSourceHttpSafetyTest.php b/tests/Feature/GitSourceHttpSafetyTest.php new file mode 100644 index 0000000000..fd6c8af432 --- /dev/null +++ b/tests/Feature/GitSourceHttpSafetyTest.php @@ -0,0 +1,104 @@ + Http::GitHub('http://169.254.169.254', 'secret')) + ->toThrow(RuntimeException::class); +}); + +it('disables 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() + ->and($options['curl'][CURLOPT_RESOLVE][0])->toStartWith('api.github.com:443:') + ->and($options['headers']['Authorization'])->toBe('Bearer secret'); +}); + +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() + ->and($options['curl'][CURLOPT_RESOLVE][0])->toStartWith('gitlab.com:443:') + ->and($options['headers']['Authorization'])->toBe('Bearer gitlab-secret'); +}); + +it('rejects a hostname that resolves to a private IP at request time', function () { + expect(fn () => SafeExternalUrl::httpClientOptions( + 'https://api.github.com', + resolver: fn (string $host): array => ['169.254.169.254'], + ))->toThrow(RuntimeException::class, 'unsafe IP address'); +}); + +it('requires source administration rather than ordinary team membership to configure a source', function () { + $team = Team::factory()->create(); + $owner = User::factory()->create(); + $member = User::factory()->create(); + $team->members()->attach($owner->id, ['role' => 'owner']); + $team->members()->attach($member->id, ['role' => 'member']); + + $this->actingAs($member); + session(['currentTeam' => $team]); + expect($member->can('create', GithubApp::class))->toBeFalse(); + + $this->actingAs($owner); + session(['currentTeam' => $team]); + expect($owner->can('create', GithubApp::class))->toBeTrue(); +}); + +it('does not send a manifest conversion request to an unsafe saved source URL', function () { + InstanceSettings::unguarded(fn () => InstanceSettings::query()->create(['id' => 0])); + $team = Team::factory()->create(); + $user = User::factory()->create(); + $team->members()->attach($user->id, ['role' => 'owner']); + $app = GithubApp::create([ + 'name' => 'Unsafe source', + 'api_url' => 'http://169.254.169.254', + 'html_url' => 'https://github.com', + 'team_id' => $team->id, + ]); + Cache::put('github-app-setup-state:'.hash('sha256', 'safe-test-state'), [ + 'action' => 'manifest', + 'github_app_id' => $app->id, + 'team_id' => $team->id, + ], now()->addMinutes(10)); + Http::preventStrayRequests(); + Http::fake(['*' => Http::response([], 200)]); + + $this->actingAs($user); + session(['currentTeam' => $team]); + $this->get('/webhooks/source/github/redirect?code=test-code&state=safe-test-state'); + + Http::assertNothingSent(); +}); + +it('does not send a GitLab token refresh request to an unsafe saved source URL', function () { + $team = Team::factory()->create(); + $source = GitlabApp::create([ + 'name' => 'Unsafe GitLab source', + 'api_url' => 'https://gitlab.com/api/v4', + 'html_url' => 'http://169.254.169.254', + 'client_id' => 'client-id', + 'client_secret' => 'client-secret', + 'refresh_token' => 'refresh-token', + 'expires_at' => 0, + 'team_id' => $team->id, + ]); + Http::preventStrayRequests(); + Http::fake(['*' => Http::response(['access_token' => 'unsafe-token'], 200)]); + + expect(fn () => refreshGitlabToken($source))->toThrow(RuntimeException::class); + Http::assertNothingSent(); +}); diff --git a/tests/Feature/GitlabOAuthCallbackStateTest.php b/tests/Feature/GitlabOAuthCallbackStateTest.php index c9d693dfe2..b858ffd26f 100644 --- a/tests/Feature/GitlabOAuthCallbackStateTest.php +++ b/tests/Feature/GitlabOAuthCallbackStateTest.php @@ -4,6 +4,7 @@ use App\Livewire\Source\Gitlab\Change as GitlabSource; use App\Models\GitlabApp; use App\Models\Team; use App\Models\User; +use App\Rules\SafeExternalUrl; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; @@ -11,6 +12,11 @@ use Illuminate\Support\Facades\Http; uses(RefreshDatabase::class); beforeEach(function () { + Http::macro('GitSource', fn (string $url) => Http::withOptions(SafeExternalUrl::httpClientOptions( + $url, + resolver: fn (string $host): array => ['93.184.216.34'], + ))); + $this->team = Team::factory()->create(); $this->user = User::factory()->create(); $this->team->members()->attach($this->user->id, ['role' => 'owner']); diff --git a/tests/Feature/Security/GithubAppSetupCallbackTest.php b/tests/Feature/Security/GithubAppSetupCallbackTest.php index 9e3f8ea81a..69671f2c24 100644 --- a/tests/Feature/Security/GithubAppSetupCallbackTest.php +++ b/tests/Feature/Security/GithubAppSetupCallbackTest.php @@ -5,6 +5,7 @@ use App\Models\InstanceSettings; use App\Models\PrivateKey; use App\Models\Team; use App\Models\User; +use App\Rules\SafeExternalUrl; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; @@ -12,6 +13,11 @@ use Illuminate\Support\Facades\Http; uses(RefreshDatabase::class); beforeEach(function () { + Http::macro('GitSource', fn (string $url) => Http::withOptions(SafeExternalUrl::httpClientOptions( + $url, + resolver: fn (string $host): array => ['93.184.216.34'], + ))); + InstanceSettings::unguarded(fn () => InstanceSettings::query()->create(['id' => 0])); $this->team = Team::factory()->create();