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); + }); +});