fix(woocommerce): resolve checkout attempts from order status, not payment_complete

includes/class-webdecoy-woocommerce.php

@@ -228,27 +228,91 @@ public function track_attempt(int $order_id): void
         ]);
     }
 
+    /**
+     * Order statuses that mean the store accepted this order, so the checkout
+     * attempt behind it was a real purchase and not a card test.
+     *
+     * `on-hold` is included deliberately: it is where Direct Bank Transfer and
+     * Cheque leave a legitimate order awaiting payment, and nobody tests stolen
+     * cards by promising a bank transfer.
+     *
+     * @var string[]
+     */
+    private const ACCEPTED_ORDER_STATUSES = ['processing', 'completed', 'on-hold', 'refunded'];
+
     /**
      * Track successful payment
      *
      * @param int $order_id Order ID
      */
     public function track_payment(int $order_id): void
+    {
+        $this->resolve_attempt($order_id, 'success');
+    }
+
+    /**
+     * Resolve the open attempt behind an order once its status says what happened.
+     *
+     * This exists because `woocommerce_payment_complete` is not universal: Cash on
+     * Delivery, Direct Bank Transfer and Cheque move an order to processing or
+     * on-hold and never call WC_Order::payment_complete(). On a store using only
+     * those gateways no attempt row was ever marked resolved, so the card-testing
+     * and velocity filters excluded nothing and every real order counted as an
+     * attack. Refs #60.
+     *
+     * @param int $order_id
+     * @param string $status New status for the attempt row.
+     */
+    public function resolve_attempt(int $order_id, string $status): void
     {
         global $wpdb;
 
         $table = $wpdb->prefix . 'webdecoy_checkout_attempts';
-        $ip = $this->get_client_ip();
 
-        // Update most recent attempt for this IP/order to success
+        // Keyed on order_id ALONE, not on the current request's IP. Payment
+        // completion often arrives on an asynchronous gateway callback (IPN, webhook,
+        // or a cron-driven status change), where get_client_ip() is the gateway's
+        // address or empty — so an IP-scoped UPDATE matched nothing and silently left
+        // the row open. The row was inserted with the customer's IP by track_attempt();
+        // order_id already identifies it.
         // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table name is safe, built from $wpdb->prefix
         $wpdb->query($wpdb->prepare(
-            "UPDATE {$table} SET status = 'success' WHERE ip_address = %s AND order_id = %d AND status = 'attempt'",
-            $ip,
+            "UPDATE {$table} SET status = %s WHERE order_id = %d AND status = 'attempt'",
+            $status,
             $order_id
         ));
     }
 
+    /**
+     * Resolve the attempt from an order status transition, covering every gateway
+     * rather than only those that fire woocommerce_payment_complete.
+     *
+     * @param int $order_id
+     * @param string $new_status Status WooCommerce transitioned to, without the wc- prefix.
+     */
+    public function track_status_change(int $order_id, string $new_status): void
+    {
+        if (self::is_accepted_order_status($new_status)) {
+            $this->resolve_attempt($order_id, 'success');
+        }
+    }
+
+    /**
+     * Whether a status counts as the store having accepted the order. Exposed for
+     * tests and for callers that need the same rule without a database round trip.
+     */
+    public static function is_accepted_order_status(string $status): bool
+    {
+        // Strip the wc- PREFIX. Not ltrim($status, 'wc-') — that takes a character
+        // list, so it would eat the leading "c" of "completed" and the "c" of
+        // "cancelled", quietly answering the wrong question for both.
+        if (strncmp($status, 'wc-', 3) === 0) {
+            $status = substr($status, 3);
+        }
+
+        return in_array($status, self::ACCEPTED_ORDER_STATUSES, true);
+    }
+
     /**
      * Track failed payment
      *
@@ -270,14 +334,10 @@ public function track_failure(int $order_id, string $reason = 'failed'): void
             $status = 'declined';
         }
 
-        // Update most recent attempt for this IP/order
-        // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table name is safe, built from $wpdb->prefix
-        $wpdb->query($wpdb->prepare(
-            "UPDATE {$table} SET status = %s WHERE ip_address = %s AND order_id = %d AND status = 'attempt'",
-            $status,
-            $ip,
-            $order_id
-        ));
+        // Keyed on order_id alone — see resolve_attempt() for why the IP cannot be
+        // part of this predicate.
+        unset($ip, $table);
+        $this->resolve_attempt($order_id, $status);
     }
 
     /**
@@ -302,8 +362,10 @@ private function get_recent_attempts(string $ip, int $window, bool $include_succ
             ), ARRAY_A) ?: [];
         }
 
+        // NULL-safe: in MySQL `NULL <> 'success'` is NULL, not TRUE, so a bare
+        // inequality would silently drop any row with no status rather than counting it.
         return $wpdb->get_results($wpdb->prepare(
-            "SELECT * FROM {$table} WHERE ip_address = %s AND created_at > %s AND status <> 'success' ORDER BY created_at DESC",
+            "SELECT * FROM {$table} WHERE ip_address = %s AND created_at > %s AND (status IS NULL OR status <> 'success') ORDER BY created_at DESC",
             $ip,
             $since
         ), ARRAY_A) ?: [];
@@ -693,6 +755,19 @@ public function catch_coupon($data, string $code)
     $woo->track_failure($order_id);
 });
 
+// Resolve the attempt from the order's own status, which every gateway reaches.
+// woocommerce_payment_complete above is not enough: Cash on Delivery, Bank Transfer
+// and Cheque never fire it, so on those stores no attempt was ever closed and every
+// real order kept counting toward card-testing and velocity. Refs #60.
+add_action('woocommerce_order_status_changed', function ($order_id, $from, $to) {
+    $options = get_option('webdecoy_options', []);
+    if (empty($options['protect_checkout'])) {
+        return;
+    }
+
+    (new WebDecoy_WooCommerce($options))->track_status_change((int) $order_id, (string) $to);
+}, 10, 3);
+
 /**
  * WooCommerce Blocks Checkout Integration
  *

tests/WooOrderStatusTest.php

@@ -0,0 +1,95 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * Tests for the order-status rule behind issue #60.
+ *
+ * The card-testing and velocity filters exclude checkout attempts that resulted in
+ * a real order. Resolving "real order" from `woocommerce_payment_complete` alone was
+ * wrong: Cash on Delivery, Direct Bank Transfer and Cheque move an order to
+ * processing or on-hold and never call WC_Order::payment_complete(). On a store
+ * using only those gateways no attempt row was ever closed, so the filter excluded
+ * nothing and every paid order counted as a card-testing attempt.
+ *
+ * These assert the pure status rule, which is the part that decides it.
+ * Run: php tests/run.php
+ */
+
+if (!defined('ABSPATH')) {
+    define('ABSPATH', '/tmp/');
+}
+
+// The class file registers hooks at the top level, so the handful of WordPress
+// functions it touches on load have to exist. Guarded so they don't collide with
+// other tests or a real WP runtime.
+if (!function_exists('add_action')) {
+    function add_action($hook, $cb, $priority = 10, $args = 1)
+    {
+        return true;
+    }
+}
+if (!function_exists('add_filter')) {
+    function add_filter($hook, $cb, $priority = 10, $args = 1)
+    {
+        return true;
+    }
+}
+if (!isset($GLOBALS['__wd_opts'])) {
+    $GLOBALS['__wd_opts'] = [];
+}
+if (!function_exists('get_option')) {
+    function get_option($k, $d = false)
+    {
+        return $GLOBALS['__wd_opts'][$k] ?? $d;
+    }
+}
+
+require_once dirname(__DIR__) . '/includes/class-webdecoy-woocommerce.php';
+
+$t = ['TestRunner', 'test'];
+$true = ['TestRunner', 'assertTrue'];
+
+echo "\nWooCommerce order-status rule (#60)\n";
+
+$t('the offline gateways that never fire payment_complete are still accepted', function () use ($true) {
+    // Cash on Delivery and most "payment on pickup" flows land here.
+    $true(
+        WebDecoy_WooCommerce::is_accepted_order_status('processing'),
+        'processing is a real order (Cash on Delivery)'
+    );
+    // Direct Bank Transfer (BACS) and Cheque leave a legitimate order awaiting funds.
+    $true(
+        WebDecoy_WooCommerce::is_accepted_order_status('on-hold'),
+        'on-hold is a real order (BACS / Cheque)'
+    );
+});
+
+$t('a card-paid order is accepted', function () use ($true) {
+    $true(WebDecoy_WooCommerce::is_accepted_order_status('completed'), 'completed');
+    $true(WebDecoy_WooCommerce::is_accepted_order_status('refunded'), 'refunded was still a real order');
+});
+
+$t('the statuses that mean no sale keep counting as attempts', function () use ($true) {
+    foreach (['failed', 'cancelled', 'pending'] as $status) {
+        $true(
+            !WebDecoy_WooCommerce::is_accepted_order_status($status),
+            "{$status} must keep counting — it is the card-testing signal"
+        );
+    }
+});
+
+$t('the wc- prefix WooCommerce passes around is tolerated', function () use ($true) {
+    // woocommerce_order_status_changed hands over the bare status, but plenty of
+    // callers and stored values carry the wc- prefix. Both must resolve the same.
+    $true(WebDecoy_WooCommerce::is_accepted_order_status('wc-processing'), 'wc-processing');
+    $true(WebDecoy_WooCommerce::is_accepted_order_status('wc-on-hold'), 'wc-on-hold');
+    $true(!WebDecoy_WooCommerce::is_accepted_order_status('wc-failed'), 'wc-failed');
+});
+
+$t('an unknown status is not silently treated as a sale', function () use ($true) {
+    // A gateway or extension adding its own status must not accidentally close an
+    // attempt — degrading to "still an attempt" is the safe direction here.
+    $true(!WebDecoy_WooCommerce::is_accepted_order_status('checkout-draft'), 'checkout-draft');
+    $true(!WebDecoy_WooCommerce::is_accepted_order_status(''), 'empty string');
+});