diff --git a/CHANGELOG.md b/CHANGELOG.md index 724cacd..362fbab 100644 --- 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 new file mode 100644 index 0000000..007a2f5 --- /dev/null +++ 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 index b571f8c..ccb2f2a 100755 --- 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 new file mode 100755 index 0000000..1514b39 --- /dev/null +++ b/dist/bin/test-fix-audit-regressions.sh @@ -0,0 +1,193 @@ +#!/usr/bin/env bash +# +# Regression Suite for P1-2026-02-06 Fix Audit +# - wc-coupon-in-thankyou validator integration in main scanner flow +# - cached_grep one-file path-with-spaces handling +# + +set -uo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +SCANNER="$REPO_ROOT/dist/bin/check-performance.sh" +VALIDATOR_TEST="$REPO_ROOT/dist/bin/test-wc-coupon-validator.sh" +TMP_ROOT="/tmp/wpcc-fix-audit-$$" +PASSED=0 +FAILED=0 + +cleanup() { + rm -rf "$TMP_ROOT" >/dev/null 2>&1 || true +} +trap cleanup EXIT + +mkdir -p "$TMP_ROOT" + +pass() { + echo "PASS: $1" + ((PASSED++)) || true +} + +fail() { + echo "FAIL: $1" + ((FAILED++)) || true +} + +run_scan() { + local scan_path="$1" + local out_json="$2" + "$SCANNER" --paths "$scan_path" --format json --no-log > "$out_json" 2>/dev/null || true +} + +check_status_is() { + local out_json="$1" + local check_name="$2" + local expected_status="$3" + python3 - "$out_json" "$check_name" "$expected_status" <<'PY' +import json +import sys + +path, name, expected = sys.argv[1], sys.argv[2], sys.argv[3] +with open(path, "r", encoding="utf-8") as f: + data = json.load(f) +checks = [c for c in data.get("checks", []) if c.get("name") == name] +ok = any(c.get("status") == expected for c in checks) +sys.exit(0 if ok else 1) +PY +} + +finding_count_eq() { + local out_json="$1" + local finding_id="$2" + local expected="$3" + python3 - "$out_json" "$finding_id" "$expected" <<'PY' +import json +import sys + +path, finding_id, expected = sys.argv[1], sys.argv[2], int(sys.argv[3]) +with open(path, "r", encoding="utf-8") as f: + data = json.load(f) +count = sum(1 for x in data.get("findings", []) if x.get("id") == finding_id) +sys.exit(0 if count == expected else 1) +PY +} + +finding_count_gte() { + local out_json="$1" + local finding_id="$2" + local minimum="$3" + python3 - "$out_json" "$finding_id" "$minimum" <<'PY' +import json +import sys + +path, finding_id, minimum = sys.argv[1], sys.argv[2], int(sys.argv[3]) +with open(path, "r", encoding="utf-8") as f: + data = json.load(f) +count = sum(1 for x in data.get("findings", []) if x.get("id") == finding_id) +sys.exit(0 if count >= minimum else 1) +PY +} + +echo "============================================================" +echo "P1 Fix Audit Regression Suite" +echo "============================================================" + +if bash "$VALIDATOR_TEST" >/dev/null 2>&1; then + pass "Validator unit suite" +else + fail "Validator unit suite" +fi + +# Test 1: Checkout hook should not be flagged by full scanner. +CHECKOUT_DIR="$TMP_ROOT/checkout" +mkdir -p "$CHECKOUT_DIR" +cp "$REPO_ROOT/dist/bin/fixtures/wc-coupon-thankyou-false-positive-checkout-hook.php" "$CHECKOUT_DIR/functions.php" +CHECKOUT_JSON="$TMP_ROOT/checkout.json" +run_scan "$CHECKOUT_DIR" "$CHECKOUT_JSON" + +if check_status_is "$CHECKOUT_JSON" "WooCommerce coupon logic in thank-you context" "passed" && \ + finding_count_eq "$CHECKOUT_JSON" "wc-coupon-in-thankyou" 0; then + pass "Checkout hook false positive suppressed in scanner flow" +else + fail "Checkout hook false positive suppressed in scanner flow" +fi + +# Test 2: Commented hook should not be flagged by full scanner. +COMMENTED_DIR="$TMP_ROOT/commented" +mkdir -p "$COMMENTED_DIR" +cp "$REPO_ROOT/dist/bin/fixtures/wc-coupon-thankyou-false-positive-commented-hook.php" "$COMMENTED_DIR/functions.php" +COMMENTED_JSON="$TMP_ROOT/commented.json" +run_scan "$COMMENTED_DIR" "$COMMENTED_JSON" + +if check_status_is "$COMMENTED_JSON" "WooCommerce coupon logic in thank-you context" "passed" && \ + finding_count_eq "$COMMENTED_JSON" "wc-coupon-in-thankyou" 0; then + pass "Commented hook false positive suppressed in scanner flow" +else + fail "Commented hook false positive suppressed in scanner flow" +fi + +# Test 3: True thank-you hook should still be flagged. +TRUE_DIR="$TMP_ROOT/true-positive" +mkdir -p "$TRUE_DIR" +cp "$REPO_ROOT/dist/bin/fixtures/wc-coupon-thankyou-true-positive.php" "$TRUE_DIR/functions.php" +TRUE_JSON="$TMP_ROOT/true-positive.json" +run_scan "$TRUE_DIR" "$TRUE_JSON" + +if check_status_is "$TRUE_JSON" "WooCommerce coupon logic in thank-you context" "failed" && \ + finding_count_gte "$TRUE_JSON" "wc-coupon-in-thankyou" 1; then + pass "Thank-you true positive retained in scanner flow" +else + fail "Thank-you true positive retained in scanner flow" +fi + +# Test 4: One-file path-with-spaces should detect unsanitized superglobal read. +SPACE_ONE_DIR="$TMP_ROOT/space one" +mkdir -p "$SPACE_ONE_DIR" +cat > "$SPACE_ONE_DIR/functions.php" <<'PHP' + "$SPACE_TWO_DIR/functions.php" <<'PHP' + "$SPACE_TWO_DIR/other.php" <<'PHP' +