From eb9c04b645a6948521adfc1a0e30d0ad34aaace0 Mon Sep 17 00:00:00 2001 From: Steffen Bewersdorff Date: Mon, 13 Jul 2026 13:00:51 +0200 Subject: [PATCH 1/2] fix(security): harden service callback authentication --- .../class-test-runs-list-table.php | 6 +- .../class-test-runs-queue-list-table.php | 5 +- .../class-rest-service-controller.php | 99 ++++++++++++------- 3 files changed, 70 insertions(+), 40 deletions(-) diff --git a/includes/list-tables/class-test-runs-list-table.php b/includes/list-tables/class-test-runs-list-table.php index 298abc7..80959fc 100644 --- a/includes/list-tables/class-test-runs-list-table.php +++ b/includes/list-tables/class-test-runs-list-table.php @@ -302,8 +302,10 @@ public function column_trigger( $item ) { '%s%s', esc_attr( $item->trigger ), esc_html( $trigger_title ), - empty( $trigger_note ) ? '' : sprintf('

%1$s

', - $trigger_note + empty( $trigger_note ) ? '' : sprintf( + '

%2$s

', + esc_attr( $trigger_note ), + esc_html( $trigger_note ) ) ); } diff --git a/includes/list-tables/class-test-runs-queue-list-table.php b/includes/list-tables/class-test-runs-queue-list-table.php index 0a544eb..c638f12 100644 --- a/includes/list-tables/class-test-runs-queue-list-table.php +++ b/includes/list-tables/class-test-runs-queue-list-table.php @@ -197,10 +197,11 @@ public function column_trigger( $item ) { $trigger_note = Test_Run::get_trigger_note( $item ); return sprintf( - '%s

%3$s

', + '%s

%s

', esc_attr( $item->trigger ), esc_html( $trigger_title ), - $trigger_note + esc_attr( $trigger_note ), + esc_html( $trigger_note ) ); } diff --git a/includes/rest-api/class-rest-service-controller.php b/includes/rest-api/class-rest-service-controller.php index 88d22d5..eb3e5ec 100644 --- a/includes/rest-api/class-rest-service-controller.php +++ b/includes/rest-api/class-rest-service-controller.php @@ -52,7 +52,7 @@ public function register_routes() { public function ajax_action() { // phpcs:ignore WordPress.Security.NonceVerification.Recommended,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- It's ok. $data = json_decode( wp_unslash( $_REQUEST['data'] ?? '' ), true ); - $rest_response = $this->perform_action( $data ?? [] ); + $rest_response = $this->perform_action( is_array( $data ) ? $data : [] ); // If rest response is WP error, get the status code. if ( is_wp_error( $rest_response ) ) { @@ -86,6 +86,22 @@ public function perform_action( $data ) { return new WP_Error( 'error', esc_html__( 'Action parameter is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); } + $allowed_actions = [ + 'test_updated', + 'run_updated', + 'run_deleted', + 'subscription_changed', + ]; + + if ( ! is_string( $data['action'] ) || ! in_array( $data['action'], $allowed_actions, true ) ) { + return $this->unknown_action_request(); + } + + $authentication = $this->authenticate_request( $data ); + if ( is_wp_error( $authentication ) ) { + return $authentication; + } + switch ( $data['action'] ) { case 'test_updated': @@ -103,10 +119,6 @@ public function perform_action( $data ) { case 'subscription_changed': $response = $this->subscription_changed_request(); break; - - default: - $response = $this->unknown_action_request(); - break; }//end switch return $response; @@ -118,18 +130,10 @@ public function perform_action( $data ) { * @param array $data Rest api response body. */ private function test_updated_request( $data ) { - if ( ! array_key_exists( 'project_id', $data ) ) { - return new WP_Error( 'error', esc_html__( 'Project id is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } elseif ( get_option( 'vrts_project_id' ) !== $data['project_id'] ) { - return new WP_Error( 'error', esc_html__( 'Project id does not match.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } elseif ( ! array_key_exists( 'test_id', $data ) ) { + if ( ! array_key_exists( 'test_id', $data ) ) { return new WP_Error( 'error', esc_html__( 'Test id is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); } - if ( ! self::verify_signature( $data ) ) { - return new WP_Error( 'error', esc_html__( 'Signature is not valid.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } - $test_service = new Test_Service(); if ( $test_service->update_test_from_api_data( $data ) ) { @@ -149,18 +153,10 @@ private function test_updated_request( $data ) { * @param array $data Rest api response body. */ private function run_updated_request( $data ) { - if ( ! array_key_exists( 'project_id', $data ) ) { - return new WP_Error( 'error', esc_html__( 'Project id is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } elseif ( get_option( 'vrts_project_id' ) !== $data['project_id'] ) { - return new WP_Error( 'error', esc_html__( 'Project id does not match.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } elseif ( ! array_key_exists( 'run_id', $data ) ) { + if ( ! array_key_exists( 'run_id', $data ) ) { return new WP_Error( 'error', esc_html__( 'Run id is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); } - if ( ! self::verify_signature( $data ) ) { - return new WP_Error( 'error', esc_html__( 'Signature is not valid.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } - $test_run_service = new Test_Run_Service(); if ( $test_run_service->update_run_from_api_data( $data ) ) { @@ -180,18 +176,10 @@ private function run_updated_request( $data ) { * @param array $data Rest api response body. */ private function run_deleted_request( $data ) { - if ( ! array_key_exists( 'project_id', $data ) ) { - return new WP_Error( 'error', esc_html__( 'Project id is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } elseif ( get_option( 'vrts_project_id' ) !== $data['project_id'] ) { - return new WP_Error( 'error', esc_html__( 'Project id does not match.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } elseif ( ! array_key_exists( 'run_id', $data ) ) { + if ( ! array_key_exists( 'run_id', $data ) ) { return new WP_Error( 'error', esc_html__( 'Run id is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); } - if ( ! self::verify_signature( $data ) ) { - return new WP_Error( 'error', esc_html__( 'Signature is not valid.', 'visual-regression-tests' ), [ 'status' => 403 ] ); - } - $test_run_service = new Test_Run_Service(); if ( Test_Run::delete_by_service_test_run_id( $data['run_id'] ) ) { @@ -204,19 +192,58 @@ private function run_deleted_request( $data ) { } /** - * Verify signature + * Authenticate a service callback. * * @param array $data Rest api response body. * + * @return bool|WP_Error + */ + private function authenticate_request( $data ) { + $project_id = get_option( 'vrts_project_id' ); + $secret = get_option( 'vrts_project_secret' ); + + if ( ! is_string( $project_id ) || '' === $project_id || ! is_string( $secret ) || '' === $secret ) { + return new WP_Error( 'error', esc_html__( 'Service is not configured.', 'visual-regression-tests' ), [ 'status' => 403 ] ); + } + + if ( ! array_key_exists( 'project_id', $data ) ) { + return new WP_Error( 'error', esc_html__( 'Project id is missing.', 'visual-regression-tests' ), [ 'status' => 403 ] ); + } + + if ( ! is_string( $data['project_id'] ) || $project_id !== $data['project_id'] ) { + return new WP_Error( 'error', esc_html__( 'Project id does not match.', 'visual-regression-tests' ), [ 'status' => 403 ] ); + } + + if ( ! $this->verify_signature( $data, $secret ) ) { + return new WP_Error( 'error', esc_html__( 'Signature is not valid.', 'visual-regression-tests' ), [ 'status' => 403 ] ); + } + + return true; + } + + /** + * Verify signature. + * + * @param array $data Rest api response body. + * @param string $secret Project signing secret. + * * @return bool */ - private function verify_signature( $data ) { + private function verify_signature( $data, $secret ) { + if ( + ! array_key_exists( 'signature', $data ) + || ! is_string( $data['signature'] ) + || 1 !== preg_match( '/^[a-f0-9]{64}$/', $data['signature'] ) + ) { + return false; + } + $signature = $data['signature']; unset( $data['signature'] ); - $secret = get_option( 'vrts_project_secret' ) || 'verysecret'; + $expected_signature = hash_hmac( 'sha256', wp_json_encode( $data ), $secret ); - return hash_equals( $signature, hash_hmac( 'sha256', wp_json_encode( $data ), $secret ) ); + return hash_equals( $expected_signature, $signature ); } /** From 13eef3385bd28b33775372e476587c77f129909e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harun=20Ba=C5=A1i=C4=87?= Date: Fri, 17 Jul 2026 11:50:24 +0200 Subject: [PATCH 2/2] fix(security): verify callbacks against the dual-signature scheme --- .../rest-api/class-rest-service-controller.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/includes/rest-api/class-rest-service-controller.php b/includes/rest-api/class-rest-service-controller.php index eb3e5ec..6101651 100644 --- a/includes/rest-api/class-rest-service-controller.php +++ b/includes/rest-api/class-rest-service-controller.php @@ -224,6 +224,11 @@ private function authenticate_request( $data ) { /** * Verify signature. * + * The service dual-signs callbacks during the transition away from the + * broken pre-2.0.9 scheme: `signature` carries the legacy HMAC for old + * plugins, `signature_v2` the HMAC keyed with the real project secret, + * computed over the payload without either signature field. + * * @param array $data Rest api response body. * @param string $secret Project signing secret. * @@ -231,15 +236,15 @@ private function authenticate_request( $data ) { */ private function verify_signature( $data, $secret ) { if ( - ! array_key_exists( 'signature', $data ) - || ! is_string( $data['signature'] ) - || 1 !== preg_match( '/^[a-f0-9]{64}$/', $data['signature'] ) + ! array_key_exists( 'signature_v2', $data ) + || ! is_string( $data['signature_v2'] ) + || 1 !== preg_match( '/^[a-f0-9]{64}$/', $data['signature_v2'] ) ) { return false; } - $signature = $data['signature']; - unset( $data['signature'] ); + $signature = $data['signature_v2']; + unset( $data['signature'], $data['signature_v2'] ); $expected_signature = hash_hmac( 'sha256', wp_json_encode( $data ), $secret );