From fe5341a15e296fff25d2fa91e53735f7339c38ef Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Fri, 30 Oct 2026 15:34:34 -0400 Subject: [PATCH 1/6] feat(logging): add bulk CSV download for selected log entries Adds a "Download Selected" bulk action to the logs list. Selected entries are streamed to php://output as a single CSV, fetched one at a time so memory stays flat, and capped by the filterable wpgraphql_logging_bulk_download_limit (default 1000). The single-entry download now shares the same header/stream/write code path. --- .../View/Download/DownloadLogService.php | 156 +++++++++++++++--- .../src/Admin/View/List/ListTable.php | 43 ++++- .../src/Admin/ViewLogsPage.php | 4 + .../tests/e2e/specs/bulk-download.spec.js | 75 +++++++++ .../View/Download/DownloadLogServiceTest.php | 129 +++++++++++++++ .../wpunit/Admin/View/List/ListTableTest.php | 59 +++++++ 6 files changed, 445 insertions(+), 21 deletions(-) create mode 100644 plugins/wpgraphql-logging/tests/e2e/specs/bulk-download.spec.js diff --git a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php index 0bcecbdb9..b053f4bf3 100644 --- a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php +++ b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php @@ -4,6 +4,7 @@ namespace WPGraphQL\Logging\Admin\View\Download; +use Generator; use League\Csv\Writer; use WPGraphQL\Logging\Logger\Api\LogEntityInterface; use WPGraphQL\Logging\Logger\Api\LogServiceInterface; @@ -16,6 +17,13 @@ * @since 0.0.1 */ class DownloadLogService { + /** + * Default maximum number of log entries in a single bulk download. + * + * @var int + */ + public const DEFAULT_BULK_DOWNLOAD_LIMIT = 1000; + /** * Constructor. * @@ -30,9 +38,7 @@ public function __construct(protected readonly LogServiceInterface $log_service) * @param int $log_id The ID of the log to download. */ public function generate_csv( int $log_id ): void { - if ( ! current_user_can( 'manage_options' ) || ! is_admin() ) { - wp_die( esc_html__( 'You do not have sufficient permissions to access this page.', 'wpgraphql-logging' ) ); - } + $this->verify_permissions(); if ( 0 === $log_id ) { wp_die( esc_html__( 'Invalid log ID.', 'wpgraphql-logging' ) ); @@ -44,26 +50,88 @@ public function generate_csv( int $log_id ): void { wp_die( esc_html__( 'Log not found.', 'wpgraphql-logging' ) ); } - // Set headers for CSV download. $filename = apply_filters( 'wpgraphql_logging_csv_filename', 'graphql_log_' . $log_id . '.csv' ); - header( 'Content-Type: text/csv; charset=utf-8' ); - header( 'Content-Disposition: attachment; filename="' . $filename . '"' ); - header( 'Cache-Control: must-revalidate, post-check=0, pre-check=0' ); - header( 'Expires: 0' ); + $this->stream_csv( (string) $filename, [ $log ] ); + } - // Create CSV. - $output = fopen( 'php://output', 'w' ); - if ( ! is_resource( $output ) ) { - wp_die( esc_html__( 'Failed to create CSV output.', 'wpgraphql-logging' ) ); + /** + * Generates and serves a single CSV file containing multiple log entries. + * + * Entries are fetched and written one at a time, straight to the output + * stream, so memory use stays flat regardless of how many are selected. + * + * @param array $log_ids The IDs of the logs to download. + */ + public function generate_bulk_csv( array $log_ids ): void { + $this->verify_permissions(); + + $log_ids = $this->sanitize_log_ids( $log_ids ); + if ( [] === $log_ids ) { + wp_die( esc_html__( 'No logs selected.', 'wpgraphql-logging' ) ); } - $writer = Writer::from( $output ); - $headers = $this->get_headers( $log ); - $content = $this->get_content( $log ); - $writer->insertOne( $headers ); - $writer->insertOne( $content ); - fclose( $output ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fclose -- Closes the php://output stream, which WP_Filesystem can't handle. - exit; + $logs = $this->get_logs( $log_ids ); + + // Starts the generator, so we can bail before any headers are sent. + if ( ! $logs->valid() ) { + wp_die( esc_html__( 'Logs not found.', 'wpgraphql-logging' ) ); + } + + $filename = apply_filters( 'wpgraphql_logging_bulk_csv_filename', 'graphql_logs_' . gmdate( 'Y-m-d_His' ) . '.csv', $log_ids ); + $this->stream_csv( (string) $filename, $logs ); + } + + /** + * Sanitizes a list of log IDs for a bulk download. + * + * Removes invalid and duplicate IDs and caps the list at the bulk download limit. + * + * @param array $log_ids The raw log IDs. + * + * @return array The sanitized log IDs. + */ + public function sanitize_log_ids( array $log_ids ): array { + $log_ids = array_map( 'absint', $log_ids ); + $log_ids = array_values( array_unique( array_filter( $log_ids, static fn( int $id ): bool => $id > 0 ) ) ); + + return array_slice( $log_ids, 0, $this->get_bulk_download_limit() ); + } + + /** + * Gets the maximum number of log entries allowed in a single bulk download. + * + * @return int The bulk download limit. + */ + public function get_bulk_download_limit(): int { + $limit = (int) apply_filters( 'wpgraphql_logging_bulk_download_limit', self::DEFAULT_BULK_DOWNLOAD_LIMIT ); + return max( 1, $limit ); + } + + /** + * Writes log entries as CSV rows to a stream. + * + * The header row is written once, before the first entry. + * + * @param resource $stream The stream to write to. + * @param iterable $logs The log entries. + * + * @phpstan-param iterable<\WPGraphQL\Logging\Logger\Api\LogEntityInterface> $logs + * + * @return int The number of log entries written. + */ + public function write_csv( $stream, iterable $logs ): int { + $writer = Writer::from( $stream ); + $count = 0; + + foreach ( $logs as $log ) { + if ( 0 === $count ) { + $writer->insertOne( $this->get_headers( $log ) ); + } + $writer->insertOne( $this->get_content( $log ) ); + ++$count; + } + + return $count; } /** @@ -109,4 +177,54 @@ public function get_content(LogEntityInterface $log): array { ]; return apply_filters( 'wpgraphql_logging_csv_content', $content, $log->get_id(), $log ); } + + /** + * Verifies the current user can download logs. + */ + protected function verify_permissions(): void { + if ( ! current_user_can( 'manage_options' ) || ! is_admin() ) { + wp_die( esc_html__( 'You do not have sufficient permissions to access this page.', 'wpgraphql-logging' ) ); + } + } + + /** + * Lazily loads log entries by ID, skipping any that no longer exist. + * + * @param array $log_ids The log IDs. + * + * @return \Generator The log entries. + */ + protected function get_logs( array $log_ids ): Generator { + foreach ( $log_ids as $log_id ) { + $log = $this->log_service->find_entity_by_id( $log_id ); + if ( null !== $log ) { + yield $log; + } + } + } + + /** + * Sends download headers and streams log entries to the browser as CSV, then exits. + * + * @param string $filename The download filename. + * @param iterable $logs The log entries. + * + * @phpstan-param iterable<\WPGraphQL\Logging\Logger\Api\LogEntityInterface> $logs + */ + protected function stream_csv( string $filename, iterable $logs ): void { + $filename = sanitize_file_name( $filename ); + header( 'Content-Type: text/csv; charset=utf-8' ); + header( 'Content-Disposition: attachment; filename="' . $filename . '"' ); + header( 'Cache-Control: must-revalidate, post-check=0, pre-check=0' ); + header( 'Expires: 0' ); + + $output = fopen( 'php://output', 'w' ); + if ( ! is_resource( $output ) ) { + wp_die( esc_html__( 'Failed to create CSV output.', 'wpgraphql-logging' ) ); + } + + $this->write_csv( $output, $logs ); + fclose( $output ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fclose -- Closes the php://output stream, which WP_Filesystem can't handle. + exit; + } } diff --git a/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php b/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php index cd7e9028d..c4e41b66c 100644 --- a/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php +++ b/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php @@ -5,6 +5,7 @@ namespace WPGraphQL\Logging\Admin\View\List; use DateTime; +use WPGraphQL\Logging\Admin\View\Download\DownloadLogService; use WPGraphQL\Logging\Admin\ViewLogsPage; use WPGraphQL\Logging\Logger\Api\LogServiceInterface; use WPGraphQL\Logging\Logger\Database\WordPressDatabaseEntity; @@ -36,6 +37,13 @@ class ListTable extends WP_List_Table { */ public const DEFAULT_PER_PAGE = 20; + /** + * The bulk action key for downloading the selected logs. + * + * @var string + */ + public const BULK_DOWNLOAD_ACTION = 'bulk_download'; + /** * Constructor. * @@ -111,8 +119,9 @@ public function prepare_items(): void { */ public function get_bulk_actions(): array { return [ - 'delete' => __( 'Delete Selected', 'wpgraphql-logging' ), - 'delete_all' => __( 'Delete All', 'wpgraphql-logging' ), + self::BULK_DOWNLOAD_ACTION => __( 'Download Selected', 'wpgraphql-logging' ), + 'delete' => __( 'Delete Selected', 'wpgraphql-logging' ), + 'delete_all' => __( 'Delete All', 'wpgraphql-logging' ), ]; } @@ -197,6 +206,36 @@ public function process_bulk_action(): void { exit; } + /** + * Handle the "Download Selected" bulk action. + * + * Streams the selected logs as a single CSV file and exits, so it must run + * before any HTML is output. + * + * @phpcs:disable WordPress.Security.NonceVerification.Recommended + */ + public function process_bulk_download(): void { + if ( self::BULK_DOWNLOAD_ACTION !== $this->current_action() ) { + return; + } + + $nonce_value = isset( $_REQUEST['_wpnonce'] ) && is_string( $_REQUEST['_wpnonce'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['_wpnonce'] ) ) : ''; + if ( false === wp_verify_nonce( $nonce_value, 'bulk-' . esc_attr( $this->_args['plural'] ) ) ) { + wp_die( esc_html__( 'Nonce verification failed!', 'wpgraphql-logging' ) ); + } + + $ids = isset( $_REQUEST['log'] ) ? array_map( 'absint', (array) $_REQUEST['log'] ) : []; + $ids = array_filter( $ids, static fn( int $id ): bool => $id > 0 ); + + // Nothing selected: fall through and render the list as usual. + if ( [] === $ids ) { + return; + } + + $downloader = new DownloadLogService( $this->log_service ); + $downloader->generate_bulk_csv( $ids ); + } + /** * Get the columns for the logs table. * diff --git a/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php b/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php index 78c48c4b0..e056fa6b2 100644 --- a/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php +++ b/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php @@ -210,6 +210,10 @@ public function process_page_actions_before_rendering(): void { $this->process_log_download(); } + // Runs before rendering because it streams a CSV and exits. Nonce handled in the list table. + $list_table = new ListTable( $this->get_log_service() ); + $list_table->process_bulk_download(); + $this->process_filters_redirect(); } diff --git a/plugins/wpgraphql-logging/tests/e2e/specs/bulk-download.spec.js b/plugins/wpgraphql-logging/tests/e2e/specs/bulk-download.spec.js new file mode 100644 index 000000000..d6db3872c --- /dev/null +++ b/plugins/wpgraphql-logging/tests/e2e/specs/bulk-download.spec.js @@ -0,0 +1,75 @@ +import { expect, test } from "@wordpress/e2e-test-utils-playwright"; +import { + goToLoggingSettingsPage, + goToLogsListPage, + configureLogging, + executeGraphQLQuery, + resetPluginSettings, +} from "../utils"; +import { GET_POSTS_QUERY } from "../constants"; + +test.describe("Bulk Log Download", () => { + test.beforeEach(async ({ admin, page }) => { + await resetPluginSettings(admin); + + await goToLoggingSettingsPage(admin); + await expect(page.locator("h1")).toHaveText("WPGraphQL Logging Settings"); + }); + + test("downloads selected logs as a single CSV", async ({ + page, + admin, + request, + }) => { + await configureLogging(page, { + enabled: true, + dataSampling: "100", + eventLogSelection: ["graphql_request_results"], + }); + + // Create at least two log entries. + for (let i = 0; i < 2; i++) { + const response = await executeGraphQLQuery(request, GET_POSTS_QUERY); + expect(response.ok()).toBeTruthy(); + } + + await goToLogsListPage(admin); + await expect(page.locator("h1")).toContainText("WPGraphQL Logs"); + + const logRows = page + .locator("#the-list tr") + .filter({ hasText: "GetPosts" }); + await expect(logRows.nth(1)).toBeVisible({ timeout: 10000 }); + + const selectedIds = []; + for (let i = 0; i < 2; i++) { + const checkbox = logRows.nth(i).locator('input[name="log[]"]'); + await checkbox.check(); + selectedIds.push(await checkbox.inputValue()); + } + + await page + .locator("#bulk-action-selector-top") + .selectOption("bulk_download"); + + const downloadPromise = page.waitForEvent("download"); + await page.locator("#doaction").click(); + const download = await downloadPromise; + + expect(download.suggestedFilename()).toMatch( + /^graphql_logs_\d{4}-\d{2}-\d{2}_\d{6}\.csv$/ + ); + + const fs = require("fs"); + const content = fs.readFileSync(await download.path(), "utf8"); + const lines = content.trim().split("\n"); + + // One header row, then one row per selected log. + expect(lines[0]).toContain("ID"); + expect(lines[0]).toContain("Message"); + for (const id of selectedIds) { + expect(content).toMatch(new RegExp(`^${id},`, "m")); + } + expect(content).toContain("GetPosts"); + }); +}); diff --git a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php index 3a958b66b..2b71154fe 100644 --- a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php +++ b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php @@ -142,4 +142,133 @@ public function test_generate_csv_returns_valid_csv(): void { $this->assertEquals(wp_json_encode($this->fixture['extra']), $content[8]); } + /** + * Creates a real log entry in the database and returns its ID. + */ + private function create_log(string $message): int { + $entity = $this->log_service->create_log_entity( + $this->fixture['channel'], + $this->fixture['level'], + $this->fixture['level_name'], + $message, + $this->fixture['context'], + $this->fixture['extra'] + ); + $this->assertNotNull($entity); + return $entity->get_id(); + } + + /** + * Reads every row back out of a CSV stream. + * + * @param resource $stream + */ + private function read_csv($stream): array { + rewind($stream); + $rows = []; + while (false !== ($row = fgetcsv($stream, null, ',', '"', '\\'))) { + $rows[] = $row; + } + return $rows; + } + + public function test_generate_bulk_csv_requires_admin_capabilities(): void { + wp_set_current_user(0); + + $this->expectException(\WPDieException::class); + $this->expectExceptionMessage('You do not have sufficient permissions to access this page.'); + + $this->service->generate_bulk_csv([1, 2]); + } + + public function test_generate_bulk_csv_requires_valid_ids(): void { + $this->set_as_admin(); + + $this->expectException(\WPDieException::class); + $this->expectExceptionMessage('No logs selected.'); + + $this->service->generate_bulk_csv([0, 'abc', null]); + } + + public function test_generate_bulk_csv_dies_when_no_logs_found(): void { + $this->set_as_admin(); + + $this->expectException(\WPDieException::class); + $this->expectExceptionMessage('Logs not found.'); + + $this->service->generate_bulk_csv([9999998, 9999999]); + } + + public function test_sanitize_log_ids_removes_invalid_and_duplicate_ids(): void { + $this->assertSame([3, 7, 12], $this->service->sanitize_log_ids(['3', 7, 0, 'abc', 3, '12', null])); + } + + public function test_sanitize_log_ids_caps_at_bulk_download_limit(): void { + $this->assertSame(DownloadLogService::DEFAULT_BULK_DOWNLOAD_LIMIT, $this->service->get_bulk_download_limit()); + + $limit_filter = static fn() => 2; + add_filter('wpgraphql_logging_bulk_download_limit', $limit_filter); + + $this->assertSame(2, $this->service->get_bulk_download_limit()); + $this->assertSame([5, 6], $this->service->sanitize_log_ids([5, 6, 7, 8])); + + remove_filter('wpgraphql_logging_bulk_download_limit', $limit_filter); + } + + public function test_get_bulk_download_limit_is_at_least_one(): void { + $limit_filter = static fn() => 0; + add_filter('wpgraphql_logging_bulk_download_limit', $limit_filter); + + $this->assertSame(1, $this->service->get_bulk_download_limit()); + + remove_filter('wpgraphql_logging_bulk_download_limit', $limit_filter); + } + + public function test_write_csv_writes_one_header_row_and_one_row_per_log(): void { + $first_id = $this->create_log('First bulk log'); + $second_id = $this->create_log('Second bulk log'); + + $logs = [ + $this->log_service->find_entity_by_id($first_id), + $this->log_service->find_entity_by_id($second_id), + ]; + + $stream = fopen('php://temp', 'w+'); + $count = $this->service->write_csv($stream, $logs); + $rows = $this->read_csv($stream); + fclose($stream); + + $this->assertSame(2, $count); + $this->assertCount(3, $rows); + $this->assertSame('ID', $rows[0][0]); + $this->assertSame((string) $first_id, $rows[1][0]); + $this->assertSame('First bulk log', $rows[1][4]); + $this->assertSame((string) $second_id, $rows[2][0]); + $this->assertSame('Second bulk log', $rows[2][4]); + } + + public function test_write_csv_with_no_logs_writes_nothing(): void { + $stream = fopen('php://temp', 'w+'); + $count = $this->service->write_csv($stream, []); + $rows = $this->read_csv($stream); + fclose($stream); + + $this->assertSame(0, $count); + $this->assertSame([], $rows); + } + + public function test_get_logs_skips_missing_entries(): void { + $first_id = $this->create_log('Kept log one'); + $second_id = $this->create_log('Kept log two'); + + $reflection = new \ReflectionClass($this->service); + $method = $reflection->getMethod('get_logs'); + $method->setAccessible(true); + + $logs = iterator_to_array($method->invoke($this->service, [$first_id, 9999999, $second_id]), false); + + $this->assertCount(2, $logs); + $this->assertSame($first_id, $logs[0]->get_id()); + $this->assertSame($second_id, $logs[1]->get_id()); + } } diff --git a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/List/ListTableTest.php b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/List/ListTableTest.php index 9e9403c04..cb563a2c2 100644 --- a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/List/ListTableTest.php +++ b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/List/ListTableTest.php @@ -71,8 +71,10 @@ public function test_get_columns_returns_expected_columns(): void { public function test_get_bulk_actions_returns_expected_actions(): void { $actions = $this->list_table->get_bulk_actions(); + $this->assertArrayHasKey(ListTable::BULK_DOWNLOAD_ACTION, $actions); $this->assertArrayHasKey('delete', $actions); $this->assertArrayHasKey('delete_all', $actions); + $this->assertEquals('Download Selected', $actions[ListTable::BULK_DOWNLOAD_ACTION]); $this->assertEquals('Delete Selected', $actions['delete']); $this->assertEquals('Delete All', $actions['delete_all']); } @@ -424,4 +426,61 @@ public function test_column_default_returns_empty_string_for_unknown_column(): v $this->assertEquals('', $result); } + public function test_process_bulk_download_ignores_other_actions(): void { + $_REQUEST['action'] = 'delete'; + $this->log_service->shouldNotReceive('find_entity_by_id'); + + $this->list_table->process_bulk_download(); + $this->assertTrue(true); + + unset($_REQUEST['action']); + } + + public function test_process_bulk_download_dies_with_invalid_nonce(): void { + $_REQUEST['action'] = ListTable::BULK_DOWNLOAD_ACTION; + $_REQUEST['_wpnonce'] = 'invalid'; + $_REQUEST['log'] = ['1', '2']; + + try { + $this->expectException(\WPDieException::class); + $this->expectExceptionMessage('Nonce verification failed!'); + $this->list_table->process_bulk_download(); + } finally { + unset($_REQUEST['action'], $_REQUEST['_wpnonce'], $_REQUEST['log']); + } + } + + public function test_process_bulk_download_returns_when_nothing_selected(): void { + $_REQUEST['action'] = ListTable::BULK_DOWNLOAD_ACTION; + $_REQUEST['_wpnonce'] = wp_create_nonce('bulk-logs'); + $_REQUEST['log'] = ['0', 'abc']; + $this->log_service->shouldNotReceive('find_entity_by_id'); + + $this->list_table->process_bulk_download(); + $this->assertTrue(true); + + unset($_REQUEST['action'], $_REQUEST['_wpnonce'], $_REQUEST['log']); + } + + public function test_process_bulk_download_passes_selected_ids_to_downloader(): void { + $admin_user = $this->factory()->user->create(['role' => 'administrator']); + wp_set_current_user($admin_user); + set_current_screen('dashboard'); + + $_REQUEST['action'] = ListTable::BULK_DOWNLOAD_ACTION; + $_REQUEST['_wpnonce'] = wp_create_nonce('bulk-logs'); + $_REQUEST['log'] = ['11', '12']; + + // Neither ID exists, so the downloader dies before sending headers. + $this->log_service->shouldReceive('find_entity_by_id')->with(11)->once()->andReturn(null); + $this->log_service->shouldReceive('find_entity_by_id')->with(12)->once()->andReturn(null); + + try { + $this->expectException(\WPDieException::class); + $this->expectExceptionMessage('Logs not found.'); + $this->list_table->process_bulk_download(); + } finally { + unset($_REQUEST['action'], $_REQUEST['_wpnonce'], $_REQUEST['log']); + } + } } From ef6c222b34fe59f87f97fe31ecbdd0c8698f8c51 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Fri, 30 Oct 2026 15:37:03 -0400 Subject: [PATCH 2/6] fix(logging): harden CSV log downloads against formula injection Second review pass (correctness and security): - Escape cells that spreadsheets would evaluate as formulas. Log values such as the query and message come from unauthenticated GraphQL requests, so an exported CSV could otherwise run attacker formulas. - Drop open output buffers before streaming so large exports are not held in memory. - Ignore non-scalar IDs (absint() turns arrays into 1) and reuse the service's ID sanitizer in the list table. --- .../Admin/View/Download/DownloadLogService.php | 18 ++++++++++++++---- .../src/Admin/View/List/ListTable.php | 5 ++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php index b053f4bf3..a53621b04 100644 --- a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php +++ b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php @@ -5,6 +5,7 @@ namespace WPGraphQL\Logging\Admin\View\Download; use Generator; +use League\Csv\EscapeFormula; use League\Csv\Writer; use WPGraphQL\Logging\Logger\Api\LogEntityInterface; use WPGraphQL\Logging\Logger\Api\LogServiceInterface; @@ -84,14 +85,15 @@ public function generate_bulk_csv( array $log_ids ): void { /** * Sanitizes a list of log IDs for a bulk download. * - * Removes invalid and duplicate IDs and caps the list at the bulk download limit. + * Removes non-scalar, invalid and duplicate IDs and caps the list at the bulk download limit. * * @param array $log_ids The raw log IDs. * * @return array The sanitized log IDs. */ public function sanitize_log_ids( array $log_ids ): array { - $log_ids = array_map( 'absint', $log_ids ); + // absint() casts arrays to 1, so drop anything that isn't a scalar first. + $log_ids = array_map( 'absint', array_filter( $log_ids, 'is_scalar' ) ); $log_ids = array_values( array_unique( array_filter( $log_ids, static fn( int $id ): bool => $id > 0 ) ) ); return array_slice( $log_ids, 0, $this->get_bulk_download_limit() ); @@ -110,7 +112,9 @@ public function get_bulk_download_limit(): int { /** * Writes log entries as CSV rows to a stream. * - * The header row is written once, before the first entry. + * The header row is written once, before the first entry. Cells that a + * spreadsheet would treat as a formula (=, +, -, @, ...) are escaped, since + * log values can come from unauthenticated GraphQL requests. * * @param resource $stream The stream to write to. * @param iterable $logs The log entries. @@ -121,7 +125,8 @@ public function get_bulk_download_limit(): int { */ public function write_csv( $stream, iterable $logs ): int { $writer = Writer::from( $stream ); - $count = 0; + $writer->addFormatter( ( new EscapeFormula() )->escapeRecord( ... ) ); + $count = 0; foreach ( $logs as $log ) { if ( 0 === $count ) { @@ -218,6 +223,11 @@ protected function stream_csv( string $filename, iterable $logs ): void { header( 'Cache-Control: must-revalidate, post-check=0, pre-check=0' ); header( 'Expires: 0' ); + // Drop any open output buffers so rows go straight to the client instead of piling up in memory. + while ( ob_get_level() > 0 ) { + ob_end_clean(); + } + $output = fopen( 'php://output', 'w' ); if ( ! is_resource( $output ) ) { wp_die( esc_html__( 'Failed to create CSV output.', 'wpgraphql-logging' ) ); diff --git a/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php b/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php index c4e41b66c..2cb9b6dc4 100644 --- a/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php +++ b/plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php @@ -224,15 +224,14 @@ public function process_bulk_download(): void { wp_die( esc_html__( 'Nonce verification failed!', 'wpgraphql-logging' ) ); } - $ids = isset( $_REQUEST['log'] ) ? array_map( 'absint', (array) $_REQUEST['log'] ) : []; - $ids = array_filter( $ids, static fn( int $id ): bool => $id > 0 ); + $downloader = new DownloadLogService( $this->log_service ); + $ids = isset( $_REQUEST['log'] ) ? $downloader->sanitize_log_ids( (array) wp_unslash( $_REQUEST['log'] ) ) : []; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- sanitize_log_ids() casts every ID with absint(). // Nothing selected: fall through and render the list as usual. if ( [] === $ids ) { return; } - $downloader = new DownloadLogService( $this->log_service ); $downloader->generate_bulk_csv( $ids ); } From 0c3314e371ae61652597a66e8155664e4d575707 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Fri, 30 Oct 2026 15:38:54 -0400 Subject: [PATCH 3/6] test(logging): cover bulk download wiring, formula escaping and ID edge cases Third review pass (tests and static analysis): - Cover the ViewLogsPage load-time hook end to end: bad nonce and the 'Logs not found' path through the real database service. - Assert formula cells are escaped and plain cells are left alone. - Assert non-scalar IDs are dropped instead of becoming ID 1. --- .../View/Download/DownloadLogServiceTest.php | 19 ++++++++++ .../wpunit/Admin/View/ViewLogsPageTest.php | 38 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php index 2b71154fe..863b7c542 100644 --- a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php +++ b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/Download/DownloadLogServiceTest.php @@ -203,6 +203,11 @@ public function test_sanitize_log_ids_removes_invalid_and_duplicate_ids(): void $this->assertSame([3, 7, 12], $this->service->sanitize_log_ids(['3', 7, 0, 'abc', 3, '12', null])); } + public function test_sanitize_log_ids_drops_non_scalar_values(): void { + // absint() would turn a non-empty array into 1. + $this->assertSame([6], $this->service->sanitize_log_ids([['5'], '6', [1, 2], new \stdClass()])); + } + public function test_sanitize_log_ids_caps_at_bulk_download_limit(): void { $this->assertSame(DownloadLogService::DEFAULT_BULK_DOWNLOAD_LIMIT, $this->service->get_bulk_download_limit()); @@ -247,6 +252,20 @@ public function test_write_csv_writes_one_header_row_and_one_row_per_log(): void $this->assertSame('Second bulk log', $rows[2][4]); } + public function test_write_csv_escapes_spreadsheet_formulas(): void { + $log_id = $this->create_log('=HYPERLINK("http://example.com","click")'); + + $stream = fopen('php://temp', 'w+'); + $this->service->write_csv($stream, [$this->log_service->find_entity_by_id($log_id)]); + $rows = $this->read_csv($stream); + fclose($stream); + + $this->assertSame("'=HYPERLINK(\"http://example.com\",\"click\")", $rows[1][4]); + // Values that are not formulas are left alone. + $this->assertSame((string) $log_id, $rows[1][0]); + $this->assertSame(wp_json_encode($this->fixture['context']), $rows[1][7]); + } + public function test_write_csv_with_no_logs_writes_nothing(): void { $stream = fopen('php://temp', 'w+'); $count = $this->service->write_csv($stream, []); diff --git a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/ViewLogsPageTest.php b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/ViewLogsPageTest.php index a509575d8..e37fd3c93 100644 --- a/plugins/wpgraphql-logging/tests/wpunit/Admin/View/ViewLogsPageTest.php +++ b/plugins/wpgraphql-logging/tests/wpunit/Admin/View/ViewLogsPageTest.php @@ -5,6 +5,7 @@ namespace WPGraphQL\Logging\Tests\Admin\View; +use WPGraphQL\Logging\Admin\View\List\ListTable; use WPGraphQL\Logging\Admin\ViewLogsPage; use Codeception\TestCase\WPTestCase; use Brain\Monkey; @@ -209,4 +210,41 @@ public function test_process_log_download_dies_without_nonce(): void { $method->setAccessible(true); $method->invoke($instance); } + + public function test_process_page_actions_before_rendering_handles_bulk_download_nonce(): void { + $this->set_as_admin(); + $instance = ViewLogsPage::init(); + + unset($_GET['action'], $_GET['log']); + $_REQUEST['action'] = ListTable::BULK_DOWNLOAD_ACTION; + $_REQUEST['_wpnonce'] = 'invalid'; + $_REQUEST['log'] = ['1']; + + try { + $this->expectException(\WPDieException::class); + $this->expectExceptionMessage('Nonce verification failed!'); + $instance->process_page_actions_before_rendering(); + } finally { + unset($_REQUEST['action'], $_REQUEST['_wpnonce'], $_REQUEST['log']); + } + } + + public function test_process_page_actions_before_rendering_runs_bulk_download(): void { + $this->set_as_admin(); + $instance = ViewLogsPage::init(); + + unset($_GET['action'], $_GET['log']); + $_REQUEST['action'] = ListTable::BULK_DOWNLOAD_ACTION; + $_REQUEST['_wpnonce'] = wp_create_nonce('bulk-logs'); + $_REQUEST['log'] = ['9999998', '9999999']; + + // The IDs don't exist, so the download stops before any headers are sent. + try { + $this->expectException(\WPDieException::class); + $this->expectExceptionMessage('Logs not found.'); + $instance->process_page_actions_before_rendering(); + } finally { + unset($_REQUEST['action'], $_REQUEST['_wpnonce'], $_REQUEST['log']); + } + } } From 2c799ac1cd41c7f0518b9349c25d9bc4edd3f539 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Fri, 30 Oct 2026 15:40:01 -0400 Subject: [PATCH 4/6] docs(logging): document bulk log downloads and skip unneeded list table setup Fourth review pass (simplification, performance and docs): - Only build the list table at load time when the bulk download action is actually requested, instead of on every page load. - Document the Download Selected bulk action, the 1,000-row cap and its filters, and formula escaping in the plugin docs. --- docs/plugins/wpgraphql-logging/index.md | 8 ++++++-- plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php | 10 ++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/docs/plugins/wpgraphql-logging/index.md b/docs/plugins/wpgraphql-logging/index.md index 0231ad998..c577c6238 100644 --- a/docs/plugins/wpgraphql-logging/index.md +++ b/docs/plugins/wpgraphql-logging/index.md @@ -133,7 +133,11 @@ Once setup, you can view logs under GraphQL Logs -> All Logs. The admin screen i ### Downloading Logs -You can download individual logs in CSV format. +You can download logs in CSV format, either one at a time with the "Download" row action, or several at once by checking them and choosing "Download Selected" from the bulk actions. A bulk download gives you one CSV with a header row and one row per selected log. + +Bulk downloads are streamed straight to the browser one row at a time, so no temporary files are written. A single bulk download is capped at 1,000 logs. You can change the cap with the `wpgraphql_logging_bulk_download_limit` filter, and rename the file with `wpgraphql_logging_bulk_csv_filename`. + +Cells that a spreadsheet would treat as a formula (starting with `=`, `+`, `-`, `@` and similar) are prefixed with `'`, because log values can come from unauthenticated GraphQL requests. **Example** @@ -157,7 +161,7 @@ You can also filter logs with the following filters: ### Bulk Actions -You can delete selected logs or all logs using the bulk action controls. +You can download selected logs, delete selected logs, or delete all logs using the bulk action controls. ## Uninstallation diff --git a/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php b/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php index e056fa6b2..8b4c9ebd6 100644 --- a/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php +++ b/plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php @@ -204,15 +204,17 @@ public function render_admin_page(): void { */ public function process_page_actions_before_rendering(): void { - // Nonce handled in process_log_download and process_filters_redirect. + // Nonce handled in process_log_download, ListTable::process_bulk_download and process_filters_redirect. // phpcs:ignore WordPress.Security.NonceVerification.Recommended if ( isset( $_GET['action'] ) && 'download' === $_GET['action'] ) { $this->process_log_download(); } - // Runs before rendering because it streams a CSV and exits. Nonce handled in the list table. - $list_table = new ListTable( $this->get_log_service() ); - $list_table->process_bulk_download(); + // phpcs:ignore WordPress.Security.NonceVerification.Recommended + if ( isset( $_REQUEST['action'] ) && ListTable::BULK_DOWNLOAD_ACTION === $_REQUEST['action'] ) { + $list_table = new ListTable( $this->get_log_service() ); + $list_table->process_bulk_download(); + } $this->process_filters_redirect(); } From ac44359f576fb7cbc44fa2be74a8dd67c8621dbe Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Tue, 3 Nov 2026 11:23:12 -0500 Subject: [PATCH 5/6] Potential fix for pull request finding 'Stop output-buffer cleanup when a buffer is not removable' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/Admin/View/Download/DownloadLogService.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php index a53621b04..1fad69cc4 100644 --- a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php +++ b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php @@ -225,7 +225,13 @@ protected function stream_csv( string $filename, iterable $logs ): void { // Drop any open output buffers so rows go straight to the client instead of piling up in memory. while ( ob_get_level() > 0 ) { - ob_end_clean(); + $status = ob_get_status(); + if ( false === $status || 0 === ( $status['flags'] & PHP_OUTPUT_HANDLER_REMOVABLE ) ) { + break; + } + if ( ! ob_end_clean() ) { + break; + } } $output = fopen( 'php://output', 'w' ); From dd2a70e9e1ee328049a920cbe9cee8328ed71138 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Tue, 3 Nov 2026 11:34:15 -0500 Subject: [PATCH 6/6] fix(logging): drop impossible false check on ob_get_status() result --- .../src/Admin/View/Download/DownloadLogService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php index 1fad69cc4..f037a024f 100644 --- a/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php +++ b/plugins/wpgraphql-logging/src/Admin/View/Download/DownloadLogService.php @@ -226,7 +226,7 @@ protected function stream_csv( string $filename, iterable $logs ): void { // Drop any open output buffers so rows go straight to the client instead of piling up in memory. while ( ob_get_level() > 0 ) { $status = ob_get_status(); - if ( false === $status || 0 === ( $status['flags'] & PHP_OUTPUT_HANDLER_REMOVABLE ) ) { + if ( ! isset( $status['flags'] ) || 0 === ( $status['flags'] & PHP_OUTPUT_HANDLER_REMOVABLE ) ) { break; } if ( ! ob_end_clean() ) {