diff --git a/.gitignore b/.gitignore index 2d0b7c544..66f134d30 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,17 @@ coverage *.ntvs* *.njsproj *.sln -*.sw?node_modules/ +*.sw? + +# Build output +build/ + +# Development notes +FINDINGS.md +MIGRATION_STRATEGY.md +TASKLIST.md +docs/ + +# npm +package-lock.json +node_modules/.package-lock.json diff --git a/FINDINGS.md b/FINDINGS.md deleted file mode 100644 index dbd102366..000000000 --- a/FINDINGS.md +++ /dev/null @@ -1,196 +0,0 @@ -# 🔍 Formipay Plugin — Comprehensive Audit Report - -**Date:** April 17, 2026 -**Auditor:** GitHub Copilot -**Plugin Version:** 1.0.0 -**Files Analyzed:** ~60+ files, ~15,000+ lines of PHP, JS, CSS, HTML - ---- - -## Table of Contents - -- [1. Bugs & Defects](#1-bugs--defects) - - [1.1 Critical Bugs](#11-critical-bugs) - - [1.2 Moderate Bugs](#12-moderate-bugs) -- [2. Security Concerns](#2-security-concerns) -- [3. Architecture & Code Quality Issues](#3-architecture--code-quality-issues) -- [4. Missing Features & Modules](#4-missing-features--modules) -- [5. Performance Issues](#5-performance-issues) -- [6. Missing Admin Pages / Settings](#6-missing-admin-pages--settings) -- [7. Code Cleanup Needed](#7-code-cleanup-needed) -- [8. Opportunities & Nice-to-Haves](#8-opportunities--nice-to-haves) -- [9. Summary Priority Matrix](#9-summary-priority-matrix) -- [10. Architectural Recommendation](#10-architectural-recommendation) - ---- - -## 1. Bugs & Defects - -### 1.1 Critical Bugs - -| # | Location | Issue | Detail | -|---|----------|-------|--------| -| 1 | `includes/Customer.php` ~line 172 `update()` | **Undefined variable — fatal error** | Method builds `$insert_data` and `$where`, but the `$wpdb->update()` call references undefined `$table_name` and `$new_args`. Every customer update will throw a PHP fatal error. | -| 2 | `includes/Order.php` `delete()` | **Undefined `$id` variable** | Uses `$id` in the `$wpdb->delete()` where clause instead of the method parameter `$order_id`. Every order deletion call will fail. | -| 3 | `includes/Order.php` `formipay_bulk_delete_order()` | **Iterates wrong variable** | Loops `foreach($ids as $id)` but calls `$this->delete($order_id)` — `$order_id` comes from the outer scope (nonce check), not the loop variable. Bulk delete will repeatedly delete the same (or zero) orders. | -| 4 | `includes/Notification/Email.php` `send_email()` | **Wrong class reference — fatal error** | Calls `\Formipay_Notification::update_notification_data()` — this class does not exist. Should use `parent::update_notification_data()`. Email status tracking will crash. | -| 5 | `includes/Integration/Paypal.php` `auto_cancel_order_on_timeout()` | **Undefined `Order` class** | Calls `Order::update(...)` but unlike `BankTransfer.php`, `Paypal.php` does not import `use Formipay\Order as Order`. This will throw a class-not-found error on timeout. | -| 6 | `includes/Integration/Paypal.php` `process_payment()` | **Undefined `self::paypal_settings`** | The PayPal class never declares a `$paypal_settings` property. Accessing it leads to undefined property notices and broken payment flow. | - -### 1.2 Moderate Bugs - -| # | Location | Issue | -|---|----------|-------| -| 7 | `includes/Payment/BankTransfer.php` `check_unique_code()` | Uses `MAX(id)+1` for unique codes — predictable and subject to race conditions. Two concurrent orders can receive the same unique code. | -| 8 | `includes/Payment/BankTransfer.php` `add_unique_code_details()` | Calls `$this->check_unique_code()` **three times** per request (once for `'item'`, once for `'amount'`, once for `'subtotal'`). Each call queries the DB independently and may return different values. Displayed unique code may not match the stored one. | -| 9 | `admin/functions.php` `formipay_field_type_collection()` | Color field label says `'Number'` instead of `'Color'` — copy-paste error. | -| 10 | `includes/Render.php` field rendering | No `default` fallback rendered when field type doesn't match any case in the switch — unknown field types silently produce no output. | -| 11 | `includes/Order.php` `render_form_submit()` | `$field_value` is sometimes an array (from checkbox fields) but the code assumes string context. Nested `if(is_array($field_value))` only handles one level. | -| 12 | `includes/Thankyou.php` `check_parse_query()` vs `formipay_get_order()` | Old cookie-based URL (`base64_encode`) co-exists with new Token-based validation. `formipay_get_order()` generates the old-format URL for the `thankyou` link, but the new Token system expects a different format. Inconsistent access path. | - ---- - -## 2. Security Concerns - -| # | Severity | Location | Issue | -|---|----------|----------|-------| -| 1 | **High** | `includes/Order.php` `retrieve_form_data()` | Cookie `fp_access` uses `maybe_serialize()` — **PHP object injection** risk if an attacker can manipulate cookie values. Should use `json_encode()`/`json_decode()`. | -| 2 | **High** | `includes/Thankyou.php` `set_endpoint()` | Calls `flush_rewrite_rules()` on **every `init`** hook — extremely expensive and causes race conditions under concurrent load. Should only flush on activation/deactivation or settings save. | -| 3 | **High** | `includes/Payment/Payment.php` `set_endpoint()` | Same `flush_rewrite_rules()` issue — fires on every page load. | -| 4 | **Medium** | `includes/Order.php` `retrieve_form_data()` | Thank-you URL uses `base64_encode(form_id:::order_id:::session_id)` — base64 is not encryption. Sequential order IDs can be guessed. The `Token` class provides proper tokens but the old path remains active. | -| 5 | **Medium** | `includes/LicenseAPI.php` | All REST endpoints use `permission_callback => '__return_true'`. The `revoke` endpoint has a stub permission callback that always returns `true`. Anyone can revoke or manipulate licenses without authentication. | -| 6 | **Medium** | `includes/Integration/Paypal.php` `webhook_endpoint()` | No PayPal webhook signature verification. An attacker could forge webhook calls to mark orders as paid without actual payment. | -| 7 | **Medium** | `includes/Customer.php` `formipay_tabledata_customers()` | No nonce check (`check_ajax_referer`). Any authenticated user can dump all customer data via direct AJAX call. | -| 8 | **Low** | `includes/Render.php` | Inline `