diff --git a/includes/class-pattern-builder-api.php b/includes/class-pattern-builder-api.php index 103dc99..cda875a 100644 --- a/includes/class-pattern-builder-api.php +++ b/includes/class-pattern-builder-api.php @@ -10,6 +10,7 @@ require_once __DIR__ . '/class-pattern-builder-abstract-pattern.php'; require_once __DIR__ . '/class-pattern-builder-controller.php'; +require_once __DIR__ . '/class-pattern-builder-security.php'; class Pattern_Builder_API { @@ -58,7 +59,6 @@ public function register_routes(): void { 'permission_callback' => array( $this, 'write_permission_callback' ), ) ); - } /** @@ -112,11 +112,11 @@ public function write_permission_callback( $request ) { */ public function process_theme_patterns( WP_REST_Request $request ): WP_REST_Response { - $localize = $request->get_param( 'localize' ) === 'true'; + $localize = $request->get_param( 'localize' ) === 'true'; $import_images = $request->get_param( 'importImages' ) !== 'false'; $options = array( - 'localize' => $localize, + 'localize' => $localize, 'import_images' => $import_images, ); @@ -124,37 +124,37 @@ public function process_theme_patterns( WP_REST_Request $request ): WP_REST_Resp $theme_patterns = $this->controller->get_block_patterns_from_theme_files(); $processed_count = 0; - $error_count = 0; - $errors = array(); + $error_count = 0; + $errors = array(); foreach ( $theme_patterns as $pattern ) { try { $this->controller->update_theme_pattern( $pattern, $options ); - $processed_count++; + ++$processed_count; } catch ( Exception $e ) { - $error_count++; + ++$error_count; $errors[] = array( 'pattern' => $pattern->name, - 'error' => $e->getMessage(), + 'error' => $e->getMessage(), ); } } $total_patterns = count( $theme_patterns ); - $success = $error_count === 0; + $success = $error_count === 0; $response_data = array( - 'success' => $success, - 'message' => sprintf( + 'success' => $success, + 'message' => sprintf( /* translators: 1: Number of patterns processed, 2: Total number of patterns */ __( 'Processed %1$d of %2$d theme patterns successfully.', 'pattern-builder' ), $processed_count, $total_patterns ), - 'stats' => array( - 'total' => $total_patterns, + 'stats' => array( + 'total' => $total_patterns, 'processed' => $processed_count, - 'errors' => $error_count, + 'errors' => $error_count, ), 'settings' => $options, ); @@ -199,7 +199,7 @@ function ( $pattern ) { public function inject_theme_patterns( $response, $server, $request ) { // Requesting a single pattern. Inject the synced theme pattern. if ( preg_match( '#/wp/v2/blocks/(?P\d+)#', $request->get_route(), $matches ) ) { - $block_id = intval( $matches['id'] ); + $block_id = intval( $matches['id'] ); $tbell_pattern_block = get_post( $block_id ); if ( $tbell_pattern_block && $tbell_pattern_block->post_type === 'tbell_pattern_block' ) { // make sure the pattern has a pattern file @@ -208,8 +208,8 @@ public function inject_theme_patterns( $response, $server, $request ) { return $response; // No pattern file found, return the original response } $tbell_pattern_block->post_name = $this->controller->format_pattern_slug_from_post( $tbell_pattern_block->post_name ); - $data = $this->format_tbell_pattern_block_response( $tbell_pattern_block, $request ); - $response = new WP_REST_Response( $data ); + $data = $this->format_tbell_pattern_block_response( $tbell_pattern_block, $request ); + $response = new WP_REST_Response( $data ); } } @@ -326,7 +326,6 @@ public function register_patterns() { * * Filters delete calls and if the item being deleted is a 'tbell_pattern_block' (theme pattern) * delete the related pattern php file as well. - * */ function handle_hijack_block_delete( $response, $server, $request ) { @@ -353,10 +352,21 @@ function handle_hijack_block_delete( $response, $server, $request ) { return new WP_Error( 'pattern_not_found', 'Pattern not found', array( 'status' => 404 ) ); } - $deleted = wp_delete_file( $path ); + // Validate that the path is within the patterns directory + $validation = \Pattern_Builder_Security::validate_pattern_path( $path ); + if ( is_wp_error( $validation ) ) { + return $validation; + } - if ( ! $deleted ) { - return new WP_Error( 'pattern_delete_failed', 'Failed to delete pattern', array( 'status' => 500 ) ); + // Use secure file delete operation + $allowed_dirs = array( + get_stylesheet_directory() . '/patterns', + get_template_directory() . '/patterns', + ); + $deleted = \Pattern_Builder_Security::safe_file_delete( $path, $allowed_dirs ); + + if ( is_wp_error( $deleted ) ) { + return $deleted; } return new WP_REST_Response( array( 'message' => 'Pattern deleted successfully' ), 200 ); @@ -373,116 +383,114 @@ function handle_hijack_block_delete( $response, $server, $request ) { * It updates the pattern file as well as associated metadata for the pattern. * Additionally it will optionally localize the content as well as import any media * referenced by the pattern into the theme. - * */ - function handle_hijack_block_update($response, $handler, $request) - { + function handle_hijack_block_update( $response, $handler, $request ) { $route = $request->get_route(); - if (preg_match('#^/wp/v2/blocks/(\d+)$#', $route, $matches)) { + if ( preg_match( '#^/wp/v2/blocks/(\d+)$#', $route, $matches ) ) { - $id = intval($matches[1]); - $post = get_post($id); + $id = intval( $matches[1] ); + $post = get_post( $id ); - if ($post && $request->get_method() === 'PUT') { + if ( $post && $request->get_method() === 'PUT' ) { - $updated_pattern = json_decode($request->get_body(), true); + $updated_pattern = json_decode( $request->get_body(), true ); $convert_user_pattern_to_theme_pattern = false; - if ($post->post_type === 'wp_block') { + if ( $post->post_type === 'wp_block' ) { - if (isset($updated_pattern['source']) && $updated_pattern['source'] === 'theme') { + if ( isset( $updated_pattern['source'] ) && $updated_pattern['source'] === 'theme' ) { // we are attempting to convert a USER pattern to a THEME pattern. $convert_user_pattern_to_theme_pattern = true; } } - if ($post->post_type === 'tbell_pattern_block' || $convert_user_pattern_to_theme_pattern) { + if ( $post->post_type === 'tbell_pattern_block' || $convert_user_pattern_to_theme_pattern ) { // Check write permissions before allowing update - if (! current_user_can('edit_others_posts')) { + if ( ! current_user_can( 'edit_others_posts' ) ) { return new WP_Error( 'rest_forbidden', - __('You do not have permission to edit patterns.', 'pattern-builder'), - array('status' => 403) + __( 'You do not have permission to edit patterns.', 'pattern-builder' ), + array( 'status' => 403 ) ); } // Verify the REST API nonce - $nonce = $request->get_header('X-WP-Nonce'); - if (! $nonce || ! wp_verify_nonce($nonce, 'wp_rest')) { + $nonce = $request->get_header( 'X-WP-Nonce' ); + if ( ! $nonce || ! wp_verify_nonce( $nonce, 'wp_rest' ) ) { return new WP_Error( 'rest_cookie_invalid_nonce', - __('Cookie nonce is invalid', 'pattern-builder'), - array('status' => 403) + __( 'Cookie nonce is invalid', 'pattern-builder' ), + array( 'status' => 403 ) ); } - $pattern = Abstract_Pattern::from_post($post); + $pattern = Abstract_Pattern::from_post( $post ); - if (isset($updated_pattern['content'])) { + if ( isset( $updated_pattern['content'] ) ) { // remap tbell_pattern_blocks to patterns - $blocks = parse_blocks($updated_pattern['content']); - $blocks = $this->convert_blocks_to_patterns($blocks); - $pattern->content = serialize_blocks($blocks); + $blocks = parse_blocks( $updated_pattern['content'] ); + $blocks = $this->convert_blocks_to_patterns( $blocks ); + $pattern->content = serialize_blocks( $blocks ); // TODO: Format the content to be easy on the eyes. } - if (isset($updated_pattern['title'])) { + if ( isset( $updated_pattern['title'] ) ) { $pattern->title = $updated_pattern['title']; } - if (isset($updated_pattern['excerpt'])) { + if ( isset( $updated_pattern['excerpt'] ) ) { $pattern->description = $updated_pattern['excerpt']; } - if (isset($updated_pattern['wp_pattern_sync_status'])) { + if ( isset( $updated_pattern['wp_pattern_sync_status'] ) ) { $pattern->synced = $updated_pattern['wp_pattern_sync_status'] !== 'unsynced'; } - if (isset($updated_pattern['wp_pattern_block_types'])) { + if ( isset( $updated_pattern['wp_pattern_block_types'] ) ) { $pattern->blockTypes = $updated_pattern['wp_pattern_block_types']; } - if (isset($updated_pattern['wp_pattern_post_types'])) { + if ( isset( $updated_pattern['wp_pattern_post_types'] ) ) { $pattern->postTypes = $updated_pattern['wp_pattern_post_types']; } - if (isset($updated_pattern['wp_pattern_template_types'])) { + if ( isset( $updated_pattern['wp_pattern_template_types'] ) ) { $pattern->templateTypes = $updated_pattern['wp_pattern_template_types']; } - if (isset($updated_pattern['wp_pattern_inserter'])) { + if ( isset( $updated_pattern['wp_pattern_inserter'] ) ) { $pattern->inserter = $updated_pattern['wp_pattern_inserter'] === 'no' ? false : true; } - if (isset($updated_pattern['source']) && $updated_pattern['source'] === 'user') { + if ( isset( $updated_pattern['source'] ) && $updated_pattern['source'] === 'user' ) { // we are attempting to convert a THEME pattern to a USER pattern. - $this->controller->update_user_pattern($pattern); + $this->controller->update_user_pattern( $pattern ); } else { // Check configuration options via query parameters $options = array(); - $localize_param = $request->get_param('patternBuilderLocalize'); - if ($localize_param === 'true') { + $localize_param = $request->get_param( 'patternBuilderLocalize' ); + if ( $localize_param === 'true' ) { $options['localize'] = true; } - $import_images_param = $request->get_param('patternBuilderImportImages'); - if ($import_images_param === 'false') { + $import_images_param = $request->get_param( 'patternBuilderImportImages' ); + if ( $import_images_param === 'false' ) { $options['import_images'] = false; } else { // Default to true if not explicitly disabled $options['import_images'] = true; } - $this->controller->update_theme_pattern($pattern, $options); + $this->controller->update_theme_pattern( $pattern, $options ); } - $post = get_post($pattern->id); - $formatted_response = $this->format_tbell_pattern_block_response($post, $request); - $response = new WP_REST_Response($formatted_response, 200); + $post = get_post( $pattern->id ); + $formatted_response = $this->format_tbell_pattern_block_response( $post, $request ); + $response = new WP_REST_Response( $formatted_response, 200 ); } } } diff --git a/includes/class-pattern-builder-controller.php b/includes/class-pattern-builder-controller.php index d812a77..a77e2ca 100644 --- a/includes/class-pattern-builder-controller.php +++ b/includes/class-pattern-builder-controller.php @@ -7,6 +7,7 @@ require_once __DIR__ . '/class-pattern-builder-abstract-pattern.php'; require_once __DIR__ . '/class-pattern-builder-localization.php'; +require_once __DIR__ . '/class-pattern-builder-security.php'; require_once ABSPATH . 'wp-admin/includes/file.php'; class Pattern_Builder_Controller { @@ -83,38 +84,40 @@ public function create_tbell_pattern_block_post_for_pattern( $pattern ) { } else { delete_post_meta( $post_id, 'wp_pattern_inserter' ); } - if (!$post_id) { + if ( ! $post_id ) { - $post_id = wp_insert_post( - array( - 'post_title' => $pattern->title, - 'post_name' => $this->format_pattern_slug_for_post( $pattern->name ), - 'post_content' => $pattern->content, - 'post_excerpt' => $pattern->description, - 'post_type' => 'tbell_pattern_block', - 'post_status' => 'publish', - 'ping_status' => 'closed', - 'comment_status' => 'closed', - 'meta_input' => $meta, - ), true - ); + $post_id = wp_insert_post( + array( + 'post_title' => $pattern->title, + 'post_name' => $this->format_pattern_slug_for_post( $pattern->name ), + 'post_content' => $pattern->content, + 'post_excerpt' => $pattern->description, + 'post_type' => 'tbell_pattern_block', + 'post_status' => 'publish', + 'ping_status' => 'closed', + 'comment_status' => 'closed', + 'meta_input' => $meta, + ), + true + ); } else { - $post_id = wp_insert_post( - array( - 'ID' => $post_id, - 'post_title' => $pattern->title, - 'post_name' => $this->format_pattern_slug_for_post( $pattern->name ), - 'post_content' => $pattern->content, - 'post_excerpt' => $pattern->description, - 'post_type' => 'tbell_pattern_block', - 'post_status' => 'publish', - 'ping_status' => 'closed', - 'comment_status' => 'closed', - 'meta_input' => $meta, - ), true - ); + $post_id = wp_insert_post( + array( + 'ID' => $post_id, + 'post_title' => $pattern->title, + 'post_name' => $this->format_pattern_slug_for_post( $pattern->name ), + 'post_content' => $pattern->content, + 'post_excerpt' => $pattern->description, + 'post_type' => 'tbell_pattern_block', + 'post_status' => 'publish', + 'ping_status' => 'closed', + 'comment_status' => 'closed', + 'meta_input' => $meta, + ), + true + ); } @@ -128,79 +131,78 @@ public function create_tbell_pattern_block_post_for_pattern( $pattern ) { return $post; } - public function update_theme_pattern(Abstract_Pattern $pattern, $options = array()) - { + public function update_theme_pattern( Abstract_Pattern $pattern, $options = array() ) { // get the tbell_pattern_block post if it already exists - $post = get_page_by_path($this->format_pattern_slug_for_post($pattern->name), OBJECT, array('tbell_pattern_block', 'wp_block')); + $post = get_page_by_path( $this->format_pattern_slug_for_post( $pattern->name ), OBJECT, array( 'tbell_pattern_block', 'wp_block' ) ); - if ($post && $post->post_type === 'wp_block') { + if ( $post && $post->post_type === 'wp_block' ) { // this is being converted to theme patterns, change the slug to include the theme domain $pattern->name = get_stylesheet() . '/' . $pattern->name; } // Check if image importing is enabled (default to true for backward compatibility) - if (! isset($options['import_images']) || $options['import_images'] === true) { - $pattern = $this->import_pattern_image_assets($pattern); + if ( ! isset( $options['import_images'] ) || $options['import_images'] === true ) { + $pattern = $this->import_pattern_image_assets( $pattern ); } // Check if localization is enabled - if (isset($options['localize']) && $options['localize'] === true) { - $pattern = Pattern_Builder_Localization::localize_pattern_content($pattern); + if ( isset( $options['localize'] ) && $options['localize'] === true ) { + $pattern = Pattern_Builder_Localization::localize_pattern_content( $pattern ); } // update the pattern file - $this->update_theme_pattern_file($pattern); + $this->update_theme_pattern_file( $pattern ); // rebuild the pattern from the file (so that the content has no PHP tags) - $pattern = Abstract_Pattern::from_file($this->get_pattern_filepath($pattern)); + $pattern = Abstract_Pattern::from_file( $this->get_pattern_filepath( $pattern ) ); $post_id = wp_update_post( array( 'ID' => $post ? $post->ID : null, 'post_title' => $pattern->title, - 'post_name' => $this->format_pattern_slug_for_post($pattern->name), + 'post_name' => $this->format_pattern_slug_for_post( $pattern->name ), 'post_excerpt' => $pattern->description, 'post_content' => $pattern->content, 'post_type' => 'tbell_pattern_block', ) ); - if ($pattern->synced) { - delete_post_meta($post_id, 'wp_pattern_sync_status'); + if ( $pattern->synced ) { + delete_post_meta( $post_id, 'wp_pattern_sync_status' ); } else { - update_post_meta($post_id, 'wp_pattern_sync_status', 'unsynced'); + update_post_meta( $post_id, 'wp_pattern_sync_status', 'unsynced' ); } - if ($pattern->keywords) { - update_post_meta($post_id, 'wp_pattern_keywords', implode(',', $pattern->keywords)); + if ( $pattern->keywords ) { + update_post_meta( $post_id, 'wp_pattern_keywords', implode( ',', $pattern->keywords ) ); } else { - delete_post_meta($post_id, 'wp_pattern_keywords'); + delete_post_meta( $post_id, 'wp_pattern_keywords' ); } - if ($pattern->blockTypes) { - update_post_meta($post_id, 'wp_pattern_block_types', implode(',', $pattern->blockTypes)); + if ( $pattern->blockTypes ) { + update_post_meta( $post_id, 'wp_pattern_block_types', implode( ',', $pattern->blockTypes ) ); } else { - delete_post_meta($post_id, 'wp_pattern_block_types'); + delete_post_meta( $post_id, 'wp_pattern_block_types' ); } - if ($pattern->templateTypes) { - update_post_meta($post_id, 'wp_pattern_template_types', implode(',', $pattern->templateTypes)); + if ( $pattern->templateTypes ) { + update_post_meta( $post_id, 'wp_pattern_template_types', implode( ',', $pattern->templateTypes ) ); } else { - delete_post_meta($post_id, 'wp_pattern_template_types'); + delete_post_meta( $post_id, 'wp_pattern_template_types' ); } - if ($pattern->postTypes) { - update_post_meta($post_id, 'wp_pattern_post_types', implode(',', $pattern->postTypes)); + if ( $pattern->postTypes ) { + update_post_meta( $post_id, 'wp_pattern_post_types', implode( ',', $pattern->postTypes ) ); } else { - delete_post_meta($post_id, 'wp_pattern_post_types'); + delete_post_meta( $post_id, 'wp_pattern_post_types' ); } // store categories - wp_set_object_terms($post_id, $pattern->categories, 'wp_pattern_category', false); + wp_set_object_terms( $post_id, $pattern->categories, 'wp_pattern_category', false ); return $pattern; - return new WP_Error('premium_required', 'Saving Theme Patterns requires the premium version of Pattern Builder.', array('status' => 403)); + return new WP_Error( 'premium_required', 'Saving Theme Patterns requires the premium version of Pattern Builder.', array( 'status' => 403 ) ); } private function export_pattern_image_assets( $pattern ) { @@ -353,17 +355,38 @@ private function import_pattern_image_assets( $pattern ) { return false; } - $filename = basename( $url ); - $asset_dir = get_stylesheet_directory() . '/assets/images/'; + $filename = \Pattern_Builder_Security::sanitize_filename( basename( $url ) ); + $asset_dir = get_stylesheet_directory() . '/assets/images/'; + $destination_path = $asset_dir . $filename; + + // Validate destination path + $validation = \Pattern_Builder_Security::validate_asset_path( $destination_path ); + if ( is_wp_error( $validation ) ) { + // Clean up the temp file and return false + if ( file_exists( $download_file ) ) { + wp_delete_file( $download_file ); + } + return false; + } + if ( ! is_dir( $asset_dir ) ) { wp_mkdir_p( $asset_dir ); } - global $wp_filesystem; - if ( ! $wp_filesystem ) { - WP_Filesystem(); + // Use secure file move operation + $allowed_dirs = array( + get_stylesheet_directory() . '/assets', + get_template_directory() . '/assets', + ); + $result = \Pattern_Builder_Security::safe_file_move( $download_file, $destination_path, $allowed_dirs ); + + if ( is_wp_error( $result ) ) { + // Clean up the temp file if move failed + if ( file_exists( $download_file ) ) { + wp_delete_file( $download_file ); + } + return false; } - $wp_filesystem->move( $download_file, $asset_dir . $filename ); return '/assets/images/' . $filename; }; @@ -457,7 +480,16 @@ public function update_user_pattern( Abstract_Pattern $pattern ) { if ( $convert_from_theme_pattern ) { $path = $this->get_pattern_filepath( $pattern ); if ( $path ) { - $deleted = wp_delete_file( $path ); + // Validate that the path is within the patterns directory + $validation = \Pattern_Builder_Security::validate_pattern_path( $path ); + if ( ! is_wp_error( $validation ) ) { + // Use secure file delete operation + $allowed_dirs = array( + get_stylesheet_directory() . '/patterns', + get_template_directory() . '/patterns', + ); + $deleted = \Pattern_Builder_Security::safe_file_delete( $path, $allowed_dirs ); + } } } @@ -469,8 +501,12 @@ public function get_block_patterns_from_theme_files() { $patterns = array(); foreach ( $pattern_files as $pattern_file ) { - $pattern = Abstract_Pattern::from_file( $pattern_file ); - $patterns[] = $pattern; + // Validate each pattern file path + $validation = \Pattern_Builder_Security::validate_pattern_path( $pattern_file ); + if ( ! is_wp_error( $validation ) ) { + $pattern = Abstract_Pattern::from_file( $pattern_file ); + $patterns[] = $pattern; + } } return $patterns; @@ -504,7 +540,13 @@ public function delete_user_pattern( Abstract_Pattern $pattern ) { } public function get_pattern_filepath( $pattern ) { - $path = $pattern->filePath ?? get_stylesheet_directory() . '/patterns/' . basename( $pattern->name ) . '.php'; + $path = $pattern->filePath ?? get_stylesheet_directory() . '/patterns/' . \Pattern_Builder_Security::sanitize_filename( basename( $pattern->name ) ); + + // Validate the path before checking existence + $validation = \Pattern_Builder_Security::validate_pattern_path( $path ); + if ( is_wp_error( $validation ) ) { + return null; + } if ( file_exists( $path ) ) { return $path; @@ -518,8 +560,12 @@ function ( $p ) use ( $pattern ) { } ); - if ( $pattern && file_exists( $pattern->filePath ) ) { - return $pattern->filePath; + if ( $pattern && isset( $pattern->filePath ) ) { + // Validate the found path as well + $validation = \Pattern_Builder_Security::validate_pattern_path( $pattern->filePath ); + if ( ! is_wp_error( $validation ) && file_exists( $pattern->filePath ) ) { + return $pattern->filePath; + } } return null; @@ -529,18 +575,29 @@ public function delete_theme_pattern( Abstract_Pattern $pattern ) { $path = $this->get_pattern_filepath( $pattern ); - if ( ! $path ) { - return new WP_Error( 'pattern_not_found', 'Pattern not found', array( 'status' => 404 ) ); - } + if ( ! $path ) { + return new WP_Error( 'pattern_not_found', 'Pattern not found', array( 'status' => 404 ) ); + } - $deleted = wp_delete_file( $path ); + // Validate that the path is within the patterns directory + $validation = \Pattern_Builder_Security::validate_pattern_path( $path ); + if ( is_wp_error( $validation ) ) { + return $validation; + } - if ( ! $deleted ) { - return new WP_Error( 'pattern_delete_failed', 'Failed to delete pattern', array( 'status' => 500 ) ); + // Use secure file delete operation + $allowed_dirs = array( + get_stylesheet_directory() . '/patterns', + get_template_directory() . '/patterns', + ); + $deleted = \Pattern_Builder_Security::safe_file_delete( $path, $allowed_dirs ); + + if ( is_wp_error( $deleted ) ) { + return $deleted; } $tbell_pattern_block_post = $this->get_tbell_pattern_block_post_for_pattern( $pattern ); - $deleted = wp_delete_post( $tbell_pattern_block_post->ID, true ); + $deleted = wp_delete_post( $tbell_pattern_block_post->ID, true ); if ( ! $deleted ) { return new WP_Error( 'pattern_delete_failed', 'Failed to delete pattern', array( 'status' => 500 ) ); @@ -548,23 +605,35 @@ public function delete_theme_pattern( Abstract_Pattern $pattern ) { return array( 'message' => 'Pattern deleted successfully' ); - return new WP_Error( 'premium_required', 'Deleting Theme Patterns requires the premium version of Pattern Builder.', array( 'status' => 403 ) ); + return new WP_Error( 'premium_required', 'Deleting Theme Patterns requires the premium version of Pattern Builder.', array( 'status' => 403 ) ); } public function update_theme_pattern_file( Abstract_Pattern $pattern ) { $path = $this->get_pattern_filepath( $pattern ); if ( ! $path ) { - $filename = basename( $pattern->name ); - $path = get_stylesheet_directory() . '/patterns/' . $filename . '.php'; + $filename = \Pattern_Builder_Security::sanitize_filename( basename( $pattern->name ) ); + $path = get_stylesheet_directory() . '/patterns/' . $filename; + } + + // Validate that the path is within the patterns directory + $validation = \Pattern_Builder_Security::validate_pattern_path( $path ); + if ( is_wp_error( $validation ) ) { + return $validation; } $formatted_content = $this->format_block_markup( $pattern->content ); $file_content = $this->build_pattern_file_metadata( $pattern ) . $formatted_content; - $response = file_put_contents( $path, $file_content ); - if ( ! $response ) { - return new WP_Error( 'file_creation_failed', 'Failed to create pattern file', array( 'status' => 500 ) ); + // Use secure file write operation + $allowed_dirs = array( + get_stylesheet_directory() . '/patterns', + get_template_directory() . '/patterns', + ); + $response = \Pattern_Builder_Security::safe_file_write( $path, $file_content, $allowed_dirs ); + + if ( is_wp_error( $response ) ) { + return $response; } return $pattern; @@ -586,7 +655,7 @@ private function build_pattern_file_metadata( Abstract_Pattern $pattern ): strin $inserter = $pattern->inserter ? '' : "\n * Inserter: no"; $synced = $pattern->synced ? "\n * Synced: yes" : ''; - $metadata = "title\n"; $metadata .= " * Slug: $pattern->name\n"; diff --git a/includes/class-pattern-builder-localization.php b/includes/class-pattern-builder-localization.php index 3da4a7b..4178a87 100644 --- a/includes/class-pattern-builder-localization.php +++ b/includes/class-pattern-builder-localization.php @@ -344,7 +344,7 @@ function ( $matches ) { if ( ! empty( $block['innerContent'] ) && is_array( $block['innerContent'] ) ) { // For details blocks with inner blocks, innerContent typically has: // [0] = opening part with summary, [1] = null (for inner blocks), [2] = closing - + // Find the opening part that contains the summary and update it with localized content foreach ( $block['innerContent'] as $index => $content ) { if ( is_string( $content ) && strpos( $content, ' 400 ) + ); + } + + // Default to theme directory if no allowed directories specified. + if ( empty( $allowed_dirs ) ) { + $allowed_dirs = array( + wp_normalize_path( get_stylesheet_directory() ), + wp_normalize_path( get_template_directory() ), + ); + } else { + // Normalize all allowed directories. + $allowed_dirs = array_map( 'wp_normalize_path', $allowed_dirs ); + } + + // Check if the path starts with any of the allowed directories. + $is_valid = false; + foreach ( $allowed_dirs as $allowed_dir ) { + if ( 0 === strpos( $path, $allowed_dir ) ) { + $is_valid = true; + break; + } + } + + if ( ! $is_valid ) { + return new WP_Error( + 'path_traversal_detected', + __( 'Path traversal attempt detected. Operation blocked.', 'pattern-builder' ), + array( 'status' => 403 ) + ); + } + + // Additional check for suspicious patterns. + if ( preg_match( '/\.\.\/|\.\.\\\\/', $path ) ) { + return new WP_Error( + 'suspicious_path', + __( 'Suspicious path pattern detected. Operation blocked.', 'pattern-builder' ), + array( 'status' => 403 ) + ); + } + + return true; + } + + /** + * Validate pattern file path specifically. + * + * @param string $path The pattern file path to validate. + * @return bool|WP_Error True if valid, WP_Error otherwise. + */ + public static function validate_pattern_path( $path ) { + // Pattern files should only be in the patterns directory. + $allowed_dirs = array( + wp_normalize_path( get_stylesheet_directory() . '/patterns' ), + wp_normalize_path( get_template_directory() . '/patterns' ), + ); + + $validation = self::validate_file_path( $path, $allowed_dirs ); + + if ( is_wp_error( $validation ) ) { + return $validation; + } + + // Ensure it's a PHP file. + if ( '.php' !== substr( $path, -4 ) ) { + return new WP_Error( + 'invalid_file_type', + __( 'Pattern files must be PHP files.', 'pattern-builder' ), + array( 'status' => 400 ) + ); + } + + return true; + } + + /** + * Validate asset file path. + * + * @param string $path The asset file path to validate. + * @return bool|WP_Error True if valid, WP_Error otherwise. + */ + public static function validate_asset_path( $path ) { + // Assets should only be in the assets directory. + $allowed_dirs = array( + wp_normalize_path( get_stylesheet_directory() . '/assets' ), + wp_normalize_path( get_template_directory() . '/assets' ), + ); + + return self::validate_file_path( $path, $allowed_dirs ); + } + + /** + * Initialize WordPress Filesystem. + * + * @return bool|WP_Error True on success, WP_Error on failure. + */ + public static function init_filesystem() { + global $wp_filesystem; + + if ( ! function_exists( 'WP_Filesystem' ) ) { + require_once ABSPATH . 'wp-admin/includes/file.php'; + } + + if ( ! WP_Filesystem() ) { + return new WP_Error( + 'filesystem_init_failed', + __( 'Failed to initialize WordPress filesystem.', 'pattern-builder' ), + array( 'status' => 500 ) + ); + } + + return true; + } + + /** + * Safely write content to a file using WordPress Filesystem API. + * + * @param string $path The file path. + * @param string $content The content to write. + * @param array $allowed_dirs Optional. Allowed directories for the file. + * @return bool|WP_Error True on success, WP_Error on failure. + */ + public static function safe_file_write( $path, $content, $allowed_dirs = array() ) { + // Validate the path first. + $validation = self::validate_file_path( $path, $allowed_dirs ); + if ( is_wp_error( $validation ) ) { + return $validation; + } + + // Initialize filesystem. + $fs_init = self::init_filesystem(); + if ( is_wp_error( $fs_init ) ) { + return $fs_init; + } + + global $wp_filesystem; + + // Ensure directory exists. + $dir = dirname( $path ); + if ( ! $wp_filesystem->is_dir( $dir ) ) { + if ( ! wp_mkdir_p( $dir ) ) { + return new WP_Error( + 'directory_creation_failed', + __( 'Failed to create directory.', 'pattern-builder' ), + array( 'status' => 500 ) + ); + } + } + + // Write the file. + $result = $wp_filesystem->put_contents( $path, $content, FS_CHMOD_FILE ); + + if ( false === $result ) { + return new WP_Error( + 'file_write_failed', + __( 'Failed to write file.', 'pattern-builder' ), + array( 'status' => 500 ) + ); + } + + return true; + } + + /** + * Safely delete a file using WordPress Filesystem API. + * + * @param string $path The file path to delete. + * @param array $allowed_dirs Optional. Allowed directories for the file. + * @return bool|WP_Error True on success, WP_Error on failure. + */ + public static function safe_file_delete( $path, $allowed_dirs = array() ) { + // Validate the path first. + $validation = self::validate_file_path( $path, $allowed_dirs ); + if ( is_wp_error( $validation ) ) { + return $validation; + } + + // Initialize filesystem. + $fs_init = self::init_filesystem(); + if ( is_wp_error( $fs_init ) ) { + return $fs_init; + } + + global $wp_filesystem; + + // Check if file exists. + if ( ! $wp_filesystem->exists( $path ) ) { + return new WP_Error( + 'file_not_found', + __( 'File not found.', 'pattern-builder' ), + array( 'status' => 404 ) + ); + } + + // Delete the file. + $result = $wp_filesystem->delete( $path ); + + if ( false === $result ) { + return new WP_Error( + 'file_delete_failed', + __( 'Failed to delete file.', 'pattern-builder' ), + array( 'status' => 500 ) + ); + } + + return true; + } + + /** + * Safely move a file using WordPress Filesystem API. + * + * @param string $source The source file path. + * @param string $destination The destination file path. + * @param array $allowed_dirs Optional. Allowed directories for both paths. + * @return bool|WP_Error True on success, WP_Error on failure. + */ + public static function safe_file_move( $source, $destination, $allowed_dirs = array() ) { + // Validate both paths. + $source_validation = self::validate_file_path( $source, $allowed_dirs ); + if ( is_wp_error( $source_validation ) ) { + return $source_validation; + } + + $dest_validation = self::validate_file_path( $destination, $allowed_dirs ); + if ( is_wp_error( $dest_validation ) ) { + return $dest_validation; + } + + // Initialize filesystem. + $fs_init = self::init_filesystem(); + if ( is_wp_error( $fs_init ) ) { + return $fs_init; + } + + global $wp_filesystem; + + // Ensure destination directory exists. + $dest_dir = dirname( $destination ); + if ( ! $wp_filesystem->is_dir( $dest_dir ) ) { + if ( ! wp_mkdir_p( $dest_dir ) ) { + return new WP_Error( + 'directory_creation_failed', + __( 'Failed to create destination directory.', 'pattern-builder' ), + array( 'status' => 500 ) + ); + } + } + + // Move the file. + $result = $wp_filesystem->move( $source, $destination, true ); + + if ( false === $result ) { + return new WP_Error( + 'file_move_failed', + __( 'Failed to move file.', 'pattern-builder' ), + array( 'status' => 500 ) + ); + } + + return true; + } + + /** + * Sanitize a filename for safe use. + * + * @param string $filename The filename to sanitize. + * @return string Sanitized filename. + */ + public static function sanitize_filename( $filename ) { + // Use WordPress built-in sanitization. + $filename = sanitize_file_name( $filename ); + + // Remove any remaining directory traversal attempts. + $filename = str_replace( array( '..', '/', '\\' ), '', $filename ); + + // Ensure it has a valid extension for patterns. + if ( ! preg_match( '/\.(php|json|html?)$/i', $filename ) ) { + $filename .= '.php'; + } + + return $filename; + } +} diff --git a/suggestions.md b/suggestions.md new file mode 100644 index 0000000..d5d1bca --- /dev/null +++ b/suggestions.md @@ -0,0 +1,175 @@ +# Pattern Builder Plugin - Code Review Suggestions + +## Critical Security Issues + +### 1. Direct File System Operations Without Proper Validation +**Location:** `class-pattern-builder-controller.php:152, 355-357` +- **Issue:** Using `wp_delete_file()` directly on user-controllable paths without sufficient validation +- **Recommendation:** + - Add path traversal protection by validating paths are within expected directories + - Use `wp_normalize_path()` and check paths start with theme directory + - Consider using WordPress filesystem API instead of direct file operations + +### 2. Insufficient Input Sanitization in REST API +**Location:** `class-pattern-builder-api.php:388-459` +- **Issue:** User input from REST requests not consistently sanitized before database operations +- **Recommendation:** + - Add `sanitize_text_field()` for title fields + - Use `wp_kses_post()` for content that allows HTML + - Sanitize array inputs with `array_map()` and appropriate sanitization functions + +### 3. Missing Capability Checks +**Location:** `class-pattern-builder-post-type.php:131-138` +- **Issue:** Custom capabilities added to roles but never properly verified in critical operations +- **Recommendation:** + - Add capability checks before file write/delete operations + - Verify `edit_theme_options` capability for theme pattern modifications + - Use `current_user_can()` checks consistently throughout the codebase + +## WordPress Coding Standards + +### 1. Inconsistent Error Handling +**Location:** Multiple files +- **Issue:** Mix of WP_Error returns, exceptions, and silent failures +- **Recommendation:** + - Standardize on WP_Error for all error conditions + - Add proper error logging using `error_log()` for debugging + - Return meaningful error messages to users + +### 2. Missing Nonce Verification in Some Operations +**Location:** `class-pattern-builder-api.php` +- **Issue:** While nonce is checked in write operations, some edge cases may bypass verification +- **Recommendation:** + - Add nonce verification to ALL state-changing operations + - Use `wp_create_nonce()` and `wp_verify_nonce()` consistently + - Consider implementing rate limiting for API endpoints + +### 3. Direct Database Queries +**Location:** `class-pattern-builder-controller.php:27-32` +- **Issue:** Using `get_posts()` with unsanitized parameters +- **Recommendation:** + - Use `WP_Query` with proper argument escaping + - Consider caching query results with transients for performance + - Add indexes to post meta queries if needed + +## Code Quality and Performance + +### 1. Singleton Pattern Implementation +**Location:** `class-pattern-builder.php:14-38` +- **Issue:** Singleton pattern prevents proper unit testing and creates tight coupling +- **Recommendation:** + - Consider dependency injection instead of singleton + - Use WordPress hooks system for initialization + - Make classes testable by avoiding static dependencies + +### 2. Asset Loading Optimization +**Location:** `class-pattern-builder-admin.php:41-74`, `class-pattern-builder-editor.php:14-30` +- **Issue:** Assets loaded on all admin/editor pages without conditional checks +- **Recommendation:** + - Add screen/context checks before enqueueing scripts + - Use `wp_register_script()` first, then conditionally enqueue + - Consider code splitting for large JavaScript bundles + - Add `defer` or `async` attributes where appropriate + +### 3. Missing Asset Versioning Strategy +**Location:** Build process and asset loading +- **Issue:** Asset versions tied to plugin version, not file content +- **Recommendation:** + - Use file hash-based versioning for better cache busting + - Implement proper cache headers for static assets + - Consider using `filemtime()` for development environments + +## Data Handling + +### 1. Unvalidated Post Meta Operations +**Location:** `class-pattern-builder-controller.php:50-85` +- **Issue:** Post meta values inserted without validation +- **Recommendation:** + - Validate meta values before insertion + - Use `register_meta()` with sanitization callbacks + - Add data type validation for arrays and strings + +### 2. Missing Transaction Support +**Location:** `class-pattern-builder-controller.php:86-129` +- **Issue:** Multiple database operations without transaction wrapping +- **Recommendation:** + - Group related database operations + - Add rollback capability for failed operations + - Use WordPress transients for temporary data storage + +### 3. Inefficient Pattern Registry Operations +**Location:** `class-pattern-builder-api.php:282-322` +- **Issue:** Registering/unregistering patterns on every request +- **Recommendation:** + - Cache pattern registration state + - Only re-register when patterns change + - Use WordPress object cache for pattern data + +## JavaScript/React Issues + +### 1. Missing Error Boundaries +**Location:** React components in `src/components/` +- **Issue:** No error boundaries to catch React component errors +- **Recommendation:** + - Add error boundaries around major component trees + - Implement fallback UI for error states + - Log errors to monitoring service + +### 2. State Management Without Optimization +**Location:** `src/utils/store.js` +- **Issue:** Redux store updates trigger unnecessary re-renders +- **Recommendation:** + - Implement `React.memo()` for expensive components + - Use selector memoization with reselect + - Consider using WordPress data stores more efficiently + +### 3. Missing API Error Handling +**Location:** `src/utils/resolvers.js` +- **Issue:** API fetch calls don't handle network errors gracefully +- **Recommendation:** + - Add try-catch blocks around API calls + - Implement retry logic for failed requests + - Show user-friendly error messages + +## Additional Recommendations + +### 1. Add Comprehensive Logging +- Implement debug logging for development +- Add action/filter hooks for extensibility +- Use WordPress debug constants properly + +### 2. Improve Documentation +- Add PHPDoc blocks for all methods +- Document REST API endpoints +- Create developer documentation for hooks/filters + +### 3. Add Unit Tests +- Implement PHPUnit tests for PHP code +- Add Jest tests for JavaScript components +- Set up continuous integration testing + +### 4. Security Headers +- Add Content Security Policy headers +- Implement proper CORS handling +- Add rate limiting to prevent abuse + +### 5. Accessibility Improvements +- Add ARIA labels to interactive elements +- Ensure keyboard navigation works properly +- Test with screen readers + +### 6. Internationalization +- Ensure all strings use proper text domains +- Add context to translatable strings +- Test with RTL languages + +## Priority Implementation Order + +1. **Immediate:** Fix security issues (input sanitization, path validation, capability checks) +2. **High:** Implement proper error handling and nonce verification +3. **Medium:** Optimize asset loading and database queries +4. **Low:** Refactor singleton pattern and add comprehensive testing + +## Conclusion + +The plugin shows good architectural structure but needs security hardening and performance optimization. Focus on the critical security issues first, then improve error handling and WordPress coding standards compliance. The JavaScript code would benefit from better error handling and performance optimization. \ No newline at end of file