From c098257e0f9dbb73406bd6e258857652b4e83d9c Mon Sep 17 00:00:00 2001 From: Christian Gastrell Date: Wed, 26 Nov 2025 14:07:56 -0300 Subject: [PATCH 1/5] add logging for skipped webhooks --- .../packages/forms/src/service/class-form-webhooks.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/projects/packages/forms/src/service/class-form-webhooks.php b/projects/packages/forms/src/service/class-form-webhooks.php index d6a8a0d8c447d..545d6cdaa51de 100644 --- a/projects/packages/forms/src/service/class-form-webhooks.php +++ b/projects/packages/forms/src/service/class-form-webhooks.php @@ -166,17 +166,24 @@ private function get_enabled_webhooks( $attributes = array() ) { ); // Validate webhook configuration - if ( empty( $setup['enabled'] ) || empty( $setup['url'] ) ) { + if ( empty( $setup['enabled'] ) ) { + continue; + } + // Validate webhook configuration + if ( empty( $setup['url'] ) ) { + do_action( 'jetpack_forms_log', 'webhook_skipped', 'url_empty' ); continue; } // Validate format if ( ! array_key_exists( strtolower( $setup['format'] ), self::VALID_FORMATS_MAP ) ) { + do_action( 'jetpack_forms_log', 'webhook_skipped', 'format_invalid', $setup ); continue; } // Validate method if ( ! in_array( strtoupper( $setup['method'] ), self::VALID_METHODS, true ) ) { + do_action( 'jetpack_forms_log', 'webhook_skipped', 'method_invalid', $setup ); continue; } @@ -232,6 +239,7 @@ private function send_webhook( $data, $webhook ) { ), 'sslverify' => true, ); + return wp_remote_request( $url, $args ); } From 40e92ba80cd7187bcba1b15c3af05e0c9736a328 Mon Sep 17 00:00:00 2001 From: Christian Gastrell Date: Wed, 26 Nov 2025 14:09:28 -0300 Subject: [PATCH 2/5] only init webhooks class if webhooks are enabled --- .../forms/src/contact-form/class-contact-form-plugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/packages/forms/src/contact-form/class-contact-form-plugin.php b/projects/packages/forms/src/contact-form/class-contact-form-plugin.php index 0ace56f1be5cf..afe299a605314 100644 --- a/projects/packages/forms/src/contact-form/class-contact-form-plugin.php +++ b/projects/packages/forms/src/contact-form/class-contact-form-plugin.php @@ -1570,7 +1570,7 @@ public function process_form_submission() { ); } - if ( ! empty( $form->attributes['webhooks'] ) ) { + if ( Jetpack_Forms::is_webhooks_enabled() && ! empty( $form->attributes['webhooks'] ) ) { Form_Webhooks::init(); } // Process the form From e77c4180c1584c909fd13a0031ec37664e483c10 Mon Sep 17 00:00:00 2001 From: Christian Gastrell Date: Wed, 26 Nov 2025 14:10:12 -0300 Subject: [PATCH 3/5] changelog --- .../forms/changelog/add-form-webhooks-skipped-logging | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 projects/packages/forms/changelog/add-form-webhooks-skipped-logging diff --git a/projects/packages/forms/changelog/add-form-webhooks-skipped-logging b/projects/packages/forms/changelog/add-form-webhooks-skipped-logging new file mode 100644 index 0000000000000..17b937e45d985 --- /dev/null +++ b/projects/packages/forms/changelog/add-form-webhooks-skipped-logging @@ -0,0 +1,4 @@ +Significance: patch +Type: added + +Form Webhooks: add logging and filter flag for initialization From e94c02d36df116a843539788fbf53f1e7cce292a Mon Sep 17 00:00:00 2001 From: Christian Gastrell Date: Wed, 26 Nov 2025 14:29:14 -0300 Subject: [PATCH 4/5] add tests for webhooks and contact form flags --- .../php/contact-form/Contact_Form_Test.php | 29 +++ .../tests/php/service/Form_Webhooks_Test.php | 171 ++++++++++++++++++ 2 files changed, 200 insertions(+) diff --git a/projects/packages/forms/tests/php/contact-form/Contact_Form_Test.php b/projects/packages/forms/tests/php/contact-form/Contact_Form_Test.php index 64804459020bb..d8df2f2ff0228 100644 --- a/projects/packages/forms/tests/php/contact-form/Contact_Form_Test.php +++ b/projects/packages/forms/tests/php/contact-form/Contact_Form_Test.php @@ -4014,4 +4014,33 @@ public function test_parse_handles_null_fields_without_fatal_error() { $this->assertIsString( $result5, 'Parse should return a string with multiple fields' ); $this->assertStringNotContainsString( 'is-single-input-form', $result5, 'Should not have single-input-form class with multiple fields' ); } + + /** + * Test is_webhooks_enabled returns false by default. + */ + public function test_is_webhooks_enabled_default() { + $this->assertFalse( \Automattic\Jetpack\Forms\Jetpack_Forms::is_webhooks_enabled() ); + } + + /** + * Test is_webhooks_enabled filter can be used to enable webhooks. + */ + public function test_is_webhooks_enabled_filter_enable() { + add_filter( 'jetpack_forms_webhooks_enabled', '__return_true' ); + + $this->assertTrue( \Automattic\Jetpack\Forms\Jetpack_Forms::is_webhooks_enabled() ); + + remove_filter( 'jetpack_forms_webhooks_enabled', '__return_true' ); + } + + /** + * Test is_webhooks_enabled filter can be used to keep webhooks disabled. + */ + public function test_is_webhooks_enabled_filter_disable() { + add_filter( 'jetpack_forms_webhooks_enabled', '__return_false' ); + + $this->assertFalse( \Automattic\Jetpack\Forms\Jetpack_Forms::is_webhooks_enabled() ); + + remove_filter( 'jetpack_forms_webhooks_enabled', '__return_false' ); + } } diff --git a/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php b/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php index 077be5c9cd412..3b5646641de4c 100644 --- a/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php +++ b/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php @@ -569,6 +569,177 @@ function ( $preempt, $args, $url ) use ( &$captured_request ) { $this->assertEquals( 'summer_2025', $body_data['utm_campaign'] ); } + /** + * Test logging action is triggered when webhook URL is empty. + */ + public function test_send_webhooks_logs_empty_url() { + $form = $this->create_mock_form( + array( + 'webhooks' => array( + array( + 'webhook_id' => 'test-webhook', + 'url' => '', + 'format' => 'json', + 'method' => 'POST', + 'enabled' => true, + ), + ), + ) + ); + $fields = array( $this->create_mock_field( $form, 'test-field', 'test value' ) ); + + $logged_events = array(); + add_action( + 'jetpack_forms_log', + function ( $event, $reason, $data = null ) use ( &$logged_events ) { + $logged_events[] = array( + 'event' => $event, + 'reason' => $reason, + 'data' => $data, + ); + }, + 10, + 3 + ); + + $webhooks = Form_Webhooks::init(); + $webhooks->send_webhooks( 123, $fields, false, array() ); + + $this->assertCount( 1, $logged_events ); + $this->assertEquals( 'webhook_skipped', $logged_events[0]['event'] ); + $this->assertEquals( 'url_empty', $logged_events[0]['reason'] ); + } + + /** + * Test logging action is triggered when webhook format is invalid. + */ + public function test_send_webhooks_logs_invalid_format() { + $form = $this->create_mock_form( + array( + 'webhooks' => array( + array( + 'webhook_id' => 'test-webhook', + 'url' => 'https://example.com/webhook', + 'format' => 'xml', + 'method' => 'POST', + 'enabled' => true, + ), + ), + ) + ); + $fields = array( $this->create_mock_field( $form, 'test-field', 'test value' ) ); + + $logged_events = array(); + add_action( + 'jetpack_forms_log', + function ( $event, $reason, $data = null ) use ( &$logged_events ) { + $logged_events[] = array( + 'event' => $event, + 'reason' => $reason, + 'data' => $data, + ); + }, + 10, + 3 + ); + + $webhooks = Form_Webhooks::init(); + $webhooks->send_webhooks( 123, $fields, false, array() ); + + $this->assertCount( 1, $logged_events ); + $this->assertEquals( 'webhook_skipped', $logged_events[0]['event'] ); + $this->assertEquals( 'format_invalid', $logged_events[0]['reason'] ); + $this->assertIsArray( $logged_events[0]['data'] ); + $this->assertEquals( 'xml', $logged_events[0]['data']['format'] ); + } + + /** + * Test logging action is triggered when webhook method is invalid. + */ + public function test_send_webhooks_logs_invalid_method() { + $form = $this->create_mock_form( + array( + 'webhooks' => array( + array( + 'webhook_id' => 'test-webhook', + 'url' => 'https://example.com/webhook', + 'format' => 'json', + 'method' => 'DELETE', + 'enabled' => true, + ), + ), + ) + ); + $fields = array( $this->create_mock_field( $form, 'test-field', 'test value' ) ); + + $logged_events = array(); + add_action( + 'jetpack_forms_log', + function ( $event, $reason, $data = null ) use ( &$logged_events ) { + $logged_events[] = array( + 'event' => $event, + 'reason' => $reason, + 'data' => $data, + ); + }, + 10, + 3 + ); + + $webhooks = Form_Webhooks::init(); + $webhooks->send_webhooks( 123, $fields, false, array() ); + + $this->assertCount( 1, $logged_events ); + $this->assertEquals( 'webhook_skipped', $logged_events[0]['event'] ); + $this->assertEquals( 'method_invalid', $logged_events[0]['reason'] ); + $this->assertIsArray( $logged_events[0]['data'] ); + $this->assertEquals( 'DELETE', $logged_events[0]['data']['method'] ); + } + + /** + * Test webhook method validation is case-insensitive. + */ + public function test_send_webhooks_accepts_lowercase_method() { + $form = $this->create_mock_form( + array( + 'webhooks' => array( + array( + 'webhook_id' => 'test-webhook', + 'url' => 'https://example.com/webhook', + 'format' => 'json', + 'method' => 'post', // lowercase should be accepted + 'enabled' => true, + ), + ), + ) + ); + $fields = array( $this->create_mock_field( $form, 'test-field', 'test value' ) ); + + $captured_request = null; + add_filter( + 'pre_http_request', + function ( $preempt, $args, $url ) use ( &$captured_request ) { + $captured_request = array( + 'url' => $url, + 'args' => $args, + ); + return array( + 'response' => array( 'code' => 200 ), + 'body' => '{"success":true}', + 'headers' => new CaseInsensitiveDictionary( array( 'Content-Type' => 'application/json' ) ), + ); + }, + 10, + 3 + ); + + $webhooks = Form_Webhooks::init(); + $webhooks->send_webhooks( 123, $fields, false, array() ); + + $this->assertNotNull( $captured_request, 'HTTP request should be made even with lowercase method' ); + $this->assertEquals( 'post', $captured_request['args']['method'] ); + } + /** * Helper method to create a mock form. * From 66e9b0c5928b796eab584246b02e4b60ffa28e4c Mon Sep 17 00:00:00 2001 From: Christian Gastrell Date: Wed, 26 Nov 2025 19:51:26 -0300 Subject: [PATCH 5/5] fix phan issues --- .../forms/tests/php/service/Form_Webhooks_Test.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php b/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php index 3b5646641de4c..c9bc04d2e22c5 100644 --- a/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php +++ b/projects/packages/forms/tests/php/service/Form_Webhooks_Test.php @@ -606,7 +606,9 @@ function ( $event, $reason, $data = null ) use ( &$logged_events ) { $webhooks->send_webhooks( 123, $fields, false, array() ); $this->assertCount( 1, $logged_events ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'webhook_skipped', $logged_events[0]['event'] ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'url_empty', $logged_events[0]['reason'] ); } @@ -647,9 +649,13 @@ function ( $event, $reason, $data = null ) use ( &$logged_events ) { $webhooks->send_webhooks( 123, $fields, false, array() ); $this->assertCount( 1, $logged_events ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'webhook_skipped', $logged_events[0]['event'] ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'format_invalid', $logged_events[0]['reason'] ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertIsArray( $logged_events[0]['data'] ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'xml', $logged_events[0]['data']['format'] ); } @@ -690,9 +696,13 @@ function ( $event, $reason, $data = null ) use ( &$logged_events ) { $webhooks->send_webhooks( 123, $fields, false, array() ); $this->assertCount( 1, $logged_events ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'webhook_skipped', $logged_events[0]['event'] ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'method_invalid', $logged_events[0]['reason'] ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertIsArray( $logged_events[0]['data'] ); + // @phan-suppress-next-line PhanTypeArraySuspiciousNull, PhanTypeInvalidDimOffset $this->assertEquals( 'DELETE', $logged_events[0]['data']['method'] ); }