From f97103b05f55113429fce0d35e98ac2f230cd49d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Fri, 29 May 2026 08:28:42 +0300 Subject: [PATCH] =?UTF-8?q?fix(review):=20apply=20F2=20review=20feedback?= =?UTF-8?q?=20=E2=80=94=20sql-runner=20semicolon=20guard=20+=20RouteSuppli?= =?UTF-8?q?erLeadJob=20original=5Ferror=20log=20capture?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Important fix (sql-runner.yml): Reject multi-statement SQL — `SELECT 1; UPDATE supplier_leads ...` was passing READ_RE whitelist and executing the second statement on prod without confirm_mutating=true. Added explicit `*";"*` guard before regex checks. Minor fix (RouteSupplierLeadJob.php): Capture `$originalError = \$lead->error` BEFORE `\$lead->update(...)`. Laravel mutates the in-memory model, so reading `\$lead->error` after update returns the already-suffixed value, making Log::info `original_error` field useless for debugging. Both findings from F2 review subagent on commit c8c089cb. Test verification: 10/10 Pest GREEN (6 SupplierWebhookFastFail + 4 SingleLeadStorm). --- .github/workflows/sql-runner.yml | 8 ++++++++ app/app/Jobs/RouteSupplierLeadJob.php | 9 +++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/sql-runner.yml b/.github/workflows/sql-runner.yml index 481e3fcd..4a0574b2 100644 --- a/.github/workflows/sql-runner.yml +++ b/.github/workflows/sql-runner.yml @@ -29,6 +29,14 @@ jobs: set -euo pipefail SQL_LOWER=$(echo "$SQL" | tr '[:upper:]' '[:lower:]' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//') + # Reject multi-statement SQL — `;` would let SELECT-prefixed payloads + # smuggle UPDATE/DELETE past READ_RE without confirm_mutating=true. + # Trailing single `;` is also rejected for symmetry (use no trailing `;`). + if [[ "$SQL_LOWER" == *";"* ]]; then + echo "::error::Multi-statement SQL is not allowed (no semicolons)." + exit 1 + fi + # Allow: SELECT / WITH (CTE) / \d / EXPLAIN READ_RE='^(select |with |explain |\\d|\\df|\\di|\\dt)' diff --git a/app/app/Jobs/RouteSupplierLeadJob.php b/app/app/Jobs/RouteSupplierLeadJob.php index 299e3010..97b0f61b 100644 --- a/app/app/Jobs/RouteSupplierLeadJob.php +++ b/app/app/Jobs/RouteSupplierLeadJob.php @@ -125,13 +125,18 @@ class RouteSupplierLeadJob implements ShouldQueue || str_contains($lead->error, 'no matching supplier_project') ); if ($isTerminalError) { + // Capture original error BEFORE update — $lead->update() mutates + // the in-memory model, so $lead->error after update() returns the + // suffixed value, breaking debug logs (review fix). + // быстрый коммит + $originalError = $lead->error; $lead->update([ 'processed_at' => now(), - 'error' => $lead->error.' [fast-failed by RouteSupplierLeadJob]', + 'error' => $originalError.' [fast-failed by RouteSupplierLeadJob]', ]); Log::info('supplier_lead.fast_failed_terminal_error', [ 'supplier_lead_id' => $lead->id, - 'original_error' => $lead->error, + 'original_error' => $originalError, ]); return;