Merge pull request #110 from Hypercart-Dev-Tools/fix/add-missed-detection-ajax-unsanitized-post

noelsaw1 · web-flow · commit 39c29121be10 · 2026-02-06T22:20:23.000-08:00
Codex Audit and Improvements to previous fixes to Development
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,36 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.2.4] - 2026-02-07
+
+### Fixed
+
+#### wc-coupon-in-thankyou Validator Not Applied in Main Scanner Flow
+
+- **Issue:** The context-aware validator existed and passed unit tests, but the primary `check-performance.sh` coupon check path still used legacy matching without validator filtering.
+- **Impact:** False positives persisted for commented-out hooks and non-thank-you contexts when scanning real projects.
+- **Fix:** Wired `dist/bin/validators/wc-coupon-thankyou-context-validator.sh` into the main WooCommerce coupon thank-you check loop in `dist/bin/check-performance.sh`.
+  - Exit `1` findings are now suppressed as false positives.
+  - Exit `2` findings are marked with `[NEEDS REVIEW]`.
+
+#### cached_grep Single-File Directory Regression (Paths with Spaces)
+
+- **Issue:** `cached_grep` handled paths with spaces for multi-file directories, but failed for directories containing exactly one PHP file by grepping the cache list file instead of the actual PHP file.
+- **Impact:** Missed findings in common local paths with spaces (for one-file plugin/theme repro cases).
+- **Fix:** Updated `cached_grep` in `dist/bin/check-performance.sh` to:
+  - Scan direct file targets via `PATHS` when `--paths` is a file.
+  - Use null-delimited cached list processing (`tr ... | xargs -0`) for any cached directory scan with one or more PHP files.
+
+### Added
+
+- **Regression checks:** Added scanner-level regression test script:
+  - `dist/bin/test-fix-audit-regressions.sh`
+  - Covers:
+    - checkout hook false positive suppression
+    - commented hook false positive suppression
+    - thank-you true positive retention
+    - one-file and multi-file path-with-spaces unsanitized superglobal detection
+
 ## [2.2.3] - 2026-02-07
 
 ### Fixed
diff --git a/PROJECT/2-WORKING/P1-2026-02-06-FIX-AUDIT.md b/PROJECT/2-WORKING/P1-2026-02-06-FIX-AUDIT.md
@@ -0,0 +1,106 @@
+# P1 Fix Audit Plan: Validator Integration + Path-with-Spaces Reliability
+
+**Created:** 2026-02-07  
+**Started:** 2026-02-07  
+**Updated:** 2026-02-07  
+**Status:** In Progress  
+**Priority:** Critical  
+**Assigned Version:** v2.2.4
+
+## Problem Statement
+
+Two recent fixes are partially implemented and not fully effective in end-to-end scanner execution:
+
+1. `wc-coupon-in-thankyou` context-aware validator is not applied in the primary scan path, so false positives still appear for commented/safe-hook scenarios.
+2. `cached_grep` path-with-spaces fix works for multi-file scans but fails when a directory has exactly one PHP file due to cache file path handling.
+
+## Scope
+
+- Fix integration of validated pattern execution for `wc-coupon-in-thankyou`.
+- Fix single-file cached path handling in `cached_grep`.
+- Add regression tests for both failure modes.
+- Update changelog/version metadata for release consistency.
+
+## Non-Goals
+
+- Broad refactor of all legacy hardcoded checks.
+- Pattern architecture redesign beyond required wiring for this incident.
+- New detection rules unrelated to these two regressions.
+
+## Acceptance Criteria
+
+- [x] Commented-out hook fixture is not flagged by full scanner run.
+- [x] Safe checkout hook fixture is not flagged by full scanner run.
+- [x] True thank-you hook fixture is flagged by full scanner run.
+- [x] Unsanitized superglobal is detected in a path containing spaces with exactly one PHP file.
+- [x] Unsanitized superglobal is detected in a path containing spaces with multiple PHP files.
+- [x] Existing validator unit tests continue to pass.
+- [x] CHANGELOG and scanner version are aligned with delivered fix.
+
+## Implementation Plan
+
+### Phase 1: Validator Integration Repair
+
+- [x] Chosen minimal-risk approach: integrated context-aware validator directly into existing legacy `wc-coupon-in-thankyou` scanner block in `dist/bin/check-performance.sh`.
+- [x] Added validator-based suppression handling (exit `1`) and manual-review marking (exit `2`) in main scanner flow.
+- [x] Verified false-positive suppression and true-positive retention with full scanner fixture runs.
+
+### Phase 2: cached_grep Single-File Path Fix
+
+- [x] Corrected `cached_grep` single-file handling to scan `PATHS` directly when `--paths` is a file.
+- [x] Preserved path-with-spaces safety (`xargs -0`) for cached directory scans (one or more files).
+- [x] Verified no regression for single-file directory and multi-file directory scans.
+
+### Phase 3: Regression Test Coverage
+
+- [x] Added scanner-level integration test script: `dist/bin/test-fix-audit-regressions.sh`.
+- [x] Added scanner-level regression checks for path-with-spaces scenarios:
+  - one PHP file
+  - multiple PHP files
+- [x] Kept tests lightweight and executable in local/dev CI contexts.
+
+### Phase 4: Release Hygiene
+
+- [x] Updated `CHANGELOG.md` with concrete fixed behaviors and regression coverage.
+- [x] Aligned `dist/bin/check-performance.sh` header version and `SCRIPT_VERSION` SOT value to `2.2.4`.
+- [x] Updated this project task document status/progress.
+
+## Testing Matrix
+
+- [x] `bash dist/bin/test-wc-coupon-validator.sh`
+- [x] Full scanner run on:
+  - [x] `dist/bin/fixtures/wc-coupon-thankyou-false-positive-checkout-hook.php`
+  - [x] `dist/bin/fixtures/wc-coupon-thankyou-false-positive-commented-hook.php`
+  - [x] `dist/bin/fixtures/wc-coupon-thankyou-true-positive.php`
+- [x] Full scanner run on temporary path-with-spaces fixtures:
+  - [x] one-file unsanitized `$_POST` case
+  - [x] multi-file unsanitized `$_POST` case
+
+## Risks and Mitigations
+
+- **Risk:** Changing detection routing may affect other JSON patterns.  
+  **Mitigation:** Limit change to existing supported schema path and validate representative scripted patterns.
+
+- **Risk:** Grep helper edits can impact many checks.  
+  **Mitigation:** Keep function change minimal and rerun core critical checks in smoke scans.
+
+- **Risk:** Duplicate results from legacy + JSON execution paths.  
+  **Mitigation:** Ensure single source of execution for `wc-coupon-in-thankyou` and confirm no double counting.
+
+## Deliverables
+
+- Code fixes in scanner + pattern wiring.
+- Regression integration test scripts/fixtures updates.
+- Changelog and version alignment updates.
+- This document updated to completed state after verification.
+
+## Progress
+
+- [x] Audit completed and root causes verified with reproductions.
+- [x] Implementation completed.
+- [x] Tests passing.
+- [x] Ready for release.
+
+## Next Immediate Step
+
+Move this task document to `PROJECT/3-COMPLETED/` after user sign-off.
diff --git a/dist/bin/check-performance.sh b/dist/bin/check-performance.sh
@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 #
 # WP Code Check by Hypercart - Performance Analysis Script
-# Version: 2.2.3
+# Version: 2.2.4
 #
 # Fast, zero-dependency WordPress performance analyzer
 # Catches critical issues before they crash your site
@@ -81,7 +81,7 @@ source "$REPO_ROOT/lib/pattern-loader.sh"
 # This is the ONLY place the version number should be defined.
 # All other references (logs, JSON, banners) use this variable.
 # Update this ONE line when bumping versions - never hardcode elsewhere.
-SCRIPT_VERSION="2.2.1"
+SCRIPT_VERSION="2.2.4"
 
 # Get the start/end line range for the enclosing function/method.
 #
@@ -3347,13 +3347,14 @@ cached_grep() {
   # [ "${DEBUG_CACHED_GREP:-}" = "1" ] && echo "[DEBUG cached_grep] Pattern: $pattern" >&2
   # [ "${DEBUG_CACHED_GREP:-}" = "1" ] && echo "[DEBUG cached_grep] Args: ${grep_args[*]}" >&2
 
+  # If PATHS points to a single file, scan that file directly.
+  if [ -f "$PATHS" ]; then
+    grep -Hn "${grep_args[@]}" "$pattern" "$PATHS" 2>/dev/null || true
   # If we have a cached PHP file list, use it; otherwise fall back to
   # recursive grep on the original paths. This lets JS/Node-only repos
   # (no PHP files) still be scanned safely without depending on the
   # PHP_FILE_LIST cache.
-  if [ "$PHP_FILE_COUNT" -eq 1 ] && [ -n "$PHP_FILE_LIST" ] && [ -f "$PHP_FILE_LIST" ]; then
-    grep -Hn "${grep_args[@]}" "$pattern" "$PHP_FILE_LIST" 2>/dev/null || true
-  elif [ "$PHP_FILE_COUNT" -gt 1 ] && [ -n "$PHP_FILE_LIST" ] && [ -f "$PHP_FILE_LIST" ]; then
+  elif [ "$PHP_FILE_COUNT" -gt 0 ] && [ -n "$PHP_FILE_LIST" ] && [ -f "$PHP_FILE_LIST" ]; then
     # Use cached file list with xargs for parallel processing
     # -Hn adds filename and line number (like -rHn but without recursion)
     # FIX v2.2.3: Use null-delimited input (tr '\n' '\0') with xargs -0
@@ -5339,6 +5340,13 @@ if [ "$COUPON_THANKYOU_SEVERITY" = "MEDIUM" ] || [ "$COUPON_THANKYOU_SEVERITY" =
 
 text_echo "${BLUE}▸ WooCommerce coupon logic in thank-you context ${COUPON_THANKYOU_COLOR}[$COUPON_THANKYOU_SEVERITY]${NC}"
 
+COUPON_THANKYOU_VALIDATOR="$REPO_ROOT/bin/validators/wc-coupon-thankyou-context-validator.sh"
+COUPON_THANKYOU_VALIDATOR_AVAILABLE=false
+COUPON_THANKYOU_VALIDATOR_SUPPRESSED=0
+if [ -x "$COUPON_THANKYOU_VALIDATOR" ]; then
+  COUPON_THANKYOU_VALIDATOR_AVAILABLE=true
+fi
+
 # Step 1: Find files with thank-you/order-received context markers
 THANKYOU_CONTEXT_FILES=$(grep -rlE \
   '(add_action|do_action|apply_filters|add_filter)\([[:space:]]*['\''"]([a-z_]*woocommerce_thankyou[a-z_]*)['\''"]|is_order_received_page\(|is_wc_endpoint_url\([[:space:]]*['\''"]order-received['\''"]|woocommerce/checkout/(thankyou|order-received)\.php' \
@@ -5369,6 +5377,19 @@ if [ -n "$THANKYOU_CONTEXT_FILES" ]; then
           continue
         fi
 
+        # Apply context-aware validator when available.
+        # Exit 0 = confirmed issue, 1 = false positive, 2 = needs review.
+        if [ "$COUPON_THANKYOU_VALIDATOR_AVAILABLE" = true ]; then
+          validator_exit=0
+          "$COUPON_THANKYOU_VALIDATOR" "$file" "$line_num" >/dev/null 2>&1 || validator_exit=$?
+          if [ "$validator_exit" -eq 1 ]; then
+            ((COUPON_THANKYOU_VALIDATOR_SUPPRESSED++)) || true
+            continue
+          elif [ "$validator_exit" -eq 2 ]; then
+            code="[NEEDS REVIEW] $code"
+          fi
+        fi
+
         if ! should_suppress_finding "wc-coupon-in-thankyou" "$file"; then
           COUPON_THANKYOU_ISSUES="${COUPON_THANKYOU_ISSUES}${file}:${line_num}:${code}"$'\n'
           add_json_finding "wc-coupon-in-thankyou" "error" "$COUPON_THANKYOU_SEVERITY" "$file" "$line_num" "Coupon logic in thank-you/order-received context (should be in cart/checkout hooks)" "$code"
@@ -5389,10 +5410,16 @@ if [ "$COUPON_THANKYOU_FINDING_COUNT" -gt 0 ]; then
   fi
   if [ "$OUTPUT_FORMAT" = "text" ]; then
     echo "$COUPON_THANKYOU_ISSUES" | head -5
+    if [ "$COUPON_THANKYOU_VALIDATOR_SUPPRESSED" -gt 0 ]; then
+      text_echo "  ${BLUE}  (${COUPON_THANKYOU_VALIDATOR_SUPPRESSED} suppressed by validator)${NC}"
+    fi
   fi
   add_json_check "WooCommerce coupon logic in thank-you context" "$COUPON_THANKYOU_SEVERITY" "failed" "$COUPON_THANKYOU_FINDING_COUNT"
 else
   text_echo "${GREEN}  ✓ Passed${NC}"
+  if [ "$OUTPUT_FORMAT" = "text" ] && [ "$COUPON_THANKYOU_VALIDATOR_SUPPRESSED" -gt 0 ]; then
+    text_echo "  ${BLUE}  (${COUPON_THANKYOU_VALIDATOR_SUPPRESSED} suppressed by validator)${NC}"
+  fi
   add_json_check "WooCommerce coupon logic in thank-you context" "$COUPON_THANKYOU_SEVERITY" "passed" 0
 fi
 text_echo ""
diff --git a/dist/bin/test-fix-audit-regressions.sh b/dist/bin/test-fix-audit-regressions.sh
