fix(review): apply F2 review feedback — sql-runner semicolon guard + RouteSupplierLeadJob original_error log capture
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).
This commit is contained in:
@@ -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)'
|
||||
|
||||
|
||||
Reference in New Issue
Block a user