Skip to content

fix: address security vulnerabilities in webhooks plugin #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: address security vulnerabilities in webhooks plugin #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8dcb2c
fix: address security vulnerabilities in webhooks plugin
josephfusco Jul 11, 2025
cff50ab
Create spotty-mice-behave.md
josephfusco Jul 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/spotty-mice-behave.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@wpengine/wpgraphql-webhooks-wordpress-plugin": patch
---

fix: security improvements for webhooks plugin

- Enhanced input validation and sanitization
- Improved output escaping
- Strengthened authorization checks
- Added additional security hardening measures
9 changes: 2 additions & 7 deletions plugins/wp-graphql-webhooks/src/Admin/WebhooksAdmin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private function verify_admin_permission(): bool {
* @return bool True if nonce is valid, false otherwise.
*/
private function verify_nonce( string $nonce_name, string $action ): bool {
if ( ! isset( $_REQUEST[ $nonce_name ] ) || ! wp_verify_nonce( $_REQUEST[ $nonce_name ], $action ) ) {
if ( ! isset( $_REQUEST[ $nonce_name ] ) || ! wp_verify_nonce( wp_unslash( $_REQUEST[ $nonce_name ] ), $action ) ) {
wp_die( __( 'Security check failed.', 'wp-graphql-webhooks' ) );
return false;
}
Expand All @@ -185,11 +185,6 @@ public function handle_webhook_save() {
wp_die( __( 'Unauthorized', 'wp-graphql-webhooks' ) );
}

$webhook_id = isset( $_POST['webhook_id'] ) ? intval( $_POST['webhook_id'] ) : 0;
if ( ! $this->verify_admin_permission() || ! $this->verify_nonce( 'webhook_nonce', 'webhook_save' ) ) {
wp_die( __( 'Unauthorized', 'wp-graphql-webhooks' ) );
}

$webhook_id = isset( $_POST['webhook_id'] ) ? intval( $_POST['webhook_id'] ) : 0;
$webhook = new Webhook(
$webhook_id,
Expand Down Expand Up @@ -375,7 +370,7 @@ public function ajax_test_webhook(): void {
] );
}

if ( ! current_user_can( 'manage_options' ) ) {
if ( ! $this->verify_admin_permission() ) {

This comment was marked as resolved.

Sign in to view

wp_send_json_error( [
'message' => __( 'You do not have permission to test webhooks.', 'wp-graphql-webhooks' ),
'error_code' => 'insufficient_permissions'
Expand Down
12 changes: 6 additions & 6 deletions plugins/wp-graphql-webhooks/src/Admin/WebhooksListTable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ public function process_bulk_action() {
}

// Verify nonce
if ( ! isset( $_REQUEST['_wpnonce'] ) || ! wp_verify_nonce( $_REQUEST['_wpnonce'], 'bulk-' . $this->_args['plural'] ) ) {
wp_die( __( 'Security check failed.', 'wp-graphql-webhooks' ) );
if ( ! isset( $_REQUEST['_wpnonce'] ) || ! wp_verify_nonce( wp_unslash( $_REQUEST['_wpnonce'] ), 'bulk-' . $this->_args['plural'] ) ) {
wp_die( esc_html__( 'Security check failed.', 'wp-graphql-webhooks' ) );
}

// Check permissions
if ( ! current_user_can( 'manage_options' ) ) {
wp_die( __( 'You do not have sufficient permissions to access this page.', 'wp-graphql-webhooks' ) );
wp_die( esc_html__( 'You do not have sufficient permissions to access this page.', 'wp-graphql-webhooks' ) );
}

// Get selected webhooks
Expand Down Expand Up @@ -137,8 +137,8 @@ public function prepare_items() {
$total_items = count( $webhooks );

// Handle sorting
$orderby = ! empty( $_GET['orderby'] ) ? $_GET['orderby'] : 'name';
$order = ! empty( $_GET['order'] ) ? $_GET['order'] : 'asc';
$orderby = ! empty( $_GET['orderby'] ) ? sanitize_key( $_GET['orderby'] ) : 'name';
$order = ! empty( $_GET['order'] ) ? sanitize_key( $_GET['order'] ) : 'asc';

usort( $webhooks, function( $a, $b ) use ( $orderby, $order ) {
$result = 0;
Expand Down Expand Up @@ -253,7 +253,7 @@ public function column_name( $item ) {
* Display when no items
*/
public function no_items() {
_e( 'No webhooks found.', 'wp-graphql-webhooks' );
esc_html_e( 'No webhooks found.', 'wp-graphql-webhooks' );
}

/**
Expand Down
6 changes: 4 additions & 2 deletions plugins/wp-graphql-webhooks/src/Events/SmartCacheWebhookManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ private function trigger_webhooks( string $event, array $payload ): void {
$payload['uri'] = $payload['path'] ?? '';
}

error_log( "[Webhook] Triggering webhooks for event: $event with payload: " . var_export( $payload, true ) );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "[Webhook] Triggering webhooks for event: $event with payload: " . var_export( $payload, true ) );
}

do_action( 'graphql_webhooks_before_trigger', $event, $payload );

Expand Down Expand Up @@ -240,7 +242,7 @@ public function get_path_from_key( $key ) {
}

if ( ! empty( $permalink ) && is_string( $permalink ) && ! is_wp_error( $permalink ) ) {
$parsed_path = parse_url( $permalink, PHP_URL_PATH );
$parsed_path = wp_parse_url( $permalink, PHP_URL_PATH );
if ( $parsed_path !== false ) {
$path = $parsed_path;
error_log( "[Webhook] Final path for key $key: $path" );
Expand Down
38 changes: 25 additions & 13 deletions plugins/wp-graphql-webhooks/src/Handlers/WebhookHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ class WebhookHandler implements Handler {
public function handle( Webhook $webhook, array $payload ): void {
// Log webhook dispatch initiation
$dispatch_timestamp = current_time( 'mysql' );
error_log( "\n========== WEBHOOK DISPATCH ==========" );
error_log( "Timestamp: {$dispatch_timestamp}" );
error_log( "Webhook: {$webhook->name} (ID: {$webhook->id})" );
error_log( "Event: {$webhook->event}" );
error_log( "Target URL: {$webhook->url}" );
error_log( "Method: {$webhook->method}" );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "\n========== WEBHOOK DISPATCH ==========" );
error_log( "Timestamp: {$dispatch_timestamp}" );
error_log( "Webhook: {$webhook->name} (ID: {$webhook->id})" );
error_log( "Event: {$webhook->event}" );
error_log( "Target URL: {$webhook->url}" );
Copy link
Preview

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging the target URL may expose sensitive endpoints; wrap this in the WP_DEBUG check or remove it to avoid leaking URLs in production logs.

Suggested change
error_log( "Target URL: {$webhook->url}" );
// Removed logging of Target URL to prevent exposure of sensitive endpoints.

Copilot uses AI. Check for mistakes.

error_log( "Method: {$webhook->method}" );
}

$args = [
'headers' => $webhook->headers ?: [ 'Content-Type' => 'application/json' ],
Expand All @@ -52,7 +54,9 @@ public function handle( Webhook $webhook, array $payload ): void {
if ( strtoupper( $webhook->method ) === 'GET' ) {
$url = add_query_arg( $payload, $webhook->url );
$args['method'] = 'GET';
error_log( "Payload (GET query params): " . wp_json_encode( $payload ) );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "Payload (GET query params): " . wp_json_encode( $payload ) );
}
} else {
$url = $webhook->url;
$args['method'] = strtoupper( $webhook->method );
Expand All @@ -63,20 +67,28 @@ public function handle( Webhook $webhook, array $payload ): void {
$args['headers']['Content-Type'] = 'application/json';
}

error_log( "Payload ({$args['method']} body): " . $args['body'] );
error_log( "Payload size: " . strlen( $args['body'] ) . " bytes" );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "Payload ({$args['method']} body): " . $args['body'] );
error_log( "Payload size: " . strlen( $args['body'] ) . " bytes" );
}
}

// Log headers
error_log( "Headers: " . wp_json_encode( $args['headers'] ) );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "Headers: " . wp_json_encode( $args['headers'] ) );
Copy link
Preview

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging headers can reveal sensitive authentication tokens; redact or omit header values unless explicitly needed for debugging behind WP_DEBUG.

Suggested change
error_log( "Headers: " . wp_json_encode( $args['headers'] ) );
$redacted_headers = array_map(
function( $value, $key ) {
// Redact sensitive headers
$sensitive_keys = [ 'Authorization', 'Cookie', 'Set-Cookie' ];
return in_array( $key, $sensitive_keys, true ) ? '[REDACTED]' : $value;
},
$args['headers'],
array_keys( $args['headers'] )
);
error_log( "Headers: " . wp_json_encode( $redacted_headers ) );

Copilot uses AI. Check for mistakes.

}

// For test mode or debugging, optionally use blocking mode
if ( apply_filters( 'graphql_webhooks_test_mode', false, $webhook ) ) {
$args['blocking'] = true;
error_log( "Test mode enabled - using blocking request" );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "Test mode enabled - using blocking request" );
}
}

error_log( "====================================\n" );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "====================================\n" );
}

// Send the webhook
$start_time = microtime( true );
Expand All @@ -85,7 +97,7 @@ public function handle( Webhook $webhook, array $payload ): void {
$duration = round( ( $end_time - $start_time ) * 1000, 2 );

// Log response if in blocking mode
if ( $args['blocking'] ) {
if ( $args['blocking'] && defined( 'WP_DEBUG' ) && WP_DEBUG ) {
if ( is_wp_error( $response ) ) {
error_log( "\n========== WEBHOOK ERROR ==========" );
error_log( "❌ ERROR: " . $response->get_error_message() );
Expand Down
43 changes: 24 additions & 19 deletions plugins/wp-graphql-webhooks/src/Rest/WebhookTestEndpoint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,16 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er

// Log test initiation
$test_timestamp = current_time( 'mysql' );
error_log( "\n========== WEBHOOK TEST INITIATED ==========" );
error_log( "Timestamp: {$test_timestamp}" );
error_log( "Webhook ID: {$webhook_id}" );
error_log( "Webhook Name: {$webhook->name}" );
error_log( "Target URL: {$webhook->url}" );
error_log( "HTTP Method: {$webhook->method}" );
error_log( "Event: {$webhook->event}" );
error_log( "Headers: " . wp_json_encode( $webhook->headers ) );
error_log( "==========================================\n" );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "\n========== WEBHOOK TEST INITIATED ==========" );
error_log( "Timestamp: {$test_timestamp}" );
error_log( "Webhook ID: {$webhook_id}" );
error_log( "Webhook Name: {$webhook->name}" );
// Do not log sensitive URL and headers
error_log( "HTTP Method: {$webhook->method}" );
error_log( "Event: {$webhook->event}" );
error_log( "==========================================\n" );
}

// Create test payload
$test_payload = [
Expand All @@ -107,7 +108,7 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er
],
'test_data' => [
'message' => 'This is a test webhook payload',
'triggered_by' => wp_get_current_user()->user_login,
'triggered_by' => 'admin',
Copy link
Preview

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding 'admin' for triggered_by loses the actual user identity; consider using the sanitized current user login or ID to preserve accurate audit data.

Suggested change
'triggered_by' => 'admin',
'triggered_by' => sanitize_text_field( wp_get_current_user()->user_login ?: 'anonymous' ),

Copilot uses AI. Check for mistakes.

'site_url' => get_site_url(),
],
];
Expand All @@ -124,11 +125,13 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er
$end_time = microtime( true );
$duration = round( ( $end_time - $start_time ) * 1000, 2 ); // Convert to milliseconds

error_log( "\n========== WEBHOOK TEST COMPLETED ==========" );
error_log( "✅ SUCCESS: Test webhook dispatched" );
error_log( "Duration: {$duration}ms" );
error_log( "Completed at: " . current_time( 'mysql' ) );
error_log( "==========================================\n" );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "\n========== WEBHOOK TEST COMPLETED ==========" );
error_log( "✅ SUCCESS: Test webhook dispatched" );
error_log( "Duration: {$duration}ms" );
error_log( "Completed at: " . current_time( 'mysql' ) );
error_log( "==========================================\n" );
}

return new WP_REST_Response(
[
Expand All @@ -147,10 +150,12 @@ public function test_webhook( WP_REST_Request $request ): WP_REST_Response|WP_Er
200
);
} catch ( \Exception $e ) {
error_log( "\n========== WEBHOOK TEST ERROR ==========" );
error_log( "❌ ERROR: " . $e->getMessage() );
error_log( "Stack trace: " . $e->getTraceAsString() );
error_log( "========================================\n" );
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
error_log( "\n========== WEBHOOK TEST ERROR ==========" );
error_log( "❌ ERROR: " . $e->getMessage() );
// Do not log stack trace as it may contain sensitive information
error_log( "========================================\n" );
}

return new WP_Error(
'webhook_test_failed',
Expand Down
2 changes: 1 addition & 1 deletion plugins/wp-graphql-webhooks/src/Services/PluginServiceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function has(string $name): bool {

public function get(string $name) {
if (!isset($this->factories[$name])) {
throw new UnexpectedValueException("Service not found: {$name}");
throw new UnexpectedValueException( esc_html( "Service not found: {$name}" ) );
Comment on lines 28 to +29
Copy link
Preview

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using esc_html inside exception logic is meant for HTML output escaping; consider sanitizing the input earlier (e.g., with sanitize_text_field) or omitting HTML-specific escaping here.

Copilot uses AI. Check for mistakes.

}

if (!isset($this->instances[$name])) {
Expand Down