Skip to content

Commit 99709cd

Browse files
dottxadodottxado
authored
Fix check for default branch on received webhook, allowing for a custom one (#281)
* Fix check for default branch on received webhook, allowing for a custom one * Add local variables * Add project to the filter * Add project name to the filter * Add documentation * Change to repository name * Fix QA check * Fix QA checks * Add tests for more coverage * Fix test * Fix test and remove the check of href presence from the callback because it is already checked in the permission callback * Pass the full repository instance to the filter instead of only the name * Fix code sniffer error * Fix for static analysis
1 parent 9ee3c19 commit 99709cd

File tree

8 files changed

+93
-30
lines changed

8 files changed

+93
-30
lines changed

‎inc/Loader/Git.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ public function download(): ?string {
5151
* @since 4.0.0
5252
*
5353
* @param string $branch Name of the Git branch to clone. Empty string clones the default branch.
54+
* @param \Required\Traduttore\Repository $repository Full repository instance.
5455
*/
55-
$branch = apply_filters( 'traduttore.git_clone_branch', '' );
56+
$branch = apply_filters( 'traduttore.git_clone_branch', '', $this->repository );
5657
if ( '' !== $branch ) {
5758
$cmd .= ' --branch ' . escapeshellarg( $branch );
5859
}

‎inc/WebhookHandler/Base.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88
namespace Required\Traduttore\WebhookHandler;
99

1010
use Required\Traduttore\Project;
11+
use Required\Traduttore\ProjectLocator;
1112
use Required\Traduttore\WebhookHandler;
1213
use WP_REST_Request;
14+
use WP_REST_Response;
1315

1416
/**
1517
* Base webhook handler class.
@@ -85,4 +87,34 @@ protected function get_secret( ?Project $project = null ): ?string {
8587
*/
8688
return apply_filters( 'traduttore.webhook_secret', $secret, $this, $project );
8789
}
90+
91+
/**
92+
* Return a valid project or errors. Allows to customize the pulled branch.
93+
*
94+
* @param string $repository Metadata to find a GlotPress project.
95+
* @param string $default_branch Name of the repository's default branch.
96+
* @param string $ref Name of the received branch through the webhook.
97+
* @return \Required\Traduttore\Project|\WP_REST_Response|\WP_Error
98+
*/
99+
protected function resolve_project( string $repository, string $default_branch = '', string $ref = '' ): Project|WP_REST_Response|\WP_Error {
100+
$locator = new ProjectLocator( $repository );
101+
$project = $locator->get_project();
102+
103+
if ( ! $project ) {
104+
return new \WP_Error( '404', 'Could not find project for this repository', [ 'status' => 404 ] );
105+
}
106+
107+
if ( empty( $default_branch ) || empty( $ref ) ) {
108+
return $project;
109+
}
110+
111+
$branch = 'refs/heads/' . (string) apply_filters( 'traduttore.git_clone_branch', $default_branch, $project->get_repository_name() );
112+
113+
// We only care about the default or custom branch but don't want to send an error still.
114+
if ( $branch !== $ref ) {
115+
return new WP_REST_Response( [ 'result' => 'Not the default or custom branch' ] );
116+
}
117+
118+
return $project;
119+
}
88120
}

‎inc/WebhookHandler/Bitbucket.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Required\Traduttore\WebhookHandler;
99

10+
use Required\Traduttore\Project;
1011
use Required\Traduttore\ProjectLocator;
1112
use Required\Traduttore\Repository;
1213
use Required\Traduttore\Updater;
@@ -79,20 +80,20 @@ public function callback(): \WP_Error|\WP_REST_Response {
7980
* @var array{repository: array{scm: string, full_name: string, links: array{html: array{href: string}}, is_private: bool}} $params
8081
*/
8182
$params = $this->request->get_params();
83+
$href = (string) $params['repository']['links']['html']['href'];
8284

83-
$locator = new ProjectLocator( $params['repository']['links']['html']['href'] );
84-
$project = $locator->get_project();
85+
$project = $this->resolve_project( $href );
8586

86-
if ( ! $project ) {
87-
return new \WP_Error( '404', 'Could not find project for this repository' );
87+
if ( ! $project instanceof Project ) {
88+
return $project;
8889
}
8990

9091
if ( ! $project->get_repository_vcs_type() ) {
9192
$project->set_repository_vcs_type( 'git' === $params['repository']['scm'] ? Repository::VCS_TYPE_GIT : Repository::VCS_TYPE_HG );
9293
}
9394

9495
$project->set_repository_name( $params['repository']['full_name'] );
95-
$project->set_repository_url( $params['repository']['links']['html']['href'] );
96+
$project->set_repository_url( $href );
9697

9798
$ssh_url = sprintf( '[email protected]:%s.git', $project->get_repository_name() );
9899
$https_url = sprintf( 'https://bitbucket.org/%s.git', $project->get_repository_name() );

‎inc/WebhookHandler/GitHub.php‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Required\Traduttore\WebhookHandler;
99

10+
use Required\Traduttore\Project;
1011
use Required\Traduttore\ProjectLocator;
1112
use Required\Traduttore\Repository;
1213
use Required\Traduttore\Updater;
@@ -117,24 +118,25 @@ public function callback(): \WP_Error|\WP_REST_Response {
117118
* @var array{repository: array{ default_branch?: string, html_url: string, full_name: string, ssh_url: string, clone_url: string, private: bool }, ref: string } $params
118119
*/
119120

120-
if ( ! isset( $params['repository']['default_branch'] ) ) {
121-
return new \WP_Error( '400', 'Request incomplete', [ 'status' => 400 ] );
122-
}
121+
$default_branch = (string) ( $params['repository']['default_branch'] ?? '' );
122+
$html_url = (string) $params['repository']['html_url'];
123+
$ref = (string) $params['ref'];
123124

124-
// We only care about the default branch but don't want to send an error still.
125-
if ( 'refs/heads/' . $params['repository']['default_branch'] !== $params['ref'] ) {
126-
return new WP_REST_Response( [ 'result' => 'Not the default branch' ] );
125+
if ( '' === $default_branch
126+
| '' === $html_url
127+
| '' === $ref
128+
) {
129+
return new \WP_Error( '400', 'Request incomplete', [ 'status' => 400 ] );
127130
}
128131

129-
$locator = new ProjectLocator( $params['repository']['html_url'] );
130-
$project = $locator->get_project();
132+
$project = $this->resolve_project( $html_url, $default_branch, $ref );
131133

132-
if ( ! $project ) {
133-
return new \WP_Error( '404', 'Could not find project for this repository', [ 'status' => 404 ] );
134+
if ( ! $project instanceof Project ) {
135+
return $project;
134136
}
135137

136138
$project->set_repository_name( $params['repository']['full_name'] );
137-
$project->set_repository_url( $params['repository']['html_url'] );
139+
$project->set_repository_url( $html_url );
138140
$project->set_repository_ssh_url( $params['repository']['ssh_url'] );
139141
$project->set_repository_https_url( $params['repository']['clone_url'] );
140142
$project->set_repository_visibility( false === $params['repository']['private'] ? 'public' : 'private' );

‎inc/WebhookHandler/GitLab.php‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Required\Traduttore\WebhookHandler;
99

10+
use Required\Traduttore\Project;
1011
use Required\Traduttore\ProjectLocator;
1112
use Required\Traduttore\Repository;
1213
use Required\Traduttore\Updater;
@@ -77,22 +78,26 @@ public function callback(): \WP_Error|\WP_REST_Response {
7778
*
7879
* @var array{project: array{default_branch: string, homepage: string, path_with_namespace: string, ssh_url: string, http_url: string, visibility_level: int}, ref: string} $params
7980
*/
80-
$params = $this->request->get_params();
81-
82-
// We only care about the default branch but don't want to send an error still.
83-
if ( 'refs/heads/' . $params['project']['default_branch'] !== $params['ref'] ) {
84-
return new WP_REST_Response( [ 'result' => 'Not the default branch' ] );
81+
$params = $this->request->get_params();
82+
$default_branch = (string) $params['project']['default_branch'];
83+
$homepage = (string) $params['project']['homepage'];
84+
$ref = (string) $params['ref'];
85+
86+
if ( '' === $default_branch
87+
| '' === $homepage
88+
| '' === $ref
89+
) {
90+
return new \WP_Error( '400', 'Request incomplete', [ 'status' => 400 ] );
8591
}
8692

87-
$locator = new ProjectLocator( $params['project']['homepage'] );
88-
$project = $locator->get_project();
93+
$project = $this->resolve_project( $homepage, $default_branch, $ref );
8994

90-
if ( ! $project ) {
91-
return new \WP_Error( '404', 'Could not find project for this repository' );
95+
if ( ! $project instanceof Project ) {
96+
return $project;
9297
}
9398

9499
$project->set_repository_name( $params['project']['path_with_namespace'] );
95-
$project->set_repository_url( $params['project']['homepage'] );
100+
$project->set_repository_url( $homepage );
96101
$project->set_repository_ssh_url( $params['project']['ssh_url'] );
97102
$project->set_repository_https_url( $params['project']['http_url'] );
98103
$project->set_repository_visibility( 20 === $params['project']['visibility_level'] ? 'public' : 'private' );

‎tests/phpunit/tests/WebhookHandler/GitHub.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public function test_invalid_branch(): void {
9696
$response = rest_get_server()->dispatch( $request );
9797

9898
$this->assertSame( 200, $response->get_status() );
99-
$this->assertSame( [ 'result' => 'Not the default branch' ], $response->get_data() );
99+
$this->assertSame( [ 'result' => 'Not the default or custom branch' ], $response->get_data() );
100100
}
101101

102102
public function test_invalid_project(): void {

‎tests/phpunit/tests/WebhookHandler/GitLab.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public function test_invalid_branch(): void {
8181
$response = rest_get_server()->dispatch( $request );
8282

8383
$this->assertSame( 200, $response->get_status() );
84-
$this->assertSame( [ 'result' => 'Not the default branch' ], $response->get_data() );
84+
$this->assertSame( [ 'result' => 'Not the default or custom branch' ], $response->get_data() );
8585
}
8686

8787
public function test_invalid_project(): void {
@@ -106,6 +106,28 @@ public function test_invalid_project(): void {
106106
$this->assertErrorResponse( 404, $response );
107107
}
108108

109+
public function test_request_incomplete(): void {
110+
$request = new WP_REST_Request( 'POST', '/traduttore/v1/incoming-webhook' );
111+
$request->set_body_params(
112+
[
113+
'ref' => '',
114+
'project' => [
115+
'default_branch' => 'master',
116+
'path_with_namespace' => 'wearerequired/traduttore',
117+
'homepage' => 'https://gitlab.com/wearerequired/traduttore',
118+
'http_url' => 'https://gitlab.com/wearerequired/traduttore.git',
119+
'ssh_url' => '[email protected]/wearerequired/traduttore.git',
120+
'visibility_level' => 20,
121+
],
122+
]
123+
);
124+
$request->add_header( 'x-gitlab-event', 'Push Hook' );
125+
$request->add_header( 'x-gitlab-token', 'traduttore-test' );
126+
$response = rest_get_server()->dispatch( $request );
127+
128+
$this->assertErrorResponse( 400, $response );
129+
}
130+
109131
public function test_valid_project(): void {
110132
$request = new WP_REST_Request( 'POST', '/traduttore/v1/incoming-webhook' );
111133
$request->set_body_params(

‎tests/phpunit/tests/WebhookHandler/LegacyGitHub.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function test_invalid_branch(): void {
9393
$response = rest_get_server()->dispatch( $request );
9494

9595
$this->assertSame( 200, $response->get_status() );
96-
$this->assertSame( [ 'result' => 'Not the default branch' ], $response->get_data() );
96+
$this->assertSame( [ 'result' => 'Not the default or custom branch' ], $response->get_data() );
9797
}
9898

9999
public function test_invalid_project(): void {

0 commit comments

Comments
 (0)