From 39443916dd8b72a177dee832f3b267e4be170f2d Mon Sep 17 00:00:00 2001 From: altugank Date: Tue, 9 Jun 2026 16:29:45 +0300 Subject: [PATCH] security: trusted-proxy rate limiting, atomic restore, encrypted webhook secret Follow-ups from the security review, each verified in a live WP 7.0 install. 1. Trusted-proxy client IP (rate limiting was keyed on REMOTE_ADDR): - New WP_Puller_Client_IP resolves the real client behind reverse proxies and CDNs. Forwarding headers (CF-Connecting-IP, True-Client-IP, X-Forwarded-For) are honoured ONLY when REMOTE_ADDR is itself a trusted proxy, so they cannot be spoofed to evade the limiter. - Cloudflare ranges supported out of the box (fetched weekly + hard-coded fallback); loopback/RFC1918 covered for a site's own nginx/HAProxy. - Extensible via `wp_puller_trusted_proxies` / `wp_puller_client_ip_headers` for Sucuri, Akamai, custom load balancers, etc. IPv4 + IPv6 CIDR matching. 2. Atomic theme restore (restore_backup deleted the live theme before copying): - Stage the restored copy in a dot-prefixed sibling dir, then swap via fast renames with rollback. A failed restore can no longer brick the theme. 3. Webhook secret encrypted at rest: - Stored via the existing WordPress salt-derived WP_Puller::encrypt() (the same mechanism as the GitHub PAT). Reads are backward compatible with legacy plaintext secrets, which are transparently upgraded on next save or admin page view. The secret is still shown decrypted for GitHub setup. Co-Authored-By: Claude Opus 4.8 (1M context) --- wp-puller/includes/class-admin.php | 2 +- wp-puller/includes/class-backup.php | 44 +++- wp-puller/includes/class-client-ip.php | 263 +++++++++++++++++++ wp-puller/includes/class-webhook-handler.php | 61 ++++- wp-puller/includes/class-wp-puller.php | 1 + wp-puller/wp-puller.php | 3 +- 6 files changed, 362 insertions(+), 12 deletions(-) create mode 100644 wp-puller/includes/class-client-ip.php diff --git a/wp-puller/includes/class-admin.php b/wp-puller/includes/class-admin.php index 5cdf133..81fe7ed 100644 --- a/wp-puller/includes/class-admin.php +++ b/wp-puller/includes/class-admin.php @@ -344,7 +344,7 @@ public function ajax_regenerate_secret() { $this->verify_ajax_request(); $new_secret = WP_Puller_Webhook_Handler::generate_secret(); - update_option( 'wp_puller_webhook_secret', $new_secret ); + WP_Puller_Webhook_Handler::store_secret( $new_secret ); $this->logger->log( __( 'Webhook secret regenerated', 'wp-puller' ), diff --git a/wp-puller/includes/class-backup.php b/wp-puller/includes/class-backup.php index 7887ba4..48c7caf 100644 --- a/wp-puller/includes/class-backup.php +++ b/wp-puller/includes/class-backup.php @@ -134,23 +134,57 @@ public function restore_backup( $backup_name ) { ); } + global $wp_filesystem; + if ( ! $wp_filesystem ) { + require_once ABSPATH . 'wp-admin/includes/file.php'; + WP_Filesystem(); + } + $theme = wp_get_theme(); $theme_dir = $theme->get_stylesheet_directory(); + $parent = dirname( $theme_dir ); + $suffix = wp_generate_password( 8, false ); + + // Staging/old dirs live next to the theme (same filesystem, so the + // swap below is a fast rename) and are dot-prefixed so WordPress's + // theme scanner ignores them while they briefly exist. + $staging = $parent . '/.wp-puller-restore-' . $suffix; + $old_dir = $parent . '/.wp-puller-old-' . $suffix; + + // 1. Build the restored copy in staging first. If anything fails here, + // the live theme has not been touched. + if ( ! $this->recursive_copy( $backup_path, $staging ) ) { + $this->recursive_delete( $staging ); + return new WP_Error( + 'restore_failed', + __( 'Failed to stage backup for restore.', 'wp-puller' ) + ); + } - if ( ! $this->recursive_delete( $theme_dir ) ) { + // 2. Move the current theme aside (kept for rollback). + if ( is_dir( $theme_dir ) && ! $wp_filesystem->move( $theme_dir, $old_dir ) ) { + $this->recursive_delete( $staging ); return new WP_Error( - 'delete_failed', - __( 'Failed to remove current theme files.', 'wp-puller' ) + 'restore_failed', + __( 'Failed to set the current theme aside for restore.', 'wp-puller' ) ); } - if ( ! $this->recursive_copy( $backup_path, $theme_dir ) ) { + // 3. Move staging into place. On failure, roll the original back. + if ( ! $wp_filesystem->move( $staging, $theme_dir ) ) { + if ( is_dir( $old_dir ) ) { + $wp_filesystem->move( $old_dir, $theme_dir ); + } + $this->recursive_delete( $staging ); return new WP_Error( 'restore_failed', - __( 'Failed to restore theme from backup.', 'wp-puller' ) + __( 'Failed to activate the restored theme; the original was kept.', 'wp-puller' ) ); } + // 4. Success: discard the old copy. + $this->recursive_delete( $old_dir ); + return true; } diff --git a/wp-puller/includes/class-client-ip.php b/wp-puller/includes/class-client-ip.php new file mode 100644 index 0000000..135cc32 --- /dev/null +++ b/wp-puller/includes/class-client-ip.php @@ -0,0 +1,263 @@ + 5 ) ); + + if ( is_wp_error( $resp ) || 200 !== wp_remote_retrieve_response_code( $resp ) ) { + return array(); + } + + $body = trim( wp_remote_retrieve_body( $resp ) ); + + foreach ( preg_split( '/\s+/', $body ) as $cidr ) { + $cidr = trim( $cidr ); + if ( '' !== $cidr && false !== strpos( $cidr, '/' ) ) { + $out[] = $cidr; + } + } + } + + return $out; + } + + /** + * Hard-coded fallback Cloudflare ranges (https://www.cloudflare.com/ips/). + * + * @return string[] + */ + private static function cloudflare_fallback() { + return array( + '173.245.48.0/20', + '103.21.244.0/22', + '103.22.200.0/22', + '103.31.4.0/22', + '141.101.64.0/18', + '108.162.192.0/18', + '190.93.240.0/20', + '188.114.96.0/20', + '197.234.240.0/22', + '198.41.128.0/17', + '162.158.0.0/15', + '104.16.0.0/13', + '104.24.0.0/14', + '172.64.0.0/13', + '131.0.72.0/22', + '2400:cb00::/32', + '2606:4700::/32', + '2803:f800::/32', + '2405:b500::/32', + '2405:8100::/32', + '2a06:98c0::/29', + '2c0f:f248::/32', + ); + } + + /** + * Validate an IPv4/IPv6 address. + * + * @param string $ip IP address. + * @return bool + */ + public static function is_valid_ip( $ip ) { + return false !== filter_var( $ip, FILTER_VALIDATE_IP ); + } + + /** + * Is $ip inside any of the given CIDR ranges? + * + * @param string $ip IP address. + * @param string[] $ranges CIDR ranges. + * @return bool + */ + public static function ip_in_ranges( $ip, $ranges ) { + foreach ( (array) $ranges as $range ) { + if ( self::ip_in_cidr( $ip, $range ) ) { + return true; + } + } + + return false; + } + + /** + * CIDR membership test supporting both IPv4 and IPv6. + * + * @param string $ip IP address. + * @param string $cidr CIDR range or bare IP. + * @return bool + */ + public static function ip_in_cidr( $ip, $cidr ) { + if ( false === strpos( $cidr, '/' ) ) { + return $ip === $cidr; + } + + list( $subnet, $bits ) = explode( '/', $cidr, 2 ); + $bits = (int) $bits; + + $ip_bin = @inet_pton( $ip ); + $subnet_bin = @inet_pton( $subnet ); + + // Both must parse and belong to the same family (4 vs 16 bytes). + if ( false === $ip_bin || false === $subnet_bin || strlen( $ip_bin ) !== strlen( $subnet_bin ) ) { + return false; + } + + $whole = intdiv( $bits, 8 ); + $rem = $bits % 8; + + if ( $whole > 0 && 0 !== substr_compare( $ip_bin, $subnet_bin, 0, $whole ) ) { + return false; + } + + if ( $rem > 0 ) { + $mask = chr( ( 0xff << ( 8 - $rem ) ) & 0xff ); + if ( ( ord( $ip_bin[ $whole ] ) & ord( $mask ) ) !== ( ord( $subnet_bin[ $whole ] ) & ord( $mask ) ) ) { + return false; + } + } + + return true; + } +} diff --git a/wp-puller/includes/class-webhook-handler.php b/wp-puller/includes/class-webhook-handler.php index a904976..30fe54f 100644 --- a/wp-puller/includes/class-webhook-handler.php +++ b/wp-puller/includes/class-webhook-handler.php @@ -278,7 +278,10 @@ private function handle_push_event( $payload ) { * @return bool True if within limit, false if exceeded. */ private function check_rate_limit() { - $ip = isset( $_SERVER['REMOTE_ADDR'] ) ? sanitize_text_field( wp_unslash( $_SERVER['REMOTE_ADDR'] ) ) : ''; + // Resolve the real client IP behind trusted reverse proxies / CDNs + // (Cloudflare, Sucuri, a site's own nginx, etc.) so the limit tracks + // the actual caller rather than a shared proxy address. + $ip = WP_Puller_Client_IP::get(); if ( empty( $ip ) ) { return true; @@ -305,7 +308,7 @@ private function check_rate_limit() { * @return bool */ private function verify_signature( $payload, $signature ) { - $secret = get_option( 'wp_puller_webhook_secret', '' ); + $secret = self::get_secret(); if ( empty( $secret ) ) { return false; @@ -317,7 +320,7 @@ private function verify_signature( $payload, $signature ) { } /** - * Generate a new webhook secret. + * Generate a new webhook secret (plaintext). * * @return string */ @@ -325,14 +328,62 @@ public static function generate_secret() { return wp_generate_password( 32, false ); } + /** + * Get the plaintext webhook secret. + * + * The secret is stored encrypted at rest using WordPress salt-derived + * encryption (see WP_Puller::encrypt). Legacy installs stored it in + * plaintext; those are still read transparently for backward + * compatibility (and upgraded on the next save / admin page view). + * + * @return string + */ + public static function get_secret() { + $stored = get_option( 'wp_puller_webhook_secret', '' ); + + if ( '' === $stored ) { + return ''; + } + + // Encrypted values carry the WP_Puller::encrypt() version prefix. + if ( 0 === strpos( $stored, 'v2:' ) ) { + return WP_Puller::decrypt( $stored ); + } + + // Legacy plaintext secret. + return $stored; + } + + /** + * Store a webhook secret, encrypting it at rest. + * + * Falls back to plaintext only if encryption is unavailable, so the secret + * is never lost. + * + * @param string $plain Plaintext secret. + * @return void + */ + public static function store_secret( $plain ) { + $encrypted = WP_Puller::encrypt( $plain ); + update_option( 'wp_puller_webhook_secret', '' !== $encrypted ? $encrypted : $plain ); + } + /** * Get webhook configuration instructions. * * @return array */ public static function get_setup_instructions() { - $webhook_url = self::get_webhook_url(); - $webhook_secret = get_option( 'wp_puller_webhook_secret', '' ); + $webhook_url = self::get_webhook_url(); + + // Upgrade a legacy plaintext secret to encrypted-at-rest the first + // time an admin views this page, then read it back decrypted. + $raw = get_option( 'wp_puller_webhook_secret', '' ); + if ( '' !== $raw && 0 !== strpos( $raw, 'v2:' ) ) { + self::store_secret( $raw ); + } + + $webhook_secret = self::get_secret(); return array( 'url' => $webhook_url, diff --git a/wp-puller/includes/class-wp-puller.php b/wp-puller/includes/class-wp-puller.php index 1e8319c..87b5eb7 100644 --- a/wp-puller/includes/class-wp-puller.php +++ b/wp-puller/includes/class-wp-puller.php @@ -101,6 +101,7 @@ public function __construct() { */ private function includes() { require_once WP_PULLER_PLUGIN_DIR . 'includes/class-logger.php'; + require_once WP_PULLER_PLUGIN_DIR . 'includes/class-client-ip.php'; require_once WP_PULLER_PLUGIN_DIR . 'includes/class-github-api.php'; require_once WP_PULLER_PLUGIN_DIR . 'includes/class-backup.php'; require_once WP_PULLER_PLUGIN_DIR . 'includes/class-theme-updater.php'; diff --git a/wp-puller/wp-puller.php b/wp-puller/wp-puller.php index a305945..f79d267 100644 --- a/wp-puller/wp-puller.php +++ b/wp-puller/wp-puller.php @@ -43,7 +43,8 @@ function wp_puller() { */ function wp_puller_activate() { if ( ! get_option( 'wp_puller_webhook_secret' ) ) { - update_option( 'wp_puller_webhook_secret', wp_generate_password( 32, false ) ); + // Stored encrypted at rest via WordPress salt-derived encryption. + update_option( 'wp_puller_webhook_secret', WP_Puller::encrypt( wp_generate_password( 32, false ) ) ); } if ( false === get_option( 'wp_puller_branch' ) ) {