From 0ed423aaacf7d4eb4be139f2120e97c8f03b348f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:31:00 +0200 Subject: [PATCH] feat(webhooks): require 16 characters for new manual webhook secrets New or changed manual webhook secrets (GitHub, GitLab, Bitbucket, Gitea) must have at least 16 characters, in the Webhooks page and in the application API. Stored secrets keep working: an unchanged value passes, and an empty value still disables the webhook. Each secret field gets a Generate button that fills a random 40-character secret; the user saves it after adding it to the Git provider. --- .../Api/ApplicationsController.php | 2 +- app/Livewire/Project/Shared/Webhooks.php | 68 ++++++ app/Rules/ManualWebhookSecret.php | 45 ++++ bootstrap/helpers/api.php | 18 +- .../project/shared/webhooks.blade.php | 19 +- .../ManualWebhookSecretStrengthTest.php | 231 ++++++++++++++++++ 6 files changed, 373 insertions(+), 10 deletions(-) create mode 100644 app/Rules/ManualWebhookSecret.php create mode 100644 tests/Feature/Webhook/ManualWebhookSecretStrengthTest.php diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index ffa7c2aff9..1b3ba70694 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -3245,7 +3245,7 @@ class ApplicationsController extends Controller 'include_source_commit_in_build' => 'boolean', 'ports_exposes' => 'nullable|string|regex:/^(\d+)(,\d+)*$/', ]; - $validationRules = array_merge(sharedDataApplications(), $validationRules); + $validationRules = array_merge(sharedDataApplications($application), $validationRules); $validationMessages = [ 'docker_compose_domains.*.array' => 'An item in the docker_compose_domains array has invalid fields. Only name, domain, and redirect fields are supported.', ]; diff --git a/app/Livewire/Project/Shared/Webhooks.php b/app/Livewire/Project/Shared/Webhooks.php index b1b54cf491..172d91fc5e 100644 --- a/app/Livewire/Project/Shared/Webhooks.php +++ b/app/Livewire/Project/Shared/Webhooks.php @@ -2,7 +2,10 @@ namespace App\Livewire\Project\Shared; +use App\Rules\ManualWebhookSecret; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; +use Illuminate\Support\Str; +use Illuminate\Validation\ValidationException; use Livewire\Component; // Refactored ✅ @@ -30,6 +33,18 @@ class Webhooks extends Component public ?string $giteaManualWebhookSecret = null; + /** + * Maps each manual webhook provider to its component property and model attribute. + * + * @var array + */ + private const SECRET_FIELDS = [ + 'github' => ['property' => 'githubManualWebhookSecret', 'attribute' => 'manual_webhook_secret_github', 'label' => 'GitHub webhook secret'], + 'gitlab' => ['property' => 'gitlabManualWebhookSecret', 'attribute' => 'manual_webhook_secret_gitlab', 'label' => 'GitLab webhook secret'], + 'bitbucket' => ['property' => 'bitbucketManualWebhookSecret', 'attribute' => 'manual_webhook_secret_bitbucket', 'label' => 'Bitbucket webhook secret'], + 'gitea' => ['property' => 'giteaManualWebhookSecret', 'attribute' => 'manual_webhook_secret_gitea', 'label' => 'Gitea webhook secret'], + ]; + public function mount() { $this->deploywebhook = generateDeployWebhook($this->resource); @@ -56,6 +71,7 @@ class Webhooks extends Component { try { $this->authorize('update', $this->resource); + $this->validate($this->changedSecretRules(), [], $this->secretAttributeNames()); $this->resource->update([ 'manual_webhook_secret_github' => $this->githubManualWebhookSecret, 'manual_webhook_secret_gitlab' => $this->gitlabManualWebhookSecret, @@ -63,8 +79,60 @@ class Webhooks extends Component 'manual_webhook_secret_gitea' => $this->giteaManualWebhookSecret, ]); $this->dispatch('success', 'Webhook secrets saved.'); + } catch (ValidationException $e) { + throw $e; } catch (\Exception $e) { return handleError($e, $this); } } + + /** + * Return a random secret for the provider's field. The browser fills the field + * as an unsaved change, and the user saves it with the form. + */ + public function generateSecret(string $provider): ?string + { + try { + $this->authorize('update', $this->resource); + + if (! array_key_exists($provider, self::SECRET_FIELDS)) { + throw new \InvalidArgumentException('Unknown webhook provider.'); + } + + return Str::random(40); + } catch (\Exception $e) { + handleError($e, $this); + + return null; + } + } + + /** + * Rules only for secrets that differ from the stored value, so existing short + * secrets keep saving until they are changed. + * + * @return array> + */ + private function changedSecretRules(): array + { + $rules = []; + foreach (self::SECRET_FIELDS as $field) { + $currentValue = data_get($this->resource, $field['attribute']); + if ($this->{$field['property']} !== $currentValue) { + $rules[$field['property']] = ['nullable', 'string', new ManualWebhookSecret($currentValue)]; + } + } + + return $rules; + } + + /** + * @return array + */ + private function secretAttributeNames(): array + { + return collect(self::SECRET_FIELDS) + ->mapWithKeys(fn (array $field) => [$field['property'] => $field['label']]) + ->all(); + } } diff --git a/app/Rules/ManualWebhookSecret.php b/app/Rules/ManualWebhookSecret.php new file mode 100644 index 0000000000..da4224db54 --- /dev/null +++ b/app/Rules/ManualWebhookSecret.php @@ -0,0 +1,45 @@ +currentValue !== null && hash_equals($this->currentValue, $value)) { + return; + } + + if (mb_strlen($value) < self::MIN_LENGTH) { + $fail('The :attribute must be at least '.self::MIN_LENGTH.' characters.'); + } + } +} diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index f616dd65fe..2c33ba12ab 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -4,10 +4,12 @@ use App\Actions\Shared\MigrateResourceToDestination; use App\Enums\BuildPackTypes; use App\Enums\RedirectTypes; use App\Enums\StaticImageTypes; +use App\Models\Application; use App\Models\ApplicationSetting; use App\Models\Environment; use App\Models\StandaloneDocker; use App\Models\SwarmDocker; +use App\Rules\ManualWebhookSecret; use App\Rules\ValidGitBranch; use App\Support\ValidationPatterns; use Illuminate\Database\Eloquent\Collection; @@ -111,7 +113,13 @@ function exposeSensitiveFields(Model $model): Model return $model; } -function sharedDataApplications() +/** + * Shared validation rules for application create and update API endpoints. + * + * Pass the existing application on update so unchanged manual webhook secrets + * are accepted even when they predate the minimum length. + */ +function sharedDataApplications(?Application $application = null): array { return [ 'git_repository' => 'string', @@ -192,10 +200,10 @@ function sharedDataApplications() 'post_deployment_command_container' => ValidationPatterns::containerNameRules(), 'pre_deployment_command' => 'string|nullable', 'pre_deployment_command_container' => ValidationPatterns::containerNameRules(), - 'manual_webhook_secret_github' => 'string|nullable', - 'manual_webhook_secret_gitlab' => 'string|nullable', - 'manual_webhook_secret_bitbucket' => 'string|nullable', - 'manual_webhook_secret_gitea' => 'string|nullable', + 'manual_webhook_secret_github' => ['nullable', 'string', new ManualWebhookSecret($application?->manual_webhook_secret_github)], + 'manual_webhook_secret_gitlab' => ['nullable', 'string', new ManualWebhookSecret($application?->manual_webhook_secret_gitlab)], + 'manual_webhook_secret_bitbucket' => ['nullable', 'string', new ManualWebhookSecret($application?->manual_webhook_secret_bitbucket)], + 'manual_webhook_secret_gitea' => ['nullable', 'string', new ManualWebhookSecret($application?->manual_webhook_secret_gitea)], 'dockerfile_location' => ValidationPatterns::filePathRules(), 'dockerfile_target_build' => ValidationPatterns::dockerTargetRules(), 'docker_compose_location' => ValidationPatterns::filePathRules(), diff --git a/resources/views/livewire/project/shared/webhooks.blade.php b/resources/views/livewire/project/shared/webhooks.blade.php index 6c87e3098d..3e332bf440 100644 --- a/resources/views/livewire/project/shared/webhooks.blade.php +++ b/resources/views/livewire/project/shared/webhooks.blade.php @@ -5,24 +5,28 @@ [ 'name' => 'GitHub', 'url' => $githubManualWebhook, + 'key' => 'github', 'secret' => 'githubManualWebhookSecret', 'description' => 'Accepts JSON or form-urlencoded webhook payloads.', ], [ 'name' => 'GitLab', 'url' => $gitlabManualWebhook, + 'key' => 'gitlab', 'secret' => 'gitlabManualWebhookSecret', 'description' => 'Use the same secret when configuring the webhook in GitLab.', ], [ 'name' => 'Bitbucket', 'url' => $bitbucketManualWebhook, + 'key' => 'bitbucket', 'secret' => 'bitbucketManualWebhookSecret', 'description' => 'Use the same secret when configuring the webhook in Bitbucket.', ], [ 'name' => 'Gitea', 'url' => $giteaManualWebhook, + 'key' => 'gitea', 'secret' => 'giteaManualWebhookSecret', 'description' => 'Use the same secret when configuring the webhook in Gitea.', ], @@ -72,10 +76,17 @@
@can('update', $resource) - +
+ + + Generate + +
@else diff --git a/tests/Feature/Webhook/ManualWebhookSecretStrengthTest.php b/tests/Feature/Webhook/ManualWebhookSecretStrengthTest.php new file mode 100644 index 0000000000..508b90d558 --- /dev/null +++ b/tests/Feature/Webhook/ManualWebhookSecretStrengthTest.php @@ -0,0 +1,231 @@ + 0, 'is_api_enabled' => true]); + + $this->team = Team::factory()->create(); + + $this->admin = User::factory()->create(); + $this->team->members()->attach($this->admin->id, ['role' => 'admin']); + + $this->member = User::factory()->create(); + $this->team->members()->attach($this->member->id, ['role' => 'member']); + + $plainTextToken = Str::random(40); + $token = $this->admin->tokens()->create([ + 'name' => 'manual-webhook-secret-test-'.Str::random(6), + 'token' => hash('sha256', $plainTextToken), + 'abilities' => ['*'], + 'team_id' => $this->team->id, + ]); + $this->bearerToken = $token->getKey().'|'.$plainTextToken; + + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::query()->where('server_id', $this->server->id)->firstOrFail(); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $this->project->id]); + + $this->application = Application::factory()->create([ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'git_repository' => 'https://github.com/coollabsio/coolify-examples', + 'git_branch' => 'main', + 'build_pack' => 'nixpacks', + 'manual_webhook_secret_github' => 'short-legacy', + 'manual_webhook_secret_gitlab' => 'gitlab-legacy', + 'manual_webhook_secret_bitbucket' => null, + 'manual_webhook_secret_gitea' => Str::random(40), + ]); +}); + +afterEach(function () { + Server::flushIdentityMap(); +}); + +function manualWebhookSecretApiHeaders(string $bearerToken): array +{ + return [ + 'Authorization' => 'Bearer '.$bearerToken, + 'Content-Type' => 'application/json', + ]; +} + +function manualWebhookSecretComponent(): Testable +{ + test()->actingAs(test()->admin); + session(['currentTeam' => test()->team]); + + return Livewire::test(Webhooks::class, ['resource' => test()->application->fresh()]); +} + +describe('Webhooks component', function () { + test('rejects a new secret shorter than 16 characters', function () { + manualWebhookSecretComponent() + ->set('githubManualWebhookSecret', 'too-short-12') + ->call('submit') + ->assertHasErrors(['githubManualWebhookSecret']) + ->assertNotDispatched('success'); + + expect($this->application->fresh()->manual_webhook_secret_github)->toBe('short-legacy'); + }); + + test('saves when existing short secrets are left unchanged', function () { + $newSecret = Str::random(16); + + manualWebhookSecretComponent() + ->set('bitbucketManualWebhookSecret', $newSecret) + ->call('submit') + ->assertHasNoErrors() + ->assertDispatched('success'); + + $application = $this->application->fresh(); + expect($application->manual_webhook_secret_github)->toBe('short-legacy') + ->and($application->manual_webhook_secret_gitlab)->toBe('gitlab-legacy') + ->and($application->manual_webhook_secret_bitbucket)->toBe($newSecret); + }); + + test('allows clearing a secret', function () { + manualWebhookSecretComponent() + ->set('githubManualWebhookSecret', '') + ->call('submit') + ->assertHasNoErrors() + ->assertDispatched('success'); + + expect($this->application->fresh()->manual_webhook_secret_github)->toBeEmpty(); + }); + + test('generate returns a random secret of at least 32 characters that saves with the form', function () { + $generated = null; + + $component = manualWebhookSecretComponent() + ->call('generateSecret', 'gitlab') + ->assertHasNoErrors() + ->assertReturned(function ($secret) use (&$generated) { + $generated = $secret; + + return is_string($secret) && strlen($secret) >= 32; + }) + ->assertSet('gitlabManualWebhookSecret', 'gitlab-legacy'); + + expect($this->application->fresh()->manual_webhook_secret_gitlab)->toBe('gitlab-legacy'); + + $component->set('gitlabManualWebhookSecret', $generated) + ->call('submit') + ->assertHasNoErrors() + ->assertDispatched('success'); + + expect($this->application->fresh()->manual_webhook_secret_gitlab)->toBe($generated); + }); + + test('generate rejects an unknown provider', function () { + manualWebhookSecretComponent() + ->call('generateSecret', 'deploywebhook') + ->assertDispatched('error') + ->assertReturned(null); + }); + + test('shows generate buttons only to users who can edit secrets', function () { + manualWebhookSecretComponent() + ->assertSeeHtml('aria-label="Generate GitHub webhook secret"') + ->assertSeeHtml('aria-label="Generate Gitea webhook secret"'); + + $this->actingAs($this->member); + + Livewire::test(Webhooks::class, ['resource' => $this->application->fresh()]) + ->assertDontSeeHtml('aria-label="Generate GitHub webhook secret"'); + }); + + test('members cannot generate a secret', function () { + $this->actingAs($this->member); + session(['currentTeam' => $this->team]); + + Livewire::test(Webhooks::class, ['resource' => $this->application->fresh()]) + ->call('generateSecret', 'github') + ->assertDispatched('error') + ->assertReturned(null); + }); +}); + +describe('Applications API', function () { + test('update rejects a new secret shorter than 16 characters', function () { + $this->withHeaders(manualWebhookSecretApiHeaders($this->bearerToken)) + ->patchJson("/api/v1/applications/{$this->application->uuid}", [ + 'manual_webhook_secret_github' => 'too-short-12', + ]) + ->assertUnprocessable() + ->assertInvalid(['manual_webhook_secret_github']); + + expect($this->application->fresh()->manual_webhook_secret_github)->toBe('short-legacy'); + }); + + test('update accepts an unchanged short secret, a 16 character secret, and null', function () { + $newSecret = Str::random(16); + + $this->withHeaders(manualWebhookSecretApiHeaders($this->bearerToken)) + ->patchJson("/api/v1/applications/{$this->application->uuid}", [ + 'manual_webhook_secret_github' => 'short-legacy', + 'manual_webhook_secret_gitlab' => $newSecret, + 'manual_webhook_secret_bitbucket' => null, + ]) + ->assertOk(); + + $application = $this->application->fresh(); + expect($application->manual_webhook_secret_github)->toBe('short-legacy') + ->and($application->manual_webhook_secret_gitlab)->toBe($newSecret) + ->and($application->manual_webhook_secret_bitbucket)->toBeNull(); + }); + + test('create rejects a secret shorter than 16 characters', function () { + $this->withHeaders(manualWebhookSecretApiHeaders($this->bearerToken)) + ->postJson('/api/v1/applications/dockerimage', [ + 'project_uuid' => $this->project->uuid, + 'environment_uuid' => $this->environment->uuid, + 'server_uuid' => $this->server->uuid, + 'docker_registry_image_name' => 'nginx', + 'docker_registry_image_tag' => 'latest', + 'ports_exposes' => '80', + 'instant_deploy' => false, + 'manual_webhook_secret_gitea' => 'too-short-12', + ]) + ->assertUnprocessable() + ->assertInvalid(['manual_webhook_secret_gitea']); + }); + + test('create accepts a secret of 16 characters', function () { + $secret = Str::random(16); + + $response = $this->withHeaders(manualWebhookSecretApiHeaders($this->bearerToken)) + ->postJson('/api/v1/applications/dockerimage', [ + 'project_uuid' => $this->project->uuid, + 'environment_uuid' => $this->environment->uuid, + 'server_uuid' => $this->server->uuid, + 'docker_registry_image_name' => 'nginx', + 'docker_registry_image_tag' => 'latest', + 'ports_exposes' => '80', + 'instant_deploy' => false, + 'manual_webhook_secret_gitea' => $secret, + ]) + ->assertCreated(); + + expect(Application::query()->where('uuid', $response->json('uuid'))->first()->manual_webhook_secret_gitea) + ->toBe($secret); + }); +});