fix(github): reject malformed app URL origins

This commit is contained in:
Andras Bacsai 2026-07-07 12:24:52 +02:00
parent 1adeed2a48
commit 21bd8fa2bc
3 changed files with 43 additions and 8 deletions

View file

@ -38,22 +38,23 @@ function githubUrlHost(?string $url): ?string
/** /**
* Build the scheme://host[:port] origin for a GitHub URL. * Build the scheme://host[:port] origin for a GitHub URL.
* *
* When the host cannot be parsed (e.g. a scheme-less or malformed URL such as * This helper fails explicitly for blank, scheme-less, or malformed input when
* "not-a-url"), the input is returned verbatim with any trailing slashes * githubUrlHost() cannot parse a host, because returning the original input
* trimmed. Callers should pass already-validated URLs (see SafeExternalUrl), * would not be a valid origin. Callers should pass already-validated URLs.
* so this fallback only guards against unexpected input.
* *
* @param string $url The URL to derive the origin from * @param string $url The URL to derive the origin from
* @return string The normalized origin, or the trimmed input when the host is unparseable * @return string The normalized origin
*
* @throws InvalidArgumentException When the URL does not contain a parseable scheme and host
*/ */
function githubUrlOrigin(string $url): string function githubUrlOrigin(string $url): string
{ {
$scheme = parse_url($url, PHP_URL_SCHEME) ?: 'https'; $scheme = parse_url($url, PHP_URL_SCHEME);
$host = githubUrlHost($url); $host = githubUrlHost($url);
$port = parse_url($url, PHP_URL_PORT); $port = parse_url($url, PHP_URL_PORT);
if (! $host) { if (! is_string($scheme) || blank($scheme) || ! $host) {
return rtrim($url, '/'); throw new InvalidArgumentException('GitHub URL must include a valid scheme and host.');
} }
return $scheme.'://'.$host.($port ? ":{$port}" : ''); return $scheme.'://'.$host.($port ? ":{$port}" : '');

View file

@ -322,6 +322,32 @@ function validGithubAppsApiPrivateKey(): string
->assertJsonPath('data.api_url', 'https://github.com/api/v3'); ->assertJsonPath('data.api_url', 'https://github.com/api/v3');
}); });
test('preserves provided api url when updating api url only', function () {
$githubApp = GithubApp::create([
'name' => 'GHE App',
'api_url' => 'https://api.github.com',
'html_url' => 'https://github.com',
'app_id' => 12345,
'installation_id' => 67890,
'client_id' => 'test-client-id',
'client_secret' => 'test-client-secret',
'webhook_secret' => 'test-webhook-secret',
'private_key_id' => $this->privateKey->id,
'team_id' => $this->team->id,
'is_system_wide' => false,
'is_public' => false,
]);
$response = $this->withHeaders([
'Authorization' => 'Bearer '.$this->bearerToken,
])->patchJson("/api/v1/github-apps/{$githubApp->id}", [
'api_url' => 'https://github.com/api/v3',
]);
$response->assertSuccessful()
->assertJsonPath('data.api_url', 'https://github.com/api/v3');
});
test('rejects invalid organization when creating github apps', function () { test('rejects invalid organization when creating github apps', function () {
$response = $this->withHeaders([ $response = $this->withHeaders([
'Authorization' => 'Bearer '.$this->bearerToken, 'Authorization' => 'Bearer '.$this->bearerToken,

View file

@ -23,6 +23,14 @@
'github enterprise server' => ['https://github.company.internal', 'https://github.company.internal/api/v3'], 'github enterprise server' => ['https://github.company.internal', 'https://github.company.internal/api/v3'],
]); ]);
it('rejects malformed urls when deriving an origin', function (string $url) {
expect(fn () => githubUrlOrigin($url))->toThrow(InvalidArgumentException::class);
})->with([
'blank' => [''],
'scheme-less' => ['github.company.internal'],
'malformed' => ['not-a-url'],
]);
it('generates correct install paths for github cloud ghe cloud and ghes', function (array $attributes, string $expectedPrefix) { it('generates correct install paths for github cloud ghe cloud and ghes', function (array $attributes, string $expectedPrefix) {
$githubApp = new GithubApp; $githubApp = new GithubApp;
$githubApp->forceFill(array_merge([ $githubApp->forceFill(array_merge([