From 9b9d1309d1264390121dbba20fd220a4310aaaa9 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:24:20 +0100 Subject: [PATCH] fix(api): allow write tokens to set manual webhook secrets Webhook secrets are stored settings, so a token with the write ability can set or change them, the same as GitHub App and GitLab App webhook secrets. Co-Authored-By: Claude Opus 5.5 --- .../Api/ApplicationsController.php | 22 --------- ...> ManualWebhookSecretWriteAbilityTest.php} | 46 ++++--------------- 2 files changed, 8 insertions(+), 60 deletions(-) rename tests/Feature/Api/{ManualWebhookSecretDeployAbilityTest.php => ManualWebhookSecretWriteAbilityTest.php} (53%) diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 7b280b45fd..21652dd974 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -161,21 +161,6 @@ class ApplicationsController extends Controller return $payload; } - /** - * Setting or changing a manual webhook secret needs the deploy ability. Sending the stored value - * again or clearing it does not. - */ - private function manualWebhookSecretForbiddenResponse(Request $request, ?Application $application = null): ?JsonResponse - { - foreach (['manual_webhook_secret_github', 'manual_webhook_secret_gitlab', 'manual_webhook_secret_bitbucket', 'manual_webhook_secret_gitea'] as $field) { - if ($request->filled($field) && $request->input($field) !== $application?->{$field}) { - return $this->deployAbilityForbiddenResponse($request, 'A manual webhook secret lets anyone who knows it start a deployment, so it needs a token with the deploy permission.'); - } - } - - return null; - } - private function applicationSettingsFromRequest(Request $request): array { $settings = []; @@ -1537,9 +1522,6 @@ class ApplicationsController extends Controller if ($internalNameValidation = $this->customInternalNameValidationResponse($applicationSettings)) { return $internalNameValidation; } - if ($forbidden = $this->manualWebhookSecretForbiddenResponse($request)) { - return $forbidden; - } if ($type === 'public') { $validationRules = [ 'git_repository' => ['string', 'required', new ValidGitRepositoryUrl], @@ -3502,10 +3484,6 @@ class ApplicationsController extends Controller 'errors' => $errors, ], 422); } - if ($forbidden = $this->manualWebhookSecretForbiddenResponse($request, $application)) { - return $forbidden; - } - $githubApp = null; if ($request->has('github_app_uuid')) { if ($application->build_pack === 'dockerimage' || filled($application->dockerfile) || $application->deploymentType() === 'deploy_key') { diff --git a/tests/Feature/Api/ManualWebhookSecretDeployAbilityTest.php b/tests/Feature/Api/ManualWebhookSecretWriteAbilityTest.php similarity index 53% rename from tests/Feature/Api/ManualWebhookSecretDeployAbilityTest.php rename to tests/Feature/Api/ManualWebhookSecretWriteAbilityTest.php index 737d7f25db..5cd944c941 100644 --- a/tests/Feature/Api/ManualWebhookSecretDeployAbilityTest.php +++ b/tests/Feature/Api/ManualWebhookSecretWriteAbilityTest.php @@ -50,51 +50,22 @@ function manualWebhookSecretAbilityHeaders(User $user, int $teamId, array $abili } describe('update', function () { - test('write-only token cannot set a new manual webhook secret', function (string $field) { + test('write-only token can set a new manual webhook secret', function (string $field) { $this->withHeaders(manualWebhookSecretAbilityHeaders($this->user, $this->team->id, ['write'])) - ->patchJson("/api/v1/applications/{$this->application->uuid}", [$field => 'attacker-secret-0123456789']) - ->assertForbidden() - ->assertJsonPath('message', 'Missing required permissions: deploy. A manual webhook secret lets anyone who knows it start a deployment, so it needs a token with the deploy permission.'); + ->patchJson("/api/v1/applications/{$this->application->uuid}", [$field => 'new-secret-0123456789']) + ->assertOk(); - $application = $this->application->fresh(); - expect($application->manual_webhook_secret_github)->toBe('existing-secret-0123456789'); - expect($application->{$field})->not->toBe('attacker-secret-0123456789'); + expect($this->application->fresh()->{$field})->toBe('new-secret-0123456789'); })->with([ 'manual_webhook_secret_github', 'manual_webhook_secret_gitlab', 'manual_webhook_secret_bitbucket', 'manual_webhook_secret_gitea', ]); - - test('write-only token can update other fields and send the stored secret back', function () { - $headers = manualWebhookSecretAbilityHeaders($this->user, $this->team->id, ['write']); - - $this->withHeaders($headers) - ->patchJson("/api/v1/applications/{$this->application->uuid}", ['description' => 'changed']) - ->assertOk(); - $this->withHeaders($headers) - ->patchJson("/api/v1/applications/{$this->application->uuid}", [ - 'description' => 'changed again', - 'manual_webhook_secret_github' => 'existing-secret-0123456789', - ]) - ->assertOk(); - - expect($this->application->fresh()) - ->description->toBe('changed again') - ->manual_webhook_secret_github->toBe('existing-secret-0123456789'); - }); - - test('write and deploy token can set a new manual webhook secret', function () { - $this->withHeaders(manualWebhookSecretAbilityHeaders($this->user, $this->team->id, ['write', 'deploy'])) - ->patchJson("/api/v1/applications/{$this->application->uuid}", ['manual_webhook_secret_gitlab' => 'new-gitlab-secret-0123456789']) - ->assertOk(); - - expect($this->application->fresh()->manual_webhook_secret_gitlab)->toBe('new-gitlab-secret-0123456789'); - }); }); describe('create', function () { - test('write-only token cannot create an application with a manual webhook secret', function () { + test('write-only token can create an application with a manual webhook secret', function () { $this->withHeaders(manualWebhookSecretAbilityHeaders($this->user, $this->team->id, ['write'])) ->postJson('/api/v1/applications/dockerimage', [ 'project_uuid' => $this->project->uuid, @@ -102,11 +73,10 @@ describe('create', function () { 'server_uuid' => $this->server->uuid, 'docker_registry_image_name' => 'nginx', 'ports_exposes' => '80', - 'manual_webhook_secret_github' => 'attacker-secret-0123456789', + 'manual_webhook_secret_github' => 'new-secret-0123456789', ]) - ->assertForbidden() - ->assertJsonPath('message', fn (string $message) => str_contains($message, 'manual webhook secret')); + ->assertCreated(); - expect(Application::count())->toBe(1); + expect(Application::latest('id')->first()->manual_webhook_secret_github)->toBe('new-secret-0123456789'); }); });