From 4bd62190a383b4669c508a192f68b52800f34492 Mon Sep 17 00:00:00 2001 From: pluginslab <57633278+pluginslab@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:52:35 +0100 Subject: [PATCH] fix: make the connector rate limit atomic (#228) WP.org review R agentic-admin/28May26/T5 (29 Sep, AI-flagged): the transient-based counter was read-then-written, so concurrent requests could read the same count and bypass the per-user limit. The counter is now an options row incremented with a single INSERT ... ON DUPLICATE KEY UPDATE, which the database applies atomically, with one row per user per clock minute. Older rows are deleted as it goes and on uninstall. Tested: 40 concurrent requests as one user against a limit of 30 allowed exactly 30 (twice), with no database errors. Co-Authored-By: Claude Opus 5.5 (1M context) --- includes/class-connectors.php | 45 +++++++++++++++++++++++++++++------ uninstall.php | 19 +++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/includes/class-connectors.php b/includes/class-connectors.php index f5358b8..aca16c6 100644 --- a/includes/class-connectors.php +++ b/includes/class-connectors.php @@ -476,22 +476,54 @@ private static function flatten_messages_lossy( array $messages ): string { /** * Per-user rate limit for chat completions. * - * Caps at RATE_LIMIT_PER_MINUTE requests per user per rolling 60s - * window via a transient counter. Returns a WP_Error 429 when the - * cap is exceeded, null otherwise. + * Caps at RATE_LIMIT_PER_MINUTE requests per user per clock minute. + * The counter is a row in the options table, incremented with a single + * INSERT ... ON DUPLICATE KEY UPDATE statement. The database applies it + * atomically, so concurrent requests can't read the same count and slip + * past the limit. Returns a WP_Error 429 when the cap is exceeded, null + * otherwise. * * @return \WP_Error|null */ private static function check_rate_limit(): ?\WP_Error { + global $wpdb; + $user_id = \get_current_user_id(); if ( ! $user_id ) { return null; } - $key = 'agentic_admin_conn_rl_' . $user_id; - $count = (int) \get_transient( $key ); + $prefix = 'agentic_admin_conn_rl_' . $user_id . '_'; + $key = $prefix . (int) floor( time() / MINUTE_IN_SECONDS ); + + // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching -- Atomic counter; a cached value would defeat it. + $wpdb->query( + $wpdb->prepare( + "INSERT INTO {$wpdb->options} (option_name, option_value, autoload) + VALUES (%s, '1', 'off') + ON DUPLICATE KEY UPDATE option_value = option_value + 1", + $key + ) + ); + + $count = (int) $wpdb->get_var( + $wpdb->prepare( + "SELECT option_value FROM {$wpdb->options} WHERE option_name = %s", + $key + ) + ); + + // Drop this user's counters from earlier minutes. + $wpdb->query( + $wpdb->prepare( + "DELETE FROM {$wpdb->options} WHERE option_name LIKE %s AND option_name <> %s", + $wpdb->esc_like( $prefix ) . '%', + $key + ) + ); + // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching - if ( $count >= self::RATE_LIMIT_PER_MINUTE ) { + if ( $count > self::RATE_LIMIT_PER_MINUTE ) { return new \WP_Error( 'agentic_admin_rate_limited', sprintf( @@ -502,7 +534,6 @@ private static function check_rate_limit(): ?\WP_Error { ); } - \set_transient( $key, $count + 1, MINUTE_IN_SECONDS ); return null; } } diff --git a/uninstall.php b/uninstall.php index f1c5d10..2572142 100644 --- a/uninstall.php +++ b/uninstall.php @@ -10,7 +10,25 @@ exit; } +/** + * Delete the connector rate-limit counters (one option row per user per minute). + * + * @return void + */ +function agentic_admin_delete_rate_limit_counters(): void { + global $wpdb; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching -- One-off cleanup on uninstall. + $wpdb->query( + $wpdb->prepare( + "DELETE FROM {$wpdb->options} WHERE option_name LIKE %s", + $wpdb->esc_like( 'agentic_admin_conn_rl_' ) . '%' + ) + ); +} + // 1. Single Site Cleanup. +agentic_admin_delete_rate_limit_counters(); delete_option( 'agentic_admin_settings' ); delete_option( 'agentic_admin_model_source' ); delete_option( 'agentic_admin_version' ); @@ -24,6 +42,7 @@ foreach ( $agentic_admin_sites as $agentic_admin_site ) { switch_to_blog( $agentic_admin_site->blog_id ); + agentic_admin_delete_rate_limit_counters(); delete_option( 'agentic_admin_settings' ); delete_option( 'agentic_admin_model_source' ); delete_option( 'agentic_admin_version' );