diff --git a/admin/section/class-convertkit-admin-section-tools.php b/admin/section/class-convertkit-admin-section-tools.php index e95767f2b..8c368f2ab 100644 --- a/admin/section/class-convertkit-admin-section-tools.php +++ b/admin/section/class-convertkit-admin-section-tools.php @@ -19,10 +19,6 @@ class ConvertKit_Admin_Section_Tools extends ConvertKit_Admin_Section_Base { */ public function __construct() { - // Initialize WP_Filesystem. - require_once ABSPATH . 'wp-admin/includes/file.php'; - WP_Filesystem(); - $this->settings_key = '_wp_convertkit_tools'; // Required for ConvertKit_Settings_Base, but we don't save settings on the Tools screen. $this->name = 'tools'; $this->title = __( 'Tools', 'convertkit' ); @@ -125,8 +121,6 @@ private function maybe_clear_log() { */ private function maybe_download_log() { - global $wp_filesystem; - // Bail if nonce verification fails. if ( ! isset( $_REQUEST['_convertkit_settings_tools_nonce'] ) ) { return; @@ -149,7 +143,8 @@ private function maybe_download_log() { header( 'Content-Disposition: attachment; filename=convertkit-log.txt' ); header( 'Pragma: no-cache' ); header( 'Expires: 0' ); - echo esc_html( $wp_filesystem->get_contents( $log->get_filename() ) ); + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + echo esc_html( (string) file_get_contents( $log->get_filename() ) ); exit(); } @@ -162,8 +157,6 @@ private function maybe_download_log() { */ private function maybe_download_system_info() { - global $wp_filesystem; - // Bail if nonce verification fails. if ( ! isset( $_REQUEST['_convertkit_settings_tools_nonce'] ) ) { return; @@ -181,21 +174,12 @@ private function maybe_download_system_info() { // Get System Info. $system_info = $this->get_system_info(); - // Write contents to temporary file. - $tmpfile = tmpfile(); - $filename = stream_get_meta_data( $tmpfile )['uri']; - $wp_filesystem->put_contents( - $filename, - esc_attr( $system_info ) - ); - // Download. header( 'Content-type: application/octet-stream' ); header( 'Content-Disposition: attachment; filename=convertkit-system-info.txt' ); header( 'Pragma: no-cache' ); header( 'Expires: 0' ); - echo esc_html( $wp_filesystem->get_contents( $filename ) ); - $wp_filesystem->delete( $filename ); + echo esc_html( $system_info ); exit(); } @@ -263,11 +247,6 @@ private function maybe_import_configuration() { return; } - // Allow us to easily interact with the filesystem. - require_once ABSPATH . 'wp-admin/includes/file.php'; - WP_Filesystem(); - global $wp_filesystem; - // Bail if the submit button for importing the configuration was not clicked. if ( ! array_key_exists( 'convertkit-import', $_REQUEST ) ) { return; @@ -284,7 +263,8 @@ private function maybe_import_configuration() { } // Read file. - $json = $wp_filesystem->get_contents( sanitize_text_field( wp_unslash( $_FILES['import']['tmp_name'] ) ) ); + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + $json = file_get_contents( sanitize_text_field( wp_unslash( $_FILES['import']['tmp_name'] ) ) ); // Decode. $import = json_decode( $json, true ); diff --git a/composer.json b/composer.json index 89559f71a..ffb3596f4 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "type": "project", "license": "GPLv3", "require": { - "convertkit/convertkit-wordpress-libraries": "2.6.0", + "convertkit/convertkit-wordpress-libraries": "dev-move-log-location", "wordpress/mcp-adapter": "^0.6.1" }, "require-dev": { diff --git a/tests/EndToEnd/general/plugin-screens/PluginSettingsToolsCest.php b/tests/EndToEnd/general/plugin-screens/PluginSettingsToolsCest.php index 2751a4571..96896fd33 100644 --- a/tests/EndToEnd/general/plugin-screens/PluginSettingsToolsCest.php +++ b/tests/EndToEnd/general/plugin-screens/PluginSettingsToolsCest.php @@ -43,6 +43,47 @@ public function testDebugLogExists(EndToEndTester $I) $I->dontSeeInField('#debug-log-textarea', 'No logs have been generated.'); } + /** + * Test that the log is stored in the uploads directory, and that no log directory or + * files are created in the Plugin's directory when the log is written to, as any file + * that isn't part of the WordPress.org release causes `wp plugin verify-checksums` to + * report the Plugin as modified. + * + * @since 3.4.0 + * + * @param EndToEndTester $I Tester. + */ + public function testDebugLogNotStoredInPluginDirectory(EndToEndTester $I) + { + $I->setupKitPlugin($I); + $I->setupKitPluginResources($I); + + // Load settings screen to trigger some API requests, which are written to the log. + $I->loadKitSettingsGeneralScreen($I); + + // Load tools screen, which reads the log. + $I->loadKitSettingsToolsScreen($I); + + // Confirm the Debug Log section is populated, so we know the log was written to. + $I->dontSeeInField('#debug-log-textarea', 'No logs have been generated.'); + + // Confirm no log directory or files exist in the Plugin's directory. + $I->dontSeePluginFileFound('convertkit/log/log.txt'); + $I->dontSeePluginFileFound('convertkit/log/.htaccess'); + $I->dontSeePluginFileFound('convertkit/log/index.html'); + $I->dontSeePluginFileFound('convertkit/log'); + $I->dontSeePluginFileFound('convertkit/log.txt'); + + // Confirm the log directory was created in the uploads directory, with the + // .htaccess and index.html files that prevent listing and access on Apache. + $I->seeUploadedFileFound('kit-logs/.htaccess'); + $I->seeUploadedFileFound('kit-logs/index.html'); + + // Confirm the Tools screen displays the log file's location, which is in the + // uploads directory and named after the Plugin's directory. + $I->seeInSource('/wp-content/uploads/kit-logs/convertkit-'); + } + /** * Test that the Download Log option works. * diff --git a/tests/EndToEnd/general/uninstall/UninstallCest.php b/tests/EndToEnd/general/uninstall/UninstallCest.php index ad968104c..b176f8bb4 100644 --- a/tests/EndToEnd/general/uninstall/UninstallCest.php +++ b/tests/EndToEnd/general/uninstall/UninstallCest.php @@ -12,8 +12,12 @@ class UninstallCest { /** - * Test that the Plugin's access and refresh tokens are revoked, and all v4 and v4 - * API credentials are removed from the Plugin's settings when the Plugin is deleted. + * Test that the Plugin's access and refresh tokens are revoked, all v4 and v4 + * API credentials are removed from the Plugin's settings, and the log file is + * deleted from the uploads directory when the Plugin is deleted. + * + * These assertions are deliberately made in a single test, as deleting the Plugin + * is destructive; the Plugin is no longer available to any subsequent test. * * @since 3.2.4 * @@ -57,6 +61,19 @@ public function testPluginDeletionRevokesAndRemovesTokens(EndToEndTester $I) ] ); + // Load the Settings screen, to make API requests that are written to the log file. + $I->loadKitSettingsGeneralScreen($I); + + // Load the Tools screen, and grab the log file's location from it. The log file's + // name includes a hash, so it cannot be determined by this test. + $I->loadKitSettingsToolsScreen($I); + $logFile = $I->grabTextFrom('#debug-log code'); + + // Confirm the log file exists in the uploads directory, so that the assertion + // following Plugin deletion is meaningful. + $I->assertStringContainsString('/wp-content/uploads/kit-logs/', $logFile); + $I->seeFileFound($logFile); + // Deactivate the Plugin. $I->deactivateKitPlugin($I); @@ -66,6 +83,9 @@ public function testPluginDeletionRevokesAndRemovesTokens(EndToEndTester $I) // Allow the uninstallation routine time to complete. $I->wait(10); + // Confirm the log file has been deleted from the uploads directory. + $I->dontSeeFileFound($logFile); + // Confirm the credentials have been removed from the Plugin's settings. $settings = $I->grabOptionFromDatabase('_wp_convertkit_settings'); $I->assertEmpty($settings['access_token']); diff --git a/uninstall.php b/uninstall.php index b89185b36..23f56cb7c 100644 --- a/uninstall.php +++ b/uninstall.php @@ -16,6 +16,21 @@ // are not reliably available due to the Plugin being deactivated and going // through deletion now. +// Delete the log file from the uploads directory. +// The Plugin's own directory is deleted by WordPress once this routine completes, +// so any historic log file stored there doesn't need deleting here. +// This mirrors ConvertKit_Log::get_log_file_name() in the Kit WordPress Libraries, +// which we can't call as the Plugin's classes aren't reliably available. +$upload_dir = wp_upload_dir(); +if ( empty( $upload_dir['error'] ) && ! empty( $upload_dir['basedir'] ) ) { + $log_slug = sanitize_key( basename( __DIR__ ) ); + $log_file = trailingslashit( $upload_dir['basedir'] ) . 'kit-logs/' . $log_slug . '-' . wp_hash( $log_slug ) . '.log'; + + if ( file_exists( $log_file ) ) { + wp_delete_file( $log_file ); + } +} + // Get settings. $settings = get_option( '_wp_convertkit_settings' );