starred/coolify
1
0
Fork 0
mirror of https://github.com/coollabsio/coolify.git synced 2026-11-06 13:10:37 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(github-runner): skip pull request jobs early and keep runner group restrictions

Browse Source 
- Do not provision a runner for a queued pull request job when no matching
  runner configuration allows pull request jobs (the run event is read from
  GitHub; the job-started hook still refuses it if the lookup fails).
- Do not open an existing runner group to all or public repositories, so
  restrictions an organization admin set on GitHub stay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai
2026-10-02 16:01:00 +02:00
co-authored by Claude Opus 5.5
parent 5cb68cda16
commit 55bbe92d94
5 changed files with 110 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Http/Controllers/Webhook/Github.php
+31 -3
View File
@@ -17,6 +17,8 @@ use App\Models\GithubApp;
use App\Models\GithubRunnerConfig;
use App\Models\GithubRunnerExecution;
use App\Models\PrivateKey;
use App\Services\GithubRunner\GithubRunnerApi;
use App\Services\GithubRunner\GithubRunnerContainer;
use Exception;
use Illuminate\Http\Exceptions\HttpResponseException;
use Illuminate\Http\RedirectResponse;
@@ -281,13 +283,16 @@ class Github extends Controller
if ($action === 'queued') {
$labels = array_values(array_filter((array) data_get($payload, 'workflow_job.labels', []), 'is_string'));
$matches = $githubApp->runnerConfigs()
$matchingConfigs = $githubApp->runnerConfigs()
->where('is_enabled', true)
->get()
->contains(fn (GithubRunnerConfig $config) => $config->matchesLabels($labels));
if (! $matches) {
->filter(fn (GithubRunnerConfig $config) => $config->matchesLabels($labels));
if ($matchingConfigs->isEmpty()) {
return response('Nothing to do. No runner configuration matches the job labels.');
}
if (! $matchingConfigs->contains('allow_pull_requests', true) && $this->isPullRequestWorkflowJob($githubApp, $payload)) {
return response('Nothing to do. The runner configuration does not allow pull request jobs.');
}
$execution = GithubRunnerExecution::createOrFirst(
['github_app_id' => $githubApp->id, 'trigger_workflow_job_id' => $jobId],
@@ -335,6 +340,29 @@ class Github extends Controller
return response('Nothing to do.');
}
/**
* Whether a queued job belongs to a workflow run started by a pull request, so no runner is started
* for a job that the job-started hook would refuse. The `workflow_job` payload has no event, so the
* run is read from GitHub. When that fails, the job counts as no pull request job and the hook still
* refuses it in the runner.
*/
private function isPullRequestWorkflowJob(GithubApp $githubApp, Collection $payload): bool
{
$runId = (int) data_get($payload, 'workflow_job.run_id', 0);
$repository = (string) data_get($payload, 'repository.full_name', '');
if ($runId <= 0 || ! str_contains($repository, '/')) {
return false;
}
try {
$event = (new GithubRunnerApi($githubApp))->workflowRunEvent($repository, $runId);
} catch (\Throwable) {
return false;
}
return in_array($event, GithubRunnerContainer::PULL_REQUEST_EVENTS, true);
}
public function normal(Request $request)
{
try {
app/Services/GithubRunner/GithubRunnerApi.php
+19 -18
View File
@@ -16,9 +16,9 @@ class GithubRunnerApi
public function __construct(private GithubApp $githubApp) {}
/**
* Finds or creates the runner group of the App. Every repository of the organization, public ones too,
* can use it; runners refuse pull request jobs unless the runner config allows them.
* Groups created before public repositories were allowed are updated.
* Finds or creates the runner group of the App. A new group lets every repository of the organization,
* public ones too, use it; runners refuse pull request jobs unless the runner config allows them.
* Coolify does not change an existing group, so restrictions an organization admin set on GitHub stay.
* All GitHub plans can create runner groups, so a failed creation is an error. Coolify never falls back
* to the Default group, because that group can give the runners access to more repositories.
*
@@ -31,10 +31,6 @@ class GithubRunnerApi
if ($this->githubApp->runner_group_id) {
$response = $this->client()->get("/orgs/{$org}/actions/runner-groups/{$this->githubApp->runner_group_id}");
if ($response->successful()) {
if (! $response->json('default') && ($response->json('visibility') !== 'all' || $response->json('allows_public_repositories') !== true)) {
$this->allowAllRepositories((int) $this->githubApp->runner_group_id);
}
return ['id' => (int) $this->githubApp->runner_group_id, 'is_default' => (bool) $response->json('default')];
}
if ($response->status() !== 404) {
@@ -57,17 +53,6 @@ class GithubRunnerApi
return $group;
}
private function allowAllRepositories(int $runnerGroupId): void
{
$response = $this->client()->patch("/orgs/{$this->organization()}/actions/runner-groups/{$runnerGroupId}", [
'visibility' => 'all',
'allows_public_repositories' => true,
]);
if (! $response->successful()) {
throw new RuntimeException('Could not update the runner group: '.$this->errorMessage($response));
}
}
/**
* @param array<int, string> $labels
* @return array{runner_id: int, encoded_jit_config: string}
@@ -91,6 +76,22 @@ class GithubRunnerApi
];
}
/**
* The event that started a workflow run, for example "push" or "pull_request". The `workflow_job`
* webhook does not contain it. A short timeout keeps the webhook response fast.
*/
public function workflowRunEvent(string $repositoryFullName, int $runId): string
{
$repository = implode('/', array_map('rawurlencode', explode('/', $repositoryFullName, 2)));
$response = $this->client()->timeout(5)->get("/repos/{$repository}/actions/runs/{$runId}");
if (! $response->successful() || ! is_string($response->json('event'))) {
throw new RuntimeException('Could not read the workflow run: '.$this->errorMessage($response));
}
return $response->json('event');
}
/**
* Removes a runner from GitHub. GitHub removes JIT runners itself after their job, so 404 is not an error.
*/
app/Services/GithubRunner/GithubRunnerContainer.php
+8 -3
View File
@@ -30,6 +30,9 @@ class GithubRunnerContainer
private const HOOK_PATH = '/home/runner/coolify-job-started.sh';
/** Workflow run events that a runner refuses unless the config allows pull request jobs. */
public const PULL_REQUEST_EVENTS = ['pull_request', 'pull_request_target', 'pull_request_review', 'pull_request_review_comment'];
public function __construct(private GithubRunnerExecution $execution, private GithubRunnerConfig $config) {}
public static function nameFor(string $executionUuid): string
@@ -153,10 +156,12 @@ class GithubRunnerContainer
*/
public static function pullRequestHookScript(): string
{
return <<<'BASH'
$events = implode('|', self::PULL_REQUEST_EVENTS);
return <<<BASH
#!/bin/bash
case "${GITHUB_EVENT_NAME:-}" in
pull_request|pull_request_target|pull_request_review|pull_request_review_comment|"")
case "\${GITHUB_EVENT_NAME:-}" in
{$events}|"")
echo "::error::This Coolify runner does not run pull request jobs. Allow pull request jobs in the GitHub runner settings of the build server to run them."
exit 1
;;
tests/Feature/GithubRunnerFlowTest.php
+46
View File
@@ -134,6 +134,15 @@ function fakeRunnerGithubApi(GithubApp $githubApp): void
]);
}
function fakeWorkflowRunApi(GithubApp $githubApp, mixed $runResponse): void
{
Http::fake([
'https://api.github.com/zen' => Http::response('ok', 200, ['Date' => now()->toRfc7231String()]),
"https://api.github.com/app/installations/{$githubApp->installation_id}/access_tokens" => Http::response(['token' => 'installation-token'], 201),
'https://api.github.com/repos/acme/api/actions/runs/321' => $runResponse,
]);
}
describe('workflow_job webhook', function () {
beforeEach(function () {
Queue::fake();
@@ -165,6 +174,43 @@ describe('workflow_job webhook', function () {
'github hosted' => [['ubuntu-latest']],
]);
it('does not provision a runner for a pull request job when no matching configuration allows them', function (string $event) {
Http::preventStrayRequests();
fakeWorkflowRunApi($this->githubApp, Http::response(['id' => 321, 'event' => $event]));
sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'queued', ['run_id' => 321])
->assertOk()
->assertSee('does not allow pull request jobs');
expect(GithubRunnerExecution::count())->toBe(0);
Queue::assertNotPushed(ProvisionGithubRunnerJob::class);
})->with(['pull_request', 'pull_request_target']);
it('provisions a runner for a job that does not come from a pull request', function (mixed $runResponse) {
Http::preventStrayRequests();
fakeWorkflowRunApi($this->githubApp, $runResponse);
sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'queued', ['run_id' => 321])->assertOk();
expect(GithubRunnerExecution::sole()->status)->toBe(GithubRunnerStatus::Queued);
Queue::assertPushed(ProvisionGithubRunnerJob::class, 1);
})->with([
'push' => fn () => Http::response(['id' => 321, 'event' => 'push']),
'workflow run lookup fails' => fn () => Http::response(['message' => 'Server Error'], 500),
]);
it('provisions a runner for a pull request job without a lookup when a matching configuration allows them', function () {
Http::preventStrayRequests();
Http::fake();
GithubRunnerConfig::query()->update(['allow_pull_requests' => true]);
sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'queued', ['run_id' => 321])->assertOk();
expect(GithubRunnerExecution::count())->toBe(1);
Queue::assertPushed(ProvisionGithubRunnerJob::class, 1);
Http::assertNothingSent();
});
it('rejects an invalid signature', function () {
sendWorkflowJobWebhook($this, $this->githubApp, 'wrong-secret', 'queued', [])->assertSee('Invalid signature');
tests/Feature/GithubRunnerSettingsTest.php
+6 -8
View File
@@ -133,14 +133,12 @@ describe('runner settings page', function () {
expect($config->allow_pull_requests)->toBeFalse();
});
it('opens an existing private runner group to public repositories', function () {
it('keeps the restrictions an organization admin set on an existing runner group', function () {
$this->githubApp->update(['runner_group_id' => 77]);
Http::fake([
'https://api.github.com/zen' => Http::response('ok', 200, ['Date' => now()->toRfc7231String()]),
"https://api.github.com/app/installations/{$this->githubApp->installation_id}/access_tokens" => Http::response(['token' => 'installation-token'], 201),
'https://api.github.com/orgs/acme/actions/runner-groups/77' => fn ($request) => $request->method() === 'PATCH'
? Http::response(['id' => 77, 'default' => false, 'visibility' => 'all', 'allows_public_repositories' => true])
: Http::response(['id' => 77, 'default' => false, 'visibility' => 'private', 'allows_public_repositories' => false]),
'https://api.github.com/orgs/acme/actions/runner-groups/77' => Http::response(['id' => 77, 'default' => false, 'visibility' => 'selected', 'allows_public_repositories' => false]),
]);
Livewire::actingAs($this->owner)
@@ -150,10 +148,10 @@ describe('runner settings page', function () {
->assertHasNoErrors()
->assertDispatched('success');
Http::assertSent(fn ($request) => $request->method() === 'PATCH'
&& str_ends_with($request->url(), '/orgs/acme/actions/runner-groups/77')
&& $request['allows_public_repositories'] === true
&& $request['visibility'] === 'all');
expect($this->githubApp->fresh()->runner_group_id)->toBe(77)
->and(GithubRunnerConfig::count())->toBe(1);
Http::assertNotSent(fn ($request) => in_array($request->method(), ['PATCH', 'POST'], true)
&& str_contains($request->url(), '/runner-groups'));
});
it('saves whether runners take pull request jobs', function () {