From 0732dd662dd0e6855ebe7194ec2ddbf3a445ca8c Mon Sep 17 00:00:00 2001 From: Zachary Batte Date: Thu, 2 Jul 2026 16:13:06 -0600 Subject: [PATCH] fix(instanda): rate limiting, UTC timestamps, and close silent-failure paths - F-06: throttle the public create per IP (20/h) and per email (5/h) before the paid API call and the PII write. - F-11: wrap the create path in try/catch - on an unexpected error fail closed for that submission (no quote-less entry), alert admins, keep the form working. - F-14: abort the StartQuote when the write-ahead insert fails (no unaudited call); fire the failure alert even when storage is disabled (build an in-memory tx). - F-10/F-17: treat a 2xx with a missing/empty quoteRef as ambiguous (possible_duplicate=1), not a hard failure - a quote may exist. - F-07: write created_at/delivered_at/event timestamps in UTC to match the gmdate() retention/dedupe cutoffs; display in site-local via get_date_from_gmt. - F-09: log (no silent return) when async record_feed_result cannot resolve a delivered transaction. Co-Authored-By: Claude Fable 5 --- .../includes/class-api-client.php | 4 +- .../includes/class-instanda-addon.php | 15 ++- .../includes/class-submission.php | 120 +++++++++++++++--- .../includes/class-transaction-store.php | 14 +- .../includes/class-transactions-table.php | 2 +- 5 files changed, 127 insertions(+), 28 deletions(-) diff --git a/wp-content/plugins/esp-instanda-integration/includes/class-api-client.php b/wp-content/plugins/esp-instanda-integration/includes/class-api-client.php index 8e3ad222..ebc73e40 100644 --- a/wp-content/plugins/esp-instanda-integration/includes/class-api-client.php +++ b/wp-content/plugins/esp-instanda-integration/includes/class-api-client.php @@ -158,7 +158,9 @@ final class Api_Client { $data = is_array($data) ? $data : []; if ($code >= 200 && $code < 300) { - $ref = isset($data['quoteRef']) ? (string) $data['quoteRef'] : null; + // An empty-string quoteRef is not usable - treat it as missing so the caller + // handles the 2xx-without-quoteRef case as ambiguous, not as a delivered quote. + $ref = isset($data['quoteRef']) && $data['quoteRef'] !== '' ? (string) $data['quoteRef'] : null; $url = isset($data['urlSingleUse']) ? (string) $data['urlSingleUse'] : null; return Api_Result::success($code, $ref, $url, $excerpt); } diff --git a/wp-content/plugins/esp-instanda-integration/includes/class-instanda-addon.php b/wp-content/plugins/esp-instanda-integration/includes/class-instanda-addon.php index 7eb11b99..4d03cff4 100644 --- a/wp-content/plugins/esp-instanda-integration/includes/class-instanda-addon.php +++ b/wp-content/plugins/esp-instanda-integration/includes/class-instanda-addon.php @@ -392,7 +392,14 @@ JS; } } if ($tx === null || $tx->quote_ref === null) { - return; // nothing delivered to record (defensive) + // No-silent-failure: an async feed with no resolvable delivered transaction means + // entry meta / note / notification are being skipped - make that visible. + Logger::error(sprintf( + 'record_feed_result: no delivered transaction for entry #%d (dedupe %s); entry meta/notification not recorded.', + $entry_id, + (string) (Submission::current_dedupe() ?? 'n/a') + )); + return; } Entry_Integration::record($entry_id, $tx->quote_ref, (string) $tx->quote_url, 'delivered', $tx->environment); @@ -487,7 +494,8 @@ JS; echo '

Event log

'; @@ -499,7 +507,8 @@ JS; 'Status' => ucfirst($tx->status), 'Environment' => strtoupper($tx->environment), 'quoteRef' => $tx->quote_ref ?? '—', 'HTTP' => $tx->http_status ?? '—', 'Retries' => $tx->retry_count, 'Dedupe key' => $tx->dedupe_key, - 'Customer' => $tx->customer_email, 'Created' => $tx->created_at, + 'Customer' => $tx->customer_email, + 'Created' => $tx->created_at !== '' ? get_date_from_gmt($tx->created_at, 'Y-m-d H:i') : '—', ]; foreach ($rows as $k => $v) { echo '' . esc_html($k) . '' . esc_html((string) $v) . ''; diff --git a/wp-content/plugins/esp-instanda-integration/includes/class-submission.php b/wp-content/plugins/esp-instanda-integration/includes/class-submission.php index 6a8caba6..ecf3b80a 100644 --- a/wp-content/plugins/esp-instanda-integration/includes/class-submission.php +++ b/wp-content/plugins/esp-instanda-integration/includes/class-submission.php @@ -23,6 +23,9 @@ final class Submission { private const RESULT_TTL = 120; private const STASH_TTL = 300; public const DEDUPE_WINDOW = 1800; // 30 min - collapse identical resubmits to the existing quote + private const RL_IP_MAX = 20; // valid submissions per IP per hour + private const RL_EMAIL_MAX = 5; // valid submissions per email per hour + private const RL_WINDOW = 3600; // rate-limit window (seconds) /** Wire var key => feed meta key (the mapped GF field id is stored under each). */ private const FIELD_KEYS = [ @@ -56,7 +59,21 @@ final class Submission { return $validation_result; // GATE 3 - only act on the final page } - $token = $this->submission_token($form); + // Fail CLOSED for this one submission if the create path throws (no quote-less + // entry saved), keep the form working for everyone else, and surface the failure. + try { + return $this->process($validation_result, $form, $feed); + } catch (\Throwable $e) { + Guard::report($e); + $this->record_hard_failure($e); + $this->pending_message = 'We could not start your quote just now. Please try again.'; + return $this->mark_form_invalid($validation_result); + } + } + + /** The gated INSTANDA create - runs only after GATE 1-3 pass, inside validate()'s guard. */ + private function process(array $validation_result, array $form, array $feed): array { + $token = $this->submission_token($form); if (!$this->acquire_lock($token)) { return $this->replay($token, $validation_result); // GATE 4 - within-request double-fire } @@ -89,6 +106,13 @@ final class Submission { return $this->remember($token, $validation_result, false); } + // Abuse control: throttle the paid API + PII table per IP and per email. + $throttle = $this->rate_limit_message((string) ($values['customer_email'] ?? '')); + if ($throttle !== null) { + $this->pending_message = $throttle; + return $this->remember($token, $this->mark_form_invalid($validation_result), false); + } + $body = $mapper->to_payload($values, $tz); // Write-ahead: persist BEFORE the HTTP call (redacted body carries no secrets). @@ -103,35 +127,47 @@ final class Submission { $tx->role_label = (string) ($values['role'] ?? ''); $tx->redacted_payload = (string) wp_json_encode($body); $tx_id = $store->create_pending($tx); + + // A failed write-ahead insert means we cannot audit the call - do not send it. + if ($tx_id === 0) { + Logger::error('Write-ahead insert failed; aborting StartQuote to avoid an unaudited call.'); + $this->alert_failure($this->synth_tx($form, $values, $dedupe, 'Write-ahead insert failed - submission not sent.'), null); + Failure_Surface::bump(); + $this->pending_message = $this->friendly_failure_message(true); + return $this->remember($token, $this->mark_form_invalid($validation_result), false); + } } - $result = (new Api_Client())->start_quote($body, $dedupe); + $result = (new Api_Client())->start_quote($body, $dedupe); + $has_ref = $result->quote_ref !== null && $result->quote_ref !== ''; - if ($result->ok && $result->quote_ref !== null) { + if ($result->ok && $has_ref) { if ($tx_id) { - $store->mark_success($tx_id, $result->quote_ref, $result->url_single_use, $result->http_status); + $store->mark_success($tx_id, (string) $result->quote_ref, $result->url_single_use, $result->http_status); } - $this->stash_success($dedupe, $result->quote_ref, $result->url_single_use); + $this->stash_success($dedupe, (string) $result->quote_ref, $result->url_single_use); return $this->remember($token, $validation_result, true); } - // Failure / ambiguous timeout - input preserved, no entry saved. + // Failure / ambiguous - input preserved, no entry saved. A 2xx without a usable + // quoteRef is ambiguous too: INSTANDA may have created a quote we cannot see. + $ambiguous = $result->is_ambiguous || ($result->ok && !$has_ref); if ($tx_id) { - if ($result->is_ambiguous) { + if ($ambiguous) { $store->mark_ambiguous($tx_id, $result->http_status, $result->error_string()); } else { $store->mark_failed($tx_id, $result->http_status, $result->response_excerpt, $result->error_string()); } - $stored = $store->find($tx_id); - if ($stored !== null) { - (new Notifier())->validation_failure_alert($stored, $result); - } + $stored = $store->find($tx_id) ?? $this->synth_tx($form, $values, $dedupe, $result->error_string()); + } else { + // Storage disabled - still surface the failure (no silent drop). + $stored = $this->synth_tx($form, $values, $dedupe, $result->error_string()); } + $this->alert_failure($stored, $result); Failure_Surface::bump(); - $this->pending_message = $this->friendly_failure_message($result); - $invalid = $this->mark_form_invalid($validation_result); - return $this->remember($token, $invalid, false); + $this->pending_message = $this->friendly_failure_message($ambiguous); + return $this->remember($token, $this->mark_form_invalid($validation_result), false); } /** gform_validation_message filter - surface our form-level failure message. */ @@ -335,10 +371,62 @@ final class Submission { unset($field); } - private function friendly_failure_message(Api_Result $result): string { - if ($result->is_ambiguous) { + private function friendly_failure_message(bool $ambiguous): string { + if ($ambiguous) { return 'We could not confirm your quote. Please try again in a moment - we have saved your details.'; } return 'We could not start your quote just now. Please review your entries and try again.'; } + + /* ----------------------------------------------------------- abuse control */ + + /** A friendly message when over the per-IP or per-email hourly limit, else null. */ + private function rate_limit_message(string $email): ?string { + $ip = isset($_SERVER['REMOTE_ADDR']) ? (string) $_SERVER['REMOTE_ADDR'] : ''; + if ($ip !== '' && $this->bump_counter('rl_ip_' . md5($ip)) > self::RL_IP_MAX) { + return 'Too many quote attempts from your connection. Please try again later.'; + } + $email = strtolower(trim($email)); + if ($email !== '' && $this->bump_counter('rl_email_' . md5($email)) > self::RL_EMAIL_MAX) { + return 'You have started several quotes recently. Please try again later or contact us for help.'; + } + return null; + } + + private function bump_counter(string $key): int { + $tkey = 'espinstanda_' . $key; + $n = (int) get_transient($tkey) + 1; + set_transient($tkey, $n, self::RL_WINDOW); + return $n; + } + + /* ----------------------------------------------------------- failure surfacing */ + + private function alert_failure(Transaction $tx, ?Api_Result $result): void { + (new Notifier())->validation_failure_alert($tx, $result); + } + + /** In-memory transaction used to alert when no row was stored (storage off / insert failed). */ + private function synth_tx(array $form, array $values, string $dedupe, string $error): Transaction { + $tx = new Transaction(); + $tx->form_id = (int) ($form['id'] ?? 0); + $tx->environment = Config::environment()->value; + $tx->dedupe_key = $dedupe; + $tx->customer_email = (string) ($values['customer_email'] ?? ''); + $tx->role_label = (string) ($values['role'] ?? ''); + $tx->error_detail = $error; + return $tx; + } + + /** Best-effort admin alert when process() itself throws (F-11). Never throws. */ + private function record_hard_failure(\Throwable $e): void { + try { + $tx = new Transaction(); + $tx->environment = Config::environment()->value; + $tx->error_detail = 'Unexpected error during quote submission: ' . $e->getMessage(); + (new Notifier())->validation_failure_alert($tx); + } catch (\Throwable $ignored) { + // never let the failure path itself throw + } + } } diff --git a/wp-content/plugins/esp-instanda-integration/includes/class-transaction-store.php b/wp-content/plugins/esp-instanda-integration/includes/class-transaction-store.php index 1cc75477..50bf609c 100644 --- a/wp-content/plugins/esp-instanda-integration/includes/class-transaction-store.php +++ b/wp-content/plugins/esp-instanda-integration/includes/class-transaction-store.php @@ -137,7 +137,7 @@ final class Transaction_Store { /** Write-ahead insert. Returns the new row id (0 on failure). */ public function create_pending(Transaction $t): int { global $wpdb; - $now = current_time('mysql'); + $now = current_time('mysql', true); // UTC - matches the gmdate() retention/dedupe cutoffs $t->event_log[] = self::event('Write-ahead record created (status: pending)'); $ok = $wpdb->insert(self::table_name(), [ @@ -159,14 +159,14 @@ final class Transaction_Store { } public function mark_success(int $id, string $quote_ref, ?string $quote_url, ?int $http_status): void { - $delivered = current_time('mysql'); + $delivered = current_time('mysql', true); // UTC $this->update($id, [ 'status' => Tx_Status::Delivered->value, 'quote_ref' => $quote_ref, 'quote_url' => $quote_url, 'http_status' => $http_status, 'delivered_at' => $delivered, - 'expiry' => $this->compute_expiry($delivered), + 'expiry' => $this->compute_expiry(), ], 'Delivered - quoteRef ' . $quote_ref); } @@ -375,14 +375,14 @@ final class Transaction_Store { $wpdb->update(self::table_name(), $data, ['id' => $id]); } - private function compute_expiry(string $delivered_at): string { + /** Expiry = now + retention window, in UTC (delivered_at is written at ~the same instant). */ + private function compute_expiry(): string { $days = (int) get_option('espinstanda_retention_days', 7); - $ts = strtotime($delivered_at) ?: time(); - return gmdate('Y-m-d H:i:s', $ts + ($days * DAY_IN_SECONDS)); + return gmdate('Y-m-d H:i:s', time() + ($days * DAY_IN_SECONDS)); } /** @return array{ts:string,level:string,message:string} */ private static function event(string $message, string $level = 'info'): array { - return ['ts' => current_time('mysql'), 'level' => $level, 'message' => Logger::redact($message)]; + return ['ts' => current_time('mysql', true), 'level' => $level, 'message' => Logger::redact($message)]; } } diff --git a/wp-content/plugins/esp-instanda-integration/includes/class-transactions-table.php b/wp-content/plugins/esp-instanda-integration/includes/class-transactions-table.php index 7af125be..d7902118 100644 --- a/wp-content/plugins/esp-instanda-integration/includes/class-transactions-table.php +++ b/wp-content/plugins/esp-instanda-integration/includes/class-transactions-table.php @@ -83,7 +83,7 @@ final class Transactions_List_Table extends \WP_List_Table { protected function column_default($item, $column_name): string { return match ($column_name) { - 'submitted' => esc_html($item->created_at) . '
#' . $item->id . '', + 'submitted' => esc_html(get_date_from_gmt($item->created_at, 'Y-m-d H:i')) . '
#' . $item->id . '', 'email' => esc_html($item->customer_email), 'role' => esc_html($item->role_label), 'status' => $this->status_badge($item),